[PATCH] MMCI: use _cansleep GPIO functions

[PATCH] MMCI: use _cansleep GPIO functions

[Linus Walleij]

Currently the kernel is screaming about slowpath at me for the
wp/cd callbacks. Swicth to the _cansleep variants so as to silence
this. Also remove the double-negate in front of the cd GPIO read.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/mmc/host/mmci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f0c7313..082abad 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -614,7 +614,7 @@ static int mmci_get_ro(struct mmc_host *mmc)
 	if (host->gpio_wp == -ENOSYS)
 		return -ENOSYS;
 
-	return gpio_get_value(host->gpio_wp);
+	return gpio_get_value_cansleep(host->gpio_wp);
 }
 
 static int mmci_get_cd(struct mmc_host *mmc)
@@ -629,7 +629,8 @@ static int mmci_get_cd(struct mmc_host *mmc)
 
 		status = plat->status(mmc_dev(host->mmc));
 	} else
-		status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
+		status = gpio_get_value_cansleep(host->gpio_cd) ^
+			plat->cd_invert;
 
 	/*
 	 * Use positive logic throughout - status is zero for no card,
-- 
1.6.3.3

[PATCH] MMCI: use _cansleep GPIO functions

[Rabin Vincent]

On Fri, Sep 10, 2010 at 2:01 AM, Linus Walleij
<linus.walleij@stericsson.com> wrote:
> - ? ? ? ? ? ? ? status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
> + ? ? ? ? ? ? ? status = gpio_get_value_cansleep(host->gpio_cd) ^
> + ? ? ? ? ? ? ? ? ? ? ? plat->cd_invert;

Why do you remove the double negation?  gpio_get_value*()
can return non-zero values, and we need to make it 1 for the
inversion xor to work.

[PATCH] MMCI: use _cansleep GPIO functions

[Linus Walleij]

2010/9/11 Rabin Vincent <rabin@rab.in>:
> On Fri, Sep 10, 2010 at 2:01 AM, Linus Walleij
> <linus.walleij@stericsson.com> wrote:
>> - ? ? ? ? ? ? ? status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
>> + ? ? ? ? ? ? ? status = gpio_get_value_cansleep(host->gpio_cd) ^
>> + ? ? ? ? ? ? ? ? ? ? ? plat->cd_invert;
>
> Why do you remove the double negation? ?gpio_get_value*()
> can return non-zero values, and we need to make it 1 for the
> inversion xor to work.

Aha I get it. But !! is a bit hard to parse in head is it not...

Cast a non-zero to C internal bool type, negate it and then
negate the negation so the end result is bool, not just
non-zero. I never saw that idiom before.

I don't know if I'm particularly dumb but it makes my reading
take time.

I'll slam in some intermediate bool or something so it's super
clear what's happening, OK?

Yours,
Linus Walleij

[PATCH] MMCI: use _cansleep GPIO functions

[Linus Walleij]

2010/9/11 Linus Walleij <linus.ml.walleij@gmail.com>:

> I'll slam in some intermediate bool or something so it's super
> clear what's happening, OK?

Like this (untested):

From: Linus Walleij <linus.walleij@stericsson.com>
Date: Sat, 11 Sep 2010 13:29:04 +0200
Subject: [PATCH] MMCI: use _cansleep GPIO functions

Currently the kernel is screaming about slowpath at me for the
wp/cd callbacks. Swicth to the _cansleep variants so as to silence
this. Also rewrite the double-negate in front of the cd GPIO read
to make it more clear what is happening.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/mmc/host/mmci.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f0c7313..603d4d5 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -614,7 +614,7 @@ static int mmci_get_ro(struct mmc_host *mmc)
 	if (host->gpio_wp == -ENOSYS)
 		return -ENOSYS;

-	return gpio_get_value(host->gpio_wp);
+	return gpio_get_value_cansleep(host->gpio_wp);
 }

 static int mmci_get_cd(struct mmc_host *mmc)
@@ -629,7 +629,9 @@ static int mmci_get_cd(struct mmc_host *mmc)

 		status = plat->status(mmc_dev(host->mmc));
 	} else
-		status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
+		/* Convert the status to boolean and optionally invert */
+		status = (gpio_get_value_cansleep(host->gpio_cd) ? 1 : 0)
+			^ plat->cd_invert;

 	/*
 	 * Use positive logic throughout - status is zero for no card,
-- 
1.7.2.2

[PATCH] MMCI: use _cansleep GPIO functions

[Russell King - ARM Linux]

On Sat, Sep 11, 2010 at 01:22:04PM +0200, Linus Walleij wrote:
> I'll slam in some intermediate bool or something so it's super
> clear what's happening, OK?

No - once you've learnt that !! turns an int into a bool, there's
nothing more that needs to be done.

You'll find !! used throughout the kernel sources.

[PATCH] MMCI: use _cansleep GPIO functions

[Linus Walleij]

2010/9/11 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> You'll find !! used throughout the kernel sources.

Aha it's a kernel idiom, OK!!

I'll just test it on hardware and I'll put it in the patch tracker.

From: Linus Walleij <linus.walleij@stericsson.com>
Date: Sat, 11 Sep 2010 13:36:20 +0200
Subject: [PATCH] MMCI: use _cansleep GPIO functions

Currently the kernel is screaming about slowpath at me for the
wp/cd callbacks. Swicth to the _cansleep variants so as to silence
this.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/mmc/host/mmci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index f0c7313..c5fbf19 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -614,7 +614,7 @@ static int mmci_get_ro(struct mmc_host *mmc)
 	if (host->gpio_wp == -ENOSYS)
 		return -ENOSYS;

-	return gpio_get_value(host->gpio_wp);
+	return gpio_get_value_cansleep(host->gpio_wp);
 }

 static int mmci_get_cd(struct mmc_host *mmc)
@@ -629,7 +629,8 @@ static int mmci_get_cd(struct mmc_host *mmc)

 		status = plat->status(mmc_dev(host->mmc));
 	} else
-		status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
+		status = !!gpio_get_value_cansleep(host->gpio_cd)
+			^ plat->cd_invert;

 	/*
 	 * Use positive logic throughout - status is zero for no card,
-- 
1.7.2.2

[PATCH] MMCI: use _cansleep GPIO functions

[Russell King - ARM Linux]

On Sat, Sep 11, 2010 at 01:39:04PM +0200, Linus Walleij wrote:
> 2010/9/11 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> 
> > You'll find !! used throughout the kernel sources.
> 
> Aha it's a kernel idiom, OK!!
> 
> I'll just test it on hardware and I'll put it in the patch tracker.
> 
> From: Linus Walleij <linus.walleij@stericsson.com>
> Date: Sat, 11 Sep 2010 13:36:20 +0200
> Subject: [PATCH] MMCI: use _cansleep GPIO functions
> 
> Currently the kernel is screaming about slowpath at me for the
> wp/cd callbacks. Swicth to the _cansleep variants so as to silence
> this.

You may wish to correct that spelling error.

[PATCH] MMCI: use _cansleep GPIO functions

[Sergei Shtylyov]

Hello.

Linus Walleij wrote:

> From: Linus Walleij <linus.walleij@stericsson.com>
> Date: Sat, 11 Sep 2010 13:36:20 +0200
> Subject: [PATCH] MMCI: use _cansleep GPIO functions

> Currently the kernel is screaming about slowpath at me for the
> wp/cd callbacks. Swicth to the _cansleep variants so as to silence
> this.

> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/mmc/host/mmci.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)

> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index f0c7313..c5fbf19 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
[...]
>  static int mmci_get_cd(struct mmc_host *mmc)
> @@ -629,7 +629,8 @@ static int mmci_get_cd(struct mmc_host *mmc)
> 
>  		status = plat->status(mmc_dev(host->mmc));
>  	} else
> -		status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
> +		status = !!gpio_get_value_cansleep(host->gpio_cd)
> +			^ plat->cd_invert;

    Should be no space between ^ and operand, no?

WBR, Sergei

[PATCH] MMCI: use _cansleep GPIO functions

[Sergei Shtylyov]

I wrote:

>> From: Linus Walleij <linus.walleij@stericsson.com>
>> Date: Sat, 11 Sep 2010 13:36:20 +0200
>> Subject: [PATCH] MMCI: use _cansleep GPIO functions

>> Currently the kernel is screaming about slowpath at me for the
>> wp/cd callbacks. Swicth to the _cansleep variants so as to silence
>> this.

>> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
>> ---
>>  drivers/mmc/host/mmci.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)

>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index f0c7313..c5fbf19 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
> [...]
>>  static int mmci_get_cd(struct mmc_host *mmc)
>> @@ -629,7 +629,8 @@ static int mmci_get_cd(struct mmc_host *mmc)
>>
>>          status = plat->status(mmc_dev(host->mmc));
>>      } else
>> -        status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
>> +        status = !!gpio_get_value_cansleep(host->gpio_cd)
>> +            ^ plat->cd_invert;

>    Should be no space between ^ and operand, no?

    Oops, I've mixed it with ~. Still, I'd have left ^ on the first line.

WBR, Sergei