On Tue, Sep 26, 2017 at 01:17:05PM +0000, Quentin Schulz wrote: > Hi Maxime, > > On 26/09/2017 15:01, Maxime Ripard wrote: > > On Tue, Sep 26, 2017 at 12:17:13PM +0000, Quentin Schulz wrote: > >> Instead of using a function to retrieve each pin's correct control > >> register, use drv_data within pinctrl_pin_desc to store the ctrl reg. > >> > >> Remove axp20x_gpio_get_reg and replace every occurrence by a get from > >> drv_data. > > > > Why do you need to do that? This should be explained. > > > > Agreed that it misses an explanation. > > Today, to get a register addr of one of the GPIOs in the PMIC, we > basically get the GPIO number and returns the register via this info. > > There are 3 GPIOs in AXP209, 2 in AXP813. I didn't want to have a switch > case for the GPIO number and then an if/else inside one of the case to > check if the device is AXP209 or AXP813 in which case we return -EINVAL > instead of the GPIO2 reg. With support for new PMIC, we would have a > bunch of if conditions and complexify the process for something really > simple. I'm not sure how that relates to your code actually. The only thing that patch is doing is to move the register offset from a function to the structure associated to the pin. However, even in the AXP813 case, you're using exactly the same values, so that's not really needed. Now, you also mentionned the pin number. While this patch doesn't really address it, it's also no really needed. The number of pins is already known and registered in the GPIO framework. If the framework doesn't already do it (which would be surprising), you can just check that the pin number passed is not going to be higher than the one you registered. > IMHO, this also allows easier integration of future PMICs which might > have different regs for the GPIOs. Let's worry about future PMICs in the future. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Attachment:
signature.asc
Description: PGP signature
