2014-06-27 14:39:47

by Andreas Fenkart

[permalink] [raw]
Subject: mwifiex card reset

I have an mwifiex module(sd8787) behind omap_hsmmc(am33xx-soc)
The module is non-removable wired fix to soc. Now the wifi module
needs 2 clocks; one is a sleep clock the other used when not
sleeping.
While the sleep clock is fixed, @32 kHz, the system clock can be
one out of several values, but can be be derived automatically
from the sleep clock. But this requires the clock to be present
when the wifi module comes out of reset.

Another problem is software reboot, at initial power on the wifi
card will react to mmc discovery. After a software reset of the
host it will not, because it has already been disvovered and
didn't notice the host rebooted, unless we reset it as well.
While this seems obvious, the problem is that the reset is needed
before the card can be discovered. Which means we cannot move the
reset logic into the mwifiex driver, since that driver has not
yet been selected through vendor/product id.

the reset logic:
gpiod_set_value(card->gpiod_reset, 0);
clk_enable(card->sleep_clock);
udelay(1000);
gpiod_set_value(card->gpiod_reset, 1);

The idea so far was to extend the device-tree node of the
mmc slot by some reset leafs;

&mmc {
ti,non-removable;
..
peripheral-clocks = <&clk &clk2 ...>;
peripheral-reset-gpios = <&gpio0 1 1 &gpio1 2 3 ...>;
}

in mmc_add_host, all clocks will be enabled and gpios be pulled
low for 1 msec

Is this a feasible solution?


Another related issue, is the card reset in mwifiex driver:

static void sdio_card_reset_worker(struct work_struct *work)
{
struct mmc_host *target = reset_host;

pr_err("Resetting card...\n");
mmc_remove_host(target);
/* 20ms delay is based on experiment with sdhci controller */
mdelay(20);
mmc_add_host(target);
}
static DECLARE_WORK(card_reset_work, sdio_card_reset_worker);

There are obviously a lot of problems with this, e.g. custom debugfs
entries created in the driver will be lost. But sometimes this
code might avert disaster in case the command handlers between
driver and firmware lost synchronization

How could this be done in a better way?


2014-06-29 09:41:38

by Andreas Fenkart

[permalink] [raw]
Subject: Re: mwifiex card reset

2014-06-28 9:23 GMT+02:00 Tony Lindgren <[email protected]>:
> * James Cameron <[email protected]> [140628 08:24]:
>> On Fri, Jun 27, 2014 at 04:39:42PM +0200, Andreas Fenkart wrote:
>> > I have an mwifiex module(sd8787) behind omap_hsmmc(am33xx-soc)
>> > The module is non-removable wired fix to soc. Now the wifi module
>> > needs 2 clocks; one is a sleep clock the other used when not
>> > sleeping.
>> > While the sleep clock is fixed, @32 kHz, the system clock can be
>> > one out of several values, but can be be derived automatically
>> > from the sleep clock. But this requires the clock to be present
>> > when the wifi module comes out of reset.
>> >

ref 1)

