Re: [PATCH v1 1/2] dt-binding: pinctrl: Add NPCM8XX pinctrl and GPIO documentation

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 12/07/2022 20:44, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your clarifications.
> 
> On Tue, 12 Jul 2022 at 16:45, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 12/07/2022 15:29, Tomer Maimon wrote:
>>> Hi Krzysztof,
>>>
>>> Thanks for your comments.
>>>
>>> On Tue, 12 Jul 2022 at 12:48, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 10/07/2022 12:21, Tomer Maimon wrote:
>>>>> Added device tree binding documentation for Nuvoton Arbel BMC NPCM8XX
>>>>> pinmux and GPIO controller.
>>>>>
>>>>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>>>>> ---
>>>>>  .../pinctrl/nuvoton,npcm845-pinctrl.yaml      | 205 ++++++++++++++++++
>>>>>  1 file changed, 205 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..6395ef2bf5b3
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
>>>>> @@ -0,0 +1,205 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,npcm845-pinctrl.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Nuvoton NPCM845 Pin Controller and GPIO
>>>>> +
>>>>> +maintainers:
>>>>> +  - Tomer Maimon <tmaimon77@gmail.com>
>>>>> +
>>>>> +description:
>>>>> +  The Nuvoton BMC NPCM8XX Pin Controller multi-function routed through
>>>>> +  the multiplexing block, Each pin supports GPIO functionality (GPIOx)
>>>>> +  and multiple functions that directly connect the pin to different
>>>>> +  hardware blocks.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: nuvoton,npcm845-pinctrl
>>>>> +
>>>>> +  ranges:
>>>>> +    maxItems: 1
>>>>
>>>> ranges without reg? Does it even work? Did you test the bindings?
>>> The ranges related to GPIO node reg
>>
>> But you do not allow here a 'reg', do you? So how can you have an unit
>> address in pinctrl node?
> I allow the reg unit address in the GPIO node.
> This is why reg is in the GPIO node as follow:
> 
>                 compatible = "nuvoton,npcm845-pinctrl";
>                 ranges = <0x0 0x0 0xf0010000 0x8000>;
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 status = "okay";
>                 gpio0: gpio@f0010000 {
>                         gpio-controller;
>                         #gpio-cells = <2>;
>                         reg = <0x0 0xB0>;
>                         interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
>                         gpio-ranges = <&pinctrl 0 0 32>;
>                 };
>                 gpio1: gpio@f0011000 {
>                         gpio-controller;
>                         #gpio-cells = <2>;
>                         reg = <0x1000 0xB0>;
>                         interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
>                         gpio-ranges = <&pinctrl 0 32 32>;
>                 };
>                 gpio2: gpio@f0012000 {
>                         gpio-controller;
>                         #gpio-cells = <2>;
>                         reg = <0x2000 0xB0>;
>                         interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
>                         gpio-ranges = <&pinctrl 0 64 32>;
>                 };
> ...
> Is it problematic?


It seems not, looks ok because of ranges, although it is a bit confusing
that your pinctrl unit address is 0xf0800000 but ranges is 0xf0010000.

>>
>>>
>>> I did test the pin controller document and it passed.
>>> bash-4.2$ make ARCH=arm64 dt_binding_check
>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
>>>   LINT    Documentation/devicetree/bindings
>>>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>>>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>>>   DTEX    Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dts
>>>   DTC     Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb
>>>   CHECK   Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.example.dtb
>>> Did I need to run anything else than dt_binding_check for testing the document?
>>
>> Indeed it will pass, because you do not have reg in pinctrl node. But
>> your dts won't pass make dtbs W=1
> After running make ARCH=arm64 dtbs W=1 I don't see warning related to pinctrl
> bash-4.2$ make ARCH=arm64 dtbs W=1
>   DTC     arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dtb
> arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:69.7-183.5:
> Warning (unit_address_vs_reg): /ahb/apb: node has a reg or ranges
> property, but no unit name
> arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts:20.9-22.4: Warning
> (unit_address_vs_reg): /memory: node has a reg or ranges property, but
> no unit name
> arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:69.7-183.5:
> Warning (simple_bus_reg): /ahb/apb: simple-bus unit address format
> error, expected "f0000000"
> arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi:56.35-61.5:
> Warning (unique_unit_address): /ahb/reset-controller@f0801000:
> duplicate unit-address (also used in node
> /ahb/clock-controller@f0801000)
> I did got warning but it dont related to the pinctrl, Maybe I didn't
> run the test correct?

Looks correct, indeed.

Best regards,
Krzysztof