| Message ID | 1455098283-14500-1-git-send-email-ludovic.desroches@atmel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/02/16 11:58, Ludovic Desroches wrote: > By putting the device in suspend at the end of the probe, it is > impossible to wake up on non software event such as card > insertion/removal. > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > --- > > Hi, > > Since I had no feedback on this topic: > http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 > > I would like to no more put the device in suspend at the end of the probe. If > my device is suspended at the end of the probe, I have no issue to resume on > a software event such as mounting my sdcard but hardware event such as card > insertion and removal do not trigger a resume. You can't use runtime PM unless you have a way to wake-up. Currently, sdhci disables card detect interrupts when runtime suspended, and drivers use a card-detect GPIO to wake-up. > > It seems there are only two sdhci drivers using runtime pm so maybe nobody has > noticed this issue. > > Regards > > Ludovic > > drivers/mmc/host/sdhci-of-at91.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c > index 9cb86fb..ae24dea 100644 > --- a/drivers/mmc/host/sdhci-of-at91.c > +++ b/drivers/mmc/host/sdhci-of-at91.c > @@ -210,8 +210,6 @@ static int sdhci_at91_probe(struct platform_device *pdev) > if (ret) > goto pm_runtime_disable; > > - pm_runtime_put_autosuspend(&pdev->dev); > - > return 0; > > pm_runtime_disable: > -- 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
Hi Adrian, On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: > On 10/02/16 11:58, Ludovic Desroches wrote: > > By putting the device in suspend at the end of the probe, it is > > impossible to wake up on non software event such as card > > insertion/removal. > > > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > > --- > > > > Hi, > > > > Since I had no feedback on this topic: > > http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 > > > > I would like to no more put the device in suspend at the end of the probe. If > > my device is suspended at the end of the probe, I have no issue to resume on > > a software event such as mounting my sdcard but hardware event such as card > > insertion and removal do not trigger a resume. > > You can't use runtime PM unless you have a way to wake-up. > Thanks for your feedback. I am a bit disappointed since Ulf advised me to use runtime PM instead of system PM. > Currently, sdhci disables card detect interrupts when runtime suspended, > and drivers use a card-detect GPIO to wake-up. > It is what I have seen going through the sdhci layer. So next question is: is it normal to not take care of card detect interrupts? We keep enabled some IRQs probably for SDIO modules IRQ but not for card detection. I don't understand the reason. > > > > > It seems there are only two sdhci drivers using runtime pm so maybe nobody has > > noticed this issue. > > > > Regards > > > > Ludovic > > > > drivers/mmc/host/sdhci-of-at91.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c > > index 9cb86fb..ae24dea 100644 > > --- a/drivers/mmc/host/sdhci-of-at91.c > > +++ b/drivers/mmc/host/sdhci-of-at91.c > > @@ -210,8 +210,6 @@ static int sdhci_at91_probe(struct platform_device *pdev) > > if (ret) > > goto pm_runtime_disable; > > > > - pm_runtime_put_autosuspend(&pdev->dev); > > - > > return 0; > > > > pm_runtime_disable: > > > -- 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
On 10/02/16 14:51, Ludovic Desroches wrote: > Hi Adrian, > > On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: >> On 10/02/16 11:58, Ludovic Desroches wrote: >>> By putting the device in suspend at the end of the probe, it is >>> impossible to wake up on non software event such as card >>> insertion/removal. >>> >>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> >>> --- >>> >>> Hi, >>> >>> Since I had no feedback on this topic: >>> http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 >>> >>> I would like to no more put the device in suspend at the end of the probe. If >>> my device is suspended at the end of the probe, I have no issue to resume on >>> a software event such as mounting my sdcard but hardware event such as card >>> insertion and removal do not trigger a resume. >> >> You can't use runtime PM unless you have a way to wake-up. >> > > Thanks for your feedback. I am a bit disappointed since Ulf advised me to use > runtime PM instead of system PM. > >> Currently, sdhci disables card detect interrupts when runtime suspended, >> and drivers use a card-detect GPIO to wake-up. >> > > It is what I have seen going through the sdhci layer. So next question is: > is it normal to not take care of card detect interrupts? We keep enabled > some IRQs probably for SDIO modules IRQ but not for card detection. I > don't understand the reason. Does sdhci-of-at91.c generate card detect interrupts while runtime suspended? -- 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
On Wed, Feb 10, 2016 at 03:00:09PM +0200, Adrian Hunter wrote: > On 10/02/16 14:51, Ludovic Desroches wrote: > > Hi Adrian, > > > > On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: > >> On 10/02/16 11:58, Ludovic Desroches wrote: > >>> By putting the device in suspend at the end of the probe, it is > >>> impossible to wake up on non software event such as card > >>> insertion/removal. > >>> > >>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > >>> --- > >>> > >>> Hi, > >>> > >>> Since I had no feedback on this topic: > >>> http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 > >>> > >>> I would like to no more put the device in suspend at the end of the probe. If > >>> my device is suspended at the end of the probe, I have no issue to resume on > >>> a software event such as mounting my sdcard but hardware event such as card > >>> insertion and removal do not trigger a resume. > >> > >> You can't use runtime PM unless you have a way to wake-up. > >> > > > > Thanks for your feedback. I am a bit disappointed since Ulf advised me to use > > runtime PM instead of system PM. > > > >> Currently, sdhci disables card detect interrupts when runtime suspended, > >> and drivers use a card-detect GPIO to wake-up. > >> > > > > It is what I have seen going through the sdhci layer. So next question is: > > is it normal to not take care of card detect interrupts? We keep enabled > > some IRQs probably for SDIO modules IRQ but not for card detection. I > > don't understand the reason. > > Does sdhci-of-at91.c generate card detect interrupts while runtime suspended? It could if I keep one or two clocks enabled (need extra tests to be sure about the clock needed to get this interrupt). I have tried to modify quickly sdhci_runtime_suspend_host() to enable card insertion/removal, there is an irq generated but the kernel complains this irq is not handled. I didn't dig further, it was only to do a simple test. > > -- > 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 -- 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
On 10/02/16 16:16, Ludovic Desroches wrote: > On Wed, Feb 10, 2016 at 03:00:09PM +0200, Adrian Hunter wrote: >> On 10/02/16 14:51, Ludovic Desroches wrote: >>> Hi Adrian, >>> >>> On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: >>>> On 10/02/16 11:58, Ludovic Desroches wrote: >>>>> By putting the device in suspend at the end of the probe, it is >>>>> impossible to wake up on non software event such as card >>>>> insertion/removal. >>>>> >>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> >>>>> --- >>>>> >>>>> Hi, >>>>> >>>>> Since I had no feedback on this topic: >>>>> http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 >>>>> >>>>> I would like to no more put the device in suspend at the end of the probe. If >>>>> my device is suspended at the end of the probe, I have no issue to resume on >>>>> a software event such as mounting my sdcard but hardware event such as card >>>>> insertion and removal do not trigger a resume. >>>> >>>> You can't use runtime PM unless you have a way to wake-up. >>>> >>> >>> Thanks for your feedback. I am a bit disappointed since Ulf advised me to use >>> runtime PM instead of system PM. >>> >>>> Currently, sdhci disables card detect interrupts when runtime suspended, >>>> and drivers use a card-detect GPIO to wake-up. >>>> >>> >>> It is what I have seen going through the sdhci layer. So next question is: >>> is it normal to not take care of card detect interrupts? We keep enabled >>> some IRQs probably for SDIO modules IRQ but not for card detection. I >>> don't understand the reason. >> >> Does sdhci-of-at91.c generate card detect interrupts while runtime suspended? > > It could if I keep one or two clocks enabled (need extra tests to be > sure about the clock needed to get this interrupt). I have tried to modify > quickly sdhci_runtime_suspend_host() to enable card insertion/removal, > there is an irq generated but the kernel complains this irq is not > handled. I didn't dig further, it was only to do a simple test. sdhci_irq() returns immediately if (host->runtime_suspended && !sdhci_sdio_irq_enabled(host)) -- 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
On Wed, Feb 10, 2016 at 04:20:43PM +0200, Adrian Hunter wrote: > On 10/02/16 16:16, Ludovic Desroches wrote: > > On Wed, Feb 10, 2016 at 03:00:09PM +0200, Adrian Hunter wrote: > >> On 10/02/16 14:51, Ludovic Desroches wrote: > >>> Hi Adrian, > >>> > >>> On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: > >>>> On 10/02/16 11:58, Ludovic Desroches wrote: > >>>>> By putting the device in suspend at the end of the probe, it is > >>>>> impossible to wake up on non software event such as card > >>>>> insertion/removal. > >>>>> > >>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > >>>>> --- > >>>>> > >>>>> Hi, > >>>>> > >>>>> Since I had no feedback on this topic: > >>>>> http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 > >>>>> > >>>>> I would like to no more put the device in suspend at the end of the probe. If > >>>>> my device is suspended at the end of the probe, I have no issue to resume on > >>>>> a software event such as mounting my sdcard but hardware event such as card > >>>>> insertion and removal do not trigger a resume. > >>>> > >>>> You can't use runtime PM unless you have a way to wake-up. > >>>> > >>> > >>> Thanks for your feedback. I am a bit disappointed since Ulf advised me to use > >>> runtime PM instead of system PM. > >>> > >>>> Currently, sdhci disables card detect interrupts when runtime suspended, > >>>> and drivers use a card-detect GPIO to wake-up. > >>>> > >>> > >>> It is what I have seen going through the sdhci layer. So next question is: > >>> is it normal to not take care of card detect interrupts? We keep enabled > >>> some IRQs probably for SDIO modules IRQ but not for card detection. I > >>> don't understand the reason. > >> > >> Does sdhci-of-at91.c generate card detect interrupts while runtime suspended? > > > > It could if I keep one or two clocks enabled (need extra tests to be > > sure about the clock needed to get this interrupt). I have tried to modify > > quickly sdhci_runtime_suspend_host() to enable card insertion/removal, > > there is an irq generated but the kernel complains this irq is not > > handled. I didn't dig further, it was only to do a simple test. > > sdhci_irq() returns immediately if (host->runtime_suspended && > !sdhci_sdio_irq_enabled(host)) Ok so do I have to rework this part to allow wake up on card detect or do I have to remove runtime PM from my driver? > > -- > 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 -- 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
On 10 February 2016 at 13:51, Ludovic Desroches <ludovic.desroches@atmel.com> wrote: > Hi Adrian, > > On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: >> On 10/02/16 11:58, Ludovic Desroches wrote: >> > By putting the device in suspend at the end of the probe, it is >> > impossible to wake up on non software event such as card >> > insertion/removal. >> > >> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> >> > --- >> > >> > Hi, >> > >> > Since I had no feedback on this topic: >> > http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 >> > >> > I would like to no more put the device in suspend at the end of the probe. If >> > my device is suspended at the end of the probe, I have no issue to resume on >> > a software event such as mounting my sdcard but hardware event such as card >> > insertion and removal do not trigger a resume. >> >> You can't use runtime PM unless you have a way to wake-up. >> > > Thanks for your feedback. I am a bit disappointed since Ulf advised me to use > runtime PM instead of system PM. Sorry about that, but of course I can't know exactly how your hardware work. > >> Currently, sdhci disables card detect interrupts when runtime suspended, >> and drivers use a card-detect GPIO to wake-up. >> > > It is what I have seen going through the sdhci layer. So next question is: > is it normal to not take care of card detect interrupts? We keep enabled > some IRQs probably for SDIO modules IRQ but not for card detection. I > don't understand the reason. If SDIO IRQ is enabled the mmc controller needs to stay runtime resumed (as it's the mmc controller that monitors the IRQ), unless you can re-configure the SDIO IRQ as a wakeup. For example by re-routing it to a GPIO irq. Whether this wakeup configuration can stay the same between system PM and runtime PM is SoC dependent. Regarding card detects in runtime PM: If you have an option to use GPIO IRQs or it's possible to configure the card detect IRQ as a wakeup in any other way, runtime PM works fine. Now, when the card detect *can't* be configured as a wakeup in runtime suspend mode, there are two options. 1) Rely on using MMC_CAP_NEEDS_POLL. 2) Prevent runtime PM. Which option that's preferred is SoC/ mmc controller dependent. Although but please consider below recommendations. - In some cases using polling works really well, as the the mmc core get fast/easy information about whether a card is inserted or not, via the ->get_cd() host ops. - In some cases ->get_cd() is broken (or not implemented) and always returns a value indicating a card is inserted. That means external regulators for the card must be enabled and a card initialization sequence needs to be executed to find out whether a card was *really* inserted. So to conclude, if the controller supports card detection but without wakeup support and the polling mode sucks, then it probably better to prevent runtime PM. Otherwise polling is probably better. Regarding card detect in system PM: If the SoC supports card detect IRQs being configured as wakeup, it's recommended to do that! If it can't support wakeup, no worries! The mmc core will run a check for card insert/removal after a system PM resume sequence is completed. > >> >> > >> > It seems there are only two sdhci drivers using runtime pm so maybe nobody has >> > noticed this issue. In more general, I wouldn't be surprised if the PM related code in sdhci is fragile/broken for some SoC/sdhci variants. Although, it's just an impression I get by studying the code. >> > >> > Regards >> > >> > Ludovic >> > >> > drivers/mmc/host/sdhci-of-at91.c | 2 -- >> > 1 file changed, 2 deletions(-) >> > >> > diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c >> > index 9cb86fb..ae24dea 100644 >> > --- a/drivers/mmc/host/sdhci-of-at91.c >> > +++ b/drivers/mmc/host/sdhci-of-at91.c >> > @@ -210,8 +210,6 @@ static int sdhci_at91_probe(struct platform_device *pdev) >> > if (ret) >> > goto pm_runtime_disable; >> > >> > - pm_runtime_put_autosuspend(&pdev->dev); >> > - 1) Before doing this consider the option to use polling. (MMC_CAP_NEEDS_POLL). 2) The driver may also handle non-removable cards (depending on the SoC configuration). In those cases it's perfectly okay to use runtime PM. So, I think you need some conditional check before deciding to prevent runtime PM. And don't forget to restore the usage count in ->remove(). >> > return 0; >> > >> > pm_runtime_disable: >> > >> Kind regards Uffe -- 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
Hi Ulf, On Wed, Feb 10, 2016 at 04:56:11PM +0100, Ulf Hansson wrote: > On 10 February 2016 at 13:51, Ludovic Desroches > <ludovic.desroches@atmel.com> wrote: > > Hi Adrian, > > > > On Wed, Feb 10, 2016 at 01:50:44PM +0200, Adrian Hunter wrote: > >> On 10/02/16 11:58, Ludovic Desroches wrote: > >> > By putting the device in suspend at the end of the probe, it is > >> > impossible to wake up on non software event such as card > >> > insertion/removal. > >> > > >> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > >> > --- > >> > > >> > Hi, > >> > > >> > Since I had no feedback on this topic: > >> > http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 > >> > > >> > I would like to no more put the device in suspend at the end of the probe. If > >> > my device is suspended at the end of the probe, I have no issue to resume on > >> > a software event such as mounting my sdcard but hardware event such as card > >> > insertion and removal do not trigger a resume. > >> > >> You can't use runtime PM unless you have a way to wake-up. > >> > > > > Thanks for your feedback. I am a bit disappointed since Ulf advised me to use > > runtime PM instead of system PM. > > Sorry about that, but of course I can't know exactly how your hardware work. > Of course you can't know the hardware of all SoC vendors! No problem about that. In my mind it is not dependant of how my hardware works (I mean at SoC level), if someone decides to not use the card detect of the host and to use a gpio instead, of course he can. > > > >> Currently, sdhci disables card detect interrupts when runtime suspended, > >> and drivers use a card-detect GPIO to wake-up. > >> > > > > It is what I have seen going through the sdhci layer. So next question is: > > is it normal to not take care of card detect interrupts? We keep enabled > > some IRQs probably for SDIO modules IRQ but not for card detection. I > > don't understand the reason. > > If SDIO IRQ is enabled the mmc controller needs to stay runtime > resumed (as it's the mmc controller that monitors the IRQ), unless you > can re-configure the SDIO IRQ as a wakeup. For example by re-routing > it to a GPIO irq. > Whether this wakeup configuration can stay the same between system PM > and runtime PM is SoC dependent. I don't know if we are talking about the same thing. In sdhci_runtime_suspend_host(), we set SDHCI_INT_CARD_INT as a kind of wakeup irq before considering the host as suspended (host->runtime_suspended = true), isn't it? Why not adding SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE? In my case, it allows me to use runtime PM to disable two clocks and keep one enabled (I need this one to get the irq) when suspending the device. > > Regarding card detects in runtime PM: > > If you have an option to use GPIO IRQs or it's possible to configure > the card detect IRQ as a wakeup in any other way, runtime PM works > fine. > It will depend of the customer but I am not sure I'll want to use a pio as a gpio for this if there is a pio routed to the sdhci controller. > Now, when the card detect *can't* be configured as a wakeup in runtime > suspend mode, there are two options. > > 1) Rely on using MMC_CAP_NEEDS_POLL. > 2) Prevent runtime PM. > > Which option that's preferred is SoC/ mmc controller dependent. > Although but please consider below recommendations. > > - In some cases using polling works really well, as the the mmc core > get fast/easy information about whether a card is inserted or not, via > the ->get_cd() host ops. > > - In some cases ->get_cd() is broken (or not implemented) and always > returns a value indicating a card is inserted. That means external > regulators for the card must be enabled and a card initialization > sequence needs to be executed to find out whether a card was *really* > inserted. > > So to conclude, if the controller supports card detection but without > wakeup support and the polling mode sucks, then it probably better to > prevent runtime PM. Otherwise polling is probably better. > It is weird to claim that I need polling since I have a working card detect. > Regarding card detect in system PM: > If the SoC supports card detect IRQs being configured as wakeup, it's > recommended to do that! > > If it can't support wakeup, no worries! The mmc core will run a check > for card insert/removal after a system PM resume sequence is > completed. > > > > >> > >> > > >> > It seems there are only two sdhci drivers using runtime pm so maybe nobody has > >> > noticed this issue. > > In more general, I wouldn't be surprised if the PM related code in > sdhci is fragile/broken for some SoC/sdhci variants. Although, it's > just an impression I get by studying the code. > > >> > > >> > Regards > >> > > >> > Ludovic > >> > > >> > drivers/mmc/host/sdhci-of-at91.c | 2 -- > >> > 1 file changed, 2 deletions(-) > >> > > >> > diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c > >> > index 9cb86fb..ae24dea 100644 > >> > --- a/drivers/mmc/host/sdhci-of-at91.c > >> > +++ b/drivers/mmc/host/sdhci-of-at91.c > >> > @@ -210,8 +210,6 @@ static int sdhci_at91_probe(struct platform_device *pdev) > >> > if (ret) > >> > goto pm_runtime_disable; > >> > > >> > - pm_runtime_put_autosuspend(&pdev->dev); > >> > - > > 1) Before doing this consider the option to use polling. (MMC_CAP_NEEDS_POLL). > > 2) The driver may also handle non-removable cards (depending on the > SoC configuration). In those cases it's perfectly okay to use runtime > PM. So, I think you need some conditional check before deciding to > prevent runtime PM. And don't forget to restore the usage count in > ->remove(). > I have to think more about the other cases. I know we have some issues with a wifi SDIO module and runtime PM but I have to investigate on this topic but I suspect something as the wifi driver doesn't claim it is using the sdhci controller. > >> > return 0; > >> > > >> > pm_runtime_disable: > >> > > >> > > Kind regards > Uffe Regards Ludovic -- 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
[...] >> > >> >> Currently, sdhci disables card detect interrupts when runtime suspended, >> >> and drivers use a card-detect GPIO to wake-up. >> >> >> > >> > It is what I have seen going through the sdhci layer. So next question is: >> > is it normal to not take care of card detect interrupts? We keep enabled >> > some IRQs probably for SDIO modules IRQ but not for card detection. I >> > don't understand the reason. >> >> If SDIO IRQ is enabled the mmc controller needs to stay runtime >> resumed (as it's the mmc controller that monitors the IRQ), unless you >> can re-configure the SDIO IRQ as a wakeup. For example by re-routing >> it to a GPIO irq. >> Whether this wakeup configuration can stay the same between system PM >> and runtime PM is SoC dependent. > > I don't know if we are talking about the same thing. In > sdhci_runtime_suspend_host(), we set SDHCI_INT_CARD_INT as a kind of > wakeup irq before considering the host as suspended > (host->runtime_suspended = true), isn't it? No, you have got this wrong. The card detect IRQ is disabled at sdhci_runtime_suspend_host(). It's not related to SDIO irq, as in that case the controller can't be runtime suspended (unless wakeup is supported). > > Why not adding SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE? > > In my case, it allows me to use runtime PM to disable two clocks and > keep one enabled (I need this one to get the irq) when suspending the device. That seems like a very special case and normally not how runtime PM is used. So, if you only want to disable|enable some clocks from runtime PM, I suggest you keep that out of the library function sdhci_runtime_suspend_host() and deal with that in your driver instead. > >> >> Regarding card detects in runtime PM: >> >> If you have an option to use GPIO IRQs or it's possible to configure >> the card detect IRQ as a wakeup in any other way, runtime PM works >> fine. >> > > It will depend of the customer but I am not sure I'll want to use a pio > as a gpio for this if there is a pio routed to the sdhci controller. That all has to do with how the SoC is designed from power management point of view. In general, it's a good idea to have card detect on GPIO, as it allows to put other unused parts of the silicon into a low power state. > >> Now, when the card detect *can't* be configured as a wakeup in runtime >> suspend mode, there are two options. >> >> 1) Rely on using MMC_CAP_NEEDS_POLL. >> 2) Prevent runtime PM. >> >> Which option that's preferred is SoC/ mmc controller dependent. >> Although but please consider below recommendations. >> >> - In some cases using polling works really well, as the the mmc core >> get fast/easy information about whether a card is inserted or not, via >> the ->get_cd() host ops. >> >> - In some cases ->get_cd() is broken (or not implemented) and always >> returns a value indicating a card is inserted. That means external >> regulators for the card must be enabled and a card initialization >> sequence needs to be executed to find out whether a card was *really* >> inserted. >> >> So to conclude, if the controller supports card detection but without >> wakeup support and the polling mode sucks, then it probably better to >> prevent runtime PM. Otherwise polling is probably better. >> > > It is weird to claim that I need polling since I have a working card > detect. It's not, if you really care about saving power. Although, as I stated, which solution that's best, depends on the SoC. [...] So, we have a regression to fix here. I can propose a patch adopting the above recommendations!? That's solution doesn’t have to stay long term, as you can try to optimize it later on to what suits your SoC the best. Kind regards Uffe -- 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
On Thu, Feb 11, 2016 at 10:46:08AM +0100, Ulf Hansson wrote: > [...] > > >> > > >> >> Currently, sdhci disables card detect interrupts when runtime suspended, > >> >> and drivers use a card-detect GPIO to wake-up. > >> >> > >> > > >> > It is what I have seen going through the sdhci layer. So next question is: > >> > is it normal to not take care of card detect interrupts? We keep enabled > >> > some IRQs probably for SDIO modules IRQ but not for card detection. I > >> > don't understand the reason. > >> > >> If SDIO IRQ is enabled the mmc controller needs to stay runtime > >> resumed (as it's the mmc controller that monitors the IRQ), unless you > >> can re-configure the SDIO IRQ as a wakeup. For example by re-routing > >> it to a GPIO irq. > >> Whether this wakeup configuration can stay the same between system PM > >> and runtime PM is SoC dependent. > > > > I don't know if we are talking about the same thing. In > > sdhci_runtime_suspend_host(), we set SDHCI_INT_CARD_INT as a kind of > > wakeup irq before considering the host as suspended > > (host->runtime_suspended = true), isn't it? > > No, you have got this wrong. The card detect IRQ is disabled at > sdhci_runtime_suspend_host(). > Ok for card detect. For my knowledge, what is the purpose of enabling SHDCI_INT_CARD_INT? > It's not related to SDIO irq, as in that case the controller can't be > runtime suspended (unless wakeup is supported). > > > > > Why not adding SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE? > > > > In my case, it allows me to use runtime PM to disable two clocks and > > keep one enabled (I need this one to get the irq) when suspending the device. > > That seems like a very special case and normally not how runtime PM is used. > > So, if you only want to disable|enable some clocks from runtime PM, I > suggest you keep that out of the library function > sdhci_runtime_suspend_host() and deal with that in your driver > instead. > > > > >> > >> Regarding card detects in runtime PM: > >> > >> If you have an option to use GPIO IRQs or it's possible to configure > >> the card detect IRQ as a wakeup in any other way, runtime PM works > >> fine. > >> > > > > It will depend of the customer but I am not sure I'll want to use a pio > > as a gpio for this if there is a pio routed to the sdhci controller. > > That all has to do with how the SoC is designed from power management > point of view. > I don't agree this point. We have connected the card detect to the controller on our Xplained board but someone can do another board and use a gpio. So depending on the board we should use runtime PM or not. As you suggested, I have to find a clever way to know when I can use runtime PM or not: non removable device, gpio cd, etc. > In general, it's a good idea to have card detect on GPIO, as it allows > to put other unused parts of the silicon into a low power state. > > > > >> Now, when the card detect *can't* be configured as a wakeup in runtime > >> suspend mode, there are two options. > >> > >> 1) Rely on using MMC_CAP_NEEDS_POLL. > >> 2) Prevent runtime PM. > >> > >> Which option that's preferred is SoC/ mmc controller dependent. > >> Although but please consider below recommendations. > >> > >> - In some cases using polling works really well, as the the mmc core > >> get fast/easy information about whether a card is inserted or not, via > >> the ->get_cd() host ops. > >> > >> - In some cases ->get_cd() is broken (or not implemented) and always > >> returns a value indicating a card is inserted. That means external > >> regulators for the card must be enabled and a card initialization > >> sequence needs to be executed to find out whether a card was *really* > >> inserted. > >> > >> So to conclude, if the controller supports card detection but without > >> wakeup support and the polling mode sucks, then it probably better to > >> prevent runtime PM. Otherwise polling is probably better. > >> > > > > It is weird to claim that I need polling since I have a working card > > detect. > > It's not, if you really care about saving power. > > Although, as I stated, which solution that's best, depends on the SoC. > > [...] > > So, we have a regression to fix here. I can propose a patch adopting > the above recommendations!? > > That's solution doesn’t have to stay long term, as you can try to > optimize it later on to what suits your SoC the best. Ok I'll have a try with MMC_CAP_NEEDS_POLL and after the investigation about the sdio module I'll try to propose something better. Regards Ludovic -- 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
diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c index 9cb86fb..ae24dea 100644 --- a/drivers/mmc/host/sdhci-of-at91.c +++ b/drivers/mmc/host/sdhci-of-at91.c @@ -210,8 +210,6 @@ static int sdhci_at91_probe(struct platform_device *pdev) if (ret) goto pm_runtime_disable; - pm_runtime_put_autosuspend(&pdev->dev); - return 0; pm_runtime_disable:
By putting the device in suspend at the end of the probe, it is impossible to wake up on non software event such as card insertion/removal. Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> --- Hi, Since I had no feedback on this topic: http://permalink.gmane.org/gmane.linux.kernel.mmc/35160 I would like to no more put the device in suspend at the end of the probe. If my device is suspended at the end of the probe, I have no issue to resume on a software event such as mounting my sdcard but hardware event such as card insertion and removal do not trigger a resume. It seems there are only two sdhci drivers using runtime pm so maybe nobody has noticed this issue. Regards Ludovic drivers/mmc/host/sdhci-of-at91.c | 2 -- 1 file changed, 2 deletions(-)