* [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors
[not found] <20181112141239.19646-1-linus.walleij@linaro.org>
@ 2018-11-12 14:12 ` Linus Walleij
2018-11-19 23:54 ` Paul Burton
2018-11-20 9:30 ` Ulf Hansson
2018-11-12 14:12 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power Linus Walleij
1 sibling, 2 replies; 8+ messages in thread
From: Linus Walleij @ 2018-11-12 14:12 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Linus Walleij, Paul Cercueil, linux-mips
Modifty the JZ4740 driver to retrieve card detect and write
protect GPIO pins from GPIO descriptors instead of hard-coded
global numbers. Augment the only board file using this in the
process and cut down on passed in platform data.
Preserve the code setting the caps2 flags for CD and WP
as active low or high since the slot GPIO code currently
ignores the gpiolib polarity inversion semantice and uses
the raw accessors to read the GPIO lines, but set the right
polarity flags in the descriptor table for jz4740.
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: linux-mips@linux-mips.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
.../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 --
arch/mips/jz4740/board-qi_lb60.c | 12 ++++++++---
drivers/mmc/host/jz4740_mmc.c | 20 +++++++++----------
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
index e9cc62cfac99..ff50aeb1a933 100644
--- a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
+++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
@@ -4,8 +4,6 @@
struct jz4740_mmc_platform_data {
int gpio_power;
- int gpio_card_detect;
- int gpio_read_only;
unsigned card_detect_active_low:1;
unsigned read_only_active_low:1;
unsigned power_active_low:1;
diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
index af0c8ace0141..705593d40d12 100644
--- a/arch/mips/jz4740/board-qi_lb60.c
+++ b/arch/mips/jz4740/board-qi_lb60.c
@@ -43,7 +43,6 @@
#include "clock.h"
/* GPIOs */
-#define QI_LB60_GPIO_SD_CD JZ_GPIO_PORTD(0)
#define QI_LB60_GPIO_SD_VCC_EN_N JZ_GPIO_PORTD(2)
#define QI_LB60_GPIO_KEYOUT(x) (JZ_GPIO_PORTC(10) + (x))
@@ -386,12 +385,18 @@ static struct platform_device qi_lb60_gpio_keys = {
};
static struct jz4740_mmc_platform_data qi_lb60_mmc_pdata = {
- .gpio_card_detect = QI_LB60_GPIO_SD_CD,
- .gpio_read_only = -1,
.gpio_power = QI_LB60_GPIO_SD_VCC_EN_N,
.power_active_low = 1,
};
+static struct gpiod_lookup_table qi_lb60_mmc_gpio_table = {
+ .dev_id = "jz4740-mmc.0",
+ .table = {
+ GPIO_LOOKUP("GPIOD", 0, "cd", GPIO_ACTIVE_HIGH),
+ { },
+ },
+};
+
/* beeper */
static struct pwm_lookup qi_lb60_pwm_lookup[] = {
PWM_LOOKUP("jz4740-pwm", 4, "pwm-beeper", NULL, 0,
@@ -500,6 +505,7 @@ static int __init qi_lb60_init_platform_devices(void)
gpiod_add_lookup_table(&qi_lb60_audio_gpio_table);
gpiod_add_lookup_table(&qi_lb60_nand_gpio_table);
gpiod_add_lookup_table(&qi_lb60_spigpio_gpio_table);
+ gpiod_add_lookup_table(&qi_lb60_mmc_gpio_table);
spi_register_board_info(qi_lb60_spi_board_info,
ARRAY_SIZE(qi_lb60_spi_board_info));
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 0c1efd5100b7..44ea452add8e 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -983,17 +983,17 @@ static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
if (!pdata->read_only_active_low)
mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
- if (gpio_is_valid(pdata->gpio_card_detect)) {
- ret = mmc_gpio_request_cd(mmc, pdata->gpio_card_detect, 0);
- if (ret)
- return ret;
- }
+ /*
+ * Get optional card detect and write protect GPIOs,
+ * only back out on probe deferral.
+ */
+ ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0, NULL);
+ if (ret == -EPROBE_DEFER)
+ return ret;
- if (gpio_is_valid(pdata->gpio_read_only)) {
- ret = mmc_gpio_request_ro(mmc, pdata->gpio_read_only);
- if (ret)
- return ret;
- }
+ ret = mmc_gpiod_request_ro(mmc, "wp", 0, false, 0, NULL);
+ if (ret == -EPROBE_DEFER)
+ return ret;
return jz4740_mmc_request_gpio(&pdev->dev, pdata->gpio_power,
"MMC read only", true, pdata->power_active_low);
--
2.17.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power
[not found] <20181112141239.19646-1-linus.walleij@linaro.org>
2018-11-12 14:12 ` [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors Linus Walleij
@ 2018-11-12 14:12 ` Linus Walleij
2018-11-19 23:55 ` Paul Burton
2018-11-20 9:30 ` Ulf Hansson
1 sibling, 2 replies; 8+ messages in thread
From: Linus Walleij @ 2018-11-12 14:12 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Linus Walleij, Paul Cercueil, linux-mips
The power GPIO line is passed with inversion flags and all from
the platform data. Switch to using an optional GPIO descriptor and
use this to switch the power.
Augment the only boardfile to pass in the proper "power" descriptor
in the GPIO descriptor machine table instead.
As the GPIO handling is now much simpler, we can cut down on some
overhead code.
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: linux-mips@linux-mips.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
.../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 -
arch/mips/jz4740/board-qi_lb60.c | 6 +-
drivers/mmc/host/jz4740_mmc.c | 65 +++++--------------
3 files changed, 18 insertions(+), 55 deletions(-)
diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
index ff50aeb1a933..9a7de47c7c79 100644
--- a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
+++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
@@ -3,10 +3,8 @@
#define __LINUX_MMC_JZ4740_MMC
struct jz4740_mmc_platform_data {
- int gpio_power;
unsigned card_detect_active_low:1;
unsigned read_only_active_low:1;
- unsigned power_active_low:1;
unsigned data_1bit:1;
};
diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
index 705593d40d12..6718efb400f4 100644
--- a/arch/mips/jz4740/board-qi_lb60.c
+++ b/arch/mips/jz4740/board-qi_lb60.c
@@ -43,8 +43,6 @@
#include "clock.h"
/* GPIOs */
-#define QI_LB60_GPIO_SD_VCC_EN_N JZ_GPIO_PORTD(2)
-
#define QI_LB60_GPIO_KEYOUT(x) (JZ_GPIO_PORTC(10) + (x))
#define QI_LB60_GPIO_KEYIN(x) (JZ_GPIO_PORTD(18) + (x))
#define QI_LB60_GPIO_KEYIN8 JZ_GPIO_PORTD(26)
@@ -385,14 +383,14 @@ static struct platform_device qi_lb60_gpio_keys = {
};
static struct jz4740_mmc_platform_data qi_lb60_mmc_pdata = {
- .gpio_power = QI_LB60_GPIO_SD_VCC_EN_N,
- .power_active_low = 1,
+ /* Intentionally left blank */
};
static struct gpiod_lookup_table qi_lb60_mmc_gpio_table = {
.dev_id = "jz4740-mmc.0",
.table = {
GPIO_LOOKUP("GPIOD", 0, "cd", GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP("GPIOD", 2, "power", GPIO_ACTIVE_LOW),
{ },
},
};
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 44ea452add8e..6f7a99e54af0 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -21,7 +21,7 @@
#include <linux/dmaengine.h>
#include <linux/dma-mapping.h>
#include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
@@ -136,6 +136,7 @@ struct jz4740_mmc_host {
struct platform_device *pdev;
struct jz4740_mmc_platform_data *pdata;
struct clk *clk;
+ struct gpio_desc *power;
enum jz4740_mmc_version version;
@@ -903,18 +904,16 @@ static void jz4740_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
case MMC_POWER_UP:
jz4740_mmc_reset(host);
- if (host->pdata && gpio_is_valid(host->pdata->gpio_power))
- gpio_set_value(host->pdata->gpio_power,
- !host->pdata->power_active_low);
+ if (host->power)
+ gpiod_set_value(host->power, 1);
host->cmdat |= JZ_MMC_CMDAT_INIT;
clk_prepare_enable(host->clk);
break;
case MMC_POWER_ON:
break;
default:
- if (host->pdata && gpio_is_valid(host->pdata->gpio_power))
- gpio_set_value(host->pdata->gpio_power,
- host->pdata->power_active_low);
+ if (host->power)
+ gpiod_set_value(host->power, 0);
clk_disable_unprepare(host->clk);
break;
}
@@ -947,30 +946,9 @@ static const struct mmc_host_ops jz4740_mmc_ops = {
.enable_sdio_irq = jz4740_mmc_enable_sdio_irq,
};
-static int jz4740_mmc_request_gpio(struct device *dev, int gpio,
- const char *name, bool output, int value)
-{
- int ret;
-
- if (!gpio_is_valid(gpio))
- return 0;
-
- ret = gpio_request(gpio, name);
- if (ret) {
- dev_err(dev, "Failed to request %s gpio: %d\n", name, ret);
- return ret;
- }
-
- if (output)
- gpio_direction_output(gpio, value);
- else
- gpio_direction_input(gpio);
-
- return 0;
-}
-
-static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
- struct platform_device *pdev)
+static int jz4740_mmc_request_gpios(struct jz4740_mmc_host *host,
+ struct mmc_host *mmc,
+ struct platform_device *pdev)
{
struct jz4740_mmc_platform_data *pdata = dev_get_platdata(&pdev->dev);
int ret = 0;
@@ -995,19 +973,12 @@ static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
if (ret == -EPROBE_DEFER)
return ret;
- return jz4740_mmc_request_gpio(&pdev->dev, pdata->gpio_power,
- "MMC read only", true, pdata->power_active_low);
-}
-
-static void jz4740_mmc_free_gpios(struct platform_device *pdev)
-{
- struct jz4740_mmc_platform_data *pdata = dev_get_platdata(&pdev->dev);
-
- if (!pdata)
- return;
+ host->power = devm_gpiod_get_optional(&pdev->dev, "power",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(host->power))
+ return PTR_ERR(host->power);
- if (gpio_is_valid(pdata->gpio_power))
- gpio_free(pdata->gpio_power);
+ return 0;
}
static const struct of_device_id jz4740_mmc_of_match[] = {
@@ -1053,7 +1024,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
mmc->caps |= MMC_CAP_SDIO_IRQ;
if (!(pdata && pdata->data_1bit))
mmc->caps |= MMC_CAP_4_BIT_DATA;
- ret = jz4740_mmc_request_gpios(mmc, pdev);
+ ret = jz4740_mmc_request_gpios(host, mmc, pdev);
if (ret)
goto err_free_host;
}
@@ -1104,7 +1075,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
dev_name(&pdev->dev), host);
if (ret) {
dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
- goto err_free_gpios;
+ goto err_free_host;
}
jz4740_mmc_clock_disable(host);
@@ -1135,8 +1106,6 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
jz4740_mmc_release_dma_channels(host);
err_free_irq:
free_irq(host->irq, host);
-err_free_gpios:
- jz4740_mmc_free_gpios(pdev);
err_free_host:
mmc_free_host(mmc);
@@ -1155,8 +1124,6 @@ static int jz4740_mmc_remove(struct platform_device *pdev)
free_irq(host->irq, host);
- jz4740_mmc_free_gpios(pdev);
-
if (host->use_dma)
jz4740_mmc_release_dma_channels(host);
--
2.17.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors
2018-11-12 14:12 ` [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors Linus Walleij
@ 2018-11-19 23:54 ` Paul Burton
2018-11-20 9:30 ` Ulf Hansson
1 sibling, 0 replies; 8+ messages in thread
From: Paul Burton @ 2018-11-19 23:54 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-mmc, Ulf Hansson, Paul Cercueil, linux-mips
Hi Linus,
On Mon, Nov 12, 2018 at 03:12:31PM +0100, Linus Walleij wrote:
> Modifty the JZ4740 driver to retrieve card detect and write
> protect GPIO pins from GPIO descriptors instead of hard-coded
> global numbers. Augment the only board file using this in the
> process and cut down on passed in platform data.
>
> Preserve the code setting the caps2 flags for CD and WP
> as active low or high since the slot GPIO code currently
> ignores the gpiolib polarity inversion semantice and uses
> the raw accessors to read the GPIO lines, but set the right
> polarity flags in the descriptor table for jz4740.
>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: linux-mips@linux-mips.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
> ---
> .../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 --
> arch/mips/jz4740/board-qi_lb60.c | 12 ++++++++---
> drivers/mmc/host/jz4740_mmc.c | 20 +++++++++----------
> 3 files changed, 19 insertions(+), 15 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power
2018-11-12 14:12 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power Linus Walleij
@ 2018-11-19 23:55 ` Paul Burton
2018-11-20 9:30 ` Ulf Hansson
1 sibling, 0 replies; 8+ messages in thread
From: Paul Burton @ 2018-11-19 23:55 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-mmc, Ulf Hansson, Paul Cercueil, linux-mips
Hi Linus,
On Mon, Nov 12, 2018 at 03:12:32PM +0100, Linus Walleij wrote:
> The power GPIO line is passed with inversion flags and all from
> the platform data. Switch to using an optional GPIO descriptor and
> use this to switch the power.
>
> Augment the only boardfile to pass in the proper "power" descriptor
> in the GPIO descriptor machine table instead.
>
> As the GPIO handling is now much simpler, we can cut down on some
> overhead code.
>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: linux-mips@linux-mips.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
> ---
> .../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 -
> arch/mips/jz4740/board-qi_lb60.c | 6 +-
> drivers/mmc/host/jz4740_mmc.c | 65 +++++--------------
> 3 files changed, 18 insertions(+), 55 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors
2018-11-12 14:12 ` [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors Linus Walleij
2018-11-19 23:54 ` Paul Burton
@ 2018-11-20 9:30 ` Ulf Hansson
1 sibling, 0 replies; 8+ messages in thread
From: Ulf Hansson @ 2018-11-20 9:30 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-mmc, Paul Cercueil, linux-mips
On 12 November 2018 at 15:12, Linus Walleij <linus.walleij@linaro.org> wrote:
> Modifty the JZ4740 driver to retrieve card detect and write
> protect GPIO pins from GPIO descriptors instead of hard-coded
> global numbers. Augment the only board file using this in the
> process and cut down on passed in platform data.
>
> Preserve the code setting the caps2 flags for CD and WP
> as active low or high since the slot GPIO code currently
> ignores the gpiolib polarity inversion semantice and uses
> the raw accessors to read the GPIO lines, but set the right
> polarity flags in the descriptor table for jz4740.
>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: linux-mips@linux-mips.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied for next, thanks!
Kind regards
Uffe
> ---
> .../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 --
> arch/mips/jz4740/board-qi_lb60.c | 12 ++++++++---
> drivers/mmc/host/jz4740_mmc.c | 20 +++++++++----------
> 3 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
> index e9cc62cfac99..ff50aeb1a933 100644
> --- a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
> +++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
> @@ -4,8 +4,6 @@
>
> struct jz4740_mmc_platform_data {
> int gpio_power;
> - int gpio_card_detect;
> - int gpio_read_only;
> unsigned card_detect_active_low:1;
> unsigned read_only_active_low:1;
> unsigned power_active_low:1;
> diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
> index af0c8ace0141..705593d40d12 100644
> --- a/arch/mips/jz4740/board-qi_lb60.c
> +++ b/arch/mips/jz4740/board-qi_lb60.c
> @@ -43,7 +43,6 @@
> #include "clock.h"
>
> /* GPIOs */
> -#define QI_LB60_GPIO_SD_CD JZ_GPIO_PORTD(0)
> #define QI_LB60_GPIO_SD_VCC_EN_N JZ_GPIO_PORTD(2)
>
> #define QI_LB60_GPIO_KEYOUT(x) (JZ_GPIO_PORTC(10) + (x))
> @@ -386,12 +385,18 @@ static struct platform_device qi_lb60_gpio_keys = {
> };
>
> static struct jz4740_mmc_platform_data qi_lb60_mmc_pdata = {
> - .gpio_card_detect = QI_LB60_GPIO_SD_CD,
> - .gpio_read_only = -1,
> .gpio_power = QI_LB60_GPIO_SD_VCC_EN_N,
> .power_active_low = 1,
> };
>
> +static struct gpiod_lookup_table qi_lb60_mmc_gpio_table = {
> + .dev_id = "jz4740-mmc.0",
> + .table = {
> + GPIO_LOOKUP("GPIOD", 0, "cd", GPIO_ACTIVE_HIGH),
> + { },
> + },
> +};
> +
> /* beeper */
> static struct pwm_lookup qi_lb60_pwm_lookup[] = {
> PWM_LOOKUP("jz4740-pwm", 4, "pwm-beeper", NULL, 0,
> @@ -500,6 +505,7 @@ static int __init qi_lb60_init_platform_devices(void)
> gpiod_add_lookup_table(&qi_lb60_audio_gpio_table);
> gpiod_add_lookup_table(&qi_lb60_nand_gpio_table);
> gpiod_add_lookup_table(&qi_lb60_spigpio_gpio_table);
> + gpiod_add_lookup_table(&qi_lb60_mmc_gpio_table);
>
> spi_register_board_info(qi_lb60_spi_board_info,
> ARRAY_SIZE(qi_lb60_spi_board_info));
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 0c1efd5100b7..44ea452add8e 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -983,17 +983,17 @@ static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
> if (!pdata->read_only_active_low)
> mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
>
> - if (gpio_is_valid(pdata->gpio_card_detect)) {
> - ret = mmc_gpio_request_cd(mmc, pdata->gpio_card_detect, 0);
> - if (ret)
> - return ret;
> - }
> + /*
> + * Get optional card detect and write protect GPIOs,
> + * only back out on probe deferral.
> + */
> + ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0, NULL);
> + if (ret == -EPROBE_DEFER)
> + return ret;
>
> - if (gpio_is_valid(pdata->gpio_read_only)) {
> - ret = mmc_gpio_request_ro(mmc, pdata->gpio_read_only);
> - if (ret)
> - return ret;
> - }
> + ret = mmc_gpiod_request_ro(mmc, "wp", 0, false, 0, NULL);
> + if (ret == -EPROBE_DEFER)
> + return ret;
>
> return jz4740_mmc_request_gpio(&pdev->dev, pdata->gpio_power,
> "MMC read only", true, pdata->power_active_low);
> --
> 2.17.2
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power
2018-11-12 14:12 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power Linus Walleij
2018-11-19 23:55 ` Paul Burton
@ 2018-11-20 9:30 ` Ulf Hansson
2018-11-22 16:26 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power, Paul Cercueil
1 sibling, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2018-11-20 9:30 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-mmc, Paul Cercueil, linux-mips
On 12 November 2018 at 15:12, Linus Walleij <linus.walleij@linaro.org> wrote:
> The power GPIO line is passed with inversion flags and all from
> the platform data. Switch to using an optional GPIO descriptor and
> use this to switch the power.
>
> Augment the only boardfile to pass in the proper "power" descriptor
> in the GPIO descriptor machine table instead.
>
> As the GPIO handling is now much simpler, we can cut down on some
> overhead code.
>
> Cc: Paul Cercueil <paul@crapouillou.net>
> Cc: linux-mips@linux-mips.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied for next, thanks!
Kind regards
Uffe
> ---
> .../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 -
> arch/mips/jz4740/board-qi_lb60.c | 6 +-
> drivers/mmc/host/jz4740_mmc.c | 65 +++++--------------
> 3 files changed, 18 insertions(+), 55 deletions(-)
>
> diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
> index ff50aeb1a933..9a7de47c7c79 100644
> --- a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
> +++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
> @@ -3,10 +3,8 @@
> #define __LINUX_MMC_JZ4740_MMC
>
> struct jz4740_mmc_platform_data {
> - int gpio_power;
> unsigned card_detect_active_low:1;
> unsigned read_only_active_low:1;
> - unsigned power_active_low:1;
>
> unsigned data_1bit:1;
> };
> diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
> index 705593d40d12..6718efb400f4 100644
> --- a/arch/mips/jz4740/board-qi_lb60.c
> +++ b/arch/mips/jz4740/board-qi_lb60.c
> @@ -43,8 +43,6 @@
> #include "clock.h"
>
> /* GPIOs */
> -#define QI_LB60_GPIO_SD_VCC_EN_N JZ_GPIO_PORTD(2)
> -
> #define QI_LB60_GPIO_KEYOUT(x) (JZ_GPIO_PORTC(10) + (x))
> #define QI_LB60_GPIO_KEYIN(x) (JZ_GPIO_PORTD(18) + (x))
> #define QI_LB60_GPIO_KEYIN8 JZ_GPIO_PORTD(26)
> @@ -385,14 +383,14 @@ static struct platform_device qi_lb60_gpio_keys = {
> };
>
> static struct jz4740_mmc_platform_data qi_lb60_mmc_pdata = {
> - .gpio_power = QI_LB60_GPIO_SD_VCC_EN_N,
> - .power_active_low = 1,
> + /* Intentionally left blank */
> };
>
> static struct gpiod_lookup_table qi_lb60_mmc_gpio_table = {
> .dev_id = "jz4740-mmc.0",
> .table = {
> GPIO_LOOKUP("GPIOD", 0, "cd", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("GPIOD", 2, "power", GPIO_ACTIVE_LOW),
> { },
> },
> };
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 44ea452add8e..6f7a99e54af0 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -21,7 +21,7 @@
> #include <linux/dmaengine.h>
> #include <linux/dma-mapping.h>
> #include <linux/err.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/irq.h>
> @@ -136,6 +136,7 @@ struct jz4740_mmc_host {
> struct platform_device *pdev;
> struct jz4740_mmc_platform_data *pdata;
> struct clk *clk;
> + struct gpio_desc *power;
>
> enum jz4740_mmc_version version;
>
> @@ -903,18 +904,16 @@ static void jz4740_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> switch (ios->power_mode) {
> case MMC_POWER_UP:
> jz4740_mmc_reset(host);
> - if (host->pdata && gpio_is_valid(host->pdata->gpio_power))
> - gpio_set_value(host->pdata->gpio_power,
> - !host->pdata->power_active_low);
> + if (host->power)
> + gpiod_set_value(host->power, 1);
> host->cmdat |= JZ_MMC_CMDAT_INIT;
> clk_prepare_enable(host->clk);
> break;
> case MMC_POWER_ON:
> break;
> default:
> - if (host->pdata && gpio_is_valid(host->pdata->gpio_power))
> - gpio_set_value(host->pdata->gpio_power,
> - host->pdata->power_active_low);
> + if (host->power)
> + gpiod_set_value(host->power, 0);
> clk_disable_unprepare(host->clk);
> break;
> }
> @@ -947,30 +946,9 @@ static const struct mmc_host_ops jz4740_mmc_ops = {
> .enable_sdio_irq = jz4740_mmc_enable_sdio_irq,
> };
>
> -static int jz4740_mmc_request_gpio(struct device *dev, int gpio,
> - const char *name, bool output, int value)
> -{
> - int ret;
> -
> - if (!gpio_is_valid(gpio))
> - return 0;
> -
> - ret = gpio_request(gpio, name);
> - if (ret) {
> - dev_err(dev, "Failed to request %s gpio: %d\n", name, ret);
> - return ret;
> - }
> -
> - if (output)
> - gpio_direction_output(gpio, value);
> - else
> - gpio_direction_input(gpio);
> -
> - return 0;
> -}
> -
> -static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
> - struct platform_device *pdev)
> +static int jz4740_mmc_request_gpios(struct jz4740_mmc_host *host,
> + struct mmc_host *mmc,
> + struct platform_device *pdev)
> {
> struct jz4740_mmc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> int ret = 0;
> @@ -995,19 +973,12 @@ static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
> if (ret == -EPROBE_DEFER)
> return ret;
>
> - return jz4740_mmc_request_gpio(&pdev->dev, pdata->gpio_power,
> - "MMC read only", true, pdata->power_active_low);
> -}
> -
> -static void jz4740_mmc_free_gpios(struct platform_device *pdev)
> -{
> - struct jz4740_mmc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> -
> - if (!pdata)
> - return;
> + host->power = devm_gpiod_get_optional(&pdev->dev, "power",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(host->power))
> + return PTR_ERR(host->power);
>
> - if (gpio_is_valid(pdata->gpio_power))
> - gpio_free(pdata->gpio_power);
> + return 0;
> }
>
> static const struct of_device_id jz4740_mmc_of_match[] = {
> @@ -1053,7 +1024,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
> mmc->caps |= MMC_CAP_SDIO_IRQ;
> if (!(pdata && pdata->data_1bit))
> mmc->caps |= MMC_CAP_4_BIT_DATA;
> - ret = jz4740_mmc_request_gpios(mmc, pdev);
> + ret = jz4740_mmc_request_gpios(host, mmc, pdev);
> if (ret)
> goto err_free_host;
> }
> @@ -1104,7 +1075,7 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
> dev_name(&pdev->dev), host);
> if (ret) {
> dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
> - goto err_free_gpios;
> + goto err_free_host;
> }
>
> jz4740_mmc_clock_disable(host);
> @@ -1135,8 +1106,6 @@ static int jz4740_mmc_probe(struct platform_device* pdev)
> jz4740_mmc_release_dma_channels(host);
> err_free_irq:
> free_irq(host->irq, host);
> -err_free_gpios:
> - jz4740_mmc_free_gpios(pdev);
> err_free_host:
> mmc_free_host(mmc);
>
> @@ -1155,8 +1124,6 @@ static int jz4740_mmc_remove(struct platform_device *pdev)
>
> free_irq(host->irq, host);
>
> - jz4740_mmc_free_gpios(pdev);
> -
> if (host->use_dma)
> jz4740_mmc_release_dma_channels(host);
>
> --
> 2.17.2
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power,
2018-11-20 9:30 ` Ulf Hansson
@ 2018-11-22 16:26 ` Paul Cercueil
2018-11-22 22:30 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power Linus Walleij
0 siblings, 1 reply; 8+ messages in thread
From: Paul Cercueil @ 2018-11-22 16:26 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Linus Walleij, linux-mmc, linux-mips
Le 2018-11-20 10:30, Ulf Hansson a écrit :
> On 12 November 2018 at 15:12, Linus Walleij <linus.walleij@linaro.org>
> wrote:
>> The power GPIO line is passed with inversion flags and all from
>> the platform data. Switch to using an optional GPIO descriptor and
>> use this to switch the power.
>>
>> Augment the only boardfile to pass in the proper "power" descriptor
>> in the GPIO descriptor machine table instead.
>>
>> As the GPIO handling is now much simpler, we can cut down on some
>> overhead code.
>>
>> Cc: Paul Cercueil <paul@crapouillou.net>
>> Cc: linux-mips@linux-mips.org
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> Applied for next, thanks!
>
> Kind regards
> Uffe
Sorry for being late to the party. I would have NACK'd this :(
I think the driver should just call mmc_regulator_get_supply() in the
probe function,
and then call mmc_regulator_set_ocr() to power ON or OFF the card. The
board file would
then provide the power regulator that triggers the GPIO line.
-Paul
>> ---
>> .../mips/include/asm/mach-jz4740/jz4740_mmc.h | 2 -
>> arch/mips/jz4740/board-qi_lb60.c | 6 +-
>> drivers/mmc/host/jz4740_mmc.c | 65
>> +++++--------------
>> 3 files changed, 18 insertions(+), 55 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
>> b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
>> index ff50aeb1a933..9a7de47c7c79 100644
>> --- a/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
>> +++ b/arch/mips/include/asm/mach-jz4740/jz4740_mmc.h
>> @@ -3,10 +3,8 @@
>> #define __LINUX_MMC_JZ4740_MMC
>>
>> struct jz4740_mmc_platform_data {
>> - int gpio_power;
>> unsigned card_detect_active_low:1;
>> unsigned read_only_active_low:1;
>> - unsigned power_active_low:1;
>>
>> unsigned data_1bit:1;
>> };
>> diff --git a/arch/mips/jz4740/board-qi_lb60.c
>> b/arch/mips/jz4740/board-qi_lb60.c
>> index 705593d40d12..6718efb400f4 100644
>> --- a/arch/mips/jz4740/board-qi_lb60.c
>> +++ b/arch/mips/jz4740/board-qi_lb60.c
>> @@ -43,8 +43,6 @@
>> #include "clock.h"
>>
>> /* GPIOs */
>> -#define QI_LB60_GPIO_SD_VCC_EN_N JZ_GPIO_PORTD(2)
>> -
>> #define QI_LB60_GPIO_KEYOUT(x) (JZ_GPIO_PORTC(10) + (x))
>> #define QI_LB60_GPIO_KEYIN(x) (JZ_GPIO_PORTD(18) + (x))
>> #define QI_LB60_GPIO_KEYIN8 JZ_GPIO_PORTD(26)
>> @@ -385,14 +383,14 @@ static struct platform_device qi_lb60_gpio_keys
>> = {
>> };
>>
>> static struct jz4740_mmc_platform_data qi_lb60_mmc_pdata = {
>> - .gpio_power = QI_LB60_GPIO_SD_VCC_EN_N,
>> - .power_active_low = 1,
>> + /* Intentionally left blank */
>> };
>>
>> static struct gpiod_lookup_table qi_lb60_mmc_gpio_table = {
>> .dev_id = "jz4740-mmc.0",
>> .table = {
>> GPIO_LOOKUP("GPIOD", 0, "cd", GPIO_ACTIVE_HIGH),
>> + GPIO_LOOKUP("GPIOD", 2, "power", GPIO_ACTIVE_LOW),
>> { },
>> },
>> };
>> diff --git a/drivers/mmc/host/jz4740_mmc.c
>> b/drivers/mmc/host/jz4740_mmc.c
>> index 44ea452add8e..6f7a99e54af0 100644
>> --- a/drivers/mmc/host/jz4740_mmc.c
>> +++ b/drivers/mmc/host/jz4740_mmc.c
>> @@ -21,7 +21,7 @@
>> #include <linux/dmaengine.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/err.h>
>> -#include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/irq.h>
>> @@ -136,6 +136,7 @@ struct jz4740_mmc_host {
>> struct platform_device *pdev;
>> struct jz4740_mmc_platform_data *pdata;
>> struct clk *clk;
>> + struct gpio_desc *power;
>>
>> enum jz4740_mmc_version version;
>>
>> @@ -903,18 +904,16 @@ static void jz4740_mmc_set_ios(struct mmc_host
>> *mmc, struct mmc_ios *ios)
>> switch (ios->power_mode) {
>> case MMC_POWER_UP:
>> jz4740_mmc_reset(host);
>> - if (host->pdata &&
>> gpio_is_valid(host->pdata->gpio_power))
>> - gpio_set_value(host->pdata->gpio_power,
>> -
>> !host->pdata->power_active_low);
>> + if (host->power)
>> + gpiod_set_value(host->power, 1);
>> host->cmdat |= JZ_MMC_CMDAT_INIT;
>> clk_prepare_enable(host->clk);
>> break;
>> case MMC_POWER_ON:
>> break;
>> default:
>> - if (host->pdata &&
>> gpio_is_valid(host->pdata->gpio_power))
>> - gpio_set_value(host->pdata->gpio_power,
>> -
>> host->pdata->power_active_low);
>> + if (host->power)
>> + gpiod_set_value(host->power, 0);
>> clk_disable_unprepare(host->clk);
>> break;
>> }
>> @@ -947,30 +946,9 @@ static const struct mmc_host_ops jz4740_mmc_ops =
>> {
>> .enable_sdio_irq = jz4740_mmc_enable_sdio_irq,
>> };
>>
>> -static int jz4740_mmc_request_gpio(struct device *dev, int gpio,
>> - const char *name, bool output, int value)
>> -{
>> - int ret;
>> -
>> - if (!gpio_is_valid(gpio))
>> - return 0;
>> -
>> - ret = gpio_request(gpio, name);
>> - if (ret) {
>> - dev_err(dev, "Failed to request %s gpio: %d\n", name,
>> ret);
>> - return ret;
>> - }
>> -
>> - if (output)
>> - gpio_direction_output(gpio, value);
>> - else
>> - gpio_direction_input(gpio);
>> -
>> - return 0;
>> -}
>> -
>> -static int jz4740_mmc_request_gpios(struct mmc_host *mmc,
>> - struct platform_device *pdev)
>> +static int jz4740_mmc_request_gpios(struct jz4740_mmc_host *host,
>> + struct mmc_host *mmc,
>> + struct platform_device *pdev)
>> {
>> struct jz4740_mmc_platform_data *pdata =
>> dev_get_platdata(&pdev->dev);
>> int ret = 0;
>> @@ -995,19 +973,12 @@ static int jz4740_mmc_request_gpios(struct
>> mmc_host *mmc,
>> if (ret == -EPROBE_DEFER)
>> return ret;
>>
>> - return jz4740_mmc_request_gpio(&pdev->dev, pdata->gpio_power,
>> - "MMC read only", true,
>> pdata->power_active_low);
>> -}
>> -
>> -static void jz4740_mmc_free_gpios(struct platform_device *pdev)
>> -{
>> - struct jz4740_mmc_platform_data *pdata =
>> dev_get_platdata(&pdev->dev);
>> -
>> - if (!pdata)
>> - return;
>> + host->power = devm_gpiod_get_optional(&pdev->dev, "power",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(host->power))
>> + return PTR_ERR(host->power);
>>
>> - if (gpio_is_valid(pdata->gpio_power))
>> - gpio_free(pdata->gpio_power);
>> + return 0;
>> }
>>
>> static const struct of_device_id jz4740_mmc_of_match[] = {
>> @@ -1053,7 +1024,7 @@ static int jz4740_mmc_probe(struct
>> platform_device* pdev)
>> mmc->caps |= MMC_CAP_SDIO_IRQ;
>> if (!(pdata && pdata->data_1bit))
>> mmc->caps |= MMC_CAP_4_BIT_DATA;
>> - ret = jz4740_mmc_request_gpios(mmc, pdev);
>> + ret = jz4740_mmc_request_gpios(host, mmc, pdev);
>> if (ret)
>> goto err_free_host;
>> }
>> @@ -1104,7 +1075,7 @@ static int jz4740_mmc_probe(struct
>> platform_device* pdev)
>> dev_name(&pdev->dev), host);
>> if (ret) {
>> dev_err(&pdev->dev, "Failed to request irq: %d\n",
>> ret);
>> - goto err_free_gpios;
>> + goto err_free_host;
>> }
>>
>> jz4740_mmc_clock_disable(host);
>> @@ -1135,8 +1106,6 @@ static int jz4740_mmc_probe(struct
>> platform_device* pdev)
>> jz4740_mmc_release_dma_channels(host);
>> err_free_irq:
>> free_irq(host->irq, host);
>> -err_free_gpios:
>> - jz4740_mmc_free_gpios(pdev);
>> err_free_host:
>> mmc_free_host(mmc);
>>
>> @@ -1155,8 +1124,6 @@ static int jz4740_mmc_remove(struct
>> platform_device *pdev)
>>
>> free_irq(host->irq, host);
>>
>> - jz4740_mmc_free_gpios(pdev);
>> -
>> if (host->use_dma)
>> jz4740_mmc_release_dma_channels(host);
>>
>> --
>> 2.17.2
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power
2018-11-22 16:26 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power, Paul Cercueil
@ 2018-11-22 22:30 ` Linus Walleij
0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2018-11-22 22:30 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Ulf Hansson, linux-mmc, Linux MIPS
On Thu, Nov 22, 2018 at 5:26 PM Paul Cercueil <paul@crapouillou.net> wrote:
> I think the driver should just call mmc_regulator_get_supply() in the
> probe function,
> and then call mmc_regulator_set_ocr() to power ON or OFF the card. The
> board file would
> then provide the power regulator that triggers the GPIO line.
One does not exclude the other, feel free (or anyone else)
to switch this to a fixed regulator. (Using descriptors!)
I feel insecure about cold-coding that without being able to
test it, I think I am on deep waters already. Someone with
the proper hardware should do radical semantic patches
like that. Introducing the regulator abstraction can lead
to probe deferrals and whatnot.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-22 22:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20181112141239.19646-1-linus.walleij@linaro.org>
2018-11-12 14:12 ` [PATCH 02/10] mmc: jz4740: Get CD/WP GPIOs from descriptors Linus Walleij
2018-11-19 23:54 ` Paul Burton
2018-11-20 9:30 ` Ulf Hansson
2018-11-12 14:12 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power Linus Walleij
2018-11-19 23:55 ` Paul Burton
2018-11-20 9:30 ` Ulf Hansson
2018-11-22 16:26 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power, Paul Cercueil
2018-11-22 22:30 ` [PATCH 03/10] mmc: jz4740: Use GPIO descriptor for power Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.