Messages in this thread |  | | From | Aidan MacDonald <> | Subject | Re: [PATCH v4 12/15] pinctrl: Add AXP192 pin control driver | Date | Fri, 01 Jul 2022 16:37:45 +0100 |
| |
Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> On Wed, Jun 29, 2022 at 4:30 PM Aidan MacDonald > <aidanmacdonald.0x0@gmail.com> wrote: >> >> The AXP192 PMIC's GPIO registers are much different from the GPIO >> registers of the AXP20x and AXP813 PMICs supported by the existing >> pinctrl-axp209 driver. It makes more sense to add a new driver for >> the AXP192, rather than add support in the existing axp20x driver. >> >> The pinctrl-axp192 driver is considerably more flexible in terms of >> register layout and should be able to support other X-Powers PMICs. >> Interrupts and pull down resistor configuration are supported too. > > ... > >> +config PINCTRL_AXP192 >> + tristate "X-Powers AXP192 PMIC pinctrl and GPIO Support" >> + depends on MFD_AXP20X >> + select PINMUX >> + select GENERIC_PINCONF >> + select GPIOLIB >> + help >> + AXP PMICs provide multiple GPIOs that can be muxed for different >> + functions. This driver bundles a pinctrl driver to select the function >> + muxing and a GPIO driver to handle the GPIO when the GPIO function is >> + selected. >> + Say Y to enable pinctrl and GPIO support for the AXP192 PMIC. > > What will be the module name if compiled as a module? > > ... > >> +/** >> + * struct axp192_pctl_function - describes a function that GPIOs may have >> + * >> + * @name: Function name >> + * @muxvals: Mux values used for selecting this function, one per GPIO. >> + * The i'th element corresponds to the i'th GPIO and is written >> + * to the GPIO's control register field to select this function. >> + * U8_MAX indicates that the pin does not support this function. >> + * @groups: Array of @ngroups groups listing pins supporting this function. >> + * @ngroups: Number of pin groups. >> + */ >> +struct axp192_pctl_function { >> + const char *name; >> + /* Mux value written to the control register to select the function (-1 if unsupported) */ >> + const u8 *muxvals; >> + const char * const *groups; >> + unsigned int ngroups; >> +}; > > Can it be replaced by struct function_desc? > https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/pinmux.h#L130
That'd work, but using the generic infrastructure doesn't allow me to simplify anything -- I can eliminate three trivial functions, but the generic code is higher overhead (extra allocations, radix trees, ...) so I'd prefer to stick with the current approach.
>> + ret = devm_gpiochip_add_data(dev, &pctl->chip, pctl); >> + if (ret) >> + dev_err_probe(dev, ret, "Failed to register GPIO chip\n"); > > Missed return.
Thanks for catching this, that was pretty silly of me...
|  |