From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Date: Wed, 18 Dec 2013 00:40:03 +0000 Subject: Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes Message-Id: <24775601.3oo6NRXHkY@avalon> List-Id: References: <20131217050232.727.9552.sendpatchset@w520> <1836950.NOThFz7jV4@avalon> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-arm-kernel@lists.infradead.org Hi Magnus, On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote: > On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote: > > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote: > >> From: Magnus Damm > >> > >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11 > >> and jtagport0. > >> > >> Signed-off-by: Magnus Damm > >> --- > >> > >> arch/arm/boot/dts/r7s72100.dtsi | 154 +++++++++++++++++++++++++++++++++ > >> 1 file changed, 154 insertions(+) > >> > >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi > >> +++ work/arch/arm/boot/dts/r7s72100.dtsi 2013-11-27 > >> 16:06:36.000000000 +0900 [snip] > >> + > >> + port0: gpio@fcfe3100 { > >> + compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz"; > >> + reg = <0xfcfe3100 0x4>, /* PSR */ > >> + <0xfcfe3200 0x2>, /* PPR */ > >> + <0xfcfe3800 0x4>; /* PMSR */ > >> + #gpio-cells = <2>; > >> + gpio-controller; > >> + gpio-ranges = <&pfc 0 0 6>; > >> + }; > >> + > >> + port1: gpio@fcfe3104 { > >> + compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz"; > >> + reg = <0xfcfe3104 0x4>, /* PSR */ > >> + <0xfcfe3204 0x2>, /* PPR */ > >> + <0xfcfe3804 0x4>; /* PMSR */ > >> + #gpio-cells = <2>; > >> + gpio-controller; > >> + gpio-ranges = <&pfc 0 16 16>; > > > > As P0 has 6 pins only this should ideally be > > > > gpio-ranges = <&pfc 0 6 16>; > > > > Otherwise the PFC driver will expose pins that don't exist. However, that > > would require computing the pin numbers in the PFC driver differently, as > > we currently just use the bank * 16 + index formula. Given that we only > > have three ports with less than 16 pins we could come up with a not > > overly complex formula that can be evaluated at compile time. Something > > like this should do. > > > > #define RZ_PORT_PIN(bank, pin) \ > > > > (bank) < 1 ? (pin) : \ > > (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \ > > (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \ > > 6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin)) > > Uhm, well, you can make the mapping more compact yes, but I'm not sure > if I agree that it becomes any better. Isn't it better to simply > follow the per-port setup that the manual defines? Is there an actual > problem with having unused GPIOs? If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory in data tables, although by a relatively small amount. Oh, and of course, it's not clean ;-) Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H is a good candidate for that, as each pin is handled individually, and several registers could be handled to with a small amount of code instead of large data tables. It's just a thought for now, I have more urgent tasks to work on. > Actually, I prefer going in the opposite direction so I would like to > share the simple version of RZ_PORT_PIN() in a header file like we do > with RCAR_GP_PIN() in . This because > we would like to use the same macro in the GPIO driver and in the > current PFC code (and potentially more PFCs using the same GPIO > driver). What do you need it for in the GPIO driver ? > Please let me know what you think. -- Regards, Laurent Pinchart