| Message ID | E1TbgcZ-0007Kt-35@rmk-PC.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 22, 2012 at 11:55:51PM +0000, Russell King wrote: > This commit taken from Rabeeh's Cubox kernel and re-worked for DT; > Sebastian Hasselbrath is believed to be the original author. > > Some Cuboxes require a GPIO for card detection; this implements the > optional GPIO support for card detection. This GPIO is logic 0 for > card inserted. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > drivers/mmc/host/sdhci-dove.c | 73 +++++++++++++++++++++++++++++++++++++--- > 1 files changed, 67 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c > index 4af64f3..e621448 100644 > --- a/drivers/mmc/host/sdhci-dove.c > +++ b/drivers/mmc/host/sdhci-dove.c > @@ -19,20 +19,30 @@ > * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > */ > > -#include <linux/err.h> > -#include <linux/io.h> > #include <linux/clk.h> > #include <linux/err.h> > -#include <linux/module.h> > +#include <linux/gpio.h> > +#include <linux/io.h> > #include <linux/mmc/host.h> > +#include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_gpio.h> > > #include "sdhci-pltfm.h" > > struct sdhci_dove_priv { > struct clk *clk; > + int gpio_cd; > }; > > +static irqreturn_t sdhci_dove_carddetect_irq(int irq, void *data) > +{ > + struct sdhci_host *host = data; > + > + tasklet_schedule(&host->card_tasklet); > + return IRQ_HANDLED; > +} > + > static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) > { > u16 ret; > @@ -50,16 +60,25 @@ static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) > > static u32 sdhci_dove_readl(struct sdhci_host *host, int reg) > { > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_dove_priv *priv = pltfm_host->priv; > u32 ret; > > + ret = readl(host->ioaddr + reg); > + > switch (reg) { > case SDHCI_CAPABILITIES: > - ret = readl(host->ioaddr + reg); > /* Mask the support for 3.0V */ > ret &= ~SDHCI_CAN_VDD_300; > break; > - default: > - ret = readl(host->ioaddr + reg); > + case SDHCI_PRESENT_STATE: > + if (gpio_is_valid(priv->gpio_cd)) { > + if (gpio_get_value(priv->gpio_cd) == 0) > + ret |= SDHCI_CARD_PRESENT; > + else > + ret &= ~SDHCI_CARD_PRESENT; > + } > + break; It seems that the helpers in drivers/mmc/core/slot-gpio.c can help to simplify the patch a lot. With sdhci core code being calling mmc_gpio_get_cd() to query card present, the fake SDHCI_PRESENT_STATE implementation above can be totally saved. Shawn > } > return ret; > } > @@ -94,6 +113,23 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev) > > priv->clk = devm_clk_get(&pdev->dev, NULL); > > + if (pdev->dev.of_node) { > + priv->gpio_cd = of_get_named_gpio(pdev->dev.of_node, > + "cd-gpios", 0); > + } else { > + priv->gpio_cd = -EINVAL; > + } > + > + if (gpio_is_valid(priv->gpio_cd)) { > + ret = gpio_request(priv->gpio_cd, "sdhci-cd"); > + if (ret) { > + dev_err(&pdev->dev, "card detect gpio request failed: %d\n", > + ret); > + return ret; > + } > + gpio_direction_input(priv->gpio_cd); > + } > + > host = sdhci_pltfm_init(pdev, &sdhci_dove_pdata); > if (IS_ERR(host)) { > ret = PTR_ERR(host); > @@ -112,13 +148,33 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev) > if (ret) > goto err_sdhci_add; > > + /* > + * We must request the IRQ after sdhci_add_host(), as the tasklet only > + * gets setup in sdhci_add_host() and we oops. > + */ > + if (gpio_is_valid(priv->gpio_cd)) { > + ret = request_irq(gpio_to_irq(priv->gpio_cd), > + sdhci_dove_carddetect_irq, > + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > + mmc_hostname(host->mmc), host); > + if (ret) { > + dev_err(&pdev->dev, "card detect irq request failed: %d\n", > + ret); > + goto err_request_irq; > + } > + } > + > return 0; > > +err_request_irq: > + sdhci_remove_host(host, 0); > err_sdhci_add: > if (!IS_ERR(priv->clk)) > clk_disable_unprepare(priv->clk); > sdhci_pltfm_free(pdev); > err_sdhci_pltfm_init: > + if (gpio_is_valid(priv->gpio_cd)) > + gpio_free(priv->gpio_cd); > return ret; > } > > @@ -130,6 +186,11 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev) > > sdhci_pltfm_unregister(pdev); > > + if (gpio_is_valid(priv->gpio_cd)) { > + free_irq(gpio_to_irq(priv->gpio_cd), host); > + gpio_free(priv->gpio_cd); > + } > + > if (!IS_ERR(priv->clk)) > clk_disable_unprepare(priv->clk); > > -- > 1.7.4.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
On Fri, Nov 23, 2012 at 04:31:20PM +0800, Shawn Guo wrote: > On Thu, Nov 22, 2012 at 11:55:51PM +0000, Russell King wrote: > > This commit taken from Rabeeh's Cubox kernel and re-worked for DT; > > Sebastian Hasselbrath is believed to be the original author. > > > > Some Cuboxes require a GPIO for card detection; this implements the > > optional GPIO support for card detection. This GPIO is logic 0 for > > card inserted. > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > drivers/mmc/host/sdhci-dove.c | 73 +++++++++++++++++++++++++++++++++++++--- > > 1 files changed, 67 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c > > index 4af64f3..e621448 100644 > > --- a/drivers/mmc/host/sdhci-dove.c > > +++ b/drivers/mmc/host/sdhci-dove.c > > @@ -19,20 +19,30 @@ > > * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > > */ > > > > -#include <linux/err.h> > > -#include <linux/io.h> > > #include <linux/clk.h> > > #include <linux/err.h> > > -#include <linux/module.h> > > +#include <linux/gpio.h> > > +#include <linux/io.h> > > #include <linux/mmc/host.h> > > +#include <linux/module.h> > > #include <linux/of.h> > > +#include <linux/of_gpio.h> > > > > #include "sdhci-pltfm.h" > > > > struct sdhci_dove_priv { > > struct clk *clk; > > + int gpio_cd; > > }; > > > > +static irqreturn_t sdhci_dove_carddetect_irq(int irq, void *data) > > +{ > > + struct sdhci_host *host = data; > > + > > + tasklet_schedule(&host->card_tasklet); > > + return IRQ_HANDLED; > > +} > > + > > static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) > > { > > u16 ret; > > @@ -50,16 +60,25 @@ static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) > > > > static u32 sdhci_dove_readl(struct sdhci_host *host, int reg) > > { > > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > + struct sdhci_dove_priv *priv = pltfm_host->priv; > > u32 ret; > > > > + ret = readl(host->ioaddr + reg); > > + > > switch (reg) { > > case SDHCI_CAPABILITIES: > > - ret = readl(host->ioaddr + reg); > > /* Mask the support for 3.0V */ > > ret &= ~SDHCI_CAN_VDD_300; > > break; > > - default: > > - ret = readl(host->ioaddr + reg); > > + case SDHCI_PRESENT_STATE: > > + if (gpio_is_valid(priv->gpio_cd)) { > > + if (gpio_get_value(priv->gpio_cd) == 0) > > + ret |= SDHCI_CARD_PRESENT; > > + else > > + ret &= ~SDHCI_CARD_PRESENT; > > + } > > + break; > > It seems that the helpers in drivers/mmc/core/slot-gpio.c can help to > simplify the patch a lot. With sdhci core code being calling > mmc_gpio_get_cd() to query card present, the fake SDHCI_PRESENT_STATE > implementation above can be totally saved. Does this work with the sdhci stuff? I notice that sdhci does some special additional resetting of the host when it detects that a card is removed; this would be lost if that were to be used because the tasklet in sdhci.c would be bypassed. So no, I don't think slot-gpio.c can be used here.
On Fri, Nov 23, 2012 at 08:57:26AM +0000, Russell King - ARM Linux wrote:
> Does this work with the sdhci stuff?
Honestly, I'm not sure, but I guess it does, since I have seen
sdhci-pxav3 driver using the helpers. Anyway, I'm going to adopt
the helpers for sdhci-esdhc-imx driver to find it out.
Shawn
On Fri, Nov 23, 2012 at 08:14:27PM +0800, Shawn Guo wrote: > On Fri, Nov 23, 2012 at 08:57:26AM +0000, Russell King - ARM Linux wrote: > > Does this work with the sdhci stuff? > > Honestly, I'm not sure, but I guess it does, since I have seen > sdhci-pxav3 driver using the helpers. Anyway, I'm going to adopt > the helpers for sdhci-esdhc-imx driver to find it out. The thing that worries me is this: static void sdhci_tasklet_card(unsigned long param) { ... /* Check host->mrq first in case we are runtime suspended */ if (host->mrq && !(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) { pr_err("%s: Card removed during transfer!\n", mmc_hostname(host->mmc)); pr_err("%s: Resetting controller.\n", mmc_hostname(host->mmc)); sdhci_reset(host, SDHCI_RESET_CMD); sdhci_reset(host, SDHCI_RESET_DATA); host->mrq->cmd->error = -ENOMEDIUM; tasklet_schedule(&host->finish_tasklet); } ... mmc_detect_change(host->mmc, msecs_to_jiffies(200)); } That gets called whenever a card is inserted/removed by the SDHCI code (if the SDHCI card detect is wired), or in my case by the interrupt routine the code in my patch adds. The slot-gpio.c stuff directly calls into mmc_detect_change (a) with a shorter delay, and (b) completely omits the above handling and resetting. My guess from the above code is that it'll work fine 90% of the time (because you'll remove the card without an active request), but the above code looks like it's addressing a corner case which will be omitted with the "generic" slot-gpio.c solution. So I don't think it's a good idea to use slot-gpio.c in this case.
Hi, adding Guennadi, On Fri, Nov 23 2012, Russell King - ARM Linux wrote: > On Fri, Nov 23, 2012 at 08:14:27PM +0800, Shawn Guo wrote: >> On Fri, Nov 23, 2012 at 08:57:26AM +0000, Russell King - ARM Linux wrote: >> > Does this work with the sdhci stuff? >> >> Honestly, I'm not sure, but I guess it does, since I have seen >> sdhci-pxav3 driver using the helpers. Anyway, I'm going to adopt >> the helpers for sdhci-esdhc-imx driver to find it out. > > The thing that worries me is this: > > static void sdhci_tasklet_card(unsigned long param) > { > ... > /* Check host->mrq first in case we are runtime suspended */ > if (host->mrq && > !(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) { > pr_err("%s: Card removed during transfer!\n", > mmc_hostname(host->mmc)); > pr_err("%s: Resetting controller.\n", > mmc_hostname(host->mmc)); > > sdhci_reset(host, SDHCI_RESET_CMD); > sdhci_reset(host, SDHCI_RESET_DATA); > > host->mrq->cmd->error = -ENOMEDIUM; > tasklet_schedule(&host->finish_tasklet); > } > ... > mmc_detect_change(host->mmc, msecs_to_jiffies(200)); > } > > > That gets called whenever a card is inserted/removed by the SDHCI code (if > the SDHCI card detect is wired), or in my case by the interrupt routine > the code in my patch adds. > > The slot-gpio.c stuff directly calls into mmc_detect_change (a) with a > shorter delay, and (b) completely omits the above handling and resetting. > My guess from the above code is that it'll work fine 90% of the time > (because you'll remove the card without an active request), but the above > code looks like it's addressing a corner case which will be omitted with > the "generic" slot-gpio.c solution. > > So I don't think it's a good idea to use slot-gpio.c in this case. Guennadi, what are your thoughts about consolidating this reset logic between the sdhci tasklet and slot-gpio? It would certainly be nice to use slot-gpio in cases like this one, so it's worth fixing. Thanks, - Chris.
On Thu, Nov 22, 2012 at 11:55:51PM +0000, Russell King wrote: > This commit taken from Rabeeh's Cubox kernel and re-worked for DT; > Sebastian Hasselbrath is believed to be the original author. > > Some Cuboxes require a GPIO for card detection; this implements the > optional GPIO support for card detection. This GPIO is logic 0 for > card inserted. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > drivers/mmc/host/sdhci-dove.c | 73 +++++++++++++++++++++++++++++++++++++--- > 1 files changed, 67 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c > index 4af64f3..e621448 100644 > --- a/drivers/mmc/host/sdhci-dove.c > +++ b/drivers/mmc/host/sdhci-dove.c > @@ -19,20 +19,30 @@ > * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > */ > > -#include <linux/err.h> > -#include <linux/io.h> > #include <linux/clk.h> > #include <linux/err.h> > -#include <linux/module.h> > +#include <linux/gpio.h> > +#include <linux/io.h> > #include <linux/mmc/host.h> > +#include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_gpio.h> > > #include "sdhci-pltfm.h" > > struct sdhci_dove_priv { > struct clk *clk; > + int gpio_cd; > }; > > +static irqreturn_t sdhci_dove_carddetect_irq(int irq, void *data) > +{ > + struct sdhci_host *host = data; > + > + tasklet_schedule(&host->card_tasklet); > + return IRQ_HANDLED; > +} > + > static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) > { > u16 ret; > @@ -50,16 +60,25 @@ static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) > > static u32 sdhci_dove_readl(struct sdhci_host *host, int reg) > { > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_dove_priv *priv = pltfm_host->priv; > u32 ret; > > + ret = readl(host->ioaddr + reg); > + > switch (reg) { > case SDHCI_CAPABILITIES: > - ret = readl(host->ioaddr + reg); > /* Mask the support for 3.0V */ > ret &= ~SDHCI_CAN_VDD_300; > break; > - default: > - ret = readl(host->ioaddr + reg); > + case SDHCI_PRESENT_STATE: > + if (gpio_is_valid(priv->gpio_cd)) { > + if (gpio_get_value(priv->gpio_cd) == 0) > + ret |= SDHCI_CARD_PRESENT; > + else > + ret &= ~SDHCI_CARD_PRESENT; > + } > + break; > } > return ret; > } > @@ -94,6 +113,23 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev) > > priv->clk = devm_clk_get(&pdev->dev, NULL); > > + if (pdev->dev.of_node) { > + priv->gpio_cd = of_get_named_gpio(pdev->dev.of_node, > + "cd-gpios", 0); > + } else { > + priv->gpio_cd = -EINVAL; > + } > + > + if (gpio_is_valid(priv->gpio_cd)) { > + ret = gpio_request(priv->gpio_cd, "sdhci-cd"); > + if (ret) { > + dev_err(&pdev->dev, "card detect gpio request failed: %d\n", > + ret); > + return ret; > + } > + gpio_direction_input(priv->gpio_cd); > + } > + > host = sdhci_pltfm_init(pdev, &sdhci_dove_pdata); > if (IS_ERR(host)) { > ret = PTR_ERR(host); > @@ -112,13 +148,33 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev) > if (ret) > goto err_sdhci_add; > > + /* > + * We must request the IRQ after sdhci_add_host(), as the tasklet only > + * gets setup in sdhci_add_host() and we oops. > + */ > + if (gpio_is_valid(priv->gpio_cd)) { > + ret = request_irq(gpio_to_irq(priv->gpio_cd), > + sdhci_dove_carddetect_irq, > + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > + mmc_hostname(host->mmc), host); > + if (ret) { > + dev_err(&pdev->dev, "card detect irq request failed: %d\n", > + ret); > + goto err_request_irq; > + } > + } > + > return 0; > > +err_request_irq: > + sdhci_remove_host(host, 0); > err_sdhci_add: > if (!IS_ERR(priv->clk)) > clk_disable_unprepare(priv->clk); > sdhci_pltfm_free(pdev); > err_sdhci_pltfm_init: > + if (gpio_is_valid(priv->gpio_cd)) > + gpio_free(priv->gpio_cd); > return ret; > } > > @@ -130,6 +186,11 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev) > > sdhci_pltfm_unregister(pdev); > > + if (gpio_is_valid(priv->gpio_cd)) { > + free_irq(gpio_to_irq(priv->gpio_cd), host); > + gpio_free(priv->gpio_cd); > + } > + You can probably use devm_gpio_request() and devm_request_irq() to save these cleanups like what patch #1 is doing? Shawn > if (!IS_ERR(priv->clk)) > clk_disable_unprepare(priv->clk); > > -- > 1.7.4.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
Hi Chris On Sun, 25 Nov 2012, Chris Ball wrote: > Hi, adding Guennadi, > > On Fri, Nov 23 2012, Russell King - ARM Linux wrote: > > On Fri, Nov 23, 2012 at 08:14:27PM +0800, Shawn Guo wrote: > >> On Fri, Nov 23, 2012 at 08:57:26AM +0000, Russell King - ARM Linux wrote: > >> > Does this work with the sdhci stuff? > >> > >> Honestly, I'm not sure, but I guess it does, since I have seen > >> sdhci-pxav3 driver using the helpers. Anyway, I'm going to adopt > >> the helpers for sdhci-esdhc-imx driver to find it out. > > > > The thing that worries me is this: > > > > static void sdhci_tasklet_card(unsigned long param) > > { > > ... > > /* Check host->mrq first in case we are runtime suspended */ > > if (host->mrq && > > !(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) { > > pr_err("%s: Card removed during transfer!\n", > > mmc_hostname(host->mmc)); > > pr_err("%s: Resetting controller.\n", > > mmc_hostname(host->mmc)); > > > > sdhci_reset(host, SDHCI_RESET_CMD); > > sdhci_reset(host, SDHCI_RESET_DATA); > > > > host->mrq->cmd->error = -ENOMEDIUM; > > tasklet_schedule(&host->finish_tasklet); > > } > > ... > > mmc_detect_change(host->mmc, msecs_to_jiffies(200)); > > } > > > > > > That gets called whenever a card is inserted/removed by the SDHCI code (if > > the SDHCI card detect is wired), or in my case by the interrupt routine > > the code in my patch adds. > > > > The slot-gpio.c stuff directly calls into mmc_detect_change (a) with a > > shorter delay, and (b) completely omits the above handling and resetting. > > My guess from the above code is that it'll work fine 90% of the time > > (because you'll remove the card without an active request), but the above > > code looks like it's addressing a corner case which will be omitted with > > the "generic" slot-gpio.c solution. > > > > So I don't think it's a good idea to use slot-gpio.c in this case. > > Guennadi, what are your thoughts about consolidating this reset logic > between the sdhci tasklet and slot-gpio? It would certainly be nice to > use slot-gpio in cases like this one, so it's worth fixing. Sure, this can be added. As for how - I see at least two possibilities: (1) put the complete above block in a new mmc host operation and just call it from the GPIO card-detect ISR, (2) taking into account, that many (nearly all? all?) host drivers keep a pointer to the current mrq in their private data struct, we could instead add such a pointer to struct mmc_host, then the check "request in progress" could also be generalised, and the operation would just have to reset the host and complete the request (in the sdhci case schedule the task). Note, that there's already a .hw_reset() operation, used by sdhci-pci only so far, still, it seems we cannot (easily) hijack it. So, what would be the preferred way? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Hi Guennadi, On Mon, Nov 26 2012, Guennadi Liakhovetski wrote: >> Guennadi, what are your thoughts about consolidating this reset logic >> between the sdhci tasklet and slot-gpio? It would certainly be nice to >> use slot-gpio in cases like this one, so it's worth fixing. > > Sure, this can be added. As for how - I see at least two possibilities: > (1) put the complete above block in a new mmc host operation and just call > it from the GPIO card-detect ISR, (2) taking into account, that many > (nearly all? all?) host drivers keep a pointer to the current mrq in their > private data struct, we could instead add such a pointer to struct > mmc_host, then the check "request in progress" could also be generalised, > and the operation would just have to reset the host and complete the > request (in the sdhci case schedule the task). They both sound pretty attractive. Maybe we start out with (1), which would create a patch we could more reasonably send to stable@ to get slot-gpio handling the reset during transfers properly in older kernels, and then refactor into (2) later? > Note, that there's already a .hw_reset() operation, used by sdhci-pci > only so far, still, it seems we cannot (easily) hijack it. That one's implementing an eMMC 4.5 spec feature, not related to this kind of reset. Thanks! - Chris.
Hi Chris On Mon, 3 Dec 2012, Chris Ball wrote: > Hi Guennadi, > > On Mon, Nov 26 2012, Guennadi Liakhovetski wrote: > >> Guennadi, what are your thoughts about consolidating this reset logic > >> between the sdhci tasklet and slot-gpio? It would certainly be nice to > >> use slot-gpio in cases like this one, so it's worth fixing. > > > > Sure, this can be added. As for how - I see at least two possibilities: > > (1) put the complete above block in a new mmc host operation and just call > > it from the GPIO card-detect ISR, (2) taking into account, that many > > (nearly all? all?) host drivers keep a pointer to the current mrq in their > > private data struct, we could instead add such a pointer to struct > > mmc_host, then the check "request in progress" could also be generalised, > > and the operation would just have to reset the host and complete the > > request (in the sdhci case schedule the task). > > They both sound pretty attractive. Maybe we start out with (1), which > would create a patch we could more reasonably send to stable@ to get > slot-gpio handling the reset during transfers properly in older kernels, > and then refactor into (2) later? Just posted 3 patches for this, have a look if that's what you were thinking about. Not sure though why this is needed for stable, but I'm probably just missing some crucial information on the topic. Thanks Guennadi > > Note, that there's already a .hw_reset() operation, used by sdhci-pci > > only so far, still, it seems we cannot (easily) hijack it. > > That one's implementing an eMMC 4.5 spec feature, not related to this > kind of reset. > > Thanks! > > - Chris. > -- > Chris Ball <cjb@laptop.org> <http://printf.net/> > One Laptop Per Child > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
Hi Guennadi, On Tue, Dec 04 2012, Guennadi Liakhovetski wrote: >> They both sound pretty attractive. Maybe we start out with (1), which >> would create a patch we could more reasonably send to stable@ to get >> slot-gpio handling the reset during transfers properly in older kernels, >> and then refactor into (2) later? > > Just posted 3 patches for this, have a look if that's what you were > thinking about. Not sure though why this is needed for stable, but I'm > probably just missing some crucial information on the topic. Thanks! I'll take a look. I agree that this isn't definitely needed in -stable, but I'm glad we have the option if someone finds that their host isn't functioning after card removal during a transfer. Russell, are you happy with switching sdhci-dove over to slot-gpio with this patchset? Thanks, - Chris.
On Tue, Dec 04, 2012 at 11:06:39AM -0500, Chris Ball wrote: > Hi Guennadi, > > On Tue, Dec 04 2012, Guennadi Liakhovetski wrote: > >> They both sound pretty attractive. Maybe we start out with (1), which > >> would create a patch we could more reasonably send to stable@ to get > >> slot-gpio handling the reset during transfers properly in older kernels, > >> and then refactor into (2) later? > > > > Just posted 3 patches for this, have a look if that's what you were > > thinking about. Not sure though why this is needed for stable, but I'm > > probably just missing some crucial information on the topic. > > Thanks! I'll take a look. I agree that this isn't definitely needed > in -stable, but I'm glad we have the option if someone finds that their > host isn't functioning after card removal during a transfer. > > Russell, are you happy with switching sdhci-dove over to slot-gpio with > this patchset? I'd rather not until I've moved my cubox kernel tree to v3.7 (when it's out.) Keeping stuff straight between that tree and mainline is far from easy - I'm having to maintain two independent patches for each change, one against each kernel tree, one gets tested (the one in the cubox tree) the other (against mainline) does not.
Hi, On Tue, Dec 04 2012, Russell King - ARM Linux wrote: >> Russell, are you happy with switching sdhci-dove over to slot-gpio with >> this patchset? > > I'd rather not until I've moved my cubox kernel tree to v3.7 (when it's > out.) Keeping stuff straight between that tree and mainline is far from > easy - I'm having to maintain two independent patches for each change, > one against each kernel tree, one gets tested (the one in the cubox tree) > the other (against mainline) does not. Okay -- I'll take the patchset as-is for 3.8, with a plan to move to slot-gpio after you're caught up. Thanks! - Chris.
diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c index 4af64f3..e621448 100644 --- a/drivers/mmc/host/sdhci-dove.c +++ b/drivers/mmc/host/sdhci-dove.c @@ -19,20 +19,30 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -#include <linux/err.h> -#include <linux/io.h> #include <linux/clk.h> #include <linux/err.h> -#include <linux/module.h> +#include <linux/gpio.h> +#include <linux/io.h> #include <linux/mmc/host.h> +#include <linux/module.h> #include <linux/of.h> +#include <linux/of_gpio.h> #include "sdhci-pltfm.h" struct sdhci_dove_priv { struct clk *clk; + int gpio_cd; }; +static irqreturn_t sdhci_dove_carddetect_irq(int irq, void *data) +{ + struct sdhci_host *host = data; + + tasklet_schedule(&host->card_tasklet); + return IRQ_HANDLED; +} + static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) { u16 ret; @@ -50,16 +60,25 @@ static u16 sdhci_dove_readw(struct sdhci_host *host, int reg) static u32 sdhci_dove_readl(struct sdhci_host *host, int reg) { + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_dove_priv *priv = pltfm_host->priv; u32 ret; + ret = readl(host->ioaddr + reg); + switch (reg) { case SDHCI_CAPABILITIES: - ret = readl(host->ioaddr + reg); /* Mask the support for 3.0V */ ret &= ~SDHCI_CAN_VDD_300; break; - default: - ret = readl(host->ioaddr + reg); + case SDHCI_PRESENT_STATE: + if (gpio_is_valid(priv->gpio_cd)) { + if (gpio_get_value(priv->gpio_cd) == 0) + ret |= SDHCI_CARD_PRESENT; + else + ret &= ~SDHCI_CARD_PRESENT; + } + break; } return ret; } @@ -94,6 +113,23 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev) priv->clk = devm_clk_get(&pdev->dev, NULL); + if (pdev->dev.of_node) { + priv->gpio_cd = of_get_named_gpio(pdev->dev.of_node, + "cd-gpios", 0); + } else { + priv->gpio_cd = -EINVAL; + } + + if (gpio_is_valid(priv->gpio_cd)) { + ret = gpio_request(priv->gpio_cd, "sdhci-cd"); + if (ret) { + dev_err(&pdev->dev, "card detect gpio request failed: %d\n", + ret); + return ret; + } + gpio_direction_input(priv->gpio_cd); + } + host = sdhci_pltfm_init(pdev, &sdhci_dove_pdata); if (IS_ERR(host)) { ret = PTR_ERR(host); @@ -112,13 +148,33 @@ static int __devinit sdhci_dove_probe(struct platform_device *pdev) if (ret) goto err_sdhci_add; + /* + * We must request the IRQ after sdhci_add_host(), as the tasklet only + * gets setup in sdhci_add_host() and we oops. + */ + if (gpio_is_valid(priv->gpio_cd)) { + ret = request_irq(gpio_to_irq(priv->gpio_cd), + sdhci_dove_carddetect_irq, + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, + mmc_hostname(host->mmc), host); + if (ret) { + dev_err(&pdev->dev, "card detect irq request failed: %d\n", + ret); + goto err_request_irq; + } + } + return 0; +err_request_irq: + sdhci_remove_host(host, 0); err_sdhci_add: if (!IS_ERR(priv->clk)) clk_disable_unprepare(priv->clk); sdhci_pltfm_free(pdev); err_sdhci_pltfm_init: + if (gpio_is_valid(priv->gpio_cd)) + gpio_free(priv->gpio_cd); return ret; } @@ -130,6 +186,11 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev) sdhci_pltfm_unregister(pdev); + if (gpio_is_valid(priv->gpio_cd)) { + free_irq(gpio_to_irq(priv->gpio_cd), host); + gpio_free(priv->gpio_cd); + } + if (!IS_ERR(priv->clk)) clk_disable_unprepare(priv->clk);
This commit taken from Rabeeh's Cubox kernel and re-worked for DT; Sebastian Hasselbrath is believed to be the original author. Some Cuboxes require a GPIO for card detection; this implements the optional GPIO support for card detection. This GPIO is logic 0 for card inserted. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/mmc/host/sdhci-dove.c | 73 +++++++++++++++++++++++++++++++++++++--- 1 files changed, 67 insertions(+), 6 deletions(-)