>> > Another problem is software reboot, at initial power on the wifi
>> > card will react to mmc discovery. After a software reset of the
>> > host it will not, because it has already been discovered and
>> > didn't notice the host rebooted, unless we reset it as well.
>> > While this seems obvious, the problem is that the reset is needed
>> > before the card can be discovered. Which means we cannot move the
>> > reset logic into the mwifiex driver, since that driver has not
>> > yet been selected through vendor/product id.
>> >
>>
>> we had same problem with sd8787 and a different system design. we
>> added reset-gpios property [1] and used it in sdhci-pxav3.c [2] as
>> sdhci_pxav3_toggle_reset_gpio()
>>
>> 1.
>>
>> http://code.coreboot.org/p/openfirmware/source/tree/HEAD/cpu/arm/olpc/sdhci.fth#L104
>>
>> 2.
>>
>> http://dev.laptop.org/git/olpc-kernel/tree/drivers/mmc/host/sdhci-pxav3.c?h=arm-3.5#n229
>>
>> > the reset logic:
>> > gpiod_set_value(card->gpiod_reset, 0);
>> > clk_enable(card->sleep_clock);
>> > udelay(1000);
>> > gpiod_set_value(card->gpiod_reset, 1);
>> >
>> > The idea so far was to extend the device-tree node of the
>> > mmc slot by some reset leafs;
>> >
>> > &mmc {
>> > ti,non-removable;
>> > ..
>> > peripheral-clocks = <&clk &clk2 ...>;
>> > peripheral-reset-gpios = <&gpio0 1 1 &gpio1 2 3 ...>;
>> > }
>> >
>> > in mmc_add_host, all clocks will be enabled and gpios be pulled
>> > low for 1 msec
>>
>> we used 5ms.
>>
>> > Is this a feasible solution?
>> >
>> >
>> > Another related issue, is the card reset in mwifiex driver:
>> >
>> > static void sdio_card_reset_worker(struct work_struct *work)
>> > {
>> > struct mmc_host *target = reset_host;
>> >
>> > pr_err("Resetting card...\n");
>> > mmc_remove_host(target);
>> > /* 20ms delay is based on experiment with sdhci controller */
>> > mdelay(20);
>> > mmc_add_host(target);
>> > }
>> > static DECLARE_WORK(card_reset_work, sdio_card_reset_worker);
>> >
>> > There are obviously a lot of problems with this, e.g. custom debugfs
>> > entries created in the driver will be lost. But sometimes this
>> > code might avert disaster in case the command handlers between
>> > driver and firmware lost synchronization
>> >
>> > How could this be done in a better way?
>>
>> i'm interested as well.
>
> Wouldn't it be best to have the mwifiex properly handle the
> reset GPIOs and idle status pins?

doesn't work see ref 1) above

> Those are not part of the SDIO spec AFAIK, and the mmc
> controller should not need to care about those.
>
> Also, at least omaps also have an issue where suspend won't
> work with mwifiex loaded FYI.

first command after resume needs to be cmd52, not cmd53 as the
driver would by default. it's another issue

>
> Regards,
>
> Tony

2014-06-28 05:22:42

by James Cameron

[permalink] [raw]
Subject: Re: mwifiex card reset

On Fri, Jun 27, 2014 at 04:39:42PM +0200, Andreas Fenkart wrote:
> I have an mwifiex module(sd8787) behind omap_hsmmc(am33xx-soc)
> The module is non-removable wired fix to soc. Now the wifi module
> needs 2 clocks; one is a sleep clock the other used when not
> sleeping.
> While the sleep clock is fixed, @32 kHz, the system clock can be
> one out of several values, but can be be derived automatically
> from the sleep clock. But this requires the clock to be present
> when the wifi module comes out of reset.
>
> Another problem is software reboot, at initial power on the wifi
> card will react to mmc discovery. After a software reset of the
> host it will not, because it has already been disvovered and
> didn't notice the host rebooted, unless we reset it as well.
> While this seems obvious, the problem is that the reset is needed
> before the card can be discovered. Which means we cannot move the
> reset logic into the mwifiex driver, since that driver has not
> yet been selected through vendor/product id.
>

we had same problem with sd8787 and a different system design. we
added reset-gpios property [1] and used it in sdhci-pxav3.c [2] as
sdhci_pxav3_toggle_reset_gpio()

1.

http://code.coreboot.org/p/openfirmware/source/tree/HEAD/cpu/arm/olpc/sdhci.fth#L104

2.

http://dev.laptop.org/git/olpc-kernel/tree/drivers/mmc/host/sdhci-pxav3.c?h=arm-3.5#n229

> the reset logic:
> gpiod_set_value(card->gpiod_reset, 0);
> clk_enable(card->sleep_clock);
> udelay(1000);
> gpiod_set_value(card->gpiod_reset, 1);
>
> The idea so far was to extend the device-tree node of the
> mmc slot by some reset leafs;
>
> &mmc {
> ti,non-removable;
> ..
> peripheral-clocks = <&clk &clk2 ...>;
> peripheral-reset-gpios = <&gpio0 1 1 &gpio1 2 3 ...>;
> }
>
> in mmc_add_host, all clocks will be enabled and gpios be pulled
> low for 1 msec

we used 5ms.

