* Linus Walleij <linus.walleij@xxxxxxxxxx> [130613 02:42]:
> 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?
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.
>
> > + 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
