| Message ID | 20230715112523.2533742-3-contact@jookia.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Conor Dooley |
| 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 15/07/2023 13:25, Jookia wrote: > From: John Watts <contact@jookia.org> > > 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: > Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. > - 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(+) > > diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi > index 1bb1e5cae602..b185398334be 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 { Wrong node naming. Underscores are not allowed. It does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Best regards, Krzysztof
Hello, Thanks for spending your time replying to this. On Sun, Jul 16, 2023 at 06:35:17PM +0200, Krzysztof Kozlowski wrote: > On 15/07/2023 13:25, Jookia wrote: > > From: John Watts <contact@jookia.org> > > > > 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: > > > > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC (and consider --no-git-fallback argument). It might > happen, that command when run on an older kernel, gives you outdated > entries. Therefore please be sure you base your patches on recent Linux > kernel. I ran this script and selected some of the emails and CCed them, at least I believe that's what I did. Maybe I lost them when copying them to vim to make the command line arguments. > > + > > + /omit-if-no-ref/ > > + can0_pins: can0_pins { > > Wrong node naming. Underscores are not allowed. > > It does not look like you tested the DTS against bindings. Please run > `make dtbs_check` (see > Documentation/devicetree/bindings/writing-schema.rst or > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ > for instructions). Hmm. I spent a few hours struggling and testing with make dtbs_check and dt_binding_check. It doesn't seem to pick up on this specific issue, or at least not in this case. But yes I do see this, thanks for the catch. Will fix in v2. I'm basing this patch series on 6.5-rc1, does that make a difference? > Best regards, > Krzysztof Thanks, John.
On 16/07/2023 18:50, John Watts wrote: > Hello, > > Thanks for spending your time replying to this. > > On Sun, Jul 16, 2023 at 06:35:17PM +0200, Krzysztof Kozlowski wrote: >> On 15/07/2023 13:25, Jookia wrote: >>> From: John Watts <contact@jookia.org> >>> >>> 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: >>> >> >> Please use scripts/get_maintainers.pl to get a list of necessary people >> and lists to CC (and consider --no-git-fallback argument). It might >> happen, that command when run on an older kernel, gives you outdated >> entries. Therefore please be sure you base your patches on recent Linux >> kernel. > > I ran this script and selected some of the emails and CCed them, at least I So the others you skipped Like let's pick this maintainer, but skip the others? This is not how does it work. You are expected to CC all maintainers (and to clarify the obvious: maintainers, not random contributors). Some maintainers explicitly agree to be skipped, but where do you see such statement from me? Best regards, Krzysztof
On Mon, Jul 17, 2023 at 08:39:52AM +0200, Krzysztof Kozlowski wrote: > So the others you skipped Like let's pick this maintainer, but skip the > others? This is not how does it work. You are expected to CC all > maintainers (and to clarify the obvious: maintainers, not random > contributors). Some maintainers explicitly agree to be skipped, but > where do you see such statement from me? Apologies, I didn't know you were supposed to CC all maintainers in the list. > > Best regards, > Krzysztof > John.
diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi index 1bb1e5cae602..b185398334be 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"; + }; }; };