Skip to contentSkip to conversation

i2c: i2c-sh_mobile irq rollback fix

PATCH Applied Magnus Damm 5 participants 7 messages Aug 2, 2010 – Aug 31, 2010 3 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 i2c: i2c-sh_mobile irq rollback fix

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

---

 drivers/i2c/busses/i2c-sh_mobile.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
drivers/i2c/busses/i2c-sh_mobile.c +13 -10
@@ -538,15 +538,17 @@ static int sh_mobile_i2c_hook_irqs(struc
 {
 	struct resource *res;
 	int ret = -ENXIO;
-	int q, m;
-	int k = 0;
-	int n = 0;
+	int n, k = 0;
 
 	while ((res = platform_get_resource(dev, IORESOURCE_IRQ, k))) {
 		for (n = res->start; hook && n <= res->end; n++) {
 			if (request_irq(n, sh_mobile_i2c_isr, IRQF_DISABLED,
-					dev_name(&dev->dev), dev))
+					dev_name(&dev->dev), dev)) {
+				for (n--; n >= res->start; n--)
+					free_irq(n, dev);
+
 				goto rollback;
+			}
 		}
 		k++;
 	}
@@ -554,16 +556,17 @@ static int sh_mobile_i2c_hook_irqs(struc
 	if (hook)
 		return k > 0 ? 0 : -ENOENT;
 
-	k--;
 	ret = 0;
 
  rollback:
-	for (q = k; k >= 0; k--) {
-		for (m = n; m >= res->start; m--)
-			free_irq(m, dev);
+	k--;
+
+	while (k >= 0) {
+		res = platform_get_resource(dev, IORESOURCE_IRQ, k);
+		for (n = res->start; n <= res->end; n++)
+			free_irq(n, dev);
 
-		res = platform_get_resource(dev, IORESOURCE_IRQ, k - 1);
-		m = res->end;
+		k--;
 	}
 
 	return ret;
Applied
#1 lore raw
Quoted 8 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

Applied, thanks.

#2 lore raw
In reply to Magnus Damm
Quoted 16 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

---

 drivers/i2c/busses/i2c-sh_mobile.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

drivers/i2c/busses/i2c-sh_mobile.c static int sh_mobile_i2c_hook_irqs(struc · line 541

Quoted 7 lines
drivers/i2c/busses/i2c-sh_mobile.c +1 -3
@@ -538,15 +538,17 @@ static int sh_mobile_i2c_hook_irqs(struc
 {
 	struct resource *res;
 	int ret = -ENXIO;
-	int q, m;
-	int k = 0;
-	int n = 0;
+	int n, k = 0;

You can't really hope to get the code right with such variable names.

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 14 lines · Jean Delvare
Quoted 13 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

---

 drivers/i2c/busses/i2c-sh_mobile.c |   23 +++++++++++++----------

 1 file changed, 13 insertions(+), 10 deletions(-)
Quoted 12 lines

drivers/i2c/busses/i2c-sh_mobile.c   2010-08-02 15:14:48.000000000 +0900 static int sh_mobile_i2c_hook_irqs(struc · line 541

Quoted 10 lines
drivers/i2c/busses/i2c-sh_mobile.c   2010-08-02 15:14:48.000000000 +0900 renamed from drivers/i2c/busses/i2c-sh_mobile.c +1 -3
@@ -538,15 +538,17 @@ static int sh_mobile_i2c_hook_irqs(struc
  {
       struct resource *res;
       int ret = -ENXIO;
-     int q, m;
-     int k = 0;
-     int n = 0;
+     int n, k = 0;

You can't really hope to get the code right with such variable names.

And your suggestion is...? If you see something wrong with the code please let me know.

Thanks,

/ magnus

A message that is not in the archive (sent off-list or not yet received): AANLkTikQuCX7YC1tQ5MvwBdgxg1LE6k9f98-3M8khBJ6-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#6 lore raw
Quoted 28 lines · Magnus Damm
Quoted 14 lines · Jean Delvare
Quoted 13 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

---

 drivers/i2c/busses/i2c-sh_mobile.c |   23 +++++++++++++----------

 1 file changed, 13 insertions(+), 10 deletions(-)
Quoted 12 lines

drivers/i2c/busses/i2c-sh_mobile.c   2010-08-02 15:14:48.000000000 +0900 static int sh_mobile_i2c_hook_irqs(struc · line 541

Quoted 10 lines
drivers/i2c/busses/i2c-sh_mobile.c   2010-08-02 15:14:48.000000000 +0900 renamed from drivers/i2c/busses/i2c-sh_mobile.c +1 -3
@@ -538,15 +538,17 @@ static int sh_mobile_i2c_hook_irqs(struc
  {
       struct resource *res;
       int ret = -ENXIO;
-     int q, m;
-     int k = 0;
-     int n = 0;
+     int n, k = 0;

You can't really hope to get the code right with such variable names.

Quoted 2 lines

And your suggestion is...? If you see something wrong with the code please let me know.

Nothing specific. I'm just trying to make you realize that one-letter variable names don't carry any meaning and thus make the code harder to write, read and debug.

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#8 lore raw
Quoted 29 lines · Jean Delvare
Quoted 28 lines · Magnus Damm
Quoted 26 lines · Jean Delvare
Quoted 16 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

---

 drivers/i2c/busses/i2c-sh_mobile.c |   23 +++++++++++++----------

 1 file changed, 13 insertions(+), 10 deletions(-)

drivers/i2c/busses/i2c-sh_mobile.c   2010-08-02 15:14:48.000000000 +0900 static int sh_mobile_i2c_hook_irqs(struc · line 541

Quoted 7 lines
drivers/i2c/busses/i2c-sh_mobile.c   2010-08-02 15:14:48.000000000 +0900 renamed from drivers/i2c/busses/i2c-sh_mobile.c +1 -3
@@ -538,15 +538,17 @@ static int sh_mobile_i2c_hook_irqs(struc
  {
       struct resource *res;
       int ret = -ENXIO;
-     int q, m;
-     int k = 0;
-     int n = 0;
+     int n, k = 0;

You can't really hope to get the code right with such variable names.

Quoted 6 lines
Quoted 2 lines

And your suggestion is...? If you see something wrong with the code please let me know.

Nothing specific. I'm just trying to make you realize that one-letter variable names don't carry any meaning and thus make the code harder to write, read and debug.

Yes, I admit that four one-letter variables in a single function is begging for trouble. Having one or two one-letter variable names is not so bad IMO.

I guess one-letter variable names is my attempt to stick to pure function and avoid going in the direction of endless coding style discussions. =)

Cheers,

/ magnus

#9 lore raw
In reply to Jean Delvare
Quoted 14 lines · Jean Delvare
Quoted 13 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Update the i2c-sh_mobile driver to properly free interrupts. The function sh_mobile_i2c_hook_irqs() is fixed so module both unload and load are working as expected.

---

 drivers/i2c/busses/i2c-sh_mobile.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
Quoted 12 lines

drivers/i2c/busses/i2c-sh_mobile.c static int sh_mobile_i2c_hook_irqs(struc · line 541

Quoted 10 lines
drivers/i2c/busses/i2c-sh_mobile.c +1 -3
@@ -538,15 +538,17 @@ static int sh_mobile_i2c_hook_irqs(struc
 {
 	struct resource *res;
 	int ret = -ENXIO;
-	int q, m;
-	int k = 0;
-	int n = 0;
+	int n, k = 0;

You can't really hope to get the code right with such variable names.

I agree, this isn't very readable. If possible, please have a rethink of the code. You may also want to start using devm_ to track your resources.

Signature · 4 lines
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.