> Is this a feasible solution?
>
>
> Another related issue, is the card reset in mwifiex driver:
>
> static void sdio_card_reset_worker(struct work_struct *work)
> {
> struct mmc_host *target = reset_host;
>
> pr_err("Resetting card...\n");
> mmc_remove_host(target);
> /* 20ms delay is based on experiment with sdhci controller */
> mdelay(20);
> mmc_add_host(target);
> }
> static DECLARE_WORK(card_reset_work, sdio_card_reset_worker);
>
> There are obviously a lot of problems with this, e.g. custom debugfs
> entries created in the driver will be lost. But sometimes this
> code might avert disaster in case the command handlers between
> driver and firmware lost synchronization
>
> How could this be done in a better way?

i'm interested as well.

--
James Cameron
http://quozl.linux.org.au/

2014-06-30 06:19:34

by Tony Lindgren

[permalink] [raw]
Subject: Re: mwifiex card reset

* Andreas Fenkart <[email protected]> [140629 12:43]:
> 2014-06-28 9:23 GMT+02:00 Tony Lindgren <[email protected]>:
> > * James Cameron <[email protected]> [140628 08:24]:
> >> On Fri, Jun 27, 2014 at 04:39:42PM +0200, Andreas Fenkart wrote:
> >> > I have an mwifiex module(sd8787) behind omap_hsmmc(am33xx-soc)
> >> > The module is non-removable wired fix to soc. Now the wifi module
> >> > needs 2 clocks; one is a sleep clock the other used when not
> >> > sleeping.
> >> > While the sleep clock is fixed, @32 kHz, the system clock can be
> >> > one out of several values, but can be be derived automatically
> >> > from the sleep clock. But this requires the clock to be present
> >> > when the wifi module comes out of reset.
> >> >
>
> ref 1)
>
> >> > Another problem is software reboot, at initial power on the wifi
> >> > card will react to mmc discovery. After a software reset of the
> >> > host it will not, because it has already been discovered and
> >> > didn't notice the host rebooted, unless we reset it as well.
> >> > While this seems obvious, the problem is that the reset is needed
> >> > before the card can be discovered. Which means we cannot move the
> >> > reset logic into the mwifiex driver, since that driver has not
> >> > yet been selected through vendor/product id.
> >> >
> >>
> >> we had same problem with sd8787 and a different system design. we
> >> added reset-gpios property [1] and used it in sdhci-pxav3.c [2] as
> >> sdhci_pxav3_toggle_reset_gpio()
> >>
> >> 1.
> >>
> >> http://code.coreboot.org/p/openfirmware/source/tree/HEAD/cpu/arm/olpc/sdhci.fth#L104
> >>
> >> 2.
> >>
> >> http://dev.laptop.org/git/olpc-kernel/tree/drivers/mmc/host/sdhci-pxav3.c?h=arm-3.5#n229
> >>
> >> > the reset logic:
> >> > gpiod_set_value(card->gpiod_reset, 0);
> >> > clk_enable(card->sleep_clock);
> >> > udelay(1000);
> >> > gpiod_set_value(card->gpiod_reset, 1);
> >> >
> >> > The idea so far was to extend the device-tree node of the
> >> > mmc slot by some reset leafs;
> >> >
> >> > &mmc {
> >> > ti,non-removable;
> >> > ..
> >> > peripheral-clocks = <&clk &clk2 ...>;
> >> > peripheral-reset-gpios = <&gpio0 1 1 &gpio1 2 3 ...>;
> >> > }
> >> >
> >> > in mmc_add_host, all clocks will be enabled and gpios be pulled
> >> > low for 1 msec
> >>
> >> we used 5ms.
> >>
> >> > Is this a feasible solution?
> >> >
> >> >
> >> > Another related issue, is the card reset in mwifiex driver:
> >> >
> >> > static void sdio_card_reset_worker(struct work_struct *work)
> >> > {
> >> > struct mmc_host *target = reset_host;
> >> >
> >> > pr_err("Resetting card...\n");
> >> > mmc_remove_host(target);
> >> > /* 20ms delay is based on experiment with sdhci controller */
> >> > mdelay(20);
> >> > mmc_add_host(target);
> >> > }
> >> > static DECLARE_WORK(card_reset_work, sdio_card_reset_worker);
> >> >
> >> > There are obviously a lot of problems with this, e.g. custom debugfs
> >> > entries created in the driver will be lost. But sometimes this
> >> > code might avert disaster in case the command handlers between
> >> > driver and firmware lost synchronization
> >> >
> >> > How could this be done in a better way?
> >>
> >> i'm interested as well.
> >
> > Wouldn't it be best to have the mwifiex properly handle the
> > reset GPIOs and idle status pins?
>
> doesn't work see ref 1) above

