2014-12-24 21:22:32

by NeilBrown

[permalink] [raw]
Subject: [PATCH 0/3] Convert omap_hsmmc to use common devicetree parsing code.

This is a resend against latest
git://git.linaro.org/people/ulf.hansson/mmc next

with requested changes.

My goal is to get omap_hsmmc to use the common code for parsing of,
particularly so that I can set "cap-power-off-card", which omap_hsmmc
doesn't explicitly report.

Thanks,
NeilBrown



---

NeilBrown (3):
mmc: core: Allow host driver to provide isr for card-detect interrupts.
mmc: omap_hsmmc: use slot-gpio library for gpio support.
mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration.


drivers/mmc/core/slot-gpio.c | 20 ++++++++
drivers/mmc/host/omap_hsmmc.c | 100 +++++++++--------------------------------
include/linux/mmc/slot-gpio.h | 2 +
3 files changed, 43 insertions(+), 79 deletions(-)

--
Signature


2014-12-24 21:22:41

by NeilBrown

[permalink] [raw]
Subject: [PATCH 1/3] mmc: core: Allow host driver to provide isr for card-detect interrupts.

One of the reasons omap_hsmmc doesn't use the slot-gpio library
is that it has some non-standard functionality in the card-detect
interrupt service routine.

To make it possible for omap_hsmmc (and maybe others) to be converted
to use slot-gpio, add 'mmc_gpio_set_cd_isr' which provides an
alternate isr to be registered by the slot-gpio code.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/core/slot-gpio.c | 20 +++++++++++++++++++-
include/linux/mmc/slot-gpio.h | 2 ++
2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 1a3edbd47719..1826b25a2a40 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -25,6 +25,7 @@ struct mmc_gpio {
struct gpio_desc *cd_gpio;
bool override_ro_active_level;
bool override_cd_active_level;
+ irqreturn_t (*cd_gpio_isr)(int irq, void *dev_id);
char *ro_label;
char cd_label[0];
};
@@ -136,8 +137,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
irq = -EINVAL;

if (irq >= 0) {
+ if (!ctx->cd_gpio_isr)
+ ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
ret = devm_request_threaded_irq(host->parent, irq,
- NULL, mmc_gpio_cd_irqt,
+ NULL, ctx->cd_gpio_isr,
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
ctx->cd_label, host);
if (ret < 0)
@@ -151,6 +154,21 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
}
EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);

+/* Register an alternate interrupt service routine for
+ * the card-detect GPIO.
+ */
+int mmc_gpio_set_cd_isr(struct mmc_host *host,
+ irqreturn_t (*isr)(int irq, void *dev_id))
+{
+ struct mmc_gpio *ctx;
+
+ ctx = host->slot.handler_priv;
+ if (ctx->cd_gpio_isr)
+ return -EBUSY;
+ ctx->cd_gpio_isr = isr;
+ return 0;
+}
+
/**
* mmc_gpio_request_cd - request a gpio for card-detection
* @host: mmc host
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index 4a36d6954631..4faf41c9a77a 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -26,6 +26,8 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
unsigned int idx, bool override_active_level,
unsigned int debounce, bool *gpio_invert);
+int mmc_gpio_set_cd_isr(struct mmc_host *host,
+ irqreturn_t (*isr)(int irq, void *dev_id));
void mmc_gpiod_request_cd_irq(struct mmc_host *host);

#endif

2014-12-24 21:22:55

by NeilBrown

[permalink] [raw]
Subject: [PATCH 3/3] mmc: omap_hsmmc: use mmc_of_parse to parse common mmc configuration.

This ensures that all standard options are available to hsmmc,
In particular, I need cap-power-off-card.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/host/omap_hsmmc.c | 33 ++++++++-------------------------
1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 149cc252fcbd..36cd73342afc 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1931,13 +1931,6 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
{
struct omap_hsmmc_platform_data *pdata;
struct device_node *np = dev->of_node;
- u32 bus_width, max_freq;
- int cd_gpio, wp_gpio;
-
- cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
- wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
- if (cd_gpio == -EPROBE_DEFER || wp_gpio == -EPROBE_DEFER)
- return ERR_PTR(-EPROBE_DEFER);

pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
@@ -1946,34 +1939,20 @@ static struct omap_hsmmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
if (of_find_property(np, "ti,dual-volt", NULL))
pdata->controller_flags |= OMAP_HSMMC_SUPPORTS_DUAL_VOLT;

- pdata->switch_pin = cd_gpio;
- pdata->gpio_wp = wp_gpio;
+ pdata->switch_pin = -EINVAL;
+ pdata->gpio_wp = -EINVAL;

if (of_find_property(np, "ti,non-removable", NULL)) {
pdata->nonremovable = true;
pdata->no_regulator_off_init = true;
}
- of_property_read_u32(np, "bus-width", &bus_width);
- if (bus_width == 4)
- pdata->caps |= MMC_CAP_4_BIT_DATA;
- else if (bus_width == 8)
- pdata->caps |= MMC_CAP_8_BIT_DATA;

if (of_find_property(np, "ti,needs-special-reset", NULL))
pdata->features |= HSMMC_HAS_UPDATED_RESET;

- if (!of_property_read_u32(np, "max-frequency", &max_freq))
- pdata->max_freq = max_freq;
-
if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
pdata->features |= HSMMC_HAS_HSPE_SUPPORT;

- if (of_find_property(np, "keep-power-in-suspend", NULL))
- pdata->pm_caps |= MMC_PM_KEEP_POWER;
-
- if (of_find_property(np, "enable-sdio-wakeup", NULL))
- pdata->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
-
return pdata;
}
#else
@@ -2031,6 +2010,10 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
goto err;
}

+ ret = mmc_of_parse(mmc);
+ if (ret)
+ goto err1;
+
host = mmc_priv(mmc);
host->mmc = mmc;
host->pdata = pdata;
@@ -2059,7 +2042,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)

if (pdata->max_freq > 0)
mmc->f_max = pdata->max_freq;
- else
+ else if (mmc->f_max == 0)
mmc->f_max = OMAP_MMC_MAX_CLOCK;

spin_lock_init(&host->irq_lock);
@@ -2113,7 +2096,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
if (mmc_pdata(host)->nonremovable)
mmc->caps |= MMC_CAP_NONREMOVABLE;

- mmc->pm_caps = mmc_pdata(host)->pm_caps;
+ mmc->pm_caps |= mmc_pdata(host)->pm_caps;

omap_hsmmc_conf_bus_power(host);


2014-12-24 21:22:48

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/3] mmc: omap_hsmmc: use slot-gpio library for gpio support.

Using the common code removes some code duplication, and
makes it easier to switch to using mmc_of_parse() which
will remove more duplication.

This uses the new mmc_gpio_set_cd_isr to provide a non-standard
interrupt service routine for card-detect interrupts.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/host/omap_hsmmc.c | 67 +++++++++--------------------------------
1 file changed, 14 insertions(+), 53 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 537cba8f1de1..149cc252fcbd 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -36,6 +36,7 @@
#include <linux/mmc/host.h>
#include <linux/mmc/core.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/slot-gpio.h>
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/gpio.h>
@@ -251,28 +252,22 @@ static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host);
static int omap_hsmmc_card_detect(struct device *dev)
{
struct omap_hsmmc_host *host = dev_get_drvdata(dev);
- struct omap_hsmmc_platform_data *mmc = host->pdata;

- /* NOTE: assumes card detect signal is active-low */
- return !gpio_get_value_cansleep(mmc->switch_pin);
+ return mmc_gpio_get_cd(host->mmc);
}

