All of lore.kernel.org
 help / color / mirror / Atom feed
* mwifiex card reset
@ 2014-06-27 14:39 ` Andreas Fenkart
  0 siblings, 0 replies; 25+ messages in thread
From: Andreas Fenkart @ 2014-06-27 14:39 UTC (permalink / raw)
  To: Bing Zhao, Ulf Hansson, linux-wireless, linux-mmc, devicetree
  Cc: Tony Lindgren, Daniel Mack

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?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* mwifiex card reset
@ 2014-06-27 14:39 ` Andreas Fenkart
  0 siblings, 0 replies; 25+ messages in thread
From: Andreas Fenkart @ 2014-06-27 14:39 UTC (permalink / raw)
  To: Bing Zhao, Ulf Hansson, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Tony Lindgren, Daniel Mack

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
@ 2014-06-28  5:22   ` James Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: James Cameron @ 2014-06-28  5:22 UTC (permalink / raw)
  To: Andreas Fenkart
  Cc: Bing Zhao, Ulf Hansson, linux-wireless, linux-mmc, devicetree,
	Tony Lindgren, Daniel Mack

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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
@ 2014-06-28  5:22   ` James Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: James Cameron @ 2014-06-28  5:22 UTC (permalink / raw)
  To: Andreas Fenkart
  Cc: Bing Zhao, Ulf Hansson, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc, devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
	Daniel Mack

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
@ 2014-06-28  7:23     ` Tony Lindgren
  0 siblings, 0 replies; 25+ messages in thread
From: Tony Lindgren @ 2014-06-28  7:23 UTC (permalink / raw)
  To: James Cameron
  Cc: Andreas Fenkart, Bing Zhao, Ulf Hansson, linux-wireless,
	linux-mmc, devicetree, Daniel Mack

* James Cameron <quozl@laptop.org> [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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
@ 2014-06-28  7:23     ` Tony Lindgren
  0 siblings, 0 replies; 25+ messages in thread
From: Tony Lindgren @ 2014-06-28  7:23 UTC (permalink / raw)
  To: James Cameron
  Cc: Andreas Fenkart, Bing Zhao, Ulf Hansson,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, linux-mmc,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Daniel Mack

