Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bfca0379-7346-13e7-a18f-66740c5871b3@linaro.org>
Date:   Mon, 19 Sep 2022 18:06:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Subject: Re: [PATCH v2 1/2] dt-binding: pinctrl: Add NPCM8XX pinctrl and GPIO
 documentation
Content-Language: en-US
To:     Tomer Maimon <tmaimon77@gmail.com>
Cc:     Rob Herring <robh@kernel.org>,
        Avi Fishman <avifishman70@gmail.com>,
        Tali Perry <tali.perry1@gmail.com>,
        Joel Stanley <joel@jms.id.au>,
        Patrick Venture <venture@google.com>,
        Nancy Yuen <yuenn@google.com>,
        Benjamin Fair <benjaminfair@google.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        =?UTF-8?Q?Jonathan_Neusch=c3=a4fer?= <j.neuschaefer@gmx.net>,
        zhengbin13@huawei.com, OpenBMC Maillist <openbmc@lists.ozlabs.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>
References: <20220714122322.63663-1-tmaimon77@gmail.com>
 <20220714122322.63663-2-tmaimon77@gmail.com>
 <20220718211046.GA3547663-robh@kernel.org>
 <CAP6Zq1hQ5m2kkQOKaYsKhPQhCW+vdsdyPRxxb_yRGMB=gJCPdw@mail.gmail.com>
 <3981e6e8-d4bb-b13d-7aaa-7aea83ffaad9@linaro.org>
 <CAP6Zq1gp1ph1wixgb6nL+2R8We2YJ2HQM2iC05itq_XWd2Cwig@mail.gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <CAP6Zq1gp1ph1wixgb6nL+2R8We2YJ2HQM2iC05itq_XWd2Cwig@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 19/09/2022 16:31, Tomer Maimon wrote:
>>>>> +examples:
>>>>> +  - |
>>>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> +    #include <dt-bindings/gpio/gpio.h>
>>>>> +
>>>>> +    soc {
>>>>> +      #address-cells = <2>;
>>>>> +      #size-cells = <2>;
>>>>> +
>>>>> +      pinctrl: pinctrl@f0800000 {
>>>>> +        compatible = "nuvoton,npcm845-pinctrl";
>>>>> +        ranges = <0x0 0x0 0xf0010000 0x8000>;
>>>>> +        #address-cells = <1>;
>>>>> +        #size-cells = <1>;
>>>>> +        nuvoton,sysgcr = <&gcr>;
>>>>> +
>>>>> +        gpio0: gpio@f0010000 {
>>>>
>>>> gpio@0
>>>>
>>>> Is this really a child block of the pinctrl? Doesn't really look like it
>>>> based on addressess. Where are the pinctrl registers? In the sysgcr? If
>>>> so, then pinctrl should be a child of it. But that doesn't really work
>>>> too well with gpio child nodes...
>>> the pin controller mux is handled by sysgcr this is why the sysgcr in
>>> the mother node,
>>> and the pin configuration are handled by the GPIO registers.  each
>>> GPIO bank (child) contains 32 GPIO.
>>> this is why the GPIO is the child node.
>>
>> Then maybe pinctrl should be the sysgcr and expose regmap for other devices?
> The pin controller using the sysgcr to handle the pinmux, this is why
> the sysgcr is in the mother node, is it problematic?

You said pin-controller mux registers are in sysgcr, so it should not be
used via syscon.

Please provide address map description to convince us that this is
correct HW representation.

Best regards,
Krzysztof