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_request_cd_isr' which provide an
alternate isr to be register by the slot-gpio code.
Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/core/slot-gpio.c | 24 +++++++++++++++++++++++-
include/linux/mmc/slot-gpio.h | 2 ++
2 files changed, 25 insertions(+), 1 deletion(-)
This and following are the result of splitting the previous
'2/3' into to patches: core and omap_hsmmc as requested.
NeilBrown
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 69bbf2adb329..f56323f5a996 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -23,6 +23,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];
};
@@ -156,8 +157,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
irq = -EINVAL;
if (irq >= 0) {
+ if (ctx->cd_gpio_isr == NULL)
+ ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
ret = devm_request_threaded_irq(&host->class_dev, 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)
@@ -171,6 +174,25 @@ 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_request_cd_isr(struct mmc_host *host,
+ irqreturn_t (*isr)(int irq, void *dev_id))
+{
+ struct mmc_gpio *ctx;
+ int ret;
+
+ ret = mmc_gpio_alloc(host);
+ if (ret < 0)
+ return ret;
+ 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 e56fa24c9322..9e55db60deb0 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -28,6 +28,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_request_cd_isr(struct mmc_host *host,
+ irqreturn_t (*isr)(int irq, void *dev_id));
void mmc_gpiod_free_cd(struct mmc_host *host);
void mmc_gpiod_request_cd_irq(struct mmc_host *host);
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_request_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..32514b648e3c 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_request_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;
On 20 December 2014 at 00:07, 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_request_cd_isr' which provide an
> alternate isr to be register by the slot-gpio code.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/mmc/core/slot-gpio.c | 24 +++++++++++++++++++++++-
> include/linux/mmc/slot-gpio.h | 2 ++
> 2 files changed, 25 insertions(+), 1 deletion(-)
>
> This and following are the result of splitting the previous
> '2/3' into to patches: core and omap_hsmmc as requested.
>
> NeilBrown
>
>
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 69bbf2adb329..f56323f5a996 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -23,6 +23,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];
> };
> @@ -156,8 +157,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
> irq = -EINVAL;
>
> if (irq >= 0) {
> + if (ctx->cd_gpio_isr == NULL)
Please change to:
if (!ctx->cd_gpio_isr)
> + ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
> ret = devm_request_threaded_irq(&host->class_dev, 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)
> @@ -171,6 +174,25 @@ 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_request_cd_isr(struct mmc_host *host,
> + irqreturn_t (*isr)(int irq, void *dev_id))
> +{
> + struct mmc_gpio *ctx;
> + int ret;
> +
> + ret = mmc_gpio_alloc(host);
> + if (ret < 0)
> + return ret;
> + ctx = host->slot.handler_priv;
> + if (ctx->cd_gpio_isr)
> + return -EBUSY;
> + ctx->cd_gpio_isr = isr;
> + return 0;
> +}
I decided to queue those patchsets I recently posted which simplifies
the slot-gpio API. Please re-base this patch on top of my next branch.
Moreover, I actually wonder whether we need to add this API at all.
After my changes, all you need to do from your host driver ->probe(),
is to assign your isr routine to ctx->cd_gpio_isr.
> +
> /**
> * 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 e56fa24c9322..9e55db60deb0 100644
> --- a/include/linux/mmc/slot-gpio.h
> +++ b/include/linux/mmc/slot-gpio.h
> @@ -28,6 +28,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_request_cd_isr(struct mmc_host *host,
> + irqreturn_t (*isr)(int irq, void *dev_id));
> void mmc_gpiod_free_cd(struct mmc_host *host);
> void mmc_gpiod_request_cd_irq(struct mmc_host *host);
>
>
>
Kind regards
Uffe
On Mon, 22 Dec 2014 16:35:40 +0100 Ulf Hansson <[email protected]> wrote:
> On 20 December 2014 at 00:07, 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_request_cd_isr' which provide an
> > alternate isr to be register by the slot-gpio code.
> >
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > drivers/mmc/core/slot-gpio.c | 24 +++++++++++++++++++++++-
> > include/linux/mmc/slot-gpio.h | 2 ++
> > 2 files changed, 25 insertions(+), 1 deletion(-)
> >
> > This and following are the result of splitting the previous
> > '2/3' into to patches: core and omap_hsmmc as requested.
> >
> > NeilBrown
> >
> >
> > diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> > index 69bbf2adb329..f56323f5a996 100644
> > --- a/drivers/mmc/core/slot-gpio.c
> > +++ b/drivers/mmc/core/slot-gpio.c
> > @@ -23,6 +23,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];
> > };
> > @@ -156,8 +157,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
> > irq = -EINVAL;
> >
> > if (irq >= 0) {
> > + if (ctx->cd_gpio_isr == NULL)
>
> Please change to:
> if (!ctx->cd_gpio_isr)
will do (though I personally prefer the explicit "NULL" :-).
>
> > + ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
> > ret = devm_request_threaded_irq(&host->class_dev, 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)
> > @@ -171,6 +174,25 @@ 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_request_cd_isr(struct mmc_host *host,
> > + irqreturn_t (*isr)(int irq, void *dev_id))
> > +{
> > + struct mmc_gpio *ctx;
> > + int ret;
> > +
> > + ret = mmc_gpio_alloc(host);
> > + if (ret < 0)
> > + return ret;
> > + ctx = host->slot.handler_priv;
> > + if (ctx->cd_gpio_isr)
> > + return -EBUSY;
> > + ctx->cd_gpio_isr = isr;
> > + return 0;
> > +}
>
> I decided to queue those patchsets I recently posted which simplifies
> the slot-gpio API. Please re-base this patch on top of my next branch.
OK.
>
> Moreover, I actually wonder whether we need to add this API at all.
> After my changes, all you need to do from your host driver ->probe(),
> is to assign your isr routine to ctx->cd_gpio_isr.
'struct mmc_gpio' is local to slot-gpio.c, so code in other files cannot
access the members directly.
If you want to move it to include/linux/mmc/host.h and change
void *handler_priv;
to
struct mmc_gpio *gpios;
or similar, then I'll happily update it directly.
What do you think?
NeilBrown
>
> > +
> > /**
> > * 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 e56fa24c9322..9e55db60deb0 100644
> > --- a/include/linux/mmc/slot-gpio.h
> > +++ b/include/linux/mmc/slot-gpio.h
> > @@ -28,6 +28,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_request_cd_isr(struct mmc_host *host,
> > + irqreturn_t (*isr)(int irq, void *dev_id));
> > void mmc_gpiod_free_cd(struct mmc_host *host);
> > void mmc_gpiod_request_cd_irq(struct mmc_host *host);
> >
> >
> >
>
> Kind regards
> Uffe
On 23 December 2014 at 09:48, NeilBrown <[email protected]> wrote:
> On Mon, 22 Dec 2014 16:35:40 +0100 Ulf Hansson <[email protected]> wrote:
>
>> On 20 December 2014 at 00:07, 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_request_cd_isr' which provide an
>> > alternate isr to be register by the slot-gpio code.
>> >
>> > Signed-off-by: NeilBrown <[email protected]>
>> > ---
>> > drivers/mmc/core/slot-gpio.c | 24 +++++++++++++++++++++++-
>> > include/linux/mmc/slot-gpio.h | 2 ++
>> > 2 files changed, 25 insertions(+), 1 deletion(-)
>> >
>> > This and following are the result of splitting the previous
>> > '2/3' into to patches: core and omap_hsmmc as requested.
>> >
>> > NeilBrown
>> >
>> >
>> > diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
>> > index 69bbf2adb329..f56323f5a996 100644
>> > --- a/drivers/mmc/core/slot-gpio.c
>> > +++ b/drivers/mmc/core/slot-gpio.c
>> > @@ -23,6 +23,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];
>> > };
>> > @@ -156,8 +157,10 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
>> > irq = -EINVAL;
>> >
>> > if (irq >= 0) {
>> > + if (ctx->cd_gpio_isr == NULL)
>>
>> Please change to:
>> if (!ctx->cd_gpio_isr)
>
> will do (though I personally prefer the explicit "NULL" :-).
>
>>
>> > + ctx->cd_gpio_isr = mmc_gpio_cd_irqt;
>> > ret = devm_request_threaded_irq(&host->class_dev, 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)
>> > @@ -171,6 +174,25 @@ 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_request_cd_isr(struct mmc_host *host,
>> > + irqreturn_t (*isr)(int irq, void *dev_id))
>> > +{
>> > + struct mmc_gpio *ctx;
>> > + int ret;
>> > +
>> > + ret = mmc_gpio_alloc(host);
>> > + if (ret < 0)
>> > + return ret;
>> > + ctx = host->slot.handler_priv;
>> > + if (ctx->cd_gpio_isr)
>> > + return -EBUSY;
>> > + ctx->cd_gpio_isr = isr;
>> > + return 0;
>> > +}
>>
>> I decided to queue those patchsets I recently posted which simplifies
>> the slot-gpio API. Please re-base this patch on top of my next branch.
>
> OK.
>
>>
>> Moreover, I actually wonder whether we need to add this API at all.
>> After my changes, all you need to do from your host driver ->probe(),
>> is to assign your isr routine to ctx->cd_gpio_isr.
>
> 'struct mmc_gpio' is local to slot-gpio.c, so code in other files cannot
> access the members directly.
You are right. I don't think they should.
Let's add the API instead, but rename it to something like
mmc_gpio_set_cd_isr().
Kind regards
Uffe
>
> If you want to move it to include/linux/mmc/host.h and change
> void *handler_priv;
> to
> struct mmc_gpio *gpios;
>
> or similar, then I'll happily update it directly.
> What do you think?
>
> NeilBrown
>
>
>>
>> > +
>> > /**
>> > * 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 e56fa24c9322..9e55db60deb0 100644
>> > --- a/include/linux/mmc/slot-gpio.h
>> > +++ b/include/linux/mmc/slot-gpio.h
>> > @@ -28,6 +28,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_request_cd_isr(struct mmc_host *host,
>> > + irqreturn_t (*isr)(int irq, void *dev_id));
>> > void mmc_gpiod_free_cd(struct mmc_host *host);
>> > void mmc_gpiod_request_cd_irq(struct mmc_host *host);
>> >
>> >
>> >
>>
>> Kind regards
>> Uffe
>