linux-kernel - Re: [PATCH 2/2] pinctrl: add a generic control 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:	Thu, 20 Oct 2011 16:18:49 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Shawn Guo <shawn.guo@...escale.com>
Cc:	Barry Song <21cnbao@...il.com>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	linux-kernel@...r.kernel.org, Stephen Warren <swarren@...dia.com>,
	Linaro Dev <linaro-dev@...ts.linaro.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	David Brown <davidb@...eaurora.org>,
	Grant Likely <grant.likely@...retlab.ca>
Subject: Re: [PATCH 2/2] pinctrl: add a generic control interface

On Thu, Oct 20, 2011 at 3:10 PM, Shawn Guo <shawn.guo@...escale.com> wrote:

>> without the common definition from  PIN_CONFIG_PULL to
>> PIN_CONFIG_USER, many platforms will need to definite them repeatedly.
>> that is what we hate.
>>
> I prefer to completely leave the encoding of this 'u32 param' to pinctrl
> drivers, so that they can map the bit definition of 'param' to the
> controller register as much as they can. On imx, we have one register
> accommodating all pin configuration options for one pin, so we can
> write 'param' directly into register with exactly same bit definition
> as register.

I understand this argument, because it makes it more convenient to
write drivers and you can cut a few corners.

However I would prefer to keep things more abstract, and the reason
is that we strive to have drivers reused across SoCs. For example
drivers/tty/serial/amba-pl011.c is used on a multitude of SoCs,
U300, Nomadik, Ux500, ARM Versatile, ARM Vexpress, LPC32XX
etc etc.

What if this common driver suddenly need to bias a pin? Is it:

#include <linus/pinctrl/pinctrl.h>

ret = pin_config(pctldev, pin, PIN_CONFIG_BIAS_HIGH, 0);

Or (OK silly example but you get the point):

#include <linus/pinctrl/pinctrl.h>

if (machine_is_u300())
    ret = pin_config(pctldev, pin, PIN_CONFIG_U300_BIAS_VDD, 0);
else if (machine_is_nomadik())
    ret = pin_config(pctldev, pin, PIN_CONFIG_NOMADIK_BIAS_UP, 0);
else if (machine_is_ux500())
    ret = pin_config(pctldev, pin, PIN_CONFIG_UX500_PULLHIGH, 0);
else if (machine_is_versatile())
    ret = pin_config(pctldev, pin, PIN_CONFIG_VERSATILE_POW, 0);
else if (machine_is_vexpress())
    ret = pin_config(pctldev, pin, PIN_CONFIG_VEXPRESS_XTRA, 0);
else if (machine_is_lpc32xx())
    ret = pin_config(pctldev, pin, PIN_CONFIG_LPC_SUPPLY, 0);
...

IMO we have enough strange and custom things in the kernel
already and we need to make things more abstract, not the least
so that people can understand what the code is doing.

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/