* Linus Walleij [off-list ref] [130613 02:42]:

On Thu, Jun 6, 2013 at 9:14 PM, Balaji T K [off-list ref] wrote:

quoted

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 <redacted>

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.

quoted

+       /* 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?

I agree, it should be a regulator for the MMC driver and that's what I
already suggested earlier. Having it as a regulator allows us to get
rid of all the non-standard before and after calls in the omap_hsmmc.c.
This way the omap_hsmmc.c code can handle the internal and external
voltages the same way.
 

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

The PBIAS for MMC1 is a mux + comparator for the MMC pin, so it makes
sense for the regulator driver to access the register via pinctrl API.
I think the reason why we have registers like this and the USB comparators
in the omap SCM (System Control Module) as the all seem to relate to
pin states.
 

This also looks strange from an MMC point of view.

Yes I agree, it should be a regulator for MMC. Doing it this way just
adds yet more code that's usable for only one of the omap MMC
controllers.
 

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.

quoted

+       usleep_range(350, 400);

And the regulator framework supports power-on delays.

Yes. And it seems that the delays should not be needed, but instead
the comparator bits should be checked.

Regards,

Tony