diff mbox

mmc: core: Add DT bindings for card detect debounce time

Message ID 1399649837-27286-1-git-send-email-balajitk@ti.com
State Superseded, archived
Headers show

Commit Message

balajitk@ti.com May 9, 2014, 3:37 p.m. UTC
Provide an option to get CD debounce time from DT

Signed-off-by: Balaji T K <balajitk@ti.com>
---
 Documentation/devicetree/bindings/mmc/mmc.txt |    1 +
 drivers/mmc/core/host.c                       |    6 +++++-
 2 files changed, 6 insertions(+), 1 deletions(-)

Comments

balajitk@ti.com June 18, 2014, 1:04 p.m. UTC | #1
On Friday 09 May 2014 09:07 PM, Balaji T K wrote:
> Provide an option to get CD debounce time from DT
>
> Signed-off-by: Balaji T K <balajitk@ti.com>
Hi Chris/Ulf,

Any comments on this patch ?

> ---
>   Documentation/devicetree/bindings/mmc/mmc.txt |    1 +
>   drivers/mmc/core/host.c                       |    6 +++++-
>   2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 9dce540..fae590b 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -17,6 +17,7 @@ Optional properties:
>   - bus-width: Number of data lines, can be <1>, <4>, or <8>.  The default
>     will be <1> if the property is absent.
>   - wp-gpios: Specify GPIOs for write protection, see gpio binding
> +- cd-debounce-us: debounce time in microseconds for card detect gpio.
>   - cd-inverted: when present, polarity on the CD line is inverted. See the note
>     below for the case, when a GPIO is used for the CD line
>   - wp-inverted: when present, polarity on the WP line is inverted. See the note
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index fdea825..59cd3a0 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -313,6 +313,7 @@ int mmc_of_parse(struct mmc_host *host)
>   	bool explicit_inv_wp, gpio_inv_wp = false;
>   	enum of_gpio_flags flags;
>   	int len, ret, gpio;
> +	unsigned int debounce;
>
>   	if (!host->parent || !host->parent->of_node)
>   		return 0;
> @@ -367,6 +368,9 @@ int mmc_of_parse(struct mmc_host *host)
>   		if (of_find_property(np, "broken-cd", &len))
>   			host->caps |= MMC_CAP_NEEDS_POLL;
>
> +		if (of_property_read_u32(np, "cd-debounce-us", &debounce) < 0)
> +			debounce = 0;
> +
>   		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
>   		if (gpio == -EPROBE_DEFER)
>   			return gpio;
> @@ -374,7 +378,7 @@ int mmc_of_parse(struct mmc_host *host)
>   			if (!(flags & OF_GPIO_ACTIVE_LOW))
>   				gpio_inv_cd = true;
>
> -			ret = mmc_gpio_request_cd(host, gpio, 0);
> +			ret = mmc_gpio_request_cd(host, gpio, debounce);
>   			if (ret < 0) {
>   				dev_err(host->parent,
>   					"Failed to request CD GPIO #%d: %d!\n",
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson June 18, 2014, 1:47 p.m. UTC | #2
On 9 May 2014 17:37, Balaji T K <balajitk@ti.com> wrote:
> Provide an option to get CD debounce time from DT
>
> Signed-off-by: Balaji T K <balajitk@ti.com>
> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt |    1 +
>  drivers/mmc/core/host.c                       |    6 +++++-
>  2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 9dce540..fae590b 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -17,6 +17,7 @@ Optional properties:
>  - bus-width: Number of data lines, can be <1>, <4>, or <8>.  The default
>    will be <1> if the property is absent.
>  - wp-gpios: Specify GPIOs for write protection, see gpio binding
> +- cd-debounce-us: debounce time in microseconds for card detect gpio.
>  - cd-inverted: when present, polarity on the CD line is inverted. See the note
>    below for the case, when a GPIO is used for the CD line
>  - wp-inverted: when present, polarity on the WP line is inverted. See the note
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index fdea825..59cd3a0 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -313,6 +313,7 @@ int mmc_of_parse(struct mmc_host *host)
>         bool explicit_inv_wp, gpio_inv_wp = false;
>         enum of_gpio_flags flags;
>         int len, ret, gpio;
> +       unsigned int debounce;
>
>         if (!host->parent || !host->parent->of_node)
>                 return 0;
> @@ -367,6 +368,9 @@ int mmc_of_parse(struct mmc_host *host)
>                 if (of_find_property(np, "broken-cd", &len))
>                         host->caps |= MMC_CAP_NEEDS_POLL;
>
> +               if (of_property_read_u32(np, "cd-debounce-us", &debounce) < 0)
> +                       debounce = 0;
> +

Hi Balaji,

Sorry for a late reply.

I am wondering whether this should be a generic gpio of property,
instead of a mmc specific thing.

I have added Linus Walleij and Alexandre Courbot, the maintainers of
gpio. Let's see if they can point us in a direction.

>                 gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
>                 if (gpio == -EPROBE_DEFER)
>                         return gpio;
> @@ -374,7 +378,7 @@ int mmc_of_parse(struct mmc_host *host)
>                         if (!(flags & OF_GPIO_ACTIVE_LOW))
>                                 gpio_inv_cd = true;
>
> -                       ret = mmc_gpio_request_cd(host, gpio, 0);
> +                       ret = mmc_gpio_request_cd(host, gpio, debounce);
>                         if (ret < 0) {
>                                 dev_err(host->parent,
>                                         "Failed to request CD GPIO #%d: %d!\n",
> --
> 1.7.5.4
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot June 21, 2014, 7:22 a.m. UTC | #3
On Wed, Jun 18, 2014 at 10:47 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 9 May 2014 17:37, Balaji T K <balajitk@ti.com> wrote:
>> Provide an option to get CD debounce time from DT
>>
>> Signed-off-by: Balaji T K <balajitk@ti.com>
>> ---
>>  Documentation/devicetree/bindings/mmc/mmc.txt |    1 +
>>  drivers/mmc/core/host.c                       |    6 +++++-
>>  2 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
>> index 9dce540..fae590b 100644
>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
>> @@ -17,6 +17,7 @@ Optional properties:
>>  - bus-width: Number of data lines, can be <1>, <4>, or <8>.  The default
>>    will be <1> if the property is absent.
>>  - wp-gpios: Specify GPIOs for write protection, see gpio binding
>> +- cd-debounce-us: debounce time in microseconds for card detect gpio.
>>  - cd-inverted: when present, polarity on the CD line is inverted. See the note
>>    below for the case, when a GPIO is used for the CD line
>>  - wp-inverted: when present, polarity on the WP line is inverted. See the note
>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>> index fdea825..59cd3a0 100644
>> --- a/drivers/mmc/core/host.c
>> +++ b/drivers/mmc/core/host.c
>> @@ -313,6 +313,7 @@ int mmc_of_parse(struct mmc_host *host)
>>         bool explicit_inv_wp, gpio_inv_wp = false;
>>         enum of_gpio_flags flags;
>>         int len, ret, gpio;
>> +       unsigned int debounce;
>>
>>         if (!host->parent || !host->parent->of_node)
>>                 return 0;
>> @@ -367,6 +368,9 @@ int mmc_of_parse(struct mmc_host *host)
>>                 if (of_find_property(np, "broken-cd", &len))
>>                         host->caps |= MMC_CAP_NEEDS_POLL;
>>
>> +               if (of_property_read_u32(np, "cd-debounce-us", &debounce) < 0)
>> +                       debounce = 0;
>> +
>
> Hi Balaji,
>
> Sorry for a late reply.
>
> I am wondering whether this should be a generic gpio of property,
> instead of a mmc specific thing.
>
> I have added Linus Walleij and Alexandre Courbot, the maintainers of
> gpio. Let's see if they can point us in a direction.

I agree it would be nice if the debounce value could be handled by the
GPIO framework. I just wonder what would be the correct way of doing
it? Contrary to ACTIVE_LOW and other flags which can be specified with
the GPIO phandle, debounce is a numeric value. Maybe using a different
property, e.g.:

cd-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
cd-gpios-debounce = <10>;

When looking up a GPIO through gpiod_get(), the GPIO framework could
then check for -debounce property and set the debounce time
accordingly. At first sight I'd say that would work and could be used
for MMC and all other subsystems that need something similar.

Alex.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 7, 2014, 3:12 p.m. UTC | #4
On Sat, Jun 21, 2014 at 9:22 AM, Alexandre Courbot <gnurou@gmail.com> wrote:

>> I have added Linus Walleij and Alexandre Courbot, the maintainers of
>> gpio. Let's see if they can point us in a direction.
>
> I agree it would be nice if the debounce value could be handled by the
> GPIO framework.

I agree too.

> I just wonder what would be the correct way of doing
> it? Contrary to ACTIVE_LOW and other flags which can be specified with
> the GPIO phandle, debounce is a numeric value. Maybe using a different
> property, e.g.:
>
> cd-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
> cd-gpios-debounce = <10>;
>
> When looking up a GPIO through gpiod_get(), the GPIO framework could
> then check for -debounce property and set the debounce time
> accordingly. At first sight I'd say that would work and could be used
> for MMC and all other subsystems that need something similar.

Yes, but we also need to write generic debounce handling into
the gpiolib, so it doesn't matter if the GPIO driver can or cannot
handle this itself. Some hardware has the capability to debounce
lines in *hardware*.

Right now a call to gpiod_set_debounce() will fail unless the
hardware has a debounce option.

What we should really do is make gpiod_set_debounce() always
work, put the debounce value into the gpio_desc, and move some
logic similar to what exists in drivers/input/keyboard/gpio_keys.c
into the gpiolib, then get rid of any local implementations like
in gpio_keys.

Then we can rely on the gpiolib handling this at all times, and also
to get the debounce config from DT.

Dmitry, opinions on this?

Anyone up for implementing the timer in gpiolib and moving the
keyboard driver over to using the gpiolib debounce set?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov July 7, 2014, 4:59 p.m. UTC | #5
On Mon, Jul 7, 2014 at 8:12 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Jun 21, 2014 at 9:22 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>
>>> I have added Linus Walleij and Alexandre Courbot, the maintainers of
>>> gpio. Let's see if they can point us in a direction.
>>
>> I agree it would be nice if the debounce value could be handled by the
>> GPIO framework.
>
> I agree too.
>
>> I just wonder what would be the correct way of doing
>> it? Contrary to ACTIVE_LOW and other flags which can be specified with
>> the GPIO phandle, debounce is a numeric value. Maybe using a different
>> property, e.g.:
>>
>> cd-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
>> cd-gpios-debounce = <10>;
>>
>> When looking up a GPIO through gpiod_get(), the GPIO framework could
>> then check for -debounce property and set the debounce time
>> accordingly. At first sight I'd say that would work and could be used
>> for MMC and all other subsystems that need something similar.
>
> Yes, but we also need to write generic debounce handling into
> the gpiolib, so it doesn't matter if the GPIO driver can or cannot
> handle this itself. Some hardware has the capability to debounce
> lines in *hardware*.
>
> Right now a call to gpiod_set_debounce() will fail unless the
> hardware has a debounce option.
>
> What we should really do is make gpiod_set_debounce() always
> work, put the debounce value into the gpio_desc, and move some
> logic similar to what exists in drivers/input/keyboard/gpio_keys.c
> into the gpiolib, then get rid of any local implementations like
> in gpio_keys.
>
> Then we can rely on the gpiolib handling this at all times, and also
> to get the debounce config from DT.
>
> Dmitry, opinions on this?

I am always happy to hear when common functionality is moved into
appropriate layer.

Thanks.
Alexandre Courbot July 9, 2014, 12:28 p.m. UTC | #6
On Tue, Jul 8, 2014 at 12:12 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Jun 21, 2014 at 9:22 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>
>>> I have added Linus Walleij and Alexandre Courbot, the maintainers of
>>> gpio. Let's see if they can point us in a direction.
>>
>> I agree it would be nice if the debounce value could be handled by the
>> GPIO framework.
>
> I agree too.
>
>> I just wonder what would be the correct way of doing
>> it? Contrary to ACTIVE_LOW and other flags which can be specified with
>> the GPIO phandle, debounce is a numeric value. Maybe using a different
>> property, e.g.:
>>
>> cd-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
>> cd-gpios-debounce = <10>;
>>
>> When looking up a GPIO through gpiod_get(), the GPIO framework could
>> then check for -debounce property and set the debounce time
>> accordingly. At first sight I'd say that would work and could be used
>> for MMC and all other subsystems that need something similar.
>
> Yes, but we also need to write generic debounce handling into
> the gpiolib, so it doesn't matter if the GPIO driver can or cannot
> handle this itself. Some hardware has the capability to debounce
> lines in *hardware*.
>
> Right now a call to gpiod_set_debounce() will fail unless the
> hardware has a debounce option.
>
> What we should really do is make gpiod_set_debounce() always
> work, put the debounce value into the gpio_desc, and move some
> logic similar to what exists in drivers/input/keyboard/gpio_keys.c
> into the gpiolib, then get rid of any local implementations like
> in gpio_keys.
>
> Then we can rely on the gpiolib handling this at all times, and also
> to get the debounce config from DT.
>
> Dmitry, opinions on this?
>
> Anyone up for implementing the timer in gpiolib and moving the
> keyboard driver over to using the gpiolib debounce set?

If this can wait a bit for the Grand GPIO Refactoring and concurrent
GPIO consumers to be done, I don't mind giving this a shot next. If
someone else wants/needs it faster, please feel free to take over of
course.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 9dce540..fae590b 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -17,6 +17,7 @@  Optional properties:
 - bus-width: Number of data lines, can be <1>, <4>, or <8>.  The default
   will be <1> if the property is absent.
 - wp-gpios: Specify GPIOs for write protection, see gpio binding
+- cd-debounce-us: debounce time in microseconds for card detect gpio.
 - cd-inverted: when present, polarity on the CD line is inverted. See the note
   below for the case, when a GPIO is used for the CD line
 - wp-inverted: when present, polarity on the WP line is inverted. See the note
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index fdea825..59cd3a0 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -313,6 +313,7 @@  int mmc_of_parse(struct mmc_host *host)
 	bool explicit_inv_wp, gpio_inv_wp = false;
 	enum of_gpio_flags flags;
 	int len, ret, gpio;
+	unsigned int debounce;
 
 	if (!host->parent || !host->parent->of_node)
 		return 0;
@@ -367,6 +368,9 @@  int mmc_of_parse(struct mmc_host *host)
 		if (of_find_property(np, "broken-cd", &len))
 			host->caps |= MMC_CAP_NEEDS_POLL;
 
+		if (of_property_read_u32(np, "cd-debounce-us", &debounce) < 0)
+			debounce = 0;
+
 		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
 		if (gpio == -EPROBE_DEFER)
 			return gpio;
@@ -374,7 +378,7 @@  int mmc_of_parse(struct mmc_host *host)
 			if (!(flags & OF_GPIO_ACTIVE_LOW))
 				gpio_inv_cd = true;
 
-			ret = mmc_gpio_request_cd(host, gpio, 0);
+			ret = mmc_gpio_request_cd(host, gpio, debounce);
 			if (ret < 0) {
 				dev_err(host->parent,
 					"Failed to request CD GPIO #%d: %d!\n",