linux-kernel - Re: [PATCH v6] pinctrl: add a pin config interface

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 14 Dec 2011 10:05:42 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Chanho Park <chanho61.park@...sung.com>
Cc:	Linus Walleij <linus.walleij@...ricsson.com>,
	linux-kernel@...r.kernel.org, Stephen Warren <swarren@...dia.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Barry Song <21cnbao@...il.com>,
	Shawn Guo <shawn.guo@...escale.com>,
	Thomas Abraham <thomas.abraham@...aro.org>,
	Dong Aisheng <dong.aisheng@...aro.org>,
	Rajendra Nayak <rajendra.nayak@...aro.org>,
	Haojian Zhuang <haojian.zhuang@...vell.com>
Subject: Re: [PATCH v6] pinctrl: add a pin config interface

On Wed, Dec 14, 2011 at 8:04 AM, Chanho Park <chanho61.park@...sung.com> wrote:

>> ChangeLog v5->v6:
>> - Change to using a pin name as parameter for the
>>   [get|set]_config() functions, as suggested by Stephen Warren.
>>   This is more natural as names will be what a developer has
>>   access to in written documentation etc.
>
>
> I don't agree with it.
> Someone doesn't like to assign a pin name individually because a
> pin number is sufficient to represent each pin.
> We also provide a macro to define a anonymous pin descriptor.
>
> /* Convenience macro to define a single named or anonymous pin descriptor */
> #define PINCTRL_PIN(a, b) { .number = a, .name = b }
> #define PINCTRL_PIN_ANON(a) { .number = a }

That is true, anonymous pins won't be reachable from the
outside. But to some extent that's to be expected.

What turned me around to see things Stephen's way was the fact
that if someone reads this code and finds:

pin_config_set(dev, 42, MY_FUNNY_PULLUP_MACRO);

It is harder to understand than:

pin_config_set(dev, "I2C0 SCL", MY_FUNNY_PULLUP_MACRO);

The second gives and idea about what's going on - you can
look up the pin name "I2C0 SCL" in the datasheet and if
you know something about I2C electronic requirements you
immediately see that "aha, it's pulling up I2C0 SCL line, yeah
that's required of course".

> If we use only the pin name for [get|set]_config(), we must assign the name
> of all pins.

No, you only need to assign a name for the pins you want to
send control commands to. You can still use the pins for
say pinmuxing without naming them.

> IMHO, we also provide pin numbers to control anonymous pins.

Also or instead of? I think the interface needs to be
either names or pin numbers, supporting both will
be confusing.

Is this a blocker for the Samsung way of doing things?

I'm not entirely sure about this ... what do others say?

Yours.
Linus Walleij
--
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/