Hi Linus,
Thank you for the patch.
On Monday, 26 November 2018 00:52:07 EET Linus Walleij wrote:
> Switch the SPI MMC driver to use GPIO descriptors internally
> and just look those up using the standard slot GPIO
> functions mmc_gpiod_request_cd() and mmc_gpiod_request_ro().
> Make sure to request index 0 and 1 in accordance with the
> SPI MMC DT binding,
It would be nice to update the mmc-spi-slot bindings to use cd-gpios and wp-
gpios (of course keeping backward compatibility on the driver side).
> and add the same GPIOs in machine
> descriptor tables on all boards that use SPI MMC in
> board files.
>
> The lines are flagged as GPIO_ACTIVE_[LOW|HIGH] 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 flags MMC_CAP2_CD_ACTIVE_HIGH and
> MMC_CAP2_RO_ACTIVE_HIGH for polarity inversion semantics
> as of now.
>
> Cc: Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> # Vision EP9307
> 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:
> - Resending on top of the applied patches.
> - Hartley: it'd be great if you could test this series
> on the Vision EP9307.
> - Kuninori/Laurent: hope one of you has this Ecovec board
> so you can test it too.
> ---
> arch/arm/mach-ep93xx/simone.c | 14 +++++++++---
> arch/arm/mach-ep93xx/vision_ep9307.c | 17 ++++++++++----
> arch/sh/boards/mach-ecovec24/setup.c | 17 +++++++++++---
> drivers/mmc/host/mmc_spi.c | 27 ++++++++++++----------
> drivers/mmc/host/of_mmc_spi.c | 34 ----------------------------
> include/linux/spi/mmc_spi.h | 15 ------------
> 6 files changed, 53 insertions(+), 71 deletions(-)
>
> diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
> index 41aa57581356..80ccb984d521 100644
> --- a/arch/arm/mach-ep93xx/simone.c
> +++ b/arch/arm/mach-ep93xx/simone.c
> @@ -25,6 +25,7 @@
> #include <linux/platform_data/video-ep93xx.h>
> #include <linux/platform_data/spi-ep93xx.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
>
> #include <mach/hardware.h>
> #include <mach/gpio-ep93xx.h>
> @@ -45,9 +46,15 @@ static struct ep93xxfb_mach_info __initdata
> simone_fb_info = { static struct mmc_spi_platform_data simone_mmc_spi_data
> = {
> .detect_delay = 500,
> .ocr_mask = MMC_VDD_32_33 | MMC_VDD_33_34,
> - .flags = MMC_SPI_USE_CD_GPIO,
> - .cd_gpio = EP93XX_GPIO_LINE_EGPIO0,
> - .cd_debounce = 1,
> +};
> +
> +static struct gpiod_lookup_table simone_mmc_spi_gpio_table = {
> + .dev_id = "mmc_spi.0", /* "mmc_spi" @ CS0 */
> + .table = {
> + /* Card detect */
> + GPIO_LOOKUP_IDX("A", 0, NULL, 0, GPIO_ACTIVE_LOW),
> + { },
> + },
> };
>
> static struct spi_board_info simone_spi_devices[] __initdata = {
> @@ -105,6 +112,7 @@ static void __init simone_init_machine(void)
> ep93xx_register_fb(&simone_fb_info);
> ep93xx_register_i2c(simone_i2c_board_info,
> ARRAY_SIZE(simone_i2c_board_info));
> + gpiod_add_lookup_table(&simone_mmc_spi_gpio_table);
> ep93xx_register_spi(&simone_spi_info, simone_spi_devices,
> ARRAY_SIZE(simone_spi_devices));
> simone_register_audio();
> diff --git a/arch/arm/mach-ep93xx/vision_ep9307.c
> b/arch/arm/mach-ep93xx/vision_ep9307.c index 5a0b6187990a..767ee64628dc
> 100644
> --- a/arch/arm/mach-ep93xx/vision_ep9307.c
> +++ b/arch/arm/mach-ep93xx/vision_ep9307.c
> @@ -18,6 +18,7 @@
> #include <linux/platform_device.h>
> #include <linux/irq.h>
> #include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> #include <linux/fb.h>
> #include <linux/io.h>
> #include <linux/mtd/partitions.h>
> @@ -202,13 +203,20 @@ static struct mmc_spi_platform_data
> vision_spi_mmc_data = { .detect_delay = 100,
> .powerup_msecs = 100,
> .ocr_mask = MMC_VDD_32_33 | MMC_VDD_33_34,
> - .flags = MMC_SPI_USE_CD_GPIO | MMC_SPI_USE_RO_GPIO,
> - .cd_gpio = EP93XX_GPIO_LINE_EGPIO15,
> - .cd_debounce = 1,
> - .ro_gpio = EP93XX_GPIO_LINE_F(0),
> .caps2 = MMC_CAP2_RO_ACTIVE_HIGH,
> };
>
> +static struct gpiod_lookup_table vision_spi_mmc_gpio_table = {
> + .dev_id = "mmc_spi.2", /* "mmc_spi @ CS2 */
> + .table = {
> + /* Card detect */
> + GPIO_LOOKUP_IDX("B", 7, NULL, 0, GPIO_ACTIVE_LOW),
> + /* Write protect */
> + GPIO_LOOKUP_IDX("F", 0, NULL, 1, GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> /*************************************************************************
> * SPI Bus
> *************************************************************************/
> @@ -286,6 +294,7 @@ static void __init vision_init_machine(void)
>
> ep93xx_register_i2c(vision_i2c_info,
> ARRAY_SIZE(vision_i2c_info));
> + gpiod_add_lookup_table(&vision_spi_mmc_gpio_table);
> ep93xx_register_spi(&vision_spi_master, vision_spi_board_info,
> ARRAY_SIZE(vision_spi_board_info));
> vision_register_i2s();
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c
> b/arch/sh/boards/mach-ecovec24/setup.c index 06a894526a0b..3097307b7cb7
> 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -776,9 +776,19 @@ static struct mmc_spi_platform_data mmc_spi_info = {
> .caps2 = MMC_CAP2_RO_ACTIVE_HIGH,
> .ocr_mask = MMC_VDD_32_33 | MMC_VDD_33_34, /* 3.3V only */
> .setpower = mmc_spi_setpower,
> - .flags = MMC_SPI_USE_CD_GPIO | MMC_SPI_USE_RO_GPIO,
> - .cd_gpio = GPIO_PTY7,
> - .ro_gpio = GPIO_PTY6,
> +};
> +
> +static struct gpiod_lookup_table mmc_spi_gpio_table = {
> + .dev_id = "mmc_spi.0", /* device "mmc_spi" @ CS0 */
> + .table = {
> + /* Card detect */
> + GPIO_LOOKUP_IDX("sh7724_pfc", GPIO_PTY7, NULL, 0,
> + GPIO_ACTIVE_LOW),
> + /* Write protect */
> + GPIO_LOOKUP_IDX("sh7724_pfc", GPIO_PTY6, NULL, 1,
> + GPIO_ACTIVE_HIGH),
> + { },
> + },
> };
>
> static struct spi_board_info spi_bus[] = {
> @@ -1282,6 +1292,7 @@ static int __init arch_setup(void)
> gpio_request(GPIO_PTB6, NULL); /* 3.3V power control */
> gpio_direction_output(GPIO_PTB6, 0); /* disable power by default */
>
> + gpiod_add_lookup_table(&mmc_spi_gpio_table);
> spi_register_board_info(spi_bus, ARRAY_SIZE(spi_bus));
> #endif
>
> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
> index 476e53d30128..10ba46b728e8 100644
> --- a/drivers/mmc/host/mmc_spi.c
> +++ b/drivers/mmc/host/mmc_spi.c
> @@ -1434,13 +1434,16 @@ static int mmc_spi_probe(struct spi_device *spi)
> if (status != 0)
> goto fail_add_host;
>
> - if (host->pdata && host->pdata->flags & MMC_SPI_USE_CD_GPIO) {
> - status = mmc_gpio_request_cd(mmc, host->pdata->cd_gpio,
> - host->pdata->cd_debounce);
> - if (status != 0)
> - goto fail_add_host;
> -
> - /* The platform has a CD GPIO signal that may support
> + /*
> + * Index 0 is card detect
> + * Old boardfiles were specifying 1 ms as debounce
> + */
> + status = mmc_gpiod_request_cd(mmc, NULL, 0, false, 1, NULL);
As for patch 02/11, shouldn't the override argument be set to true ?
> + if (status == -EPROBE_DEFER)
> + goto fail_add_host;
> + if (!status) {
> + /*
> + * The platform has a CD GPIO signal that may support
> * interrupts, so let mmc_gpiod_request_cd_irq() decide
> * if polling is needed or not.
> */
> @@ -1448,12 +1451,12 @@ static int mmc_spi_probe(struct spi_device *spi)
> mmc_gpiod_request_cd_irq(mmc);
> }
>
> - if (host->pdata && host->pdata->flags & MMC_SPI_USE_RO_GPIO) {
> + /* Index 1 is write protect/read only */
> + status = mmc_gpiod_request_ro(mmc, NULL, 1, false, 0, NULL);
Same here.
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Please note that I don't have any of the boards above, so I haven't been able
to test the changes.
> + if (status == -EPROBE_DEFER)
> + goto fail_add_host;
> + if (!status)
> has_ro = true;
> - status = mmc_gpio_request_ro(mmc, host->pdata->ro_gpio);
> - if (status != 0)
> - goto fail_add_host;
> - }
>
> dev_info(&spi->dev, "SD/MMC host %s%s%s%s%s\n",
> dev_name(&mmc->class_dev),
> diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
> index c9eed8436b6b..b294b221f225 100644
> --- a/drivers/mmc/host/of_mmc_spi.c
> +++ b/drivers/mmc/host/of_mmc_spi.c
> @@ -16,9 +16,7 @@
> #include <linux/device.h>
> #include <linux/slab.h>
> #include <linux/irq.h>
> -#include <linux/gpio.h>
> #include <linux/of.h>
> -#include <linux/of_gpio.h>
> #include <linux/of_irq.h>
> #include <linux/spi/spi.h>
> #include <linux/spi/mmc_spi.h>
> @@ -32,15 +30,7 @@
>
> MODULE_LICENSE("GPL");
>
> -enum {
> - CD_GPIO = 0,
> - WP_GPIO,
> - NUM_GPIOS,
> -};
> -
> struct of_mmc_spi {
> - int gpios[NUM_GPIOS];
> - bool alow_gpios[NUM_GPIOS];
> int detect_irq;
> struct mmc_spi_platform_data pdata;
> };
> @@ -102,30 +92,6 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct
> spi_device *spi) oms->pdata.ocr_mask |= mask;
> }
>
> - for (i = 0; i < ARRAY_SIZE(oms->gpios); i++) {
> - enum of_gpio_flags gpio_flags;
> -
> - oms->gpios[i] = of_get_gpio_flags(np, i, &gpio_flags);
> - if (!gpio_is_valid(oms->gpios[i]))
> - continue;
> -
> - if (gpio_flags & OF_GPIO_ACTIVE_LOW)
> - oms->alow_gpios[i] = true;
> - }
> -
> - if (gpio_is_valid(oms->gpios[CD_GPIO])) {
> - oms->pdata.cd_gpio = oms->gpios[CD_GPIO];
> - oms->pdata.flags |= MMC_SPI_USE_CD_GPIO;
> - if (!oms->alow_gpios[CD_GPIO])
> - oms->pdata.caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> - }
> - if (gpio_is_valid(oms->gpios[WP_GPIO])) {
> - oms->pdata.ro_gpio = oms->gpios[WP_GPIO];
> - oms->pdata.flags |= MMC_SPI_USE_RO_GPIO;
> - if (!oms->alow_gpios[WP_GPIO])
> - oms->pdata.caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> - }
> -
> oms->detect_irq = irq_of_parse_and_map(np, 0);
> if (oms->detect_irq != 0) {
> oms->pdata.init = of_mmc_spi_init;
> diff --git a/include/linux/spi/mmc_spi.h b/include/linux/spi/mmc_spi.h
> index bfde741a543d..778ae8eb1f3e 100644
> --- a/include/linux/spi/mmc_spi.h
> +++ b/include/linux/spi/mmc_spi.h
> @@ -8,11 +8,6 @@
> struct device;
> struct mmc_host;
>
> -#define MMC_SPI_USE_CD_GPIO (1 << 0)
> -#define MMC_SPI_USE_RO_GPIO (1 << 1)
> -#define MMC_SPI_CD_GPIO_ACTIVE_LOW (1 << 2)
> -#define MMC_SPI_RO_GPIO_ACTIVE_LOW (1 << 3)
> -
> /* Put this in platform_data of a device being used to manage an MMC/SD
> * card slot. (Modeled after PXA mmc glue; see that for usage examples.)
> *
> @@ -27,16 +22,6 @@ struct mmc_spi_platform_data {
> void *);
> void (*exit)(struct device *, void *);
>
> - /*
> - * Card Detect and Read Only GPIOs. To enable debouncing on the card
> - * detect GPIO, set the cd_debounce to the debounce time in
> - * microseconds.
> - */
> - unsigned int flags;
> - unsigned int cd_gpio;
> - unsigned int cd_debounce;
> - unsigned int ro_gpio;
> -
> /* Capabilities to pass into mmc core (e.g. MMC_CAP_NEEDS_POLL). */
> unsigned long caps;
> unsigned long caps2;
--
Regards,
Laurent Pinchart
