diff mbox series

[2/2,v2] mmc: omap_hsmmc: Delete platform data GPIO CD and WP

Message ID 20180924113051.8699-2-linus.walleij@linaro.org
State New
Headers show
Series [1/2,v2] mmc: omap_hsmmc: Kill off cover detection | expand

Commit Message

Linus Walleij Sept. 24, 2018, 11:30 a.m. UTC 
The OMAP HSMMC driver has some elaborate and hairy handling for
passing GPIO card detect and write protect lines from a boardfile
into the driver: the machine defines a struct omap2_hsmmc_info
that is copied into struct omap_hsmmc_platform_data by
omap_hsmmc_pdata_init() in arch/arm/mach-omap2/hsmmc.c.

However the .gpio_cd and .gpio_wp fields are not copied from
omap2_hsmmc_info to omap_hsmmc_platform_data by
omap_hsmmc_pdata_init() so they remain unused. The only platform
defining omap2_hsmmc_info also define both to -1, unused.

It turn out there are no boardfiles passing any valid GPIO
lines into the OMAP HSMMC driver at all. And since we are not
going to add any more OMAP2 boardfiles, we can delete this
card detect and write protect handling altogether.

This seems to also fix a bug: the card detect callback
mmc_gpio_get_cd() in the slot GPIO core needs to be called
by drivers utilizing slot GPIO. It appears the the boardfile
quirks were not doing this right, so this would only get
called for boardfiles, i.e. since no boardfile was using it,
never.

Just assign mmc_gpio_get_cd() unconditionally to omap_hsmmc_ops
.get_cd() so card detects from the device tree works.
AFAICT card detect with GPIO lines assigned from
mmc_of_parse() are not working at the moment, but that is
no regression since it probably never worked.

Cc: Tony Lindgren <tony@atomide.com>
Cc: linux-omap@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
ChangeLog v1->v2:
- New patch after discovering that the whole card detect
  and write protect logic was unused.
---
 arch/arm/mach-omap2/hsmmc.h              |  2 -
 arch/arm/mach-omap2/pdata-quirks.c       |  2 -
 drivers/mmc/host/omap_hsmmc.c            | 52 +-----------------------
 include/linux/platform_data/hsmmc-omap.h |  2 -
 4 files changed, 1 insertion(+), 57 deletions(-)

-- 
2.17.1

Comments

Ulf Hansson Sept. 26, 2018, 11:48 p.m. UTC | #1 
On 24 September 2018 at 13:30, Linus Walleij <linus.walleij@linaro.org> wrote:
> The OMAP HSMMC driver has some elaborate and hairy handling for

> passing GPIO card detect and write protect lines from a boardfile

> into the driver: the machine defines a struct omap2_hsmmc_info

> that is copied into struct omap_hsmmc_platform_data by

> omap_hsmmc_pdata_init() in arch/arm/mach-omap2/hsmmc.c.

>

> However the .gpio_cd and .gpio_wp fields are not copied from

> omap2_hsmmc_info to omap_hsmmc_platform_data by

> omap_hsmmc_pdata_init() so they remain unused. The only platform

> defining omap2_hsmmc_info also define both to -1, unused.

>

> It turn out there are no boardfiles passing any valid GPIO

> lines into the OMAP HSMMC driver at all. And since we are not

> going to add any more OMAP2 boardfiles, we can delete this

> card detect and write protect handling altogether.

>

> This seems to also fix a bug: the card detect callback

> mmc_gpio_get_cd() in the slot GPIO core needs to be called

> by drivers utilizing slot GPIO. It appears the the boardfile

> quirks were not doing this right, so this would only get

> called for boardfiles, i.e. since no boardfile was using it,

> never.

>

> Just assign mmc_gpio_get_cd() unconditionally to omap_hsmmc_ops

> .get_cd() so card detects from the device tree works.

> AFAICT card detect with GPIO lines assigned from

> mmc_of_parse() are not working at the moment, but that is

> no regression since it probably never worked.

>

> Cc: Tony Lindgren <tony@atomide.com>

> Cc: linux-omap@vger.kernel.org

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


Applied for next, thanks!

Kind regards
Uffe

> ---

> ChangeLog v1->v2:

> - New patch after discovering that the whole card detect

>   and write protect logic was unused.

> ---

>  arch/arm/mach-omap2/hsmmc.h              |  2 -

>  arch/arm/mach-omap2/pdata-quirks.c       |  2 -

>  drivers/mmc/host/omap_hsmmc.c            | 52 +-----------------------

>  include/linux/platform_data/hsmmc-omap.h |  2 -

>  4 files changed, 1 insertion(+), 57 deletions(-)

>

> diff --git a/arch/arm/mach-omap2/hsmmc.h b/arch/arm/mach-omap2/hsmmc.h

