linux-kernel - Re: [PATCH v2 4/4] ARM: dts: mt8135: Add pinctrl node for mt8135.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAGb2v67F_3CCA2LA887X4TbewUcFj+bXzq912x6t5K4t2X_9mA@mail.gmail.com>
Date:	Tue, 23 Sep 2014 22:16:47 +0800
From:	Chen-Yu Tsai <wens@...e.org>
To:	"Joe.C" <srv_yingjoe.chen@...iatek.com>
Cc:	Arnd Bergmann <arnd@...db.de>, Mark Rutland <mark.rutland@....com>,
	devicetree <devicetree@...r.kernel.org>,
	"Hongzhou. Yang" <srv_hongzhou.yang@...iatek.com>,
	Vladimir Murzin <vladimir.murzin@....com>,
	Russell King <linux@....linux.org.uk>,
	srv_heupstream@...iatek.com, Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Hongzhou Yang <hongzhou.yang@...iatek.com>,
	Catalin Marinas <catalin.marinas@....com>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	Sascha Hauer <kernel@...gutronix.de>,
	Kumar Gala <galak@...eaurora.org>,
	Grant Likely <grant.likely@...aro.org>, dandan.he@...iatek.com,
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 4/4] ARM: dts: mt8135: Add pinctrl node for mt8135.

Hi,

On Tue, Sep 23, 2014 at 9:58 PM, Joe.C <srv_yingjoe.chen@...iatek.com> wrote:
> On Tue, 2014-09-23 at 15:03 +0200, Arnd Bergmann wrote:
>> On Tuesday 23 September 2014 11:39:05 Hongzhou. Yang wrote:
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_GPIO0 (MT_PIN_NO(0) | 0)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_MSDC0_DAT7 (MT_PIN_NO(0) | 1)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_EINT49 (MT_PIN_NO(0) | 2)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_I2SOUT_DAT (MT_PIN_NO(0) | 3)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_DAC_DAT_OUT (MT_PIN_NO(0) | 4)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_PCM1_DO (MT_PIN_NO(0) | 5)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_SPI1_MO (MT_PIN_NO(0) | 6)
>> > +#define MT8135_PIN_0_MSDC0_DAT7__FUNC_NALE (MT_PIN_NO(0) | 7)
>> > +
>>
>> This list looks like it just describes the hardware, I think it would
>> be better to put the values directly into the DT, rather than
>> using such macros.
>
> Hi,
>
> Thanks for review.
> The intend for these macros is helpin pinctrl user to write DT node.
> With these macro, we could write like this for i2c0:
>
> mediatek,pinfunc = <MT8135_PIN_100_SDA0__FUNC_SDA0
>                         MT8135_PIN_101_SCL0__FUNC_SCL0>;
>
> We feel this is less error prone and easier to write than this:
>
> mediatek,pinfunc = <MT_PIN_FUNC(100, 1) MT_PIN_FUNC(101, 1)>


Since you've already imposed the following limit in the DT bindings:

  The mediatek,pinfunc can be either a single value or an array.
  If it is an array, that means all pins use same config in this node.

Maybe you could split the bindings into

  mediatek,pins = <1 2 3 4>;
  mediatek,func = <1>;  (or some macro)

Or better yet, since you've already defined all the function names
in the pinctrl driver, you could just match by name for the functions.
That makes it very verbose and explicit about what you specify.

That's how we do it for sunxi anyway. Of course we have all the pin
function documentation open, so users and reviewers alike can double
check. Just a suggestion. :)


Cheers
ChenYu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/