* James Cameron <quozl-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> [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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-06-28  7:23     ` Tony Lindgren
  (?)
@ 2014-06-29  9:41     ` Andreas Fenkart
  2014-06-30  6:19       ` Tony Lindgren
  -1 siblings, 1 reply; 25+ messages in thread
From: Andreas Fenkart @ 2014-06-29  9:41 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: James Cameron, Bing Zhao, Ulf Hansson, linux-wireless, linux-mmc,
	devicetree, Daniel Mack

2014-06-28 9:23 GMT+02:00 Tony Lindgren <tony@atomide.com>:
> * James Cameron <quozl@laptop.org> [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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-06-29  9:41     ` Andreas Fenkart
@ 2014-06-30  6:19       ` Tony Lindgren
  2014-06-30 19:30         ` Daniel Mack
  0 siblings, 1 reply; 25+ messages in thread
From: Tony Lindgren @ 2014-06-30  6:19 UTC (permalink / raw)
  To: Andreas Fenkart
  Cc: James Cameron, Bing Zhao, Ulf Hansson, linux-wireless, linux-mmc,
	devicetree, Daniel Mack

* Andreas Fenkart <afenkart@gmail.com> [140629 12:43]:
> 2014-06-28 9:23 GMT+02:00 Tony Lindgren <tony@atomide.com>:
> > * James Cameron <quozl@laptop.org> [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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-06-30  6:19       ` Tony Lindgren
@ 2014-06-30 19:30         ` Daniel Mack
  2014-07-01  6:44             ` Bing Zhao
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Mack @ 2014-06-30 19:30 UTC (permalink / raw)
  To: Tony Lindgren, Andreas Fenkart
  Cc: James Cameron, Bing Zhao, Ulf Hansson, linux-wireless, linux-mmc,
	devicetree

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 <afenkart@gmail.com> [140629 12:43]:
>> 2014-06-28 9:23 GMT+02:00 Tony Lindgren <tony@atomide.com>:
>>> * James Cameron <quozl@laptop.org> [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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: mwifiex card reset
@ 2014-07-01  6:44             ` Bing Zhao
  0 siblings, 0 replies; 25+ messages in thread
From: Bing Zhao @ 2014-07-01  6:44 UTC (permalink / raw)
  To: Daniel Mack, Tony Lindgren, Andreas Fenkart
  Cc: James Cameron, Ulf Hansson, linux-wireless, linux-mmc, devicetree

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: mwifiex card reset
@ 2014-07-01  6:44             ` Bing Zhao
  0 siblings, 0 replies; 25+ messages in thread
From: Bing Zhao @ 2014-07-01  6:44 UTC (permalink / raw)
  To: Daniel Mack, Tony Lindgren, Andreas Fenkart
  Cc: James Cameron, Ulf Hansson,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, linux-mmc,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Daniel,

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

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

Thanks,
Bing

> 
> 
> Thanks,
> Daniel


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01  6:44             ` Bing Zhao
  (?)
@ 2014-07-01  6:57             ` James Cameron
  2014-07-01  7:02                 ` Bing Zhao
  2014-07-01 12:20                 ` Yuvaraj Cd
  -1 siblings, 2 replies; 25+ messages in thread
From: James Cameron @ 2014-07-01  6:57 UTC (permalink / raw)
  To: Bing Zhao
  Cc: Daniel Mack, Tony Lindgren, Andreas Fenkart, Ulf Hansson,
	linux-wireless, linux-mmc, devicetree

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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: mwifiex card reset
@ 2014-07-01  7:02                 ` Bing Zhao
  0 siblings, 0 replies; 25+ messages in thread
From: Bing Zhao @ 2014-07-01  7:02 UTC (permalink / raw)
  To: quozl
  Cc: Daniel Mack, Tony Lindgren, Andreas Fenkart, Ulf Hansson,
	linux-wireless, linux-mmc, devicetree

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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: mwifiex card reset
@ 2014-07-01  7:02                 ` Bing Zhao
  0 siblings, 0 replies; 25+ messages in thread
From: Bing Zhao @ 2014-07-01  7:02 UTC (permalink / raw)
  To: quozl-2X9k7bc8m7Mdnm+yROfE0A
  Cc: Daniel Mack, Tony Lindgren, Andreas Fenkart, Ulf Hansson,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, linux-mmc,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01  7:02                 ` Bing Zhao
  (?)
@ 2014-07-01  7:03                 ` James Cameron
  2014-07-01  7:41                     ` Tony Lindgren
  -1 siblings, 1 reply; 25+ messages in thread
From: James Cameron @ 2014-07-01  7:03 UTC (permalink / raw)
  To: Bing Zhao
  Cc: Daniel Mack, Tony Lindgren, Andreas Fenkart, Ulf Hansson,
	linux-wireless, linux-mmc, devicetree

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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
@ 2014-07-01  7:41                     ` Tony Lindgren
  0 siblings, 0 replies; 25+ messages in thread
From: Tony Lindgren @ 2014-07-01  7:41 UTC (permalink / raw)
  To: James Cameron
  Cc: Bing Zhao, Daniel Mack, Andreas Fenkart, Ulf Hansson,
	linux-wireless, linux-mmc, devicetree

* James Cameron <quozl@laptop.org> [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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
@ 2014-07-01  7:41                     ` Tony Lindgren
  0 siblings, 0 replies; 25+ messages in thread
From: Tony Lindgren @ 2014-07-01  7:41 UTC (permalink / raw)
  To: James Cameron
  Cc: Bing Zhao, Daniel Mack, Andreas Fenkart, Ulf Hansson,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, linux-mmc,
	devicetree-u79uwXL29TY76Z2rM5mHXA

* James Cameron <quozl-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> [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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01  6:44             ` Bing Zhao
  (?)
  (?)
@ 2014-07-01  7:52             ` Daniel Mack
  -1 siblings, 0 replies; 25+ messages in thread
From: Daniel Mack @ 2014-07-01  7:52 UTC (permalink / raw)
  To: Bing Zhao, Tony Lindgren, Andreas Fenkart
  Cc: James Cameron, Ulf Hansson, linux-wireless, linux-mmc, devicetree

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01  6:44             ` Bing Zhao
                               ` (2 preceding siblings ...)
  (?)
@ 2014-07-01  8:44             ` Andreas Fenkart
  -1 siblings, 0 replies; 25+ messages in thread
From: Andreas Fenkart @ 2014-07-01  8:44 UTC (permalink / raw)
  To: Bing Zhao
  Cc: Daniel Mack, Tony Lindgren, James Cameron, Ulf Hansson,
	linux-wireless, linux-mmc, devicetree

Hi Bing,

2014-07-01 8:44 GMT+02:00 Bing Zhao <bzhao@marvell.com>:
[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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01  6:57             ` James Cameron
@ 2014-07-01 12:20                 ` Yuvaraj Cd
  2014-07-01 12:20                 ` Yuvaraj Cd
  1 sibling, 0 replies; 25+ messages in thread
From: Yuvaraj Cd @ 2014-07-01 12:20 UTC (permalink / raw)
  To: James Cameron
  Cc: Bing Zhao, Daniel Mack, Tony Lindgren, Andreas Fenkart,
	Ulf Hansson, linux-wireless, linux-mmc, devicetree,
	linux-arm-kernel, linux-samsung-soc, Douglas Anderson,
	Tomasz Figa, sunil joshi, prashanth.g

On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <quozl@laptop.org> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* mwifiex card reset
@ 2014-07-01 12:20                 ` Yuvaraj Cd
  0 siblings, 0 replies; 25+ messages in thread
From: Yuvaraj Cd @ 2014-07-01 12:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <quozl@laptop.org> 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 majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01 12:20                 ` Yuvaraj Cd
@ 2014-07-01 15:09                   ` Doug Anderson
  -1 siblings, 0 replies; 25+ messages in thread
From: Doug Anderson @ 2014-07-01 15:09 UTC (permalink / raw)
  To: Yuvaraj Cd, Olof Johansson
  Cc: James Cameron, Bing Zhao, Daniel Mack, Tony Lindgren,
	Andreas Fenkart, Ulf Hansson, linux-wireless, linux-mmc,
	devicetree, linux-arm-kernel, linux-samsung-soc, Tomasz Figa,
	sunil joshi, Prashanth G

+Olof who posted the patch that Yuvaraj referenced.

On Tue, Jul 1, 2014 at 5:20 AM, Yuvaraj Cd <yuvaraj.lkml@gmail.com> wrote:
> On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <quozl@laptop.org> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* mwifiex card reset
@ 2014-07-01 15:09                   ` Doug Anderson
  0 siblings, 0 replies; 25+ messages in thread
From: Doug Anderson @ 2014-07-01 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

+Olof who posted the patch that Yuvaraj referenced.

On Tue, Jul 1, 2014 at 5:20 AM, Yuvaraj Cd <yuvaraj.lkml@gmail.com> wrote:
> On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <quozl@laptop.org> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: mwifiex card reset
  2014-07-01 15:09                   ` Doug Anderson
@ 2014-07-15 13:25                     ` Andreas Fenkart
  -1 siblings, 0 replies; 25+ messages in thread
From: Andreas Fenkart @ 2014-07-15 13:25 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Yuvaraj Cd, Olof Johansson, James Cameron, Bing Zhao,
	Daniel Mack, Tony Lindgren, Ulf Hansson, linux-wireless,
	linux-mmc, devicetree, linux-arm-kernel, linux-samsung-soc,
	Tomasz Figa, sunil joshi, Prashanth G

2014-07-01 17:09 GMT+02:00 Doug Anderson <dianders@chromium.org>:
> +Olof who posted the patch that Yuvaraj referenced.
>
> On Tue, Jul 1, 2014 at 5:20 AM, Yuvaraj Cd <yuvaraj.lkml@gmail.com> wrote:
>> On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <quozl@laptop.org> 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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* mwifiex card reset
@ 2014-07-15 13:25                     ` Andreas Fenkart
  0 siblings, 0 replies; 25+ messages in thread
From: Andreas Fenkart @ 2014-07-15 13:25 UTC (permalink / raw)
  To: linux-arm-kernel

2014-07-01 17:09 GMT+02:00 Doug Anderson <dianders@chromium.org>:
> +Olof who posted the patch that Yuvaraj referenced.
>
> On Tue, Jul 1, 2014 at 5:20 AM, Yuvaraj Cd <yuvaraj.lkml@gmail.com> wrote:
>> On Tue, Jul 1, 2014 at 12:27 PM, James Cameron <quozl@laptop.org> 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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2014-07-15 13:25 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-27 14:39 mwifiex card reset Andreas Fenkart
2014-06-27 14:39 ` Andreas Fenkart
2014-06-28  5:22 ` James Cameron
2014-06-28  5:22   ` James Cameron
2014-06-28  7:23   ` Tony Lindgren
2014-06-28  7:23     ` Tony Lindgren
2014-06-29  9:41     ` Andreas Fenkart
2014-06-30  6:19       ` Tony Lindgren
2014-06-30 19:30         ` Daniel Mack
2014-07-01  6:44           ` Bing Zhao
2014-07-01  6:44             ` Bing Zhao
2014-07-01  6:57             ` James Cameron
2014-07-01  7:02               ` Bing Zhao
2014-07-01  7:02                 ` Bing Zhao
2014-07-01  7:03                 ` James Cameron
2014-07-01  7:41                   ` Tony Lindgren
2014-07-01  7:41                     ` Tony Lindgren
2014-07-01 12:20               ` Yuvaraj Cd
2014-07-01 12:20                 ` Yuvaraj Cd
2014-07-01 15:09                 ` Doug Anderson
2014-07-01 15:09                   ` Doug Anderson
2014-07-15 13:25                   ` Andreas Fenkart
2014-07-15 13:25                     ` Andreas Fenkart
2014-07-01  7:52             ` Daniel Mack
2014-07-01  8:44             ` Andreas Fenkart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.