Attention is currently required from: Maxim Polyakov.
Patch set 12:Code-Review +1
4 comments:
File src/ec/kontron/kempld/chip.h:
Patch Set #5, Line 38: gpio_pad
SGTM.
Ack
File src/ec/kontron/kempld/kempld_gpio.c:
Patch Set #5, Line 16: offset
LGTM.
Ack
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;
}
New code looks good to me.
Looks good to me as well. While my approach skips acquiring the kempld mutex for `KEMPLD_GPIO_DEFAULT` (a micro-optimisation), your solution is much more elegant and easier to understand. And I prefer easy-to-maintain code over code that's sliiiiightly faster but considerably messier.
File src/ec/kontron/kempld/kempld_internal.h:
#define KEMPLD_GPIO_MASK(x) (1 << ((x) % 8))
#define KEMPLD_GPIO_DIR(pad_num) (0x40 + (pad_num) / 8)
#define KEMPLD_GPIO_LVL(pad_num) (0x42 + (pad_num) / 8)
nit: Use the same name for the macro parameters, e.g. `pin_num`?
To view, visit change 47595. To unsubscribe, or for help writing mail filters, visit settings.