> index af9af5094ec3..bf99aec5a155 100644

> --- a/arch/arm/mach-omap2/hsmmc.h

> +++ b/arch/arm/mach-omap2/hsmmc.h

> @@ -12,8 +12,6 @@ struct omap2_hsmmc_info {

>         u8      mmc;            /* controller 1/2/3 */

>         u32     caps;           /* 4/8 wires and any additional host

>                                  * capabilities OR'd (ref. linux/mmc/host.h) */

> -       int     gpio_cd;        /* or -EINVAL */

> -       int     gpio_wp;        /* or -EINVAL */

>         struct platform_device *pdev;   /* mmc controller instance */

>         /* init some special card */

>         void (*init_card)(struct mmc_card *card);

> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c

> index 7f02743edbe4..fe7c1fdb51d8 100644

> --- a/arch/arm/mach-omap2/pdata-quirks.c

> +++ b/arch/arm/mach-omap2/pdata-quirks.c

> @@ -363,8 +363,6 @@ static struct omap2_hsmmc_info pandora_mmc3[] = {

>         {

>                 .mmc            = 3,

>                 .caps           = MMC_CAP_4_BIT_DATA | MMC_CAP_POWER_OFF_CARD,

> -               .gpio_cd        = -EINVAL,

> -               .gpio_wp        = -EINVAL,

>                 .init_card      = pandora_wl1251_init_card,

>         },

>         {}      /* Terminator */

> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c

> index 3b38d2a592c2..1b57e23263e9 100644

> --- a/drivers/mmc/host/omap_hsmmc.c

> +++ b/drivers/mmc/host/omap_hsmmc.c

> @@ -30,7 +30,6 @@

>  #include <linux/clk.h>

>  #include <linux/of.h>

>  #include <linux/of_irq.h>

> -#include <linux/of_gpio.h>

>  #include <linux/of_device.h>

>  #include <linux/mmc/host.h>

>  #include <linux/mmc/core.h>

> @@ -38,7 +37,6 @@

>  #include <linux/mmc/slot-gpio.h>

>  #include <linux/io.h>

>  #include <linux/irq.h>

> -#include <linux/gpio.h>

>  #include <linux/regulator/consumer.h>

>  #include <linux/pinctrl/consumer.h>

>  #include <linux/pm_runtime.h>

> @@ -206,7 +204,6 @@ struct omap_hsmmc_host {

>  #define HSMMC_SDIO_IRQ_ENABLED (1 << 1)        /* SDIO irq enabled */

>         struct omap_hsmmc_next  next_data;

>         struct  omap_hsmmc_platform_data        *pdata;

> -       int (*card_detect)(struct device *dev);

>  };

>

>  struct omap_mmc_of_data {

> @@ -216,13 +213,6 @@ struct omap_mmc_of_data {

>

>  static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host);

>

> -static int omap_hsmmc_card_detect(struct device *dev)

> -{

> -       struct omap_hsmmc_host *host = dev_get_drvdata(dev);

> -

> -       return mmc_gpio_get_cd(host->mmc);

> -}

> -

>  static int omap_hsmmc_enable_supply(struct mmc_host *mmc)

>  {

>         int ret;

> @@ -467,29 +457,6 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)

>         return 0;

>  }

>

> -static int omap_hsmmc_gpio_init(struct mmc_host *mmc,

> -                               struct omap_hsmmc_host *host,

> -                               struct omap_hsmmc_platform_data *pdata)

> -{

> -       int ret;

> -

> -       if (gpio_is_valid(pdata->gpio_cd)) {

> -               ret = mmc_gpio_request_cd(mmc, pdata->gpio_cd, 0);

> -               if (ret)

> -                       return ret;

> -

> -               host->card_detect = omap_hsmmc_card_detect;

> -       }

> -

> -       if (gpio_is_valid(pdata->gpio_wp)) {

> -               ret = mmc_gpio_request_ro(mmc, pdata->gpio_wp);

> -               if (ret)

> -                       return ret;

> -       }

> -

> -       return 0;

> -}

> -

>  /*

>   * Start clock to the card

>   */

> @@ -1539,15 +1506,6 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

>         omap_hsmmc_set_bus_mode(host);

>  }

>

> -static int omap_hsmmc_get_cd(struct mmc_host *mmc)

> -{

> -       struct omap_hsmmc_host *host = mmc_priv(mmc);

> -

> -       if (!host->card_detect)

> -               return -ENOSYS;

> -       return host->card_detect(host->dev);

> -}

> -

>  static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)

