[PATCH v2 0/2] ARM: dts: sunxi: Add CAN node and can0_pins_a pinctrl setting

[PATCH v2 0/2] ARM: dts: sunxi: Add CAN node and can0_pins_a pinctrl setting

[Patrick Menschel]

The Allwinner A10/A20 SoCs have an on-board CAN (Controller Area Network)
controller. This patch adds the CAN core to the SoC's include files,
sun4i-a10.dtsi and sun7i-a20.dtsi.

On linux-can mailing list was a discussion about updating the device tree bindings
https://lkml.org/lkml/2015/9/17/220
but it did not progress past writing the documentation file.
Documentation/devicetree/bindings/net/can/sun4i_can.txt

The CAN controller can be enabled in a board specific dts file as
described in the documentation file or by using a device tree overlay.

I have tested the patch v2 on a Banana Pi (A20 SoC) with mainline kernel 4.10.5.

History:
    v2: changed can0_pins_a node contents to new generic binding method, 
	changed can0_pins_a node position by alphabetical order,
        changed can0 device node position by rising physical address order

    v1: initial

Patrick Menschel (2):
  ARM: dts: sun4i: Add CAN node and can0_pins_a pinctrl settings
  ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

 arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
 2 files changed, 26 insertions(+)

-- 
1.9.1

[PATCH v2 0/2] ARM: dts: sunxi: Add CAN node and can0_pins_a pinctrl setting

[Patrick Menschel]

The Allwinner A10/A20 SoCs have an on-board CAN (Controller Area Network)
controller. This patch adds the CAN core to the SoC's include files,
sun4i-a10.dtsi and sun7i-a20.dtsi.

On linux-can mailing list was a discussion about updating the device tree bindings
https://lkml.org/lkml/2015/9/17/220
but it did not progress past writing the documentation file.
Documentation/devicetree/bindings/net/can/sun4i_can.txt

The CAN controller can be enabled in a board specific dts file as
described in the documentation file or by using a device tree overlay.

I have tested the patch v2 on a Banana Pi (A20 SoC) with mainline kernel 4.10.5.

History:
    v2: changed can0_pins_a node contents to new generic binding method, 
	changed can0_pins_a node position by alphabetical order,
        changed can0 device node position by rising physical address order

    v1: initial

Patrick Menschel (2):
  ARM: dts: sun4i: Add CAN node and can0_pins_a pinctrl settings
  ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

 arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
 2 files changed, 26 insertions(+)

-- 
1.9.1

[PATCH v2 1/2] ARM: dts: sun4i: Add CAN node and can0_pins_a pinctrl settings

[Patrick Menschel]

The A10 SoC has an on-board CAN controller. This patch adds the device node
and the corresponding pinctrl settings for pins PH20 and PH21.

This patch is adapted from the description in
Documentation/devicetree/bindings/net/can/sun4i_can.txt

Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index ba20b48..f7dced4 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -974,6 +974,11 @@
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
+			can0_pins_a: can0@0 {
+				pins = "PH20","PH21";
+				function = "can";
+			};
+
 			emac_pins_a: emac0@0 {
 				pins = "PA0", "PA1", "PA2",
 				       "PA3", "PA4", "PA5", "PA6",
@@ -1313,6 +1318,14 @@
 			#size-cells = <0>;
 		};
 
