linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Introduce support for WP GPIO in the core SDHCI
@ 2019-01-15 16:28 Thomas Petazzoni
  2019-01-15 16:28 ` [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro() Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2019-01-15 16:28 UTC (permalink / raw)
  To: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Thierry Reding, Jonathan Hunter
  Cc: linux-mmc, linux-kernel, linux-tegra, Gregory Clement, Thomas Petazzoni

Hello,

While doing the bring up of a Zynq 7000 platform where the WP signal
of a SD slot is connected to a regular GPIO rather than through the
SDHCI WP pin, I realized that the GPIO described by wp-gpios was
properly requested, but it was in fact not used at all.

Indeed, the SDHCI core implements sdhci_check_ro() by:

 - Calling a controller-specific ->get_ro() callback if it exists. A
   few controller-specific drivers implement this, but not
   sdhci-of-arasan, which is used on Zynq 7000.

 - Using the SDHCI_PRESENT_STATE register, which reports the state of
   the SDHCI interface WP pin, and obvisouly not the state of a
   separate WP GPIO.

This patch series therefore changes sdhci_check_ro() to behave like
sdhci_get_cd(): use a GPIO first if available, and if not, fallback to
using the SDHCI_PRESENT_STATE register. Indeed, if there's a wp-gpios
described in the DT, it quite certainly indicates that the SDHCI WP
signal is not used, and the WP GPIO should be used instead.

As part of this series, two SDHCI drivers are modified to no longer
implement their custom ->get_ro() hook, since the core SDHCI now does
the right thing with the WP GPIO.

Best regards,

Thomas

Thomas Petazzoni (3):
  mmc: sdhci: use WP GPIO in sdhci_check_ro()
  mmc: sdhci-omap: drop ->get_ro() implementation
  mmc: sdhci-tegra: drop ->get_ro() implementation

 drivers/mmc/host/sdhci-omap.c  | 1 -
 drivers/mmc/host/sdhci-tegra.c | 9 ---------
 drivers/mmc/host/sdhci.c       | 9 ++++++---
 3 files changed, 6 insertions(+), 13 deletions(-)

-- 
2.20.1


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

* [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro()
  2019-01-15 16:28 [PATCH 0/3] Introduce support for WP GPIO in the core SDHCI Thomas Petazzoni
@ 2019-01-15 16:28 ` Thomas Petazzoni
  2019-01-16 12:32   ` Thierry Reding
  2019-01-16 12:59   ` Adrian Hunter
  2019-01-15 16:28 ` [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation Thomas Petazzoni
  2019-01-15 16:28 ` [PATCH 3/3] mmc: sdhci-tegra: " Thomas Petazzoni
  2 siblings, 2 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2019-01-15 16:28 UTC (permalink / raw)
  To: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Thierry Reding, Jonathan Hunter
  Cc: linux-mmc, linux-kernel, linux-tegra, Gregory Clement, Thomas Petazzoni

Even though SDHCI controllers may have a dedicated WP pin that can be
queried using the SDHCI_PRESENT_STATE register, some platforms may
chose to use a separate regular GPIO to route the WP signal. Such a
GPIO is typically represented using the wp-gpios property in the
Device Tree.

Unfortunately, the current sdhci_check_ro() function does not make use
of such GPIO when available: it either uses a host controller specific
->get_ro() operation, or uses the SDHCI_PRESENT_STATE. Several host
controller specific ->get_ro() functions are implemented just to use
check a WP GPIO state.

Instead of pushing this to more controller-specific implementations,
let's handle this in the core SDHCI code, just like it is already done
for the CD GPIO in sdhci_get_cd().

The below patch simply changes sdhci_check_ro() to use the value of
the WP GPIO if available. We need to adjust the prototype of the
function to use a mmc_host* as argument instead of sdhci_host*, since
the mmc_can_gpio_ro() and mmc_gpio_get_ro() helpers take a mmc_host*.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 drivers/mmc/host/sdhci.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index df05352b6a4a..63cc4bd033b9 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2022,8 +2022,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
 	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT);
 }
 
