[PATCH 0/3] mmc: tmio: fix card detection

[PATCH 0/3] mmc: tmio: fix card detection

[Masahiro Yamada]

Masahiro Yamada (3):
  mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
  mmc: tmio: use mmc_can_gpio_cd() instead of checking
    TMIO_MMC_USE_GPIO_CD
  mmc: tmio: support IP-builtin card detection logic

 drivers/mmc/host/tmio_mmc_core.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH 1/3] mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()

[Masahiro Yamada]

Drivers do not need to call mmc_gpiod_request_cd_irq() explicitly
because mmc_start_host() calls it.  To make it work, cd_gpio must
be set before mmc_add_host().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/host/tmio_mmc_core.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 2abc6bc..7c0e952 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1201,6 +1201,12 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (ret < 0)
 		return ret;
 
+	if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
+		ret = mmc_gpio_request_cd(mmc, pdata->cd_gpio, 0);
+		if (ret)
+			return ret;
+	}
+
 	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 |= pdata->capabilities2;
 	mmc->max_segs = pdata->max_segs ? : 32;
@@ -1296,14 +1302,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
-	if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
-		ret = mmc_gpio_request_cd(mmc, pdata->cd_gpio, 0);
-		if (ret)
-			goto remove_host;
-
-		mmc_gpiod_request_cd_irq(mmc);
-	}
-
 	return 0;
 
 remove_host:
-- 
2.7.4

[PATCH 2/3] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Masahiro Yamada]

To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is used
by a legacy board (arch/sh/boards/mach-ecovec24).

For DT platforms, the "cd-gpios" property is a legitimate way for that
in case the IP-builtin card detection can not be used for some reason.
mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
the "cd-gpios" property is specified.

To cater to both cases, mmc_can_gpio_cd() is a correct way to check
which card detection logic is used.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/host/tmio_mmc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 7c0e952..9fd0874 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1228,7 +1228,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	}
 	mmc->max_seg_size = mmc->max_req_size;
 
-	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
+	_host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
 				  !mmc_card_is_removable(mmc));
 
-- 
2.7.4

[PATCH 3/3] mmc: tmio: support IP-builtin card detection logic

[Masahiro Yamada]

A card detect GPIO is set up only for platforms with "cd-gpios"
DT property or TMIO_MMC_USE_GPIO_CD flag.  However, the driver
core always uses mmc_gpio_get_cd, which just fails with -ENOSYS
if ctx->cd_gpio is unset.

The bit 5 of the status register provides the current signal level
of the CD line.  Allow to use it if the GPIO is unused.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/host/tmio_mmc_core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 9fd0874..85a2944 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1087,6 +1087,19 @@ static int tmio_mmc_get_ro(struct mmc_host *mmc)
 	return ret;
 }
 
+static int tmio_mmc_get_cd(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	int ret;
+
+	ret = mmc_gpio_get_cd(mmc);
+	if (ret >= 0)
+		return ret;
+
+	return !!(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) &
+							TMIO_STAT_SIGSTATE);
+}
+
 static int tmio_multi_io_quirk(struct mmc_card *card,
 			       unsigned int direction, int blk_size)
 {
@@ -1102,7 +1115,7 @@ static const struct mmc_host_ops tmio_mmc_ops = {
 	.request	= tmio_mmc_request,
 	.set_ios	= tmio_mmc_set_ios,
 	.get_ro         = tmio_mmc_get_ro,
-	.get_cd		= mmc_gpio_get_cd,
+	.get_cd		= tmio_mmc_get_cd,
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 	.multi_io_quirk	= tmio_multi_io_quirk,
 	.hw_reset	= tmio_mmc_hw_reset,
-- 
2.7.4

Re: [PATCH 3/3] mmc: tmio: support IP-builtin card detection logic

[Wolfram Sang]

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

On Wed, Nov 08, 2017 at 07:17:00PM +0900, Masahiro Yamada wrote:
> A card detect GPIO is set up only for platforms with "cd-gpios"
> DT property or TMIO_MMC_USE_GPIO_CD flag.  However, the driver
> core always uses mmc_gpio_get_cd, which just fails with -ENOSYS
> if ctx->cd_gpio is unset.
> 
> The bit 5 of the status register provides the current signal level
> of the CD line.  Allow to use it if the GPIO is unused.

I might be missing something: Why do we need to report something
sensible in this case? The card insert/eject interrupts calling
mmc_detect_change is not enough?


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

Re: [PATCH 2/3] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Wolfram Sang]

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

On Wed, Nov 08, 2017 at 07:16:59PM +0900, Masahiro Yamada wrote:
> To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is used
> by a legacy board (arch/sh/boards/mach-ecovec24).
> 
> For DT platforms, the "cd-gpios" property is a legitimate way for that
> in case the IP-builtin card detection can not be used for some reason.
> mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
> the "cd-gpios" property is specified.

So, it fixed the DT not recognized via TMIO_MMC_USE_GPIO_CD? Did I get
this right?

> 
> To cater to both cases, mmc_can_gpio_cd() is a correct way to check
> which card detection logic is used.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/mmc/host/tmio_mmc_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 7c0e952..9fd0874 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1228,7 +1228,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>  	}
>  	mmc->max_seg_size = mmc->max_req_size;
>  
> -	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
> +	_host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
>  				  mmc->caps & MMC_CAP_NEEDS_POLL ||
>  				  !mmc_card_is_removable(mmc));
>  
> -- 
> 2.7.4
> 

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

