linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0
@ 2024-02-23 18:14 Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 1/5] ARM: OMAP: fix bogus MMC GPIO labels " Aaro Koskinen
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Aaro Koskinen @ 2024-02-23 18:14 UTC (permalink / raw)
  To: Tony Lindgren, Ulf Hansson, Linus Walleij, linux-omap
  Cc: linux-kernel, Aaro Koskinen

Hi,

Nokia N8x0 MMC has been pretty much broken starting from v6.3. These
patches restore the functionality. Tested on N810 with eMMC and external
miniSD card, and on N800 with SD card in the inner slot.

A.

Aaro Koskinen (5):
  ARM: OMAP: fix bogus MMC GPIO labels on Nokia N8x0
  ARM: OMAP: fix N810 MMC gpiod table
  MMC: OMAP: fix broken slot switch lookup
  MMC: OMAP: fix deferred probe
  MMC: OMAP: restore original power up/down steps

 arch/arm/mach-omap2/board-n8x0.c | 17 ++++++-----
 drivers/mmc/host/omap.c          | 48 +++++++++++++++++++++-----------
 2 files changed, 39 insertions(+), 26 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] ARM: OMAP: fix bogus MMC GPIO labels on Nokia N8x0
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
@ 2024-02-23 18:14 ` Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 2/5] ARM: OMAP: fix N810 MMC gpiod table Aaro Koskinen
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Aaro Koskinen @ 2024-02-23 18:14 UTC (permalink / raw)
  To: Tony Lindgren, Ulf Hansson, Linus Walleij, linux-omap
  Cc: linux-kernel, Aaro Koskinen

The GPIO bank width is 32 on OMAP2, so all labels are incorrect.

Fixes: e519f0bb64ef ("ARM/mmc: Convert old mmci-omap to GPIO descriptors")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 arch/arm/mach-omap2/board-n8x0.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 31755a378c73..3e48f34016c1 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -144,8 +144,7 @@ static struct gpiod_lookup_table nokia8xx_mmc_gpio_table = {
 	.dev_id = "mmci-omap.0",
 	.table = {
 		/* Slot switch, GPIO 96 */
-		GPIO_LOOKUP("gpio-80-111", 16,
-			    "switch", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("gpio-96-127", 0, "switch", GPIO_ACTIVE_HIGH),
 		{ }
 	},
 };
@@ -154,11 +153,9 @@ static struct gpiod_lookup_table nokia810_mmc_gpio_table = {
 	.dev_id = "mmci-omap.0",
 	.table = {
 		/* Slot index 1, VSD power, GPIO 23 */
-		GPIO_LOOKUP_IDX("gpio-16-31", 7,
-				"vsd", 1, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-0-31", 23, "vsd", 1, GPIO_ACTIVE_HIGH),
 		/* Slot index 1, VIO power, GPIO 9 */
-		GPIO_LOOKUP_IDX("gpio-0-15", 9,
-				"vio", 1, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-0-31", 9, "vio", 1, GPIO_ACTIVE_HIGH),
 		{ }
 	},
 };
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] ARM: OMAP: fix N810 MMC gpiod table
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 1/5] ARM: OMAP: fix bogus MMC GPIO labels " Aaro Koskinen
@ 2024-02-23 18:14 ` Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 3/5] MMC: OMAP: fix broken slot switch lookup Aaro Koskinen
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Aaro Koskinen @ 2024-02-23 18:14 UTC (permalink / raw)
  To: Tony Lindgren, Ulf Hansson, Linus Walleij, linux-omap
  Cc: linux-kernel, Aaro Koskinen

Trying to append a second table for the same dev_id doesn't seem to work.
The second table is just silently ignored. As a result eMMC GPIOs are not
present.

Fix by using separate tables for N800 and N810.

Fixes: e519f0bb64ef ("ARM/mmc: Convert old mmci-omap to GPIO descriptors")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 arch/arm/mach-omap2/board-n8x0.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 3e48f34016c1..c933a91751e4 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -140,7 +140,7 @@ static int slot1_cover_open;
 static int slot2_cover_open;
 static struct device *mmc_device;
 
