Hi Linus,
Thank you for the patch.
On Monday, 26 November 2018 00:52:08 EET Linus Walleij wrote:
> The TMIO MMC driver was passing global GPIO numbers around for
> card detect. It turns out only one single board in the kernel
> was actually making use of this feature so it is pretty easy
> to convert the driver to use only GPIO descriptors.
>
> The lines are flagged as GPIO_ACTIVE_LOW as that is what they
> are, but this will be ignored by the MMC slot GPIO code that
> uses the *raw accessors and rely on the caps2 flag for any
> inversion semantics.
>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> ChangeLog v1->v3:
> - Kuninori/Laurent: any tests/ACKs appreciated.
> ---
> arch/sh/boards/mach-ecovec24/setup.c | 26 ++++++++++++++++++++++----
> drivers/mmc/host/tmio_mmc_core.c | 12 +++++++-----
> include/linux/mfd/tmio.h | 9 ++-------
> 3 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c
> b/arch/sh/boards/mach-ecovec24/setup.c index 3097307b7cb7..af2c28946319
> 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -696,13 +696,20 @@ static struct gpiod_lookup_table
> sdhi0_power_gpiod_table = { },
> };
>
> +static struct gpiod_lookup_table sdhi0_gpio_table = {
> + .dev_id = "sh_mobile_sdhi.0",
> + .table = {
> + /* Card detect */
> + GPIO_LOOKUP("sh7724_pfc", GPIO_PTY7, "cd", GPIO_ACTIVE_LOW),
> + { },
> + },
> +};
> +
> static struct tmio_mmc_data sdhi0_info = {
> .chan_priv_tx = (void *)SHDMA_SLAVE_SDHI0_TX,
> .chan_priv_rx = (void *)SHDMA_SLAVE_SDHI0_RX,
> .capabilities = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
> MMC_CAP_NEEDS_POLL,
> - .flags = TMIO_MMC_USE_GPIO_CD,
> - .cd_gpio = GPIO_PTY7,
> };
>
> static struct resource sdhi0_resources[] = {
> @@ -735,8 +742,15 @@ static struct tmio_mmc_data sdhi1_info = {
> .chan_priv_rx = (void *)SHDMA_SLAVE_SDHI1_RX,
> .capabilities = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
> MMC_CAP_NEEDS_POLL,
> - .flags = TMIO_MMC_USE_GPIO_CD,
> - .cd_gpio = GPIO_PTW7,
> +};
> +
> +static struct gpiod_lookup_table sdhi1_gpio_table = {
> + .dev_id = "sh_mobile_sdhi.1",
> + .table = {
> + /* Card detect */
> + GPIO_LOOKUP("sh7724_pfc", GPIO_PTW7, "cd", GPIO_ACTIVE_LOW),
> + { },
> + },
> };
>
> static struct resource sdhi1_resources[] = {
> @@ -1445,6 +1459,10 @@ static int __init arch_setup(void)
> gpiod_add_lookup_table(&cn12_power_gpiod_table);
> #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)
> gpiod_add_lookup_table(&sdhi0_power_gpiod_table);
> + gpiod_add_lookup_table(&sdhi0_gpio_table);
> +#endif
> +#if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
> + gpiod_add_lookup_table(&sdhi1_gpio_table);
> #endif
>
> return platform_add_devices(ecovec_devices,
> diff --git a/drivers/mmc/host/tmio_mmc_core.c
> b/drivers/mmc/host/tmio_mmc_core.c index 8d64f6196f33..487b88dceff4 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1167,11 +1167,13 @@ 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;
> - }
> + /*
> + * Look for a card detect GPIO, if it fails with anything
> + * else than a probe deferral, just live without it.
> + */
> + ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0, NULL);
> + if (ret == -EPROBE_DEFER)
> + return ret;
Shouldn't you set the override_active_level argument to true in order for the
raw GPIO accessors to be used, as explained in the commit message ?
Apart from this the patch looks good to me,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
> mmc->caps2 |= pdata->capabilities2;
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 1e70060c92ce..e2687a30e5a1 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -54,12 +54,8 @@
> * idle before writing to some registers.
> */
> #define TMIO_MMC_HAS_IDLE_WAIT BIT(4)
> -/*
> - * A GPIO is used for card hotplug detection. We need an extra flag for
> this, - * because 0 is a valid GPIO number too, and requiring users to
> specify - * cd_gpio < 0 to disable GPIO hotplug would break backwards
> compatibility. - */
> -#define TMIO_MMC_USE_GPIO_CD BIT(5)
> +
> +/* BIT(5) is unused */
>
> /*
> * Some controllers have CMD12 automatically
> @@ -104,7 +100,6 @@ struct tmio_mmc_data {
> unsigned long capabilities2;
> unsigned long flags;
> u32 ocr_mask; /* available voltages */
> - unsigned int cd_gpio;
> int alignment_shift;
> dma_addr_t dma_rx_offset;
> unsigned int max_blk_count;
--
Regards,
Laurent Pinchart
