[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5dec7907-9cab-6735-4775-142aeede185e@linaro.org>
Date: Tue, 12 Jul 2022 22:44:08 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Tomer Maimon <tmaimon77@...il.com>
Cc: Avi Fishman <avifishman70@...il.com>,
Tali Perry <tali.perry1@...il.com>,
Joel Stanley <joel@....id.au>,
Patrick Venture <venture@...gle.com>,
Nancy Yuen <yuenn@...gle.com>,
Benjamin Fair <benjaminfair@...gle.com>,
Linus Walleij <linus.walleij@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Neuschäfer <j.neuschaefer@....net>,
zhengbin13@...wei.com, OpenBMC Maillist <openbmc@...ts.ozlabs.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
devicetree <devicetree@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] dt-binding: pinctrl: Add NPCM8XX pinctrl and GPIO
documentation
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@...aro.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@...aro.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@...il.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@...il.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@...10000 {
> gpio-controller;
> #gpio-cells = <2>;
> reg = <0x0 0xB0>;
> interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
> gpio-ranges = <&pinctrl 0 0 32>;
> };
> gpio1: gpio@...11000 {
> gpio-controller;
> #gpio-cells = <2>;
> reg = <0x1000 0xB0>;
> interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> gpio-ranges = <&pinctrl 0 32 32>;
> };
> gpio2: gpio@...12000 {
> 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@...01000:
> duplicate unit-address (also used in node
> /ahb/clock-controller@...01000)
> 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
Powered by blists - more mailing lists