LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Chen-Yu Tsai <wenst@chromium.org>
To: "zhiyong.tao" <zhiyong.tao@mediatek.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Sean Wang <sean.wang@kernel.org>,
	srv_heupstream <srv_heupstream@mediatek.com>,
	hui.liu@mediatek.com, Eddie Huang <eddie.huang@mediatek.com>,
	Light Hsieh <light.hsieh@mediatek.com>,
	Biao Huang <biao.huang@mediatek.com>,
	Hongzhou Yang <hongzhou.yang@mediatek.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Seiya Wang <seiya.wang@mediatek.com>,
	Devicetree List <devicetree@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define
Date: Tue, 17 Aug 2021 13:44:13 +0800	[thread overview]
Message-ID: <CAGXv+5FtL2zaSWx4tUymx6mpCSb5dXG4XNWM9AJL+b6Ok3dxMg@mail.gmail.com> (raw)
In-Reply-To: <a7c8ab68ac3513865698cde27e665bdd554f459e.camel@mediatek.com>

On Tue, Aug 17, 2021 at 10:21 AM zhiyong.tao <zhiyong.tao@mediatek.com> wrote:
>
> On Tue, 2021-08-17 at 01:00 +0200, Linus Walleij wrote:
> > On Mon, Aug 16, 2021 at 5:38 PM Chen-Yu Tsai <wenst@chromium.org>
> > wrote:
> > > On Mon, Aug 16, 2021 at 6:48 PM zhiyong.tao <
> > > zhiyong.tao@mediatek.com> wrote:
> > > > > I'll take that as "use SI units whenever possible and
> > > > > reasonable".
> > > >
> > > > ==> so It doesn't need to change the define, is it right?
> > > > we will keep the common define.
> > >
> > > Actually I think it would be possible and reasonable to use SI
> > > units
> > > in this case, since you are the vendor and have the resistor values
> > > to implement the support. Having different sets of values for
> > > different
> > > chips is nothing out of the ordinary. We already have to account
> > > for
> > > different number of pins and different pin functions. That is what
> > > compatible strings are for.
> >
> > I fully agree with Chen-Yu's analysis here.
> >
> > Zhiyong can you make an attempt to use SI units (Ohms) and see
> > what it will look like? I think it will look better for users and it
> > will
> > be less risk to make mistakes.
> >
> > Yours,
> > Linus Walleij
>
> Hi Linus & chen-yu,
>
> The rsel actual bias resistance of each setting is different in
> different IC. For example, in mt8195, the rsel actual bias resistance
> setting like as:
> MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD;
> MTK_PULL_SET
> _RSEL_001:10k in PU, 5k in PD;
> MTK_PULL_SET_RSEL_010:5k in PU, 75k in
> PD;
> MTK_PULL_SET_RSEL_011:4k in PU, 5K in PD;
> MTK_PULL_SET_RSEL_100:3k in
> PU, 75k in PD;
> MTK_PULL_SET_RSEL_101:2k in PU, 5K in PD;
> MTK_PULL_SET_RSE
> L_110:1.5k in PU, 75k in PD;
> MTK_PULL_SET_RSEL_111:1k in PU, 5k in PD.
>
> but in mt8192, the rsel actual bias resistance setting like as:
> MTK_PULL_SET_RSEL_000:75K in PU, 75k in PD;
> MTK_PULL_SET_RSEL_001:3k in PU, 5k in PD;
> MTK_PULL_SET_RSEL_010:10k in PU, 75k in PD;
> MTK_PULL_SET_RSEL_011:1k in PU, 5K in PD;
>
> Can you help me to provide a suggestion common define for the all
> different IC?
> It seems that we should add a new define, if we upstream a new IC
> pinctrl driver in the future.

I assume you mean the macros used in the device tree?

The point of using SI units is to get rid of the macros. Instead of:

    bias-pull-up = <MTK_PULL_SET_RSEL_000>;

and

    bias-pull-down = <MTK_PULL_SET_RSEL_011>;

We want:

    bias-pull-up = <75000>;

and

    bias-pull-down = <5000>;

And the pinctrl driver then converts the real values in the device tree
into register values using some lookup table.

The DT schema could then enumerate all the valid resistor values,
and get proper validity checking.

Now if you really wanted to keep some symbols for mapping hardware
register values to resistor values, you could have

    #define MT8192_PULL_UP_RSEL_001      75000
    #define MT8192_PULL_DOWN_RSEL_001     5000

or have them all named MTK_PULL_{UP,DOWN}_RSEL_NNN, but split into
different header files, one per SoC.

Personally I think having the macros is a bad idea if proper values
are available. It just adds another layer of indirection, and another
area where errors can creep in.


Regards
ChenYu

  parent reply	other threads:[~2021-08-17  5:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-10  8:17 [PATCH v10 0/2] Mediatek pinctrl patch on mt8195 Zhiyong Tao
2021-07-10  8:17 ` [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define Zhiyong Tao
2021-07-13  7:17   ` Chen-Yu Tsai
2021-07-22  7:54     ` zhiyong tao
2021-07-26  8:02       ` Chen-Yu Tsai
     [not found]         ` <07388dac4e25e0f260725e8f80ba099d5aa80949.camel@mediatek.com>
2021-07-29  9:43           ` Chen-Yu Tsai
2021-08-04 23:01             ` Linus Walleij
2021-08-16  6:10               ` Chen-Yu Tsai
     [not found]                 ` <4fd12d5c53f6492e5fa3ba94a78b9a149f5b6ed9.camel@mediatek.com>
2021-08-16 15:37                   ` Chen-Yu Tsai
2021-08-16 23:00                     ` Linus Walleij
     [not found]                       ` <a7c8ab68ac3513865698cde27e665bdd554f459e.camel@mediatek.com>
2021-08-17  5:44                         ` Chen-Yu Tsai [this message]
     [not found]                           ` <d40f6d5fd26aedb13e7a393202e5794b1893ecf8.camel@mediatek.com>
2021-08-17 20:09                             ` Linus Walleij
2021-08-18  6:25                               ` Chen-Yu Tsai
2021-07-10  8:17 ` [PATCH v10 2/2] pinctrl: mediatek: add rsel setting on MT8195 Zhiyong Tao

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=CAGXv+5FtL2zaSWx4tUymx6mpCSb5dXG4XNWM9AJL+b6Ok3dxMg@mail.gmail.com \
    --to=wenst@chromium.org \
    --cc=biao.huang@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=eddie.huang@mediatek.com \
    --cc=hongzhou.yang@mediatek.com \
    --cc=hui.liu@mediatek.com \
    --cc=light.hsieh@mediatek.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sean.wang@kernel.org \
    --cc=sean.wang@mediatek.com \
    --cc=seiya.wang@mediatek.com \
    --cc=srv_heupstream@mediatek.com \
    --cc=zhiyong.tao@mediatek.com \
    --subject='Re: [PATCH v10 1/2] dt-bindings: pinctrl: mt8195: add rsel define' \
    /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

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).