Hi Larry, thanks for your patch! I would really like Andy to look at this, because ACPI and I'm not good at ACPI. On Tue, Aug 8, 2023 at 4:57 PM larry.lai <larry.lai@xxxxxxxxxxxxxxx> wrote: > The UP Squared board <http://www.upboard.com> implements certain > features (pin control) through an on-board FPGA. > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Signed-off-by: Gary Wang <garywang@xxxxxxxxxxxx> > Signed-off-by: larry.lai <larry.lai@xxxxxxxxxxxxxxx> (...) > + if (strstr(pctrl->pctldesc->pins[offset[i]].name, "I2C") || > + strstr(pctrl->pctldesc->pins[offset[i]].name, "PINMUX")) { I think there is something like match a string list in the kernel, Andy will know. > +static int upboard_gpio_get(struct gpio_chip *gc, unsigned int offset) > +{ > + struct upboard_pinctrl *pctrl = container_of(gc, struct upboard_pinctrl, chip); > + unsigned int pin = pctrl->rpi_mapping[offset]; > + int gpio = upboard_rpi_to_native_gpio(gc, offset); > + > + if (gpio < 0) > + return gpio; > + > + /* APL03 board open drain GPIO */ > + if (pctrl->ident == BOARD_UP_APL03) { > + int val = 0; > + > + switch (pin) { > + case 0: > + case 1: > + case 9: > + case 23: > + val = readl(pctrl->pins[pin].regs); > + return val & 0x00000003; > + default: > + break; > + } > + } > + > + return gpiod_get_value(gpio_to_desc(gpio)); I don't really like this construction bouncing us back into the global GPIO numberspace to get to the descriptor of the corresponding native GPIO. gpio_to_desc() is an uglyhack and shall not be used. You need to come up with something better here: request all the underlying GPIO descriptors with gpiod_get(dev, .. ASIS) and store them in an array. Look in e.g. drivers/gpio/gpio-aggregator.c drivers/gpio/gpio-latch.c for examples of how we do this in other drivers. The underlying GPIO lines need to be specified somehow, in worst case using descriptor tables. > +/* DMI Matches for older bios without fpga initialization */ > +static const struct dmi_system_id upboard_dmi_table[] __initconst = { > + { > + .ident = BOARD_U A DMI table OK.... > +static struct platform_driver upboard_pinctrl_driver = { > + .driver = { > + .name = "upboard-pinctrl", > + }, > +}; > +module_platform_driver_probe(upboard_pinctrl_driver, upboard_pinctrl_probe); OK I don't know ACPI, but what I've seen the DMI table is supposed to be used for some kind of lookup, such as checking in .init if the device is there, so I don't get this. Also doesn't ACPI drivers use the ACPI bus? Yours, Linus Walleij
