| Message ID | 1394456564-13783-2-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Adrain. I have tested with your patch-set related slot-gpio. It's working fine. And looks good to me. I have a question. what's override_cd/ro_active_level? I didn't fully understand this value. Could you explain to me, plz? Best Regards, Jaehoon Chung On 03/10/2014 10:02 PM, Adrian Hunter wrote: > In preparation for adding a descriptor-based CD GPIO > API, switch from recording GPIO numbers to recording > GPIO descriptors. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------ > 1 file changed, 27 insertions(+), 18 deletions(-) > > diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c > index 46596b71..86547a2 100644 > --- a/drivers/mmc/core/slot-gpio.c > +++ b/drivers/mmc/core/slot-gpio.c > @@ -10,6 +10,7 @@ > > #include <linux/err.h> > #include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > #include <linux/interrupt.h> > #include <linux/jiffies.h> > #include <linux/mmc/host.h> > @@ -18,8 +19,10 @@ > #include <linux/slab.h> > > struct mmc_gpio { > - int ro_gpio; > - int cd_gpio; > + struct gpio_desc *ro_gpio; > + struct gpio_desc *cd_gpio; > + bool override_ro_active_level; > + bool override_cd_active_level; > char *ro_label; > char cd_label[0]; > }; > @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host) > ctx->ro_label = ctx->cd_label + len; > snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); > snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); > - ctx->cd_gpio = -EINVAL; > - ctx->ro_gpio = -EINVAL; > host->slot.handler_priv = ctx; > } > } > @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host) > { > struct mmc_gpio *ctx = host->slot.handler_priv; > > - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) > + if (!ctx || !ctx->ro_gpio) > return -ENOSYS; > > - return !gpio_get_value_cansleep(ctx->ro_gpio) ^ > - !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); > + if (ctx->override_ro_active_level) > + return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^ > + !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); > + > + return gpiod_get_value_cansleep(ctx->ro_gpio); > } > EXPORT_SYMBOL(mmc_gpio_get_ro); > > @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host) > { > struct mmc_gpio *ctx = host->slot.handler_priv; > > - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) > + if (!ctx || !ctx->cd_gpio) > return -ENOSYS; > > - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ > - !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); > + if (ctx->override_cd_active_level) > + return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^ > + !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); > + > + return gpiod_get_value_cansleep(ctx->cd_gpio); > } > EXPORT_SYMBOL(mmc_gpio_get_cd); > > @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio) > if (ret < 0) > return ret; > > - ctx->ro_gpio = gpio; > + ctx->override_ro_active_level = true; > + ctx->ro_gpio = gpio_to_desc(gpio); > > return 0; > } > @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio, > if (irq < 0) > host->caps |= MMC_CAP_NEEDS_POLL; > > - ctx->cd_gpio = gpio; > + ctx->override_cd_active_level = true; > + ctx->cd_gpio = gpio_to_desc(gpio); > > return 0; > } > @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host) > struct mmc_gpio *ctx = host->slot.handler_priv; > int gpio; > > - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) > + if (!ctx || !ctx->ro_gpio) > return; > > - gpio = ctx->ro_gpio; > - ctx->ro_gpio = -EINVAL; > + gpio = desc_to_gpio(ctx->ro_gpio); > + ctx->ro_gpio = NULL; > > devm_gpio_free(&host->class_dev, gpio); > } > @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host) > struct mmc_gpio *ctx = host->slot.handler_priv; > int gpio; > > - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) > + if (!ctx || !ctx->cd_gpio) > return; > > if (host->slot.cd_irq >= 0) { > @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host) > host->slot.cd_irq = -EINVAL; > } > > - gpio = ctx->cd_gpio; > - ctx->cd_gpio = -EINVAL; > + gpio = desc_to_gpio(ctx->cd_gpio); > + ctx->cd_gpio = NULL; > > devm_gpio_free(&host->class_dev, gpio); > } > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13.03.2014 15:00, Jaehoon Chung wrote: > Dear Adrain. > > I have tested with your patch-set related slot-gpio. > It's working fine. And looks good to me. > > I have a question. what's override_cd/ro_active_level? > I didn't fully understand this value. Could you explain to me, plz? The gpio descriptor records whether the gpio is active high or active low. When using gpiod_get_value_cansleep() the value is inverted on that basis. When using gpiod_get_raw_value_cansleep() the value is not inverted. The legacy GPIO API did not record active low/high. So then we use gpiod_get_raw_value_cansleep(). The new API should be able to use gpiod_get_value_cansleep(). In addition, for the device that I have the active high/low information passed by ACPI is wrong anyway, so I also have to use the override flag. > > Best Regards, > Jaehoon Chung > > On 03/10/2014 10:02 PM, Adrian Hunter wrote: >> In preparation for adding a descriptor-based CD GPIO >> API, switch from recording GPIO numbers to recording >> GPIO descriptors. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------ >> 1 file changed, 27 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c >> index 46596b71..86547a2 100644 >> --- a/drivers/mmc/core/slot-gpio.c >> +++ b/drivers/mmc/core/slot-gpio.c >> @@ -10,6 +10,7 @@ >> >> #include <linux/err.h> >> #include <linux/gpio.h> >> +#include <linux/gpio/consumer.h> >> #include <linux/interrupt.h> >> #include <linux/jiffies.h> >> #include <linux/mmc/host.h> >> @@ -18,8 +19,10 @@ >> #include <linux/slab.h> >> >> struct mmc_gpio { >> - int ro_gpio; >> - int cd_gpio; >> + struct gpio_desc *ro_gpio; >> + struct gpio_desc *cd_gpio; >> + bool override_ro_active_level; >> + bool override_cd_active_level; >> char *ro_label; >> char cd_label[0]; >> }; >> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host) >> ctx->ro_label = ctx->cd_label + len; >> snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); >> snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); >> - ctx->cd_gpio = -EINVAL; >> - ctx->ro_gpio = -EINVAL; >> host->slot.handler_priv = ctx; >> } >> } >> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host) >> { >> struct mmc_gpio *ctx = host->slot.handler_priv; >> >> - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) >> + if (!ctx || !ctx->ro_gpio) >> return -ENOSYS; >> >> - return !gpio_get_value_cansleep(ctx->ro_gpio) ^ >> - !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); >> + if (ctx->override_ro_active_level) >> + return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^ >> + !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); >> + >> + return gpiod_get_value_cansleep(ctx->ro_gpio); >> } >> EXPORT_SYMBOL(mmc_gpio_get_ro); >> >> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host) >> { >> struct mmc_gpio *ctx = host->slot.handler_priv; >> >> - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) >> + if (!ctx || !ctx->cd_gpio) >> return -ENOSYS; >> >> - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ >> - !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); >> + if (ctx->override_cd_active_level) >> + return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^ >> + !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); >> + >> + return gpiod_get_value_cansleep(ctx->cd_gpio); >> } >> EXPORT_SYMBOL(mmc_gpio_get_cd); >> >> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio) >> if (ret < 0) >> return ret; >> >> - ctx->ro_gpio = gpio; >> + ctx->override_ro_active_level = true; >> + ctx->ro_gpio = gpio_to_desc(gpio); >> >> return 0; >> } >> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio, >> if (irq < 0) >> host->caps |= MMC_CAP_NEEDS_POLL; >> >> - ctx->cd_gpio = gpio; >> + ctx->override_cd_active_level = true; >> + ctx->cd_gpio = gpio_to_desc(gpio); >> >> return 0; >> } >> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host) >> struct mmc_gpio *ctx = host->slot.handler_priv; >> int gpio; >> >> - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) >> + if (!ctx || !ctx->ro_gpio) >> return; >> >> - gpio = ctx->ro_gpio; >> - ctx->ro_gpio = -EINVAL; >> + gpio = desc_to_gpio(ctx->ro_gpio); >> + ctx->ro_gpio = NULL; >> >> devm_gpio_free(&host->class_dev, gpio); >> } >> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host) >> struct mmc_gpio *ctx = host->slot.handler_priv; >> int gpio; >> >> - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) >> + if (!ctx || !ctx->cd_gpio) >> return; >> >> if (host->slot.cd_irq >= 0) { >> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host) >> host->slot.cd_irq = -EINVAL; >> } >> >> - gpio = ctx->cd_gpio; >> - ctx->cd_gpio = -EINVAL; >> + gpio = desc_to_gpio(ctx->cd_gpio); >> + ctx->cd_gpio = NULL; >> >> devm_gpio_free(&host->class_dev, gpio); >> } >> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Adrian. Thanks for kindly explanation. Best Regards, Jaehoon Chung On 03/13/2014 10:33 PM, Adrian Hunter wrote: > On 13.03.2014 15:00, Jaehoon Chung wrote: >> Dear Adrain. >> >> I have tested with your patch-set related slot-gpio. >> It's working fine. And looks good to me. >> >> I have a question. what's override_cd/ro_active_level? >> I didn't fully understand this value. Could you explain to me, plz? > > The gpio descriptor records whether the gpio is active high or active low. > When using gpiod_get_value_cansleep() the value is inverted on that basis. > When using gpiod_get_raw_value_cansleep() the value is not inverted. > > The legacy GPIO API did not record active low/high. So then we use > gpiod_get_raw_value_cansleep(). The new API should be able to use > gpiod_get_value_cansleep(). > > In addition, for the device that I have the active high/low > information passed by ACPI is wrong anyway, so I also have to use the > override flag. > > >> >> Best Regards, >> Jaehoon Chung >> >> On 03/10/2014 10:02 PM, Adrian Hunter wrote: >>> In preparation for adding a descriptor-based CD GPIO >>> API, switch from recording GPIO numbers to recording >>> GPIO descriptors. >>> >>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >>> --- >>> drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------ >>> 1 file changed, 27 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c >>> index 46596b71..86547a2 100644 >>> --- a/drivers/mmc/core/slot-gpio.c >>> +++ b/drivers/mmc/core/slot-gpio.c >>> @@ -10,6 +10,7 @@ >>> >>> #include <linux/err.h> >>> #include <linux/gpio.h> >>> +#include <linux/gpio/consumer.h> >>> #include <linux/interrupt.h> >>> #include <linux/jiffies.h> >>> #include <linux/mmc/host.h> >>> @@ -18,8 +19,10 @@ >>> #include <linux/slab.h> >>> >>> struct mmc_gpio { >>> - int ro_gpio; >>> - int cd_gpio; >>> + struct gpio_desc *ro_gpio; >>> + struct gpio_desc *cd_gpio; >>> + bool override_ro_active_level; >>> + bool override_cd_active_level; >>> char *ro_label; >>> char cd_label[0]; >>> }; >>> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host) >>> ctx->ro_label = ctx->cd_label + len; >>> snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); >>> snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); >>> - ctx->cd_gpio = -EINVAL; >>> - ctx->ro_gpio = -EINVAL; >>> host->slot.handler_priv = ctx; >>> } >>> } >>> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host) >>> { >>> struct mmc_gpio *ctx = host->slot.handler_priv; >>> >>> - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) >>> + if (!ctx || !ctx->ro_gpio) >>> return -ENOSYS; >>> >>> - return !gpio_get_value_cansleep(ctx->ro_gpio) ^ >>> - !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); >>> + if (ctx->override_ro_active_level) >>> + return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^ >>> + !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); >>> + >>> + return gpiod_get_value_cansleep(ctx->ro_gpio); >>> } >>> EXPORT_SYMBOL(mmc_gpio_get_ro); >>> >>> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host) >>> { >>> struct mmc_gpio *ctx = host->slot.handler_priv; >>> >>> - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) >>> + if (!ctx || !ctx->cd_gpio) >>> return -ENOSYS; >>> >>> - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ >>> - !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); >>> + if (ctx->override_cd_active_level) >>> + return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^ >>> + !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); >>> + >>> + return gpiod_get_value_cansleep(ctx->cd_gpio); >>> } >>> EXPORT_SYMBOL(mmc_gpio_get_cd); >>> >>> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio) >>> if (ret < 0) >>> return ret; >>> >>> - ctx->ro_gpio = gpio; >>> + ctx->override_ro_active_level = true; >>> + ctx->ro_gpio = gpio_to_desc(gpio); >>> >>> return 0; >>> } >>> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio, >>> if (irq < 0) >>> host->caps |= MMC_CAP_NEEDS_POLL; >>> >>> - ctx->cd_gpio = gpio; >>> + ctx->override_cd_active_level = true; >>> + ctx->cd_gpio = gpio_to_desc(gpio); >>> >>> return 0; >>> } >>> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host) >>> struct mmc_gpio *ctx = host->slot.handler_priv; >>> int gpio; >>> >>> - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) >>> + if (!ctx || !ctx->ro_gpio) >>> return; >>> >>> - gpio = ctx->ro_gpio; >>> - ctx->ro_gpio = -EINVAL; >>> + gpio = desc_to_gpio(ctx->ro_gpio); >>> + ctx->ro_gpio = NULL; >>> >>> devm_gpio_free(&host->class_dev, gpio); >>> } >>> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host) >>> struct mmc_gpio *ctx = host->slot.handler_priv; >>> int gpio; >>> >>> - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) >>> + if (!ctx || !ctx->cd_gpio) >>> return; >>> >>> if (host->slot.cd_irq >= 0) { >>> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host) >>> host->slot.cd_irq = -EINVAL; >>> } >>> >>> - gpio = ctx->cd_gpio; >>> - ctx->cd_gpio = -EINVAL; >>> + gpio = desc_to_gpio(ctx->cd_gpio); >>> + ctx->cd_gpio = NULL; >>> >>> devm_gpio_free(&host->class_dev, gpio); >>> } >>> >> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
After applied "[PATCH 1/6]~[PATCH 3/6]", I have tested on exynos board. Tested-by: Jaehoon Chung <jh80.chung@samsung.com> On 03/13/2014 10:39 PM, Jaehoon Chung wrote: > Hi, Adrian. > > Thanks for kindly explanation. > > Best Regards, > Jaehoon Chung > > On 03/13/2014 10:33 PM, Adrian Hunter wrote: >> On 13.03.2014 15:00, Jaehoon Chung wrote: >>> Dear Adrain. >>> >>> I have tested with your patch-set related slot-gpio. >>> It's working fine. And looks good to me. >>> >>> I have a question. what's override_cd/ro_active_level? >>> I didn't fully understand this value. Could you explain to me, plz? >> >> The gpio descriptor records whether the gpio is active high or active low. >> When using gpiod_get_value_cansleep() the value is inverted on that basis. >> When using gpiod_get_raw_value_cansleep() the value is not inverted. >> >> The legacy GPIO API did not record active low/high. So then we use >> gpiod_get_raw_value_cansleep(). The new API should be able to use >> gpiod_get_value_cansleep(). >> >> In addition, for the device that I have the active high/low >> information passed by ACPI is wrong anyway, so I also have to use the >> override flag. >> >> >>> >>> Best Regards, >>> Jaehoon Chung >>> >>> On 03/10/2014 10:02 PM, Adrian Hunter wrote: >>>> In preparation for adding a descriptor-based CD GPIO >>>> API, switch from recording GPIO numbers to recording >>>> GPIO descriptors. >>>> >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >>>> --- >>>> drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------ >>>> 1 file changed, 27 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c >>>> index 46596b71..86547a2 100644 >>>> --- a/drivers/mmc/core/slot-gpio.c >>>> +++ b/drivers/mmc/core/slot-gpio.c >>>> @@ -10,6 +10,7 @@ >>>> >>>> #include <linux/err.h> >>>> #include <linux/gpio.h> >>>> +#include <linux/gpio/consumer.h> >>>> #include <linux/interrupt.h> >>>> #include <linux/jiffies.h> >>>> #include <linux/mmc/host.h> >>>> @@ -18,8 +19,10 @@ >>>> #include <linux/slab.h> >>>> >>>> struct mmc_gpio { >>>> - int ro_gpio; >>>> - int cd_gpio; >>>> + struct gpio_desc *ro_gpio; >>>> + struct gpio_desc *cd_gpio; >>>> + bool override_ro_active_level; >>>> + bool override_cd_active_level; >>>> char *ro_label; >>>> char cd_label[0]; >>>> }; >>>> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host) >>>> ctx->ro_label = ctx->cd_label + len; >>>> snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); >>>> snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); >>>> - ctx->cd_gpio = -EINVAL; >>>> - ctx->ro_gpio = -EINVAL; >>>> host->slot.handler_priv = ctx; >>>> } >>>> } >>>> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host) >>>> { >>>> struct mmc_gpio *ctx = host->slot.handler_priv; >>>> >>>> - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) >>>> + if (!ctx || !ctx->ro_gpio) >>>> return -ENOSYS; >>>> >>>> - return !gpio_get_value_cansleep(ctx->ro_gpio) ^ >>>> - !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); >>>> + if (ctx->override_ro_active_level) >>>> + return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^ >>>> + !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); >>>> + >>>> + return gpiod_get_value_cansleep(ctx->ro_gpio); >>>> } >>>> EXPORT_SYMBOL(mmc_gpio_get_ro); >>>> >>>> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host) >>>> { >>>> struct mmc_gpio *ctx = host->slot.handler_priv; >>>> >>>> - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) >>>> + if (!ctx || !ctx->cd_gpio) >>>> return -ENOSYS; >>>> >>>> - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ >>>> - !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); >>>> + if (ctx->override_cd_active_level) >>>> + return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^ >>>> + !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); >>>> + >>>> + return gpiod_get_value_cansleep(ctx->cd_gpio); >>>> } >>>> EXPORT_SYMBOL(mmc_gpio_get_cd); >>>> >>>> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio) >>>> if (ret < 0) >>>> return ret; >>>> >>>> - ctx->ro_gpio = gpio; >>>> + ctx->override_ro_active_level = true; >>>> + ctx->ro_gpio = gpio_to_desc(gpio); >>>> >>>> return 0; >>>> } >>>> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio, >>>> if (irq < 0) >>>> host->caps |= MMC_CAP_NEEDS_POLL; >>>> >>>> - ctx->cd_gpio = gpio; >>>> + ctx->override_cd_active_level = true; >>>> + ctx->cd_gpio = gpio_to_desc(gpio); >>>> >>>> return 0; >>>> } >>>> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host) >>>> struct mmc_gpio *ctx = host->slot.handler_priv; >>>> int gpio; >>>> >>>> - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) >>>> + if (!ctx || !ctx->ro_gpio) >>>> return; >>>> >>>> - gpio = ctx->ro_gpio; >>>> - ctx->ro_gpio = -EINVAL; >>>> + gpio = desc_to_gpio(ctx->ro_gpio); >>>> + ctx->ro_gpio = NULL; >>>> >>>> devm_gpio_free(&host->class_dev, gpio); >>>> } >>>> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host) >>>> struct mmc_gpio *ctx = host->slot.handler_priv; >>>> int gpio; >>>> >>>> - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) >>>> + if (!ctx || !ctx->cd_gpio) >>>> return; >>>> >>>> if (host->slot.cd_irq >= 0) { >>>> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host) >>>> host->slot.cd_irq = -EINVAL; >>>> } >>>> >>>> - gpio = ctx->cd_gpio; >>>> - ctx->cd_gpio = -EINVAL; >>>> + gpio = desc_to_gpio(ctx->cd_gpio); >>>> + ctx->cd_gpio = NULL; >>>> >>>> devm_gpio_free(&host->class_dev, gpio); >>>> } >>>> >>> >>> >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > In preparation for adding a descriptor-based CD GPIO > API, switch from recording GPIO numbers to recording > GPIO descriptors. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Certainly this makes things look better from a GPIO POV so Acked-by. However: (...) > + ctx->ro_gpio = gpio_to_desc(gpio); (...) > + ctx->cd_gpio = gpio_to_desc(gpio); How much harder would it be to switch the platforms using this MMC driver to pass GPIO descriptors using the gpiod_lookup_table in affected platforms (or device tree or ACPI) and then use devm_gpiod_get[_index]() in probe()? This can certainly be step 2, but that is where we want to go with the GPIO clean-up work in the long run. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14.03.2014 11:06, Linus Walleij wrote: > On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> In preparation for adding a descriptor-based CD GPIO >> API, switch from recording GPIO numbers to recording >> GPIO descriptors. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > Certainly this makes things look better from a GPIO POV > so Acked-by. > > However: > > (...) >> + ctx->ro_gpio = gpio_to_desc(gpio); > (...) >> + ctx->cd_gpio = gpio_to_desc(gpio); > > How much harder would it be to switch the platforms using this > MMC driver to pass GPIO descriptors using the gpiod_lookup_table > in affected platforms (or device tree or ACPI) and then use > devm_gpiod_get[_index]() in probe()? There are 10 drivers plus users of mmc_of_config(), so too much work for me right now. Also parts that relate to device-tree are outside my sphere. > This can certainly be step 2, but that is where we want to go with > the GPIO clean-up work in the long run. Perhaps Ulf or Guennadi would be interested. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Resent with updated email address for Ulf. On 14.03.2014 11:06, Linus Walleij wrote: > On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> In preparation for adding a descriptor-based CD GPIO >> API, switch from recording GPIO numbers to recording >> GPIO descriptors. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > Certainly this makes things look better from a GPIO POV > so Acked-by. > > However: > > (...) >> + ctx->ro_gpio = gpio_to_desc(gpio); > (...) >> + ctx->cd_gpio = gpio_to_desc(gpio); > > How much harder would it be to switch the platforms using this > MMC driver to pass GPIO descriptors using the gpiod_lookup_table > in affected platforms (or device tree or ACPI) and then use > devm_gpiod_get[_index]() in probe()? There are 10 drivers plus users of mmc_of_config(), so too much work for me right now. Also parts that relate to device-tree are outside my sphere. > This can certainly be step 2, but that is where we want to go with > the GPIO clean-up work in the long run. Perhaps Ulf or Guennadi would be interested. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c index 46596b71..86547a2 100644 --- a/drivers/mmc/core/slot-gpio.c +++ b/drivers/mmc/core/slot-gpio.c @@ -10,6 +10,7 @@ #include <linux/err.h> #include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/interrupt.h> #include <linux/jiffies.h> #include <linux/mmc/host.h> @@ -18,8 +19,10 @@ #include <linux/slab.h> struct mmc_gpio { - int ro_gpio; - int cd_gpio; + struct gpio_desc *ro_gpio; + struct gpio_desc *cd_gpio; + bool override_ro_active_level; + bool override_cd_active_level; char *ro_label; char cd_label[0]; }; @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host) ctx->ro_label = ctx->cd_label + len; snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); - ctx->cd_gpio = -EINVAL; - ctx->ro_gpio = -EINVAL; host->slot.handler_priv = ctx; } } @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host) { struct mmc_gpio *ctx = host->slot.handler_priv; - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) + if (!ctx || !ctx->ro_gpio) return -ENOSYS; - return !gpio_get_value_cansleep(ctx->ro_gpio) ^ - !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); + if (ctx->override_ro_active_level) + return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^ + !!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH); + + return gpiod_get_value_cansleep(ctx->ro_gpio); } EXPORT_SYMBOL(mmc_gpio_get_ro); @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host) { struct mmc_gpio *ctx = host->slot.handler_priv; - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) + if (!ctx || !ctx->cd_gpio) return -ENOSYS; - return !gpio_get_value_cansleep(ctx->cd_gpio) ^ - !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); + if (ctx->override_cd_active_level) + return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^ + !!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH); + + return gpiod_get_value_cansleep(ctx->cd_gpio); } EXPORT_SYMBOL(mmc_gpio_get_cd); @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio) if (ret < 0) return ret; - ctx->ro_gpio = gpio; + ctx->override_ro_active_level = true; + ctx->ro_gpio = gpio_to_desc(gpio); return 0; } @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio, if (irq < 0) host->caps |= MMC_CAP_NEEDS_POLL; - ctx->cd_gpio = gpio; + ctx->override_cd_active_level = true; + ctx->cd_gpio = gpio_to_desc(gpio); return 0; } @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host) struct mmc_gpio *ctx = host->slot.handler_priv; int gpio; - if (!ctx || !gpio_is_valid(ctx->ro_gpio)) + if (!ctx || !ctx->ro_gpio) return; - gpio = ctx->ro_gpio; - ctx->ro_gpio = -EINVAL; + gpio = desc_to_gpio(ctx->ro_gpio); + ctx->ro_gpio = NULL; devm_gpio_free(&host->class_dev, gpio); } @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host) struct mmc_gpio *ctx = host->slot.handler_priv; int gpio; - if (!ctx || !gpio_is_valid(ctx->cd_gpio)) + if (!ctx || !ctx->cd_gpio) return; if (host->slot.cd_irq >= 0) { @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host) host->slot.cd_irq = -EINVAL; } - gpio = ctx->cd_gpio; - ctx->cd_gpio = -EINVAL; + gpio = desc_to_gpio(ctx->cd_gpio); + ctx->cd_gpio = NULL; devm_gpio_free(&host->class_dev, gpio); }
In preparation for adding a descriptor-based CD GPIO API, switch from recording GPIO numbers to recording GPIO descriptors. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-)