SDHCI-S3C updates
Subsystems (4)
PATCH 1/1 SDHCI-S3C updates
Hello,
This patch series includes various updates to sdhci-s3c driver. The patches has been rebased onto latest -mm kernel tree fromgit://zen-kernel.org/kernel/mmotm.git (2010-07-27-14-56 snapshot).
Some patches has been already accepted by Andrew Morton, but they became incompatible after threaded_irq changes in sdhci driver, so they have been dropped. I've adapted them to new driver base, see the change log in each patch.
All required platform changes has been already merged by Kukjin Kim and are available in linux-next kernel tree - see commit:848a3aeee01c8c805f8dad4ae2963473975410d1 fromgit://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
for-next branch.
A complete list of patches:
[PATCHv2 1/4] sdhci-s3c: add support for the non standard minimal clock value [PATCH 2/4] sdhci-s3c: enable SDHCI_QUIRK_NO_HISPD_BIT quirk [PATCHv5 3/4] sdhci-s3c: add support for new card detection methods (driver part) [PATCH 4/4] sdhci-s3c: add regulator support
Best regards
Signature · 2 lines
Marek Szyprowski Samsung Poland R&D Center
PATCH v2 1/4 sdhci-s3c: add support for the non standard minimal clock value
S3C SDHCI host controller can change the source for generating mmc clock. By default host bus clock is used, what causes some problems on machines with 133MHz bus, because the SDHCI divider cannot be as high get proper clock value for identification mode. This is not a problem for the controller, because it can generate lower frequencies from other clock sources. This patch adds a new quirk to SDHCI driver to calculate the minimal supported clock frequency.
This fixes the flood of the following warnings on Samsung S5PV210 SoCs:
mmc0: Minimum clock frequency too high for identification mode
---
Changes since V1:
- rebased onto latest -mm kernel tree
--- drivers/mmc/host/sdhci-of-esdhc.c | 1 + drivers/mmc/host/sdhci-s3c.c | 29 +++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c | 2 +- drivers/mmc/host/sdhci.h | 2 ++ 4 files changed, 33 insertions(+), 1 deletions(-)
drivers/mmc/host/sdhci-of-esdhc.c +1 -0
@@ -124,6 +124,7 @@ struct sdhci_of_data sdhci_esdhc = {SDHCI_QUIRK_BROKEN_CARD_DETECTION |SDHCI_QUIRK_NO_BUSY_IRQ |SDHCI_QUIRK_NONSTANDARD_CLOCK | +SDHCI_QUIRK_NONSTANDARD_MINCLOCK |SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |SDHCI_QUIRK_PIO_NEEDS_DELAY | SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |
drivers/mmc/host/sdhci-s3c.c +29 -0
@@ -203,9 +203,36 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock) } } +/** +* sdhci_s3c_get_min_clock - callback to get minimal supported clock value +* @host: The SDHCI host being queried +* +* To init mmc host properly a minimal clock value is needed. For high system +* bus clock's values the standard formula gives values out of allowed range. +* The clock still can be set to lower values, if clock source other then +* system bus is selected. +*/ +static unsigned int sdhci_s3c_get_min_clock (struct sdhci_host *host) +{ +struct sdhci_s3c *ourhost =to_s3c (host); +unsigned int delta, min =UINT_MAX ; +int src; + +for (src =0 ; src <MAX_BUS_CLK ; src++) { + delta =sdhci_s3c_consider_clock (ourhost, src,0 ); +if (delta ==UINT_MAX ) +continue ; +/* delta is a negative value in this case */ +if (-delta < min) + min = -delta; + } +return min; +} +static struct sdhci_ops sdhci_s3c_ops = { .get_max_clock = sdhci_s3c_get_max_clk, .set_clock = sdhci_s3c_set_clock, + .get_min_clock = sdhci_s3c_get_min_clock, };static int __devinit sdhci_s3c_probe (struct platform_device *pdev)
@@ -309,6 +336,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) host->quirks =0 ; host->irq = irq; + host->quirks |=SDHCI_QUIRK_NONSTANDARD_MINCLOCK ; +/* Setup quirks for the controller */ host->quirks |=SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC ;
drivers/mmc/host/sdhci.c +1 -1
@@ -1776,7 +1776,7 @@ int sdhci_add_host(struct sdhci_host *host)* Set host parameters. */ mmc->ops = &sdhci_ops; -if (host->quirks &SDHCI_QUIRK_NONSTANDARD_CLOCK && +if (host->quirks &SDHCI_QUIRK_NONSTANDARD_MINCLOCK && host->ops->get_min_clock) mmc->f_min = host->ops->get_min_clock (host);else
drivers/mmc/host/sdhci.h +2 -0
@@ -247,6 +247,8 @@ struct sdhci_host {#define SDHCI_QUIRK_NO_HISPD_BIT (1<<27)/* Controller is missing device caps. Use caps provided by host */ #define SDHCI_QUIRK_MISSING_CAPS (1<<28) +/* Controller has nonstandard clock management */ +#define SDHCI_QUIRK_NONSTANDARD_MINCLOCK (1<<29)int irq;/* Device IRQ */ void __iomem * ioaddr;/* Mapped address */
Signature · 1 line
1.7.1.569.g6f426
PATCH v2 1/4 sdhci-s3c: add support for the non standard minimal clock value
Quoted 92 lines · Marek Szyprowski
S3C SDHCI host controller can change the source for generating mmc clock. By default host bus clock is used, what causes some problems on machines with 133MHz bus, because the SDHCI divider cannot be as high get proper clock value for identification mode. This is not a problem for the controller, because it can generate lower frequencies from other clock sources. This patch adds a new quirk to SDHCI driver to calculate the minimal supported clock frequency.
This fixes the flood of the following warnings on Samsung S5PV210 SoCs:
mmc0: Minimum clock frequency too high for identification modeSigned-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes since V1:
- rebased onto latest -mm kernel tree--- drivers/mmc/host/sdhci-of-esdhc.c | 1 + drivers/mmc/host/sdhci-s3c.c | 29 +++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c | 2 +- drivers/mmc/host/sdhci.h | 2 ++ 4 files changed, 33 insertions(+), 1 deletions(-)drivers/mmc/host/sdhci-of-esdhc.c +1
-0@@ -124,6 +124,7 @@ struct sdhci_of_data sdhci_esdhc = {SDHCI_QUIRK_BROKEN_CARD_DETECTION |SDHCI_QUIRK_NO_BUSY_IRQ |SDHCI_QUIRK_NONSTANDARD_CLOCK | +SDHCI_QUIRK_NONSTANDARD_MINCLOCK |SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |SDHCI_QUIRK_PIO_NEEDS_DELAY | SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |drivers/mmc/host/sdhci-s3c.c +29
-0@@ -203,9 +203,36 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock) } } +/** +* sdhci_s3c_get_min_clock - callback to get minimal supported clock value +* @host: The SDHCI host being queried +* +* To init mmc host properly a minimal clock value is needed. For high system +* bus clock's values the standard formula gives values out of allowed range. +* The clock still can be set to lower values, if clock source other then +* system bus is selected. +*/ +static unsigned int sdhci_s3c_get_min_clock (struct sdhci_host *host) +{ +struct sdhci_s3c *ourhost =to_s3c (host); +unsigned int delta, min =UINT_MAX ; +int src; + +for (src =0 ; src <MAX_BUS_CLK ; src++) { + delta =sdhci_s3c_consider_clock (ourhost, src,0 ); +if (delta ==UINT_MAX ) +continue ; +/* delta is a negative value in this case */ +if (-delta < min) + min = -delta; + } +return min; +} +static struct sdhci_ops sdhci_s3c_ops = { .get_max_clock = sdhci_s3c_get_max_clk, .set_clock = sdhci_s3c_set_clock, + .get_min_clock = sdhci_s3c_get_min_clock, };static int __devinit sdhci_s3c_probe (struct platform_device *pdev)@@ -309,6 +336,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) host->quirks =0 ; host->irq = irq; + host->quirks |=SDHCI_QUIRK_NONSTANDARD_MINCLOCK ; +/* Setup quirks for the controller */ host->quirks |=SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC ;drivers/mmc/host/sdhci.c +0
-0
drivers/mmc/host/sdhci.c int sdhci_add_host(struct sdhci_host *host) · line 1781
Quoted 7 lines
drivers/mmc/host/sdhci.c +1
-1@@ -1776,7 +1776,7 @@ int sdhci_add_host(struct sdhci_host *host)* Set host parameters. */ mmc->ops = &sdhci_ops; -if (host->quirks &SDHCI_QUIRK_NONSTANDARD_CLOCK && +if (host->quirks &SDHCI_QUIRK_NONSTANDARD_MINCLOCK && host->ops->get_min_clock) mmc->f_min = host->ops->get_min_clock (host);
We're begining to run out of quirk space, given these fields will be initialised to NULL if not used, then why not just check for the op being present?
PATCH v2 1/4 sdhci-s3c: add support for the non standard minimal clock value
Hello,
Quoted 95 lines · Ben Dooks
drivers/mmc/host/sdhci-
Quoted 26 lines · Marek Szyprowski
S3C SDHCI host controller can change the source for generating mmc clock. By default host bus clock is used, what causes some problems on machines with 133MHz bus, because the SDHCI divider cannot be as high get proper clock value for identification mode. This is not a problem for the controller, because it can generate lower frequencies from other clock sources. This patch adds a new quirk to SDHCI driver to calculate the minimal supported clock frequency.
This fixes the flood of the following warnings on Samsung S5PV210 SoCs:
mmc0: Minimum clock frequency too high for identification modeSigned-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes since V1:
- rebased onto latest -mm kernel tree--- drivers/mmc/host/sdhci-of-esdhc.c | 1 + drivers/mmc/host/sdhci-s3c.c | 29 +++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c | 2 +- drivers/mmc/host/sdhci.h | 2 ++ 4 files changed, 33 insertions(+), 1 deletions(-)drivers/mmc/host/sdhci- renamed from drivers/mmc/host/sdhci-of-esdhc.c +0
-0
of-esdhc.cdrivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host
Quoted 16 lines
index e9f99fe..763b364 100644
drivers/mmc/host/sdhci-of-esdhc.c +1
-0@@ -124,6 +124,7 @@ struct sdhci_of_data sdhci_esdhc = {SDHCI_QUIRK_BROKEN_CARD_DETECTION |SDHCI_QUIRK_NO_BUSY_IRQ |SDHCI_QUIRK_NONSTANDARD_CLOCK | +SDHCI_QUIRK_NONSTANDARD_MINCLOCK |SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |SDHCI_QUIRK_PIO_NEEDS_DELAY | SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |drivers/mmc/host/sdhci-s3c.c +0
-0@@ -203,9 +203,36 @@ static void sdhci_s3c_set_clock(struct sdhci_host*host, unsigned int clock)
drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c +2
-0} } +/** +* sdhci_s3c_get_min_clock - callback to get minimal supported clock value
drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +3
-0+* @host: The SDHCI host being queried +* +* To init mmc host properly a minimal clock value is needed. For high system
drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host · line 211
Quoted 1 line
drivers/mmc/host/sdhci-s3c.c +1
-0+* bus clock's values the standard formula gives values out of allowed range.
drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host · line 212
Quoted 1 line
drivers/mmc/host/sdhci-s3c.c +1
-0+* The clock still can be set to lower values, if clock source other then
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 27 lines
drivers/mmc/host/sdhci-s3c.c +20
-0+* system bus is selected. +*/ +static unsigned int sdhci_s3c_get_min_clock (struct sdhci_host *host) +{ +struct sdhci_s3c *ourhost =to_s3c (host); +unsigned int delta, min =UINT_MAX ; +int src; + +for (src =0 ; src <MAX_BUS_CLK ; src++) { + delta =sdhci_s3c_consider_clock (ourhost, src,0 ); +if (delta ==UINT_MAX ) +continue ; +/* delta is a negative value in this case */ +if (-delta < min) + min = -delta; + } +return min; +} +static struct sdhci_ops sdhci_s3c_ops = { .get_max_clock = sdhci_s3c_get_max_clk, .set_clock = sdhci_s3c_set_clock, + .get_min_clock = sdhci_s3c_get_min_clock, };static int __devinit sdhci_s3c_probe (struct platform_device *pdev)@@ -309,6 +336,8 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c +2
-0host->quirks =0 ; host->irq = irq; + host->quirks |=SDHCI_QUIRK_NONSTANDARD_MINCLOCK ; +/* Setup quirks for the controller */ host->quirks |=SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC ;
Quoted 16 lines
drivers/mmc/host/sdhci.c int sdhci_add_host(struct sdhci_host *host) · line 1781
Quoted 12 lines
drivers/mmc/host/sdhci.c +1
-1@@ -1776,7 +1776,7 @@ int sdhci_add_host(struct sdhci_host *host)* Set host parameters. */ mmc->ops = &sdhci_ops; -if (host->quirks &SDHCI_QUIRK_NONSTANDARD_CLOCK && +if (host->quirks &SDHCI_QUIRK_NONSTANDARD_MINCLOCK && host->ops->get_min_clock) mmc->f_min = host->ops->get_min_clock (host);We're begining to run out of quirk space, given these fields will be initialised to NULL if not used, then why not just check for the op being present?
Ok, I can remove the quirk and just check if the op is present. However having a separate quirk makes the code easier to understand (quirk shows that the particular implementation differs from the standard).
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
PATCH 2/4 sdhci-s3c: enable SDHCI_QUIRK_NO_HISPD_BIT quirk
In reply to Marek SzyprowskiThis patch enables SDHCI_QUIRK_NO_HISPD_BIT on Samsung SDHCI driver. This solves detection problems with some external SD cards. This change has been tested on S5PC100 and S5PC110. It has no inpact on driver speed.
--- drivers/mmc/host/sdhci-s3c.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +1 -0
@@ -337,6 +337,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) host->irq = irq; host->quirks |=SDHCI_QUIRK_NONSTANDARD_MINCLOCK ; + host->quirks |=SDHCI_QUIRK_NO_HISPD_BIT ;/* Setup quirks for the controller */ host->quirks |=SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC ;
Signature · 1 line
1.7.1.569.g6f426
PATCH v5 3/4 sdhci-s3c: add support for new card detection methods (driver part)
In reply to Marek SzyprowskiOn some Samsung SoCs not all SDHCI controllers have card detect (CD) line. For some embedded designs it is not even needed, because ususally the device (like SDIO flash memory or wifi controller) is permanently wired to the controller. There are also systems which have a card detect line connected to some of the external interrupt lines or the presence of the card depends on some other actions (like enabling a power regulator).
This patch adds support for all these cases. The following card detection methods are possible:
1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller (once detected host won't poll it any more)
By default, all existing code would use method #1, what is compatible with the previous version of the driver.
In case of external event, two callbacks must be provided in platdata:ext_cd_init and ext_cd_cleanup. Both of them get a callback to a function that notifies the s3c-sdhci host contoller as their argument. That callback function should be called from the even dispatcher to let host notice the card insertion/removal.
In case of external gpio interrupt, a gpio pin number must be provided in platdata (ext_cd_gpio parameter), as well as the information about the polarity of that gpio pin (ext_cd_gpio_invert). By default (ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed', but this can be changed to 'card has been removed' when ext_cd_gpio_invert == 1.
This patch adds all required changes to sdhci-s3c driver.
---
Changes since V4:
- adapted to threaded irq approach in sdhci driver
- rebased onto latest -mm kernel tree
- fixed problem when gpio-base card detection was not called on driver init
Changes since V3:
- renamed patch to avoid confusion with the patch for the s3c-sdhci platform changes - added "(driver part)" in subject
Changes since V2:
According to Andrew Morton's suggestion local_irq_save() call in the sdhci_s3c_notify_change function has been replaced by spin_lock_irqsave (host driver already has a spinlock that is used for protecting internal state of the driver).
Changes since V1:
- added support for gpio external interrupt card based detect method directly to sdhci-s3c driver
--- drivers/mmc/host/sdhci-s3c.c | 78 ++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c | 47 ++++++++++++++----------- drivers/mmc/host/sdhci.h | 1 + 3 files changed, 106 insertions(+), 20 deletions(-)
drivers/mmc/host/sdhci-s3c.c +78 -0
@@ -18,6 +18,7 @@#include <linux/slab.h> #include <linux/clk.h> #include <linux/io.h> +#include <linux/gpio.h> #include <linux/mmc/host.h>
@@ -44,6 +45,8 @@ struct sdhci_s3c {struct resource *ioarea;struct s3c_sdhci_platdata *pdata;unsigned int cur_clk; +int ext_cd_irq; +int ext_cd_gpio;struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ];
@@ -235,6 +238,36 @@ static struct sdhci_ops sdhci_s3c_ops = { .get_min_clock = sdhci_s3c_get_min_clock, }; +static void sdhci_s3c_notify_change (struct platform_device *dev,int state) +{ +struct sdhci_host *host =platform_get_drvdata (dev); +if (host) { +mutex_lock (&host->lock); +if (state) { +dev_dbg (&dev->dev,"card inserted.\n" ); + host->flags &= ~SDHCI_DEVICE_DEAD ; + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }else { +dev_dbg (&dev->dev,"card removed.\n" ); + host->flags |=SDHCI_DEVICE_DEAD ; + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + } +mutex_unlock (&host->lock); + } +} + +static irqreturn_t sdhci_s3c_gpio_card_detect_thread (int irq,void *dev_id) +{ +struct sdhci_s3c *sc = dev_id; +int status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); +return IRQ_HANDLED ; +} +static int __devinit sdhci_s3c_probe (struct platform_device *pdev) {struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
@@ -272,6 +305,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) sc->host = host; sc->pdev = pdev; sc->pdata = pdata; + sc->ext_cd_gpio =-1 ;platform_set_drvdata (pdev, host);
@@ -355,6 +389,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)* SDHCI block, or a missing configuration that needs to be set. */ host->quirks |=SDHCI_QUIRK_NO_BUSY_IRQ ; +if (pdata->cd_type ==S3C_SDHCI_CD_NONE || + pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; + +if (pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->mmc->caps =MMC_CAP_NONREMOVABLE ; + host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |SDHCI_QUIRK_32BIT_DMA_SIZE );
@@ -367,6 +408,33 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) goto err_add_host; } +/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately, +so it can be called only after sdhci_add_host() */ +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init) + pdata->ext_cd_init (&sdhci_s3c_notify_change); + +if (pdata->cd_type ==S3C_SDHCI_CD_GPIO && +gpio_is_valid (pdata->ext_cd_gpio)) { +int status; + +gpio_request (pdata->ext_cd_gpio,"SDHCI EXT CD" ); + sc->ext_cd_gpio = pdata->ext_cd_gpio; + + sc->ext_cd_irq =gpio_to_irq (pdata->ext_cd_gpio); +if (sc->ext_cd_irq && +request_threaded_irq (sc->ext_cd_irq,NULL , sdhci_s3c_gpio_card_detect_thread, +IRQF_TRIGGER_RISING |IRQF_TRIGGER_FALLING , +dev_name (&pdev->dev), sc)) { +dev_err (&pdev->dev,"cannot request irq for card detect\n" ); + sc->ext_cd_irq =0 ; + } + + status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); + } +return 0 ; err_add_host:
@@ -391,10 +459,20 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) static int __devexit sdhci_s3c_remove(struct platform_device *pdev) { +struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;struct sdhci_host *host =platform_get_drvdata (pdev);struct sdhci_s3c *sc =sdhci_priv (host);int ptr; +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup) + pdata->ext_cd_cleanup (&sdhci_s3c_notify_change); + +if (sc->ext_cd_irq) +free_irq (sc->ext_cd_irq, sc); + +if (sc->ext_cd_gpio !=-1 ) +gpio_free (sc->ext_cd_gpio); +sdhci_remove_host (host,1 );for (ptr =0 ; ptr <3 ; ptr++) {
drivers/mmc/host/sdhci.c +27 -20
59 lines not on this page · raw message
drivers/mmc/host/sdhci.h +1 -0
@@ -417,6 +417,7 @@ static inline void *sdhci_priv(struct sdhci_host *host)return (void *)host->private; } +extern void sdhci_card_detect (struct sdhci_host *host);extern int sdhci_add_host (struct sdhci_host *host);extern void sdhci_remove_host (struct sdhci_host *host,int dead);
Signature · 1 line
1.7.1.569.g6f426
PATCH v5 3/4 sdhci-s3c: add support for new card detection methods (driver part)
Op 28-07-10 16:19, Marek Szyprowski schreef:
Quoted 97 lines
On some Samsung SoCs not all SDHCI controllers have card detect (CD) line. For some embedded designs it is not even needed, because ususally the device (like SDIO flash memory or wifi controller) is permanently wired to the controller. There are also systems which have a card detect line connected to some of the external interrupt lines or the presence of the card depends on some other actions (like enabling a power regulator).
This patch adds support for all these cases. The following card detection methods are possible:
1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller (once detected host won't poll it any more)By default, all existing code would use method #1, what is compatible with the previous version of the driver.
In case of external event, two callbacks must be provided in platdata:
ext_cd_initandext_cd_cleanup. Both of them get a callback to a function that notifies the s3c-sdhci host contoller as their argument. That callback function should be called from the even dispatcher to let host notice the card insertion/removal.In case of external gpio interrupt, a gpio pin number must be provided in platdata (
ext_cd_gpioparameter), as well as the information about the polarity of that gpio pin (ext_cd_gpio_invert). By default (ext_cd_gpio_invert== 0) gpio value 0 means 'card has been removed', but this can be changed to 'card has been removed' whenext_cd_gpio_invert== 1.This patch adds all required changes to sdhci-s3c driver.
Signed-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes since V4:
- adapted to threaded irq approach in sdhci driver
- rebased onto latest -mm kernel tree
- fixed problem when gpio-base card detection was not called on driver initChanges since V3:
- renamed patch to avoid confusion with the patch for the s3c-sdhci platform changes - added "(driver part)" in subjectChanges since V2:
According to Andrew Morton's suggestionlocal_irq_save()call in thesdhci_s3c_notify_changefunction has been replaced byspin_lock_irqsave(host driver already has a spinlock that is used for protecting internal state of the driver).Changes since V1:
- added support for gpio external interrupt card based detect method directly to sdhci-s3c driver--- drivers/mmc/host/sdhci-s3c.c | 78 ++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c | 47 ++++++++++++++----------- drivers/mmc/host/sdhci.h | 1 + 3 files changed, 106 insertions(+), 20 deletions(-)drivers/mmc/host/sdhci-s3c.c +9
-0@@ -18,6 +18,7 @@#include <linux/slab.h> #include <linux/clk.h> #include <linux/io.h> +#include <linux/gpio.h> #include <linux/mmc/host.h> @@ -44,6 +45,8 @@ struct sdhci_s3c {struct resource *ioarea;struct s3c_sdhci_platdata *pdata;unsigned int cur_clk; +int ext_cd_irq; +int ext_cd_gpio;struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ];@@ -235,6 +238,36 @@ static struct sdhci_ops sdhci_s3c_ops = { .get_min_clock = sdhci_s3c_get_min_clock, }; +static void sdhci_s3c_notify_change (struct platform_device *dev,int state) +{ +struct sdhci_host *host =platform_get_drvdata (dev); +if (host) { +mutex_lock (&host->lock); +if (state) {
drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = { · line 256
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10
-0+dev_dbg (&dev->dev,"card inserted.\n" ); + host->flags &= ~SDHCI_DEVICE_DEAD ; + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }else { +dev_dbg (&dev->dev,"card removed.\n" ); + host->flags |=SDHCI_DEVICE_DEAD ; + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }
You could move the sdhci_card_detect outside the if.
Quoted 43 lines
drivers/mmc/host/sdhci-s3c.c +22
-0+mutex_unlock (&host->lock); + } +} + +static irqreturn_t sdhci_s3c_gpio_card_detect_thread (int irq,void *dev_id) +{ +struct sdhci_s3c *sc = dev_id; +int status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); +return IRQ_HANDLED ; +} +static int __devinit sdhci_s3c_probe (struct platform_device *pdev) {struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;@@ -272,6 +305,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) sc->host = host; sc->pdev = pdev; sc->pdata = pdata; + sc->ext_cd_gpio =-1 ;platform_set_drvdata (pdev, host);@@ -355,6 +389,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)* SDHCI block, or a missing configuration that needs to be set. */ host->quirks |=SDHCI_QUIRK_NO_BUSY_IRQ ; +if (pdata->cd_type ==S3C_SDHCI_CD_NONE || + pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; + +if (pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->mmc->caps =MMC_CAP_NONREMOVABLE ; + host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |SDHCI_QUIRK_32BIT_DMA_SIZE );@@ -367,6 +408,33 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) goto err_add_host; }
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 420
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10
-0+/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately, +so it can be called only after sdhci_add_host() */ +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init) + pdata->ext_cd_init (&sdhci_s3c_notify_change); + +if (pdata->cd_type ==S3C_SDHCI_CD_GPIO && +gpio_is_valid (pdata->ext_cd_gpio)) { +int status; + +gpio_request (pdata->ext_cd_gpio,"SDHCI EXT CD" );
Shouldn't you check the return value of gpio_request()?
Quoted 26 lines
drivers/mmc/host/sdhci-s3c.c +18
-0+ sc->ext_cd_gpio = pdata->ext_cd_gpio; + + sc->ext_cd_irq =gpio_to_irq (pdata->ext_cd_gpio); +if (sc->ext_cd_irq && +request_threaded_irq (sc->ext_cd_irq,NULL , sdhci_s3c_gpio_card_detect_thread, +IRQF_TRIGGER_RISING |IRQF_TRIGGER_FALLING , +dev_name (&pdev->dev), sc)) { +dev_err (&pdev->dev,"cannot request irq for card detect\n" ); + sc->ext_cd_irq =0 ; + } + + status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); + } +return 0 ; err_add_host:@@ -391,10 +459,20 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) static int __devexit sdhci_s3c_remove(struct platform_device *pdev) { +struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;struct sdhci_host *host =platform_get_drvdata (pdev);
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 473
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +7
-0struct sdhci_s3c *sc =sdhci_priv (host);int ptr; +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup) + pdata->ext_cd_cleanup (&sdhci_s3c_notify_change); + +if (sc->ext_cd_irq) +free_irq (sc->ext_cd_irq, sc); + +if (sc->ext_cd_gpio !=-1 )
Perhaps if (gpio_is_valid(...)) ?
Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c +2
-0+gpio_free (sc->ext_cd_gpio); +sdhci_remove_host (host,1 );for (ptr =0 ; ptr <3 ; ptr++) {
Signature · 1 line
Maurus Cuelenaere
PATCH v5 3/4 sdhci-s3c: add support for new card detection methods (driver part)
Hello,
Quoted 104 lines · Maurus Cuelenaere
Op 28-07-10 16:19, Marek Szyprowski schreef:
Quoted 56 lines
On some Samsung SoCs not all SDHCI controllers have card detect (CD) line. For some embedded designs it is not even needed, because ususally the device (like SDIO flash memory or wifi controller) is permanently wired to the controller. There are also systems which have a card detect line connected to some of the external interrupt lines or the presence of the card depends on some other actions (like enabling a power regulator).
This patch adds support for all these cases. The following card detection methods are possible:
1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller (once detected host won't poll it any more)By default, all existing code would use method #1, what is compatible with the previous version of the driver.
In case of external event, two callbacks must be provided in platdata:
ext_cd_initandext_cd_cleanup. Both of them get a callback to a function that notifies the s3c-sdhci host contoller as their argument. That callback function should be called from the even dispatcher to let host notice the card insertion/removal.In case of external gpio interrupt, a gpio pin number must be provided in platdata (
ext_cd_gpioparameter), as well as the information about the polarity of that gpio pin (ext_cd_gpio_invert). By default (ext_cd_gpio_invert== 0) gpio value 0 means 'card has been removed', but this can be changed to 'card has been removed' whenext_cd_gpio_invert== 1.This patch adds all required changes to sdhci-s3c driver.
Signed-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes since V4:
- adapted to threaded irq approach in sdhci driver
- rebased onto latest -mm kernel tree
- fixed problem when gpio-base card detection was not called on driver initChanges since V3:
- renamed patch to avoid confusion with the patch for the s3c-sdhci platform changes - added "(driver part)" in subjectChanges since V2:
According to Andrew Morton's suggestionlocal_irq_save()call in thesdhci_s3c_notify_changefunction has been replaced byspin_lock_irqsave(host driver already has a spinlock that is used for protecting internal state of the driver).Quoted 6 lines
Changes since V1:
- added support for gpio external interrupt card based detect method directly to sdhci-s3c driver--- drivers/mmc/host/sdhci-s3c.c | 78++++++++++++++++++++++++++++++++++++++++++
Quoted 26 lines
drivers/mmc/host/sdhci.c | 47 ++++++++++++++----------- drivers/mmc/host/sdhci.h | 1 + 3 files changed, 106 insertions(+), 20 deletions(-)drivers/mmc/host/sdhci-s3c.c +3
-0@@ -18,6 +18,7 @@#include <linux/slab.h> #include <linux/clk.h> #include <linux/io.h> +#include <linux/gpio.h> #include <linux/mmc/host.h> @@ -44,6 +45,8 @@ struct sdhci_s3c {struct resource *ioarea;struct s3c_sdhci_platdata *pdata;unsigned int cur_clk; +int ext_cd_irq; +int ext_cd_gpio;struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ];drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = { · line 241
Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +1
-0@@ -235,6 +238,36 @@ static struct sdhci_ops sdhci_s3c_ops = { .get_min_clock = sdhci_s3c_get_min_clock, }; +static void sdhci_s3c_notify_change(struct platform_device *dev, intstate)
Quoted 9 lines
drivers/mmc/host/sdhci-s3c.c +9
-0+{ +struct sdhci_host *host =platform_get_drvdata (dev); +if (host) { +mutex_lock (&host->lock); +if (state) { +dev_dbg (&dev->dev,"card inserted.\n" ); + host->flags &= ~SDHCI_DEVICE_DEAD ; + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host);
Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = { · line 256
Quoted 6 lines
drivers/mmc/host/sdhci-s3c.c +6
-0+ }else { +dev_dbg (&dev->dev,"card removed.\n" ); + host->flags |=SDHCI_DEVICE_DEAD ; + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }You could move the
sdhci_card_detectoutside the if.
Rigth, I will fix this.
Quoted 47 lines
drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = {
Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c +5
-0+mutex_unlock (&host->lock); + } +} + +static irqreturn_t sdhci_s3c_gpio_card_detect_thread(int irq, void
*dev_id)drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 13 lines
drivers/mmc/host/sdhci-s3c.c +9
-0+{ +struct sdhci_s3c *sc = dev_id; +int status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); +return IRQ_HANDLED ; +} +static int __devinit sdhci_s3c_probe (struct platform_device *pdev) {struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;@@ -272,6 +305,7 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c +1
-0sc->host = host; sc->pdev = pdev; sc->pdata = pdata; + sc->ext_cd_gpio =-1 ;platform_set_drvdata (pdev, host);@@ -355,6 +389,13 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 14 lines
drivers/mmc/host/sdhci-s3c.c +7
-0* SDHCI block, or a missing configuration that needs to be set. */ host->quirks |=SDHCI_QUIRK_NO_BUSY_IRQ ; +if (pdata->cd_type ==S3C_SDHCI_CD_NONE || + pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; + +if (pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->mmc->caps =MMC_CAP_NONREMOVABLE ; + host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |SDHCI_QUIRK_32BIT_DMA_SIZE );@@ -367,6 +408,33 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +0
-0goto err_add_host; }
Quoted 12 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 420
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10
-0+/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately, +so it can be called only after sdhci_add_host() */ +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init) + pdata->ext_cd_init (&sdhci_s3c_notify_change); + +if (pdata->cd_type ==S3C_SDHCI_CD_GPIO && +gpio_is_valid (pdata->ext_cd_gpio)) { +int status; + +gpio_request (pdata->ext_cd_gpio,"SDHCI EXT CD" );Shouldn't you check the return value of
gpio_request()?
Yes, I it should. I will fix this.
Quoted 33 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
Quoted 6 lines
drivers/mmc/host/sdhci-s3c.c +5
-0+ sc->ext_cd_gpio = pdata->ext_cd_gpio; + + sc->ext_cd_irq =gpio_to_irq (pdata->ext_cd_gpio); +if (sc->ext_cd_irq && + request_threaded_irq(sc->ext_cd_irq, NULL,
sdhci_s3c_gpio_card_detect_thread,drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +3
-0+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, + dev_name(&pdev->dev), sc)) { + dev_err(&pdev->dev, "cannot request irq for carddetect\n");
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 13 lines
drivers/mmc/host/sdhci-s3c.c +9
-0+ sc->ext_cd_irq =0 ; + } + + status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); + } +return 0 ; err_add_host:@@ -391,10 +459,20 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c +1
-0static int __devexit sdhci_s3c_remove(struct platform_device *pdev) { +struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;struct sdhci_host *host =platform_get_drvdata (pdev);struct sdhci_s3c *sc =sdhci_priv (host);int ptr;
Quoted 9 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 473
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c +7
-0+if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup) + pdata->ext_cd_cleanup (&sdhci_s3c_notify_change); + +if (sc->ext_cd_irq) +free_irq (sc->ext_cd_irq, sc); + +if (sc->ext_cd_gpio !=-1 )Perhaps if (
gpio_is_valid(...)) ?
-1 is intentionally assigned in s3c_sdhci_probe() when no gpio pin is used for card detection. I can change this to gpio_is_valid() if this increases the code readability.
Quoted 9 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 478
Quoted 6 lines
drivers/mmc/host/sdhci-s3c.c +2
-0+gpio_free (sc->ext_cd_gpio); +sdhci_remove_host (host,1 );for (ptr =0 ; ptr <3 ; ptr++) {--
Maurus Cuelenaere
Thanks for your comments!
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
PATCH v5 3/4 sdhci-s3c: add support for new card detection methods (driver part)
In reply to Marek SzyprowskiQuoted 107 lines · Marek Szyprowski
On some Samsung SoCs not all SDHCI controllers have card detect (CD) line. For some embedded designs it is not even needed, because ususally the device (like SDIO flash memory or wifi controller) is permanently wired to the controller. There are also systems which have a card detect line connected to some of the external interrupt lines or the presence of the card depends on some other actions (like enabling a power regulator).
This patch adds support for all these cases. The following card detection methods are possible:
1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller (once detected host won't poll it any more)By default, all existing code would use method #1, what is compatible with the previous version of the driver.
In case of external event, two callbacks must be provided in platdata:
ext_cd_initandext_cd_cleanup. Both of them get a callback to a function that notifies the s3c-sdhci host contoller as their argument. That callback function should be called from the even dispatcher to let host notice the card insertion/removal.In case of external gpio interrupt, a gpio pin number must be provided in platdata (
ext_cd_gpioparameter), as well as the information about the polarity of that gpio pin (ext_cd_gpio_invert). By default (ext_cd_gpio_invert== 0) gpio value 0 means 'card has been removed', but this can be changed to 'card has been removed' whenext_cd_gpio_invert== 1.This patch adds all required changes to sdhci-s3c driver.
Signed-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes since V4:
- adapted to threaded irq approach in sdhci driver
- rebased onto latest -mm kernel tree
- fixed problem when gpio-base card detection was not called on driver initChanges since V3:
- renamed patch to avoid confusion with the patch for the s3c-sdhci platform changes - added "(driver part)" in subjectChanges since V2:
According to Andrew Morton's suggestionlocal_irq_save()call in thesdhci_s3c_notify_changefunction has been replaced byspin_lock_irqsave(host driver already has a spinlock that is used for protecting internal state of the driver).Changes since V1:
- added support for gpio external interrupt card based detect method directly to sdhci-s3c driver--- drivers/mmc/host/sdhci-s3c.c | 78 ++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/host/sdhci.c | 47 ++++++++++++++----------- drivers/mmc/host/sdhci.h | 1 + 3 files changed, 106 insertions(+), 20 deletions(-)drivers/mmc/host/sdhci-s3c.c +19
-0@@ -18,6 +18,7 @@#include <linux/slab.h> #include <linux/clk.h> #include <linux/io.h> +#include <linux/gpio.h> #include <linux/mmc/host.h> @@ -44,6 +45,8 @@ struct sdhci_s3c {struct resource *ioarea;struct s3c_sdhci_platdata *pdata;unsigned int cur_clk; +int ext_cd_irq; +int ext_cd_gpio;struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ];@@ -235,6 +238,36 @@ static struct sdhci_ops sdhci_s3c_ops = { .get_min_clock = sdhci_s3c_get_min_clock, }; +static void sdhci_s3c_notify_change (struct platform_device *dev,int state) +{ +struct sdhci_host *host =platform_get_drvdata (dev); +if (host) { +mutex_lock (&host->lock); +if (state) { +dev_dbg (&dev->dev,"card inserted.\n" ); + host->flags &= ~SDHCI_DEVICE_DEAD ; + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }else { +dev_dbg (&dev->dev,"card removed.\n" ); + host->flags |=SDHCI_DEVICE_DEAD ; + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }
drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = { · line 266
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10
-0+mutex_unlock (&host->lock); + } +} + +static irqreturn_t sdhci_s3c_gpio_card_detect_thread (int irq,void *dev_id) +{ +struct sdhci_s3c *sc = dev_id; +int status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status;
maybe move the invert into sdhci_s3c_notify_change()
Quoted 30 lines
drivers/mmc/host/sdhci-s3c.c +12
-0+sdhci_s3c_notify_change (sc->pdev, status); +return IRQ_HANDLED ; +} +static int __devinit sdhci_s3c_probe (struct platform_device *pdev) {struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;@@ -272,6 +305,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) sc->host = host; sc->pdev = pdev; sc->pdata = pdata; + sc->ext_cd_gpio =-1 ;platform_set_drvdata (pdev, host);@@ -355,6 +389,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)* SDHCI block, or a missing configuration that needs to be set. */ host->quirks |=SDHCI_QUIRK_NO_BUSY_IRQ ; +if (pdata->cd_type ==S3C_SDHCI_CD_NONE || + pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; + +if (pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->mmc->caps =MMC_CAP_NONREMOVABLE ; + host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |SDHCI_QUIRK_32BIT_DMA_SIZE );
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 414
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c +4
-0@@ -367,6 +408,33 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) goto err_add_host; } +/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately, +so it can be called only after sdhci_add_host() */ +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init) + pdata->ext_cd_init (&sdhci_s3c_notify_change);
do we really need a callback for this?
Quoted 96 lines
drivers/mmc/host/sdhci-s3c.c +33
-0+ +if (pdata->cd_type ==S3C_SDHCI_CD_GPIO && +gpio_is_valid (pdata->ext_cd_gpio)) { +int status; + +gpio_request (pdata->ext_cd_gpio,"SDHCI EXT CD" ); + sc->ext_cd_gpio = pdata->ext_cd_gpio; + + sc->ext_cd_irq =gpio_to_irq (pdata->ext_cd_gpio); +if (sc->ext_cd_irq && +request_threaded_irq (sc->ext_cd_irq,NULL , sdhci_s3c_gpio_card_detect_thread, +IRQF_TRIGGER_RISING |IRQF_TRIGGER_FALLING , +dev_name (&pdev->dev), sc)) { +dev_err (&pdev->dev,"cannot request irq for card detect\n" ); + sc->ext_cd_irq =0 ; + } + + status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); + } +return 0 ; err_add_host:@@ -391,10 +459,20 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) static int __devexit sdhci_s3c_remove(struct platform_device *pdev) { +struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;struct sdhci_host *host =platform_get_drvdata (pdev);struct sdhci_s3c *sc =sdhci_priv (host);int ptr; +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup) + pdata->ext_cd_cleanup (&sdhci_s3c_notify_change); + +if (sc->ext_cd_irq) +free_irq (sc->ext_cd_irq, sc); + +if (sc->ext_cd_gpio !=-1 ) +gpio_free (sc->ext_cd_gpio); +sdhci_remove_host (host,1 );for (ptr =0 ; ptr <3 ; ptr++) {drivers/mmc/host/sdhci.c +14
-20@@ -1244,26 +1244,6 @@ static const struct mmc_host_ops sdhci_ops = {* * \*****************************************************************************/ -static void sdhci_card_detect (struct sdhci_host *host) -{ -if (!(sdhci_readl (host,SDHCI_PRESENT_STATE ) &SDHCI_CARD_PRESENT )) { -if (host->mrq) { -printk (KERN_ERR "%s: Card removed during transfer!\n" , -mmc_hostname (host->mmc)); -printk (KERN_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 ; -schedule_work (&host->finish_work); - } - } - -mmc_detect_change (host->mmc,msecs_to_jiffies (200 )); -} -static void sdhci_finish_work (struct work_struct *wk) {struct sdhci_host *host;@@ -1627,6 +1607,33 @@ EXPORT_SYMBOL_GPL(sdhci_resume_host);/*****************************************************************************\ * * +* Implementation dependent callbacks * +* * +\*****************************************************************************/ + +void sdhci_card_detect (struct sdhci_host *host) +{ +if (!(sdhci_readl (host,SDHCI_PRESENT_STATE ) &SDHCI_CARD_PRESENT )) { +if (host->mrq) { +printk (KERN_ERR "%s: Card removed during transfer!\n" , +mmc_hostname (host->mmc)); +printk (KERN_ERR "%s: Resetting controller.\n" , +mmc_hostname (host->mmc)); + +sdhci_reset (host,SDHCI_RESET_CMD );
drivers/mmc/host/sdhci.c EXPORT_SYMBOL_GPL(sdhci_resume_host); · line 1633
Quoted 10 lines
drivers/mmc/host/sdhci.c +10
-0+sdhci_reset (host,SDHCI_RESET_DATA ); + + host->mrq->cmd->error = -ENOMEDIUM ; +schedule_work (&host->finish_work); + } + } + +mmc_detect_change (host->mmc,msecs_to_jiffies (200 )); +} +EXPORT_SYMBOL_GPL (sdhci_card_detect);
somehow this function seems to have been changed without a lot of obvious changes?
PATCH v5 3/4 sdhci-s3c: add support for new card detection methods (driver part)
Hello,
Quoted 116 lines · Ben Dooks
Quoted 56 lines · Marek Szyprowski
On some Samsung SoCs not all SDHCI controllers have card detect (CD) line. For some embedded designs it is not even needed, because ususally the device (like SDIO flash memory or wifi controller) is permanently wired to the controller. There are also systems which have a card detect line connected to some of the external interrupt lines or the presence of the card depends on some other actions (like enabling a power regulator).
This patch adds support for all these cases. The following card detection methods are possible:
1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller (once detected host won't poll it any more)By default, all existing code would use method #1, what is compatible with the previous version of the driver.
In case of external event, two callbacks must be provided in platdata:
ext_cd_initandext_cd_cleanup. Both of them get a callback to a function that notifies the s3c-sdhci host contoller as their argument. That callback function should be called from the even dispatcher to let host notice the card insertion/removal.In case of external gpio interrupt, a gpio pin number must be provided in platdata (
ext_cd_gpioparameter), as well as the information about the polarity of that gpio pin (ext_cd_gpio_invert). By default (ext_cd_gpio_invert== 0) gpio value 0 means 'card has been removed', but this can be changed to 'card has been removed' whenext_cd_gpio_invert== 1.This patch adds all required changes to sdhci-s3c driver.
Signed-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes since V4:
- adapted to threaded irq approach in sdhci driver
- rebased onto latest -mm kernel tree
- fixed problem when gpio-base card detection was not called on driver initChanges since V3:
- renamed patch to avoid confusion with the patch for the s3c-sdhci platform changes - added "(driver part)" in subjectChanges since V2:
According to Andrew Morton's suggestionlocal_irq_save()call in thesdhci_s3c_notify_changefunction has been replaced byspin_lock_irqsave(host driver already has a spinlock that is used for protecting internal state of the driver).Quoted 6 lines
Changes since V1:
- added support for gpio external interrupt card based detect method directly to sdhci-s3c driver--- drivers/mmc/host/sdhci-s3c.c | 78++++++++++++++++++++++++++++++++++++++++++
Quoted 26 lines
drivers/mmc/host/sdhci.c | 47 ++++++++++++++----------- drivers/mmc/host/sdhci.h | 1 + 3 files changed, 106 insertions(+), 20 deletions(-)drivers/mmc/host/sdhci-s3c.c +3
-0@@ -18,6 +18,7 @@#include <linux/slab.h> #include <linux/clk.h> #include <linux/io.h> +#include <linux/gpio.h> #include <linux/mmc/host.h> @@ -44,6 +45,8 @@ struct sdhci_s3c {struct resource *ioarea;struct s3c_sdhci_platdata *pdata;unsigned int cur_clk; +int ext_cd_irq; +int ext_cd_gpio;struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ];drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = { · line 241
Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +1
-0@@ -235,6 +238,36 @@ static struct sdhci_ops sdhci_s3c_ops = { .get_min_clock = sdhci_s3c_get_min_clock, }; +static void sdhci_s3c_notify_change(struct platform_device *dev, intstate)
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10
-0+{ +struct sdhci_host *host =platform_get_drvdata (dev); +if (host) { +mutex_lock (&host->lock); +if (state) { +dev_dbg (&dev->dev,"card inserted.\n" ); + host->flags &= ~SDHCI_DEVICE_DEAD ; + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + }else {drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = {
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10
-0+dev_dbg (&dev->dev,"card removed.\n" ); + host->flags |=SDHCI_DEVICE_DEAD ; + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION ; +sdhci_card_detect (host); + } +mutex_unlock (&host->lock); + } +} + +static irqreturn_t sdhci_s3c_gpio_card_detect_thread(int irq, void
*dev_id)
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c static struct sdhci_ops sdhci_s3c_ops = { · line 266
Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c +5
-0+{ +struct sdhci_s3c *sc = dev_id; +int status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status;maybe move the invert into
sdhci_s3c_notify_change()
IMHO this is not a good idea. pdata->ext_cd_gpio_invert is used only for GPIO-type card detection. External callback-based card detection can perform similar inversion by itself, no need to duplicate it in sdhci-s3c driver.
Quoted 37 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 9 lines
drivers/mmc/host/sdhci-s3c.c +4
-0+sdhci_s3c_notify_change (sc->pdev, status); +return IRQ_HANDLED ; +} +static int __devinit sdhci_s3c_probe (struct platform_device *pdev) {struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;@@ -272,6 +305,7 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c +1
-0sc->host = host; sc->pdev = pdev; sc->pdata = pdata; + sc->ext_cd_gpio =-1 ;platform_set_drvdata (pdev, host);@@ -355,6 +389,13 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 14 lines
drivers/mmc/host/sdhci-s3c.c +7
-0* SDHCI block, or a missing configuration that needs to be set. */ host->quirks |=SDHCI_QUIRK_NO_BUSY_IRQ ; +if (pdata->cd_type ==S3C_SDHCI_CD_NONE || + pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->quirks |=SDHCI_QUIRK_BROKEN_CARD_DETECTION ; + +if (pdata->cd_type ==S3C_SDHCI_CD_PERMANENT ) + host->mmc->caps =MMC_CAP_NONREMOVABLE ; + host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |SDHCI_QUIRK_32BIT_DMA_SIZE );@@ -367,6 +408,33 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +0
-0goto err_add_host; }
Quoted 6 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev) · line 414
Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +4
-0+/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately, +so it can be called only after sdhci_add_host() */ +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init) + pdata->ext_cd_init (&sdhci_s3c_notify_change);do we really need a callback for this?
Yes, there are cases that will use it. An example is a wifi sdio card, permanently wired to sdhci controller, enabled/disabled by some rfkill glue. This glue enables power regulators and notifies sdhci driver that the card has been 'inserted' or 'removed'. This saves some power as otherwise sdhci driver would need to pool for the card presence.
Quoted 114 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
Quoted 11 lines
drivers/mmc/host/sdhci-s3c.c +11
-0+ + if (pdata->cd_type == S3C_SDHCI_CD_GPIO && + gpio_is_valid(pdata->ext_cd_gpio)) { + int status; + + gpio_request(pdata->ext_cd_gpio, "SDHCI EXT CD"); + sc->ext_cd_gpio = pdata->ext_cd_gpio; + + sc->ext_cd_irq = gpio_to_irq(pdata->ext_cd_gpio); + if (sc->ext_cd_irq && + request_threaded_irq(sc->ext_cd_irq, NULL,
sdhci_s3c_gpio_card_detect_thread,drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +3
-0+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, + dev_name(&pdev->dev), sc)) { + dev_err(&pdev->dev, "cannot request irq for carddetect\n");
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 13 lines
drivers/mmc/host/sdhci-s3c.c +9
-0+ sc->ext_cd_irq =0 ; + } + + status =gpio_get_value (sc->ext_cd_gpio); +if (sc->pdata->ext_cd_gpio_invert) + status = !status; +sdhci_s3c_notify_change (sc->pdev, status); + } +return 0 ; err_add_host:@@ -391,10 +459,20 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)Quoted 25 lines
drivers/mmc/host/sdhci-s3c.c +10
-0static int __devexit sdhci_s3c_remove(struct platform_device *pdev) { +struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;struct sdhci_host *host =platform_get_drvdata (pdev);struct sdhci_s3c *sc =sdhci_priv (host);int ptr; +if (pdata->cd_type ==S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup) + pdata->ext_cd_cleanup (&sdhci_s3c_notify_change); + +if (sc->ext_cd_irq) +free_irq (sc->ext_cd_irq, sc); + +if (sc->ext_cd_gpio !=-1 ) +gpio_free (sc->ext_cd_gpio); +sdhci_remove_host (host,1 );for (ptr =0 ; ptr <3 ; ptr++) {drivers/mmc/host/sdhci.c +0
-0drivers/mmc/host/sdhci.c static const struct mmc_host_ops sdhci_ops = { · line 1244
Quoted 1 line
drivers/mmc/host/sdhci.c +0
-0@@ -1244,26 +1244,6 @@ static const struct mmc_host_ops sdhci_ops = {* *
Quoted 1 line
\************************************************************************** ***/
Quoted 25 lines
drivers/mmc/host/sdhci.c +0
-20-static void sdhci_card_detect (struct sdhci_host *host) -{ -if (!(sdhci_readl (host,SDHCI_PRESENT_STATE ) &SDHCI_CARD_PRESENT )) { -if (host->mrq) { -printk (KERN_ERR "%s: Card removed during transfer!\n" , -mmc_hostname (host->mmc)); -printk (KERN_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 ; -schedule_work (&host->finish_work); - } - } - -mmc_detect_change (host->mmc,msecs_to_jiffies (200 )); -} -static void sdhci_finish_work (struct work_struct *wk) {struct sdhci_host *host;drivers/mmc/host/sdhci.c EXPORT_SYMBOL_GPL(sdhci_resume_host); · line 1608
Quoted 2 lines
drivers/mmc/host/sdhci.c +0
-0@@ -1627,6 +1607,33 @@ EXPORT_SYMBOL_GPL(sdhci_resume_host);/************************************************************************** ***\
drivers/mmc/host/sdhci.c EXPORT_SYMBOL_GPL(sdhci_resume_host); · line 1609
Quoted 1 line
drivers/mmc/host/sdhci.c +0
-0* *
drivers/mmc/host/sdhci.c EXPORT_SYMBOL_GPL(sdhci_resume_host); · line 1610
Quoted 1 line
drivers/mmc/host/sdhci.c +1
-0+* Implementation dependent callbacks *
drivers/mmc/host/sdhci.c EXPORT_SYMBOL_GPL(sdhci_resume_host); · line 1611
Quoted 1 line
drivers/mmc/host/sdhci.c +1
-0+* *
Quoted 1 line
+\************************************************************************* ****/
Quoted 16 lines
drivers/mmc/host/sdhci.c +16
-0+ +void sdhci_card_detect (struct sdhci_host *host) +{ +if (!(sdhci_readl (host,SDHCI_PRESENT_STATE ) &SDHCI_CARD_PRESENT )) { +if (host->mrq) { +printk (KERN_ERR "%s: Card removed during transfer!\n" , +mmc_hostname (host->mmc)); +printk (KERN_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 ; +schedule_work (&host->finish_work); + }
Quoted 8 lines
drivers/mmc/host/sdhci.c EXPORT_SYMBOL_GPL(sdhci_resume_host); · line 1633
Quoted 5 lines
drivers/mmc/host/sdhci.c +5
-0+ } + +mmc_detect_change (host->mmc,msecs_to_jiffies (200 )); +} +EXPORT_SYMBOL_GPL (sdhci_card_detect);somehow this function seems to have been changed without a lot of obvious changes?
I've just moved this function out of 'workers section' to the new 'implementation dependent callbacks' section to make it easier to read the code. The function itself has been modified when sdhci driver has been converted to use threaded irqs rather than tasklets.
I you think that it would be better to leave the function in the place where it was, just let me know.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
PATCH 4/4 sdhci-s3c: add regulator support
In reply to Marek SzyprowskiThis patch adds support for regulator API to sdhci-s3c driver. Regulators can be used to disable power in suspended state to reduce dissipated energy.
--- drivers/mmc/host/sdhci-s3c.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)
drivers/mmc/host/sdhci-s3c.c +29 -1
@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/gpio.h> +#include <linux/regulator/consumer.h> #include <linux/mmc/host.h>
@@ -50,6 +51,7 @@ struct sdhci_s3c {struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ]; +struct regulator *vmmc; };static inline struct sdhci_s3c *to_s3c (struct sdhci_host *host)
@@ -309,6 +311,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)platform_set_drvdata (pdev, host); + sc->vmmc =regulator_get (dev,"vmmc" ); +if (IS_ERR (sc->vmmc)) { +dev_warn (dev,"no vmmc regulator found\n" ); + sc->vmmc =NULL ; + }else +regulator_enable (sc->vmmc); + sc->clk_io =clk_get (dev,"hsmmc" );if (IS_ERR (sc->clk_io)) {dev_err (dev,"failed to get io clock\n" );
@@ -482,6 +491,11 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)clk_disable (sc->clk_io);clk_put (sc->clk_io); +if (sc->vmmc) { +regulator_disable (sc->vmmc); +regulator_put (sc->vmmc); + } +iounmap (host->ioaddr);release_resource (sc->ioarea);kfree (sc->ioarea);
@@ -496,15 +510,29 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)static int sdhci_s3c_suspend (struct platform_device *dev,pm_message_t pm) { +int ret =0 ;struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host);sdhci_suspend_host (host, pm); -return 0 ; + +if (sc->vmmc) + ret =regulator_disable (sc->vmmc); + +return ret; }static int sdhci_s3c_resume (struct platform_device *dev) {struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host); + +if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc); +if (ret) +return ret; +mdelay (2 ); + }sdhci_resume_host (host);return 0 ;
Signature · 1 line
1.7.1.569.g6f426
PATCH 4/4 sdhci-s3c: add regulator support
Op 28-07-10 16:19, Marek Szyprowski schreef:
Quoted 3 lines
This patch adds support for regulator API to sdhci-s3c driver. Regulators can be used to disable power in suspended state to reduce dissipated energy.
I'm not sure about this, when I would try to do this I'd look at implementing this in the sdhci driver itself instead of a subdriver of it.
Quoted 69 lines
Signed-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- drivers/mmc/host/sdhci-s3c.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)drivers/mmc/host/sdhci-s3c.c +21
-1@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/gpio.h> +#include <linux/regulator/consumer.h> #include <linux/mmc/host.h> @@ -50,6 +51,7 @@ struct sdhci_s3c {struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ]; +struct regulator *vmmc; };static inline struct sdhci_s3c *to_s3c (struct sdhci_host *host)@@ -309,6 +311,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)platform_set_drvdata (pdev, host); + sc->vmmc =regulator_get (dev,"vmmc" ); +if (IS_ERR (sc->vmmc)) { +dev_warn (dev,"no vmmc regulator found\n" ); + sc->vmmc =NULL ; + }else +regulator_enable (sc->vmmc); + sc->clk_io =clk_get (dev,"hsmmc" );if (IS_ERR (sc->clk_io)) {dev_err (dev,"failed to get io clock\n" );@@ -482,6 +491,11 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)clk_disable (sc->clk_io);clk_put (sc->clk_io); +if (sc->vmmc) { +regulator_disable (sc->vmmc); +regulator_put (sc->vmmc); + } +iounmap (host->ioaddr);release_resource (sc->ioarea);kfree (sc->ioarea);@@ -496,15 +510,29 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)static int sdhci_s3c_suspend (struct platform_device *dev,pm_message_t pm) { +int ret =0 ;struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host);sdhci_suspend_host (host, pm); -return 0 ; + +if (sc->vmmc) + ret =regulator_disable (sc->vmmc); + +return ret; }
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 534
Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +7
-0static int sdhci_s3c_resume (struct platform_device *dev) {struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host); + +if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc); +if (ret) +return ret; +mdelay (2 );
Shouldn't these delays be handled in the regulator framework itself?
Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +1
-0+ }sdhci_resume_host (host);return 0 ;
Signature · 1 line
Maurus Cuelenaere
PATCH 4/4 sdhci-s3c: add regulator support
Quoted 10 lines · Maurus Cuelenaere
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 534
Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c +7
-0+struct sdhci_s3c *sc =sdhci_priv (host); + +if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc); +if (ret) +return ret; +mdelay (2 );Shouldn't these delays be handled in the regulator framework itself?
A 2ms delay on power down seems suspicious for a regulator. I'm not sure why this is required but if it is I suspect it's due to a large cap on the regulator output and light load rather than something that's always true for whatever regulator is providing the supply. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100728/cb7a4997/attachment-0001.html>
PATCH 4/4 sdhci-s3c: add regulator support
Op 28-07-10 17:41, Mark Brown schreef:
Quoted 10 lines · Maurus Cuelenaere
Quoted 8 lines
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 534
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c +7
-0+struct sdhci_s3c *sc =sdhci_priv (host); + +if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc); +if (ret) +return ret; +mdelay (2 );Shouldn't these delays be handled in the regulator framework itself?
A 2ms delay on power down seems suspicious for a regulator. I'm not sure why this is required but if it is I suspect it's due to a large cap on the regulator output and light load rather than something that's always true for whatever regulator is providing the supply.
I wasn't suggesting to do the delay in the framework *itself*, rather in the regulator driver and/or the board platform code which needs this delay.
Signature · 1 line
Maurus Cuelenaere
PATCH 4/4 sdhci-s3c: add regulator support
Quoted 10 lines · Maurus Cuelenaere
Op 28-07-10 17:41, Mark Brown schreef:
Quoted 7 lines
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 534
Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c +5
-0+if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc); +if (ret) +return ret; +mdelay (2 );Shouldn't these delays be handled in the regulator framework itself?
Quoted 7 lines
Quoted 4 lines
A 2ms delay on power down seems suspicious for a regulator. I'm not sure why this is required but if it is I suspect it's due to a large cap on the regulator output and light load rather than something that's always true for whatever regulator is providing the supply.
I wasn't suggesting to do the delay in the framework *itself*, rather in the regulator driver and/or the board platform code which needs this delay.
It's unlikely to be a property of the regulator in general, it sounds like it's a property of the MMC application somehow (most likely due to very light loading). Possibly it should be platform configurable, but I suspect in the MMC driver.
It'd be nice to know exactly what the delay is for...
PATCH 4/4 sdhci-s3c: add regulator support
In reply to Maurus CuelenaereHello,
Quoted 10 lines · Maurus Cuelenaere
-----Original Message-----
From: [mailto:mcuelenaere at gmail.com]
Sent:
To: Marek Szyprowski
Cc: linux-mmc at vger.kernel.org; linux-samsung-soc at vger.kernel.org; linux- arm-kernel at lists.infradead.org; kyungmin.park at samsung.com; kgene.kim at samsung.com; ben-linux at fluff.org; akpm at linux-foundation.org; broonie at opensource.wolfsonmicro.com
Subject: Re: [PATCH 4/4] sdhci-s3c: add regulator support
Quoted 8 lines
Op 28-07-10 16:19, Marek Szyprowski schreef:
Quoted 3 lines
This patch adds support for regulator API to sdhci-s3c driver. Regulators can be used to disable power in suspended state to reduce dissipated energy.
I'm not sure about this, when I would try to do this I'd look at implementing
this in the sdhci driver itself instead of a subdriver of it.
Well, if you think that sdhci driver is better place for this I can move all my changes to the main driver.
Quoted 73 lines
drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct
Quoted 27 lines
Signed-off-by Marek Szyprowski <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- drivers/mmc/host/sdhci-s3c.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)drivers/mmc/host/sdhci-s3c.c +2
-0@@ -19,6 +19,7 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/gpio.h> +#include <linux/regulator/consumer.h> #include <linux/mmc/host.h> @@ -50,6 +51,7 @@ struct sdhci_s3c {struct clk *clk_io;struct clk *clk_bus[MAX_BUS_CLK ]; +struct regulator *vmmc; };static inline struct sdhci_s3c *to_s3c (struct sdhci_host *host)@@ -309,6 +311,13 @@ static int __devinit sdhci_s3c_probe(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct
Quoted 14 lines
drivers/mmc/host/sdhci-s3c.c +7
-0platform_set_drvdata (pdev, host); + sc->vmmc =regulator_get (dev,"vmmc" ); +if (IS_ERR (sc->vmmc)) { +dev_warn (dev,"no vmmc regulator found\n" ); + sc->vmmc =NULL ; + }else +regulator_enable (sc->vmmc); + sc->clk_io =clk_get (dev,"hsmmc" );if (IS_ERR (sc->clk_io)) {dev_err (dev,"failed to get io clock\n" );@@ -482,6 +491,11 @@ static int __devexit sdhci_s3c_remove(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct
Quoted 12 lines
drivers/mmc/host/sdhci-s3c.c +5
-0clk_disable (sc->clk_io);clk_put (sc->clk_io); +if (sc->vmmc) { +regulator_disable (sc->vmmc); +regulator_put (sc->vmmc); + } +iounmap (host->ioaddr);release_resource (sc->ioarea);kfree (sc->ioarea);@@ -496,15 +510,29 @@ static int __devexit sdhci_s3c_remove(struct
platform_device*pdev)drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct · line 510
Quoted 2 lines
drivers/mmc/host/sdhci-s3c.c +0
-0static int sdhci_s3c_suspend(struct platform_device *dev, pm_message_tpm)
Quoted 14 lines
drivers/mmc/host/sdhci-s3c.c +7
-1{ +int ret =0 ;struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host);sdhci_suspend_host (host, pm); -return 0 ; + +if (sc->vmmc) + ret =regulator_disable (sc->vmmc); + +return ret; }
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 531
Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c +4
-0static int sdhci_s3c_resume (struct platform_device *dev) {struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host); + +if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc);
Hmm, I was in a real hurry if I missed this. Should be regulator_enable() of course. Looks that I confused patches and posted an older version...
Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 534
Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +3
-0+if (ret) +return ret; +mdelay (2 );Shouldn't these delays be handled in the regulator framework itself?
This is just a left-over from an older version, should be definitely removed. Sorry for the confusion.
Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 538
Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +1
-0+ }sdhci_resume_host (host);return 0 ;
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
PATCH 4/4 sdhci-s3c: add regulator support
In reply to Marek Szyprowskidrivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 521
Quoted 12 lines · Marek Szyprowski
static int
sdhci_s3c_suspend(struct platform_device *dev, pm_message_t pm){drivers/mmc/host/sdhci-s3c.c +6
-1+int ret =0 ;struct sdhci_host *host =platform_get_drvdata (dev); +struct sdhci_s3c *sc =sdhci_priv (host);sdhci_suspend_host (host, pm); -return 0 ; + +if (sc->vmmc) + ret =regulator_disable (sc->vmmc); +
So, on suspend you disable the regulator...
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 533
Quoted 6 lines
drivers/mmc/host/sdhci-s3c.c +6
-0+struct sdhci_s3c *sc =sdhci_priv (host); + +if (sc->vmmc) { +int ret =regulator_disable (sc->vmmc); +if (ret) +return ret;
...and on resume you disable it again. I'm surprised this works properly after resume, at the very least the core is going to get confused about the reference counts I'd expect.
I suspect this is for something we need to do better in the regulator API - restoring the state of the regulators after a resume if the hardware changes them. A lot of regulators just do this in hardware so we didn't need to worry about it previously.
drivers/mmc/host/sdhci-s3c.c static int __devexit sdhci_s3c_remove(struct platform_device *pdev) · line 534
Quoted 1 line
drivers/mmc/host/sdhci-s3c.c +1
-0+mdelay (2 );
This could do with some sort of comment.