Re: [PATCH 3/3] mmc: tmio: support IP-builtin card detection logic

[Masahiro Yamada]

2017-11-20 4:45 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
> On Wed, Nov 08, 2017 at 07:17:00PM +0900, Masahiro Yamada wrote:
>> A card detect GPIO is set up only for platforms with "cd-gpios"
>> DT property or TMIO_MMC_USE_GPIO_CD flag.  However, the driver
>> core always uses mmc_gpio_get_cd, which just fails with -ENOSYS
>> if ctx->cd_gpio is unset.
>>
>> The bit 5 of the status register provides the current signal level
>> of the CD line.  Allow to use it if the GPIO is unused.
>
> I might be missing something: Why do we need to report something
> sensible in this case? The card insert/eject interrupts calling
> mmc_detect_change is not enough?
>

Not enough.

mmc_detect_change() just schedules mmc_rescan().

Without correct ->get_cd(), how can mmc_rescan()
know whether a card is inserted or not?




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Masahiro Yamada]

2017-11-20 4:47 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
> On Wed, Nov 08, 2017 at 07:16:59PM +0900, Masahiro Yamada wrote:
>> To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is used
>> by a legacy board (arch/sh/boards/mach-ecovec24).
>>
>> For DT platforms, the "cd-gpios" property is a legitimate way for that
>> in case the IP-builtin card detection can not be used for some reason.
>> mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
>> the "cd-gpios" property is specified.
>
> So, it fixed the DT not recognized via TMIO_MMC_USE_GPIO_CD? Did I get
> this right?
>
>>
>> To cater to both cases, mmc_can_gpio_cd() is a correct way to check
>> which card detection logic is used.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  drivers/mmc/host/tmio_mmc_core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
>> index 7c0e952..9fd0874 100644
>> --- a/drivers/mmc/host/tmio_mmc_core.c
>> +++ b/drivers/mmc/host/tmio_mmc_core.c
>> @@ -1228,7 +1228,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>>       }
>>       mmc->max_seg_size = mmc->max_req_size;
>>
>> -     _host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
>> +     _host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
>>                                 mmc->caps & MMC_CAP_NEEDS_POLL ||
>>                                 !mmc_card_is_removable(mmc));
>>

I do not think I fully understand
how TMIO / Renesas platforms detect the card removal/insertion.

I see TMIO_MMC_USE_GPIO_CD only in
arch/sh/boards/mach-ecovec24/setup.c


As far as I understood, ->native_hotplug flag
decides the interrupt (TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT).

If you use GPIO, enabling such IRQs is pointless.


However, currently only GPIO is supported for CD
as I addressed:
https://patchwork.kernel.org/patch/10048239/


I am puzzled by this driver.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Wolfram Sang]

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


> I am puzzled by this driver.

And now imagine that it is already cleaned up :)

I will check these CD patches tomorrow. I need a fresh brain for that.

Thanks for the work!


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