-static int sdhci_check_ro(struct sdhci_host *host)
+static int sdhci_check_ro(struct mmc_host *mmc)
 {
+	struct sdhci_host *host = mmc_priv(mmc);
 	unsigned long flags;
 	int is_readonly;
 
@@ -2031,6 +2032,8 @@ static int sdhci_check_ro(struct sdhci_host *host)
 
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		is_readonly = 0;
+	else if (mmc_can_gpio_ro(mmc))
+		is_readonly = mmc_gpio_get_ro(mmc);
 	else if (host->ops->get_ro)
 		is_readonly = host->ops->get_ro(host);
 	else
@@ -2052,11 +2055,11 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 	int i, ro_count;
 
 	if (!(host->quirks & SDHCI_QUIRK_UNSTABLE_RO_DETECT))
-		return sdhci_check_ro(host);
+		return sdhci_check_ro(mmc);
 
 	ro_count = 0;
 	for (i = 0; i < SAMPLE_COUNT; i++) {
-		if (sdhci_check_ro(host)) {
+		if (sdhci_check_ro(mmc)) {
 			if (++ro_count > SAMPLE_COUNT / 2)
 				return 1;
 		}
-- 
2.20.1


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

* [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation
  2019-01-15 16:28 [PATCH 0/3] Introduce support for WP GPIO in the core SDHCI Thomas Petazzoni
  2019-01-15 16:28 ` [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro() Thomas Petazzoni
@ 2019-01-15 16:28 ` Thomas Petazzoni
  2019-01-16 13:27   ` Thierry Reding
  2019-01-15 16:28 ` [PATCH 3/3] mmc: sdhci-tegra: " Thomas Petazzoni
  2 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2019-01-15 16:28 UTC (permalink / raw)
  To: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Thierry Reding, Jonathan Hunter
  Cc: linux-mmc, linux-kernel, linux-tegra, Gregory Clement, Thomas Petazzoni

The SDHCI core is know properly checking for the state of a WP GPIO,
so there is no longer any need for the sdhci-omap code to implement
->get_ro() using mmc_gpio_get_ro().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Note: this patch has only been compiled tested, as I don't have the
hardware to test it.
---
 drivers/mmc/host/sdhci-omap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index d264391616f9..c2a28930086f 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -987,7 +987,6 @@ static int sdhci_omap_probe(struct platform_device *pdev)
 		goto err_put_sync;
 	}
 
-	host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
 	host->mmc_host_ops.start_signal_voltage_switch =
 					sdhci_omap_start_signal_voltage_switch;
 	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;
-- 
2.20.1


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

* [PATCH 3/3] mmc: sdhci-tegra: drop ->get_ro() implementation
  2019-01-15 16:28 [PATCH 0/3] Introduce support for WP GPIO in the core SDHCI Thomas Petazzoni
  2019-01-15 16:28 ` [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro() Thomas Petazzoni
  2019-01-15 16:28 ` [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation Thomas Petazzoni
@ 2019-01-15 16:28 ` Thomas Petazzoni
  2019-01-16 13:22   ` Thierry Reding
  2 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2019-01-15 16:28 UTC (permalink / raw)
  To: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Thierry Reding, Jonathan Hunter
  Cc: linux-mmc, linux-kernel, linux-tegra, Gregory Clement, Thomas Petazzoni

The SDHCI core is know properly checking for the state of a WP GPIO,
so there is no longer any need for the sdhci-tegra code to implement
->get_ro() using mmc_gpio_get_ro().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Note: this patch has only been compiled tested, as I don't have the
hardware to test it.
---
 drivers/mmc/host/sdhci-tegra.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index e6ace31e2a41..6ed7323bf030 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -237,11 +237,6 @@ static void tegra210_sdhci_writew(struct sdhci_host *host, u16 val, int reg)
 	}
 }
 
-static unsigned int tegra_sdhci_get_ro(struct sdhci_host *host)
-{
-	return mmc_gpio_get_ro(host->mmc);
-}
-
 static bool tegra_sdhci_is_pad_and_regulator_valid(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -837,7 +832,6 @@ static void tegra_sdhci_voltage_switch(struct sdhci_host *host)
 }
 
 static const struct sdhci_ops tegra_sdhci_ops = {
-	.get_ro     = tegra_sdhci_get_ro,
 	.read_w     = tegra_sdhci_readw,
 	.write_l    = tegra_sdhci_writel,
 	.set_clock  = tegra_sdhci_set_clock,
@@ -893,7 +887,6 @@ static const struct sdhci_tegra_soc_data soc_data_tegra30 = {
 };
 
 static const struct sdhci_ops tegra114_sdhci_ops = {
-	.get_ro     = tegra_sdhci_get_ro,
 	.read_w     = tegra_sdhci_readw,
 	.write_w    = tegra_sdhci_writew,
 	.write_l    = tegra_sdhci_writel,
@@ -947,7 +940,6 @@ static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
 };
 
 static const struct sdhci_ops tegra210_sdhci_ops = {
-	.get_ro     = tegra_sdhci_get_ro,
 	.read_w     = tegra_sdhci_readw,
 	.write_w    = tegra210_sdhci_writew,
 	.write_l    = tegra_sdhci_writel,
@@ -980,7 +972,6 @@ static const struct sdhci_tegra_soc_data soc_data_tegra210 = {
 };
 
 static const struct sdhci_ops tegra186_sdhci_ops = {
-	.get_ro     = tegra_sdhci_get_ro,
 	.read_w     = tegra_sdhci_readw,
 	.write_l    = tegra_sdhci_writel,
 	.set_clock  = tegra_sdhci_set_clock,
-- 
2.20.1


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

* Re: [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro()
  2019-01-15 16:28 ` [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro() Thomas Petazzoni
@ 2019-01-16 12:32   ` Thierry Reding
  2019-01-16 12:59   ` Adrian Hunter
  1 sibling, 0 replies; 13+ messages in thread
From: Thierry Reding @ 2019-01-16 12:32 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Jonathan Hunter, linux-mmc, linux-kernel, linux-tegra,
	Gregory Clement

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

On Tue, Jan 15, 2019 at 05:28:35PM +0100, Thomas Petazzoni wrote:
> Even though SDHCI controllers may have a dedicated WP pin that can be
> queried using the SDHCI_PRESENT_STATE register, some platforms may
> chose to use a separate regular GPIO to route the WP signal. Such a
> GPIO is typically represented using the wp-gpios property in the
> Device Tree.
> 
> Unfortunately, the current sdhci_check_ro() function does not make use
> of such GPIO when available: it either uses a host controller specific
> ->get_ro() operation, or uses the SDHCI_PRESENT_STATE. Several host
> controller specific ->get_ro() functions are implemented just to use
> check a WP GPIO state.

"use check" was probably meant to be just "check"?

> 
> Instead of pushing this to more controller-specific implementations,
> let's handle this in the core SDHCI code, just like it is already done
> for the CD GPIO in sdhci_get_cd().
> 
> The below patch simply changes sdhci_check_ro() to use the value of
> the WP GPIO if available. We need to adjust the prototype of the
> function to use a mmc_host* as argument instead of sdhci_host*, since
> the mmc_can_gpio_ro() and mmc_gpio_get_ro() helpers take a mmc_host*.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  drivers/mmc/host/sdhci.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Other than the typo in the commit message, this looks fine:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro()
  2019-01-15 16:28 ` [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro() Thomas Petazzoni
  2019-01-16 12:32   ` Thierry Reding
@ 2019-01-16 12:59   ` Adrian Hunter
  2019-01-16 13:20     ` Thomas Petazzoni
  1 sibling, 1 reply; 13+ messages in thread
From: Adrian Hunter @ 2019-01-16 12:59 UTC (permalink / raw)
  To: Thomas Petazzoni, Kishon Vijay Abraham I, Ulf Hansson,
	Thierry Reding, Jonathan Hunter
  Cc: linux-mmc, linux-kernel, linux-tegra, Gregory Clement

On 15/01/19 6:28 PM, Thomas Petazzoni wrote:
> Even though SDHCI controllers may have a dedicated WP pin that can be
> queried using the SDHCI_PRESENT_STATE register, some platforms may
> chose to use a separate regular GPIO to route the WP signal. Such a
> GPIO is typically represented using the wp-gpios property in the
> Device Tree.
> 
> Unfortunately, the current sdhci_check_ro() function does not make use
> of such GPIO when available: it either uses a host controller specific
> ->get_ro() operation, or uses the SDHCI_PRESENT_STATE. Several host
> controller specific ->get_ro() functions are implemented just to use
> check a WP GPIO state.
> 
> Instead of pushing this to more controller-specific implementations,
> let's handle this in the core SDHCI code, just like it is already done
> for the CD GPIO in sdhci_get_cd().
> 
> The below patch simply changes sdhci_check_ro() to use the value of
> the WP GPIO if available. We need to adjust the prototype of the
> function to use a mmc_host* as argument instead of sdhci_host*, since
> the mmc_can_gpio_ro() and mmc_gpio_get_ro() helpers take a mmc_host*.

Why not just use host->mmc

> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  drivers/mmc/host/sdhci.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index df05352b6a4a..63cc4bd033b9 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2022,8 +2022,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
>  	return !!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT);
>  }
>  
> -static int sdhci_check_ro(struct sdhci_host *host)
> +static int sdhci_check_ro(struct mmc_host *mmc)
>  {
> +	struct sdhci_host *host = mmc_priv(mmc);
>  	unsigned long flags;
>  	int is_readonly;
>  
> @@ -2031,6 +2032,8 @@ static int sdhci_check_ro(struct sdhci_host *host)
>  
>  	if (host->flags & SDHCI_DEVICE_DEAD)
>  		is_readonly = 0;
> +	else if (mmc_can_gpio_ro(mmc))
> +		is_readonly = mmc_gpio_get_ro(mmc);

Perhaps host->ops->get_ro should be checked before mmc_can_gpio_ro()?

>  	else if (host->ops->get_ro)
>  		is_readonly = host->ops->get_ro(host);
>  	else
> @@ -2052,11 +2055,11 @@ static int sdhci_get_ro(struct mmc_host *mmc)
>  	int i, ro_count;
>  
>  	if (!(host->quirks & SDHCI_QUIRK_UNSTABLE_RO_DETECT))
> -		return sdhci_check_ro(host);
> +		return sdhci_check_ro(mmc);
>  
>  	ro_count = 0;
>  	for (i = 0; i < SAMPLE_COUNT; i++) {
> -		if (sdhci_check_ro(host)) {
> +		if (sdhci_check_ro(mmc)) {
>  			if (++ro_count > SAMPLE_COUNT / 2)
>  				return 1;
>  		}
> 


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

* Re: [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro()
  2019-01-16 12:59   ` Adrian Hunter
@ 2019-01-16 13:20     ` Thomas Petazzoni
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2019-01-16 13:20 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Kishon Vijay Abraham I, Ulf Hansson, Thierry Reding,
	Jonathan Hunter, linux-mmc, linux-kernel, linux-tegra,
	Gregory Clement

Hello Adrian,

On Wed, 16 Jan 2019 14:59:32 +0200, Adrian Hunter wrote:

> > The below patch simply changes sdhci_check_ro() to use the value of
> > the WP GPIO if available. We need to adjust the prototype of the
> > function to use a mmc_host* as argument instead of sdhci_host*, since
> > the mmc_can_gpio_ro() and mmc_gpio_get_ro() helpers take a mmc_host*.  
> 
> Why not just use host->mmc

Could do that. I just found it weird that the calling function has the
mmc_host structure, does some gymnastic to find sdhci_host, and then in
the called function, we do the opposite gymnastic to find mmc_host from
sdhci_host. But if that's the preference, I'm happy to change the patch
accordingly.


> >  	if (host->flags & SDHCI_DEVICE_DEAD)
> >  		is_readonly = 0;
> > +	else if (mmc_can_gpio_ro(mmc))
> > +		is_readonly = mmc_gpio_get_ro(mmc);  
> 
> Perhaps host->ops->get_ro should be checked before mmc_can_gpio_ro()?

That is actually a good point, using ->get_ro() should come before
using the GPIO. Indeed, some drivers may potentially have a ->get_ro
with custom logic *and* a WP GPIO, and in this case, we want ->get_ro
to take precedence. I'll send a v2 with this, once you let me know your
decision about the previous point.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



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

* Re: [PATCH 3/3] mmc: sdhci-tegra: drop ->get_ro() implementation
  2019-01-15 16:28 ` [PATCH 3/3] mmc: sdhci-tegra: " Thomas Petazzoni
@ 2019-01-16 13:22   ` Thierry Reding
  0 siblings, 0 replies; 13+ messages in thread
From: Thierry Reding @ 2019-01-16 13:22 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Jonathan Hunter, linux-mmc, linux-kernel, linux-tegra,
	Gregory Clement

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Tue, Jan 15, 2019 at 05:28:37PM +0100, Thomas Petazzoni wrote:
> The SDHCI core is know properly checking for the state of a WP GPIO,
> so there is no longer any need for the sdhci-tegra code to implement
> ->get_ro() using mmc_gpio_get_ro().
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> Note: this patch has only been compiled tested, as I don't have the
> hardware to test it.
> ---
>  drivers/mmc/host/sdhci-tegra.c | 9 ---------
>  1 file changed, 9 deletions(-)

So, if I apply this on top of next-20180116 I do get a BUG from
bio_split() on line 1833 in block/bio.c. However, that BUG also exists
without these patches.

More importantly, read-only detection with this applied still works on
for example Jetson TK1:

	[   28.547261] mmc1: new high speed SDHC card at address 59b4
	[   28.558814] mmcblk1: mmc1:59b4 SDC   14.7 GiB (ro)

So:

Tested-by: Thierry Reding <treding@nvidia.com>

and:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation
  2019-01-15 16:28 ` [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation Thomas Petazzoni
@ 2019-01-16 13:27   ` Thierry Reding
  2019-01-16 15:47     ` Tony Lindgren
  0 siblings, 1 reply; 13+ messages in thread
From: Thierry Reding @ 2019-01-16 13:27 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Adrian Hunter, Kishon Vijay Abraham I, Ulf Hansson,
	Jonathan Hunter, linux-mmc, linux-kernel, linux-tegra,
	Gregory Clement, Tony Lindgren, linux-omap

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

On Tue, Jan 15, 2019 at 05:28:36PM +0100, Thomas Petazzoni wrote:
> The SDHCI core is know properly checking for the state of a WP GPIO,

"know" -> "now"

> so there is no longer any need for the sdhci-omap code to implement
> ->get_ro() using mmc_gpio_get_ro().
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> Note: this patch has only been compiled tested, as I don't have the
> hardware to test it.
> ---
>  drivers/mmc/host/sdhci-omap.c | 1 -
>  1 file changed, 1 deletion(-)

I can't runtime test this myself, but it certainly looks like this would
do the right thing because it calls the core now call the very same
function as this used to, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>

Adding Tony and linux-omap for a slightly wider audience and hopefully
find someone to test this on. For reference, the whole series is here:

	https://patchwork.kernel.org/project/linux-mmc/list/?series=66905

Thierry

> 
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index d264391616f9..c2a28930086f 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -987,7 +987,6 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>  		goto err_put_sync;
>  	}
>  
> -	host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
>  	host->mmc_host_ops.start_signal_voltage_switch =
>  					sdhci_omap_start_signal_voltage_switch;
>  	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation
  2019-01-16 13:27   ` Thierry Reding
@ 2019-01-16 15:47     ` Tony Lindgren
  2019-01-16 15:58       ` Thomas Petazzoni
  2019-01-18 10:57       ` Faiz Abbas
  0 siblings, 2 replies; 13+ messages in thread
From: Tony Lindgren @ 2019-01-16 15:47 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Thomas Petazzoni, Adrian Hunter, Kishon Vijay Abraham I,
	Ulf Hansson, Jonathan Hunter, linux-mmc, linux-kernel,
	linux-tegra, Gregory Clement, linux-omap

* Thierry Reding <thierry.reding@gmail.com> [190116 13:28]:
> On Tue, Jan 15, 2019 at 05:28:36PM +0100, Thomas Petazzoni wrote:
> > The SDHCI core is know properly checking for the state of a WP GPIO,
> 
> "know" -> "now"
> 
> > so there is no longer any need for the sdhci-omap code to implement
> > ->get_ro() using mmc_gpio_get_ro().
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > ---
> > Note: this patch has only been compiled tested, as I don't have the
> > hardware to test it.
> > ---
> >  drivers/mmc/host/sdhci-omap.c | 1 -
> >  1 file changed, 1 deletion(-)
> 
> I can't runtime test this myself, but it certainly looks like this would
> do the right thing because it calls the core now call the very same
> function as this used to, so:
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> 
> Adding Tony and linux-omap for a slightly wider audience and hopefully
> find someone to test this on. For reference, the whole series is here:
> 
> 	https://patchwork.kernel.org/project/linux-mmc/list/?series=66905

Thanks Kishon can you please review and test?

Regards,

Tony

> > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> > index d264391616f9..c2a28930086f 100644
> > --- a/drivers/mmc/host/sdhci-omap.c
> > +++ b/drivers/mmc/host/sdhci-omap.c
> > @@ -987,7 +987,6 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> >  		goto err_put_sync;
> >  	}
> >  
> > -	host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
> >  	host->mmc_host_ops.start_signal_voltage_switch =
> >  					sdhci_omap_start_signal_voltage_switch;
> >  	host->mmc_host_ops.set_ios = sdhci_omap_set_ios;
> > -- 
> > 2.20.1
> > 



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

* Re: [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation
  2019-01-16 15:47     ` Tony Lindgren
@ 2019-01-16 15:58       ` Thomas Petazzoni
  2019-01-18 10:57       ` Faiz Abbas
  1 sibling, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2019-01-16 15:58 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Thierry Reding, Adrian Hunter, Kishon Vijay Abraham I,
	Ulf Hansson, Jonathan Hunter, linux-mmc, linux-kernel,
	linux-tegra, Gregory Clement, linux-omap

Hello,

On Wed, 16 Jan 2019 07:47:18 -0800, Tony Lindgren wrote:

> > Reviewed-by: Thierry Reding <treding@nvidia.com>
> > 
> > Adding Tony and linux-omap for a slightly wider audience and hopefully
> > find someone to test this on. For reference, the whole series is here:
> > 
> > 	https://patchwork.kernel.org/project/linux-mmc/list/?series=66905  
> 
> Thanks Kishon can you please review and test?

I think Adrian's comment that ->get_ro() should be called first in the
core code makes sense, so I'll send a v2 with this change. Perhaps
testing should be done on the v2 instead, even though I believe it
should be functionally equivalent.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

* Re: [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation
  2019-01-16 15:47     ` Tony Lindgren
  2019-01-16 15:58       ` Thomas Petazzoni
@ 2019-01-18 10:57       ` Faiz Abbas
  2019-01-18 15:32         ` Tony Lindgren
  1 sibling, 1 reply; 13+ messages in thread
From: Faiz Abbas @ 2019-01-18 10:57 UTC (permalink / raw)
  To: Tony Lindgren, Thierry Reding
  Cc: Thomas Petazzoni, Adrian Hunter, Kishon Vijay Abraham I,
	Ulf Hansson, Jonathan Hunter, linux-mmc, linux-kernel,
	linux-tegra, Gregory Clement, linux-omap

Tony,

On 16/01/19 9:17 PM, Tony Lindgren wrote:
> * Thierry Reding <thierry.reding@gmail.com> [190116 13:28]:
>> On Tue, Jan 15, 2019 at 05:28:36PM +0100, Thomas Petazzoni wrote:
>>> The SDHCI core is know properly checking for the state of a WP GPIO,
>>
>> "know" -> "now"
>>
>>> so there is no longer any need for the sdhci-omap code to implement
>>> ->get_ro() using mmc_gpio_get_ro().
>>>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>>> ---
>>> Note: this patch has only been compiled tested, as I don't have the
>>> hardware to test it.
>>> ---
>>>  drivers/mmc/host/sdhci-omap.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>
>> I can't runtime test this myself, but it certainly looks like this would
>> do the right thing because it calls the core now call the very same
>> function as this used to, so:
>>
>> Reviewed-by: Thierry Reding <treding@nvidia.com>
>>
>> Adding Tony and linux-omap for a slightly wider audience and hopefully
>> find someone to test this on. For reference, the whole series is here:
>>
>> 	https://patchwork.kernel.org/project/linux-mmc/list/?series=66905
> 
> Thanks Kishon can you please review and test?
> 

I can help with testing once Thomas posts a v2.

Thanks,
Faiz

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

* Re: [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation
  2019-01-18 10:57       ` Faiz Abbas
@ 2019-01-18 15:32         ` Tony Lindgren
  0 siblings, 0 replies; 13+ messages in thread
From: Tony Lindgren @ 2019-01-18 15:32 UTC (permalink / raw)
  To: Faiz Abbas
  Cc: Thierry Reding, Thomas Petazzoni, Adrian Hunter,
	Kishon Vijay Abraham I, Ulf Hansson, Jonathan Hunter, linux-mmc,
	linux-kernel, linux-tegra, Gregory Clement, linux-omap

* Faiz Abbas <faiz_abbas@ti.com> [190118 10:54]:
> Tony,
> 
> On 16/01/19 9:17 PM, Tony Lindgren wrote:
> > * Thierry Reding <thierry.reding@gmail.com> [190116 13:28]:
> >> On Tue, Jan 15, 2019 at 05:28:36PM +0100, Thomas Petazzoni wrote:
> >>> The SDHCI core is know properly checking for the state of a WP GPIO,
> >>
> >> "know" -> "now"
> >>
> >>> so there is no longer any need for the sdhci-omap code to implement
> >>> ->get_ro() using mmc_gpio_get_ro().
> >>>
> >>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> >>> ---
> >>> Note: this patch has only been compiled tested, as I don't have the
> >>> hardware to test it.
> >>> ---
> >>>  drivers/mmc/host/sdhci-omap.c | 1 -
> >>>  1 file changed, 1 deletion(-)
> >>
> >> I can't runtime test this myself, but it certainly looks like this would
> >> do the right thing because it calls the core now call the very same
> >> function as this used to, so:
> >>
> >> Reviewed-by: Thierry Reding <treding@nvidia.com>
> >>
> >> Adding Tony and linux-omap for a slightly wider audience and hopefully
> >> find someone to test this on. For reference, the whole series is here:
> >>
> >> 	https://patchwork.kernel.org/project/linux-mmc/list/?series=66905
> > 
> > Thanks Kishon can you please review and test?
> > 
> 
> I can help with testing once Thomas posts a v2.

OK great thanks!

Tony

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

end of thread, other threads:[~2019-01-18 15:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 16:28 [PATCH 0/3] Introduce support for WP GPIO in the core SDHCI Thomas Petazzoni
2019-01-15 16:28 ` [PATCH 1/3] mmc: sdhci: use WP GPIO in sdhci_check_ro() Thomas Petazzoni
2019-01-16 12:32   ` Thierry Reding
2019-01-16 12:59   ` Adrian Hunter
2019-01-16 13:20     ` Thomas Petazzoni
2019-01-15 16:28 ` [PATCH 2/3] mmc: sdhci-omap: drop ->get_ro() implementation Thomas Petazzoni
2019-01-16 13:27   ` Thierry Reding
2019-01-16 15:47     ` Tony Lindgren
2019-01-16 15:58       ` Thomas Petazzoni
2019-01-18 10:57       ` Faiz Abbas
2019-01-18 15:32         ` Tony Lindgren
2019-01-15 16:28 ` [PATCH 3/3] mmc: sdhci-tegra: " Thomas Petazzoni
2019-01-16 13:22   ` Thierry Reding

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).