OK so we can't do much anything at mwifiex probe time on SDIO bus because
it won't probe unless the pins are configured.

Maybe we should have a separate mwifiex-power helper module that just
manages the reset/idle/regulator pins? Then mwifiex-reset can be always
loaded and configure things so mwifiex-sdio can probe properly.

And mwifiex-reset helper can also provide the user space interfaces
for the reset/idle/regulator pins.

> > Those are not part of the SDIO spec AFAIK, and the mmc
> > controller should not need to care about those.
> >
> > Also, at least omaps also have an issue where suspend won't
> > work with mwifiex loaded FYI.
>
> first command after resume needs to be cmd52, not cmd53 as the
> driver would by default. it's another issue

Oh OK, sounds like you have some patches coming for that?

Regards,

Tony

2014-06-28 07:23:32

by Tony Lindgren

[permalink] [raw]
Subject: Re: mwifiex card reset

* James Cameron <[email protected]> [140628 08:24]:
> On Fri, Jun 27, 2014 at 04:39:42PM +0200, Andreas Fenkart wrote:
> > I have an mwifiex module(sd8787) behind omap_hsmmc(am33xx-soc)
> > The module is non-removable wired fix to soc. Now the wifi module
> > needs 2 clocks; one is a sleep clock the other used when not
> > sleeping.
> > While the sleep clock is fixed, @32 kHz, the system clock can be
> > one out of several values, but can be be derived automatically
> > from the sleep clock. But this requires the clock to be present
> > when the wifi module comes out of reset.
> >
> > Another problem is software reboot, at initial power on the wifi
> > card will react to mmc discovery. After a software reset of the
> > host it will not, because it has already been disvovered and
> > didn't notice the host rebooted, unless we reset it as well.
> > While this seems obvious, the problem is that the reset is needed
> > before the card can be discovered. Which means we cannot move the
> > reset logic into the mwifiex driver, since that driver has not
> > yet been selected through vendor/product id.
> >
>
> we had same problem with sd8787 and a different system design. we
> added reset-gpios property [1] and used it in sdhci-pxav3.c [2] as
> sdhci_pxav3_toggle_reset_gpio()
>
> 1.
>
> http://code.coreboot.org/p/openfirmware/source/tree/HEAD/cpu/arm/olpc/sdhci.fth#L104
>
> 2.
>
> http://dev.laptop.org/git/olpc-kernel/tree/drivers/mmc/host/sdhci-pxav3.c?h=arm-3.5#n229
>
> > the reset logic:
> > gpiod_set_value(card->gpiod_reset, 0);
> > clk_enable(card->sleep_clock);
> > udelay(1000);
> > gpiod_set_value(card->gpiod_reset, 1);
> >
> > The idea so far was to extend the device-tree node of the
> > mmc slot by some reset leafs;
> >
> > &mmc {
> > ti,non-removable;
> > ..
> > peripheral-clocks = <&clk &clk2 ...>;
> > peripheral-reset-gpios = <&gpio0 1 1 &gpio1 2 3 ...>;
> > }
> >
> > in mmc_add_host, all clocks will be enabled and gpios be pulled
> > low for 1 msec
>
> we used 5ms.
>
> > Is this a feasible solution?
> >
> >
> > Another related issue, is the card reset in mwifiex driver:
> >
> > static void sdio_card_reset_worker(struct work_struct *work)
> > {
> > struct mmc_host *target = reset_host;
> >
> > pr_err("Resetting card...\n");
> > mmc_remove_host(target);
> > /* 20ms delay is based on experiment with sdhci controller */
> > mdelay(20);
> > mmc_add_host(target);
> > }
> > static DECLARE_WORK(card_reset_work, sdio_card_reset_worker);
> >
> > There are obviously a lot of problems with this, e.g. custom debugfs
> > entries created in the driver will be lost. But sometimes this
> > code might avert disaster in case the command handlers between
> > driver and firmware lost synchronization
> >
> > How could this be done in a better way?
>
> i'm interested as well.

Wouldn't it be best to have the mwifiex properly handle the
reset GPIOs and idle status pins?

