From: jmondi <jacopo@jmondi.org>
To: Dong Aisheng <dongas86@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Andy Shevchenko <andy.shevchenko@gmail.com>,
Chris Brandt <Chris.Brandt@renesas.com>,
Jacopo Mondi <jacopo+renesas@jmondi.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Russell King - ARM Linux <linux@armlinux.org.uk>,
Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable
Date: Tue, 23 May 2017 20:37:35 +0200 [thread overview]
Message-ID: <20170523183735.GC13664@w540> (raw)
In-Reply-To: <CAA+hA=Q9hwe7_xL440Reoi7z7m8-SScZ5E057419KSA7dQK=tQ@mail.gmail.com>
Hi Linus,
On Tue, May 23, 2017 at 06:08:31PM +0800, Dong Aisheng wrote:
> On Tue, May 9, 2017 at 5:19 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, May 8, 2017 at 7:25 PM, jmondi <jacopo@jmondi.org> wrote:
> >
> >> From my perspective these flags are configurations internal to the pin
> >> controller hardware used to enable/disable input buffers when a pin needs to
> >> perform in both direction.
> >> The level of detail I can provide on this is the logical diagram we have pointed
> >> you to already.
> >>
> >> As I assume you are trying to get this answer from us in order to
> >> avoid duplicating things in pin controller sub-system, and I
> >> understand this, but my question here was "can we have those flags as part
> >> of the pinmux property argument list, as that property description
> >> seems to allow us to do that, instead of making them generic pin
> >> configuration properties and upset other developers?"
> >
> > Pinmux with all it's magic flags baked into one is not any better
> > or any more readable. The solution is already very pretty except
> > for these two flags which I am sure we can agree on a way forward
> > for.
> >
> > What we choose between is not this or another less transparent
> > pin configuration mechanism, the mechanism (whether magic bits
> > to pinmux or reasonable properties) does not matter.
> >
> > There is a strong preference to use the generic bindings.
> >
> > So the discussion is whether to use:
> >
> > bi-directional;
> > output-enable;
> >
> > Or some already defined config flags.
> >
> > If these are too idiomatic to be used by others, they should anyways
> > look similar, like:
> >
> > renesas,bi-directional;
> > renesas,output-enable;
> >
> > Like the Qualcomm weirdness found in drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> >
> > qcom,pull-up-strength = <..>;
> >
> > Check how they use
> > #define PMIC_GPIO_CONF_PULL_UP (PIN_CONFIG_END + 1)
> >
> > Etc.
> >
> >> Anyway, I still fail to see why those configuration flags, only
> >> affecting the way the pin controller hardware enables/disables
> >> its internal buffers and its internal operations have to be
> >> described in term of their externally visible electrically characteristics.
> >
> > To me internal vs external is not what matters. What matters is
> > if this is likely to pop up in more platforms, and then the property
> > should be generic.
> >
> > The generic pin config definitions are likely to be picked up by other
> > standards and even be inspiration to hardware engineers so that
> > is why it matters so much.
> >
> >> To me, what already exists are pin configuration properties generic to
> >> the whole pin controller subsystem, and I understand you don't want to
> >> see duplication there.
> >>
> >> At the same time, to me, those flags are settings the pin controller
> >> wants to have specified by software to overcome its hw design flaws,
> >> and are intended to configure its internal buffers in a way it cannot
> >> do by itself for some very specific operation modes (they are listed
> >> in the hw reference manual, it's not something you can chose to
> >> configure or not, if you want a pin working in i2c mode, you HAVE to
> >> pass those flags to pin controller).
> >
> > Sounds like a case for
> >
> > renesas,bi-directional;
> > renesas,output-enable;
> >
> > following the Qualcomm pattern in that case.
> >
> > But let's see if something else comes out of this discussion.
> >
>
> I did not follow too much.
> But it seems IMX7ULP/Vybrid to be also a fan of generic
> output-enable/input-enable
> property.
>
> See:
> Figure 5-2. GPIO PAD in Page 241
> http://www.nxp.com/assets/documents/data/en/reference-manuals/VFXXXRM.pdf
>
> It has separate register bits to control input buffer enable and
> output buffer enable
> and we need set it property for GPIO function.
As it seems we have another user for 'output-enable' here, what if we just
add that one to the generic bindings properties list, and we keep
'bi-directional' (which seems to be the most debated property we have
added) out of generic properties?
We can handle 'bi-directional' pins with static tables in our pin
controller driver and not have it anywhere in DT.
I see commit 42d5a11200d0[1] has not been reverted yet as Andy asked
in some previous email. I can send another version of that patch with
only 'output-enable' if you wish.
Once we reach consesus, I can then send v6 of our pin controller driver
based on that.
Thanks
j
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8c58f1a7a4b6d1d723bf25fef9d842d5a11200d0
>
> Regards
> Dong Aisheng
next prev parent reply other threads:[~2017-05-23 18:37 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-27 8:19 [PATCH v5 00/10] Renesas RZ/A1 pin and gpio controller Jacopo Mondi
2017-04-27 8:19 ` [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable Jacopo Mondi
2017-04-27 14:56 ` Andy Shevchenko
2017-04-28 8:32 ` Linus Walleij
2017-04-28 10:09 ` Geert Uytterhoeven
2017-05-07 7:50 ` Linus Walleij
2017-04-28 12:07 ` Chris Brandt
2017-04-28 12:15 ` Andy Shevchenko
2017-04-28 13:18 ` Chris Brandt
2017-04-28 14:53 ` Andy Shevchenko
2017-04-28 15:16 ` Chris Brandt
2017-04-28 15:37 ` Andy Shevchenko
2017-04-28 16:46 ` Chris Brandt
2017-05-07 7:52 ` Linus Walleij
2017-05-08 16:01 ` jmondi
2017-05-08 16:08 ` Andy Shevchenko
2017-05-08 17:02 ` Chris Brandt
2017-05-08 18:26 ` Andy Shevchenko
2017-05-08 20:05 ` Chris Brandt
2017-05-08 17:25 ` jmondi
2017-05-08 17:47 ` Andy Shevchenko
2017-05-09 9:55 ` jmondi
2017-05-08 21:19 ` Linus Walleij
2017-05-09 10:54 ` Geert Uytterhoeven
2017-05-12 9:00 ` Linus Walleij
2017-05-12 9:04 ` Linus Walleij
2017-05-12 11:11 ` Geert Uytterhoeven
2017-05-12 12:13 ` Chris Brandt
2017-05-12 12:25 ` Geert Uytterhoeven
2017-05-12 12:57 ` Chris Brandt
2017-05-12 11:44 ` Chris Brandt
2017-05-23 10:08 ` Dong Aisheng
2017-05-23 18:37 ` jmondi [this message]
2017-05-29 8:45 ` Linus Walleij
2017-05-29 10:42 ` jmondi
2017-05-30 14:12 ` Chris Brandt
2017-06-09 7:26 ` Dong Aisheng
2017-06-09 7:50 ` jmondi
2017-06-11 21:45 ` Linus Walleij
2017-06-12 9:44 ` jmondi
2017-06-13 6:25 ` Dong Aisheng
2017-06-15 11:11 ` jmondi
2017-06-19 15:43 ` Dong Aisheng
2017-04-27 8:19 ` [PATCH v5 02/10] pinctrl: generic: Add macros to unpack properties Jacopo Mondi
2017-04-27 8:37 ` Geert Uytterhoeven
2017-04-28 8:16 ` Linus Walleij
2017-04-28 10:06 ` Geert Uytterhoeven
2017-04-28 12:49 ` jmondi
2017-04-28 16:23 ` Andy Shevchenko
2017-04-27 8:19 ` [PATCH v5 03/10] pinctrl: Renesas RZ/A1 pin and gpio controller Jacopo Mondi
2017-04-27 8:37 ` Geert Uytterhoeven
2017-04-27 8:19 ` [PATCH v5 04/10] dt-bindings: pinctrl: Add RZ/A1 bindings doc Jacopo Mondi
2017-04-27 8:37 ` Geert Uytterhoeven
2017-04-28 21:03 ` Rob Herring
2017-04-27 8:19 ` [PATCH v5 05/10] arm: dts: dt-bindings: Add Renesas RZ/A1 pinctrl header Jacopo Mondi
2017-04-27 8:38 ` Geert Uytterhoeven
2017-04-28 5:19 ` Simon Horman
2017-04-27 8:19 ` [PATCH v5 06/10] arm: dts: r7s72100: Add pin controller node Jacopo Mondi
2017-04-27 8:19 ` [PATCH v5 07/10] arm: dts: genmai: Add SCIF2 pin group Jacopo Mondi
2017-04-28 5:21 ` Simon Horman
2017-04-27 8:19 ` [PATCH v5 08/10] arm: dts: genmai: Add RIIC2 " Jacopo Mondi
2017-04-27 8:19 ` [PATCH v5 09/10] arm: dts: genmai: Add user led device nodes Jacopo Mondi
2017-04-27 8:19 ` [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group Jacopo Mondi
2017-04-27 9:56 ` Geert Uytterhoeven
2017-04-27 10:48 ` Chris Brandt
2017-04-28 5:22 ` Simon Horman
2017-04-28 7:18 ` Geert Uytterhoeven
2017-04-28 14:48 ` Chris Brandt
2017-05-05 12:06 ` Linus Walleij
2017-05-05 12:20 ` Geert Uytterhoeven
2017-05-05 12:45 ` Chris Brandt
2017-05-11 13:48 ` Linus Walleij
2017-04-28 8:49 ` Linus Walleij
2017-04-28 13:50 ` Chris Brandt
2017-04-27 8:42 ` [PATCH v5 00/10] Renesas RZ/A1 pin and gpio controller Geert Uytterhoeven
2017-04-28 5:22 ` Simon Horman
2017-04-28 7:24 ` jmondi
2017-04-28 7:30 ` Geert Uytterhoeven
2017-04-28 7:31 ` Simon Horman
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=20170523183735.GC13664@w540 \
--to=jacopo@jmondi.org \
--cc=Chris.Brandt@renesas.com \
--cc=andy.shevchenko@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=dongas86@gmail.com \
--cc=geert+renesas@glider.be \
--cc=jacopo+renesas@jmondi.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--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).