-static struct gpiod_lookup_table nokia8xx_mmc_gpio_table = {
+static struct gpiod_lookup_table nokia800_mmc_gpio_table = {
 	.dev_id = "mmci-omap.0",
 	.table = {
 		/* Slot switch, GPIO 96 */
@@ -152,6 +152,8 @@ static struct gpiod_lookup_table nokia8xx_mmc_gpio_table = {
 static struct gpiod_lookup_table nokia810_mmc_gpio_table = {
 	.dev_id = "mmci-omap.0",
 	.table = {
+		/* Slot switch, GPIO 96 */
+		GPIO_LOOKUP("gpio-96-127", 0, "switch", GPIO_ACTIVE_HIGH),
 		/* Slot index 1, VSD power, GPIO 23 */
 		GPIO_LOOKUP_IDX("gpio-0-31", 23, "vsd", 1, GPIO_ACTIVE_HIGH),
 		/* Slot index 1, VIO power, GPIO 9 */
@@ -412,8 +414,6 @@ static struct omap_mmc_platform_data *mmc_data[OMAP24XX_NR_MMC];
 
 static void __init n8x0_mmc_init(void)
 {
-	gpiod_add_lookup_table(&nokia8xx_mmc_gpio_table);
-
 	if (board_is_n810()) {
 		mmc1_data.slots[0].name = "external";
 
@@ -426,6 +426,8 @@ static void __init n8x0_mmc_init(void)
 		mmc1_data.slots[1].name = "internal";
 		mmc1_data.slots[1].ban_openended = 1;
 		gpiod_add_lookup_table(&nokia810_mmc_gpio_table);
+	} else {
+		gpiod_add_lookup_table(&nokia800_mmc_gpio_table);
 	}
 
 	mmc1_data.nr_slots = 2;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] MMC: OMAP: fix broken slot switch lookup
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 1/5] ARM: OMAP: fix bogus MMC GPIO labels " Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 2/5] ARM: OMAP: fix N810 MMC gpiod table Aaro Koskinen
@ 2024-02-23 18:14 ` Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 4/5] MMC: OMAP: fix deferred probe Aaro Koskinen
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Aaro Koskinen @ 2024-02-23 18:14 UTC (permalink / raw)
  To: Tony Lindgren, Ulf Hansson, Linus Walleij, linux-omap
  Cc: linux-kernel, Aaro Koskinen

The lookup is done before host->dev is initialized. It will always just
fail silently, and the MMC behaviour is totally unpredictable as the switch
is left in an undefined state. Fix that.

Fixes: e519f0bb64ef ("ARM/mmc: Convert old mmci-omap to GPIO descriptors")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/mmc/host/omap.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 9fb8995b43a1..aa40e1a9dc29 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1384,13 +1384,6 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	if (IS_ERR(host->virt_base))
 		return PTR_ERR(host->virt_base);
 
-	host->slot_switch = gpiod_get_optional(host->dev, "switch",
-					       GPIOD_OUT_LOW);
-	if (IS_ERR(host->slot_switch))
-		return dev_err_probe(host->dev, PTR_ERR(host->slot_switch),
-				     "error looking up slot switch GPIO\n");
-
-
 	INIT_WORK(&host->slot_release_work, mmc_omap_slot_release_work);
 	INIT_WORK(&host->send_stop_work, mmc_omap_send_stop_work);
 
@@ -1409,6 +1402,12 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	host->dev = &pdev->dev;
 	platform_set_drvdata(pdev, host);
 
+	host->slot_switch = gpiod_get_optional(host->dev, "switch",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(host->slot_switch))
+		return dev_err_probe(host->dev, PTR_ERR(host->slot_switch),
+				     "error looking up slot switch GPIO\n");
+
 	host->id = pdev->id;
 	host->irq = irq;
 	host->phys_base = res->start;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] MMC: OMAP: fix deferred probe
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
                   ` (2 preceding siblings ...)
  2024-02-23 18:14 ` [PATCH 3/5] MMC: OMAP: fix broken slot switch lookup Aaro Koskinen
@ 2024-02-23 18:14 ` Aaro Koskinen
  2024-02-23 18:14 ` [PATCH 5/5] MMC: OMAP: restore original power up/down steps Aaro Koskinen
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Aaro Koskinen @ 2024-02-23 18:14 UTC (permalink / raw)
  To: Tony Lindgren, Ulf Hansson, Linus Walleij, linux-omap
  Cc: linux-kernel, Aaro Koskinen

After a deferred probe, GPIO descriptor lookup will fail with EBUSY. Fix by
using managed descriptors.

Fixes: e519f0bb64ef ("ARM/mmc: Convert old mmci-omap to GPIO descriptors")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/mmc/host/omap.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index aa40e1a9dc29..50408771ae01 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1259,18 +1259,18 @@ static int mmc_omap_new_slot(struct mmc_omap_host *host, int id)
 	slot->pdata = &host->pdata->slots[id];
 
 	/* Check for some optional GPIO controls */
-	slot->vsd = gpiod_get_index_optional(host->dev, "vsd",
-					     id, GPIOD_OUT_LOW);
+	slot->vsd = devm_gpiod_get_index_optional(host->dev, "vsd",
+						  id, GPIOD_OUT_LOW);
 	if (IS_ERR(slot->vsd))
 		return dev_err_probe(host->dev, PTR_ERR(slot->vsd),
 				     "error looking up VSD GPIO\n");
-	slot->vio = gpiod_get_index_optional(host->dev, "vio",
-					     id, GPIOD_OUT_LOW);
+	slot->vio = devm_gpiod_get_index_optional(host->dev, "vio",
+						  id, GPIOD_OUT_LOW);
 	if (IS_ERR(slot->vio))
 		return dev_err_probe(host->dev, PTR_ERR(slot->vio),
 				     "error looking up VIO GPIO\n");
-	slot->cover = gpiod_get_index_optional(host->dev, "cover",
-						id, GPIOD_IN);
+	slot->cover = devm_gpiod_get_index_optional(host->dev, "cover",
+						    id, GPIOD_IN);
 	if (IS_ERR(slot->cover))
 		return dev_err_probe(host->dev, PTR_ERR(slot->cover),
 				     "error looking up cover switch GPIO\n");
@@ -1402,8 +1402,8 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	host->dev = &pdev->dev;
 	platform_set_drvdata(pdev, host);
 
-	host->slot_switch = gpiod_get_optional(host->dev, "switch",
-					       GPIOD_OUT_LOW);
+	host->slot_switch = devm_gpiod_get_optional(host->dev, "switch",
+						    GPIOD_OUT_LOW);
 	if (IS_ERR(host->slot_switch))
 		return dev_err_probe(host->dev, PTR_ERR(host->slot_switch),
 				     "error looking up slot switch GPIO\n");
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] MMC: OMAP: restore original power up/down steps
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
                   ` (3 preceding siblings ...)
  2024-02-23 18:14 ` [PATCH 4/5] MMC: OMAP: fix deferred probe Aaro Koskinen
@ 2024-02-23 18:14 ` Aaro Koskinen
  2024-02-23 20:47 ` [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Linus Walleij
  2024-02-29 15:06 ` Ulf Hansson
  6 siblings, 0 replies; 9+ messages in thread
From: Aaro Koskinen @ 2024-02-23 18:14 UTC (permalink / raw)
  To: Tony Lindgren, Ulf Hansson, Linus Walleij, linux-omap
  Cc: linux-kernel, Aaro Koskinen

Commit e519f0bb64ef ("ARM/mmc: Convert old mmci-omap to GPIO descriptors")
moved Nokia N810 MMC power up/down from the board file into the MMC driver.

The change removed some delays, and ordering without a valid reason.
Restore power up/down to match the original code. This matters only on N810
where the 2nd GPIO is in use. Other boards will see an additional delay but
that should be a lesser concern than omitting delays altogether.

Fixes: e519f0bb64ef ("ARM/mmc: Convert old mmci-omap to GPIO descriptors")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/mmc/host/omap.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 50408771ae01..13fa8588e38c 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1119,10 +1119,25 @@ static void mmc_omap_set_power(struct mmc_omap_slot *slot, int power_on,
 
 	host = slot->host;
 
-	if (slot->vsd)
-		gpiod_set_value(slot->vsd, power_on);
-	if (slot->vio)
-		gpiod_set_value(slot->vio, power_on);
+	if (power_on) {
+		if (slot->vsd) {
+			gpiod_set_value(slot->vsd, power_on);
+			msleep(1);
+		}
+		if (slot->vio) {
+			gpiod_set_value(slot->vio, power_on);
+			msleep(1);
+		}
+	} else {
+		if (slot->vio) {
+			gpiod_set_value(slot->vio, power_on);
+			msleep(50);
+		}
+		if (slot->vsd) {
+			gpiod_set_value(slot->vsd, power_on);
+			msleep(50);
+		}
+	}
 
 	if (slot->pdata->set_power != NULL)
 		slot->pdata->set_power(mmc_dev(slot->mmc), slot->id, power_on,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
                   ` (4 preceding siblings ...)
  2024-02-23 18:14 ` [PATCH 5/5] MMC: OMAP: restore original power up/down steps Aaro Koskinen
@ 2024-02-23 20:47 ` Linus Walleij
  2024-02-26 10:56   ` Tony Lindgren
  2024-02-29 15:06 ` Ulf Hansson
  6 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2024-02-23 20:47 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: Tony Lindgren, Ulf Hansson, linux-omap, linux-kernel

On Fri, Feb 23, 2024 at 7:15 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote:

> Nokia N8x0 MMC has been pretty much broken starting from v6.3. These
> patches restore the functionality. Tested on N810 with eMMC and external
> miniSD card, and on N800 with SD card in the inner slot.

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

100% my fault, I'm sorry I couldn't dry-code any better :(

If it's any consolation, there are now no GPIOs left for me to break
on the Nokia N8xx:s.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0
  2024-02-23 20:47 ` [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Linus Walleij
@ 2024-02-26 10:56   ` Tony Lindgren
  0 siblings, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2024-02-26 10:56 UTC (permalink / raw)
  To: Linus Walleij, Ulf Hansson; +Cc: Aaro Koskinen, linux-omap, linux-kernel

* Linus Walleij <linus.walleij@linaro.org> [240223 20:47]:
> On Fri, Feb 23, 2024 at 7:15 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> 
> > Nokia N8x0 MMC has been pretty much broken starting from v6.3. These
> > patches restore the functionality. Tested on N810 with eMMC and external
> > miniSD card, and on N800 with SD card in the inner slot.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> 100% my fault, I'm sorry I couldn't dry-code any better :(
> 
> If it's any consolation, there are now no GPIOs left for me to break
> on the Nokia N8xx:s.

That's the two-step conversion by introducing bugs first :)

Maybe Ulf can ack the mmc patches and I'll apply these all into a
fixes branch.

Regards,

Tony

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0
  2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
                   ` (5 preceding siblings ...)
  2024-02-23 20:47 ` [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Linus Walleij
@ 2024-02-29 15:06 ` Ulf Hansson
  6 siblings, 0 replies; 9+ messages in thread
From: Ulf Hansson @ 2024-02-29 15:06 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: Tony Lindgren, Linus Walleij, linux-omap, linux-kernel

On Fri, 23 Feb 2024 at 19:15, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
>
> Hi,
>
> Nokia N8x0 MMC has been pretty much broken starting from v6.3. These
> patches restore the functionality. Tested on N810 with eMMC and external
> miniSD card, and on N800 with SD card in the inner slot.
>
> A.
>
> Aaro Koskinen (5):
>   ARM: OMAP: fix bogus MMC GPIO labels on Nokia N8x0
>   ARM: OMAP: fix N810 MMC gpiod table
>   MMC: OMAP: fix broken slot switch lookup
>   MMC: OMAP: fix deferred probe
>   MMC: OMAP: restore original power up/down steps
>
>  arch/arm/mach-omap2/board-n8x0.c | 17 ++++++-----
>  drivers/mmc/host/omap.c          | 48 +++++++++++++++++++++-----------
>  2 files changed, 39 insertions(+), 26 deletions(-)
>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Nitpick: Tony, while applying I think it would be nice to change the
prefixes of the commit message headers for the mmc patches to "mmc:
omap:".

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-02-29 15:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-23 18:14 [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Aaro Koskinen
2024-02-23 18:14 ` [PATCH 1/5] ARM: OMAP: fix bogus MMC GPIO labels " Aaro Koskinen
2024-02-23 18:14 ` [PATCH 2/5] ARM: OMAP: fix N810 MMC gpiod table Aaro Koskinen
2024-02-23 18:14 ` [PATCH 3/5] MMC: OMAP: fix broken slot switch lookup Aaro Koskinen
2024-02-23 18:14 ` [PATCH 4/5] MMC: OMAP: fix deferred probe Aaro Koskinen
2024-02-23 18:14 ` [PATCH 5/5] MMC: OMAP: restore original power up/down steps Aaro Koskinen
2024-02-23 20:47 ` [PATCH 0/5] Fix MMC/GPIO regression on Nokia N8x0 Linus Walleij
2024-02-26 10:56   ` Tony Lindgren
2024-02-29 15:06 ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).