From: "Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
To: Max Schwarz <max.schwarz-BGeptl67XyCzQB+pC5nmwQ@public.gmane.org>
Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
Ian Campbell
<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Subject: Re: syscon or memory mappings (was: Re: [RFC/PATCH 0/8] pinctrl-rockchip: Change wrong initial assumptions)
Date: Mon, 05 May 2014 14:11:08 +0200 [thread overview]
Message-ID: <3114029.fvjZVjkFrH@diego> (raw)
In-Reply-To: <9747726.2b5sgjTxWB@typ>
Am Samstag, 3. Mai 2014, 14:40:36 schrieb Max Schwarz:
> Hello Heiko,
>
> On Saturday 03 May 2014 at 01:45:11, Heiko Stübner wrote:
> > > However, this sheds light on an underlying issue: Rockchip is not
> > > treating
> > > the whole GPIO block as one cohesive device as we do currently. Instead,
> > > it
> > > seems to me, one GPIO bank is one device. Each has its cohesive mux,
> > > bank
> > > and pull registers - apart from rk3188-bank-0, maybe. But that one is
> > > special anyway with regards to register ordering (s.b.).
> > >
> > > The issues you had with RK3188 and now have with RK3288 seem to stem
> > > from
> > > trying to group all banks together into one pinctrl driver.
> > >
> > > So maybe we should promote the GPIO banks to full devices in the dt and
> > > make smaller mappings for each GPIO bank, i.e. three mappings for each
> > > GPIO bank (mux, bank, pull). I know we have to stay backwards compatible
> > > dt-wise, but that should be doable.
> >
> > Yes, that is another miss-conception from the early days. The
> > gpio-controllers themself are actually Synopsis Designware IPs - the
> > kernel
> > now even has a separate driver for them (gpio-dwapb). Only the real
> > pincontrol/muxing/pull/etc is from Rockchip.
> >
> > Currently I can't think of a way to move over gracefully, without
> > introducing crap into the gpio-dwapb driver. As at the moment it parses
> > all
> > information it needs from the dt directly - of course with different
> > bindings.
> >
> > That is also the reason I do not want to introduce more "special-cases" in
> > the bank-declaration we're using currently - to not make this move more
> > difficult in the future.
> >
> > As it is, with the patch attached to the last mail, the pinctrl driver
> > wouldn't even need the "rockchip,rk3188-gpio-bank0" compatible anymore, as
> > the information about its special case is now sitting in the bank
> > declaration inside the driver. Which then would enable us to remove the
> > gpio-bank subnodes alltogether and use the external gpio driver.
>
> Okay, you convinced me. That sounds like a good plan to me. Maybe we can
> introduce some compability code into the pinctrl driver to generate the
> correct dt nodes for gpio-dwabp at runtime? I think that would depend on
> something like CONFIG_OF_DYNAMIC though.
Yep, that's one option. The other would be to let the driver support both (the
internal gpio code and the real gpio-dwapb), deprecate the old one
[of course also disallowing it for rk3288 and following] and simply remove it
after the appropriate timespan (1 or more years if I remember correctly :-) ].
> > While for example syscon cannot handle clocks currently, the underlying
> > regmap can - so it would be only a matter of teaching syscon to tell
> > regmap
> > of the clock to use (GRF and PMU register-clocks in this case), instead of
> > needing to have every user of parts of these registers handle the relevant
> > clock itself on register accesses.
> >
> > Also, as you can see in the grf map, the rk3188 actually has two
> > soc_status
> > registers (0xac and 0x108) which have the dmac, cpu and drive strength
> > registers in between. So having a syscon only for soc_con and soc_status
> > will produce problems too.
> >
> > Another example, GRF_IO_VSEL at 0x0380 is most likely some sort of pin
> > voltage selection. As it only spans 1 register I'd assume it could contain
> > settings that span all pin banks.
>
> Okay, we couldn't handle that with individual devices for each bank.
>
> > And of course, splitting the register space into dozens of small
> > individual
> > mappings looks messy :-)
>
> Yes, I agree now ;-)
cool :-D
I've also just sent a v2, including the missing MFD_SYSCON dependency, thanks
for the find.
Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2014-05-05 12:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-29 22:07 [RFC/PATCH 0/8] pinctrl-rockchip: Change wrong initial assumptions Heiko Stübner
2014-04-29 22:07 ` [PATCH 1/8] pinctrl: rockchip: do not require 2nd register area Heiko Stübner
2014-04-29 22:08 ` [PATCH 2/8] pinctrl: rockchip: use regmaps instead of raw mappings Heiko Stübner
2014-05-04 13:31 ` Max Schwarz
2014-04-29 22:08 ` [PATCH 3/8] pinctrl: rockchip: rockchip_pinctrl in rockchip_get_bank_data Heiko Stübner
2014-04-29 22:09 ` [PATCH 4/8] pinctrl: rockchip: let pmu registers be supplied by a syscon Heiko Stübner
2014-04-29 22:09 ` [PATCH 5/8] pinctrl: rockchip: only map bank0-pull-region when pmu regmap missing Heiko Stübner
2014-04-29 22:10 ` [PATCH 6/8] pinctrl: rockchip: base regmap supplied by a syscon Heiko Stübner
2014-04-29 22:10 ` [PATCH 7/8] dt-bindings: adapt rockchip-pinctrl doc to changed bindings Heiko Stübner
2014-04-29 22:10 ` [PATCH 8/8] ARM: dts: rockchip: convert pinctrl nodes to new bindings Heiko Stübner
2014-05-01 10:43 ` [RFC/PATCH 0/8] pinctrl-rockchip: Change wrong initial assumptions Max Schwarz
2014-05-01 13:21 ` syscon or memory mappings (was: Re: [RFC/PATCH 0/8] pinctrl-rockchip: Change wrong initial assumptions) Heiko Stübner
2014-05-02 13:59 ` Max Schwarz
2014-05-02 23:45 ` Heiko Stübner
2014-05-03 12:40 ` Max Schwarz
2014-05-05 12:11 ` Heiko Stübner [this message]
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=3114029.fvjZVjkFrH@diego \
--to=heiko-4mtyjxux2i+zqb+pc5nmwq@public.gmane.org \
--cc=arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
--cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
--cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
--cc=max.schwarz-BGeptl67XyCzQB+pC5nmwQ@public.gmane.org \
--cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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).