Hi Jacopo, On Thursday, November 15, 2018 1, jacopo mondi wrote: > > v5: > > * Specify number of ports using of_device_id.data and save as priv- > >npins > > * Use priv->npins everywhere instead of hard coded RZA2_NPINS > > * Check gpio-ranges to make sure args matches SOC > > Sorry about this, I didn't want to ask you to do this now, it might > have had post-poned to when a new SoC will have to be supported, but.. As long as I can get this driver in for 4.21, I'll still be happy. > > +static const struct of_device_id rza2_pinctrl_of_match[] = { > > + { .compatible = "renesas,r7s9210-pinctrl", .data = (void *)22, }, > > ... I really don't like this, I'm sorry. > > I would rather make a 'struct rza_pinctrl_info' or similar which > contains all the fields you now hardcode (number of ports, pins per > port etc) and which is easily extensible in case you need to do so. I was going by if there is only 1 value being set, just pass in a number (don't make a struct). That is what is being done for the R-Car/RZA SDHI driver (renesas_sdhi_internal_dmac.c), and what I was also asked to do for the RZ/A watchdog timer (rza_wdt.c). At the moment, the number of ports in the SOC is the only variable that would be different between SoCs. For example, "pins per port" will always be 8 (it's part of the HW design of this pin controller, it can never change). We can have Geert give his opinion on the topic since it was his suggestion to begin with. > I'm sorry this is more work, and again, it might be post-poned imo, > provided you drop this change you have introduced here. Since Geert is the maintainer of the Renesas pinctrl drivers, I'll let him decide if I should drop that part for now since only 1 SOC exists today. Chris