pinctrl: dt: at91: new binding
Subsystems (4)
PATCH 1/1 pinctrl: dt: at91: new binding
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
--- .../bindings/pinctrl/atmel,at91-pinctrl.txt | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+)
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +66 -0
@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO Block + +This allow to detect pinctrl issue at DT compiling time when disable a bank + +Required properties for iomux controller: +- compatible: "atmel,at91rm9200-pio-pinctrl" or "atmel,at91sam9x5-pio-pinctrl" + or "atmel,sama5d3-pio-pinctrl" +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be + configured in this periph mode. All the periph and bank need to be describe. + +How to create such array: + +Each column will represent the possible peripheral of the pinctrl for the bank + +Take an example on the 9260 +Peripheral: 2 ( A and B) +=> + + /* A B */ + 0xffffffff 0xffc00c3b /* pioA */ + +For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin) + +Required properties for pin configuration node: +- atmel,pins: 3 integers array, represents a group of pins mux and config + setting. The format is atmel,pins = <PIN_NUM PERIPH CONFIG>. + The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B... + +Bits used for CONFIG: +cf atmel,at91-pinctrl + +pioB: gpio at fffff600 { + compatible = "atmel,at91rm9200-gpio", "atmel,at91rm9200-pio-pinctrl"; + reg = <0xfffff600 0x200>; + interrupts = <3 IRQ_TYPE_LEVEL_HIGH 1>; + #gpio-cells = <2>; + gpio-controller; + interrupt-controller; + #interrupt-cells = <2>; + clocks = <&pioB_clk>; + + /* A B */ + atmel,mux-mask = <0xffffffff 0x7fff3ccf>; + + dbgu { + pinctrl_dbgu: dbgu-0 { + atmel,pins = + <14 0x1 0x0 /* PB14 periph A */ + 15 0x1 0x1>; /* PB15 periph A with pullup */ + }; + }; +}; + +dbgu: serial at fffff200 { + compatible = "atmel,at91sam9260-usart"; + reg = <0xfffff200 0x200>; + interrupts = <1 4 7>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_dbgu>; + status = "disabled"; +}; + +if you have to use multiple bank + pinctrl-0 = <&pinctrl_ip_piaa>, <&pinctrl_ip_piab>;
Signature · 1 line
2.1.4
PATCH 1/1 pinctrl: dt: at91: new binding
Hi Jean-Christophe,
Quoted 2 lines · Jean-Christophe PLAGNIOL-VILLARD
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
Do you have a case where it breaks pinctrl? I can do more tests but with the latest patch "pinctrl: at91: allow to have disabled gpio bank", I had no issue.
Quoted 62 lines
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Linus Walleij <[email protected]>Cc devicetree at vger.kernel.org--- .../bindings/pinctrl/atmel,at91-pinctrl.txt | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+)Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +43
-0@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO Block + +This allow to detect pinctrl issue at DT compiling time when disable a bank + +Required properties for iomux controller: +- compatible: "atmel,at91rm9200-pio-pinctrl" or "atmel,at91sam9x5-pio-pinctrl" + or "atmel,sama5d3-pio-pinctrl" +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be + configured in this periph mode. All the periph and bank need to be describe. + +How to create such array: + +Each column will represent the possible peripheral of the pinctrl for the bank + +Take an example on the 9260 +Peripheral: 2 ( A and B) +=> + + /* A B */ + 0xffffffff 0xffc00c3b /* pioA */ + +For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin) + +Required properties for pin configuration node: +- atmel,pins: 3 integers array, represents a group of pins mux and config + setting. The format is atmel,pins = <PIN_NUM PERIPH CONFIG>. + The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B... + +Bits used for CONFIG: +cf atmel,at91-pinctrl + +pioB: gpio at fffff600 { + compatible = "atmel,at91rm9200-gpio", "atmel,at91rm9200-pio-pinctrl"; + reg = <0xfffff600 0x200>; + interrupts = <3 IRQ_TYPE_LEVEL_HIGH 1>; + #gpio-cells = <2>; + gpio-controller; + interrupt-controller; + #interrupt-cells = <2>; + clocks = <&pioB_clk>; +
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 203
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +10
-0+ /* A B */ + atmel,mux-mask = <0xffffffff 0x7fff3ccf>; + + dbgu { + pinctrl_dbgu: dbgu-0 { + atmel,pins = + <14 0x1 0x0 /* PB14 periph A */ + 15 0x1 0x1>; /* PB15 periph A with pullup */ + }; + };
So we have to update all device tree files, that's a lot of work... Moreover it adds complexity if we have a device using pins from different pio controllers.
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 216
Quoted 13 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +13
-0+}; + +dbgu: serial at fffff200 { + compatible = "atmel,at91sam9260-usart"; + reg = <0xfffff200 0x200>; + interrupts = <1 4 7>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_dbgu>; + status = "disabled"; +}; + +if you have to use multiple bank + pinctrl-0 = <&pinctrl_ip_piaa>, <&pinctrl_ip_piab>;
You mean pio I think.
Ludovic
PATCH 1/1 pinctrl: dt: at91: new binding
Quoted 12 lines · Ludovic Desroches
Hi Jean-Christophe,
Quoted 2 lines · Jean-Christophe PLAGNIOL-VILLARD
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
Do you have a case where it breaks pinctrl? I can do more tests but with the latest patch "pinctrl: at91: allow to have disabled gpio bank", I had no issue.
simple disable the PIO bank and if one device use it as pinctrl and no gpio is used in the DT then you get a broken platform but dtb is generated
as all the node are enabled for pinctrl
Quoted 67 lines
Quoted 67 lines
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Linus Walleij <[email protected]>Cc devicetree at vger.kernel.org---
.../bindings/pinctrl/atmel,at91-pinctrl.txt| 66 ++++++++++++++++++++++1 file changed, 66 insertions(+)Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +47
-0@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO Block + +This allow to detect pinctrl issue at DT compiling time when disable a bank + +Required properties for iomux controller: +- compatible: "atmel,at91rm9200-pio-pinctrl" or "atmel,at91sam9x5-pio-pinctrl" + or "atmel,sama5d3-pio-pinctrl" +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be + configured in this periph mode. All the periph and bank need to be describe. + +How to create such array: + +Each column will represent the possible peripheral of the pinctrl for the bank + +Take an example on the 9260 +Peripheral: 2 ( A and B) +=> + + /* A B */ + 0xffffffff 0xffc00c3b /* pioA */ + +For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin) + +Required properties for pin configuration node: +- atmel,pins: 3 integers array, represents a group of pins mux and config + setting. The format is atmel,pins = <PIN_NUM PERIPH CONFIG>. + The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B... + +Bits used for CONFIG: +cf atmel,at91-pinctrl + +pioB: gpio at fffff600 { + compatible = "atmel,at91rm9200-gpio", "atmel,at91rm9200-pio-pinctrl"; + reg = <0xfffff600 0x200>; + interrupts = <3 IRQ_TYPE_LEVEL_HIGH 1>; + #gpio-cells = <2>; + gpio-controller; + interrupt-controller; + #interrupt-cells = <2>; + clocks = <&pioB_clk>; + + /* A B */ + atmel,mux-mask = <0xffffffff 0x7fff3ccf>; + + dbgu {
Quoted 8 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 203
Quoted 6 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +6
-0+ pinctrl_dbgu: dbgu-0 { + atmel,pins = + <14 0x1 0x0 /* PB14 periph A */ + 15 0x1 0x1>; /* PB15 periph A with pullup */ + }; + };So we have to update all device tree files, that's a lot of work?
break nothing both binding support will be in the kernel
but only one enable at a time, so basically mainline will drop the old one
and as today 99% of the pinctrl are at SoC level so it?s not a big work at all
Quoted 2 lines
Moreover it adds complexity if we have a device using pins from different pio controllers.
put 2 phandle means complexity please
and it?s the key point of the new binding
Best Regards,
J.
PATCH 1/1 pinctrl: dt: at91: new binding
In reply to Jean-Christophe PLAGNIOL-VILLARDLe 26/02/2015 10:34, Jean-Christophe PLAGNIOL-VILLARD a ?crit :
Quoted 18 lines
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Linus Walleij <[email protected]>Cc devicetree at vger.kernel.org--- .../bindings/pinctrl/atmel,at91-pinctrl.txt | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+)Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +0
-0
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 152
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO Block
Sorry but NACK.
I don't want to manage another flavor of the pinmux biding with no real benefit. I would have been good if we had it from day-1. Now it's too late.
Moreover, splitting a binding definition if you have a function given by multiple banks can be weird and not well understood in regard to our current group+function definition scheme (Cf. your last example).
Quoted 54 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +53
-0+This allow to detect pinctrl issue at DT compiling time when disable a bank + +Required properties for iomux controller: +- compatible: "atmel,at91rm9200-pio-pinctrl" or "atmel,at91sam9x5-pio-pinctrl" + or "atmel,sama5d3-pio-pinctrl" +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be + configured in this periph mode. All the periph and bank need to be describe. + +How to create such array: + +Each column will represent the possible peripheral of the pinctrl for the bank + +Take an example on the 9260 +Peripheral: 2 ( A and B) +=> + + /* A B */ + 0xffffffff 0xffc00c3b /* pioA */ + +For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin) + +Required properties for pin configuration node: +- atmel,pins: 3 integers array, represents a group of pins mux and config + setting. The format is atmel,pins = <PIN_NUM PERIPH CONFIG>. + The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B... + +Bits used for CONFIG: +cf atmel,at91-pinctrl + +pioB: gpio at fffff600 { + compatible = "atmel,at91rm9200-gpio", "atmel,at91rm9200-pio-pinctrl"; + reg = <0xfffff600 0x200>; + interrupts = <3 IRQ_TYPE_LEVEL_HIGH 1>; + #gpio-cells = <2>; + gpio-controller; + interrupt-controller; + #interrupt-cells = <2>; + clocks = <&pioB_clk>; + + /* A B */ + atmel,mux-mask = <0xffffffff 0x7fff3ccf>; + + dbgu { + pinctrl_dbgu: dbgu-0 { + atmel,pins = + <14 0x1 0x0 /* PB14 periph A */ + 15 0x1 0x1>; /* PB15 periph A with pullup */ + }; + }; +}; + +dbgu: serial at fffff200 {
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 216
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +10
-0+ compatible = "atmel,at91sam9260-usart"; + reg = <0xfffff200 0x200>; + interrupts = <1 4 7>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_dbgu>; + status = "disabled"; +}; + +if you have to use multiple bank + pinctrl-0 = <&pinctrl_ip_piaa>, <&pinctrl_ip_piab>;
Best regards,
Signature · 1 line
Nicolas Ferre
PATCH 1/1 pinctrl: dt: at91: new binding
Quoted 30 lines · Nicolas Ferre
Le 26/02/2015 10:34, Jean-Christophe PLAGNIOL-VILLARD a ?crit :
Quoted 18 lines
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Linus Walleij <[email protected]>Cc devicetree at vger.kernel.org---
.../bindings/pinctrl/atmel,at91-pinctrl.txt| 66 ++++++++++++++++++++++1 file changed, 66 insertions(+)Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +0
-0Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 152
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO BlockSorry but NACK.
Quoted 2 lines
I don't want to manage another flavor of the pinmux biding with no real benefit. I would have been good if we had it from day-1. Now it's too late.
yes we do, we catch but a compiling time instead of RUNTIME which is critical
so I?ll pass on the NACK
Quoted 5 lines
Moreover, splitting a binding definition if you have a function given by multiple banks can be weird and not well understood in regard to our current group+function definition scheme (Cf. your last example).
Others already do so and this is not complex at all
Best Regards,
J.
Quoted 74 lines
Quoted 55 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +53
-0+This allow to detect pinctrl issue at DT compiling time when disable a bank + +Required properties for iomux controller: +- compatible: "atmel,at91rm9200-pio-pinctrl" or "atmel,at91sam9x5-pio-pinctrl" + or "atmel,sama5d3-pio-pinctrl" +- atmel,mux-mask: array of mask (periph per bank) to describe if a pin can be + configured in this periph mode. All the periph and bank need to be describe. + +How to create such array: + +Each column will represent the possible peripheral of the pinctrl for the bank + +Take an example on the 9260 +Peripheral: 2 ( A and B) +=> + + /* A B */ + 0xffffffff 0xffc00c3b /* pioA */ + +For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin) + +Required properties for pin configuration node: +- atmel,pins: 3 integers array, represents a group of pins mux and config + setting. The format is atmel,pins = <PIN_NUM PERIPH CONFIG>. + The PERIPH 0 means gpio, PERIPH 1 is periph A, PERIPH 2 is periph B... + +Bits used for CONFIG: +cf atmel,at91-pinctrl + +pioB: gpio at fffff600 { + compatible = "atmel,at91rm9200-gpio", "atmel,at91rm9200-pio-pinctrl"; + reg = <0xfffff600 0x200>; + interrupts = <3 IRQ_TYPE_LEVEL_HIGH 1>; + #gpio-cells = <2>; + gpio-controller; + interrupt-controller; + #interrupt-cells = <2>; + clocks = <&pioB_clk>; + + /* A B */ + atmel,mux-mask = <0xffffffff 0x7fff3ccf>; + + dbgu { + pinctrl_dbgu: dbgu-0 { + atmel,pins = + <14 0x1 0x0 /* PB14 periph A */ + 15 0x1 0x1>; /* PB15 periph A with pullup */ + }; + }; +}; + +dbgu: serial at fffff200 {Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 216
Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +10
-0+ compatible = "atmel,at91sam9260-usart"; + reg = <0xfffff200 0x200>; + interrupts = <1 4 7>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_dbgu>; + status = "disabled"; +}; + +if you have to use multiple bank + pinctrl-0 = <&pinctrl_ip_piaa>, <&pinctrl_ip_piab>;Best regards,
Signature · 6 lines
Nicolas Ferre _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 pinctrl: dt: at91: new binding
Quoted 10 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 5 lines
Sorry but NACK.
I don't want to manage another flavor of the pinmux biding with no real benefit. I would have been good if we had it from day-1. Now it's too late.
yes we do, we catch but a compiling time instead of RUNTIME which is critical
so I?ll pass on the NACK
If you are changing the binding, how about doing it right this time and completely drop the current mess?
Quoted 8 lines
Quoted 5 lines
Moreover, splitting a binding definition if you have a function given by multiple banks can be weird and not well understood in regard to our current group+function definition scheme (Cf. your last example).
Others already do so and this is not complex at all
Signature · 3 lines
Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 pinctrl: dt: at91: new binding
In reply to Jean-Christophe PLAGNIOL-VILLARDHi Jean-Christophe,
Quoted 30 lines · On Sat, 7 Mar 2015 00:49:55 +0800 Jean-Christophe PLAGNIOL-VILLARD…
Quoted 30 lines · Nicolas Ferre
Le 26/02/2015 10:34, Jean-Christophe PLAGNIOL-VILLARD a ?crit :
Quoted 18 lines
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Linus Walleij <[email protected]>Cc devicetree at vger.kernel.org---
.../bindings/pinctrl/atmel,at91-pinctrl.txt| 66 ++++++++++++++++++++++1 file changed, 66 insertions(+)Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +0
-0Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 152
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO BlockSorry but NACK.
Quoted 6 lines
Quoted 2 lines
I don't want to manage another flavor of the pinmux biding with no real benefit. I would have been good if we had it from day-1. Now it's too late.
yes we do, we catch but a compiling time instead of RUNTIME which is critical
so I?ll pass on the NACK
Tell me, how can you pass on a NACK coming from the at91 maintainer (which is also your co-maintainer) when you modify bindings of an at91 driver ?
Please let's try to be constructive here, so that we can find an acceptable solution.
Quoted 6 lines
Quoted 6 lines
Moreover, splitting a binding definition if you have a function given by multiple banks can be weird and not well understood in regard to our current group+function definition scheme (Cf. your last example).
I don't think it's a good idea either: you'll have to split pinconf definitions and that definitely doesn't improve readability.
Quoted 2 lines
Others already do so and this is not complex at all
Could you point out these bindings (and real examples please).
Best Regards,
Boris
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 pinctrl: dt: at91: new binding
Quoted 43 lines · Boris Brezillon
Hi Jean-Christophe,
Quoted 35 lines · On Sat, 7 Mar 2015 00:49:55 +0800 Jean-Christophe PLAGNIOL-VILLARD…
Quoted 30 lines · Nicolas Ferre
Le 26/02/2015 10:34, Jean-Christophe PLAGNIOL-VILLARD a ?crit :
Quoted 18 lines
Today if we want to disable a pio bank we may will siliently break pinctrl configuration in the DT. This will be detected only at runtime.
So move the pinctrl configuration to the bank instead of the bus. This allow to detect pinctrl issue at DT compiling time when disable a bank.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>Cc Linus Walleij <[email protected]>Cc devicetree at vger.kernel.org---
.../bindings/pinctrl/atmel,at91-pinctrl.txt| 66 ++++++++++++++++++++++1 file changed, 66 insertions(+)Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +0
-0Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 152
Quoted 5 lines
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0@@ -148,3 +148,69 @@ dbgu: serial at fffff200 { pinctrl-0 = <&pinctrl_dbgu>; status = "disabled"; }; + +II) New Bindings per PIO BlockSorry but NACK.
Quoted 2 lines
I don't want to manage another flavor of the pinmux biding with no real benefit. I would have been good if we had it from day-1. Now it's too late.
yes we do, we catch but a compiling time instead of RUNTIME which is critical
Quoted 7 lines
Quoted 1 line
so I?ll pass on the NACK
Tell me, how can you pass on a NACK coming from the at91 maintainer (which is also your co-maintainer) when you modify bindings of an at91 driver ?
Please let's try to be constructive here, so that we can find an acceptable solution.
I do pass on it which means, I do not accept to stop here the discussion because of this as Nicolas did for dropping the soc detection when I NACK
Quoted 10 lines
Quoted 7 lines
Quoted 7 lines
Moreover, splitting a binding definition if you have a function given by multiple banks can be weird and not well understood in regard to our current group+function definition scheme (Cf. your last example).
I don't think it's a good idea either: you'll have to split pinconf definitions and that definitely doesn't improve readability.
in HW it?s already the CASE. And today disable a bank you BROKE a board without even known it. This is NOT acceptable when we can detect it at compiling TIME.
Quoted 5 lines
Quoted 3 lines
Others already do so and this is not complex at all
Could you point out these bindings (and real examples please).
look at ST-E 9500 pinctrl they DO use it
Quoted 15 lines
Best Regards,
Boris
Signature · 8 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 pinctrl: dt: at91: new binding
In reply to Jean-Christophe PLAGNIOL-VILLARDDocumentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 174
Quoted 2 lines · Jean-Christophe PLAGNIOL-VILLARD
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0+For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin)
This seems to be describing driver intrinsics in the device tree, like how the hardware is routed on the inside and what it can do.
IMO that is driver territory, the driver should know these limitations and protest if you try to do something illegal.
Anyway as the AT91 maintainers seem to disagree I will allow some more time for discussion before merging the patch.
I can't really have one AT91 maintainer NACKing another, it doesn't matter that this is a separate driver, in my book the MAINTAINERS entry for AT91 as a whole overrides that so can you please find an agreement on how to handle this or I will stall the patch until you're in agreement.
ARM SoC maintainers input would be welcomed.
Yours,
Linus Walleij
PATCH 1/1 pinctrl: dt: at91: new binding
Quoted 12 lines · Linus Walleij
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 174
Quoted 2 lines · Jean-Christophe PLAGNIOL-VILLARD
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0+For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin)This seems to be describing driver intrinsics in the device tree, like how the hardware is routed on the inside and what it can do.
IMO that is driver territory, the driver should know these limitations and protest if you try to do something illegal.
Quoted 8 lines
Anyway as the AT91 maintainers seem to disagree I will allow some more time for discussion before merging the patch.
I can't really have one AT91 maintainer NACKing another, it doesn't matter that this is a separate driver, in my book the
MAINTAINERSentry for AT91 as a whole overrides that so can you please find an agreement on how to handle this or I will stall the patch until you're in agreement.
Nicolas has been the de-facto maintainer of AT91 for quite a while now, even though more of them are listed on the maintainers entry. It would be inappropriate to merge something that he disagreed with on that platform.
Quoted 1 line
ARM SoC maintainers input would be welcomed.
It seems appropriate to ask the at91 folks to come back with a solution that everybody is OK with, and until then hold off merging this.
-Olof
PATCH 1/1 pinctrl: dt: at91: new binding
Quoted 22 lines · Olof Johansson
Quoted 12 lines · Linus Walleij
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt dbgu: serial at fffff200 { · line 174
Quoted 2 lines · Jean-Christophe PLAGNIOL-VILLARD
Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt +2
-0+For each peripheral/bank we will descibe in a u32 if a pin can be +configured in it by putting 1 to the pin bit (1 << pin)This seems to be describing driver intrinsics in the device tree, like how the hardware is routed on the inside and what it can do.
IMO that is driver territory, the driver should know these limitations and protest if you try to do something illegal.
Quoted 8 lines
Anyway as the AT91 maintainers seem to disagree I will allow some more time for discussion before merging the patch.
I can't really have one AT91 maintainer NACKing another, it doesn't matter that this is a separate driver, in my book the
MAINTAINERSentry for AT91 as a whole overrides that so can you please find an agreement on how to handle this or I will stall the patch until you're in agreement.
Quoted 4 lines
Nicolas has been the de-facto maintainer of AT91 for quite a while now, even though more of them are listed on the maintainers entry. It would be inappropriate to merge something that he disagreed with on that platform.
I'm still following it
Quoted 6 lines
Quoted 2 lines
ARM SoC maintainers input would be welcomed.
It seems appropriate to ask the at91 folks to come back with a solution that everybody is OK with, and until then hold off merging this.
when the nack is motivated not just I don't like it
Best Regards,
J.