Hi Andre, On Mon, Jan 04, 2016 at 11:02:37AM +0000, Andre Przywara wrote: > Hi, > > while looking at the Allwinner A64 SoC support, I was wondering why we > would actually need a pinctrl driver (file) for each and every Allwinner > SoC that we support. This was actually requested during the initial driver submission http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136809.html And I actually came to like it, even though the initial port is indeed quite painful, for several reasons: 1) In those times, we didn't have a datasheet available for all these SoCs, so no easy way to find out the value to set in the register to mux a particular function in. These days are mostly over, but we're still in that situation for a few SoCs (mostly the R* and H* SoCs though). 2) It's quite easy to read and debug, at least from the DT PoV. The pain mostly comes from writing that table in the first place, but the alternative other SoCs have picked to have something readable in the DT is a SoC specific header file with the defines of all the functions availables for all the pins, which is just as painful to write and review, and is essentialy the same thing. 3) It's easy to maintain. If one of the entry in the table came to be wrong, we simply have to update the table, it's completely contained in the kernel itself and doesn't require fixing (and updating) the DT. 4) It allows to catch misconfiguration in the cases where you set a function on a pin that doesn't support it as well, which is also quite convenient, and wouldn't be possible at all in the case of a header, or simply open-coding the value. > Looking at both the A20 and the A64 doc I don't see any differences > in the port controller implementation apart from the actual muxval > <-> subsystem assignment, which is just data, right? There's also the interrupts over GPIOs that change slightly, the A20 having a single bank and the A64 having several of them. The interrupt mechanism is quite interesting, since it's also a muxing option of a given pin, and only a few pins implement that option. On the A20 and earlier, we had 32 interrupts scattered around the available pins, on A31 and later, we still have the same mechanism, this time with multiple banks of 32 interrupts. We have to have a table somewhere as well to give the mapping a pin and the interrupt that might be connected to it. > Comparing the code files in drivers/pinctrl/sunxi seems to support this, > as those drivers only consist of the table and some boilerplate code. > > Now I was wondering whether we could get away with one generic Allwinner > pinctrl driver and put the SoC specific pin assignments in DT instead. > It looks like adding an "allwinner,muxval" property in addition to the > existing "allwinner,function" in the SoC's .dtsi would give us all the > information we need. This could look like: > > uart0_pins_a: uart0@0 { > allwinner,pins = "PB22", "PB23"; > + allwinner,muxval = <0x02 0x02>; > allwinner,function = "uart0"; > allwinner,drive = <SUN4I_PINCTRL_10_MA>; > allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; > }; > > Would it make sense that I sit down and prototype such a driver? > > We should keep compatibility with older DTs by keeping the existing > drivers in (or maybe emulating the current behaviour by providing just > those tables as a fallback) , but newer SoCs (like the A64?) would not > need a SoC specific driver, but just go with that generic driver and > appropriate DT properties. I guess in the such a case you wouldn't need the function string at all then? Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachment:
signature.asc
Description: Digital signature
