From: Jacky Huang <[email protected]>
Add 'pinctrl' node and 'gpioa' ~ 'gpion' nodes to the dtsi of ma35d1
SoC and describe default pin configurations.
Enable all UART nodes presented on som and iot boards, and add pinctrl
function settings to these nodes.
Signed-off-by: Jacky Huang <[email protected]>
---
.../boot/dts/nuvoton/ma35d1-iot-512m.dts | 70 +++++++-
.../boot/dts/nuvoton/ma35d1-som-256m.dts | 73 +++++++-
arch/arm64/boot/dts/nuvoton/ma35d1.dtsi | 159 +++++++++++++++++-
3 files changed, 293 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts
index b89e2be6abae..74a85c23c26b 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts
@@ -14,6 +14,10 @@ / {
aliases {
serial0 = &uart0;
+ serial10 = &uart10;
+ serial12 = &uart12;
+ serial13 = &uart13;
+ serial14 = &uart14;
};
chosen {
@@ -33,10 +37,6 @@ clk_hxt: clock-hxt {
};
};
-&uart0 {
- status = "okay";
-};
-
&clk {
assigned-clocks = <&clk CAPLL>,
<&clk DDRPLL>,
@@ -54,3 +54,65 @@ &clk {
"integer",
"integer";
};
+
+&pinctrl {
+ uart-grp {
+ pinctrl_uart0: uart0-pins {
+ nuvoton,pins = <4 14 1 &pcfg_default>,
+ <4 15 1 &pcfg_default>;
+ };
+
+ pinctrl_uart10: uart10-pins {
+ nuvoton,pins = <7 4 2 &pcfg_default>,
+ <7 5 2 &pcfg_default>,
+ <7 6 2 &pcfg_default>,
+ <7 7 2 &pcfg_default>;
+ };
+
+ pinctrl_uart12: uart12-pins {
+ nuvoton,pins = <2 13 2 &pcfg_default>,
+ <2 14 2 &pcfg_default>,
+ <2 15 2 &pcfg_default>;
+ };
+
+ pinctrl_uart13: uart13-pins {
+ nuvoton,pins = <7 12 3 &pcfg_default>,
+ <7 13 3 &pcfg_default>;
+ };
+
+ pinctrl_uart14: uart14-pins {
+ nuvoton,pins = <7 14 2 &pcfg_default>,
+ <7 15 2 &pcfg_default>;
+ };
+ };
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart0>;
+ status = "okay";
+};
+
+&uart10 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart10>;
+ status = "okay";
+};
+
+&uart12 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart12>;
+ status = "okay";
+};
+
+&uart13 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart13>;
+ status = "okay";
+};
+
+&uart14 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart14>;
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
index a1ebddecb7f8..2cd11e2b2067 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
@@ -14,6 +14,10 @@ / {
aliases {
serial0 = &uart0;
+ serial11 = &uart11;
+ serial12 = &uart12;
+ serial14 = &uart14;
+ serial16 = &uart16;
};
chosen {
@@ -33,10 +37,6 @@ clk_hxt: clock-hxt {
};
};
-&uart0 {
- status = "okay";
-};
-
&clk {
assigned-clocks = <&clk CAPLL>,
<&clk DDRPLL>,
@@ -54,3 +54,68 @@ &clk {
"integer",
"integer";
};
+
+&pinctrl {
+ uart-grp {
+ pinctrl_uart0: uart0-pins {
+ nuvoton,pins = <4 14 1 &pcfg_default>,
+ <4 15 1 &pcfg_default>;
+ };
+
+ pinctrl_uart11: uart11-pins {
+ nuvoton,pins = <11 0 2 &pcfg_default>,
+ <11 1 2 &pcfg_default>,
+ <11 2 2 &pcfg_default>,
+ <11 3 2 &pcfg_default>;
+ };
+
+ pinctrl_uart12: uart12-pins {
+ nuvoton,pins = <8 1 2 &pcfg_default>,
+ <8 2 2 &pcfg_default>,
+ <8 3 2 &pcfg_default>;
+ };
+
+ pinctrl_uart14: uart14-pins {
+ nuvoton,pins = <8 5 2 &pcfg_default>,
+ <8 6 2 &pcfg_default>,
+ <8 7 2 &pcfg_default>;
+ };
+
+ pinctrl_uart16: uart16-pins {
+ nuvoton,pins = <10 0 2 &pcfg_default>,
+ <10 1 2 &pcfg_default>,
+ <10 2 2 &pcfg_default>,
+ <10 3 2 &pcfg_default>;
+ };
+ };
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart0>;
+ status = "okay";
+};
+
+&uart11 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart11>;
+ status = "okay";
+};
+
+&uart12 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart12>;
+ status = "okay";
+};
+
+&uart14 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart14>;
+ status = "okay";
+};
+
+&uart16 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_uart16>;
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
index 781cdae566a0..a93bce545f5f 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
@@ -10,6 +10,7 @@
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
#include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
+#include <dt-bindings/pinctrl/ma35d1-pinfunc.h>
/ {
compatible = "nuvoton,ma35d1";
@@ -83,7 +84,7 @@ soc {
ranges;
sys: system-management@40460000 {
- compatible = "nuvoton,ma35d1-reset";
+ compatible = "nuvoton,ma35d1-reset", "syscon";
reg = <0x0 0x40460000 0x0 0x200>;
#reset-cells = <1>;
};
@@ -95,6 +96,162 @@ clk: clock-controller@40460200 {
clocks = <&clk_hxt>;
};
+ pinctrl: pinctrl@40040000 {
+ compatible = "nuvoton,ma35d1-pinctrl";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ nuvoton,sys = <&sys>;
+ ranges = <0x0 0x0 0x40040000 0xc00>;
+
+ gpioa: gpio@40040000 {
+ reg = <0x0 0x40>;
+ interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPA_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiob: gpio@40040040 {
+ reg = <0x40 0x40>;
+ interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPB_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpioc: gpio@40040080 {
+ reg = <0x80 0x40>;
+ interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPC_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiod: gpio@400400c0 {
+ reg = <0xc0 0x40>;
+ interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPD_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpioe: gpio@40040100 {
+ reg = <0x100 0x40>;
+ interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPE_GATE>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiof: gpio@40040140 {
+ reg = <0x140 0x40>;
+ interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPF_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiog: gpio@40040180 {
+ reg = <0x180 0x40>;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPG_GATE>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpioh: gpio@400401c0 {
+ reg = <0x1c0 0x40>;
+ interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPH_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpioi: gpio@40040200 {
+ reg = <0x200 0x40>;
+ interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPI_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpioj: gpio@40040240 {
+ reg = <0x240 0x40>;
+ interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPJ_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiok: gpio@40040280 {
+ reg = <0x280 0x40>;
+ interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPK_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiol: gpio@400402c0 {
+ reg = <0x2c0 0x40>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPL_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpiom: gpio@40040300 {
+ reg = <0x300 0x40>;
+ interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPM_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpion: gpio@40040340 {
+ reg = <0x340 0x40>;
+ interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk GPN_GATE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ pcfg_default: pin-default {
+ slew-rate = <0>;
+ input-schmitt-disable;
+ bias-disable;
+ power-source = <1>;
+ drive-strength = <17100>;
+ };
+ };
+
uart0: serial@40700000 {
compatible = "nuvoton,ma35d1-uart";
reg = <0x0 0x40700000 0x0 0x100>;
--
2.34.1
On 28/11/2023 07:11, Jacky Huang wrote:
> From: Jacky Huang <[email protected]>
>
> Add 'pinctrl' node and 'gpioa' ~ 'gpion' nodes to the dtsi of ma35d1
> SoC and describe default pin configurations.
>
> Enable all UART nodes presented on som and iot boards, and add pinctrl
> function settings to these nodes.
>
> Signed-off-by: Jacky Huang <[email protected]>
> +
> + gpion: gpio@40040340 {
> + reg = <0x340 0x40>;
> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk GPN_GATE>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> +
> + pcfg_default: pin-default {
> + slew-rate = <0>;
> + input-schmitt-disable;
> + bias-disable;
> + power-source = <1>;
> + drive-strength = <17100>;
> + };
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (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
Dear Krzysztof,
Thanks for your review.
On 2023/11/28 下午 03:35, Krzysztof Kozlowski wrote:
> On 28/11/2023 07:11, Jacky Huang wrote:
>> From: Jacky Huang <[email protected]>
>>
>> Add 'pinctrl' node and 'gpioa' ~ 'gpion' nodes to the dtsi of ma35d1
>> SoC and describe default pin configurations.
>>
>> Enable all UART nodes presented on som and iot boards, and add pinctrl
>> function settings to these nodes.
>>
>> Signed-off-by: Jacky Huang <[email protected]>
>
>> +
>> + gpion: gpio@40040340 {
>> + reg = <0x340 0x40>;
>> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clk GPN_GATE>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + pcfg_default: pin-default {
>> + slew-rate = <0>;
>> + input-schmitt-disable;
>> + bias-disable;
>> + power-source = <1>;
>> + drive-strength = <17100>;
>> + };
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (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
>
I forgot to remove 'ma35d1-pinfunc.h' from my local copy.
After remove the '#include <dt-bindings/pinctrl/ma35d1-pinfunc.h>', it
can pass
the `make dtbs_check W=1` check.
I will fix it in the next version.
Best Regards,
Jacky Huang
On 28/11/2023 09:37, Jacky Huang wrote:
>>> + gpion: gpio@40040340 {
>>> + reg = <0x340 0x40>;
>>> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&clk GPN_GATE>;
>>> + gpio-controller;
>>> + #gpio-cells = <2>;
>>> + interrupt-controller;
>>> + #interrupt-cells = <2>;
>>> + };
>>> +
>>> + pcfg_default: pin-default {
>>> + slew-rate = <0>;
>>> + input-schmitt-disable;
>>> + bias-disable;
>>> + power-source = <1>;
>>> + drive-strength = <17100>;
>>> + };
>> It does not look like you tested the DTS against bindings. Please run
>> `make dtbs_check W=1` (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
>>
>
> I forgot to remove 'ma35d1-pinfunc.h' from my local copy.
> After remove the '#include <dt-bindings/pinctrl/ma35d1-pinfunc.h>', it
> can pass
> the `make dtbs_check W=1` check.
> I will fix it in the next version.
Really? Then your bindings look wrong. Why do you mix MMIO nodes and
non-MMIO in one device node?
Best regards,
Krzysztof
Dear Krzysztof,
Thanks for your review.
On 2023/11/28 下午 05:34, Krzysztof Kozlowski wrote:
> On 28/11/2023 09:37, Jacky Huang wrote:
>>>> + gpion: gpio@40040340 {
>>>> + reg = <0x340 0x40>;
>>>> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
>>>> + clocks = <&clk GPN_GATE>;
>>>> + gpio-controller;
>>>> + #gpio-cells = <2>;
>>>> + interrupt-controller;
>>>> + #interrupt-cells = <2>;
>>>> + };
>>>> +
>>>> + pcfg_default: pin-default {
>>>> + slew-rate = <0>;
>>>> + input-schmitt-disable;
>>>> + bias-disable;
>>>> + power-source = <1>;
>>>> + drive-strength = <17100>;
>>>> + };
>>> It does not look like you tested the DTS against bindings. Please run
>>> `make dtbs_check W=1` (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
>>>
>> I forgot to remove 'ma35d1-pinfunc.h' from my local copy.
>> After remove the '#include <dt-bindings/pinctrl/ma35d1-pinfunc.h>', it
>> can pass
>> the `make dtbs_check W=1` check.
>> I will fix it in the next version.
> Really? Then your bindings look wrong. Why do you mix MMIO nodes and
> non-MMIO in one device node?
>
> Best regards,
> Krzysztof
>
Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
issues.
Anyway, I will fix it in the next version.
Best Regards,
Jacky Huang
On 28/11/2023 07:11, Jacky Huang wrote:
> From: Jacky Huang <[email protected]>
>
...
>
> sys: system-management@40460000 {
> - compatible = "nuvoton,ma35d1-reset";
> + compatible = "nuvoton,ma35d1-reset", "syscon";
> reg = <0x0 0x40460000 0x0 0x200>;
> #reset-cells = <1>;
> };
> @@ -95,6 +96,162 @@ clk: clock-controller@40460200 {
> clocks = <&clk_hxt>;
> };
>
> + pinctrl: pinctrl@40040000 {
> + compatible = "nuvoton,ma35d1-pinctrl";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + nuvoton,sys = <&sys>;
> + ranges = <0x0 0x0 0x40040000 0xc00>;
> +
> + gpioa: gpio@40040000 {
> + reg = <0x0 0x40>;
Your unit address does not match reg.
You must test your DTS with `dtbs_check W=1`.
> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk GPA_GATE>;
> + gpio-controller;
> + #gpio-cells = <2>;
Best regards,
Krzysztof
On 28/11/2023 07:11, Jacky Huang wrote:
> + gpioi: gpio@40040200 {
> + reg = <0x200 0x40>;
> + interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
One space after GIC_SPI, not two.
Best regards,
Krzysztof
Dear Krzysztof,
On 2023/11/28 下午 07:03, Krzysztof Kozlowski wrote:
> On 28/11/2023 07:11, Jacky Huang wrote:
>> From: Jacky Huang <[email protected]>
>>
> ...
>
>>
>> sys: system-management@40460000 {
>> - compatible = "nuvoton,ma35d1-reset";
>> + compatible = "nuvoton,ma35d1-reset", "syscon";
>> reg = <0x0 0x40460000 0x0 0x200>;
>> #reset-cells = <1>;
>> };
>> @@ -95,6 +96,162 @@ clk: clock-controller@40460200 {
>> clocks = <&clk_hxt>;
>> };
>>
>> + pinctrl: pinctrl@40040000 {
>> + compatible = "nuvoton,ma35d1-pinctrl";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + nuvoton,sys = <&sys>;
>> + ranges = <0x0 0x0 0x40040000 0xc00>;
>> +
>> + gpioa: gpio@40040000 {
>> + reg = <0x0 0x40>;
> Your unit address does not match reg.
>
> You must test your DTS with `dtbs_check W=1`.
>
>
>> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clk GPA_GATE>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
>
> Best regards,
> Krzysztof
>
Sure, I will fix it. thank you.
Best Regards,
Jacky Huang
On 28/11/2023 11:45, Jacky Huang wrote:
> Dear Krzysztof,
>
> Thanks for your review.
>
> On 2023/11/28 下午 05:34, Krzysztof Kozlowski wrote:
>> On 28/11/2023 09:37, Jacky Huang wrote:
>>>>> + gpion: gpio@40040340 {
>>>>> + reg = <0x340 0x40>;
>>>>> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + clocks = <&clk GPN_GATE>;
>>>>> + gpio-controller;
>>>>> + #gpio-cells = <2>;
>>>>> + interrupt-controller;
>>>>> + #interrupt-cells = <2>;
>>>>> + };
>>>>> +
>>>>> + pcfg_default: pin-default {
>>>>> + slew-rate = <0>;
>>>>> + input-schmitt-disable;
>>>>> + bias-disable;
>>>>> + power-source = <1>;
>>>>> + drive-strength = <17100>;
>>>>> + };
>>>> It does not look like you tested the DTS against bindings. Please run
>>>> `make dtbs_check W=1` (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
>>>>
>>> I forgot to remove 'ma35d1-pinfunc.h' from my local copy.
>>> After remove the '#include <dt-bindings/pinctrl/ma35d1-pinfunc.h>', it
>>> can pass
>>> the `make dtbs_check W=1` check.
>>> I will fix it in the next version.
>> Really? Then your bindings look wrong. Why do you mix MMIO nodes and
>> non-MMIO in one device node?
>>
>> Best regards,
>> Krzysztof
>>
>
> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
> issues.
> Anyway, I will fix it in the next version.
Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
the binding issue.
The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
I don't understand why do you need them yet. I don't see any populate of
children. There are no compatibles, either.
Which part of your driver uses them exactly?
Best regards,
Krzysztof
Hi Jacky,
kernel test robot noticed the following build errors:
[auto build test ERROR on linusw-pinctrl/devel]
[also build test ERROR on linusw-pinctrl/for-next robh/for-next linus/master v6.7-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jacky-Huang/dt-bindings-reset-Add-syscon-to-nuvoton-ma35d1-system-management-node/20231128-141443
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link: https://lore.kernel.org/r/20231128061118.575847-4-ychuang570808%40gmail.com
patch subject: [PATCH v2 3/4] arm64: dts: nuvoton: Add pinctrl support for ma35d1
config: arm64-randconfig-001-20231128 (https://download.01.org/0day-ci/archive/20231129/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
In file included from arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts:9:
>> arch/arm64/boot/dts/nuvoton/ma35d1.dtsi:13:10: fatal error: 'dt-bindings/pinctrl/ma35d1-pinfunc.h' file not found
13 | #include <dt-bindings/pinctrl/ma35d1-pinfunc.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
vim +13 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> 13 #include <dt-bindings/pinctrl/ma35d1-pinfunc.h>
14
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Jacky,
kernel test robot noticed the following build errors:
[auto build test ERROR on linusw-pinctrl/devel]
[also build test ERROR on linusw-pinctrl/for-next robh/for-next linus/master v6.7-rc3 next-20231128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jacky-Huang/dt-bindings-reset-Add-syscon-to-nuvoton-ma35d1-system-management-node/20231128-141443
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link: https://lore.kernel.org/r/20231128061118.575847-4-ychuang570808%40gmail.com
patch subject: [PATCH v2 3/4] arm64: dts: nuvoton: Add pinctrl support for ma35d1
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20231129/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
In file included from arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts:9:
>> arch/arm64/boot/dts/nuvoton/ma35d1.dtsi:13:10: fatal error: dt-bindings/pinctrl/ma35d1-pinfunc.h: No such file or directory
13 | #include <dt-bindings/pinctrl/ma35d1-pinfunc.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +13 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> 13 #include <dt-bindings/pinctrl/ma35d1-pinfunc.h>
14
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Dear Krzysztof,
Thanks for your review.
On 2023/11/28 下午 07:03, Krzysztof Kozlowski wrote:
> On 28/11/2023 07:11, Jacky Huang wrote:
>> From: Jacky Huang <[email protected]>
>>
> ...
>
>>
>> sys: system-management@40460000 {
>> - compatible = "nuvoton,ma35d1-reset";
>> + compatible = "nuvoton,ma35d1-reset", "syscon";
>> reg = <0x0 0x40460000 0x0 0x200>;
>> #reset-cells = <1>;
>> };
>> @@ -95,6 +96,162 @@ clk: clock-controller@40460200 {
>> clocks = <&clk_hxt>;
>> };
>>
>> + pinctrl: pinctrl@40040000 {
>> + compatible = "nuvoton,ma35d1-pinctrl";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + nuvoton,sys = <&sys>;
>> + ranges = <0x0 0x0 0x40040000 0xc00>;
>> +
>> + gpioa: gpio@40040000 {
>> + reg = <0x0 0x40>;
> Your unit address does not match reg.
>
> You must test your DTS with `dtbs_check W=1`.
>
>
>> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clk GPA_GATE>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
>
> Best regards,
> Krzysztof
>
"OK, I will fix 'gpioa: gpio@40040000' to 'gpioa: gpio@0', and similarly
for gpiob to gpion. I will also eliminate all redundant spaces behind
GIC_SPI." Best Regards, Jacky Huang
Dear Krzysztof,
Thanks for your review.
On 2023/11/28 下午 07:06, Krzysztof Kozlowski wrote:
> On 28/11/2023 11:45, Jacky Huang wrote:
>> Dear Krzysztof,
>>
>> Thanks for your review.
>>
>> On 2023/11/28 下午 05:34, Krzysztof Kozlowski wrote:
>>> On 28/11/2023 09:37, Jacky Huang wrote:
>>>>>> + gpion: gpio@40040340 {
>>>>>> + reg = <0x340 0x40>;
>>>>>> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
>>>>>> + clocks = <&clk GPN_GATE>;
>>>>>> + gpio-controller;
>>>>>> + #gpio-cells = <2>;
>>>>>> + interrupt-controller;
>>>>>> + #interrupt-cells = <2>;
>>>>>> + };
>>>>>> +
>>>>>> + pcfg_default: pin-default {
>>>>>> + slew-rate = <0>;
>>>>>> + input-schmitt-disable;
>>>>>> + bias-disable;
>>>>>> + power-source = <1>;
>>>>>> + drive-strength = <17100>;
>>>>>> + };
>>>>> It does not look like you tested the DTS against bindings. Please run
>>>>> `make dtbs_check W=1` (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
>>>>>
>>>> I forgot to remove 'ma35d1-pinfunc.h' from my local copy.
>>>> After remove the '#include <dt-bindings/pinctrl/ma35d1-pinfunc.h>', it
>>>> can pass
>>>> the `make dtbs_check W=1` check.
>>>> I will fix it in the next version.
>>> Really? Then your bindings look wrong. Why do you mix MMIO nodes and
>>> non-MMIO in one device node?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>> issues.
>> Anyway, I will fix it in the next version.
> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
> the binding issue.
>
> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>
> I don't understand why do you need them yet. I don't see any populate of
> children. There are no compatibles, either.
>
> Which part of your driver uses them exactly?
>
> Best regards,
> Krzysztof
>
I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
&pinctrl {
pcfg_default: pin-default {
slew-rate = <0>;
input-schmitt-disable;
bias-disable;
power-source = <1>;
drive-strength = <17100>;
};
uart-grp {
pinctrl_uart0: uart0-pins {
nuvoton,pins = <4 14 1 &pcfg_default>,
<4 15 1 &pcfg_default>;
};
pinctrl_uart11: uart11-pins {
nuvoton,pins = <11 0 2 &pcfg_default>,
<11 1 2 &pcfg_default>,
<11 2 2 &pcfg_default>,
<11 3 2 &pcfg_default>;
};
...
I use the 'pin-' and just intent to define a generic pin configuration,
such as the above 'pin-default'.
Best Regards,
Jacky Huang
On 29/11/2023 04:35, Jacky Huang wrote:
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>> issues.
>>> Anyway, I will fix it in the next version.
>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>> the binding issue.
>>
>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>
>> I don't understand why do you need them yet. I don't see any populate of
>> children. There are no compatibles, either.
>>
>> Which part of your driver uses them exactly?
>>
>> Best regards,
>> Krzysztof
>>
>
> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>
> &pinctrl {
> pcfg_default: pin-default {
> slew-rate = <0>;
> input-schmitt-disable;
> bias-disable;
> power-source = <1>;
> drive-strength = <17100>;
> };
This solves nothing. It's the same placement.
Best regards,
Krzysztof
Dear Krzysztof,
On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>> issues.
>>>> Anyway, I will fix it in the next version.
>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>> the binding issue.
>>>
>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>
>>> I don't understand why do you need them yet. I don't see any populate of
>>> children. There are no compatibles, either.
>>>
>>> Which part of your driver uses them exactly?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>
>> &pinctrl {
>> pcfg_default: pin-default {
>> slew-rate = <0>;
>> input-schmitt-disable;
>> bias-disable;
>> power-source = <1>;
>> drive-strength = <17100>;
>> };
> This solves nothing. It's the same placement.
>
>
> Best regards,
> Krzysztof
>
OK, it stil be the binding issues.
For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
rockchip,pinctrl.yaml.
My intention is to describe a generic pin configuration, aiming to make
the pin
description more concise. In actual testing, it proves to be effective.
Best Regards,
Jacky Huang
On 29/11/2023 10:41, Jacky Huang wrote:
>
> Dear Krzysztof,
>
>
> On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
>> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>>>
>>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>>> issues.
>>>>> Anyway, I will fix it in the next version.
>>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>>> the binding issue.
>>>>
>>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>>
>>>> I don't understand why do you need them yet. I don't see any populate of
>>>> children. There are no compatibles, either.
>>>>
>>>> Which part of your driver uses them exactly?
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>>
>>> &pinctrl {
>>> pcfg_default: pin-default {
>>> slew-rate = <0>;
>>> input-schmitt-disable;
>>> bias-disable;
>>> power-source = <1>;
>>> drive-strength = <17100>;
>>> };
>> This solves nothing. It's the same placement.
>>
>>
>> Best regards,
>> Krzysztof
>>
>
> OK, it stil be the binding issues.
> For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
> rockchip,pinctrl.yaml.
>
> My intention is to describe a generic pin configuration, aiming to make
> the pin
> description more concise. In actual testing, it proves to be effective.
Can you instead respond to my actual questions?
Best regards,
Krzysztof
Dear Krzysztof,
On 2023/11/29 下午 06:02, Krzysztof Kozlowski wrote:
> On 29/11/2023 10:41, Jacky Huang wrote:
>> Dear Krzysztof,
>>
>>
>> On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
>>> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>>>
>>>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>>>> issues.
>>>>>> Anyway, I will fix it in the next version.
>>>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>>>> the binding issue.
>>>>>
>>>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>>>
>>>>> I don't understand why do you need them yet. I don't see any populate of
>>>>> children. There are no compatibles, either.
>>>>>
>>>>> Which part of your driver uses them exactly?
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>>>
>>>> &pinctrl {
>>>> pcfg_default: pin-default {
>>>> slew-rate = <0>;
>>>> input-schmitt-disable;
>>>> bias-disable;
>>>> power-source = <1>;
>>>> drive-strength = <17100>;
>>>> };
>>> This solves nothing. It's the same placement.
>>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> OK, it stil be the binding issues.
>> For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
>> rockchip,pinctrl.yaml.
>>
>> My intention is to describe a generic pin configuration, aiming to make
>> the pin
>> description more concise. In actual testing, it proves to be effective.
> Can you instead respond to my actual questions?
>
> Best regards,
> Krzysztof
>
The the last one item of nuvoton,pins is a phandle, which can refer to
'&pin-default'. The following code of driver pinctrl-ma35.c parse
"nuvoton,pins", including the node reference by phandle. list =
of_get_property(np, "nuvoton,pins", &size); size /= sizeof(*list); if
(!size || size % 4) { dev_err(npctl->dev, "wrong setting!\n"); return
-EINVAL; } grp->npins = size / 4; grp->pins = devm_kzalloc(npctl->dev,
grp->npins * sizeof(*grp->pins), GFP_KERNEL); if (!grp->pins) return
-ENOMEM; pin = grp->settings = devm_kzalloc(npctl->dev, grp->npins *
sizeof(*grp->settings), GFP_KERNEL); if (!grp->settings) return -ENOMEM;
for (i = 0, j = 0; i < size; i += 4, j++) { struct device_node
*np_config; const __be32 *phandle; pin->offset = be32_to_cpu(*list++) *
MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE; pin->shift =
(be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32; pin->muxval =
be32_to_cpu(*list++); phandle = list++; if (!phandle) return -EINVAL;
np_config = of_find_node_by_phandle(be32_to_cpup(phandle)); ret =
pinconf_generic_parse_dt_config(np_config, NULL, &pin->configs,
&pin->nconfigs); if (ret) return ret; grp->pins[j] =
npctl->info->get_pin_num(pin->offset, pin->shift); pin++; } Best
Regards, Jacky Huang
On 29/11/2023 11:14, Jacky Huang wrote:
>
> Dear Krzysztof,
>
>
> On 2023/11/29 下午 06:02, Krzysztof Kozlowski wrote:
>> On 29/11/2023 10:41, Jacky Huang wrote:
>>> Dear Krzysztof,
>>>
>>>
>>> On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
>>>> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>>>>> Best regards,
>>>>>>>> Krzysztof
>>>>>>>>
>>>>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>>>>> issues.
>>>>>>> Anyway, I will fix it in the next version.
>>>>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>>>>> the binding issue.
>>>>>>
>>>>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>>>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>>>>
>>>>>> I don't understand why do you need them yet. I don't see any populate of
>>>>>> children. There are no compatibles, either.
>>>>>>
>>>>>> Which part of your driver uses them exactly?
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>>>
>>>>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>>>>
>>>>> &pinctrl {
>>>>> pcfg_default: pin-default {
>>>>> slew-rate = <0>;
>>>>> input-schmitt-disable;
>>>>> bias-disable;
>>>>> power-source = <1>;
>>>>> drive-strength = <17100>;
>>>>> };
>>>> This solves nothing. It's the same placement.
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> OK, it stil be the binding issues.
>>> For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
>>> rockchip,pinctrl.yaml.
>>>
>>> My intention is to describe a generic pin configuration, aiming to make
>>> the pin
>>> description more concise. In actual testing, it proves to be effective.
>> Can you instead respond to my actual questions?
>>
>> Best regards,
>> Krzysztof
>>
>
> The the last one item of nuvoton,pins is a phandle, which can refer to
> '&pin-default'. The following code of driver pinctrl-ma35.c parse
> "nuvoton,pins", including the node reference by phandle. list =
> of_get_property(np, "nuvoton,pins", &size); size /= sizeof(*list); if
> (!size || size % 4) { dev_err(npctl->dev, "wrong setting!\n"); return
> -EINVAL; } grp->npins = size / 4; grp->pins = devm_kzalloc(npctl->dev,
> grp->npins * sizeof(*grp->pins), GFP_KERNEL); if (!grp->pins) return
> -ENOMEM; pin = grp->settings = devm_kzalloc(npctl->dev, grp->npins *
> sizeof(*grp->settings), GFP_KERNEL); if (!grp->settings) return -ENOMEM;
> for (i = 0, j = 0; i < size; i += 4, j++) { struct device_node
> *np_config; const __be32 *phandle; pin->offset = be32_to_cpu(*list++) *
> MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE; pin->shift =
> (be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32; pin->muxval =
> be32_to_cpu(*list++); phandle = list++; if (!phandle) return -EINVAL;
> np_config = of_find_node_by_phandle(be32_to_cpup(phandle)); ret =
> pinconf_generic_parse_dt_config(np_config, NULL, &pin->configs,
> &pin->nconfigs); if (ret) return ret; grp->pins[j] =
> npctl->info->get_pin_num(pin->offset, pin->shift); pin++; } Best
> Regards, Jacky Huang
Sorry, I cannot parse it.
I was referring to the children with unit addresses. I don't see any
populate of the children, so why do you need them?
There are no compatibles, either.
Which part of your driver uses them exactly?
Best regards,
Krzysztof
Dear Krzysztof,
On 2023/11/29 下午 06:54, Krzysztof Kozlowski wrote:
> On 29/11/2023 11:14, Jacky Huang wrote:
>> Dear Krzysztof,
>>
>>
>> On 2023/11/29 下午 06:02, Krzysztof Kozlowski wrote:
>>> On 29/11/2023 10:41, Jacky Huang wrote:
>>>> Dear Krzysztof,
>>>>
>>>>
>>>> On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
>>>>> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>>>>>> Best regards,
>>>>>>>>> Krzysztof
>>>>>>>>>
>>>>>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>>>>>> issues.
>>>>>>>> Anyway, I will fix it in the next version.
>>>>>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>>>>>> the binding issue.
>>>>>>>
>>>>>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>>>>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>>>>>
>>>>>>> I don't understand why do you need them yet. I don't see any populate of
>>>>>>> children. There are no compatibles, either.
>>>>>>>
>>>>>>> Which part of your driver uses them exactly?
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>>>
>>>>>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>>>>>
>>>>>> &pinctrl {
>>>>>> pcfg_default: pin-default {
>>>>>> slew-rate = <0>;
>>>>>> input-schmitt-disable;
>>>>>> bias-disable;
>>>>>> power-source = <1>;
>>>>>> drive-strength = <17100>;
>>>>>> };
>>>>> This solves nothing. It's the same placement.
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> OK, it stil be the binding issues.
>>>> For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
>>>> rockchip,pinctrl.yaml.
>>>>
>>>> My intention is to describe a generic pin configuration, aiming to make
>>>> the pin
>>>> description more concise. In actual testing, it proves to be effective.
>>> Can you instead respond to my actual questions?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> The the last one item of nuvoton,pins is a phandle, which can refer to
>> '&pin-default'. The following code of driver pinctrl-ma35.c parse
>> "nuvoton,pins", including the node reference by phandle. list =
>> of_get_property(np, "nuvoton,pins", &size); size /= sizeof(*list); if
>> (!size || size % 4) { dev_err(npctl->dev, "wrong setting!\n"); return
>> -EINVAL; } grp->npins = size / 4; grp->pins = devm_kzalloc(npctl->dev,
>> grp->npins * sizeof(*grp->pins), GFP_KERNEL); if (!grp->pins) return
>> -ENOMEM; pin = grp->settings = devm_kzalloc(npctl->dev, grp->npins *
>> sizeof(*grp->settings), GFP_KERNEL); if (!grp->settings) return -ENOMEM;
>> for (i = 0, j = 0; i < size; i += 4, j++) { struct device_node
>> *np_config; const __be32 *phandle; pin->offset = be32_to_cpu(*list++) *
>> MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE; pin->shift =
>> (be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32; pin->muxval =
>> be32_to_cpu(*list++); phandle = list++; if (!phandle) return -EINVAL;
>> np_config = of_find_node_by_phandle(be32_to_cpup(phandle)); ret =
>> pinconf_generic_parse_dt_config(np_config, NULL, &pin->configs,
>> &pin->nconfigs); if (ret) return ret; grp->pins[j] =
>> npctl->info->get_pin_num(pin->offset, pin->shift); pin++; } Best
>> Regards, Jacky Huang
> Sorry, I cannot parse it.
>
> I was referring to the children with unit addresses. I don't see any
> populate of the children, so why do you need them?
>
> There are no compatibles, either.
>
> Which part of your driver uses them exactly?
>
> Best regards,
> Krzysztof
>
So, I should update the binding from "^pin-[a-z0-9]+$" to something like
"-pincfg$".
Just remove the unit address part, and it will become:
default-pincfg {
slew-rate = <0>;
input-schmitt-disable;
bias-disable;
power-source = <1>;
drive-strength = <17100>;
};
Best Regards,
Jacky Huang
On 30/11/2023 02:10, Jacky Huang wrote:
> Dear Krzysztof,
>
>
> On 2023/11/29 下午 06:54, Krzysztof Kozlowski wrote:
>> On 29/11/2023 11:14, Jacky Huang wrote:
>>> Dear Krzysztof,
>>>
>>>
>>> On 2023/11/29 下午 06:02, Krzysztof Kozlowski wrote:
>>>> On 29/11/2023 10:41, Jacky Huang wrote:
>>>>> Dear Krzysztof,
>>>>>
>>>>>
>>>>> On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
>>>>>> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>>>>>>> Best regards,
>>>>>>>>>> Krzysztof
>>>>>>>>>>
>>>>>>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>>>>>>> issues.
>>>>>>>>> Anyway, I will fix it in the next version.
>>>>>>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>>>>>>> the binding issue.
>>>>>>>>
>>>>>>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>>>>>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>>>>>>
>>>>>>>> I don't understand why do you need them yet. I don't see any populate of
>>>>>>>> children. There are no compatibles, either.
>>>>>>>>
>>>>>>>> Which part of your driver uses them exactly?
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Krzysztof
>>>>>>>>
>>>>>>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>>>>>>
>>>>>>> &pinctrl {
>>>>>>> pcfg_default: pin-default {
>>>>>>> slew-rate = <0>;
>>>>>>> input-schmitt-disable;
>>>>>>> bias-disable;
>>>>>>> power-source = <1>;
>>>>>>> drive-strength = <17100>;
>>>>>>> };
>>>>>> This solves nothing. It's the same placement.
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>>>
>>>>> OK, it stil be the binding issues.
>>>>> For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
>>>>> rockchip,pinctrl.yaml.
>>>>>
>>>>> My intention is to describe a generic pin configuration, aiming to make
>>>>> the pin
>>>>> description more concise. In actual testing, it proves to be effective.
>>>> Can you instead respond to my actual questions?
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> The the last one item of nuvoton,pins is a phandle, which can refer to
>>> '&pin-default'. The following code of driver pinctrl-ma35.c parse
>>> "nuvoton,pins", including the node reference by phandle. list =
>>> of_get_property(np, "nuvoton,pins", &size); size /= sizeof(*list); if
>>> (!size || size % 4) { dev_err(npctl->dev, "wrong setting!\n"); return
>>> -EINVAL; } grp->npins = size / 4; grp->pins = devm_kzalloc(npctl->dev,
>>> grp->npins * sizeof(*grp->pins), GFP_KERNEL); if (!grp->pins) return
>>> -ENOMEM; pin = grp->settings = devm_kzalloc(npctl->dev, grp->npins *
>>> sizeof(*grp->settings), GFP_KERNEL); if (!grp->settings) return -ENOMEM;
>>> for (i = 0, j = 0; i < size; i += 4, j++) { struct device_node
>>> *np_config; const __be32 *phandle; pin->offset = be32_to_cpu(*list++) *
>>> MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE; pin->shift =
>>> (be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32; pin->muxval =
>>> be32_to_cpu(*list++); phandle = list++; if (!phandle) return -EINVAL;
>>> np_config = of_find_node_by_phandle(be32_to_cpup(phandle)); ret =
>>> pinconf_generic_parse_dt_config(np_config, NULL, &pin->configs,
>>> &pin->nconfigs); if (ret) return ret; grp->pins[j] =
>>> npctl->info->get_pin_num(pin->offset, pin->shift); pin++; } Best
>>> Regards, Jacky Huang
>> Sorry, I cannot parse it.
>>
>> I was referring to the children with unit addresses. I don't see any
>> populate of the children, so why do you need them?
>>
>> There are no compatibles, either.
>>
>> Which part of your driver uses them exactly?
>>
>> Best regards,
>> Krzysztof
>>
> So, I should update the binding from "^pin-[a-z0-9]+$" to something like
> "-pincfg$".
> Just remove the unit address part, and it will become:
>
> default-pincfg {
> slew-rate = <0>;
> input-schmitt-disable;
> bias-disable;
> power-source = <1>;
> drive-strength = <17100>;
> };
>
No, it solves nothing. Instead of pasting more code, can you answer my
questions?
Best regards,
Krzysztof
On 2023/11/30 下午 04:04, Krzysztof Kozlowski wrote:
> On 30/11/2023 02:10, Jacky Huang wrote:
>> Dear Krzysztof,
>>
>>
>> On 2023/11/29 下午 06:54, Krzysztof Kozlowski wrote:
>>> On 29/11/2023 11:14, Jacky Huang wrote:
>>>> Dear Krzysztof,
>>>>
>>>>
>>>> On 2023/11/29 下午 06:02, Krzysztof Kozlowski wrote:
>>>>> On 29/11/2023 10:41, Jacky Huang wrote:
>>>>>> Dear Krzysztof,
>>>>>>
>>>>>>
>>>>>> On 2023/11/29 下午 04:11, Krzysztof Kozlowski wrote:
>>>>>>> On 29/11/2023 04:35, Jacky Huang wrote:
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Krzysztof
>>>>>>>>>>>
>>>>>>>>>> Yes, it did pass the 'dtbs_check'. I guess the tool does not detect such
>>>>>>>>>> issues.
>>>>>>>>>> Anyway, I will fix it in the next version.
>>>>>>>>> Hm, I see your bindings indeed allow pin-.* and unit addresses, so it is
>>>>>>>>> the binding issue.
>>>>>>>>>
>>>>>>>>> The examples you used as reference - xlnx,zynqmp-pinctrl.yaml and
>>>>>>>>> realtek,rtd1315e-pinctrl.yaml - do not mix these as you do.
>>>>>>>>>
>>>>>>>>> I don't understand why do you need them yet. I don't see any populate of
>>>>>>>>> children. There are no compatibles, either.
>>>>>>>>>
>>>>>>>>> Which part of your driver uses them exactly?
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Krzysztof
>>>>>>>>>
>>>>>>>> I will move the 'pcfg_default: pin-default' from dtsi to dts, like this:
>>>>>>>>
>>>>>>>> &pinctrl {
>>>>>>>> pcfg_default: pin-default {
>>>>>>>> slew-rate = <0>;
>>>>>>>> input-schmitt-disable;
>>>>>>>> bias-disable;
>>>>>>>> power-source = <1>;
>>>>>>>> drive-strength = <17100>;
>>>>>>>> };
>>>>>>> This solves nothing. It's the same placement.
>>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>>>
>>>>>> OK, it stil be the binding issues.
>>>>>> For "^pin-[a-z0-9]+$", I reference to the "pcfg-[a-z0-9-]+$" of
>>>>>> rockchip,pinctrl.yaml.
>>>>>>
>>>>>> My intention is to describe a generic pin configuration, aiming to make
>>>>>> the pin
>>>>>> description more concise. In actual testing, it proves to be effective.
>>>>> Can you instead respond to my actual questions?
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> The the last one item of nuvoton,pins is a phandle, which can refer to
>>>> '&pin-default'. The following code of driver pinctrl-ma35.c parse
>>>> "nuvoton,pins", including the node reference by phandle. list =
>>>> of_get_property(np, "nuvoton,pins", &size); size /= sizeof(*list); if
>>>> (!size || size % 4) { dev_err(npctl->dev, "wrong setting!\n"); return
>>>> -EINVAL; } grp->npins = size / 4; grp->pins = devm_kzalloc(npctl->dev,
>>>> grp->npins * sizeof(*grp->pins), GFP_KERNEL); if (!grp->pins) return
>>>> -ENOMEM; pin = grp->settings = devm_kzalloc(npctl->dev, grp->npins *
>>>> sizeof(*grp->settings), GFP_KERNEL); if (!grp->settings) return -ENOMEM;
>>>> for (i = 0, j = 0; i < size; i += 4, j++) { struct device_node
>>>> *np_config; const __be32 *phandle; pin->offset = be32_to_cpu(*list++) *
>>>> MA35_MFP_REG_SZ_PER_BANK + MA35_MFP_REG_BASE; pin->shift =
>>>> (be32_to_cpu(*list++) * MA35_MFP_BITS_PER_PORT) % 32; pin->muxval =
>>>> be32_to_cpu(*list++); phandle = list++; if (!phandle) return -EINVAL;
>>>> np_config = of_find_node_by_phandle(be32_to_cpup(phandle)); ret =
>>>> pinconf_generic_parse_dt_config(np_config, NULL, &pin->configs,
>>>> &pin->nconfigs); if (ret) return ret; grp->pins[j] =
>>>> npctl->info->get_pin_num(pin->offset, pin->shift); pin++; } Best
>>>> Regards, Jacky Huang
>>> Sorry, I cannot parse it.
>>>
>>> I was referring to the children with unit addresses. I don't see any
>>> populate of the children, so why do you need them?
>>>
>>> There are no compatibles, either.
>>>
>>> Which part of your driver uses them exactly?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> So, I should update the binding from "^pin-[a-z0-9]+$" to something like
>> "-pincfg$".
>> Just remove the unit address part, and it will become:
>>
>> default-pincfg {
>> slew-rate = <0>;
>> input-schmitt-disable;
>> bias-disable;
>> power-source = <1>;
>> drive-strength = <17100>;
>> };
>>
> No, it solves nothing. Instead of pasting more code, can you answer my
> questions?
>
> Best regards,
> Krzysztof
>
Yes, this node is invalid.
I will drop this node and update the yaml.
Best Regards,
Jacky Huang