+		can0: can@01c2bc00 {
+			compatible = "allwinner,sun4i-a10-can";
+			reg = <0x01c2bc00 0x400>;
+			interrupts = <26>;
+			clocks = <&apb1_gates 4>;
+			status = "disabled";
+		};
+
 		ps20: ps2@01c2a000 {
 			compatible = "allwinner,sun4i-a10-ps2";
 			reg = <0x01c2a000 0x400>;
-- 
1.9.1

[PATCH v2 1/2] ARM: dts: sun4i: Add CAN node and can0_pins_a pinctrl settings

[Patrick Menschel]

The A10 SoC has an on-board CAN controller. This patch adds the device node
and the corresponding pinctrl settings for pins PH20 and PH21.

This patch is adapted from the description in
Documentation/devicetree/bindings/net/can/sun4i_can.txt

Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index ba20b48..f7dced4 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -974,6 +974,11 @@
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
+			can0_pins_a: can0 at 0 {
+				pins = "PH20","PH21";
+				function = "can";
+			};
+
 			emac_pins_a: emac0 at 0 {
 				pins = "PA0", "PA1", "PA2",
 				       "PA3", "PA4", "PA5", "PA6",
@@ -1313,6 +1318,14 @@
 			#size-cells = <0>;
 		};
 
+		can0: can at 01c2bc00 {
+			compatible = "allwinner,sun4i-a10-can";
+			reg = <0x01c2bc00 0x400>;
+			interrupts = <26>;
+			clocks = <&apb1_gates 4>;
+			status = "disabled";
+		};
+
 		ps20: ps2 at 01c2a000 {
 			compatible = "allwinner,sun4i-a10-ps2";
 			reg = <0x01c2a000 0x400>;
-- 
1.9.1

[PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Patrick Menschel]

The A20 SoC has an on-board CAN controller. This patch adds the device node
and the corresponding pinctrl settings for pins PH20 and PH21.

The CAN controller is inherited from the A10 SoC and uses the same driver.

This patch is adapted from the description in
Documentation/devicetree/bindings/net/can/sun4i_can.txt

Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 2db97fc..25af586 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -1096,6 +1096,11 @@
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
+			can0_pins_a: can0@0 {
+				pins = "PH20","PH21";
+				function = "can";
+			};
+
 			clk_out_a_pins_a: clk_out_a@0 {
 				pins = "PI12";
 				function = "clk_out_a";
@@ -1582,6 +1587,14 @@
 			#size-cells = <0>;
 		};
 
+		can0: can@01c2bc00 {
+			compatible = "allwinner,sun4i-a10-can";
+			reg = <0x01c2bc00 0x400>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&apb1_gates 4>;
+			status = "disabled";
+		};
+
 		i2c4: i2c@01c2c000 {
 			compatible = "allwinner,sun7i-a20-i2c",
 				     "allwinner,sun4i-a10-i2c";
-- 
1.9.1

[PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Patrick Menschel]

The A20 SoC has an on-board CAN controller. This patch adds the device node
and the corresponding pinctrl settings for pins PH20 and PH21.

The CAN controller is inherited from the A10 SoC and uses the same driver.

This patch is adapted from the description in
Documentation/devicetree/bindings/net/can/sun4i_can.txt

Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 2db97fc..25af586 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -1096,6 +1096,11 @@
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
+			can0_pins_a: can0 at 0 {
+				pins = "PH20","PH21";
+				function = "can";
+			};
+
 			clk_out_a_pins_a: clk_out_a at 0 {
 				pins = "PI12";
 				function = "clk_out_a";
@@ -1582,6 +1587,14 @@
 			#size-cells = <0>;
 		};
 
+		can0: can at 01c2bc00 {
+			compatible = "allwinner,sun4i-a10-can";
+			reg = <0x01c2bc00 0x400>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&apb1_gates 4>;
+			status = "disabled";
+		};
+
 		i2c4: i2c at 01c2c000 {
 			compatible = "allwinner,sun7i-a20-i2c",
 				     "allwinner,sun4i-a10-i2c";
-- 
1.9.1

Re: [PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1660 bytes --]

Hi Patrick,

On Fri, Mar 24, 2017 at 10:52:19PM +0100, Patrick Menschel wrote:
> The A20 SoC has an on-board CAN controller. This patch adds the device node
> and the corresponding pinctrl settings for pins PH20 and PH21.
> 
> The CAN controller is inherited from the A10 SoC and uses the same driver.
> 
> This patch is adapted from the description in
> Documentation/devicetree/bindings/net/can/sun4i_can.txt
> 
> Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 2db97fc..25af586 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -1096,6 +1096,11 @@
>  			#interrupt-cells = <3>;
>  			#gpio-cells = <3>;
>  
> +			can0_pins_a: can0@0 {
> +				pins = "PH20","PH21";
> +				function = "can";
> +			};
> +
>  			clk_out_a_pins_a: clk_out_a@0 {
>  				pins = "PI12";
>  				function = "clk_out_a";
> @@ -1582,6 +1587,14 @@
>  			#size-cells = <0>;
>  		};
>  
> +		can0: can@01c2bc00 {
> +			compatible = "allwinner,sun4i-a10-can";

Sorry for not spotting this earlier, but this would need an A20
compatible too, to deal with the case where it turns out not to be
compatible.

you can do something like this:
compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";

Ideally the pinctrl groups addition should be split out in a separate
patch too.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

Hi Patrick,

On Fri, Mar 24, 2017 at 10:52:19PM +0100, Patrick Menschel wrote:
> The A20 SoC has an on-board CAN controller. This patch adds the device node
> and the corresponding pinctrl settings for pins PH20 and PH21.
> 
> The CAN controller is inherited from the A10 SoC and uses the same driver.
> 
> This patch is adapted from the description in
> Documentation/devicetree/bindings/net/can/sun4i_can.txt
> 
> Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 2db97fc..25af586 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -1096,6 +1096,11 @@
>  			#interrupt-cells = <3>;
>  			#gpio-cells = <3>;
>  
> +			can0_pins_a: can0@0 {
> +				pins = "PH20","PH21";
> +				function = "can";
> +			};
> +
>  			clk_out_a_pins_a: clk_out_a@0 {
>  				pins = "PI12";
>  				function = "clk_out_a";
> @@ -1582,6 +1587,14 @@
>  			#size-cells = <0>;
>  		};
>  
> +		can0: can@01c2bc00 {
> +			compatible = "allwinner,sun4i-a10-can";

Sorry for not spotting this earlier, but this would need an A20
compatible too, to deal with the case where it turns out not to be
compatible.

you can do something like this:
compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";

Ideally the pinctrl groups addition should be split out in a separate
patch too.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Maxime Ripard]

Hi Patrick,

On Fri, Mar 24, 2017 at 10:52:19PM +0100, Patrick Menschel wrote:
> The A20 SoC has an on-board CAN controller. This patch adds the device node
> and the corresponding pinctrl settings for pins PH20 and PH21.
> 
> The CAN controller is inherited from the A10 SoC and uses the same driver.
> 
> This patch is adapted from the description in
> Documentation/devicetree/bindings/net/can/sun4i_can.txt
> 
> Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 2db97fc..25af586 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -1096,6 +1096,11 @@
>  			#interrupt-cells = <3>;
>  			#gpio-cells = <3>;
>  
> +			can0_pins_a: can0 at 0 {
> +				pins = "PH20","PH21";
> +				function = "can";
> +			};
> +
>  			clk_out_a_pins_a: clk_out_a at 0 {
>  				pins = "PI12";
>  				function = "clk_out_a";
> @@ -1582,6 +1587,14 @@
>  			#size-cells = <0>;
>  		};
>  
> +		can0: can at 01c2bc00 {
> +			compatible = "allwinner,sun4i-a10-can";

Sorry for not spotting this earlier, but this would need an A20
compatible too, to deal with the case where it turns out not to be
compatible.

you can do something like this:
compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";

Ideally the pinctrl groups addition should be split out in a separate
patch too.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170327/b830b6cd/attachment.sig>

Re: [PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Patrick Menschel]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

Hello Maxime,

Am 27.03.2017 um 08:46 schrieb Maxime Ripard:
>> @@ -1582,6 +1587,14 @@
>>  			#size-cells = <0>;
>>  		};
>>  
>> +		can0: can@01c2bc00 {
>> +			compatible = "allwinner,sun4i-a10-can";
> 
> Sorry for not spotting this earlier, but this would need an A20
> compatible too, to deal with the case where it turns out not to be
> compatible.
> 
> you can do something like this:
> compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";

Adding "sun7i-a20-can" to the compatible list results in a checkpatch
warning, that the "driver" is not documented.

 WARNING: DT compatible string "allwinner,sun7i-a20-can" appears
un-documented -- check ./Documentation/devicetree/bindings/
#27: FILE: arch/arm/boot/dts/sun7i-a20.dtsi:1586:
+			compatible = "allwinner,sun7i-a20-can",

Is it really OK to add a driver that doesn't exist yet?

Thanks,
Patrick


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3709 bytes --]

[PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Patrick Menschel]

Hello Maxime,

Am 27.03.2017 um 08:46 schrieb Maxime Ripard:
>> @@ -1582,6 +1587,14 @@
>>  			#size-cells = <0>;
>>  		};
>>  
>> +		can0: can at 01c2bc00 {
>> +			compatible = "allwinner,sun4i-a10-can";
> 
> Sorry for not spotting this earlier, but this would need an A20
> compatible too, to deal with the case where it turns out not to be
> compatible.
> 
> you can do something like this:
> compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";

Adding "sun7i-a20-can" to the compatible list results in a checkpatch
warning, that the "driver" is not documented.

 WARNING: DT compatible string "allwinner,sun7i-a20-can" appears
un-documented -- check ./Documentation/devicetree/bindings/
#27: FILE: arch/arm/boot/dts/sun7i-a20.dtsi:1586:
+			compatible = "allwinner,sun7i-a20-can",

Is it really OK to add a driver that doesn't exist yet?

Thanks,
Patrick

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3709 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170331/e81c8fde/attachment.p7s>

Re: [PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Maxime Ripard]

On Fri, Mar 31, 2017 at 06:51:38PM +0200, Patrick Menschel wrote:
> Hello Maxime,
> 
> Am 27.03.2017 um 08:46 schrieb Maxime Ripard:
> >> @@ -1582,6 +1587,14 @@
> >>  			#size-cells = <0>;
> >>  		};
> >>  
> >> +		can0: can@01c2bc00 {
> >> +			compatible = "allwinner,sun4i-a10-can";
> > 
> > Sorry for not spotting this earlier, but this would need an A20
> > compatible too, to deal with the case where it turns out not to be
> > compatible.
> > 
> > you can do something like this:
> > compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";
> 
> Adding "sun7i-a20-can" to the compatible list results in a checkpatch
> warning, that the "driver" is not documented.
> 
>  WARNING: DT compatible string "allwinner,sun7i-a20-can" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #27: FILE: arch/arm/boot/dts/sun7i-a20.dtsi:1586:
> +			compatible = "allwinner,sun7i-a20-can",
> 
> Is it really OK to add a driver that doesn't exist yet?

It's not what you are doing, what you're doing is adding an extra
compatible that is not there yet.

Since it doesn't exist, it will fallback on the second one, which does
exist. So yes, it's ok to do so.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v2 2/2] ARM: dts: sun7i: Add CAN node and can0_pins_a pinctrl settings

[Maxime Ripard]

On Fri, Mar 31, 2017 at 06:51:38PM +0200, Patrick Menschel wrote:
> Hello Maxime,
> 
> Am 27.03.2017 um 08:46 schrieb Maxime Ripard:
> >> @@ -1582,6 +1587,14 @@
> >>  			#size-cells = <0>;
> >>  		};
> >>  
> >> +		can0: can at 01c2bc00 {
> >> +			compatible = "allwinner,sun4i-a10-can";
> > 
> > Sorry for not spotting this earlier, but this would need an A20
> > compatible too, to deal with the case where it turns out not to be
> > compatible.
> > 
> > you can do something like this:
> > compatible = "allwinner,sun7i-a20-can", "allwinner,sun4i-a10-can";
> 
> Adding "sun7i-a20-can" to the compatible list results in a checkpatch
> warning, that the "driver" is not documented.
> 
>  WARNING: DT compatible string "allwinner,sun7i-a20-can" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #27: FILE: arch/arm/boot/dts/sun7i-a20.dtsi:1586:
> +			compatible = "allwinner,sun7i-a20-can",
> 
> Is it really OK to add a driver that doesn't exist yet?

It's not what you are doing, what you're doing is adding an extra
compatible that is not there yet.

Since it doesn't exist, it will fallback on the second one, which does
exist. So yes, it's ok to do so.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com