Those are not part of the SDIO spec AFAIK, and the mmc
controller should not need to care about those.

Also, at least omaps also have an issue where suspend won't
work with mwifiex loaded FYI.

Regards,

Tony

2014-06-30 19:30:12

by Daniel Mack

[permalink] [raw]
Subject: Re: mwifiex card reset

Hi Tony, everyone,

Thanks Andreas for addressing this issue! So far, we've been using a
terrible hack in the hsmmc in order to bring the card into a workable
state, and we're looking for a nicer solution for awhile.

On 06/30/2014 08:19 AM, Tony Lindgren wrote:
> * Andreas Fenkart <[email protected]> [140629 12:43]:
>> 2014-06-28 9:23 GMT+02:00 Tony Lindgren <[email protected]>:
>>> * James Cameron <[email protected]> [140628 08:24]:

>>> Wouldn't it be best to have the mwifiex properly handle the
>>> reset GPIOs and idle status pins?
>>
>> doesn't work see ref 1) above
>
> OK so we can't do much anything at mwifiex probe time on SDIO bus because
> it won't probe unless the pins are configured.
>
> Maybe we should have a separate mwifiex-power helper module that just
> manages the reset/idle/regulator pins? Then mwifiex-reset can be always
> loaded and configure things so mwifiex-sdio can probe properly.
>
> And mwifiex-reset helper can also provide the user space interfaces
> for the reset/idle/regulator pins.

Yes, a helper might be the best solution. It could even be a generic
one, that just takes any number of clocks, reset GPIOs and regulators
and takes care for sequencing them. However, we need to reference that
helper from the mwifiex driver, for two reasons.

a) we need to make sure the reset helper gets to do its job before the
mwifiex driver scans the SDIO bus, and

b) the reset helper needs to be called when the mmc host controller
wants to do a card reset.

Hence, we'll need some sort of internal API for this, and a phandle in
dts. I wonder whether that glue logic might be better off living in the
mmc core, as mwifiex might well be interfaced to other hosts?


Thanks,
Daniel


2014-07-15 13:25:20

by Andreas Fenkart

[permalink] [raw]
Subject: Re: mwifiex card reset

2014-07-01 17:09 GMT+02:00 Doug Anderson <[email protected]>:
> +Olof who posted the patch that Yuvaraj referenced.
>
> On Tue, Jul 1, 2014 at 5:20 AM, Yuvaraj Cd <[email protected]> wrote:
>> On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <[email protected]> wrote:
>>> On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
>>>> I may have missed something, but doesn't the MMC_POWER_OFF and
>>>> MMC_POWER_ON|UP handling in controller driver help?
>>>> Anyway the clocks/GPIOs/regulators are sort of platform
>>>> dependent. Would it be better putting it in /arch/arm/mach-xxxxx/?
>>>
>>> Wouldn't device tree for mmc be better?
>> I have come across same problem.Below is the thread in which more
>> discussions happened on this.
>> http://patchwork.ozlabs.org/patch/312444/
>> I am adding few more those who are interested in this solution.
>
> Thanks to Yuvaraj for referencing the old thread.

http://www.spinics.net/lists/linux-mmc/msg26564.html

Quite a read indeed

Meanwhile I got a working prototype for omap_hsmmc/mwifiex based on this series:
http://www.spinics.net/lists/linux-mmc/msg27228.html

I will post my patches, once the FDT format is set in stone, and Ulf
has had some time to work on v2 of his series.

2014-07-01 07:52:47

by Daniel Mack

[permalink] [raw]
Subject: Re: mwifiex card reset

Hi Bing,

On 07/01/2014 08:44 AM, Bing Zhao wrote:
>> Hence, we'll need some sort of internal API for this, and a phandle
>> in dts. I wonder whether that glue logic might be better off living
>> in the mmc core, as mwifiex might well be interfaced to other
>> hosts?
>
> I may have missed something, but doesn't the MMC_POWER_OFF and
> MMC_POWER_ON|UP handling in controller driver help? Anyway the
> clocks/GPIOs/regulators are sort of platform dependent. Would it be
> better putting it in /arch/arm/mach-xxxxx/?

Regulators are not platform specific, they never were. For GPIOs and
clocks, we can simply depend on CONFIG_GPIO_GENERIC and
CONFIG_COMMON_CLK. I wouldn't even bother to care for anything else.
This way, we also get a way of referencing the resources in dts through
phandles for free.

