Re: [PATCH 2/2] ARM: dts: Fix WLAN regression on omap5-uevm

[Tony Lindgren <tony@atomide.com>]

* Javier Martinez Canillas <javier@dowhile0.org> [150921 02:17]:
> [adding Ulf and Tomeu to cc list]
> 
> On Fri, Sep 18, 2015 at 10:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Tony Lindgren <tony@atomide.com> [150918 10:54]:
> >> * Javier Martinez Canillas <javier@dowhile0.org> [150918 10:39]:
> >> > Hello Tony,
> >> >
> >> > [snip]
> >> >
> >> > >
> >> > > +       vmmcsdio_fixed: fixedregulator-mmcsdio {
> >> > > +               compatible = "regulator-fixed";
> >> > > +               regulator-name = "vmmcsdio_fixed";
> >> > > +               regulator-min-microvolt = <1800000>;
> >> > > +               regulator-max-microvolt = <1800000>;
> >> > > +               gpio = <&gpio5 12 GPIO_ACTIVE_HIGH>;    /* gpio140 WLAN_EN */
> >> >
> >> > I know that other OMAP boards use a fake fixed regulator to toggle the
> >> > WiFi enable pin but now the MMC subsystem has proper support for power
> >> > sequence providers so that should be used instead. For simple uses
> >> > like this, Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
> >> > should be enough.
> >>
> >> Hmm OK great, I'll take a look. Looks like that might also provide a
> >> nice solution for handling the 32k clock from palmas to the wl12xx.
> >
> > It seems it's not quite working yet.. It seems pwrseq_simple.c can't
> > deal with delay on a power pin. It might be best to treat those using
> > the regulator framework because of the refcounting and start-up delay.
> 
> Yes, pwrseq_simple does not support delaying a power pin. IIRC Tomeu
> wanted to add a delay for power / reset pins at some point but then
> the SDIO device he wanted to power up didn't need it so he dropped
> that change at the end. I may be missremembering though.
>
> > I don't think also having regulator handling in pwrseq_simple.c
> > conflicts with the MMC regulators as they can be separate?
> >
> 
> The original "mmc: add support for power-on sequencing through DT" [0]
> patch from Olof that added just properties to the MMC controller slot
> dev node had a "card-external-vcc-supply" for SDIO devices that needed
> an external supply separate to vqmmc and vmmc supplies. The
> pwrseq_simple does not support having an input supply regulator right
> now but one can be added if that is needed.

Yes I think adding support for regulators to pwerseq_simple would
solve the issues for power line for start-up latency and usecount for
shared resources.

> > Also not related to wl12xx, the power pin on mwifiex_sdio 8787 wlan
> > must be asserted low while reset is asserted and must stay low for
> > 300ms after power pin has deasserted. Again using a regulator there
> > for the power pin might make more sense.
> >
> 
> Interesting, I've a patch queued on my tree to convert pwrseq_simple
> to use the GPIO descriptors array API since all the code do now is to
> set GPIO levels high or low for all the pins at the same time. But for
> such a fine grained timing control as you said, I wonder if we should
> just keep the code as it is now. Even when is open coding what
> gpiod_{get,set,put}_array already do, it may make sense if we add
> assert and deassert delay time for each pin.

Well the power pin typically has a latency because of a regulator
somewhere.. And it needs to disabled for PM when not used.. And
it may be shared.. So rather than reinvent the roundish thingy
again, I suggest we just let a regulator handle that.

A reset pin just needs to be toggled, but may have a dependency
to the power pin like in the 8787 case.

> Or maybe such a HW should have its own power sequence provider instead
> of making pwrseq_simple more and more complex?
> 
> > Then getting the 32k clock from palmas is not working either, it
> > seems some better deferred probe handling is needed there if
> > omap_hsmmc is built-in and palmas-clk is a loadable module.
> >
> 
> IIUC from another email, you already solved this.

That deferred probe fix did not solve this one, need to debug further.

> > I think for the fix I'll stick with regulator fixed, let's update
> > it later on unless you have better suggestions.
> 
> Yes, I didn't want to block your fixed regulator approach. I was just
> pointing out that the MMC infrastructure already has support for power
> sequence providers but if what is there doesn't fit your use case, I
> agree that we can go with the regulator fixed for now and then change
> it later.

OK yeah let's fix the issues in the pwrseq so we can finally have
resets working for 8787 too :)

Regards,

Tony

> [0]: http://patchwork.ozlabs.org/patch/312444/