linux-kernel - Re: [PATCH 2/2 v4] 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:	Mon, 5 Dec 2011 14:35:07 +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>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH 2/2 v4] pinctrl: introduce generic pin config

On Thu, Dec 1, 2011 at 6:16 PM, Stephen Warren <swarren@...dia.com> wrote:
> Linus Walleij wrote at Thursday, December 01, 2011 3:12 AM:

>> That will get messy when if I refactor this stuff, add new enums
>> and whatever.
>
> I don't understand what'd be difficult about that.
>
> New standardized enums could be added with values without the top bit
> set. No existing driver would need modification, since their switch(param)
> would not have a case for that new value, and would just return an error.

That would be mixing binary #defines and enums in an unholy
manner, that sounds bad to me.

If you want to do things like that I should replace the current

enum pin_config_param {
        PIN_CONFIG_BIAS_DISABLE,
        PIN_CONFIG_BIAS_HIGH_IMPEDANCE,
       ...
}

With

#define PIN_CONFIG_BIAS_DISABLE 0x00000001
#define PIN_CONFIG_BIAS_HIGH_IMPEDANCE 0x00000002
...

Else it seems like a bit wicked mix-up. So is this what we
should do?

I am reluctant since it mirrors the problem in the GPIO
numberspace where you have no clue what, say GPIO
164 is on a multi-platform binary. It depends on which
platform it booted on. The same will be true for such
enums/defines above the predefined range, totally
depending on the system at hand.

>> Like in the generic debugfs dump function:
>>
>>       if (!ops->is_generic)
>>               return;
>>
>> If I take this out, the generic debugfs code will be used for
>> everything.
>
> I think that'd be fine; the generic code would do all the debug prints
> for the standardized enums, then the core would call into the pinctrl
> driver to perform any additional debug prints for any driver-defined
> custom parameters.

I think Marks point earlier was that he wanted the possibility
to cut out *all* the generic stuff and have only custom config
enumerators for a certain pin controller.

>> And then the generic sematics which you didn't like in the
>> previous patch:
>>
>> if (ret == -EINVAL || ret == -ENOTSUPP)
>>
>> Need to go back in, else the generic debugfs stuff won't
>> work.
>
> That'd be fine, provided the loop only checked the standardized parameters,
> or only enabled that special error-checking case for standardized parameters.

Same comment as above. This won't cut it for all-custom
pin controllers, they will get unwanted semantics pushed at
them.

Thanks,
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/