diff mbox series

[v2,3/4] arm64: dts: nuvoton: Add pinctrl support for ma35d1

Message ID 20231128061118.575847-4-ychuang570808@gmail.com
State New
Headers show
Series Add support for nuvoton ma35d1 pin control | expand

Commit Message

Jacky Huang Nov. 28, 2023, 6:11 a.m. UTC
From: Jacky Huang <ychuang3@nuvoton.com>

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 <ychuang3@nuvoton.com>
---
 .../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(-)

Comments

Krzysztof Kozlowski Nov. 28, 2023, 7:35 a.m. UTC | #1
On 28/11/2023 07:11, Jacky Huang wrote:
> From: Jacky Huang <ychuang3@nuvoton.com>
> 
> 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 <ychuang3@nuvoton.com>


> +
> +			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
Jacky Huang Nov. 28, 2023, 8:37 a.m. UTC | #2
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 <ychuang3@nuvoton.com>
>>
>> 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 <ychuang3@nuvoton.com>
>
>> +
>> +			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
Krzysztof Kozlowski Nov. 28, 2023, 9:34 a.m. UTC | #3
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
Jacky Huang Nov. 28, 2023, 10:45 a.m. UTC | #4
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
Krzysztof Kozlowski Nov. 28, 2023, 11:03 a.m. UTC | #5
On 28/11/2023 07:11, Jacky Huang wrote:
> From: Jacky Huang <ychuang3@nuvoton.com>
> 

...

>  
>  		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
Krzysztof Kozlowski Nov. 28, 2023, 11:04 a.m. UTC | #6
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
Jacky Huang Nov. 28, 2023, 11:05 a.m. UTC | #7
Dear Krzysztof,



On 2023/11/28 下午 07:03, Krzysztof Kozlowski wrote:
> On 28/11/2023 07:11, Jacky Huang wrote:
>> From: Jacky Huang <ychuang3@nuvoton.com>
>>
> ...
>
>>   
>>   		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
Krzysztof Kozlowski Nov. 28, 2023, 11:06 a.m. UTC | #8
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
kernel test robot Nov. 29, 2023, 1:11 a.m. UTC | #9
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/202311290550.7HCXMJCy-lkp@intel.com/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/202311290550.7HCXMJCy-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311290550.7HCXMJCy-lkp@intel.com/

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
kernel test robot Nov. 29, 2023, 1:11 a.m. UTC | #10
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/202311290626.fFZShqCp-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231129/202311290626.fFZShqCp-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311290626.fFZShqCp-lkp@intel.com/

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
Jacky Huang Nov. 29, 2023, 1:43 a.m. UTC | #11
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 <ychuang3@nuvoton.com>
>>
> ...
>
>>   
>>   		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
Jacky Huang Nov. 29, 2023, 3:35 a.m. UTC | #12
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
Krzysztof Kozlowski Nov. 29, 2023, 8:11 a.m. UTC | #13
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
Jacky Huang Nov. 29, 2023, 9:41 a.m. UTC | #14
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
Krzysztof Kozlowski Nov. 29, 2023, 10:02 a.m. UTC | #15
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
Jacky Huang Nov. 29, 2023, 10:14 a.m. UTC | #16
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
Krzysztof Kozlowski Nov. 29, 2023, 10:54 a.m. UTC | #17
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
Jacky Huang Nov. 30, 2023, 1:10 a.m. UTC | #18
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
Krzysztof Kozlowski Nov. 30, 2023, 8:04 a.m. UTC | #19
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
Jacky Huang Nov. 30, 2023, 8:23 a.m. UTC | #20
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
diff mbox series

Patch

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>;