RE: [PATCH v6 1/2] pinctrl: Add RZ/A2 pin and gpio controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux