From: Drew Fustini <drew@beagleboard.org>
To: Tony Lindgren <tony@atomide.com>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>,
Linus Walleij <linus.walleij@linaro.org>,
linux-omap@vger.kernel.org
Subject: Re: pinctrl-single: num_maps in generic pinconf support?
Date: Thu, 28 May 2020 14:53:23 +0200 [thread overview]
Message-ID: <20200528125323.GA3074222@x1> (raw)
In-Reply-To: <20200527224108.GM37466@atomide.com>
On Wed, May 27, 2020 at 03:41:08PM -0700, Tony Lindgren wrote:
> * Drew Fustini <drew@beagleboard.org> [200527 22:20]:
> > On Wed, May 27, 2020 at 09:51:22AM -0700, Tony Lindgren wrote:
> > > * Drew Fustini <drew@beagleboard.org> [200526 12:22]:
> > > > Hello Haojian and Linus,
> > > >
> > > > For pcs_parse_one_pinctrl_entry() in drivers/pinctrl/pinctrl-single.c,
> > > > I see that num_maps is set to 2 if PCS_HAS_PINCONF is enabled:
> > > >
> > > > 1057 if (PCS_HAS_PINCONF && function) {
> > > > 1058 res = pcs_parse_pinconf(pcs, np, function, map);
> > > > 1059 if (res)
> > > > 1060 goto free_pingroups;
> > > > 1061 *num_maps = 2;
> > > > 1062 } else {
> > > > 1063 *num_maps = 1;
> > > > 1064 }
> > > > 1065 mutex_unlock(&pcs->mutex);
> > > >
> > > > git blame shows me that came from 9dddb4df90d13:
> > > > "pinctrl: single: support generic pinconf"
> > > >
> > > > Would you be able to provide any insight as to num_maps needs to be 2
> > > > when pinconf is enabled?
> > >
> > > Only slightly related, but we should really eventually move omaps to use
> > > #pinctrl-cells = <2> (or 3) instead of 1, and pass the pinconf seprately
> > > from the mux mode.
> >
> > Thanks for the insight, Tony.
> >
> > I was not considering the situation where pinctrl-cells would be more
> > than 1.
> >
> > I see now from pinctrl-single.txt bindings doc that:
> >
> > - #pinctrl-cells : number of cells in addition to the index, set to 1
> > for pinctrl-single,pins and 2 for pinctrl-single,bits
> >
> > I am now wondering if it wrong for me to expect compatible string of
> > "pinconf-single" to work with pinctrl-cells of 1.
>
> Ideally the #pinctrl-cells would be what makes sense for the
> hardware. However, I'm guessing pinctrl-single.c needs patching
> for that to happen.
>
> > I see that arch/arm/boot/dts/da850.dtsi has:
> >
> > 154 pmx_core: pinmux@14120 {
> > 155 compatible = "pinctrl-single";
> > 156 reg = <0x14120 0x50>;
> > 157 #pinctrl-cells = <2>;
> > 158 pinctrl-single,bit-per-mux;
> >
> > and arch/arm/boot/dts/keystone-k2l.dtsi has:
> >
> > 108 k2l_pmx: pinmux@2620690 {
> > 109 compatible = "pinctrl-single";
> > 110 reg = <0x02620690 0xc>;
> > 111 #address-cells = <1>;
> > 112 #size-cells = <0>;
> > 113 #pinctrl-cells = <2>;
> > 114 pinctrl-single,bit-per-mux;
>
> Yeah so there's also the "bit-per-mux" variant. That should not
> affect #pinctrl-cells use, we just need to make it more flexible.
>
> > > We already treat them separately with the new
> > > AM33XX_PADCONF macro, so we'd only have to change one SoC at a time to
> > > use updated #pinctrl-cells. But I think pinctrl-single might need some
> > > changes before we can do that.
> >
> > Do you mean that it would be possible to make the change just for AM335x
> > to start with?
>
> Yes. So ideally we'd just fix up whatever is needed in pinctrl-single.c,
> then just set #pinctrl-cells = <2> in am33xx-l4.dtsi, and update the
> AM33XX_PADCONF accordingly.
Would you be able to describe what you think AM33XX_PADCONF would look
like if the mux and conf are seperated?
Is there an example you know of for another SoC?
> > Do you think the changes would be limited to pinctrl-single.c and the
> > associated device tree files like am33xx-l4.dtsi ?
>
> Yes that should be the case. There should be no need to churn the board
> specific dts files now that we have AM33XX_PADCONF.
Currently, the macro takes dir and mux:
include/dt-bindings/pinctrl/omap.h:
#define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))
For example, in arch/arm/boot/dts/am335x-bone-common.dtsi:
AM33XX_PADCONF(AM335X_PIN_I2C0_SDA, PIN_INPUT_PULLUP, MUX_MODE0)
I think it might be more accurate to rename 'dir' to 'conf'.
Table "9.3.1.50 conf_<module>_<pin> Register" in the AM335x TRM [0]
defines bits 0-2 as "Pad functional signal mux select." Bits 3-6 are
what I would consider pin configuration. Only bit 5 is related to
direction: "Input enable value for the PAD".
>
> Regards,
>
> Tony
thanks,
drew
next prev parent reply other threads:[~2020-05-28 12:53 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-26 12:21 pinctrl-single: num_maps in generic pinconf support? Drew Fustini
2020-05-27 16:51 ` Tony Lindgren
2020-05-27 22:19 ` Drew Fustini
2020-05-27 22:41 ` Tony Lindgren
2020-05-28 12:53 ` Drew Fustini [this message]
2020-05-29 17:40 ` Tony Lindgren
2020-06-08 18:05 ` Drew Fustini
2020-06-08 20:22 ` Drew Fustini
2020-06-09 18:04 ` Tony Lindgren
2020-06-09 17:55 ` Tony Lindgren
2020-05-29 17:55 ` Drew Fustini
2020-05-31 0:17 ` Drew Fustini
2020-06-08 12:09 ` Linus Walleij
2020-06-08 12:40 ` Drew Fustini
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=20200528125323.GA3074222@x1 \
--to=drew@beagleboard.org \
--cc=haojian.zhuang@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-omap@vger.kernel.org \
--cc=tony@atomide.com \
/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).