Attention is currently required from: Maxim Polyakov, Angel Pons.
Patch set 12:Code-Review +2
5 comments:
Patchset:
Angel, I marked some of your comments as resolved, hope you don't mind.
One remark: Sometimes it's called `pin` sometimes `pad` that's a bit
irritating (however I only noticed after going through the earlier
comments).
File src/ec/kontron/kempld/chip.h:
Patch Set #5, Line 38: gpio_pad
I would prefer gpio in the option name (array), if you don't mind?
SGTM.
File src/ec/kontron/kempld/kempld_gpio.c:
Patch Set #5, Line 16: offset
I would prefer pin_num as the variable name.
LGTM.
int kempld_gpio_direction_input(unsigned int offset)
{
int status;
if (kempld_get_mutex(100) < 0)
return -1;
status = kempld_read8((u8)KEMPLD_GPIO_DIR_NUM(offset));
status &= ~KEMPLD_GPIO_MASK(offset);
kempld_write8((u8)KEMPLD_GPIO_DIR_NUM(offset), (u8)status);
kempld_release_mutex();
return 0;
}
int kempld_gpio_direction_output(unsigned int offset, int value)
{
int status;
if (kempld_get_mutex(100) < 0)
return -1;
status = kempld_read8((u8)KEMPLD_GPIO_LVL_NUM(offset));
if (value)
status |= KEMPLD_GPIO_MASK(offset);
else
status &= ~KEMPLD_GPIO_MASK(offset);
kempld_write8((u8)KEMPLD_GPIO_LVL_NUM(offset), (u8)status);
status = kempld_read8((u8)KEMPLD_GPIO_DIR_NUM(offset));
status |= KEMPLD_GPIO_MASK(offset);
kempld_write8((u8)KEMPLD_GPIO_DIR_NUM(offset), (u8)status);
kempld_release_mutex();
return 0;
}
Thanks for your time on this work, but I would like to separate operations and use functions, if you […]
New code looks good to me.
File src/ec/kontron/kempld/kempld_internal.h:
Patch Set #12, Line 35: const
Did I write this? It's not meaningful in a forward declaration.
To view, visit change 47595. To unsubscribe, or for help writing mail filters, visit settings.