Re: [PATCH 2/2] pinctrl: add a generic control interface

[Linus Walleij]

On Thu, Oct 20, 2011 at 3:10 PM, Shawn Guo <shawn.guo@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/