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
pistachio: marduk: Switch on heartbeat LED #20
Conversation
{ | ||
u32 reg, val; | ||
|
||
/* make GPIO output */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you refactor to use a select_* function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
692697d
to
24cf58a
Compare
{ | ||
u32 reg, val; | ||
reg = PISTACHIO_GPIO + 0x204 + (((mfio) / 16) * 0x24); | ||
val = 0x10000 | ((output) ? (1) : (0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we get rid of ternary operator by directly ORing output ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am leaving as it is to be consistent with the rest of the code.
e82437d
to
71acd09
Compare
u32 reg, val; | ||
|
||
reg = PISTACHIO_GPIO + 0x208 + (((mfio) / 16) * 0x24); | ||
val = 0x10000 | ((state) ? (1) : (0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
state is already bool, why do we need ternary operator ?
@@ -103,23 +103,20 @@ static struct pistachio_mfio_reg pistachio_mfio_regs[] = | |||
{MFIO_89_FUNC_SEL_START, MFIO_89_FUNC_SEL_END}, | |||
}; | |||
|
|||
static void pistachio_select_gpio(u32 mfio, bool gpio) | |||
static void pistachio_select_gpio(u32 mfio, bool output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of pistachio_select_gpio(), can we name it pistachio_configure_gpio() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
void mfio_setup_led(void) | ||
{ | ||
/* make GPIO output */ | ||
pistachio_select_gpio(76, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have macro for this, as GPIO_OUTPUT and GPIO_INPUT
pistachio_select_gpio configures gpio as an input or an output. Inputs are configured not to generate interrupts. Signed-off-by: Francois Berder <francois.berder@imgtec.com>
Signed-off-by: Francois Berder <francois.berder@imgtec.com>
Signed-off-by: Francois Berder <francois.berder@imgtec.com>
e28c861
to
9745177
Compare
Signed-off-by: Francois Berder <francois.berder@imgtec.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and see it working and nothing else seems to be broken
this connects CreatorDev/openwrt#45
Signed-off-by: Francois Berder francois.berder@imgtec.com