Re: [PATCH v2 2/8] Add Advantech EIO GPIO driver

Hi Ramiro,

thanks for your patch!

On Tue, Jul 14, 2026 at 5:54 PM Ramiro Oliveira
[off-list ref] wrote:

+static int get_dir(struct gpio_chip *chip, unsigned int offset)
+{
+       u8 dir;
+       int ret;
+
+       ret = pmc_read(chip->parent, EIO_GPIO_PIN_DIR, offset, &dir);
+       if (ret)
+               return ret;
+
+       return dir ? 0 : 1;

Please use:
return dir ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;

+static int dir_input(struct gpio_chip *chip, unsigned int offset)
+{
+       u8 dir = 0;

Use some local define for this stuff like:

#define EIO_GPIO_DIR_IN 0
#define EIO_GPIO_DIR_OUT 1

+static int dir_output(struct gpio_chip *chip, unsigned int offset, int value)
+{
+       u8 dir = 1;

Also here.

+       u8 val = value;
+
+       pmc_write(chip->parent, EIO_GPIO_PIN_DIR, offset, &dir);
+
+       return pmc_write(chip->parent, EIO_GPIO_PIN_LEVEL, offset, &val);
+}
+
+static int gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+       u8 level;
+       int ret;
+
+       ret = pmc_read(chip->parent, EIO_GPIO_PIN_LEVEL, offset, &level);
+       if (ret)
+               return ret;
+
+       return level;

return !!level;

so you clamp this to [0, 1]

+static int check_support(struct device *dev)

Check support for what? Spell it out.

+{
+       u8  data;
+       int ret;
+
+       ret = pmc_read(dev, EIO_GPIO_STATUS, 0, &data);
+       if (ret)
+               return ret;
+
+       if ((data & 0x01) == 0)
+               return -EOPNOTSUPP;

Use:

#define EIO_GPIO_HAS_SUPPORT_FOR_X BIT(0)

if (!(data & EIO_GPIO_HAS_SUPPORT_FOR_X))
        return -EOPNOTSUPP;

+static int check_pin(struct device *dev, int pin)
+{
+       int ret;
+       int group, bit;
+       u16 data;
+
+       /* Get pin mapping */
+       ret = pmc_read(dev, EIO_GPIO_MAPPING, pin, &data);
+       if (ret)
+               return ret;
+
+       if ((data & 0xFF) > ARRAY_SIZE(group_map))
+               return -EINVAL;
+
+       group = group_map[data & 0xFF].group;
+       bit   = data >> 8;
+
+       /* Check mapped pin */
+       ret = pmc_read(dev, EIO_GPIO_GROUP_AVAIL, group, &data);
+       if (ret)
+               return ret;
+
+       return data & BIT(bit) ? 0 : -EOPNOTSUPP;
+}

So there is some pin muxing going on here. If that should be made dynamic
at some point this should make use of the pin control subsystem, but I guess
it is currently hard-coded?

Yours,
Linus Walleij