linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Andrew Jeffery" <andrew@aj.id.au>
To: "Linus Walleij" <linus.walleij@linaro.org>
Cc: "Hongwei Zhang" <hongweiz@ami.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"Joel Stanley" <joel@jms.id.au>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	linux-aspeed@lists.ozlabs.org,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Linux ARM" <linux-arm-kernel@lists.infradead.org>
Subject: Re: [v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
Date: Mon, 29 Jul 2019 09:49:31 +0930	[thread overview]
Message-ID: <f2875111-9ba9-43b7-b2a4-d00c8725f5a0@www.fastmail.com> (raw)
In-Reply-To: <CACRpkdZxsF9gQj0VicVLsPKXg6rKA1mLwbywmazOf0w8PLnOfA@mail.gmail.com>



On Mon, 29 Jul 2019, at 09:04, Linus Walleij wrote:
> On Mon, Jul 22, 2019 at 3:42 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> 
> > If the clock driver owns the control register, it also needs to know how
> > many GPIOs we want to emit on the bus. This seems like an awkward
> > configuration parameter for a clock driver.
> >
> > To avoid the weird parameter we could protect the control register
> > with a lock shared between the clock driver and the SGPIO driver. This
> > way the SGPIO driver could have the ngpios parameter, and request
> > the clock after its written the ngpios value to the control register. A
> > regmap would be useful here to avoid the resource clash and it also
> > provides the required lock.
> 
> Nah. Too complicated.
> 
> What about using the clock API locally (in the singleton driver,
> much as it is today) though, to give the right abstraction?
> 
> See
> drivers/gpu/drm/pl111/pl111_display.c
> pl111_init_clock_divider() for an example of a local
> clock.

Thanks, I'll take a look at that.

> 
> > However, that aside, we can't simply enable the bus in the clock
> > enable callback if enable is called per-bank, as it is called once on
> > the first request with further requests simply refcounted as you
> > mentioned. This is exactly the behaviour we can't tolerate with the
> > bus: it must only be enabled after the last GPIO bank is registered,
> > when we know the total number of GPIOs to emit.
> 
> So the bus needs to know the total number of GPIOs or
> everything breaks, and that is the blocker for this
> divide-and-conquer approach.
> 
> Why does the bus need to know the total number of GPIOs?
> 
> (Maybe the answer is elsewhere in the thread...)

I didn't answer it explicitly, my apologies.

The behaviour is to periodically emit the state of all enabled GPIOs
(i.e. the ngpios value), one per bus clock cycle. There's no explicit
addressing scheme, the protocol encodes the value for a given GPIO
by its position in the data stream relative to a pulse on the "load data"
(LD) line, whose envelope covers the clock cycle for the last GPIO in
the sequence. Similar to SPI the bus has both out and in lines, which
cater to output/input GPIOs.

A rough timing diagram for a 16-GPIO configuration looks like what
I've pasted here:

https://gist.github.com/amboar/c9543af1957854474b8c05ab357f0675

Hope that helps.

Andrew

  reply	other threads:[~2019-07-29  0:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-19 19:24 [v5 0/2] gpio: aspeed: Add SGPIO driver Hongwei Zhang
2019-07-19 19:24 ` [v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support Hongwei Zhang
2019-07-20  8:12   ` Linus Walleij
2019-07-22  1:42     ` Andrew Jeffery
2019-07-28 23:34       ` Linus Walleij
2019-07-29  0:19         ` Andrew Jeffery [this message]
2019-07-29 21:57           ` Linus Walleij
2019-07-30 19:25   ` Hongwei Zhang
2019-07-19 19:24 ` [v5 2/2] gpio: aspeed: Add SGPIO driver Hongwei Zhang
2019-07-20  7:36   ` Linus Walleij
2019-07-22 20:36   ` Hongwei Zhang
2019-07-28 23:37     ` Linus Walleij
2019-07-29 20:29   ` [v6 " Hongwei Zhang
2019-07-29 20:56   ` [v5 " Hongwei Zhang

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=f2875111-9ba9-43b7-b2a4-d00c8725f5a0@www.fastmail.com \
    --to=andrew@aj.id.au \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hongweiz@ami.com \
    --cc=joel@jms.id.au \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@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).