>  {

>         struct omap_hsmmc_host *host = mmc_priv(mmc);

> @@ -1686,7 +1644,7 @@ static struct mmc_host_ops omap_hsmmc_ops = {

>         .pre_req = omap_hsmmc_pre_req,

>         .request = omap_hsmmc_request,

>         .set_ios = omap_hsmmc_set_ios,

> -       .get_cd = omap_hsmmc_get_cd,

> +       .get_cd = mmc_gpio_get_cd,

>         .get_ro = mmc_gpio_get_ro,

>         .init_card = omap_hsmmc_init_card,

>         .enable_sdio_irq = omap_hsmmc_enable_sdio_irq,

> @@ -1813,9 +1771,6 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)

>         if (of_find_property(np, "ti,dual-volt", NULL))

>                 pdata->controller_flags |= OMAP_HSMMC_SUPPORTS_DUAL_VOLT;

>

> -       pdata->gpio_cd = -EINVAL;

> -       pdata->gpio_wp = -EINVAL;

> -

>         if (of_find_property(np, "ti,non-removable", NULL)) {

>                 pdata->nonremovable = true;

>                 pdata->no_regulator_off_init = true;

> @@ -1900,10 +1855,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)

>         host->pbias_enabled = 0;

>         host->vqmmc_enabled = 0;

>

> -       ret = omap_hsmmc_gpio_init(mmc, host, pdata);

> -       if (ret)

> -               goto err_gpio;

> -

>         platform_set_drvdata(pdev, host);

>

>         if (pdev->dev.of_node)

> @@ -2045,7 +1996,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)

>         if (host->dbclk)

>                 clk_disable_unprepare(host->dbclk);

>  err1:

> -err_gpio:

>         mmc_free_host(mmc);

>  err:

>         return ret;

> diff --git a/include/linux/platform_data/hsmmc-omap.h b/include/linux/platform_data/hsmmc-omap.h

> index c055d7eda085..85da11916bd5 100644

> --- a/include/linux/platform_data/hsmmc-omap.h

> +++ b/include/linux/platform_data/hsmmc-omap.h

> @@ -70,8 +70,6 @@ struct omap_hsmmc_platform_data {

>         /* string specifying a particular variant of hardware */

>         char *version;

>

> -       int gpio_cd;                    /* gpio (card detect) */

> -       int gpio_wp;                    /* gpio (write protect) */

>         /* if we have special card, init it using this callback */

>         void (*init_card)(struct mmc_card *card);

>

> --

> 2.17.1

>
Aaro Koskinen Dec. 22, 2018, 2:47 p.m. UTC | #2 
Hi,

On Sat, Dec 22, 2018 at 04:28:48PM +0200, Aaro Koskinen wrote:
> After this patch, the external MMC card is not getting detected on N900

> test/dev boards (different mechanics) unless I comment out the cd-gpios

> in the DTS.

> 

> It seems on N900 we had these GPIOs defined to 0 by accident in

> pdata-quicks, and that somehow allowed the external slot MMC work.

> 

> Anyway, do we have any other option than maintaining a different DTS for

> the test/dev boards? So far, the current DTS is intended to work on both

> (e.g. we have Ethernet in there too).


OK, found a solution. :-)

A.

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 182a53991c90..3ffb21d15882 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -813,7 +813,9 @@
 	bus-width = <4>;
 	/* For debugging, it is often good idea to remove this GPIO.
 	   It means you can remove back cover (to reboot by removing
-	   battery) and still use the MMC card. */
+	   battery) and still use the MMC card. NOTE: On the early development
+	   boards, this GPIO is controlled by the second switch behind the USB
+	   port. */
 	cd-gpios = <&gpio6 0 GPIO_ACTIVE_HIGH>; /* 160 */
 };
diff mbox series

Patch

