All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: brcm 4329 problems
       [not found]                 ` <20140209205917.GO26684@n2100.arm.linux.org.uk>
@ 2014-02-09 23:27                   ` Russell King - ARM Linux
  2014-02-10  6:50                     ` Dong Aisheng
  2014-02-10 10:07                     ` Arend van Spriel
  0 siblings, 2 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2014-02-09 23:27 UTC (permalink / raw)
  To: Arend van Spriel, Chris Ball, linux-mmc; +Cc: brcm80211-dev-list

On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
> > Yeah. I did not mention this, but indeed the first log you provided
> > already made that clear (to me). In your last log the driver sends an UP
> > command to the firmware on which no response is given. So I was hoping
> > the forensics file (which is firmware console buffer) would show an
> > error message of some kind. Also that is not the case. Have to come up
> > with new ideas about what is going wrong here.
> 
> I'm chasing a theory at the moment, but it's being complicated by the
> driver oopsing on unload...

Theory proven.

May I first take the time to apologise to Arend for wasting his time with
this issue; the issue is not the Broadcom driver, but the SDHCI driver.

My theory was that it's the sdhci driver causing the problems...  My
suspicions were first raised when I read through various SDHCI driver
functions such as the set_ios methods when chasing down a problem with
UHS-1 SD cards, and later when I was reading it's interrupt handling
code.

The driver looks very much like a patchwork quilt of different hacks,
all trying to co-operate with each other in the semblence of something
working - the result is something which does stuff in ways that the SD
card spec doesn't allow, but also does some pretty stupid things when
you have a SDIO device attached.

The SDIO problems become pretty obvious when you see this log:

