| Message ID | 20230721221552.1973203-4-contact@jookia.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6ea1ad888f5900953a21853e709fa499fdfcb317 |
| Headers | show |
| Series | Add support for Allwinner D1 CAN controllers | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 41 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Sat, Jul 22, 2023 at 08:15:51AM +1000, John Watts wrote: > ... > + /omit-if-no-ref/ > + can0_pins: can0-pins { > + pins = "PB2", "PB3"; > + function = "can0"; > + }; > ... > + can0: can@2504000 { > + 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"; > + }; Just a quick late night question to people with more knowledge than me: These chips only have one pinctrl configuration for can0 and can1. Should the can nodes have this pre-set instead of the board dts doing this? I see this happening in sun4i-a10.dtsi for instance, but it also seems like it could become a problem when it comes to re-using the dtsi for newer chip variants. John.
Dne nedelja, 23. julij 2023 ob 11:18:33 CEST je John Watts napisal(a): > On Sat, Jul 22, 2023 at 08:15:51AM +1000, John Watts wrote: > > ... > > + /omit-if-no-ref/ > > + can0_pins: can0-pins { > > + pins = "PB2", "PB3"; > > + function = "can0"; > > + }; > > ... > > + can0: can@2504000 { > > + 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"; > > + }; > > Just a quick late night question to people with more knowledge than me: > > These chips only have one pinctrl configuration for can0 and can1. Should > the can nodes have this pre-set instead of the board dts doing this? Yes, that's usually how it's done. > > I see this happening in sun4i-a10.dtsi for instance, but it also seems like > it could become a problem when it comes to re-using the dtsi for newer chip > variants. Properties can be either rewritten or deleted further down, so don't worry about that. Best regards, Jernej > > John.
On Mon, Jul 31, 2023 at 12:03:59AM +0200, Jernej Škrabec wrote: > Yes, that's usually how it's done. > > > > > I see this happening in sun4i-a10.dtsi for instance, but it also seems like > > it could become a problem when it comes to re-using the dtsi for newer chip > > variants. > > Properties can be either rewritten or deleted further down, so don't worry > about that. > > Best regards, > Jernej > > > > > John. Thanks for the feedback, I've sent a patch to address this. John.
Hi John, Jernej On Sat, Jul 22, 2023 at 08:15:51AM +1000, John Watts wrote: > ... > @@ -131,6 +131,18 @@ uart3_pb_pins: uart3-pb-pins { > pins = "PB6", "PB7"; > function = "uart3"; > }; > + > + /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"; > + }; > ... Should we also keep a pinctrl nodes itself in alphabetical order? I mean placing a CAN nodes before `clk_pg11_pin` node? Looks like the other nodes sorted in this way... Cheers, Maksim
On Sat, Aug 05, 2023 at 07:40:52PM +0300, Maksim Kiselev wrote: > Hi John, Jernej > Should we also keep a pinctrl nodes itself in alphabetical order? > I mean placing a CAN nodes before `clk_pg11_pin` node? > Looks like the other nodes sorted in this way... Good catch. Now that you mention it, the device tree nodes are sorted by memory order too! These should be after i2c3. It looks like I might need to do a patch to re-order those too. > Cheers, > Maksim John.
Dne sobota, 05. avgust 2023 ob 18:51:53 CEST je John Watts napisal(a): > On Sat, Aug 05, 2023 at 07:40:52PM +0300, Maksim Kiselev wrote: > > Hi John, Jernej > > Should we also keep a pinctrl nodes itself in alphabetical order? > > I mean placing a CAN nodes before `clk_pg11_pin` node? > > Looks like the other nodes sorted in this way... > > Good catch. Now that you mention it, the device tree nodes are sorted > by memory order too! These should be after i2c3. > > It looks like I might need to do a patch to re-order those too. It would be better if DT patches are dropped from netdev tree and then post new versions. Best regards, Jernej > > > Cheers, > > Maksim > > John.
On Sat, Aug 05, 2023 at 07:49:51PM +0200, Jernej Škrabec wrote: > Dne sobota, 05. avgust 2023 ob 18:51:53 CEST je John Watts napisal(a): > > On Sat, Aug 05, 2023 at 07:40:52PM +0300, Maksim Kiselev wrote: > > > Hi John, Jernej > > > Should we also keep a pinctrl nodes itself in alphabetical order? > > > I mean placing a CAN nodes before `clk_pg11_pin` node? > > > Looks like the other nodes sorted in this way... > > > > Good catch. Now that you mention it, the device tree nodes are sorted > > by memory order too! These should be after i2c3. > > > > It looks like I might need to do a patch to re-order those too. > > It would be better if DT patches are dropped from netdev tree and then post > new versions. > > Best regards, > Jernej Agreed. Is there a way to request that? Or will the maintainer just read this? John.
Dne nedelja, 06. avgust 2023 ob 08:33:45 CEST je John Watts napisal(a): > On Sat, Aug 05, 2023 at 07:49:51PM +0200, Jernej Škrabec wrote: > > Dne sobota, 05. avgust 2023 ob 18:51:53 CEST je John Watts napisal(a): > > > On Sat, Aug 05, 2023 at 07:40:52PM +0300, Maksim Kiselev wrote: > > > > Hi John, Jernej > > > > Should we also keep a pinctrl nodes itself in alphabetical order? > > > > I mean placing a CAN nodes before `clk_pg11_pin` node? > > > > Looks like the other nodes sorted in this way... > > > > > > Good catch. Now that you mention it, the device tree nodes are sorted > > > by memory order too! These should be after i2c3. > > > > > > It looks like I might need to do a patch to re-order those too. > > > > It would be better if DT patches are dropped from netdev tree and then > > post > > new versions. > > > > Best regards, > > Jernej > > Agreed. Is there a way to request that? Or will the maintainer just read > this? Hopefully it will. Best regards, Jernej
On 06.08.2023 13:42:28, Jernej Škrabec wrote: > Dne nedelja, 06. avgust 2023 ob 08:33:45 CEST je John Watts napisal(a): > > On Sat, Aug 05, 2023 at 07:49:51PM +0200, Jernej Škrabec wrote: > > > Dne sobota, 05. avgust 2023 ob 18:51:53 CEST je John Watts napisal(a): > > > > On Sat, Aug 05, 2023 at 07:40:52PM +0300, Maksim Kiselev wrote: > > > > > Hi John, Jernej > > > > > Should we also keep a pinctrl nodes itself in alphabetical order? > > > > > I mean placing a CAN nodes before `clk_pg11_pin` node? > > > > > Looks like the other nodes sorted in this way... > > > > > > > > Good catch. Now that you mention it, the device tree nodes are sorted > > > > by memory order too! These should be after i2c3. > > > > > > > > It looks like I might need to do a patch to re-order those too. > > > > > > It would be better if DT patches are dropped from netdev tree and then > > > post > > > new versions. > > > > > > Best regards, > > > Jernej > > > > Agreed. Is there a way to request that? Or will the maintainer just read > > this? > > Hopefully it will. I'm just catching up on last week's post (I had a long off-line weekend). I'll revert the DT changes and send a PR to net-next. Marc
On 07.08.2023 09:16:41, Marc Kleine-Budde wrote: > On 06.08.2023 13:42:28, Jernej Škrabec wrote: > > Dne nedelja, 06. avgust 2023 ob 08:33:45 CEST je John Watts napisal(a): > > > On Sat, Aug 05, 2023 at 07:49:51PM +0200, Jernej Škrabec wrote: > > > > Dne sobota, 05. avgust 2023 ob 18:51:53 CEST je John Watts napisal(a): > > > > > On Sat, Aug 05, 2023 at 07:40:52PM +0300, Maksim Kiselev wrote: > > > > > > Hi John, Jernej > > > > > > Should we also keep a pinctrl nodes itself in alphabetical order? > > > > > > I mean placing a CAN nodes before `clk_pg11_pin` node? > > > > > > Looks like the other nodes sorted in this way... > > > > > > > > > > Good catch. Now that you mention it, the device tree nodes are sorted > > > > > by memory order too! These should be after i2c3. > > > > > > > > > > It looks like I might need to do a patch to re-order those too. > > > > > > > > It would be better if DT patches are dropped from netdev tree and then > > > > post > > > > new versions. > > > > > > > > Best regards, > > > > Jernej > > > > > > Agreed. Is there a way to request that? Or will the maintainer just read > > > this? > > > > Hopefully it will. > > I'm just catching up on last week's post (I had a long off-line > weekend). > > I'll revert the DT changes and send a PR to net-next. Here's the revert: | https://lore.kernel.org/all/20230807-riscv-allwinner-d1-revert-can-controller-nodes-v1-1-eb3f70b435d9@pengutronix.de/ Marc
diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi index 1bb1e5cae602..4086c0cc0f9d 100644 --- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi +++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi @@ -131,6 +131,18 @@ uart3_pb_pins: uart3-pb-pins { pins = "PB6", "PB7"; function = "uart3"; }; + + /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"; + }; }; ccu: clock-controller@2001000 { @@ -879,5 +891,23 @@ rtc: rtc@7090000 { clock-names = "bus", "hosc", "ahb"; #clock-cells = <1>; }; + + can0: can@2504000 { + 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 { + 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"; + }; }; };
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> --- .../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+)