* reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-04 11:02 ` Andre Przywara 0 siblings, 0 replies; 19+ messages in thread From: Andre Przywara @ 2016-01-04 11:02 UTC (permalink / raw) To: Maxime Ripard, Linus Walleij Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland 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. 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? 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 appreciate any comments on this, especially if I missed something which would render this approach impossible or tedious. Cheers, Andre. ^ permalink raw reply [flat|nested] 19+ messages in thread
* reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-04 11:02 ` Andre Przywara 0 siblings, 0 replies; 19+ messages in thread From: Andre Przywara @ 2016-01-04 11:02 UTC (permalink / raw) To: linux-arm-kernel 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. 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? 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 at 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 appreciate any comments on this, especially if I missed something which would render this approach impossible or tedious. Cheers, Andre. ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <568A514D.7070102-5wv7dgnIgG8@public.gmane.org>]
* Re: reason for Allwinner SoC specific pinctrl drivers? [not found] ` <568A514D.7070102-5wv7dgnIgG8@public.gmane.org> @ 2016-01-04 17:27 ` Vishnu Patekar [not found] ` <CAEzqOZu8wkPKLp6bZZ7JuiM07ixDksdD=U-WK=3kFPYJZMon4Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-01-04 22:04 ` [linux-sunxi] " Julian Calaby 2016-01-05 13:10 ` Maxime Ripard 2 siblings, 1 reply; 19+ messages in thread From: Vishnu Patekar @ 2016-01-04 17:27 UTC (permalink / raw) To: andre.przywara-5wv7dgnIgG8 Cc: mark.rutland-5wv7dgnIgG8, Linus Walleij, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-gpio-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard [-- Attachment #1: Type: text/plain, Size: 2973 bytes --] Hello Andre, This is something we can do for future SOCs. On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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. > 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? > 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 appreciate any comments on this, especially if I missed something > which would render this approach impossible or tedious. AFAIK, there are lot of holes in pin conf registers. This will complicate the dtsi configuration. The current pinctrl driver is best possible solution with respect to time and availability of limited people working on sunxi with the cost of increase in code size. It's better to give it a shot to make it generic driver for future SOCs atleast. Like A83, A64, etc. Possible solution will always be examined by experts Linus, Maxine, etc. Regards, Vishnu > > Cheers, > Andre. > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. [-- Attachment #2: Type: text/html, Size: 3987 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <CAEzqOZu8wkPKLp6bZZ7JuiM07ixDksdD=U-WK=3kFPYJZMon4Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: reason for Allwinner SoC specific pinctrl drivers? 2016-01-04 17:27 ` Vishnu Patekar @ 2016-01-04 21:36 ` Michal Suchanek 0 siblings, 0 replies; 19+ messages in thread From: Michal Suchanek @ 2016-01-04 21:36 UTC (permalink / raw) To: vishnupatekar0510-Re5JQEeQqe8AvxtiuMwx3w Cc: andre.przywara-5wv7dgnIgG8, Mark Rutland, Linus Walleij, linux-sunxi, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-gpio-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard Hello, On 4 January 2016 at 18:27, Vishnu Patekar <vishnupatekar0510-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > Hello Andre, > This is something we can do for future SOCs. > > On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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. >> 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? >> 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"; As I understand it 1) uart0 is basically a mnemonic for muxval 2 2) if you try to mux uart0 on pins for which it is not in the table it fails So it makes no sense to have both function and muxval - this is redundant. And it does not make sense to move from function to muxval - it's like moving from assembly programing to raw machine code programming. For compatibility it's not possible to move the table to the shared SoC DT although it would be possible to have the pin tables in DT. However, it would inflate the DT and make working in u-boot (SPL) where full DT parser is not available problematic. What might be possible is merging the different pinmux drivers in one. Instead of replicating some pinmux boilerplate you will probably end up with lots of ifdefs so only tables for SoC support compiled in the kernel are built into the driver. I am not sure how large the tables are and if anybody should care but you might be also missing some symbols for them. Thanks Michal ^ permalink raw reply [flat|nested] 19+ messages in thread
* [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-04 21:36 ` Michal Suchanek 0 siblings, 0 replies; 19+ messages in thread From: Michal Suchanek @ 2016-01-04 21:36 UTC (permalink / raw) To: linux-arm-kernel Hello, On 4 January 2016 at 18:27, Vishnu Patekar <vishnupatekar0510@gmail.com> wrote: > Hello Andre, > This is something we can do for future SOCs. > > On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara@arm.com> 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. >> 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? >> 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 at 0 { >> allwinner,pins = "PB22", "PB23"; >> + allwinner,muxval = <0x02 0x02>; >> allwinner,function = "uart0"; As I understand it 1) uart0 is basically a mnemonic for muxval 2 2) if you try to mux uart0 on pins for which it is not in the table it fails So it makes no sense to have both function and muxval - this is redundant. And it does not make sense to move from function to muxval - it's like moving from assembly programing to raw machine code programming. For compatibility it's not possible to move the table to the shared SoC DT although it would be possible to have the pin tables in DT. However, it would inflate the DT and make working in u-boot (SPL) where full DT parser is not available problematic. What might be possible is merging the different pinmux drivers in one. Instead of replicating some pinmux boilerplate you will probably end up with lots of ifdefs so only tables for SoC support compiled in the kernel are built into the driver. I am not sure how large the tables are and if anybody should care but you might be also missing some symbols for them. Thanks Michal ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <CAOMqctQL3nmWECpQgtVdxgzEfn2obu4cdj67W1ESONO=5EpQdQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: reason for Allwinner SoC specific pinctrl drivers? 2016-01-04 21:36 ` [linux-sunxi] " Michal Suchanek @ 2016-01-05 12:05 ` Andre Przywara -1 siblings, 0 replies; 19+ messages in thread From: Andre Przywara @ 2016-01-05 12:05 UTC (permalink / raw) To: Michal Suchanek, vishnupatekar0510-Re5JQEeQqe8AvxtiuMwx3w Cc: Mark Rutland, Linus Walleij, linux-sunxi, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-gpio-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard Hi Michal, thanks for your input! On 04/01/16 21:36, Michal Suchanek wrote: > Hello, > > On 4 January 2016 at 18:27, Vishnu Patekar <vishnupatekar0510-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> Hello Andre, >> This is something we can do for future SOCs. >> >> On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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. >>> 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? >>> 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"; > > As I understand it > > 1) uart0 is basically a mnemonic for muxval 2 Not really. At the moment uart0 is used to lookup the (hard-coded) table entry for pins PB22 and PB23, which returns the said value of 0x02 (in this example, cf. line 246 in pinctrl-sun7i-a20.c). But if there are other pins where UART0 can be muxed too, there is another node describing them (cf. uart3_pins_a and uart3_pins_b in sun7i-a20.dtsi). This one uses the "uart0" name as well, but the muxval returned for _those pins_ can be different. So the string only makes sense in connection with a certain pin. > 2) if you try to mux uart0 on pins for which it is not in the table it fails How would you mux them if they are not in the table? > So it makes no sense to have both function and muxval - this is redundant. Kind of, but not if we want to keep compatibility with older and newer DTs and older and newer kernels (drivers, really) - which is my goal. So we just _add_ the muxval values. The existing chip-specific drivers would naturally ignore the values and just use their built-in table for lookup. The generic driver on the other hand would use the DT information. An appropriate compatible string could then be added to refer to the generic driver as a fallback. For (future) SoCs (which would not have a specific driver) we could omit the function string (if this isn't needed elsewhere, I have to check). So I don't see how the redundancy would be an issue here. > And it does not make sense to move from function to muxval - it's like > moving from assembly programing to raw machine code programming. But it removes the requirement of relying on the built-in lookup table. So by using a more readable uart0 "mnemonic" we rely on some hardcoded, chip specific table in each kernel, which is just wrong IMHO. Other DT users (be it Xen or *BSD, for instance) would have to replicate this table and since it's really SoC specific, it does not make any sense to me to keep it separate. After all this DT node is SoC specific as well, so I don't see the point of abstracting this with some string lookup. So to stay with your comparison: Yes, we move from assembly to machine code, but we get rid of the need for a SoC specific assembler, which is maintained separately. > For compatibility it's not possible to move the table to the shared > SoC DT Why is that? We have the actual pin tables in the shared SoC DT, each board specific DT just refers to the actually connected pins by reference. That wouldn't change at all. So the above example for instance is from sun7i-a20.dtsi, board specific .dts files just use: pinctrl-0 = <&uart0_pins_a>; > although it would be possible to have the pin tables in DT. > However, it would inflate the DT and make working in u-boot (SPL) > where full DT parser is not available problematic. I don't get this. Having the actual values instead of a string lookup would make it actually easier to lookup for more light-weight kernels. > What might be possible is merging the different pinmux drivers in one. > Instead of replicating some pinmux boilerplate you will probably end > up with lots of ifdefs so only tables for SoC support compiled in the > kernel are built into the driver. That doesn't sound to tempting to me. > I am not sure how large the tables are and if anybody should care but > you might be also missing some symbols for them. So I guess I just failed to express my approach properly. I will try to code a proof of concept this week and convert once SoC over to the new driver, maybe this clears things up. If I have a blatant misunderstanding of the concepts (quite possible), please keep correcting me! Thanks! Andre. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-05 12:05 ` Andre Przywara 0 siblings, 0 replies; 19+ messages in thread From: Andre Przywara @ 2016-01-05 12:05 UTC (permalink / raw) To: linux-arm-kernel Hi Michal, thanks for your input! On 04/01/16 21:36, Michal Suchanek wrote: > Hello, > > On 4 January 2016 at 18:27, Vishnu Patekar <vishnupatekar0510@gmail.com> wrote: >> Hello Andre, >> This is something we can do for future SOCs. >> >> On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara@arm.com> 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. >>> 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? >>> 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 at 0 { >>> allwinner,pins = "PB22", "PB23"; >>> + allwinner,muxval = <0x02 0x02>; >>> allwinner,function = "uart0"; > > As I understand it > > 1) uart0 is basically a mnemonic for muxval 2 Not really. At the moment uart0 is used to lookup the (hard-coded) table entry for pins PB22 and PB23, which returns the said value of 0x02 (in this example, cf. line 246 in pinctrl-sun7i-a20.c). But if there are other pins where UART0 can be muxed too, there is another node describing them (cf. uart3_pins_a and uart3_pins_b in sun7i-a20.dtsi). This one uses the "uart0" name as well, but the muxval returned for _those pins_ can be different. So the string only makes sense in connection with a certain pin. > 2) if you try to mux uart0 on pins for which it is not in the table it fails How would you mux them if they are not in the table? > So it makes no sense to have both function and muxval - this is redundant. Kind of, but not if we want to keep compatibility with older and newer DTs and older and newer kernels (drivers, really) - which is my goal. So we just _add_ the muxval values. The existing chip-specific drivers would naturally ignore the values and just use their built-in table for lookup. The generic driver on the other hand would use the DT information. An appropriate compatible string could then be added to refer to the generic driver as a fallback. For (future) SoCs (which would not have a specific driver) we could omit the function string (if this isn't needed elsewhere, I have to check). So I don't see how the redundancy would be an issue here. > And it does not make sense to move from function to muxval - it's like > moving from assembly programing to raw machine code programming. But it removes the requirement of relying on the built-in lookup table. So by using a more readable uart0 "mnemonic" we rely on some hardcoded, chip specific table in each kernel, which is just wrong IMHO. Other DT users (be it Xen or *BSD, for instance) would have to replicate this table and since it's really SoC specific, it does not make any sense to me to keep it separate. After all this DT node is SoC specific as well, so I don't see the point of abstracting this with some string lookup. So to stay with your comparison: Yes, we move from assembly to machine code, but we get rid of the need for a SoC specific assembler, which is maintained separately. > For compatibility it's not possible to move the table to the shared > SoC DT Why is that? We have the actual pin tables in the shared SoC DT, each board specific DT just refers to the actually connected pins by reference. That wouldn't change at all. So the above example for instance is from sun7i-a20.dtsi, board specific .dts files just use: pinctrl-0 = <&uart0_pins_a>; > although it would be possible to have the pin tables in DT. > However, it would inflate the DT and make working in u-boot (SPL) > where full DT parser is not available problematic. I don't get this. Having the actual values instead of a string lookup would make it actually easier to lookup for more light-weight kernels. > What might be possible is merging the different pinmux drivers in one. > Instead of replicating some pinmux boilerplate you will probably end > up with lots of ifdefs so only tables for SoC support compiled in the > kernel are built into the driver. That doesn't sound to tempting to me. > I am not sure how large the tables are and if anybody should care but > you might be also missing some symbols for them. So I guess I just failed to express my approach properly. I will try to code a proof of concept this week and convert once SoC over to the new driver, maybe this clears things up. If I have a blatant misunderstanding of the concepts (quite possible), please keep correcting me! Thanks! Andre. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? 2016-01-05 12:05 ` [linux-sunxi] " Andre Przywara @ 2016-01-05 15:20 ` Michal Suchanek -1 siblings, 0 replies; 19+ messages in thread From: Michal Suchanek @ 2016-01-05 15:20 UTC (permalink / raw) To: Andre Przywara Cc: vishnupatekar0510, Mark Rutland, Linus Walleij, linux-sunxi, linux-arm-kernel, linux-gpio, Maxime Ripard On 5 January 2016 at 13:05, Andre Przywara <andre.przywara@arm.com> wrote: > Hi Michal, > > thanks for your input! > > On 04/01/16 21:36, Michal Suchanek wrote: >> Hello, >> >> On 4 January 2016 at 18:27, Vishnu Patekar <vishnupatekar0510@gmail.com> wrote: >>> Hello Andre, >>> This is something we can do for future SOCs. >>> >>> On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara@arm.com> wrote: >>>> >>>> >>>> uart0_pins_a: uart0@0 { >>>> allwinner,pins = "PB22", "PB23"; >>>> + allwinner,muxval = <0x02 0x02>; >>>> allwinner,function = "uart0"; >> >> As I understand it >> >> 1) uart0 is basically a mnemonic for muxval 2 > > Not really. At the moment uart0 is used to lookup the (hard-coded) table > entry for pins PB22 and PB23, which returns the said value of 0x02 (in > this example, cf. line 246 in pinctrl-sun7i-a20.c). > But if there are other pins where UART0 can be muxed too, there is > another node describing them (cf. uart3_pins_a and uart3_pins_b in > sun7i-a20.dtsi). This one uses the "uart0" name as well, but the muxval > returned for _those pins_ can be different. So the string only makes > sense in connection with a certain pin. > >> 2) if you try to mux uart0 on pins for which it is not in the table it fails > > How would you mux them if they are not in the table? You cannot. The muxing fails because you request a function that is not in the table for that pin - as opposed to muxing some other random function on the pin silently. > >> So it makes no sense to have both function and muxval - this is redundant. > > Kind of, but not if we want to keep compatibility with older and newer > DTs and older and newer kernels (drivers, really) - which is my goal. > So we just _add_ the muxval values. The existing chip-specific drivers > would naturally ignore the values and just use their built-in table for > lookup. The generic driver on the other hand would use the DT > information. An appropriate compatible string could then be added to > refer to the generic driver as a fallback. > For (future) SoCs (which would not have a specific driver) we could omit > the function string (if this isn't needed elsewhere, I have to check). > So I don't see how the redundancy would be an issue here. The function string is needed for the setting to make any sense. As you have pointed out yourself the function name may resolve to different muxval for different pins. You need those SoC pin tables so you know what you multiplexed to what. They are in the kernel where they IMHO belong. Not having them at all is a no-go because then you have no idea what functions you have enabled on the SoC. What you suggest is adding duplicate lower level information in the DT so higher level pinmux driver uses symbolic function name and lower level driver using raw mux number. This means that maintenance problems increase with these low level and high level pin mux descriptions potentially going out of sync in the DT. > >> And it does not make sense to move from function to muxval - it's like >> moving from assembly programing to raw machine code programming. > > But it removes the requirement of relying on the built-in lookup table. > So by using a more readable uart0 "mnemonic" we rely on some hardcoded, > chip specific table in each kernel, which is just wrong IMHO. Other DT > users (be it Xen or *BSD, for instance) would have to replicate this > table and since it's really SoC specific, it does not make any sense to > me to keep it separate. After all this DT node is SoC specific as well, > so I don't see the point of abstracting this with some string lookup. > > So to stay with your comparison: Yes, we move from assembly to machine > code, but we get rid of the need for a SoC specific assembler, which is > maintained separately. We cannot get rid of it. And really, the assembly is the same for all the SoCs. It's the machine code which isn't and which the assembly abstracts. > >> For compatibility it's not possible to move the table to the shared >> SoC DT > > Why is that? We have the actual pin tables in the shared SoC DT, each > board specific DT just refers to the actually connected pins by > reference. That wouldn't change at all. So the above example for > instance is from sun7i-a20.dtsi, board specific .dts files just use: > pinctrl-0 = <&uart0_pins_a>; Except as has been pointed out in DT unused features are not to be included so you would lose the functions which are included in the in-kernel tables but are not wired on any supported board. Also the uart0_pins_a part is just trashed AFAIK so once the kernel boots you have no idea what pins you got in the driver. So if the low level pinmux driver were to replace the high level pinmux driver it would have to parse the pin tables in the DT and reconstruct the currently hardcoded pin tables from the DT information so that when you look at the pinmux state later you know what it means without the in-kernel hardcoded table. Then all pinmux values are technically used by the pinmux driver to reconstruct the pinmux table and should be included in the DT. > >> although it would be possible to have the pin tables in DT. >> However, it would inflate the DT and make working in u-boot (SPL) >> where full DT parser is not available problematic. > > I don't get this. Having the actual values instead of a string lookup > would make it actually easier to lookup for more light-weight kernels. No. You can just include the pin table in the C code as opposed to linking in a DT parser as well as requiring heap space to parse the DT. well, you can always cut and paste the data from DT to any code you want, anyway. So long as you do have the data somewhere. Thanks Michal ^ permalink raw reply [flat|nested] 19+ messages in thread
* [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-05 15:20 ` Michal Suchanek 0 siblings, 0 replies; 19+ messages in thread From: Michal Suchanek @ 2016-01-05 15:20 UTC (permalink / raw) To: linux-arm-kernel On 5 January 2016 at 13:05, Andre Przywara <andre.przywara@arm.com> wrote: > Hi Michal, > > thanks for your input! > > On 04/01/16 21:36, Michal Suchanek wrote: >> Hello, >> >> On 4 January 2016 at 18:27, Vishnu Patekar <vishnupatekar0510@gmail.com> wrote: >>> Hello Andre, >>> This is something we can do for future SOCs. >>> >>> On 4 Jan 2016 19:02, "Andre Przywara" <andre.przywara@arm.com> wrote: >>>> >>>> >>>> uart0_pins_a: uart0 at 0 { >>>> allwinner,pins = "PB22", "PB23"; >>>> + allwinner,muxval = <0x02 0x02>; >>>> allwinner,function = "uart0"; >> >> As I understand it >> >> 1) uart0 is basically a mnemonic for muxval 2 > > Not really. At the moment uart0 is used to lookup the (hard-coded) table > entry for pins PB22 and PB23, which returns the said value of 0x02 (in > this example, cf. line 246 in pinctrl-sun7i-a20.c). > But if there are other pins where UART0 can be muxed too, there is > another node describing them (cf. uart3_pins_a and uart3_pins_b in > sun7i-a20.dtsi). This one uses the "uart0" name as well, but the muxval > returned for _those pins_ can be different. So the string only makes > sense in connection with a certain pin. > >> 2) if you try to mux uart0 on pins for which it is not in the table it fails > > How would you mux them if they are not in the table? You cannot. The muxing fails because you request a function that is not in the table for that pin - as opposed to muxing some other random function on the pin silently. > >> So it makes no sense to have both function and muxval - this is redundant. > > Kind of, but not if we want to keep compatibility with older and newer > DTs and older and newer kernels (drivers, really) - which is my goal. > So we just _add_ the muxval values. The existing chip-specific drivers > would naturally ignore the values and just use their built-in table for > lookup. The generic driver on the other hand would use the DT > information. An appropriate compatible string could then be added to > refer to the generic driver as a fallback. > For (future) SoCs (which would not have a specific driver) we could omit > the function string (if this isn't needed elsewhere, I have to check). > So I don't see how the redundancy would be an issue here. The function string is needed for the setting to make any sense. As you have pointed out yourself the function name may resolve to different muxval for different pins. You need those SoC pin tables so you know what you multiplexed to what. They are in the kernel where they IMHO belong. Not having them at all is a no-go because then you have no idea what functions you have enabled on the SoC. What you suggest is adding duplicate lower level information in the DT so higher level pinmux driver uses symbolic function name and lower level driver using raw mux number. This means that maintenance problems increase with these low level and high level pin mux descriptions potentially going out of sync in the DT. > >> And it does not make sense to move from function to muxval - it's like >> moving from assembly programing to raw machine code programming. > > But it removes the requirement of relying on the built-in lookup table. > So by using a more readable uart0 "mnemonic" we rely on some hardcoded, > chip specific table in each kernel, which is just wrong IMHO. Other DT > users (be it Xen or *BSD, for instance) would have to replicate this > table and since it's really SoC specific, it does not make any sense to > me to keep it separate. After all this DT node is SoC specific as well, > so I don't see the point of abstracting this with some string lookup. > > So to stay with your comparison: Yes, we move from assembly to machine > code, but we get rid of the need for a SoC specific assembler, which is > maintained separately. We cannot get rid of it. And really, the assembly is the same for all the SoCs. It's the machine code which isn't and which the assembly abstracts. > >> For compatibility it's not possible to move the table to the shared >> SoC DT > > Why is that? We have the actual pin tables in the shared SoC DT, each > board specific DT just refers to the actually connected pins by > reference. That wouldn't change at all. So the above example for > instance is from sun7i-a20.dtsi, board specific .dts files just use: > pinctrl-0 = <&uart0_pins_a>; Except as has been pointed out in DT unused features are not to be included so you would lose the functions which are included in the in-kernel tables but are not wired on any supported board. Also the uart0_pins_a part is just trashed AFAIK so once the kernel boots you have no idea what pins you got in the driver. So if the low level pinmux driver were to replace the high level pinmux driver it would have to parse the pin tables in the DT and reconstruct the currently hardcoded pin tables from the DT information so that when you look at the pinmux state later you know what it means without the in-kernel hardcoded table. Then all pinmux values are technically used by the pinmux driver to reconstruct the pinmux table and should be included in the DT. > >> although it would be possible to have the pin tables in DT. >> However, it would inflate the DT and make working in u-boot (SPL) >> where full DT parser is not available problematic. > > I don't get this. Having the actual values instead of a string lookup > would make it actually easier to lookup for more light-weight kernels. No. You can just include the pin table in the C code as opposed to linking in a DT parser as well as requiring heap space to parse the DT. well, you can always cut and paste the data from DT to any code you want, anyway. So long as you do have the data somewhere. Thanks Michal ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: reason for Allwinner SoC specific pinctrl drivers? 2016-01-04 11:02 ` Andre Przywara @ 2016-01-04 22:04 ` Julian Calaby -1 siblings, 0 replies; 19+ messages in thread From: Julian Calaby @ 2016-01-04 22:04 UTC (permalink / raw) To: andre.przywara-5wv7dgnIgG8 Cc: Maxime Ripard, Linus Walleij, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland Hi Andre, On Mon, Jan 4, 2016 at 10:02 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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. > 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? > 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 appreciate any comments on this, especially if I missed something > which would render this approach impossible or tedious. I think that, as Michal said, merging the drivers might be possible, however there's another three functions the drivers serve: 1. they're good documentation of how it's all configured. I'm not sure your device tree based approach will be as user friendly in this regard. 2. they list stuff we don't have a driver / hardware for yet 3. the policy on device-tree is to only include stuff we know is working, which means we have a driver and hardware for that particular thing. Device tree files for boards or SoCs have been rejected because they list stuff that isn't used yet. Thanks, -- Julian Calaby Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-04 22:04 ` Julian Calaby 0 siblings, 0 replies; 19+ messages in thread From: Julian Calaby @ 2016-01-04 22:04 UTC (permalink / raw) To: linux-arm-kernel Hi Andre, On Mon, Jan 4, 2016 at 10:02 PM, Andre Przywara <andre.przywara@arm.com> 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. > 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? > 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 at 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 appreciate any comments on this, especially if I missed something > which would render this approach impossible or tedious. I think that, as Michal said, merging the drivers might be possible, however there's another three functions the drivers serve: 1. they're good documentation of how it's all configured. I'm not sure your device tree based approach will be as user friendly in this regard. 2. they list stuff we don't have a driver / hardware for yet 3. the policy on device-tree is to only include stuff we know is working, which means we have a driver and hardware for that particular thing. Device tree files for boards or SoCs have been rejected because they list stuff that isn't used yet. Thanks, -- Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <CAGRGNgUVNGoOUNqyySjYzkL4KfEWK8orYH-AMxoCX-9N0R80-A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: reason for Allwinner SoC specific pinctrl drivers? 2016-01-04 22:04 ` [linux-sunxi] " Julian Calaby @ 2016-01-05 12:24 ` Andre Przywara -1 siblings, 0 replies; 19+ messages in thread From: Andre Przywara @ 2016-01-05 12:24 UTC (permalink / raw) To: Julian Calaby Cc: Maxime Ripard, Linus Walleij, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland Hi Julian, On 04/01/16 22:04, Julian Calaby wrote: > Hi Andre, > > On Mon, Jan 4, 2016 at 10:02 PM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> 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. >> 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? >> 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 appreciate any comments on this, especially if I missed something >> which would render this approach impossible or tedious. > > I think that, as Michal said, merging the drivers might be possible, > however there's another three functions the drivers serve: > > 1. they're good documentation of how it's all configured. I'm not sure > your device tree based approach will be as user friendly in this > regard. > > 2. they list stuff we don't have a driver / hardware for yet > > 3. the policy on device-tree is to only include stuff we know is > working, which means we have a driver and hardware for that particular > thing. Device tree files for boards or SoCs have been rejected because > they list stuff that isn't used yet. Those are good points, thanks for bringing them up. Hopefully they are no show stoppers... I will try to come up with some kind of workaround for 2. and 3. We could find a way to document the stuff somewhere, I guess the sunxi wiki would be a good start. But actually we should just reference to publicly available docs, say the Allwinner documentation github repo. I know, I know ... ;-) Is there any known wrong or missing information for the port controller and mux settings in Allwinner's doc? Cheers, Andre. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-05 12:24 ` Andre Przywara 0 siblings, 0 replies; 19+ messages in thread From: Andre Przywara @ 2016-01-05 12:24 UTC (permalink / raw) To: linux-arm-kernel Hi Julian, On 04/01/16 22:04, Julian Calaby wrote: > Hi Andre, > > On Mon, Jan 4, 2016 at 10:02 PM, Andre Przywara <andre.przywara@arm.com> 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. >> 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? >> 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 at 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 appreciate any comments on this, especially if I missed something >> which would render this approach impossible or tedious. > > I think that, as Michal said, merging the drivers might be possible, > however there's another three functions the drivers serve: > > 1. they're good documentation of how it's all configured. I'm not sure > your device tree based approach will be as user friendly in this > regard. > > 2. they list stuff we don't have a driver / hardware for yet > > 3. the policy on device-tree is to only include stuff we know is > working, which means we have a driver and hardware for that particular > thing. Device tree files for boards or SoCs have been rejected because > they list stuff that isn't used yet. Those are good points, thanks for bringing them up. Hopefully they are no show stoppers... I will try to come up with some kind of workaround for 2. and 3. We could find a way to document the stuff somewhere, I guess the sunxi wiki would be a good start. But actually we should just reference to publicly available docs, say the Allwinner documentation github repo. I know, I know ... ;-) Is there any known wrong or missing information for the port controller and mux settings in Allwinner's doc? Cheers, Andre. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: reason for Allwinner SoC specific pinctrl drivers? 2016-01-04 11:02 ` Andre Przywara @ 2016-01-05 13:10 ` Maxime Ripard -1 siblings, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2016-01-05 13:10 UTC (permalink / raw) To: Andre Przywara Cc: Linus Walleij, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland [-- Attachment #1: Type: text/plain, Size: 3714 bytes --] 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-05 13:10 ` Maxime Ripard 0 siblings, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2016-01-05 13:10 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160105/27733273/attachment.sig> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: reason for Allwinner SoC specific pinctrl drivers? 2016-01-05 13:10 ` Maxime Ripard @ 2016-01-07 10:06 ` Linus Walleij -1 siblings, 0 replies; 19+ messages in thread From: Linus Walleij @ 2016-01-07 10:06 UTC (permalink / raw) To: Maxime Ripard Cc: Andre Przywara, linux-gpio-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland On Tue, Jan 5, 2016 at 2:10 PM, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote: > 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. They call it Stockholm Syndrome when you start to sympathize with your kidnappers :D On a serious note, we are in violent agreement on these bullets. This driver is easy to maintain and debug, and easy for new developers to get into precisely because so much knowledge is in the kernel and not in DT. A quick tour in /sys/kernel/debug/pinctrl/* should convince anyone that this is actually very helpful to get things right. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 19+ messages in thread
* reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-07 10:06 ` Linus Walleij 0 siblings, 0 replies; 19+ messages in thread From: Linus Walleij @ 2016-01-07 10:06 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 5, 2016 at 2:10 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > 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. They call it Stockholm Syndrome when you start to sympathize with your kidnappers :D On a serious note, we are in violent agreement on these bullets. This driver is easy to maintain and debug, and easy for new developers to get into precisely because so much knowledge is in the kernel and not in DT. A quick tour in /sys/kernel/debug/pinctrl/* should convince anyone that this is actually very helpful to get things right. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? 2016-01-04 11:02 ` Andre Przywara @ 2016-01-05 2:21 ` Chen-Yu Tsai -1 siblings, 0 replies; 19+ messages in thread From: Chen-Yu Tsai @ 2016-01-05 2:21 UTC (permalink / raw) To: andre.przywara Cc: Maxime Ripard, Linus Walleij, linux-gpio, linux-sunxi, linux-arm-kernel, Mark Rutland On Mon, Jan 4, 2016 at 7:02 PM, Andre Przywara <andre.przywara@arm.com> 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. > 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? > 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>; > }; Using a mux value over a string does not gain us much, other than smaller driver code, since all the strings are gone, and smaller DT, again no strings. However, if you want some sensible debug output from the driver, you're going to keep the strings anyway. The mux values would need to be macros defined in some header file, instead of raw values which are hard to understand. That's only half of it. The driver should know what pins are available and which mux values can be used for them. So you'd still need a table of some sort. That means it's unlikely you can have a generic driver. Leaking the internals into the DT is not a good way either. The current approach of a generic driver library and SoC specific tables maybe isn't the best approach, but it is easy enough to add new SoC support. Regards ChenYu > 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 appreciate any comments on this, especially if I missed something > which would render this approach impossible or tedious. > > Cheers, > Andre. > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [linux-sunxi] reason for Allwinner SoC specific pinctrl drivers? @ 2016-01-05 2:21 ` Chen-Yu Tsai 0 siblings, 0 replies; 19+ messages in thread From: Chen-Yu Tsai @ 2016-01-05 2:21 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 4, 2016 at 7:02 PM, Andre Przywara <andre.przywara@arm.com> 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. > 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? > 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 at 0 { > allwinner,pins = "PB22", "PB23"; > + allwinner,muxval = <0x02 0x02>; > allwinner,function = "uart0"; > allwinner,drive = <SUN4I_PINCTRL_10_MA>; > allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; > }; Using a mux value over a string does not gain us much, other than smaller driver code, since all the strings are gone, and smaller DT, again no strings. However, if you want some sensible debug output from the driver, you're going to keep the strings anyway. The mux values would need to be macros defined in some header file, instead of raw values which are hard to understand. That's only half of it. The driver should know what pins are available and which mux values can be used for them. So you'd still need a table of some sort. That means it's unlikely you can have a generic driver. Leaking the internals into the DT is not a good way either. The current approach of a generic driver library and SoC specific tables maybe isn't the best approach, but it is easy enough to add new SoC support. Regards ChenYu > 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 appreciate any comments on this, especially if I missed something > which would render this approach impossible or tedious. > > Cheers, > Andre. > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-01-07 10:06 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-04 11:02 reason for Allwinner SoC specific pinctrl drivers? Andre Przywara 2016-01-04 11:02 ` Andre Przywara [not found] ` <568A514D.7070102-5wv7dgnIgG8@public.gmane.org> 2016-01-04 17:27 ` Vishnu Patekar [not found] ` <CAEzqOZu8wkPKLp6bZZ7JuiM07ixDksdD=U-WK=3kFPYJZMon4Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-01-04 21:36 ` Michal Suchanek 2016-01-04 21:36 ` [linux-sunxi] " Michal Suchanek [not found] ` <CAOMqctQL3nmWECpQgtVdxgzEfn2obu4cdj67W1ESONO=5EpQdQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-01-05 12:05 ` Andre Przywara 2016-01-05 12:05 ` [linux-sunxi] " Andre Przywara 2016-01-05 15:20 ` Michal Suchanek 2016-01-05 15:20 ` Michal Suchanek 2016-01-04 22:04 ` Julian Calaby 2016-01-04 22:04 ` [linux-sunxi] " Julian Calaby [not found] ` <CAGRGNgUVNGoOUNqyySjYzkL4KfEWK8orYH-AMxoCX-9N0R80-A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-01-05 12:24 ` Andre Przywara 2016-01-05 12:24 ` [linux-sunxi] " Andre Przywara 2016-01-05 13:10 ` Maxime Ripard 2016-01-05 13:10 ` Maxime Ripard 2016-01-07 10:06 ` Linus Walleij 2016-01-07 10:06 ` Linus Walleij 2016-01-05 2:21 ` [linux-sunxi] " Chen-Yu Tsai 2016-01-05 2:21 ` Chen-Yu Tsai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.