[PATCH v6 1/8] pinctrl: single: support generic pinconf
Tony Lindgren
tony at atomide.com
Thu Jan 3 19:14:21 EST 2013
* Haojian Zhuang <haojian.zhuang at linaro.org> [121221 01:48]:
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -60,6 +61,19 @@ struct pcs_func_vals {
> };
>
> /**
> + * struct pcs_conf_vals - pinconf parameter, pinconf register offset
> + * and value/mask pair
> + * @param: config parameter
> + * @val: register value
> + * @mask: mask of register value
> + */
> +struct pcs_conf_vals {
> + enum pin_config_param param;
> + unsigned val;
> + unsigned mask;
> +};
> +
> +/**
> * struct pcs_function - pinctrl function
> * @name: pinctrl function name
> * @vals: register and vals array
> @@ -74,6 +88,8 @@ struct pcs_function {
> unsigned nvals;
> const char **pgnames;
> int npgnames;
> + struct pcs_conf_vals *conf;
> + int nconfs;
> struct list_head node;
> };
That's nice, that will work much better than the earlier solution :)
> @@ -448,25 +466,149 @@ static struct pinmux_ops pcs_pinmux_ops = {
> static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
> unsigned pin, unsigned long *config)
> {
> + struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
> + struct pin_desc *pdesc = pin_desc_get(pctldev, pin);
> + struct pcs_function *func;
> + const struct pinctrl_setting_mux *setting;
> + unsigned fselector, offset = 0, data = 0, i, j;
> +
> + /* If pin is not described in DTS & enabled, mux_setting is NULL. */
> + setting = pdesc->mux_setting;
> + if (!setting)
> + return -ENOTSUPP;
> + fselector = setting->func;
> + func = radix_tree_lookup(&pcs->ftree, fselector);
> + if (!func) {
> + dev_err(pcs->dev, "%s could not find function%i\n",
> + __func__, fselector);
> + return -ENOTSUPP;
> + }
> +
> + for (i = 0; i < func->nconfs; i++) {
> + if (pinconf_to_config_param(*config) != func->conf[i].param)
> + continue;
> + offset = pin * (pcs->width / BITS_PER_BYTE);
> + data = pcs->read(pcs->base + offset);
> + data &= func->conf[i].mask;
> + switch (func->conf[i].param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + case PIN_CONFIG_BIAS_PULL_UP:
> + case PIN_CONFIG_INPUT_SCHMITT_DISABLE:
> + if (data != func->conf[i].val)
> + return -ENOTSUPP;
> + *config = data;
> + break;
> + case PIN_CONFIG_INPUT_SCHMITT:
> + /* either INPUT_SCHMITT or DISABLE */
> + for (j = 0; j < func->nconfs; j++) {
> + switch (func->conf[j].param) {
> + case PIN_CONFIG_INPUT_SCHMITT_DISABLE:
> + if (data == func->conf[j].val)
> + return -ENOTSUPP;
> + break;
> + default:
> + break;
> + }
> + }
> + *config = data;
> + break;
We should standardize on the binding format of <enableval disableval regmask>
and then all these can be handled the same way I think. And that makes the
binding more generic.
> + default:
> + *config = data;
> + break;
> + }
> + return 0;
> + }
Should we set *config = 0 here too?
> return -ENOTSUPP;
> }
And we should probably just return the raw pinfonf register value
when PIN_CONFIG_END is passed. For write too, we should probably
just write the raw register value when PIN_CONFIG_END is passed
as there can be related pinconf settings that a client driver may
need to use. An example I have for that is a simple USB transceiver
that may provide multiple comparators to figure out the charger
state.
> +static int pcs_config_match(unsigned data, unsigned match)
> +{
> + int ret = 0;
> +
> + if (!match) {
> + if (!data)
> + ret = 1;
> + } else {
> + if ((data & match) == match)
> + ret = 1;
> + }
> + return ret;
> +}
How about do the following here:
static int pcs_config_match(unsigned data, unsigned match)
{
if (!match && !data)
return 1; /* typo? do we really return 1 here? */
if ((data & match) == match)
return 1;
return 0;
}
Regards,
Tony
More information about the linux-arm-kernel
mailing list