[PATCH v2 06/10] mmc: omap_hsmmc: add support for pbias configuration in dt

[Linus Walleij]

On Thu, Jun 6, 2013 at 9:14 PM, Balaji T K <balajitk at ti.com> wrote:

> PBIAS register configuration is based on the regulator voltage
> which supplies these pbias cells, sd i/o pads.
> With PBIAS register address and bit definitions different across
> omap[3,4,5], Simplify PBIAS configuration under three different
> regulator voltage levels - O V, 1.8 V, 3 V. Corresponding pinctrl states
> are defined as pbias_off, pbias_1v8, pbias_3v.
>
> pinctrl state mmc_init is used for configuring speed mode, loopback clock
> (in devconf0/devconf1/prog_io1 register for omap3) and pull strength
> configuration (in control_mmc1 for omap4)
>
> Signed-off-by: Balaji T K <balajitk at ti.com>

You *need* Lee Jones and Mark Brown to review this.
Maybe Laurent has something to add too.

Ux500 had the very same thing, and there this was solved using
a GPIO regulator for "vqmmc" a level-shifter. I vaguely remember
Laurent doing something similar with the SH stuff.

> +       /* 100ms delay required for PBIAS configuration */
> +       msleep(100);
> +       if (!vdd && host->pinctrl && host->pbias_off) {
> +               ret = pinctrl_select_state(host->pinctrl, host->pbias_off);
> +               if (ret < 0)
> +                       dev_err(host->dev, "pinctrl pbias_off select error\n");
> +       } else if (((1 << vdd) <= MMC_VDD_165_195) && host->pinctrl &&
> +                       host->pbias_1v8) {
> +               ret = pinctrl_select_state(host->pinctrl, host->pbias_1v8);
> +               if (ret < 0)
> +                       dev_err(host->dev, "pinctrl pbias_1v8 select error\n");
> +       } else if (((1 << vdd) > MMC_VDD_165_195) && host->pinctrl &&
> +                       host->pbias_3v) {
> +               ret = pinctrl_select_state(host->pinctrl, host->pbias_3v);
> +               if (ret < 0)
> +                       dev_err(host->dev, "pinctrl pbias_3v select error\n");
> +       }

So why does the pin control API control bias voltage?

This seem so intuitively wrong as it can possibly get, clearly this
is regulator territory.

This also looks strange from an MMC point of view.

It just seems these bits in these registers should be poked at
by the regulator world, not the pinctrl world. That the bits are
in the middle of pinctrl things does not really matter.

> +       usleep_range(350, 400);

And the regulator framework supports power-on delays.

Yours,
Linus Walleij