Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added constants to cas module #348

Merged
merged 2 commits into from Oct 30, 2017
Merged

Added constants to cas module #348

merged 2 commits into from Oct 30, 2017

Conversation

karl-malakoff
Copy link
Contributor

Generated header now includes constant for number of devices

@karl-malakoff
Added constants to cas module
7dc8c94 
Generated header now includes constant for number of devices
@karl-malakoff karl-malakoff mentioned this pull request Aug 11, 2017
Closed
mithro
mithro reviewed Aug 11, 2017
View reviewed changes
Copy link
Member

@mithro mithro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. I don't think you need the name=XXX arguments to CSRConstant, they should automagically pick up the name based on your assignments.

gateware/cas.py Outdated
@@ -30,6 +30,8 @@ def __init__(self, platform, clk_freq):
user_leds[i].eq(leds[i]),
]

self.num_leds = CSRConstant(len(user_leds), name='num_leds')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need name='num_leds' here?

gateware/cas.py Outdated
@@ -46,6 +48,8 @@ def __init__(self, platform, clk_freq):
switches[i].eq(~user_sws[i]),
]

self.num_sws = CSRConstant(len(user_sws), name='num_switches')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need name='num_switches'

gateware/cas.py Outdated
@@ -73,3 +77,4 @@ def __init__(self, platform, clk_freq):
self.submodules += [btn_timer]

self.buttons_ev.finalize()
self.num_buttons = CSRConstant(len(user_btns), name='num_buttons')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need name='num_buttons' here?

@mithro
Rename CSRConstants
b6a0fd9 
 * Remove unneeded name property.
 * Name the CSRs as `_xxx_count`, constant ends up as `cas_xxx_count_read()`.
@karl-malakoff
Copy link
Contributor Author

Thanks Tim, I'll need to go back and change the micropython code to match the constant names. Was there anything else you wanted me to do with this one?

@mithro
Copy link
Member

mithro commented Oct 30, 2017

@karl-malakoff - Sorry for the slow reply, I wanted to check the constant names looked good and they do now.

See

$ grep cas build/mimasv2_base_lm32//test/csr.csv 
csr_base,cas,0xe0006800,,
csr_register,cas_leds_out,0xe0006800,1,rw
csr_register,cas_switches_in,0xe0006804,1,ro
csr_register,cas_buttons_ev_status,0xe0006808,1,rw
csr_register,cas_buttons_ev_pending,0xe000680c,1,rw
csr_register,cas_buttons_ev_enable,0xe0006810,1,rw
constant,cas_leds_count,8,,
constant,cas_switches_count,8,,
constant,cas_buttons_count,5,,
$
$ grep -R COUNT build/mimasv2_base_lm32/software/include/generated/csr.h 
#define CAS_LEDS_COUNT 8
#define CAS_SWITCHES_COUNT 8
#define CAS_BUTTONS_COUNT 5
$

@mithro mithro merged commit 1b18887 into timvideos:master Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mithro mithro mithro left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@karl-malakoff @mithro