Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: LwW1A8ByBD+yBQ/Mc7nwGC0gUUpS4Zyp18/VCWuBjvzcMjQ/rEekYWeunGRQeW5G0mAYzSZEqO
 /cjuIUTRDbGV75TNsQADzOHlMoauPmPg373Goh/msbCft2ZFui8BtaWTiair59yBATkZ9D3CI8
 LfWxNnNy0wiUWuCaLU/H/or2Jyq2G0YBCoGNhILyKx3hl7ugIYuiRPwKmM1ilZjskJ5JQ8hBYU
 fgYi0kGYXyr6/pN0fcvJzfUQwCj8Nlu4Ei4Dw5gp2HLmBFNhjfq+xGKbHlnlBWjHt0mol8W3IJ
 L78=
References: <20200513141134.25819-1-lars.povlsen@microchip.com> <20200513141134.25819-2-lars.povlsen@microchip.com> <CACRpkdZa7OM3bqB+zRprEQ3M4m9hG3uPCoYxrdH_O=oxD8zi8Q@mail.gmail.com> <87pnb1nf2j.fsf@soft-dev15.microsemi.net> <CACRpkdYesD9sRQZXQNEaBY2Ouu3bjKKGWpRtU-Lpa4AcjyPwXw@mail.gmail.com>
From:   Lars Povlsen <lars.povlsen@microchip.com>
To:     Linus Walleij <linus.walleij@linaro.org>
CC:     Lars Povlsen <lars.povlsen@microchip.com>,
        SoC Team <soc@kernel.org>, "Rob Herring" <robh+dt@kernel.org>,
        Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>
Subject: Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio
In-Reply-To: <CACRpkdYesD9sRQZXQNEaBY2Ouu3bjKKGWpRtU-Lpa4AcjyPwXw@mail.gmail.com>
Date:   Mon, 25 May 2020 16:38:52 +0200
Message-ID: <87r1v8oz9f.fsf@soft-dev15.microsemi.net>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Linus Walleij writes:

> On Mon, May 18, 2020 at 10:50 PM Lars Povlsen
> <lars.povlsen@microchip.com> wrote:
>> Linus Walleij writes:
>>
>> > On Wed, May 13, 2020 at 4:11 PM Lars Povlsen <lars.povlsen@microchip.com> wrote:
>> >
>> >> This adds DT bindings for the Microsemi SGPIO controller, bindings
>> >> mscc,ocelot-sgpio and mscc,luton-sgpio.
>> >>
>> >> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> >> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> >
>> >> +  microchip,sgpio-ports:
>> >> +    description: This is a 32-bit bitmask, configuring whether a
>> >> +      particular port in the controller is enabled or not. This allows
>> >> +      unused ports to be removed from the bitstream and reduce latency.
>> >> +    $ref: "/schemas/types.yaml#/definitions/uint32"
>> >
>> > I don't know about this.
>> >
>> > You are saying this pin controller can have up to 32 GPIO "ports"
>> > (also known as banks).
>> >
>> > Why can't you just represent each such port as a separate GPIO
>> > node:
>> >
>> > pinctrl@nnn {
>> >     gpio@0 {
>> >         ....
>> >     };
>> >     gpio@1 {
>> >         ....
>> >     };
>> >     ....
>> >     gpio@31 {
>> >         ....
>> >     };
>> > };
>> >
>> > Then if some of them are unused just set it to status = "disabled";
>> >
>> > This also makes your Linux driver simpler because each GPIO port
>> > just becomes a set of 32bit registers and you can use
>> > select GPIO_GENERIC and bgpio_init() and save a whole
>> > slew of standard stock code.
>> >
>>
>> Linus, thank you for your input.
>>
>> The controller handles an array of 32*n signals, where n >= 1 && n <=
>> 4.
>>
>> The problem with the above approach is that the ports are disabled
>> *port*-wise - so they remove all (upto) 4 bits. That would be across the
>> banks.
>>
>> You could of course have the "implied" semantics that a disabled port at
>> any bit position disabled all (bit positions for the same port).
>
> I don't understand this, you would have to elaborate...
>
> In any case microchip,sgpio-ports is probably not the right thing,
> use ngpios which is documented and just divide by 32 to get the
> number of ports I think? But that is just in case they get
> enabled strictly in sequence, otherwise you'd need a custom
> property.
>

Hi Linus,

Yes, the problem is they're not in sequence. F.ex. you could have ports
0,1 enabled, skip 2,3,4 and have 5,6,7 enabled.

In the data stream you would then have:

p0.0 p0.1 p0.2 p0.3
p1.0 p1.1 p1.2 p1.3
p5.0 p5.1 p5.2 p5.3
p6.0 p6.1 p6.2 p6.3
p7.0 p7.1 p7.2 p7.3

I will mull about this and try to come up with something better and more
understandable.

Luckily, this is not gating for integrating sparx5, so its possible
we'll just skip the SGPIO driver for now.

I'll provide an update as soon as possible.

---Lars

> Yours,
> Linus Walleij

-- 
Lars Povlsen,
Microchip