What I was talking about above was that conducting the actual reset from
the helper must be something that either the mmc core or individual host
implementations can trigger. We need to repeat this action every time
mwifiex decides to call its reset routine, which is currently
implemented like this:

mmc_remove_host(target);
mdelay(20);
mmc_add_host(target);

But I haven't looked into possible ways of implementation yet.
MMC_POWER_* might well be a suitable thing to hook into.


Thanks,
Daniel

2014-07-01 12:20:10

by Yuvaraj Cd

[permalink] [raw]
Subject: Re: mwifiex card reset

On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <[email protected]> wrote:
> On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
>> I may have missed something, but doesn't the MMC_POWER_OFF and
>> MMC_POWER_ON|UP handling in controller driver help?
>> Anyway the clocks/GPIOs/regulators are sort of platform
>> dependent. Would it be better putting it in /arch/arm/mach-xxxxx/?
>
> Wouldn't device tree for mmc be better?
I have come across same problem.Below is the thread in which more
discussions happened on this.
http://patchwork.ozlabs.org/patch/312444/
I am adding few more those who are interested in this solution.
>
> --
> James Cameron
> http://quozl.linux.org.au/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-07-01 07:02:34

by Bing Zhao

[permalink] [raw]
Subject: RE: mwifiex card reset

Hi James,

> On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
> > I may have missed something, but doesn't the MMC_POWER_OFF and
> > MMC_POWER_ON|UP handling in controller driver help?
> > Anyway the clocks/GPIOs/regulators are sort of platform dependent.
> > Would it be better putting it in /arch/arm/mach-xxxxx/?
>
> Wouldn't device tree for mmc be better?

Yes, device tree is better for defining clocks/GPIO/regulators, etc.
But I guess the reset logic (making use of these definitions) cannot be device tree?

Regards,
Bing

>
> --
> James Cameron
> http://quozl.linux.org.au/

2014-07-01 06:57:52

by James Cameron

[permalink] [raw]
Subject: Re: mwifiex card reset

On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
> I may have missed something, but doesn't the MMC_POWER_OFF and
> MMC_POWER_ON|UP handling in controller driver help?
> Anyway the clocks/GPIOs/regulators are sort of platform
> dependent. Would it be better putting it in /arch/arm/mach-xxxxx/?

Wouldn't device tree for mmc be better?

--
James Cameron
http://quozl.linux.org.au/

2014-07-01 15:09:39

by Doug Anderson

[permalink] [raw]
Subject: Re: mwifiex card reset

+Olof who posted the patch that Yuvaraj referenced.

On Tue, Jul 1, 2014 at 5:20 AM, Yuvaraj Cd <[email protected]> wrote:
> On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <[email protected]> wrote:
>> On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
>>> I may have missed something, but doesn't the MMC_POWER_OFF and
>>> MMC_POWER_ON|UP handling in controller driver help?
>>> Anyway the clocks/GPIOs/regulators are sort of platform
>>> dependent. Would it be better putting it in /arch/arm/mach-xxxxx/?
>>
>> Wouldn't device tree for mmc be better?
> I have come across same problem.Below is the thread in which more
> discussions happened on this.
> http://patchwork.ozlabs.org/patch/312444/
> I am adding few more those who are interested in this solution.

Thanks to Yuvaraj for referencing the old thread.

It seems like there are already enough chefs in the kitchen so I'm not
going to add any more suggestions myself. I'll point out that on all
ARM Chromebooks (which to date have a Marvell part connected via SDIO)
all face the same problem. In production kernels we've continued to
get by with various hacks. The current kernels use a hack with
regulator chaining and assumptions about 32K clocks already being on.
This is less egregious than the hacks in older kernels which initted
WiFi in the LCD backlight function (dont ask) but is still a hack.

-Doug

2014-07-01 08:44:43

by Andreas Fenkart

[permalink] [raw]
Subject: Re: mwifiex card reset

Hi Bing,

