| Message ID | 1375994855-12472-1-git-send-email-csd@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 8 Aug 2013, Christian Daudt wrote: > Given that mmc_gpio_get_cd can be called in softirq > context (by sdhci_tasklet_card -> sdhci_card_event -> > sdhci_do_get_cd -> mmc_gpio_get_cd ), it is necessary > for it to use gpio_get_value instead of > gpio_get_value_cansleep > Note that at present sdhci_card_event gets called both > from mmc_gpio_cd_irqt and sdhci_tasklet_card, and from > both it gets called immediately while the actual cd > processing is debounced to 200ms later. I think that > the better solution might be to move the sdhci_card_event > callback into mmc_rescan and remove it from its 2 > present locations. That way the cd related callbacks > are aligned with the actual cd detection code. I can > submit a follow-up patch with these mods if that sounds > like a better way to solve this. > > Signed-off-by: Christian Daudt <csd@broadcom.com> I don't think this is a right approach. It will Oops if the card-detect GPIO is provided by, e.g. an I2C GPIO controller. Instead the driver should be fixed to only call this function from a sleeping context. Thanks Guennadi > > diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c > index 3242351..897b298 100644 > --- a/drivers/mmc/core/slot-gpio.c > +++ b/drivers/mmc/core/slot-gpio.c > @@ -87,7 +87,7 @@ int mmc_gpio_get_cd(struct mmc_host *host) > if (!ctx || !gpio_is_valid(ctx->cd_gpio)) > return -ENOSYS; > > - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ > + return !gpio_get_value(ctx->cd_gpio) ^ > !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); > } > EXPORT_SYMBOL(mmc_gpio_get_cd); > -- > 1.7.10.4 > > > -- > 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 > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 13-08-08 02:06 PM, Guennadi Liakhovetski wrote: > On Thu, 8 Aug 2013, Christian Daudt wrote: > >> Given that mmc_gpio_get_cd can be called in softirq >> context (by sdhci_tasklet_card -> sdhci_card_event -> >> sdhci_do_get_cd -> mmc_gpio_get_cd ), it is necessary >> for it to use gpio_get_value instead of >> gpio_get_value_cansleep >> Note that at present sdhci_card_event gets called both >> from mmc_gpio_cd_irqt and sdhci_tasklet_card, and from >> both it gets called immediately while the actual cd >> processing is debounced to 200ms later. I think that >> the better solution might be to move the sdhci_card_event >> callback into mmc_rescan and remove it from its 2 >> present locations. That way the cd related callbacks >> are aligned with the actual cd detection code. I can >> submit a follow-up patch with these mods if that sounds >> like a better way to solve this. >> >> Signed-off-by: Christian Daudt <csd@broadcom.com> > I don't think this is a right approach. It will Oops if the card-detect > GPIO is provided by, e.g. an I2C GPIO controller. Instead the driver > should be fixed to only call this function from a sleeping context. I don't think that card-detect would ever be tied to i2c but I might be wrong on that one... If there's agreement that moving sdhci_card_event into just being called from mmc_rescan is the better way to solve this, then I'll drop this patch and replace it with one that does just that. Thanks, csd -- 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 13-08-08 02:10 PM, Christian Daudt wrote: > On 13-08-08 02:06 PM, Guennadi Liakhovetski wrote: >> On Thu, 8 Aug 2013, Christian Daudt wrote: >> >>> Given that mmc_gpio_get_cd can be called in softirq >>> context (by sdhci_tasklet_card -> sdhci_card_event -> >>> sdhci_do_get_cd -> mmc_gpio_get_cd ), it is necessary >>> for it to use gpio_get_value instead of >>> gpio_get_value_cansleep >>> Note that at present sdhci_card_event gets called both >>> from mmc_gpio_cd_irqt and sdhci_tasklet_card, and from >>> both it gets called immediately while the actual cd >>> processing is debounced to 200ms later. I think that >>> the better solution might be to move the sdhci_card_event >>> callback into mmc_rescan and remove it from its 2 >>> present locations. That way the cd related callbacks >>> are aligned with the actual cd detection code. I can >>> submit a follow-up patch with these mods if that sounds >>> like a better way to solve this. >>> >>> Signed-off-by: Christian Daudt <csd@broadcom.com> >> I don't think this is a right approach. It will Oops if the card-detect >> GPIO is provided by, e.g. an I2C GPIO controller. Instead the driver >> should be fixed to only call this function from a sleeping context. > I don't think that card-detect would ever be tied to i2c but I might > be wrong on that one... If there's agreement that moving > sdhci_card_event into just being called from mmc_rescan is the better > way to solve this, then I'll drop this patch and replace it with one > that does just that. > > Thanks, > csd > [adding arm@kernel.org for more input] Guennadi, While I can't see card-detect coming in through i2c gpio controller, I think that moving this out of the sleeping context (and into mmc_rescan) is probably the better solution anyways. So I'll drop this patch and send another one with that change instead. Thanks, csd -- 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/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c index 3242351..897b298 100644 --- a/drivers/mmc/core/slot-gpio.c +++ b/drivers/mmc/core/slot-gpio.c @@ -87,7 +87,7 @@ int mmc_gpio_get_cd(struct mmc_host *host) if (!ctx || !gpio_is_valid(ctx->cd_gpio)) return -ENOSYS; - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ + return !gpio_get_value(ctx->cd_gpio) ^ !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); } EXPORT_SYMBOL(mmc_gpio_get_cd);
Given that mmc_gpio_get_cd can be called in softirq context (by sdhci_tasklet_card -> sdhci_card_event -> sdhci_do_get_cd -> mmc_gpio_get_cd ), it is necessary for it to use gpio_get_value instead of gpio_get_value_cansleep Note that at present sdhci_card_event gets called both from mmc_gpio_cd_irqt and sdhci_tasklet_card, and from both it gets called immediately while the actual cd processing is debounced to 200ms later. I think that the better solution might be to move the sdhci_card_event callback into mmc_rescan and remove it from its 2 present locations. That way the cd related callbacks are aligned with the actual cd detection code. I can submit a follow-up patch with these mods if that sounds like a better way to solve this. Signed-off-by: Christian Daudt <csd@broadcom.com>