On Fri, Apr 28, 2023 at 11:41 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Aaro reports problems on the OSK1 board after we altered
> the dynamic base for GPIO allocations.
>
> It appears this happens because the OMAP driver now
> allocates GPIO numbers dynamically, so all that is
> references by number is a bit up in the air.
>
> Let's bite the bullet and try to just move the gpio_chip
> in the tps65010 MFD driver over to using dynamic allocations.
> Alter everything in the OSK1 board file to use a GPIO
> descriptor table and lookups.
>
> Utilize the NULL device to define some board-specific
> GPIO lookups and use these to immediately look up the
> same GPIOs, convert to IRQ numbers and pass as resources
> to the devices. This is ugly but should work.
>
> The .setup() callback for tps65010 was used for some GPIO
> hogging, but since the OSK1 is the only user in the entire
> kernel we can alter the signatures to something that
> is helpful and make a clean transition.
...
> static struct gpiod_lookup_table osk_usb_gpio_table = {
> .dev_id = "ohci",
> .table = {
> /* Power GPIO on the I2C-attached TPS65010 */
> - GPIO_LOOKUP("tps65010", 0, "power", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("tps65010", OSK_TPS_GPIO_USB_PWR_EN, "power",
> + GPIO_ACTIVE_HIGH),
> GPIO_LOOKUP(OMAP_GPIO_LABEL, 9, "overcurrent",
> GPIO_ACTIVE_HIGH),
Missing terminator?
> },
...
> +static struct gpiod_lookup_table osk_irq_gpio_table = {
> + .dev_id = NULL,
> + .table = {
> + /* GPIO used for SMC91x IRQ */
> + GPIO_LOOKUP(OMAP_GPIO_LABEL, 0, "smc_irq",
> + GPIO_ACTIVE_HIGH),
> + /* GPIO used for CF IRQ */
> + GPIO_LOOKUP("gpio-48-63", 14, "cf_irq",
> + GPIO_ACTIVE_HIGH),
> + /* GPIO used by the TPS65010 chip */
> + GPIO_LOOKUP("mpuio", 1, "tps65010",
> + GPIO_ACTIVE_HIGH),
> + },
Ditto.
> +};
...
> + osk5912_smc91x_resources[1].start = gpiod_to_irq(d);
> + osk5912_smc91x_resources[1].end = gpiod_to_irq(d);
= DEFINE_RES_IRQ() ?
...
> + osk5912_cf_resources[0].start = gpiod_to_irq(d);
> + osk5912_cf_resources[0].end = gpiod_to_irq(d);
Ditto.
...
> + if (IS_ERR(d)) {
> + pr_err("Unable to get TPS65010 IRQ GPIO descriptor\n");
> + } else {
> + osk_i2c_board_info[0].irq = gpiod_to_irq(d);
> + }
{} are not needed in both branches.
...
> #ifndef __LINUX_I2C_TPS65010_H
> #define __LINUX_I2C_TPS65010_H
+ blank line
> +#include <linux/gpio/driver.h>
Why? Shouldn't
struct gpio_chip;
be sufficient?
--
With Best Regards,
Andy Shevchenko