diff --git a/arch/arm/mach-omap2/hsmmc.h b/arch/arm/mach-omap2/hsmmc.h
index af9af5094ec3..bf99aec5a155 100644
--- a/arch/arm/mach-omap2/hsmmc.h
+++ b/arch/arm/mach-omap2/hsmmc.h
@@ -12,8 +12,6 @@  struct omap2_hsmmc_info {
 	u8	mmc;		/* controller 1/2/3 */
 	u32	caps;		/* 4/8 wires and any additional host
 				 * capabilities OR'd (ref. linux/mmc/host.h) */
-	int	gpio_cd;	/* or -EINVAL */
-	int	gpio_wp;	/* or -EINVAL */
 	struct platform_device *pdev;	/* mmc controller instance */
 	/* init some special card */
 	void (*init_card)(struct mmc_card *card);
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 7f02743edbe4..fe7c1fdb51d8 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -363,8 +363,6 @@  static struct omap2_hsmmc_info pandora_mmc3[] = {
 	{
 		.mmc		= 3,
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_POWER_OFF_CARD,
-		.gpio_cd	= -EINVAL,
-		.gpio_wp	= -EINVAL,
 		.init_card	= pandora_wl1251_init_card,
 	},
 	{}	/* Terminator */
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 3b38d2a592c2..1b57e23263e9 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -30,7 +30,6 @@ 
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
-#include <linux/of_gpio.h>
 #include <linux/of_device.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/core.h>
@@ -38,7 +37,6 @@ 
 #include <linux/mmc/slot-gpio.h>
 #include <linux/io.h>
 #include <linux/irq.h>
-#include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/pm_runtime.h>
@@ -206,7 +204,6 @@  struct omap_hsmmc_host {
 #define HSMMC_SDIO_IRQ_ENABLED	(1 << 1)        /* SDIO irq enabled */
 	struct omap_hsmmc_next	next_data;
 	struct	omap_hsmmc_platform_data	*pdata;
-	int (*card_detect)(struct device *dev);
 };
 
 struct omap_mmc_of_data {
@@ -216,13 +213,6 @@  struct omap_mmc_of_data {
 
 static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host);
 
-static int omap_hsmmc_card_detect(struct device *dev)
-{
-	struct omap_hsmmc_host *host = dev_get_drvdata(dev);
-
-	return mmc_gpio_get_cd(host->mmc);
-}
-
 static int omap_hsmmc_enable_supply(struct mmc_host *mmc)
 {
 	int ret;
@@ -467,29 +457,6 @@  static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
 	return 0;
 }
 
-static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
-				struct omap_hsmmc_host *host,
-				struct omap_hsmmc_platform_data *pdata)
-{
-	int ret;
-
-	if (gpio_is_valid(pdata->gpio_cd)) {
-		ret = mmc_gpio_request_cd(mmc, pdata->gpio_cd, 0);
-		if (ret)
-			return ret;
-
-		host->card_detect = omap_hsmmc_card_detect;
-	}
-
-	if (gpio_is_valid(pdata->gpio_wp)) {
-		ret = mmc_gpio_request_ro(mmc, pdata->gpio_wp);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 /*
  * Start clock to the card
  */
@@ -1539,15 +1506,6 @@  static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	omap_hsmmc_set_bus_mode(host);
 }
 
-static int omap_hsmmc_get_cd(struct mmc_host *mmc)
-{
-	struct omap_hsmmc_host *host = mmc_priv(mmc);
-
-	if (!host->card_detect)
-		return -ENOSYS;
-	return host->card_detect(host->dev);
-}
-
 static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
@@ -1686,7 +1644,7 @@  static struct mmc_host_ops omap_hsmmc_ops = {
 	.pre_req = omap_hsmmc_pre_req,
 	.request = omap_hsmmc_request,
 	.set_ios = omap_hsmmc_set_ios,
-	.get_cd = omap_hsmmc_get_cd,
+	.get_cd = mmc_gpio_get_cd,
 	.get_ro = mmc_gpio_get_ro,
 	.init_card = omap_hsmmc_init_card,
 	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
@@ -1813,9 +1771,6 @@  static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 	if (of_find_property(np, "ti,dual-volt", NULL))
 		pdata->controller_flags |= OMAP_HSMMC_SUPPORTS_DUAL_VOLT;
 
-	pdata->gpio_cd = -EINVAL;
-	pdata->gpio_wp = -EINVAL;
-
 	if (of_find_property(np, "ti,non-removable", NULL)) {
 		pdata->nonremovable = true;
 		pdata->no_regulator_off_init = true;
@@ -1900,10 +1855,6 @@  static int omap_hsmmc_probe(struct platform_device *pdev)
 	host->pbias_enabled = 0;
 	host->vqmmc_enabled = 0;
 
-	ret = omap_hsmmc_gpio_init(mmc, host, pdata);
-	if (ret)
-		goto err_gpio;
-
 	platform_set_drvdata(pdev, host);
 
 	if (pdev->dev.of_node)
@@ -2045,7 +1996,6 @@  static int omap_hsmmc_probe(struct platform_device *pdev)
 	if (host->dbclk)
 		clk_disable_unprepare(host->dbclk);
 err1:
-err_gpio:
 	mmc_free_host(mmc);
 err:
 	return ret;
diff --git a/include/linux/platform_data/hsmmc-omap.h b/include/linux/platform_data/hsmmc-omap.h
index c055d7eda085..85da11916bd5 100644
--- a/include/linux/platform_data/hsmmc-omap.h
+++ b/include/linux/platform_data/hsmmc-omap.h
@@ -70,8 +70,6 @@  struct omap_hsmmc_platform_data {
 	/* string specifying a particular variant of hardware */
 	char *version;
 
-	int gpio_cd;			/* gpio (card detect) */
-	int gpio_wp;			/* gpio (write protect) */
 	/* if we have special card, init it using this callback */
 	void (*init_card)(struct mmc_card *card);