Messages in this thread | | | Date | Wed, 29 Nov 2023 11:35:28 +0800 | Subject | Re: [PATCH v2 3/4] arm64: dts: nuvoton: Add pinctrl support for ma35d1 | From | 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
| |