2014-07-01 8:44 GMT+02:00 Bing Zhao <[email protected]>:
[snip]
>> Yes, a helper might be the best solution. It could even be a generic one, that
>> just takes any number of clocks, reset GPIOs and regulators and takes care
>> for sequencing them. However, we need to reference that helper from the
>> mwifiex driver, for two reasons.
>>
>> a) we need to make sure the reset helper gets to do its job before the
>> mwifiex driver scans the SDIO bus, and
>>
>> b) the reset helper needs to be called when the mmc host controller wants
>> to do a card reset.
>>
>> Hence, we'll need some sort of internal API for this, and a phandle in dts. I
>> wonder whether that glue logic might be better off living in the mmc core, as
>> mwifiex might well be interfaced to other hosts?
>
> I may have missed something, but doesn't the MMC_POWER_OFF and MMC_POWER_ON|UP handling in controller driver help?
> Anyway the clocks/GPIOs/regulators are sort of platform dependent. Would it be better putting it in /arch/arm/mach-xxxxx/?

what about usb? the mwifiex can also be connected via usb to the host,
isn't the reset
logic the same in that case, independent of sdio

2014-07-01 07:03:58

by James Cameron

[permalink] [raw]
Subject: Re: mwifiex card reset

On Tue, Jul 01, 2014 at 12:02:25AM -0700, Bing Zhao wrote:
> Hi James,
>
> > On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
> > > I may have missed something, but doesn't the MMC_POWER_OFF and
> > > MMC_POWER_ON|UP handling in controller driver help?
> > > Anyway the clocks/GPIOs/regulators are sort of platform dependent.
> > > Would it be better putting it in /arch/arm/mach-xxxxx/?
> >
> > Wouldn't device tree for mmc be better?
>
> Yes, device tree is better for defining clocks/GPIO/regulators, etc.
> But I guess the reset logic (making use of these definitions) cannot
> be device tree?

I think the reset logic can exist, but does nothing unless the device
tree definitions are present.

--
James Cameron
http://quozl.linux.org.au/

2014-07-01 06:44:37

by Bing Zhao

[permalink] [raw]
Subject: RE: mwifiex card reset