static int omap_hsmmc_get_wp(struct device *dev)
{
struct omap_hsmmc_host *host = dev_get_drvdata(dev);
- struct omap_hsmmc_platform_data *mmc = host->pdata;

- /* NOTE: assumes write protect signal is active-high */
- return gpio_get_value_cansleep(mmc->gpio_wp);
+ return mmc_gpio_get_ro(host->mmc);
}

static int omap_hsmmc_get_cover_state(struct device *dev)
{
struct omap_hsmmc_host *host = dev_get_drvdata(dev);
- struct omap_hsmmc_platform_data *mmc = host->pdata;

- /* NOTE: assumes card detect signal is active-low */
- return !gpio_get_value_cansleep(mmc->switch_pin);
+ return mmc_gpio_get_cd(host->mmc);
}

#ifdef CONFIG_REGULATOR
@@ -439,7 +434,10 @@ static inline int omap_hsmmc_have_reg(void)

#endif

-static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host,
+static irqreturn_t omap_hsmmc_detect(int irq, void *dev_id);
+
+static int omap_hsmmc_gpio_init(struct mmc_host *mmc,
+ struct omap_hsmmc_host *host,
struct omap_hsmmc_platform_data *pdata)
{
int ret;
@@ -452,46 +450,26 @@ static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host,
host->card_detect = omap_hsmmc_card_detect;
host->card_detect_irq =
gpio_to_irq(pdata->switch_pin);
- ret = gpio_request(pdata->switch_pin, "mmc_cd");
+ ret = mmc_gpio_set_cd_isr(mmc, omap_hsmmc_detect);
if (ret)
return ret;
- ret = gpio_direction_input(pdata->switch_pin);
+ ret = mmc_gpio_request_cd(mmc, pdata->switch_pin, 0);
if (ret)
- goto err_free_sp;
+ return ret;
} else {
pdata->switch_pin = -EINVAL;
}

if (gpio_is_valid(pdata->gpio_wp)) {
host->get_ro = omap_hsmmc_get_wp;
- ret = gpio_request(pdata->gpio_wp, "mmc_wp");
- if (ret)
- goto err_free_cd;
- ret = gpio_direction_input(pdata->gpio_wp);
+ ret = mmc_gpio_request_ro(mmc, pdata->gpio_wp);
if (ret)
- goto err_free_wp;
+ return ret;
} else {
pdata->gpio_wp = -EINVAL;
}