[   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
[   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
[   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
[   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
[   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
[   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
[   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
[   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
[   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
[   51.128540] brcmfmac: brcmf_sdio_htclk Enter
[   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
[   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
[   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
[   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
[   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
[   51.180385] mmc0: runtime suspend
[   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
[   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
[   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
[   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
[   53.118319] brcmfmac: brcmf_sdio_htclk Enter
[   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
[   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
[   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
[   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
[   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
[   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
[   53.125898] mmc0: runtime resume
[   53.125910] mmc0: card irq raised
[   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
[   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
[   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1

The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
register values immediately before the stated action is taken bit 8 is
the interrupt enable for card interrupts.  Earlier in the log, SDIO card
interrupts were enabled (one was handled immediately before the above
broadcom cmd=2 message was sent.

Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
disabled by a runtime suspend - including any interrupt from the card.
The brcmfmac driver times out after 2 seconds having sent the "up"
command, and re-awakens the host, which is runtime resumed at 53.125898,
enabling the SDIO card interrupt at that time.

And lo and behold - the card has an interrupt pending!  Too bad, we're
too late for the driver to forward the interrupt to the SDIO interrupt
thread and get it to the driver before the time-out is processed.

Here's the proof - the above messages came from:

int sdhci_runtime_suspend_host(struct sdhci_host *host)
{
        unsigned long flags;
        int ret = 0;
        
printk(KERN_DEBUG "%s: runtime suspend\n",
        mmc_hostname(host->mmc));
...
        spin_lock_irqsave(&host->lock, flags);
        sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
        spin_unlock_irqrestore(&host->lock, flags);

int sdhci_runtime_resume_host(struct sdhci_host *host)
{
        unsigned long flags;
        int ret = 0, host_flags = host->flags;
...
        /* Enable SDIO IRQ */
        if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
                sdhci_enable_sdio_irq_nolock(host, true);
        
        /* Enable Card Detection */
        sdhci_enable_card_detection(host);
         
printk(KERN_DEBUG "%s: runtime resume\n",
        mmc_hostname(host->mmc));
        spin_unlock_irqrestore(&host->lock, flags);
        
        return ret;   
}
EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);

To me, it looks like SDHCI needs a major rework...  And there needs to
be some recognition that - maybe - leaving SDIO interrupts enabled even
though we may want the host to enter a low power mode is something that's
really very very desirable...

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

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

* Re: brcm 4329 problems
  2014-02-09 23:27                   ` brcm 4329 problems Russell King - ARM Linux
@ 2014-02-10  6:50                     ` Dong Aisheng
  2014-02-10 14:03                       ` Russell King - ARM Linux
  2014-02-10 10:07                     ` Arend van Spriel
  1 sibling, 1 reply; 16+ messages in thread
From: Dong Aisheng @ 2014-02-10  6:50 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arend van Spriel, Chris Ball, linux-mmc, brcm80211-dev-list

On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>> > Yeah. I did not mention this, but indeed the first log you provided
>> > already made that clear (to me). In your last log the driver sends an UP
>> > command to the firmware on which no response is given. So I was hoping
>> > the forensics file (which is firmware console buffer) would show an
>> > error message of some kind. Also that is not the case. Have to come up
>> > with new ideas about what is going wrong here.
>>
>> I'm chasing a theory at the moment, but it's being complicated by the
>> driver oopsing on unload...
>
> Theory proven.
>
> May I first take the time to apologise to Arend for wasting his time with
> this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>
> My theory was that it's the sdhci driver causing the problems...  My
> suspicions were first raised when I read through various SDHCI driver
> functions such as the set_ios methods when chasing down a problem with
> UHS-1 SD cards, and later when I was reading it's interrupt handling
> code.
>
> The driver looks very much like a patchwork quilt of different hacks,
> all trying to co-operate with each other in the semblence of something
> working - the result is something which does stuff in ways that the SD
> card spec doesn't allow, but also does some pretty stupid things when
> you have a SDIO device attached.
>
> The SDIO problems become pretty obvious when you see this log:
>
> [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
> [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
> [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
> [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
> [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
> [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
> [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
> [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
> [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
> [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
> [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
> [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
> [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
> [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
> [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
> [   51.180385] mmc0: runtime suspend
> [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
> [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
> [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
> [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
> [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
> [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
> [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
> [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
> [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
> [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
> [   53.125898] mmc0: runtime resume
> [   53.125910] mmc0: card irq raised
> [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
> [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
> [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>
> The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
> register values immediately before the stated action is taken bit 8 is
> the interrupt enable for card interrupts.  Earlier in the log, SDIO card
> interrupts were enabled (one was handled immediately before the above
> broadcom cmd=2 message was sent.
>
> Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
> disabled by a runtime suspend - including any interrupt from the card.
> The brcmfmac driver times out after 2 seconds having sent the "up"
> command, and re-awakens the host, which is runtime resumed at 53.125898,
> enabling the SDIO card interrupt at that time.
>
> And lo and behold - the card has an interrupt pending!  Too bad, we're
> too late for the driver to forward the interrupt to the SDIO interrupt
> thread and get it to the driver before the time-out is processed.
>
> Here's the proof - the above messages came from:
>
> int sdhci_runtime_suspend_host(struct sdhci_host *host)
> {
>         unsigned long flags;
>         int ret = 0;
>
> printk(KERN_DEBUG "%s: runtime suspend\n",
>         mmc_hostname(host->mmc));
> ...
>         spin_lock_irqsave(&host->lock, flags);
>         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>         spin_unlock_irqrestore(&host->lock, flags);
>
> int sdhci_runtime_resume_host(struct sdhci_host *host)
> {
>         unsigned long flags;
>         int ret = 0, host_flags = host->flags;
> ...
>         /* Enable SDIO IRQ */
>         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>                 sdhci_enable_sdio_irq_nolock(host, true);
>
>         /* Enable Card Detection */
>         sdhci_enable_card_detection(host);
>
> printk(KERN_DEBUG "%s: runtime resume\n",
>         mmc_hostname(host->mmc));
>         spin_unlock_irqrestore(&host->lock, flags);
>
>         return ret;
> }
> EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>
> To me, it looks like SDHCI needs a major rework...  And there needs to
> be some recognition that - maybe - leaving SDIO interrupts enabled even
> though we may want the host to enter a low power mode is something that's
> really very very desirable...
>

I'm not quite clear about your issue.
But it seems your issue is caused by runtime pm disabling the
interrupt & clocks as you said.

Can you try the patch i mentioned here:
http://www.spinics.net/lists/linux-mmc/msg24764.html

That will prevent the host to do runtime pm for SDIO devices.

Regards
Dong Aisheng

> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
> --
> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-09 23:27                   ` brcm 4329 problems Russell King - ARM Linux
  2014-02-10  6:50                     ` Dong Aisheng
@ 2014-02-10 10:07                     ` Arend van Spriel
  1 sibling, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2014-02-10 10:07 UTC (permalink / raw)
  To: Russell King - ARM Linux, Chris Ball, linux-mmc; +Cc: brcm80211-dev-list

On 02/10/2014 12:27 AM, Russell King - ARM Linux wrote:
> On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>> Yeah. I did not mention this, but indeed the first log you provided
>>> already made that clear (to me). In your last log the driver sends an UP
>>> command to the firmware on which no response is given. So I was hoping
>>> the forensics file (which is firmware console buffer) would show an
>>> error message of some kind. Also that is not the case. Have to come up
>>> with new ideas about what is going wrong here.
>>
>> I'm chasing a theory at the moment, but it's being complicated by the
>> driver oopsing on unload...
> 
> Theory proven.
> 
> May I first take the time to apologise to Arend for wasting his time with
> this issue; the issue is not the Broadcom driver, but the SDHCI driver.

'Nice' find. There were too many communications going fine to blame it
on generic interrupt issue. I was not aware runtime PM was enabled for
the device by default.

> My theory was that it's the sdhci driver causing the problems...  My
> suspicions were first raised when I read through various SDHCI driver
> functions such as the set_ios methods when chasing down a problem with
> UHS-1 SD cards, and later when I was reading it's interrupt handling
> code.
> 
> The driver looks very much like a patchwork quilt of different hacks,
> all trying to co-operate with each other in the semblence of something
> working - the result is something which does stuff in ways that the SD
> card spec doesn't allow, but also does some pretty stupid things when
> you have a SDIO device attached.

Yeah. It seems development has been driven mostly by SD memory cards as
by SDIO devices.

> The SDIO problems become pretty obvious when you see this log:
> 
> [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
> [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
> [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
> [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
> [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
> [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
> [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
> [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
> [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
> [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
> [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
> [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
> [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
> [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
> [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
> [   51.180385] mmc0: runtime suspend
> [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
> [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
> [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
> [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
> [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
> [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
> [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
> [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
> [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
> [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
> [   53.125898] mmc0: runtime resume
> [   53.125910] mmc0: card irq raised
> [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
> [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
> [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
> 
> The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
> register values immediately before the stated action is taken bit 8 is
> the interrupt enable for card interrupts.  Earlier in the log, SDIO card
> interrupts were enabled (one was handled immediately before the above
> broadcom cmd=2 message was sent.
> 
> Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
> disabled by a runtime suspend - including any interrupt from the card.
> The brcmfmac driver times out after 2 seconds having sent the "up"
> command, and re-awakens the host, which is runtime resumed at 53.125898,
> enabling the SDIO card interrupt at that time.
> 
> And lo and behold - the card has an interrupt pending!  Too bad, we're
> too late for the driver to forward the interrupt to the SDIO interrupt
> thread and get it to the driver before the time-out is processed.
> 
> Here's the proof - the above messages came from:
> 
> int sdhci_runtime_suspend_host(struct sdhci_host *host)
> {
>         unsigned long flags;
>         int ret = 0;
>         
> printk(KERN_DEBUG "%s: runtime suspend\n",
>         mmc_hostname(host->mmc));
> ...
>         spin_lock_irqsave(&host->lock, flags);
>         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>         spin_unlock_irqrestore(&host->lock, flags);
> 
> int sdhci_runtime_resume_host(struct sdhci_host *host)
> {
>         unsigned long flags;
>         int ret = 0, host_flags = host->flags;
> ...
>         /* Enable SDIO IRQ */
>         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>                 sdhci_enable_sdio_irq_nolock(host, true);
>         
>         /* Enable Card Detection */
>         sdhci_enable_card_detection(host);
>          
> printk(KERN_DEBUG "%s: runtime resume\n",
>         mmc_hostname(host->mmc));
>         spin_unlock_irqrestore(&host->lock, flags);
>         
>         return ret;   
> }
> EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
> 
> To me, it looks like SDHCI needs a major rework...  And there needs to
> be some recognition that - maybe - leaving SDIO interrupts enabled even
> though we may want the host to enter a low power mode is something that's
> really very very desirable...

The brcmfmac driver does not support runtime PM at the moment.
Should/could it explicitly disable runtime PM for the host?

Regards,
Arend

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

* Re: brcm 4329 problems
  2014-02-10  6:50                     ` Dong Aisheng
@ 2014-02-10 14:03                       ` Russell King - ARM Linux
  2014-02-10 14:19                         ` Arend van Spriel
                                           ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2014-02-10 14:03 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: Arend van Spriel, Chris Ball, linux-mmc, brcm80211-dev-list

On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
> >> > Yeah. I did not mention this, but indeed the first log you provided
> >> > already made that clear (to me). In your last log the driver sends an UP
> >> > command to the firmware on which no response is given. So I was hoping
> >> > the forensics file (which is firmware console buffer) would show an
> >> > error message of some kind. Also that is not the case. Have to come up
> >> > with new ideas about what is going wrong here.
> >>
> >> I'm chasing a theory at the moment, but it's being complicated by the
> >> driver oopsing on unload...
> >
> > Theory proven.
> >
> > May I first take the time to apologise to Arend for wasting his time with
> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
> >
> > My theory was that it's the sdhci driver causing the problems...  My
> > suspicions were first raised when I read through various SDHCI driver
> > functions such as the set_ios methods when chasing down a problem with
> > UHS-1 SD cards, and later when I was reading it's interrupt handling
> > code.
> >
> > The driver looks very much like a patchwork quilt of different hacks,
> > all trying to co-operate with each other in the semblence of something
> > working - the result is something which does stuff in ways that the SD
> > card spec doesn't allow, but also does some pretty stupid things when
> > you have a SDIO device attached.
> >
> > The SDIO problems become pretty obvious when you see this log:
> >
> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
> > [   51.180385] mmc0: runtime suspend
> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
> > [   53.125898] mmc0: runtime resume
> > [   53.125910] mmc0: card irq raised
> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
> >
> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
> > register values immediately before the stated action is taken bit 8 is
> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
> > interrupts were enabled (one was handled immediately before the above
> > broadcom cmd=2 message was sent.
> >
> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
> > disabled by a runtime suspend - including any interrupt from the card.
> > The brcmfmac driver times out after 2 seconds having sent the "up"
> > command, and re-awakens the host, which is runtime resumed at 53.125898,
> > enabling the SDIO card interrupt at that time.
> >
> > And lo and behold - the card has an interrupt pending!  Too bad, we're
> > too late for the driver to forward the interrupt to the SDIO interrupt
> > thread and get it to the driver before the time-out is processed.
> >
> > Here's the proof - the above messages came from:
> >
> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
> > {
> >         unsigned long flags;
> >         int ret = 0;
> >
> > printk(KERN_DEBUG "%s: runtime suspend\n",
> >         mmc_hostname(host->mmc));
> > ...
> >         spin_lock_irqsave(&host->lock, flags);
> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
> >         spin_unlock_irqrestore(&host->lock, flags);
> >
> > int sdhci_runtime_resume_host(struct sdhci_host *host)
> > {
> >         unsigned long flags;
> >         int ret = 0, host_flags = host->flags;
> > ...
> >         /* Enable SDIO IRQ */
> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
> >                 sdhci_enable_sdio_irq_nolock(host, true);
> >
> >         /* Enable Card Detection */
> >         sdhci_enable_card_detection(host);
> >
> > printk(KERN_DEBUG "%s: runtime resume\n",
> >         mmc_hostname(host->mmc));
> >         spin_unlock_irqrestore(&host->lock, flags);
> >
> >         return ret;
> > }
> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
> >
> > To me, it looks like SDHCI needs a major rework...  And there needs to
> > be some recognition that - maybe - leaving SDIO interrupts enabled even
> > though we may want the host to enter a low power mode is something that's
> > really very very desirable...
> >
> 
> I'm not quite clear about your issue.
> But it seems your issue is caused by runtime pm disabling the
> interrupt & clocks as you said.
> 
> Can you try the patch i mentioned here:
> http://www.spinics.net/lists/linux-mmc/msg24764.html
> 
> That will prevent the host to do runtime pm for SDIO devices.

Why not allow runtime PM at the host level, but still allow the
interrupt to be received?  Doesn't it make sense to allow PM even with
SDIO cards in place?

Once I get imx-drm off my plate, I'm going to put some work into
rewriting the sdhci driver mess in a much cleaner way - we can't go on
putting hacks on top of what's already there, it's already a total mess.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

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

* Re: brcm 4329 problems
  2014-02-10 14:03                       ` Russell King - ARM Linux
@ 2014-02-10 14:19                         ` Arend van Spriel
  2014-02-10 15:25                         ` Ulf Hansson
  2014-02-11  7:33                         ` Dong Aisheng
  2 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2014-02-10 14:19 UTC (permalink / raw)
  To: Russell King - ARM Linux, Dong Aisheng
  Cc: Chris Ball, linux-mmc, brcm80211-dev-list

On 02/10/2014 03:03 PM, Russell King - ARM Linux wrote:
> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>>> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>>>> Yeah. I did not mention this, but indeed the first log you provided
>>>>> already made that clear (to me). In your last log the driver sends an UP
>>>>> command to the firmware on which no response is given. So I was hoping
>>>>> the forensics file (which is firmware console buffer) would show an
>>>>> error message of some kind. Also that is not the case. Have to come up
>>>>> with new ideas about what is going wrong here.
>>>>
>>>> I'm chasing a theory at the moment, but it's being complicated by the
>>>> driver oopsing on unload...
>>>
>>> Theory proven.
>>>
>>> May I first take the time to apologise to Arend for wasting his time with
>>> this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>>
>>> My theory was that it's the sdhci driver causing the problems...  My
>>> suspicions were first raised when I read through various SDHCI driver
>>> functions such as the set_ios methods when chasing down a problem with
>>> UHS-1 SD cards, and later when I was reading it's interrupt handling
>>> code.
>>>
>>> The driver looks very much like a patchwork quilt of different hacks,
>>> all trying to co-operate with each other in the semblence of something
>>> working - the result is something which does stuff in ways that the SD
>>> card spec doesn't allow, but also does some pretty stupid things when
>>> you have a SDIO device attached.
>>>
>>> The SDIO problems become pretty obvious when you see this log:
>>>
>>> [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>> [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>> [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>> [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>> [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>> [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>> [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>> [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>> [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>> [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>> [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>> [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>> [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>> [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>> [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>> [   51.180385] mmc0: runtime suspend
>>> [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>> [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>> [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>> [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>> [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>> [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>> [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>> [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>> [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>> [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>> [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>> [   53.125898] mmc0: runtime resume
>>> [   53.125910] mmc0: card irq raised
>>> [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>> [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>> [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>>
>>> The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>> register values immediately before the stated action is taken bit 8 is
>>> the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>> interrupts were enabled (one was handled immediately before the above
>>> broadcom cmd=2 message was sent.
>>>
>>> Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>> disabled by a runtime suspend - including any interrupt from the card.
>>> The brcmfmac driver times out after 2 seconds having sent the "up"
>>> command, and re-awakens the host, which is runtime resumed at 53.125898,
>>> enabling the SDIO card interrupt at that time.
>>>
>>> And lo and behold - the card has an interrupt pending!  Too bad, we're
>>> too late for the driver to forward the interrupt to the SDIO interrupt
>>> thread and get it to the driver before the time-out is processed.
>>>
>>> Here's the proof - the above messages came from:
>>>
>>> int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>> {
>>>         unsigned long flags;
>>>         int ret = 0;
>>>
>>> printk(KERN_DEBUG "%s: runtime suspend\n",
>>>         mmc_hostname(host->mmc));
>>> ...
>>>         spin_lock_irqsave(&host->lock, flags);
>>>         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>>         spin_unlock_irqrestore(&host->lock, flags);
>>>
>>> int sdhci_runtime_resume_host(struct sdhci_host *host)
>>> {
>>>         unsigned long flags;
>>>         int ret = 0, host_flags = host->flags;
>>> ...
>>>         /* Enable SDIO IRQ */
>>>         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>>                 sdhci_enable_sdio_irq_nolock(host, true);
>>>
>>>         /* Enable Card Detection */
>>>         sdhci_enable_card_detection(host);
>>>
>>> printk(KERN_DEBUG "%s: runtime resume\n",
>>>         mmc_hostname(host->mmc));
>>>         spin_unlock_irqrestore(&host->lock, flags);
>>>
>>>         return ret;
>>> }
>>> EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>>
>>> To me, it looks like SDHCI needs a major rework...  And there needs to
>>> be some recognition that - maybe - leaving SDIO interrupts enabled even
>>> though we may want the host to enter a low power mode is something that's
>>> really very very desirable...
>>>
>>
>> I'm not quite clear about your issue.
>> But it seems your issue is caused by runtime pm disabling the
>> interrupt & clocks as you said.
>>
>> Can you try the patch i mentioned here:
>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>
>> That will prevent the host to do runtime pm for SDIO devices.
> 
> Why not allow runtime PM at the host level, but still allow the
> interrupt to be received?  Doesn't it make sense to allow PM even with
> SDIO cards in place?
> 
> Once I get imx-drm off my plate, I'm going to put some work into
> rewriting the sdhci driver mess in a much cleaner way - we can't go on
> putting hacks on top of what's already there, it's already a total mess.

You have my vote on that ;-) Just not sure whether you can keep it
limited to sdhci. Ah, well. I will keep an eye (or two) on the mmc list.

Regards,
Arend

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

* Re: brcm 4329 problems
  2014-02-10 14:03                       ` Russell King - ARM Linux
  2014-02-10 14:19                         ` Arend van Spriel
@ 2014-02-10 15:25                         ` Ulf Hansson
  2014-02-11  7:38                           ` Dong Aisheng
  2014-02-11  7:33                         ` Dong Aisheng
  2 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2014-02-10 15:25 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Dong Aisheng, Arend van Spriel, Chris Ball, linux-mmc,
	brcm80211-dev-list

On 10 February 2014 15:03, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>> >> > Yeah. I did not mention this, but indeed the first log you provided
>> >> > already made that clear (to me). In your last log the driver sends an UP
>> >> > command to the firmware on which no response is given. So I was hoping
>> >> > the forensics file (which is firmware console buffer) would show an
>> >> > error message of some kind. Also that is not the case. Have to come up
>> >> > with new ideas about what is going wrong here.
>> >>
>> >> I'm chasing a theory at the moment, but it's being complicated by the
>> >> driver oopsing on unload...
>> >
>> > Theory proven.
>> >
>> > May I first take the time to apologise to Arend for wasting his time with
>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>> >
>> > My theory was that it's the sdhci driver causing the problems...  My
>> > suspicions were first raised when I read through various SDHCI driver
>> > functions such as the set_ios methods when chasing down a problem with
>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>> > code.
>> >
>> > The driver looks very much like a patchwork quilt of different hacks,
>> > all trying to co-operate with each other in the semblence of something
>> > working - the result is something which does stuff in ways that the SD
>> > card spec doesn't allow, but also does some pretty stupid things when
>> > you have a SDIO device attached.
>> >
>> > The SDIO problems become pretty obvious when you see this log:
>> >
>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>> > [   51.180385] mmc0: runtime suspend
>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>> > [   53.125898] mmc0: runtime resume
>> > [   53.125910] mmc0: card irq raised
>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>> >
>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>> > register values immediately before the stated action is taken bit 8 is
>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>> > interrupts were enabled (one was handled immediately before the above
>> > broadcom cmd=2 message was sent.
>> >
>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>> > disabled by a runtime suspend - including any interrupt from the card.
>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>> > enabling the SDIO card interrupt at that time.
>> >
>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>> > too late for the driver to forward the interrupt to the SDIO interrupt
>> > thread and get it to the driver before the time-out is processed.
>> >
>> > Here's the proof - the above messages came from:
>> >
>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>> > {
>> >         unsigned long flags;
>> >         int ret = 0;
>> >
>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>> >         mmc_hostname(host->mmc));
>> > ...
>> >         spin_lock_irqsave(&host->lock, flags);
>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>> >         spin_unlock_irqrestore(&host->lock, flags);
>> >
>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>> > {
>> >         unsigned long flags;
>> >         int ret = 0, host_flags = host->flags;
>> > ...
>> >         /* Enable SDIO IRQ */
>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>> >
>> >         /* Enable Card Detection */
>> >         sdhci_enable_card_detection(host);
>> >
>> > printk(KERN_DEBUG "%s: runtime resume\n",
>> >         mmc_hostname(host->mmc));
>> >         spin_unlock_irqrestore(&host->lock, flags);
>> >
>> >         return ret;
>> > }
>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>> >
>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>> > though we may want the host to enter a low power mode is something that's
>> > really very very desirable...
>> >
>>
>> I'm not quite clear about your issue.
>> But it seems your issue is caused by runtime pm disabling the
>> interrupt & clocks as you said.
>>
>> Can you try the patch i mentioned here:
>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>
>> That will prevent the host to do runtime pm for SDIO devices.
>
> Why not allow runtime PM at the host level, but still allow the
> interrupt to be received?  Doesn't it make sense to allow PM even with
> SDIO cards in place?

Agree. Just for your reference, there has been related discussions on
the mmc list for how to solve this for host drivers.

This is a patchset for omap_hsmmc, though I don't think the maintainer
has picked it up yet.
http://www.spinics.net/lists/linux-omap/msg101469.html

Typically, once the host becomes runtime suspended, you need re-route
the DAT1 line to a GPIO irq to continue to get the irqs.

Kind regards
Ulf Hansson

>
> Once I get imx-drm off my plate, I'm going to put some work into
> rewriting the sdhci driver mess in a much cleaner way - we can't go on
> putting hacks on top of what's already there, it's already a total mess.
>
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
> --
> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-10 14:03                       ` Russell King - ARM Linux
  2014-02-10 14:19                         ` Arend van Spriel
  2014-02-10 15:25                         ` Ulf Hansson
@ 2014-02-11  7:33                         ` Dong Aisheng
  2014-02-11  9:00                           ` Ulf Hansson
  2014-02-11 10:27                           ` Russell King - ARM Linux
  2 siblings, 2 replies; 16+ messages in thread
From: Dong Aisheng @ 2014-02-11  7:33 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arend van Spriel, Chris Ball, linux-mmc, brcm80211-dev-list

On Mon, Feb 10, 2014 at 10:03 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>> >> > Yeah. I did not mention this, but indeed the first log you provided
>> >> > already made that clear (to me). In your last log the driver sends an UP
>> >> > command to the firmware on which no response is given. So I was hoping
>> >> > the forensics file (which is firmware console buffer) would show an
>> >> > error message of some kind. Also that is not the case. Have to come up
>> >> > with new ideas about what is going wrong here.
>> >>
>> >> I'm chasing a theory at the moment, but it's being complicated by the
>> >> driver oopsing on unload...
>> >
>> > Theory proven.
>> >
>> > May I first take the time to apologise to Arend for wasting his time with
>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>> >
>> > My theory was that it's the sdhci driver causing the problems...  My
>> > suspicions were first raised when I read through various SDHCI driver
>> > functions such as the set_ios methods when chasing down a problem with
>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>> > code.
>> >
>> > The driver looks very much like a patchwork quilt of different hacks,
>> > all trying to co-operate with each other in the semblence of something
>> > working - the result is something which does stuff in ways that the SD
>> > card spec doesn't allow, but also does some pretty stupid things when
>> > you have a SDIO device attached.
>> >
>> > The SDIO problems become pretty obvious when you see this log:
>> >
>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>> > [   51.180385] mmc0: runtime suspend
>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>> > [   53.125898] mmc0: runtime resume
>> > [   53.125910] mmc0: card irq raised
>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>> >
>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>> > register values immediately before the stated action is taken bit 8 is
>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>> > interrupts were enabled (one was handled immediately before the above
>> > broadcom cmd=2 message was sent.
>> >
>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>> > disabled by a runtime suspend - including any interrupt from the card.
>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>> > enabling the SDIO card interrupt at that time.
>> >
>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>> > too late for the driver to forward the interrupt to the SDIO interrupt
>> > thread and get it to the driver before the time-out is processed.
>> >
>> > Here's the proof - the above messages came from:
>> >
>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>> > {
>> >         unsigned long flags;
>> >         int ret = 0;
>> >
>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>> >         mmc_hostname(host->mmc));
>> > ...
>> >         spin_lock_irqsave(&host->lock, flags);
>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>> >         spin_unlock_irqrestore(&host->lock, flags);
>> >
>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>> > {
>> >         unsigned long flags;
>> >         int ret = 0, host_flags = host->flags;
>> > ...
>> >         /* Enable SDIO IRQ */
>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>> >
>> >         /* Enable Card Detection */
>> >         sdhci_enable_card_detection(host);
>> >
>> > printk(KERN_DEBUG "%s: runtime resume\n",
>> >         mmc_hostname(host->mmc));
>> >         spin_unlock_irqrestore(&host->lock, flags);
>> >
>> >         return ret;
>> > }
>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>> >
>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>> > though we may want the host to enter a low power mode is something that's
>> > really very very desirable...
>> >
>>
>> I'm not quite clear about your issue.
>> But it seems your issue is caused by runtime pm disabling the
>> interrupt & clocks as you said.
>>
>> Can you try the patch i mentioned here:
>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>
>> That will prevent the host to do runtime pm for SDIO devices.
>
> Why not allow runtime PM at the host level, but still allow the
> interrupt to be received?  Doesn't it make sense to allow PM even with
> SDIO cards in place?
>

It depends on both host and card that 1) if host is able to detect
SDIO interrupts without clock
and 2) if the card is able to send interrupt to host without clock.
If any one of both can't do that, we may need to tell host to not
do runtime suspend to prevent the clock from being disabled.
Or the SDIO interrupt may not work.

The IMX esdhc/usdhc can not detect the SDIO interrupts sent from card
if the clock is disabled.
So we need such quirk to not allow runtime PM for SDIO cards.
Doesn't it make sense?

Regards
Dong Aisheng

> Once I get imx-drm off my plate, I'm going to put some work into
> rewriting the sdhci driver mess in a much cleaner way - we can't go on
> putting hacks on top of what's already there, it's already a total mess.
>
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".

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

* Re: brcm 4329 problems
  2014-02-10 15:25                         ` Ulf Hansson
@ 2014-02-11  7:38                           ` Dong Aisheng
  2014-02-11  9:46                             ` Ulf Hansson
  0 siblings, 1 reply; 16+ messages in thread
From: Dong Aisheng @ 2014-02-11  7:38 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King - ARM Linux, Arend van Spriel, Chris Ball,
	linux-mmc, brcm80211-dev-list

On Mon, Feb 10, 2014 at 11:25 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 10 February 2014 15:03, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>> >> > Yeah. I did not mention this, but indeed the first log you provided
>>> >> > already made that clear (to me). In your last log the driver sends an UP
>>> >> > command to the firmware on which no response is given. So I was hoping
>>> >> > the forensics file (which is firmware console buffer) would show an
>>> >> > error message of some kind. Also that is not the case. Have to come up
>>> >> > with new ideas about what is going wrong here.
>>> >>
>>> >> I'm chasing a theory at the moment, but it's being complicated by the
>>> >> driver oopsing on unload...
>>> >
>>> > Theory proven.
>>> >
>>> > May I first take the time to apologise to Arend for wasting his time with
>>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>> >
>>> > My theory was that it's the sdhci driver causing the problems...  My
>>> > suspicions were first raised when I read through various SDHCI driver
>>> > functions such as the set_ios methods when chasing down a problem with
>>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>>> > code.
>>> >
>>> > The driver looks very much like a patchwork quilt of different hacks,
>>> > all trying to co-operate with each other in the semblence of something
>>> > working - the result is something which does stuff in ways that the SD
>>> > card spec doesn't allow, but also does some pretty stupid things when
>>> > you have a SDIO device attached.
>>> >
>>> > The SDIO problems become pretty obvious when you see this log:
>>> >
>>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>> > [   51.180385] mmc0: runtime suspend
>>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>> > [   53.125898] mmc0: runtime resume
>>> > [   53.125910] mmc0: card irq raised
>>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>> >
>>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>> > register values immediately before the stated action is taken bit 8 is
>>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>> > interrupts were enabled (one was handled immediately before the above
>>> > broadcom cmd=2 message was sent.
>>> >
>>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>> > disabled by a runtime suspend - including any interrupt from the card.
>>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>>> > enabling the SDIO card interrupt at that time.
>>> >
>>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>>> > too late for the driver to forward the interrupt to the SDIO interrupt
>>> > thread and get it to the driver before the time-out is processed.
>>> >
>>> > Here's the proof - the above messages came from:
>>> >
>>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>> > {
>>> >         unsigned long flags;
>>> >         int ret = 0;
>>> >
>>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>>> >         mmc_hostname(host->mmc));
>>> > ...
>>> >         spin_lock_irqsave(&host->lock, flags);
>>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>> >
>>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>>> > {
>>> >         unsigned long flags;
>>> >         int ret = 0, host_flags = host->flags;
>>> > ...
>>> >         /* Enable SDIO IRQ */
>>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>>> >
>>> >         /* Enable Card Detection */
>>> >         sdhci_enable_card_detection(host);
>>> >
>>> > printk(KERN_DEBUG "%s: runtime resume\n",
>>> >         mmc_hostname(host->mmc));
>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>> >
>>> >         return ret;
>>> > }
>>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>> >
>>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>>> > though we may want the host to enter a low power mode is something that's
>>> > really very very desirable...
>>> >
>>>
>>> I'm not quite clear about your issue.
>>> But it seems your issue is caused by runtime pm disabling the
>>> interrupt & clocks as you said.
>>>
>>> Can you try the patch i mentioned here:
>>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>>
>>> That will prevent the host to do runtime pm for SDIO devices.
>>
>> Why not allow runtime PM at the host level, but still allow the
>> interrupt to be received?  Doesn't it make sense to allow PM even with
>> SDIO cards in place?
>
> Agree. Just for your reference, there has been related discussions on
> the mmc list for how to solve this for host drivers.
>
> This is a patchset for omap_hsmmc, though I don't think the maintainer
> has picked it up yet.
> http://www.spinics.net/lists/linux-omap/msg101469.html
>
> Typically, once the host becomes runtime suspended, you need re-route
> the DAT1 line to a GPIO irq to continue to get the irqs.
>

That seems a way out.
But what if the card can not send interrupt to host without clock?
Then can this still work?

Regards
Dong Aisheng

> Kind regards
> Ulf Hansson
>
>>
>> Once I get imx-drm off my plate, I'm going to put some work into
>> rewriting the sdhci driver mess in a much cleaner way - we can't go on
>> putting hacks on top of what's already there, it's already a total mess.
>>
>> --
>> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
>> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
>> Estimate before purchase was "up to 13.2Mbit".
>> --
>> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-11  7:33                         ` Dong Aisheng
@ 2014-02-11  9:00                           ` Ulf Hansson
  2014-02-12  3:06                             ` Dong Aisheng
  2014-02-11 10:27                           ` Russell King - ARM Linux
  1 sibling, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2014-02-11  9:00 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Russell King - ARM Linux, Arend van Spriel, Chris Ball,
	linux-mmc, brcm80211-dev-list

On 11 February 2014 08:33, Dong Aisheng <dongas86@gmail.com> wrote:
> On Mon, Feb 10, 2014 at 10:03 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>> >> > Yeah. I did not mention this, but indeed the first log you provided
>>> >> > already made that clear (to me). In your last log the driver sends an UP
>>> >> > command to the firmware on which no response is given. So I was hoping
>>> >> > the forensics file (which is firmware console buffer) would show an
>>> >> > error message of some kind. Also that is not the case. Have to come up
>>> >> > with new ideas about what is going wrong here.
>>> >>
>>> >> I'm chasing a theory at the moment, but it's being complicated by the
>>> >> driver oopsing on unload...
>>> >
>>> > Theory proven.
>>> >
>>> > May I first take the time to apologise to Arend for wasting his time with
>>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>> >
>>> > My theory was that it's the sdhci driver causing the problems...  My
>>> > suspicions were first raised when I read through various SDHCI driver
>>> > functions such as the set_ios methods when chasing down a problem with
>>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>>> > code.
>>> >
>>> > The driver looks very much like a patchwork quilt of different hacks,
>>> > all trying to co-operate with each other in the semblence of something
>>> > working - the result is something which does stuff in ways that the SD
>>> > card spec doesn't allow, but also does some pretty stupid things when
>>> > you have a SDIO device attached.
>>> >
>>> > The SDIO problems become pretty obvious when you see this log:
>>> >
>>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>> > [   51.180385] mmc0: runtime suspend
>>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>> > [   53.125898] mmc0: runtime resume
>>> > [   53.125910] mmc0: card irq raised
>>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>> >
>>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>> > register values immediately before the stated action is taken bit 8 is
>>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>> > interrupts were enabled (one was handled immediately before the above
>>> > broadcom cmd=2 message was sent.
>>> >
>>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>> > disabled by a runtime suspend - including any interrupt from the card.
>>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>>> > enabling the SDIO card interrupt at that time.
>>> >
>>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>>> > too late for the driver to forward the interrupt to the SDIO interrupt
>>> > thread and get it to the driver before the time-out is processed.
>>> >
>>> > Here's the proof - the above messages came from:
>>> >
>>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>> > {
>>> >         unsigned long flags;
>>> >         int ret = 0;
>>> >
>>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>>> >         mmc_hostname(host->mmc));
>>> > ...
>>> >         spin_lock_irqsave(&host->lock, flags);
>>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>> >
>>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>>> > {
>>> >         unsigned long flags;
>>> >         int ret = 0, host_flags = host->flags;
>>> > ...
>>> >         /* Enable SDIO IRQ */
>>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>>> >
>>> >         /* Enable Card Detection */
>>> >         sdhci_enable_card_detection(host);
>>> >
>>> > printk(KERN_DEBUG "%s: runtime resume\n",
>>> >         mmc_hostname(host->mmc));
>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>> >
>>> >         return ret;
>>> > }
>>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>> >
>>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>>> > though we may want the host to enter a low power mode is something that's
>>> > really very very desirable...
>>> >
>>>
>>> I'm not quite clear about your issue.
>>> But it seems your issue is caused by runtime pm disabling the
>>> interrupt & clocks as you said.
>>>
>>> Can you try the patch i mentioned here:
>>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>>
>>> That will prevent the host to do runtime pm for SDIO devices.
>>
>> Why not allow runtime PM at the host level, but still allow the
>> interrupt to be received?  Doesn't it make sense to allow PM even with
>> SDIO cards in place?
>>
>
> It depends on both host and card that 1) if host is able to detect
> SDIO interrupts without clock
> and 2) if the card is able to send interrupt to host without clock.
> If any one of both can't do that, we may need to tell host to not
> do runtime suspend to prevent the clock from being disabled.
> Or the SDIO interrupt may not work.
>
> The IMX esdhc/usdhc can not detect the SDIO interrupts sent from card
> if the clock is disabled.

So you are saying you have no option to reroute the DAT1 line to a
GPIO irq, right? I guess that would be SOC specific.

> So we need such quirk to not allow runtime PM for SDIO cards.
> Doesn't it make sense?

Yes, I think so. But, maybe it should be configurable to use it?

Kind regards
Uffe

>
> Regards
> Dong Aisheng
>
>> Once I get imx-drm off my plate, I'm going to put some work into
>> rewriting the sdhci driver mess in a much cleaner way - we can't go on
>> putting hacks on top of what's already there, it's already a total mess.
>>
>> --
>> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
>> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
>> Estimate before purchase was "up to 13.2Mbit".
> --
> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-11  7:38                           ` Dong Aisheng
@ 2014-02-11  9:46                             ` Ulf Hansson
  2014-02-12  3:49                               ` Dong Aisheng
  0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2014-02-11  9:46 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Russell King - ARM Linux, Arend van Spriel, Chris Ball,
	linux-mmc, brcm80211-dev-list

On 11 February 2014 08:38, Dong Aisheng <dongas86@gmail.com> wrote:
> On Mon, Feb 10, 2014 at 11:25 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 10 February 2014 15:03, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>>>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>>>> <linux@arm.linux.org.uk> wrote:
>>>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>>> >> > Yeah. I did not mention this, but indeed the first log you provided
>>>> >> > already made that clear (to me). In your last log the driver sends an UP
>>>> >> > command to the firmware on which no response is given. So I was hoping
>>>> >> > the forensics file (which is firmware console buffer) would show an
>>>> >> > error message of some kind. Also that is not the case. Have to come up
>>>> >> > with new ideas about what is going wrong here.
>>>> >>
>>>> >> I'm chasing a theory at the moment, but it's being complicated by the
>>>> >> driver oopsing on unload...
>>>> >
>>>> > Theory proven.
>>>> >
>>>> > May I first take the time to apologise to Arend for wasting his time with
>>>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>>> >
>>>> > My theory was that it's the sdhci driver causing the problems...  My
>>>> > suspicions were first raised when I read through various SDHCI driver
>>>> > functions such as the set_ios methods when chasing down a problem with
>>>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>>>> > code.
>>>> >
>>>> > The driver looks very much like a patchwork quilt of different hacks,
>>>> > all trying to co-operate with each other in the semblence of something
>>>> > working - the result is something which does stuff in ways that the SD
>>>> > card spec doesn't allow, but also does some pretty stupid things when
>>>> > you have a SDIO device attached.
>>>> >
>>>> > The SDIO problems become pretty obvious when you see this log:
>>>> >
>>>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>>> > [   51.180385] mmc0: runtime suspend
>>>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>>> > [   53.125898] mmc0: runtime resume
>>>> > [   53.125910] mmc0: card irq raised
>>>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>>> >
>>>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>>> > register values immediately before the stated action is taken bit 8 is
>>>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>>> > interrupts were enabled (one was handled immediately before the above
>>>> > broadcom cmd=2 message was sent.
>>>> >
>>>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>>> > disabled by a runtime suspend - including any interrupt from the card.
>>>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>>>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>>>> > enabling the SDIO card interrupt at that time.
>>>> >
>>>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>>>> > too late for the driver to forward the interrupt to the SDIO interrupt
>>>> > thread and get it to the driver before the time-out is processed.
>>>> >
>>>> > Here's the proof - the above messages came from:
>>>> >
>>>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>>> > {
>>>> >         unsigned long flags;
>>>> >         int ret = 0;
>>>> >
>>>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>>>> >         mmc_hostname(host->mmc));
>>>> > ...
>>>> >         spin_lock_irqsave(&host->lock, flags);
>>>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>> >
>>>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>>>> > {
>>>> >         unsigned long flags;
>>>> >         int ret = 0, host_flags = host->flags;
>>>> > ...
>>>> >         /* Enable SDIO IRQ */
>>>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>>>> >
>>>> >         /* Enable Card Detection */
>>>> >         sdhci_enable_card_detection(host);
>>>> >
>>>> > printk(KERN_DEBUG "%s: runtime resume\n",
>>>> >         mmc_hostname(host->mmc));
>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>> >
>>>> >         return ret;
>>>> > }
>>>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>>> >
>>>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>>>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>>>> > though we may want the host to enter a low power mode is something that's
>>>> > really very very desirable...
>>>> >
>>>>
>>>> I'm not quite clear about your issue.
>>>> But it seems your issue is caused by runtime pm disabling the
>>>> interrupt & clocks as you said.
>>>>
>>>> Can you try the patch i mentioned here:
>>>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>>>
>>>> That will prevent the host to do runtime pm for SDIO devices.
>>>
>>> Why not allow runtime PM at the host level, but still allow the
>>> interrupt to be received?  Doesn't it make sense to allow PM even with
>>> SDIO cards in place?
>>
>> Agree. Just for your reference, there has been related discussions on
>> the mmc list for how to solve this for host drivers.
>>
>> This is a patchset for omap_hsmmc, though I don't think the maintainer
>> has picked it up yet.
>> http://www.spinics.net/lists/linux-omap/msg101469.html
>>
>> Typically, once the host becomes runtime suspended, you need re-route
>> the DAT1 line to a GPIO irq to continue to get the irqs.
>>
>
> That seems a way out.
> But what if the card can not send interrupt to host without clock?
> Then can this still work?

Looking into the SDIO spec it then gives you two options.

1) From SDIO 3.0, "Asynchronous SDIO irq" was added. This means no
clock is needed for the card to trigger SDIO irqs.

2) By putting the card into 1-bit data mode, the card shall be able to
send irqs without clock. Actually at system suspend, the mmc core do
put the card into 1-bit data mode, if it have MMC_PM_KEEP_POWER and
MMC_PM_WAKE_SDIO_IRQ flags set, but for some reason it doesn't set the
clock set to zero. Anyway, there are currently no smooth way for a
host to perform similar actions from it's runtime_suspend callback,
but manually itself send the command(s) to the card.

Do note that my experience involves only a couple of different SDIO
cards. All old ones (1.1 or 2.0), but still those seemed to support
"Asynchronous SDIO irq", even if that feature was added to the 3.0
spec. Obviously not an information we can build anything on, but
thought it might be good to know.

Kind regards
Ulf Hansson

>
> Regards
> Dong Aisheng
>
>> Kind regards
>> Ulf Hansson
>>
>>>
>>> Once I get imx-drm off my plate, I'm going to put some work into
>>> rewriting the sdhci driver mess in a much cleaner way - we can't go on
>>> putting hacks on top of what's already there, it's already a total mess.
>>>
>>> --
>>> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
>>> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
>>> Estimate before purchase was "up to 13.2Mbit".
>>> --
>>> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-11  7:33                         ` Dong Aisheng
  2014-02-11  9:00                           ` Ulf Hansson
@ 2014-02-11 10:27                           ` Russell King - ARM Linux
  2014-02-12  3:17                             ` Dong Aisheng
  1 sibling, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux @ 2014-02-11 10:27 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: Arend van Spriel, Chris Ball, linux-mmc, brcm80211-dev-list

On Tue, Feb 11, 2014 at 03:33:09PM +0800, Dong Aisheng wrote:
> It depends on both host and card that 1) if host is able to detect
> SDIO interrupts without clock
> and 2) if the card is able to send interrupt to host without clock.
> If any one of both can't do that, we may need to tell host to not
> do runtime suspend to prevent the clock from being disabled.
> Or the SDIO interrupt may not work.
> 
> The IMX esdhc/usdhc can not detect the SDIO interrupts sent from card
> if the clock is disabled.
> So we need such quirk to not allow runtime PM for SDIO cards.
> Doesn't it make sense?

That's strange.  I have it working as of last night.

Yes, I have to leave the AHB clock on so that the registers are readable,
in the interrupt handler, and that's something I only do if the SDIO
interrupt is enabled, but everything else gets disabled.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

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

* Re: brcm 4329 problems
  2014-02-11  9:00                           ` Ulf Hansson
@ 2014-02-12  3:06                             ` Dong Aisheng
  2014-02-12 10:01                               ` Ulf Hansson
  0 siblings, 1 reply; 16+ messages in thread
From: Dong Aisheng @ 2014-02-12  3:06 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King - ARM Linux, Arend van Spriel, Chris Ball,
	linux-mmc, brcm80211-dev-list

On Tue, Feb 11, 2014 at 5:00 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 11 February 2014 08:33, Dong Aisheng <dongas86@gmail.com> wrote:
>> On Mon, Feb 10, 2014 at 10:03 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>>>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>>>> <linux@arm.linux.org.uk> wrote:
>>>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>>> >> > Yeah. I did not mention this, but indeed the first log you provided
>>>> >> > already made that clear (to me). In your last log the driver sends an UP
>>>> >> > command to the firmware on which no response is given. So I was hoping
>>>> >> > the forensics file (which is firmware console buffer) would show an
>>>> >> > error message of some kind. Also that is not the case. Have to come up
>>>> >> > with new ideas about what is going wrong here.
>>>> >>
>>>> >> I'm chasing a theory at the moment, but it's being complicated by the
>>>> >> driver oopsing on unload...
>>>> >
>>>> > Theory proven.
>>>> >
>>>> > May I first take the time to apologise to Arend for wasting his time with
>>>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>>> >
>>>> > My theory was that it's the sdhci driver causing the problems...  My
>>>> > suspicions were first raised when I read through various SDHCI driver
>>>> > functions such as the set_ios methods when chasing down a problem with
>>>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>>>> > code.
>>>> >
>>>> > The driver looks very much like a patchwork quilt of different hacks,
>>>> > all trying to co-operate with each other in the semblence of something
>>>> > working - the result is something which does stuff in ways that the SD
>>>> > card spec doesn't allow, but also does some pretty stupid things when
>>>> > you have a SDIO device attached.
>>>> >
>>>> > The SDIO problems become pretty obvious when you see this log:
>>>> >
>>>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>>> > [   51.180385] mmc0: runtime suspend
>>>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>>> > [   53.125898] mmc0: runtime resume
>>>> > [   53.125910] mmc0: card irq raised
>>>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>>> >
>>>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>>> > register values immediately before the stated action is taken bit 8 is
>>>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>>> > interrupts were enabled (one was handled immediately before the above
>>>> > broadcom cmd=2 message was sent.
>>>> >
>>>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>>> > disabled by a runtime suspend - including any interrupt from the card.
>>>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>>>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>>>> > enabling the SDIO card interrupt at that time.
>>>> >
>>>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>>>> > too late for the driver to forward the interrupt to the SDIO interrupt
>>>> > thread and get it to the driver before the time-out is processed.
>>>> >
>>>> > Here's the proof - the above messages came from:
>>>> >
>>>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>>> > {
>>>> >         unsigned long flags;
>>>> >         int ret = 0;
>>>> >
>>>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>>>> >         mmc_hostname(host->mmc));
>>>> > ...
>>>> >         spin_lock_irqsave(&host->lock, flags);
>>>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>> >
>>>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>>>> > {
>>>> >         unsigned long flags;
>>>> >         int ret = 0, host_flags = host->flags;
>>>> > ...
>>>> >         /* Enable SDIO IRQ */
>>>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>>>> >
>>>> >         /* Enable Card Detection */
>>>> >         sdhci_enable_card_detection(host);
>>>> >
>>>> > printk(KERN_DEBUG "%s: runtime resume\n",
>>>> >         mmc_hostname(host->mmc));
>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>> >
>>>> >         return ret;
>>>> > }
>>>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>>> >
>>>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>>>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>>>> > though we may want the host to enter a low power mode is something that's
>>>> > really very very desirable...
>>>> >
>>>>
>>>> I'm not quite clear about your issue.
>>>> But it seems your issue is caused by runtime pm disabling the
>>>> interrupt & clocks as you said.
>>>>
>>>> Can you try the patch i mentioned here:
>>>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>>>
>>>> That will prevent the host to do runtime pm for SDIO devices.
>>>
>>> Why not allow runtime PM at the host level, but still allow the
>>> interrupt to be received?  Doesn't it make sense to allow PM even with
>>> SDIO cards in place?
>>>
>>
>> It depends on both host and card that 1) if host is able to detect
>> SDIO interrupts without clock
>> and 2) if the card is able to send interrupt to host without clock.
>> If any one of both can't do that, we may need to tell host to not
>> do runtime suspend to prevent the clock from being disabled.
>> Or the SDIO interrupt may not work.
>>
>> The IMX esdhc/usdhc can not detect the SDIO interrupts sent from card
>> if the clock is disabled.
>
> So you are saying you have no option to reroute the DAT1 line to a
> GPIO irq, right? I guess that would be SOC specific.

I think IMX can support reroute the DAT1 line to GPIO altough i've not tried.
We have to extend the common sdhci driver to support it.
Actually with some special hack, IMX can also detect SDIO irq without clock by
using Wakeup Event Enable On Card Interrupt since the wakeup feature
does not need clock.
It's quite similar as using GPIO way, the difference is using which
interrupt(gpio or sdhci itself)
to enable the clock for the later card interrupt detect.
The drawback is it does need a lot special and SoC specific hacks in
common sdhci driver.
Comparing simply disabling runtime pm way, I'm not so willing to do that.

And i'm also afraid it may cause performance drop since i have to:
1) sdio interrupt -> sdhci_irq enter-> disable card interrupt->
runtime_pm_get (enable clock in async way)
->sdhci_irq exist
(after clock is enabled sdio interrupt will be detected again)
2) sdhci irq -> read sdio interrupt status -> sdio_signal_irq...

Comparing to original way(2), steps 1 is the extra cost.
Not sure how many performance will drop since i still have not tried it.

>
>> So we need such quirk to not allow runtime PM for SDIO cards.
>> Doesn't it make sense?
>
> Yes, I think so. But, maybe it should be configurable to use it?
>

Well, true, it should be configurable.

Regards
Dong Aisheng

> Kind regards
> Uffe
>
>>
>> Regards
>> Dong Aisheng
>>
>>> Once I get imx-drm off my plate, I'm going to put some work into
>>> rewriting the sdhci driver mess in a much cleaner way - we can't go on
>>> putting hacks on top of what's already there, it's already a total mess.
>>>
>>> --
>>> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
>>> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
>>> Estimate before purchase was "up to 13.2Mbit".
>> --
>> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-11 10:27                           ` Russell King - ARM Linux
@ 2014-02-12  3:17                             ` Dong Aisheng
  2014-02-12  9:36                               ` Russell King - ARM Linux
  0 siblings, 1 reply; 16+ messages in thread
From: Dong Aisheng @ 2014-02-12  3:17 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arend van Spriel, Chris Ball, linux-mmc, brcm80211-dev-list

On Tue, Feb 11, 2014 at 6:27 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Feb 11, 2014 at 03:33:09PM +0800, Dong Aisheng wrote:
>> It depends on both host and card that 1) if host is able to detect
>> SDIO interrupts without clock
>> and 2) if the card is able to send interrupt to host without clock.
>> If any one of both can't do that, we may need to tell host to not
>> do runtime suspend to prevent the clock from being disabled.
>> Or the SDIO interrupt may not work.
>>
>> The IMX esdhc/usdhc can not detect the SDIO interrupts sent from card
>> if the clock is disabled.
>> So we need such quirk to not allow runtime PM for SDIO cards.
>> Doesn't it make sense?
>
> That's strange.  I have it working as of last night.
>
> Yes, I have to leave the AHB clock on so that the registers are readable,
> in the interrupt handler, and that's something I only do if the SDIO
> interrupt is enabled, but everything else gets disabled.
>

The ahb, ipg and per of usdhc are using the same clock.
Looking at arch/arm/boot/dts/imx6qdl.dtsi
usdhc2: usdhc@02194000 {
        compatible = "fsl,imx6q-usdhc";
        reg = <0x02194000 0x4000>;
        interrupts = <0 23 0x04>;
        clocks = <&clks 164>, <&clks 164>, <&clks 164>;
        clock-names = "ipg", "ahb", "per";
        bus-width = <4>;
        status = "disabled";
};
That means enabling of ahb clock actually will make ipg and per also open.
Then card interrupts can be detected well.
You can measure the card clock(pin5) with a scope to double check it.

Regards
Dong Aisheng

> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".

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

* Re: brcm 4329 problems
  2014-02-11  9:46                             ` Ulf Hansson
@ 2014-02-12  3:49                               ` Dong Aisheng
  0 siblings, 0 replies; 16+ messages in thread
From: Dong Aisheng @ 2014-02-12  3:49 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King - ARM Linux, Arend van Spriel, Chris Ball,
	linux-mmc, brcm80211-dev-list

On Tue, Feb 11, 2014 at 5:46 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 11 February 2014 08:38, Dong Aisheng <dongas86@gmail.com> wrote:
>> On Mon, Feb 10, 2014 at 11:25 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 10 February 2014 15:03, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>>> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>>>>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>>>>> <linux@arm.linux.org.uk> wrote:
>>>>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>>>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>>>> >> > Yeah. I did not mention this, but indeed the first log you provided
>>>>> >> > already made that clear (to me). In your last log the driver sends an UP
>>>>> >> > command to the firmware on which no response is given. So I was hoping
>>>>> >> > the forensics file (which is firmware console buffer) would show an
>>>>> >> > error message of some kind. Also that is not the case. Have to come up
>>>>> >> > with new ideas about what is going wrong here.
>>>>> >>
>>>>> >> I'm chasing a theory at the moment, but it's being complicated by the
>>>>> >> driver oopsing on unload...
>>>>> >
>>>>> > Theory proven.
>>>>> >
>>>>> > May I first take the time to apologise to Arend for wasting his time with
>>>>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>>>> >
>>>>> > My theory was that it's the sdhci driver causing the problems...  My
>>>>> > suspicions were first raised when I read through various SDHCI driver
>>>>> > functions such as the set_ios methods when chasing down a problem with
>>>>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>>>>> > code.
>>>>> >
>>>>> > The driver looks very much like a patchwork quilt of different hacks,
>>>>> > all trying to co-operate with each other in the semblence of something
>>>>> > working - the result is something which does stuff in ways that the SD
>>>>> > card spec doesn't allow, but also does some pretty stupid things when
>>>>> > you have a SDIO device attached.
>>>>> >
>>>>> > The SDIO problems become pretty obvious when you see this log:
>>>>> >
>>>>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>>>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>>>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>>>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>>>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>>>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>>>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>>>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>>>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>>>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>>>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>>>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>>>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>>>> > [   51.180385] mmc0: runtime suspend
>>>>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>>>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>>>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>>>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>>>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>>>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>>>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>>>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>>>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>>>> > [   53.125898] mmc0: runtime resume
>>>>> > [   53.125910] mmc0: card irq raised
>>>>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>>>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>>>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>>>> >
>>>>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>>>> > register values immediately before the stated action is taken bit 8 is
>>>>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>>>> > interrupts were enabled (one was handled immediately before the above
>>>>> > broadcom cmd=2 message was sent.
>>>>> >
>>>>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>>>> > disabled by a runtime suspend - including any interrupt from the card.
>>>>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>>>>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>>>>> > enabling the SDIO card interrupt at that time.
>>>>> >
>>>>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>>>>> > too late for the driver to forward the interrupt to the SDIO interrupt
>>>>> > thread and get it to the driver before the time-out is processed.
>>>>> >
>>>>> > Here's the proof - the above messages came from:
>>>>> >
>>>>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>>>> > {
>>>>> >         unsigned long flags;
>>>>> >         int ret = 0;
>>>>> >
>>>>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>>>>> >         mmc_hostname(host->mmc));
>>>>> > ...
>>>>> >         spin_lock_irqsave(&host->lock, flags);
>>>>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>>> >
>>>>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>>>>> > {
>>>>> >         unsigned long flags;
>>>>> >         int ret = 0, host_flags = host->flags;
>>>>> > ...
>>>>> >         /* Enable SDIO IRQ */
>>>>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>>>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>>>>> >
>>>>> >         /* Enable Card Detection */
>>>>> >         sdhci_enable_card_detection(host);
>>>>> >
>>>>> > printk(KERN_DEBUG "%s: runtime resume\n",
>>>>> >         mmc_hostname(host->mmc));
>>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>>> >
>>>>> >         return ret;
>>>>> > }
>>>>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>>>> >
>>>>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>>>>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>>>>> > though we may want the host to enter a low power mode is something that's
>>>>> > really very very desirable...
>>>>> >
>>>>>
>>>>> I'm not quite clear about your issue.
>>>>> But it seems your issue is caused by runtime pm disabling the
>>>>> interrupt & clocks as you said.
>>>>>
>>>>> Can you try the patch i mentioned here:
>>>>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>>>>
>>>>> That will prevent the host to do runtime pm for SDIO devices.
>>>>
>>>> Why not allow runtime PM at the host level, but still allow the
>>>> interrupt to be received?  Doesn't it make sense to allow PM even with
>>>> SDIO cards in place?
>>>
>>> Agree. Just for your reference, there has been related discussions on
>>> the mmc list for how to solve this for host drivers.
>>>
>>> This is a patchset for omap_hsmmc, though I don't think the maintainer
>>> has picked it up yet.
>>> http://www.spinics.net/lists/linux-omap/msg101469.html
>>>
>>> Typically, once the host becomes runtime suspended, you need re-route
>>> the DAT1 line to a GPIO irq to continue to get the irqs.
>>>
>>
>> That seems a way out.
>> But what if the card can not send interrupt to host without clock?
>> Then can this still work?
>
> Looking into the SDIO spec it then gives you two options.
>
> 1) From SDIO 3.0, "Asynchronous SDIO irq" was added. This means no
> clock is needed for the card to trigger SDIO irqs.
>
> 2) By putting the card into 1-bit data mode, the card shall be able to
> send irqs without clock. Actually at system suspend, the mmc core do
> put the card into 1-bit data mode, if it have MMC_PM_KEEP_POWER and
> MMC_PM_WAKE_SDIO_IRQ flags set, but for some reason it doesn't set the
> clock set to zero.

It seems it was originally designed for system wakeup feature and not
for runtime
suspend/resume.

So the spec said the card must be able to send irq in 1-bit mode
without clock right?
If yes, it will be working way.

Regards
Dong Aisheng

> Anyway, there are currently no smooth way for a
> host to perform similar actions from it's runtime_suspend callback,
> but manually itself send the command(s) to the card.
>
> Do note that my experience involves only a couple of different SDIO
> cards. All old ones (1.1 or 2.0), but still those seemed to support
> "Asynchronous SDIO irq", even if that feature was added to the 3.0
> spec. Obviously not an information we can build anything on, but
> thought it might be good to know.
>
> Kind regards
> Ulf Hansson
>
>>
>> Regards
>> Dong Aisheng
>>
>>> Kind regards
>>> Ulf Hansson
>>>
>>>>
>>>> Once I get imx-drm off my plate, I'm going to put some work into
>>>> rewriting the sdhci driver mess in a much cleaner way - we can't go on
>>>> putting hacks on top of what's already there, it's already a total mess.
>>>>
>>>> --
>>>> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
>>>> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
>>>> Estimate before purchase was "up to 13.2Mbit".
>>>> --
>>>> 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] 16+ messages in thread

* Re: brcm 4329 problems
  2014-02-12  3:17                             ` Dong Aisheng
@ 2014-02-12  9:36                               ` Russell King - ARM Linux
  0 siblings, 0 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2014-02-12  9:36 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: Arend van Spriel, Chris Ball, linux-mmc, brcm80211-dev-list

On Wed, Feb 12, 2014 at 11:17:09AM +0800, Dong Aisheng wrote:
> That means enabling of ahb clock actually will make ipg and per also open.
> Then card interrupts can be detected well.
> You can measure the card clock(pin5) with a scope to double check it.

I can't - the brcm is a chip not a SD card, and the signals are all
embedded on internal layers in the PCB.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

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

* Re: brcm 4329 problems
  2014-02-12  3:06                             ` Dong Aisheng
@ 2014-02-12 10:01                               ` Ulf Hansson
  0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2014-02-12 10:01 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Russell King - ARM Linux, Arend van Spriel, Chris Ball,
	linux-mmc, brcm80211-dev-list

On 12 February 2014 04:06, Dong Aisheng <dongas86@gmail.com> wrote:
> On Tue, Feb 11, 2014 at 5:00 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 11 February 2014 08:33, Dong Aisheng <dongas86@gmail.com> wrote:
>>> On Mon, Feb 10, 2014 at 10:03 PM, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>>> On Mon, Feb 10, 2014 at 02:50:42PM +0800, Dong Aisheng wrote:
>>>>> On Mon, Feb 10, 2014 at 7:27 AM, Russell King - ARM Linux
>>>>> <linux@arm.linux.org.uk> wrote:
>>>>> > On Sun, Feb 09, 2014 at 08:59:17PM +0000, Russell King - ARM Linux wrote:
>>>>> >> On Sun, Feb 09, 2014 at 10:05:13AM +0100, Arend van Spriel wrote:
>>>>> >> > Yeah. I did not mention this, but indeed the first log you provided
>>>>> >> > already made that clear (to me). In your last log the driver sends an UP
>>>>> >> > command to the firmware on which no response is given. So I was hoping
>>>>> >> > the forensics file (which is firmware console buffer) would show an
>>>>> >> > error message of some kind. Also that is not the case. Have to come up
>>>>> >> > with new ideas about what is going wrong here.
>>>>> >>
>>>>> >> I'm chasing a theory at the moment, but it's being complicated by the
>>>>> >> driver oopsing on unload...
>>>>> >
>>>>> > Theory proven.
>>>>> >
>>>>> > May I first take the time to apologise to Arend for wasting his time with
>>>>> > this issue; the issue is not the Broadcom driver, but the SDHCI driver.
>>>>> >
>>>>> > My theory was that it's the sdhci driver causing the problems...  My
>>>>> > suspicions were first raised when I read through various SDHCI driver
>>>>> > functions such as the set_ios methods when chasing down a problem with
>>>>> > UHS-1 SD cards, and later when I was reading it's interrupt handling
>>>>> > code.
>>>>> >
>>>>> > The driver looks very much like a patchwork quilt of different hacks,
>>>>> > all trying to co-operate with each other in the semblence of something
>>>>> > working - the result is something which does stuff in ways that the SD
>>>>> > card spec doesn't allow, but also does some pretty stupid things when
>>>>> > you have a SDIO device attached.
>>>>> >
>>>>> > The SDIO problems become pretty obvious when you see this log:
>>>>> >
>>>>> > [   51.112923] brcmfmac: brcmf_fil_cmd_int_set cmd=2, value=0
>>>>> > [   51.112937] brcmfmac: brcmf_proto_bcdc_set_dcmd Enter, cmd 2 len 4
>>>>> > [   51.112946] brcmfmac: brcmf_proto_bcdc_msg Enter
>>>>> > [   51.112981] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>>> > [   51.112989] brcmfmac: brcmf_sdio_clkctl Enter
>>>>> > [   51.113498] brcmfmac: brcmf_proto_bcdc_cmplt Enter
>>>>> > [   51.128501] brcmfmac: brcmf_sdio_bus_watchdog idle
>>>>> > [   51.128522] brcmfmac: brcmf_sdio_bus_sleep request SLEEP currently WAKE
>>>>> > [   51.128532] brcmfmac: brcmf_sdio_clkctl Enter
>>>>> > [   51.128540] brcmfmac: brcmf_sdio_htclk Enter
>>>>> > [   51.128549] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x00
>>>>> > [   51.128560] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>>> > [   51.128645] brcmfmac: brcmf_sdio_htclk CLKCTL: turned OFF
>>>>> > [   51.128655] brcmfmac: brcmf_sdio_sdclk Enter
>>>>> > [   51.128667] brcmfmac: brcmf_sdio_clkctl 3 -> 0
>>>>> > [   51.180385] mmc0: runtime suspend
>>>>> > [   53.112272] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>>>>> > [   53.118290] brcmfmac: brcmf_sdio_bus_sleep request WAKE currently WAKE
>>>>> > [   53.118302] brcmfmac: brcmf_sdio_clkctl Enter
>>>>> > [   53.118312] brcmfmac: brcmf_sdio_sdclk Enter
>>>>> > [   53.118319] brcmfmac: brcmf_sdio_htclk Enter
>>>>> > [   53.118329] brcmfmac: brcmf_sdiod_regwb addr:0x0001000e, data:0x10
>>>>> > [   53.118341] brcmfmac: brcmf_sdiod_request_data rw=1, func=1, addr=0x1000e, nbytes=1
>>>>> > [   53.118537] sdhci-esdhc-imx 2190000.usdhc: desired SD clock: 50000000, actual: 49500000
>>>>> > [   53.118550] mmc0: esdhc_pltfm_set_clock: CLK on
>>>>> > [   53.119723] sdhci-esdhc-imx 2190000.usdhc: change pinctrl state for uhs 0
>>>>> > [   53.125880] mmc0: sdio irq enabled: 007f0003 007f0003
>>>>> > [   53.125898] mmc0: runtime resume
>>>>> > [   53.125910] mmc0: card irq raised
>>>>> > [   53.125925] mmc0: sdio irq disabled: 007f0103 007f0103
>>>>> > [   53.126030] brcmfmac: brcmf_sdiod_regrb addr:0x0001000e
>>>>> > [   53.126055] brcmfmac: brcmf_sdiod_request_data rw=0, func=1, addr=0x1000e, nbytes=1
>>>>> >
>>>>> > The values printed in "sdio irq *abled" are the INT_ENABLE and SIGNAL_ENABLE
>>>>> > register values immediately before the stated action is taken bit 8 is
>>>>> > the interrupt enable for card interrupts.  Earlier in the log, SDIO card
>>>>> > interrupts were enabled (one was handled immediately before the above
>>>>> > broadcom cmd=2 message was sent.
>>>>> >
>>>>> > Yep, that's right - at 51.180385, the SDIO host has /all/ interrupts
>>>>> > disabled by a runtime suspend - including any interrupt from the card.
>>>>> > The brcmfmac driver times out after 2 seconds having sent the "up"
>>>>> > command, and re-awakens the host, which is runtime resumed at 53.125898,
>>>>> > enabling the SDIO card interrupt at that time.
>>>>> >
>>>>> > And lo and behold - the card has an interrupt pending!  Too bad, we're
>>>>> > too late for the driver to forward the interrupt to the SDIO interrupt
>>>>> > thread and get it to the driver before the time-out is processed.
>>>>> >
>>>>> > Here's the proof - the above messages came from:
>>>>> >
>>>>> > int sdhci_runtime_suspend_host(struct sdhci_host *host)
>>>>> > {
>>>>> >         unsigned long flags;
>>>>> >         int ret = 0;
>>>>> >
>>>>> > printk(KERN_DEBUG "%s: runtime suspend\n",
>>>>> >         mmc_hostname(host->mmc));
>>>>> > ...
>>>>> >         spin_lock_irqsave(&host->lock, flags);
>>>>> >         sdhci_mask_irqs(host, SDHCI_INT_ALL_MASK);
>>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>>> >
>>>>> > int sdhci_runtime_resume_host(struct sdhci_host *host)
>>>>> > {
>>>>> >         unsigned long flags;
>>>>> >         int ret = 0, host_flags = host->flags;
>>>>> > ...
>>>>> >         /* Enable SDIO IRQ */
>>>>> >         if ((host->flags & SDHCI_SDIO_IRQ_ENABLED))
>>>>> >                 sdhci_enable_sdio_irq_nolock(host, true);
>>>>> >
>>>>> >         /* Enable Card Detection */
>>>>> >         sdhci_enable_card_detection(host);
>>>>> >
>>>>> > printk(KERN_DEBUG "%s: runtime resume\n",
>>>>> >         mmc_hostname(host->mmc));
>>>>> >         spin_unlock_irqrestore(&host->lock, flags);
>>>>> >
>>>>> >         return ret;
>>>>> > }
>>>>> > EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host);
>>>>> >
>>>>> > To me, it looks like SDHCI needs a major rework...  And there needs to
>>>>> > be some recognition that - maybe - leaving SDIO interrupts enabled even
>>>>> > though we may want the host to enter a low power mode is something that's
>>>>> > really very very desirable...
>>>>> >
>>>>>
>>>>> I'm not quite clear about your issue.
>>>>> But it seems your issue is caused by runtime pm disabling the
>>>>> interrupt & clocks as you said.
>>>>>
>>>>> Can you try the patch i mentioned here:
>>>>> http://www.spinics.net/lists/linux-mmc/msg24764.html
>>>>>
>>>>> That will prevent the host to do runtime pm for SDIO devices.
>>>>
>>>> Why not allow runtime PM at the host level, but still allow the
>>>> interrupt to be received?  Doesn't it make sense to allow PM even with
>>>> SDIO cards in place?
>>>>
>>>
>>> It depends on both host and card that 1) if host is able to detect
>>> SDIO interrupts without clock
>>> and 2) if the card is able to send interrupt to host without clock.
>>> If any one of both can't do that, we may need to tell host to not
>>> do runtime suspend to prevent the clock from being disabled.
>>> Or the SDIO interrupt may not work.
>>>
>>> The IMX esdhc/usdhc can not detect the SDIO interrupts sent from card
>>> if the clock is disabled.
>>
>> So you are saying you have no option to reroute the DAT1 line to a
>> GPIO irq, right? I guess that would be SOC specific.
>
> I think IMX can support reroute the DAT1 line to GPIO altough i've not tried.
> We have to extend the common sdhci driver to support it.
> Actually with some special hack, IMX can also detect SDIO irq without clock by
> using Wakeup Event Enable On Card Interrupt since the wakeup feature
> does not need clock.
> It's quite similar as using GPIO way, the difference is using which
> interrupt(gpio or sdhci itself)
> to enable the clock for the later card interrupt detect.
> The drawback is it does need a lot special and SoC specific hacks in
> common sdhci driver.
> Comparing simply disabling runtime pm way, I'm not so willing to do that.

I understand the complexity needed for this, although if you have a
look how Andreas Fenkart's solution for omap_hsmmc, I think it became
quite nice. Not sure how much extra effort is needed to adopt it for
sdhci.

If you go with the solution of disabling runtime PM while SDIO irq is
enabled, it's consequences will be that the power domain which the
device resides in can't be dropped. That may, depending on SOC design,
have really bad impact on power.

On the other hand, some SDIO func drivers seems broken and at the
moment it may be considered more important to fix them. We could
handle the optimization for power save as a follow up step instead?

>
> And i'm also afraid it may cause performance drop since i have to:
> 1) sdio interrupt -> sdhci_irq enter-> disable card interrupt->
> runtime_pm_get (enable clock in async way)
> ->sdhci_irq exist
> (after clock is enabled sdio interrupt will be detected again)
> 2) sdhci irq -> read sdio interrupt status -> sdio_signal_irq...

I suppose there will be a autosuspend timeout, thus this sequence is
only relevant once the device is in runtime suspend state. Certainly
there will be an extra latency to fetch the first SDIO irq, but I
don't think performance as such will be effected.

Additionally, maybe we can use mmc_signal_sdio_irq() from the GPIO irq
handler, thus picking up the IRQ a bit sooner than waiting for the
sdhci controller to pick it up. That could work, right?

>
> Comparing to original way(2), steps 1 is the extra cost.
> Not sure how many performance will drop since i still have not tried it.
>
>>
>>> So we need such quirk to not allow runtime PM for SDIO cards.
>>> Doesn't it make sense?
>>
>> Yes, I think so. But, maybe it should be configurable to use it?
>>
>
> Well, true, it should be configurable.
>
> Regards
> Dong Aisheng
>
>> Kind regards
>> Uffe
>>
>>>
>>> Regards
>>> Dong Aisheng
>>>
>>>> Once I get imx-drm off my plate, I'm going to put some work into
>>>> rewriting the sdhci driver mess in a much cleaner way - we can't go on
>>>> putting hacks on top of what's already there, it's already a total mess.
>>>>
>>>> --
>>>> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
>>>> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
>>>> Estimate before purchase was "up to 13.2Mbit".
>>> --
>>> 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] 16+ messages in thread

end of thread, other threads:[~2014-02-12 10:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20140201221548.GB26684@n2100.arm.linux.org.uk>
     [not found] ` <52EF59CB.5050901@broadcom.com>
     [not found]   ` <20140203153239.GL26684@n2100.arm.linux.org.uk>
     [not found]     ` <52F00021.6070904@broadcom.com>
     [not found]       ` <20140203210450.GN26684@n2100.arm.linux.org.uk>
     [not found]         ` <52F14F7E.9040900@broadcom.com>
     [not found]           ` <20140205153824.GS26684@n2100.arm.linux.org.uk>
     [not found]             ` <20140208215927.GA5276@n2100.arm.linux.org.uk>
     [not found]               ` <52F744C9.7040804@broadcom.com>
     [not found]                 ` <20140209205917.GO26684@n2100.arm.linux.org.uk>
2014-02-09 23:27                   ` brcm 4329 problems Russell King - ARM Linux
2014-02-10  6:50                     ` Dong Aisheng
2014-02-10 14:03                       ` Russell King - ARM Linux
2014-02-10 14:19                         ` Arend van Spriel
2014-02-10 15:25                         ` Ulf Hansson
2014-02-11  7:38                           ` Dong Aisheng
2014-02-11  9:46                             ` Ulf Hansson
2014-02-12  3:49                               ` Dong Aisheng
2014-02-11  7:33                         ` Dong Aisheng
2014-02-11  9:00                           ` Ulf Hansson
2014-02-12  3:06                             ` Dong Aisheng
2014-02-12 10:01                               ` Ulf Hansson
2014-02-11 10:27                           ` Russell King - ARM Linux
2014-02-12  3:17                             ` Dong Aisheng
2014-02-12  9:36                               ` Russell King - ARM Linux
2014-02-10 10:07                     ` Arend van Spriel

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.