linux-kernel - Re: [PATCH 2/2 v5] pinctrl: introduce generic pin config

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:	Tue, 6 Dec 2011 23:53:55 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Stephen Warren <swarren@...dia.com>
Cc:	Linus Walleij <linus.walleij@...ricsson.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	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 2/2 v5] pinctrl: introduce generic pin config

On Mon, Dec 5, 2011 at 6:36 PM, Stephen Warren <swarren@...dia.com> wrote:
> Linus Walleij wrote at Monday, December 05, 2011 9:01 AM:
>> But if I can control the resistance of the pull-up resistor
>> that brings us to a triplet: {parameter, type, argument}
>> like this to set the generic pull-up to 100 kOhm:
>>
>> set_generic_bias(PIN_CONFIG_BIAS, PIN_CONFIG_BIAS_PULL_UP, 100000);
>>
>> parameter = BIAS
>> type = PULL_UP
>> argument = 100 kOhm
>
> I think that selecting what the value of pull-up is and enabling/disabling
> pull-up are separate things, so you'd have:
>
> set PIN_CONFIG_PULL_UP_RESISTANCE 100000
> set PIN_CONFIG_BIAS PULL_UP

So two calls to set the pull up instead of one?
That does not look good.

> Of course, this probably ties into which of the following options your chip
> HW has:
>
> a) Pull-up 100K or pull-up 10K or pull-down or tri-state
>
> b) Pull-up or pull-down or tri-state, with a second register field to
> set pull-up at 100k or 10k.

But what registers to write and in which order doesn't matter
for this interface. That is up to the driver and that is the kind
of stuff drivers are supposed to figure out.

If the driver gets a single call which says "pull up" and
a parameter "100k" it can very well figure out what to do
with that.

> ... which then goes back to the discussion of whether attempting to
> shoe-horn every SoC into a standardized parameter set makes sense.

Which is why the generic parameter set is now optional in
the first place, right?

>> Don't you mean we would then have
>>
>> pin_config_param {
>>    PIN_CONFIG_INPUT_MODE,
>>    ...
>> }
>>
>> enum pin_config_input_mode_value {
>>     PIN_CONFIG_INPUT_MODE_SCHMITT,
>>     PIN_CONFIG_INPUT_MODE_DEBOUNCE,
>> };
>
> Schmitt and debounce seem like completely orthogonal HW features to me,
> so I wouldn't make them mutually exclusive.

True. But the above enumerators does not make them
mutually exclusive do they? You can very well set both
with two calls.

>> I think it might be sub-dividing it too much, but it certainly
>> doesn't hurt the implementation much to split it in three,
>> say 8 bits parameter 8 bits type 16 bits argument if that is
>> preferable what do others say?
>
> That doesn't seem quite right to me. I guess I can't really describe why
> though; it just feels wrong to be going that way.

I feel the same way, so let's think about this.

I think we need some more air into this discussion, what
does other people think?

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/