return 0;
-
-err_free_wp:
- gpio_free(pdata->gpio_wp);
-err_free_cd:
- if (gpio_is_valid(pdata->switch_pin))
-err_free_sp:
- gpio_free(pdata->switch_pin);
- return ret;
-}
-
-static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host,
- struct omap_hsmmc_platform_data *pdata)
-{
- if (gpio_is_valid(pdata->gpio_wp))
- gpio_free(pdata->gpio_wp);
- if (gpio_is_valid(pdata->switch_pin))
- gpio_free(pdata->switch_pin);
}

/*
@@ -2066,7 +2044,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
host->next_data.cookie = 1;
host->pbias_enabled = 0;

- ret = omap_hsmmc_gpio_init(host, pdata);
+ ret = omap_hsmmc_gpio_init(mmc, host, pdata);
if (ret)
goto err_gpio;

@@ -2197,20 +2175,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)

mmc->ocr_avail = mmc_pdata(host)->ocr_mask;

- /* Request IRQ for card detect */
- if (host->card_detect_irq) {
- ret = devm_request_threaded_irq(&pdev->dev,
- host->card_detect_irq,
- NULL, omap_hsmmc_detect,
- IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- mmc_hostname(mmc), host);
- if (ret) {
- dev_err(mmc_dev(host->mmc),
- "Unable to grab MMC CD IRQ\n");
- goto err_irq_cd;
- }
- }
-
omap_hsmmc_disable_irq(host);

/*
@@ -2249,7 +2213,6 @@ static int omap_hsmmc_probe(struct platform_device *pdev)

err_slot_name:
mmc_remove_host(mmc);
-err_irq_cd:
if (host->use_reg)
omap_hsmmc_reg_put(host);
err_irq:
@@ -2262,7 +2225,6 @@ err_irq:
if (host->dbclk)
clk_disable_unprepare(host->dbclk);
err1:
- omap_hsmmc_gpio_free(host, pdata);
err_gpio:
mmc_free_host(mmc);
err:
@@ -2288,7 +2250,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
if (host->dbclk)
clk_disable_unprepare(host->dbclk);

- omap_hsmmc_gpio_free(host, host->pdata);
mmc_free_host(host->mmc);

return 0;

2014-12-30 11:08:42

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 1/3] mmc: core: Allow host driver to provide isr for card-detect interrupts.

On 24 December 2014 at 22:20, NeilBrown <[email protected]> wrote:
> One of the reasons omap_hsmmc doesn't use the slot-gpio library
> is that it has some non-standard functionality in the card-detect
> interrupt service routine.
>
> To make it possible for omap_hsmmc (and maybe others) to be converted
> to use slot-gpio, add 'mmc_gpio_set_cd_isr' which provides an
> alternate isr to be registered by the slot-gpio code.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/mmc/core/slot-gpio.c | 20 +++++++++++++++++++-
> include/linux/mmc/slot-gpio.h | 2 ++
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 1a3edbd47719..1826b25a2a40 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -25,6 +25,7 @@ struct mmc_gpio {
> struct gpio_desc *cd_gpio;
> bool override_ro_active_level;
> bool override_cd_active_level;
> + irqreturn_t (*cd_gpio_isr)(int irq, void *dev_id);
> char *ro_label;
> char cd_label[0];
> };
> @@ -136,8 +137,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
> irq = -EINVAL;
>
> if (irq >= 0) {
> + if (!ctx->cd_gpio_isr)
> + ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
> ret = devm_request_threaded_irq(host->parent, irq,
> - NULL, mmc_gpio_cd_irqt,
> + NULL, ctx->cd_gpio_isr,
> IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> ctx->cd_label, host);
> if (ret < 0)
> @@ -151,6 +154,21 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
> }
> EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
>
> +/* Register an alternate interrupt service routine for
> + * the card-detect GPIO.
> + */
> +int mmc_gpio_set_cd_isr(struct mmc_host *host,
> + irqreturn_t (*isr)(int irq, void *dev_id))
> +{
> + struct mmc_gpio *ctx;
> +
> + ctx = host->slot.handler_priv;
> + if (ctx->cd_gpio_isr)
> + return -EBUSY;
> + ctx->cd_gpio_isr = isr;
> + return 0;

I feel this is a bit more complex than what's needed.
How about just assigning ->cd_gpio_isr() callback, without checking
it's value first and then make mmc_gpio_set_cd_isr() a void function?

> +}
> +
> /**
> * mmc_gpio_request_cd - request a gpio for card-detection
> * @host: mmc host
> diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
> index 4a36d6954631..4faf41c9a77a 100644
> --- a/include/linux/mmc/slot-gpio.h
> +++ b/include/linux/mmc/slot-gpio.h
> @@ -26,6 +26,8 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
> int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
> unsigned int idx, bool override_active_level,
> unsigned int debounce, bool *gpio_invert);
> +int mmc_gpio_set_cd_isr(struct mmc_host *host,
> + irqreturn_t (*isr)(int irq, void *dev_id));
> void mmc_gpiod_request_cd_irq(struct mmc_host *host);
>
> #endif
>
>