[PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[John Watts]

The Allwinner D1, T113 provide two CAN controllers that are variants
of the R40 controller.

I have tested support for these controllers on two boards:

- A Lichee Panel RV 86 Panel running a D1 chip
- A Mango Pi MQ Dual running a T113-s3 chip

Both of these fully support both CAN controllers.

Signed-off-by: John Watts <contact@jookia.org>
---
Changes in v3:
- Set default pinctrl for can controller
- Moved can nodes to proper location
- Moved can pins to proper location

 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index d59b4acf183a..24f2e70d5886 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -52,6 +52,18 @@ pio: pinctrl@2000000 {
 			#gpio-cells = <3>;
 			#interrupt-cells = <3>;
 
+			/omit-if-no-ref/
+			can0_pins: can0-pins {
+				pins = "PB2", "PB3";
+				function = "can0";
+			};
+
+			/omit-if-no-ref/
+			can1_pins: can1-pins {
+				pins = "PB4", "PB5";
+				function = "can1";
+			};
+
 			/omit-if-no-ref/
 			clk_pg11_pin: clk-pg11-pin {
 				pins = "PG11";
@@ -356,6 +368,28 @@ i2c3: i2c@2502c00 {
 			#size-cells = <0>;
 		};
 
+		can0: can@2504000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&can0_pins>;
+			compatible = "allwinner,sun20i-d1-can";
+			reg = <0x02504000 0x400>;
+			interrupts = <SOC_PERIPHERAL_IRQ(21) IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CAN0>;
+			resets = <&ccu RST_BUS_CAN0>;
+			status = "disabled";
+		};
+
+		can1: can@2504400 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&can1_pins>;
+			compatible = "allwinner,sun20i-d1-can";
+			reg = <0x02504400 0x400>;
+			interrupts = <SOC_PERIPHERAL_IRQ(22) IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CAN1>;
+			resets = <&ccu RST_BUS_CAN1>;
+			status = "disabled";
+		};
+
 		syscon: syscon@3000000 {
 			compatible = "allwinner,sun20i-d1-system-control";
 			reg = <0x3000000 0x1000>;
-- 
2.41.0


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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[Markus Elfring]

…
> Both of these fully support both CAN controllers.

Please choose a corresponding imperative change suggestion.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc5#n94


> Signed-off-by: John Watts <contact@jookia.org>

I find the local-part of this email address suspicious according to the Developer's Certificate of Origin.
Should it usually indicate an unique person instead of the shown key word?

Regards,
Markus

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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[Conor Dooley]

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

On Tue, Aug 08, 2023 at 04:26:17PM +0200, Markus Elfring wrote:
> …
> > Both of these fully support both CAN controllers.
> 
> Please choose a corresponding imperative change suggestion.
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc5#n94
> 
> 
> > Signed-off-by: John Watts <contact@jookia.org>
> 
> I find the local-part of this email address suspicious according to the Developer's Certificate of Origin.
> Should it usually indicate an unique person instead of the shown key word?

Markus, please stop bothering developers with nuisance comments.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 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: [v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[Markus Elfring]

> Markus, please stop bothering developers with nuisance comments.

Why do you interpret a recurring patch review in such a direction?

How much do mentioned concerns hinder the integration of the discussed patch?

Regards,
Markus

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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[Marc Kleine-Budde]

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

On 08.08.2023 16:26:17, Markus Elfring wrote:
> …
> > Both of these fully support both CAN controllers.
> 
> Please choose a corresponding imperative change suggestion.
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc5#n94
> 
> 
> > Signed-off-by: John Watts <contact@jookia.org>
> 
> I find the local-part of this email address suspicious according to the Developer's Certificate of Origin.
> Should it usually indicate an unique person instead of the shown key word?

FYI: https://lore.kernel.org/all/20230718221504.GA2015343-robh@kernel.org

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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 v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[Chen-Yu Tsai]

On Tue, Aug 8, 2023 at 3:20 AM John Watts <contact@jookia.org> wrote:
>
> The Allwinner D1, T113 provide two CAN controllers that are variants
> of the R40 controller.
>
> I have tested support for these controllers on two boards:
>
> - A Lichee Panel RV 86 Panel running a D1 chip
> - A Mango Pi MQ Dual running a T113-s3 chip
>
> Both of these fully support both CAN controllers.
>
> Signed-off-by: John Watts <contact@jookia.org>
> ---
> Changes in v3:
> - Set default pinctrl for can controller
> - Moved can nodes to proper location
> - Moved can pins to proper location
>
>  .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> index d59b4acf183a..24f2e70d5886 100644
> --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> @@ -52,6 +52,18 @@ pio: pinctrl@2000000 {
>                         #gpio-cells = <3>;
>                         #interrupt-cells = <3>;
>
> +                       /omit-if-no-ref/

Just FYI this likely ends up doing nothing if you also have them
referenced through a default pinctrl setting. They end up always
referenced and always included. For the D1 series it looks like no
peripheral has default pinctrl setting given.

We can still keep it though. It would help when future chip variants
specify different pinmuxes.

> +                       can0_pins: can0-pins {
> +                               pins = "PB2", "PB3";
> +                               function = "can0";
> +                       };
> +
> +                       /omit-if-no-ref/
> +                       can1_pins: can1-pins {
> +                               pins = "PB4", "PB5";
> +                               function = "can1";
> +                       };
> +
>                         /omit-if-no-ref/
>                         clk_pg11_pin: clk-pg11-pin {
>                                 pins = "PG11";
> @@ -356,6 +368,28 @@ i2c3: i2c@2502c00 {
>                         #size-cells = <0>;
>                 };
>
> +               can0: can@2504000 {
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&can0_pins>;

The compatible string should be the first property. In other sunxi SoC dtsi
files, we put the pinctrl just before the "status" property if it's present
to specify a default pin muxing.

I can fix it up while applying.

ChenYu

> +                       compatible = "allwinner,sun20i-d1-can";
> +                       reg = <0x02504000 0x400>;
> +                       interrupts = <SOC_PERIPHERAL_IRQ(21) IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_CAN0>;
> +                       resets = <&ccu RST_BUS_CAN0>;
> +                       status = "disabled";
> +               };
> +
> +               can1: can@2504400 {
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&can1_pins>;
> +                       compatible = "allwinner,sun20i-d1-can";
> +                       reg = <0x02504400 0x400>;
> +                       interrupts = <SOC_PERIPHERAL_IRQ(22) IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&ccu CLK_BUS_CAN1>;
> +                       resets = <&ccu RST_BUS_CAN1>;
> +                       status = "disabled";
> +               };
> +
>                 syscon: syscon@3000000 {
>                         compatible = "allwinner,sun20i-d1-system-control";
>                         reg = <0x3000000 0x1000>;
> --
> 2.41.0
>
>

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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[John Watts]

On Sun, Aug 13, 2023 at 12:17:27PM +0800, Chen-Yu Tsai wrote:
> > Signed-off-by: John Watts <contact@jookia.org>
> > ---
> > Changes in v3:
> > - Set default pinctrl for can controller
> > - Moved can nodes to proper location
> > - Moved can pins to proper location
> >
> >  .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 34 +++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> > index d59b4acf183a..24f2e70d5886 100644
> > --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> > +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> > @@ -52,6 +52,18 @@ pio: pinctrl@2000000 {
> >                         #gpio-cells = <3>;
> >                         #interrupt-cells = <3>;
> >
> > +                       /omit-if-no-ref/
> 
> Just FYI this likely ends up doing nothing if you also have them
> referenced through a default pinctrl setting. They end up always
> referenced and always included. For the D1 series it looks like no
> peripheral has default pinctrl setting given.
>
> We can still keep it though. It would help when future chip variants
> specify different pinmuxes.

Oops, thanks for pointing that out. I'll try to avoid that mistake in future.

> The compatible string should be the first property. In other sunxi SoC dtsi
> files, we put the pinctrl just before the "status" property if it's present
> to specify a default pin muxing.

Oh that makes sense.

> 
> I can fix it up while applying.

Please do!

> ChenYu

Thanks,

John.

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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[Chen-Yu Tsai]

On Sun, Aug 13, 2023 at 12:23 PM John Watts <contact@jookia.org> wrote:
>
> On Sun, Aug 13, 2023 at 12:17:27PM +0800, Chen-Yu Tsai wrote:
> > > Signed-off-by: John Watts <contact@jookia.org>
> > > ---
> > > Changes in v3:
> > > - Set default pinctrl for can controller
> > > - Moved can nodes to proper location
> > > - Moved can pins to proper location
> > >
> > >  .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    | 34 +++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > >
> > > diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> > > index d59b4acf183a..24f2e70d5886 100644
> > > --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> > > +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
> > > @@ -52,6 +52,18 @@ pio: pinctrl@2000000 {
> > >                         #gpio-cells = <3>;
> > >                         #interrupt-cells = <3>;
> > >
> > > +                       /omit-if-no-ref/
> >
> > Just FYI this likely ends up doing nothing if you also have them
> > referenced through a default pinctrl setting. They end up always
> > referenced and always included. For the D1 series it looks like no
> > peripheral has default pinctrl setting given.
> >
> > We can still keep it though. It would help when future chip variants
> > specify different pinmuxes.
>
> Oops, thanks for pointing that out. I'll try to avoid that mistake in future.
>
> > The compatible string should be the first property. In other sunxi SoC dtsi
> > files, we put the pinctrl just before the "status" property if it's present
> > to specify a default pin muxing.
>
> Oh that makes sense.
>
> >
> > I can fix it up while applying.
>
> Please do!

Applied. Please check here:

https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/commit/?h=sunxi/dt-for-6.6&id=f05af44f691351bfd954f39ec376666dc5e1b869

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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[John Watts]

On Sun, Aug 13, 2023 at 02:14:42PM +0800, Chen-Yu Tsai wrote:
> Applied. Please check here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git/commit/?h=sunxi/dt-for-6.6&id=f05af44f691351bfd954f39ec376666dc5e1b869

Looks good to me. Thank you.

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

Re: [PATCH v3] riscv: dts: allwinner: d1: Add CAN controller nodes

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (fixes)
by Chen-Yu Tsai <wens@csie.org>:

On Tue,  8 Aug 2023 05:19:52 +1000 you wrote:
> The Allwinner D1, T113 provide two CAN controllers that are variants
> of the R40 controller.
> 
> I have tested support for these controllers on two boards:
> 
> - A Lichee Panel RV 86 Panel running a D1 chip
> - A Mango Pi MQ Dual running a T113-s3 chip
> 
> [...]

Here is the summary with links:
  - [v3] riscv: dts: allwinner: d1: Add CAN controller nodes
    https://git.kernel.org/riscv/c/f05af44f6913

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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