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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 6, 2013 at 9:14 PM, Balaji T K <balajitk@xxxxxx> 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@xxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux