From: Lars Povlsen <lars.povlsen@microchip.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
Lars Povlsen <lars.povlsen@microchip.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RESEND PATCH v3 2/3] pinctrl: pinctrl-mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO
Date: Fri, 9 Oct 2020 13:14:15 +0200 [thread overview]
Message-ID: <87a6wvy7lk.fsf@soft-dev15.microsemi.net> (raw)
In-Reply-To: <CACRpkdYqKqqM8D0vrBWbo0=7OFthU2kcK2tjd45dD7DxEkaYWg@mail.gmail.com>
Linus Walleij writes:
> Hi Lars,
>
> I'm overall mostly happy with the latest posting (not the one I respond to here)
I'm glad we're getting there :-)
>
> On Thu, Oct 8, 2020 at 12:57 PM Lars Povlsen <lars.povlsen@microchip.com> wrote:
>> > On Tue, Oct 6, 2020 at 4:25 PM Lars Povlsen <lars.povlsen@microchip.com> wrote:
>
>> >> + gc->of_xlate = microchip_sgpio_of_xlate;
>> >> + gc->of_gpio_n_cells = 3;
>> >
>> > So I'm sceptical to this.
>> >
>> > Why can't you just use the pin index in cell 0 directly
>> > and avoid cell 1?
>> >
>>
>> You scepticism has surfaced before :-). The (now) 2 indices relates to
>> how the hardware address signals.
>>
>> Each signal/pin is addressed by port, bit number and direction. We now
>> have the direction encoded in the bank/phandle.
>
> I'm sorry but I just don't get it, I suppose. To me it is pretty
> straight-forward
> that the cells indicate the pin and then the flags. I do understand you
> need the port at all, since this is implicit from the reg property
> of the DT node. Are these two different things?
I responded to this in your comments to the DT bindings.
I just for got to offer to add a description for "#gpio-cells", I see
that's missing. That should make it "crystal clear" - I hope!
Something like:
--- a/Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml
@@ -86,10 +86,17 @@ patternProperties:
gpio-controller: true
'#gpio-cells':
+ description: |
+ Specifies the pin (port and bit) and flags. Note that the
+ SGIO pin is defined by *2* numbers, a port number between 0
+ and 31, and a bit index, 0 to 3. The maximum bit number is
+ controlled indirectly by the "ngpios" property: (ngpios/32).
const: 3
ngpios:
- minimum: 1
+ description: The numbers of GPIO's exposed. This must be a
+ multiple of 32.
+ minimum: 32
maximum: 128
required:
Would that be adequate, or should this also be added as a comment in
microchip_sgpio_of_xlate()?
Like:
+ /* Note that the SGIO pin is defined by *2* numbers, a port
+ * number between 0 and 31, and a bit index, 0 to 3.
+ */
if (gpiospec->args[0] > SGPIO_BITS_PER_WORD ||
gpiospec->args[1] > priv->bitcount)
return -EINVAL;
I hope we can put this one to bed...
---Lars
>
> Yours,
> Linus Walleij
--
Lars Povlsen,
Microchip
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-10-09 11:15 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-06 14:25 [RESEND PATCH v3 0/3] pinctrl: Adding support for Microchip/Microsemi serial GPIO controller Lars Povlsen
2020-10-06 14:25 ` [RESEND PATCH v3 1/3] dt-bindings: pinctrl: Add bindings for pinctrl-mchp-sgpio driver Lars Povlsen
2020-10-06 22:37 ` Rob Herring
2020-10-07 11:07 ` Lars Povlsen
2020-10-06 14:25 ` [RESEND PATCH v3 2/3] pinctrl: pinctrl-mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO Lars Povlsen
2020-10-07 13:30 ` Linus Walleij
2020-10-08 10:57 ` Lars Povlsen
2020-10-09 9:38 ` Linus Walleij
2020-10-09 11:14 ` Lars Povlsen [this message]
2020-10-06 14:25 ` [RESEND PATCH v3 3/3] arm64: dts: sparx5: Add SGPIO devices Lars Povlsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a6wvy7lk.fsf@soft-dev15.microsemi.net \
--to=lars.povlsen@microchip.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=devicetree@vger.kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).