SGkgRGFuaWVsLA0KDQo+ID4gT0sgc28gd2UgY2FuJ3QgZG8gbXVjaCBhbnl0aGluZyBhdCBtd2lm
aWV4IHByb2JlIHRpbWUgb24gU0RJTyBidXMNCj4gPiBiZWNhdXNlIGl0IHdvbid0IHByb2JlIHVu
bGVzcyB0aGUgcGlucyBhcmUgY29uZmlndXJlZC4NCj4gPg0KPiA+IE1heWJlIHdlIHNob3VsZCBo
YXZlIGEgc2VwYXJhdGUgbXdpZmlleC1wb3dlciBoZWxwZXIgbW9kdWxlIHRoYXQganVzdA0KPiA+
IG1hbmFnZXMgdGhlIHJlc2V0L2lkbGUvcmVndWxhdG9yIHBpbnM/IFRoZW4gbXdpZmlleC1yZXNl
dCBjYW4gYmUNCj4gPiBhbHdheXMgbG9hZGVkIGFuZCBjb25maWd1cmUgdGhpbmdzIHNvIG13aWZp
ZXgtc2RpbyBjYW4gcHJvYmUgcHJvcGVybHkuDQo+ID4NCj4gPiBBbmQgbXdpZmlleC1yZXNldCBo
ZWxwZXIgY2FuIGFsc28gcHJvdmlkZSB0aGUgdXNlciBzcGFjZSBpbnRlcmZhY2VzDQo+ID4gZm9y
IHRoZSByZXNldC9pZGxlL3JlZ3VsYXRvciBwaW5zLg0KPiANCj4gWWVzLCBhIGhlbHBlciBtaWdo
dCBiZSB0aGUgYmVzdCBzb2x1dGlvbi4gSXQgY291bGQgZXZlbiBiZSBhIGdlbmVyaWMgb25lLCB0
aGF0DQo+IGp1c3QgdGFrZXMgYW55IG51bWJlciBvZiBjbG9ja3MsIHJlc2V0IEdQSU9zIGFuZCBy
ZWd1bGF0b3JzIGFuZCB0YWtlcyBjYXJlDQo+IGZvciBzZXF1ZW5jaW5nIHRoZW0uIEhvd2V2ZXIs
IHdlIG5lZWQgdG8gcmVmZXJlbmNlIHRoYXQgaGVscGVyIGZyb20gdGhlDQo+IG13aWZpZXggZHJp
dmVyLCBmb3IgdHdvIHJlYXNvbnMuDQo+IA0KPiBhKSB3ZSBuZWVkIHRvIG1ha2Ugc3VyZSB0aGUg
cmVzZXQgaGVscGVyIGdldHMgdG8gZG8gaXRzIGpvYiBiZWZvcmUgdGhlDQo+IG13aWZpZXggZHJp
dmVyIHNjYW5zIHRoZSBTRElPIGJ1cywgYW5kDQo+IA0KPiBiKSB0aGUgcmVzZXQgaGVscGVyIG5l
ZWRzIHRvIGJlIGNhbGxlZCB3aGVuIHRoZSBtbWMgaG9zdCBjb250cm9sbGVyIHdhbnRzDQo+IHRv
IGRvIGEgY2FyZCByZXNldC4NCj4gDQo+IEhlbmNlLCB3ZSdsbCBuZWVkIHNvbWUgc29ydCBvZiBp
bnRlcm5hbCBBUEkgZm9yIHRoaXMsIGFuZCBhIHBoYW5kbGUgaW4gZHRzLiBJDQo+IHdvbmRlciB3
aGV0aGVyIHRoYXQgZ2x1ZSBsb2dpYyBtaWdodCBiZSBiZXR0ZXIgb2ZmIGxpdmluZyBpbiB0aGUg
bW1jIGNvcmUsIGFzDQo+IG13aWZpZXggbWlnaHQgd2VsbCBiZSBpbnRlcmZhY2VkIHRvIG90aGVy
IGhvc3RzPw0KDQpJIG1heSBoYXZlIG1pc3NlZCBzb21ldGhpbmcsIGJ1dCBkb2Vzbid0IHRoZSBN
TUNfUE9XRVJfT0ZGIGFuZCBNTUNfUE9XRVJfT058VVAgaGFuZGxpbmcgaW4gY29udHJvbGxlciBk
cml2ZXIgaGVscD8NCkFueXdheSB0aGUgY2xvY2tzL0dQSU9zL3JlZ3VsYXRvcnMgYXJlIHNvcnQg
b2YgcGxhdGZvcm0gZGVwZW5kZW50LiBXb3VsZCBpdCBiZSBiZXR0ZXIgcHV0dGluZyBpdCBpbiAv
YXJjaC9hcm0vbWFjaC14eHh4eC8/DQoNClRoYW5rcywNCkJpbmcNCg0KPiANCj4gDQo+IFRoYW5r
cywNCj4gRGFuaWVsDQoNCg==

2014-07-01 07:41:13

by Tony Lindgren

[permalink] [raw]
Subject: Re: mwifiex card reset

* James Cameron <[email protected]> [140701 10:05]:
> On Tue, Jul 01, 2014 at 12:02:25AM -0700, Bing Zhao wrote:
> > Hi James,
> >
> > > On Mon, Jun 30, 2014 at 11:44:29PM -0700, Bing Zhao wrote:
> > > > I may have missed something, but doesn't the MMC_POWER_OFF and
> > > > MMC_POWER_ON|UP handling in controller driver help?
> > > > Anyway the clocks/GPIOs/regulators are sort of platform dependent.
> > > > Would it be better putting it in /arch/arm/mach-xxxxx/?
> > >
> > > Wouldn't device tree for mmc be better?
> >
> > Yes, device tree is better for defining clocks/GPIO/regulators, etc.
> > But I guess the reset logic (making use of these definitions) cannot
> > be device tree?
>
> I think the reset logic can exist, but does nothing unless the device
> tree definitions are present.

It might be possible to support the SDIO card specific
clocks/GPIOs/regulators the MMC bus driver. But that may not
work well in the long run as those pins are not connected to
the MMC bus at all. If we wanted to explore adding card
specific features to the MMC bus, I guess we should have:

- Card reset-gpios

- Card specific regulators on the card controlled by
a GPIO

- External card specific regulators

- Card specific idle status pin (no idea what these pins
do on some of the mwifiex cards though?)

And then these would have to be configured to get the
SDIO card to probe. And they should be controllable also
from user space to reset a card or to power it off.

Then if we get a device that muxes two different SDIO cards
on the same bus, then what do we do? They may both have
their own regulators and reset GPIO pins.

Regards,

Tony