Skip to contentSkip to conversation

sh: kfr2r09: fix i2c adapter leaks

PATCH In review Johan Hovold 3 participants 11 messages May 8 – Oct 1

Subsystems (2)
Stable branch SuperH
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover sh: kfr2r09: fix i2c adapter leaks

The kfr2r09 platform setup code fails to drop the references it acquires to the I2C adapter.

Johan

Johan Hovold (2):
sh: kfr2r09: fix i2c adapter leak on USB gdaget setup
sh: kfr2r09: fix i2c adapter leak on serial console setup

 arch/sh/boards/mach-kfr2r09/setup.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
Signature · 1 line
2.53.0
author
#1 lore raw

PATCH 1/2 sh: kfr2r09: fix i2c adapter leak on USB gdaget setup

Make sure to drop the reference taken to the I2C adapter (and its module) when enabling USB gadget mode which prevents the adapter from ever being deregistered.

Fixes 5a1c4cb5bc22 ("sh: add r8a66597 usb0 gadget to the kfr2r09 board")
Cc [email protected] # 2.6.32
Cc Magnus Damm <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/sh/boards/mach-kfr2r09/setup.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
arch/sh/boards/mach-kfr2r09/setup.c +10 -3
index 70236859919d..62af9a9db039 100644
@@ -368,7 +368,7 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
 	msg.flags = 0;
 	ret = i2c_transfer(a, &msg, 1);
 	if (ret != 1)
-		return -ENODEV;
+		goto err_put_adapter;
 
 	buf[0] = 0;
 	msg.addr = 0x09;

@@ -377,7 +377,7 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
 	msg.flags = I2C_M_RD;
 	ret = i2c_transfer(a, &msg, 1);
 	if (ret != 1)
-		return -ENODEV;
+		goto err_put_adapter;
 
 	buf[1] = buf[0] | (1 << 1);
 	buf[0] = 0x13;

@@ -387,9 +387,16 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
 	msg.flags = 0;
 	ret = i2c_transfer(a, &msg, 1);
 	if (ret != 1)
-		return -ENODEV;
+		goto err_put_adapter;
+
+	i2c_put_adapter(a);
 
 	return 0;
+
+err_put_adapter:
+	i2c_put_adapter(a);
+
+	return -ENODEV;
 }
 
 static int kfr2r09_serial_i2c_setup(void)
Signature · 1 line
2.53.0
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] sh: kfr2r09: fix i2c adapter leak on USB gdaget setup

Hi Johan,

Quoted 8 lines · Johan Hovold

Make sure to drop the reference taken to the I2C adapter (and its module) when enabling USB gadget mode which prevents the adapter from ever being deregistered.

Fixes 5a1c4cb5bc22 ("sh: add r8a66597 usb0 gadget to the kfr2r09 board")
Cc [email protected] # 2.6.32
Cc Magnus Damm <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Thanks for your patch!

Quoted 25 lines
arch/sh/boards/mach-kfr2r09/setup.c +2 -3
@@ -368,7 +368,7 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
        msg.flags = 0;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;
+               goto err_put_adapter;
 
        buf[0] = 0;
        msg.addr = 0x09;

@@ -377,7 +377,7 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
        msg.flags = I2C_M_RD;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;
+               goto err_put_adapter;
 
        buf[1] = buf[0] | (1 << 1);
        buf[0] = 0x13;

@@ -387,9 +387,16 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
        msg.flags = 0;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;

arch/sh/boards/mach-kfr2r09/setup.c static int kfr2r09_usb0_gadget_i2c_setup(void) · line 399

Quoted 10 lines
arch/sh/boards/mach-kfr2r09/setup.c +8 -0
+               goto err_put_adapter;
+
+       i2c_put_adapter(a);
 
        return 0;
+
+err_put_adapter:
+       i2c_put_adapter(a);
+
+       return -ENODEV;

I case i2c_transfer() returns a negative error code (the other possible value is zero, right?), you might want to propagate that to the caller. However, the single caller replaces it by -ENODEV anyway, so I guess your patch is fine.

Reviewed-by Geert Uytterhoeven <[email protected]>

arch/sh/boards/mach-kfr2r09/setup.c static int kfr2r09_usb0_gadget_i2c_setup(void) · line 402

Quoted 3 lines
arch/sh/boards/mach-kfr2r09/setup.c +0 -0
 }
 
 static int kfr2r09_serial_i2c_setup(void)

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author
#3 lore raw

Re: [PATCH 1/2] sh: kfr2r09: fix i2c adapter leak on USB gdaget setup

Hi Geert,

Quoted 19 lines · Geert Uytterhoeven
Quoted 18 lines · Johan Hovold

Make sure to drop the reference taken to the I2C adapter (and its module) when enabling USB gadget mode which prevents the adapter from ever being deregistered.

Fixes 5a1c4cb5bc22 ("sh: add r8a66597 usb0 gadget to the kfr2r09 board")
Cc [email protected] # 2.6.32
Cc Magnus Damm <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
arch/sh/boards/mach-kfr2r09/setup.c +3 -1
@@ -387,9 +387,16 @@ static int kfr2r09_usb0_gadget_i2c_setup(void)
        msg.flags = 0;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;
+               goto err_put_adapter;
+
+       i2c_put_adapter(a);
Quoted 11 lines

arch/sh/boards/mach-kfr2r09/setup.c static int kfr2r09_usb0_gadget_i2c_setup(void) · line 399

Quoted 6 lines
arch/sh/boards/mach-kfr2r09/setup.c +5 -0
        return 0;
+
+err_put_adapter:
+       i2c_put_adapter(a);
+
+       return -ENODEV;

I case i2c_transfer() returns a negative error code (the other possible value is zero, right?), you might want to propagate that to the caller. However, the single caller replaces it by -ENODEV anyway, so I guess your patch is fine.

Yes, indeed, but that's arguably a separate change. And in this case it doesn't really matter currently as you point out.

Quoted 1 line
Reviewed-by Geert Uytterhoeven <[email protected]>

Thanks for reviewing.

Johan

author
#4 lore raw

PATCH 2/2 sh: kfr2r09: fix i2c adapter leak on serial console setup

In reply to Johan Hovold

Make sure to drop the reference taken to the I2C adapter (and its module) when setting up the serial console which prevents the adapter from ever being deregistered.

Fixes e6d8460aca63 ("sh: Improve kfr2r09 serial port setup code")
Cc [email protected] # 2.6.33
Cc Magnus Damm <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/sh/boards/mach-kfr2r09/setup.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
arch/sh/boards/mach-kfr2r09/setup.c +10 -3
index 62af9a9db039..8f436bcc1ae1 100644
@@ -418,7 +418,7 @@ static int kfr2r09_serial_i2c_setup(void)
 	msg.flags = 0;
 	ret = i2c_transfer(a, &msg, 1);
 	if (ret != 1)
-		return -ENODEV;
+		goto err_put_adapter;
 
 	buf[0] = 0;
 	msg.addr = 0x09;

@@ -427,7 +427,7 @@ static int kfr2r09_serial_i2c_setup(void)
 	msg.flags = I2C_M_RD;
 	ret = i2c_transfer(a, &msg, 1);
 	if (ret != 1)
-		return -ENODEV;
+		goto err_put_adapter;
 
 	buf[1] = buf[0] | (1 << 6);
 	buf[0] = 0x13;

@@ -437,9 +437,16 @@ static int kfr2r09_serial_i2c_setup(void)
 	msg.flags = 0;
 	ret = i2c_transfer(a, &msg, 1);
 	if (ret != 1)
-		return -ENODEV;
+		goto err_put_adapter;
+
+	i2c_put_adapter(a);
 
 	return 0;
+
+err_put_adapter:
+	i2c_put_adapter(a);
+
+	return -ENODEV;
 }
 #else
 static int kfr2r09_usb0_gadget_i2c_setup(void)
Signature · 1 line
2.53.0
Reviewed-by on v1 2/2
#5 lore raw

Re: [PATCH 2/2] sh: kfr2r09: fix i2c adapter leak on serial console setup

Hi Johan,

Quoted 8 lines · Johan Hovold

Make sure to drop the reference taken to the I2C adapter (and its module) when setting up the serial console which prevents the adapter from ever being deregistered.

Fixes e6d8460aca63 ("sh: Improve kfr2r09 serial port setup code")
Cc [email protected] # 2.6.33
Cc Magnus Damm <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Thanks for your patch!

Quoted 25 lines
arch/sh/boards/mach-kfr2r09/setup.c +2 -3
@@ -418,7 +418,7 @@ static int kfr2r09_serial_i2c_setup(void)
        msg.flags = 0;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;
+               goto err_put_adapter;
 
        buf[0] = 0;
        msg.addr = 0x09;

@@ -427,7 +427,7 @@ static int kfr2r09_serial_i2c_setup(void)
        msg.flags = I2C_M_RD;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;
+               goto err_put_adapter;
 
        buf[1] = buf[0] | (1 << 6);
        buf[0] = 0x13;

@@ -437,9 +437,16 @@ static int kfr2r09_serial_i2c_setup(void)
        msg.flags = 0;
        ret = i2c_transfer(a, &msg, 1);
        if (ret != 1)
-               return -ENODEV;

arch/sh/boards/mach-kfr2r09/setup.c static int kfr2r09_serial_i2c_setup(void) · line 449

Quoted 10 lines
arch/sh/boards/mach-kfr2r09/setup.c +8 -0
+               goto err_put_adapter;
+
+       i2c_put_adapter(a);
 
        return 0;
+
+err_put_adapter:
+       i2c_put_adapter(a);
+
+       return -ENODEV;

I case i2c_transfer() returns a negative error code (the other possible value is zero, right?), you might want to propagate that to the caller. However, the single caller ignores the return value anyway, so I guess your patch is fine.

Reviewed-by Geert Uytterhoeven <[email protected]>

arch/sh/boards/mach-kfr2r09/setup.c static int kfr2r09_serial_i2c_setup(void) · line 452

Quoted 3 lines
arch/sh/boards/mach-kfr2r09/setup.c +0 -0
 }
 #else
 static int kfr2r09_usb0_gadget_i2c_setup(void)

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author
#6 lore raw
In reply to Johan Hovold
Quoted 6 lines · Johan Hovold

The kfr2r09 platform setup code fails to drop the references it acquires to the I2C adapter.

Johan Hovold (2):
sh: kfr2r09: fix i2c adapter leak on USB gdaget setup
sh: kfr2r09: fix i2c adapter leak on serial console setup

Can these be picked up now?

Johan

maintainer
#7 lore raw
Quoted 9 lines · Johan Hovold
Quoted 6 lines · Johan Hovold

The kfr2r09 platform setup code fails to drop the references it acquires to the I2C adapter.

Johan Hovold (2):
sh: kfr2r09: fix i2c adapter leak on USB gdaget setup
sh: kfr2r09: fix i2c adapter leak on serial console setup

Can these be picked up now?

I will try to review some patches this weekend.

Adrian

Signature · 4 lines
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
author
#8 lore raw
Quoted 5 lines · John Paul Adrian Glaubitz
Quoted 4 lines · Johan Hovold
Quoted 2 lines · Johan Hovold

The kfr2r09 platform setup code fails to drop the references it acquires to the I2C adapter.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

Johan Hovold (2):
sh: kfr2r09: fix i2c adapter leak on USB gdaget setup
sh: kfr2r09: fix i2c adapter leak on serial console setup

Can these be picked up now?

I will try to review some patches this weekend.

Any chance to get these into 7.3?

Johan

author
#9 lore raw
Quoted 10 lines · Johan Hovold
Quoted 9 lines · John Paul Adrian Glaubitz
Quoted 8 lines · Johan Hovold
Quoted 6 lines · Johan Hovold

The kfr2r09 platform setup code fails to drop the references it acquires to the I2C adapter.

Johan Hovold (2):
sh: kfr2r09: fix i2c adapter leak on USB gdaget setup
sh: kfr2r09: fix i2c adapter leak on serial console setup

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Can these be picked up now?

I will try to review some patches this weekend.

Any chance to get these into 7.3?

Can these be picked up now?

Johan

maintainer
#10 lore raw

Hi Johan,

Quoted 11 lines · Johan Hovold
Quoted 10 lines · Johan Hovold
Quoted 9 lines · John Paul Adrian Glaubitz
Quoted 8 lines · Johan Hovold
Quoted 6 lines · Johan Hovold

The kfr2r09 platform setup code fails to drop the references it acquires to the I2C adapter.

Johan Hovold (2):
sh: kfr2r09: fix i2c adapter leak on USB gdaget setup
sh: kfr2r09: fix i2c adapter leak on serial console setup

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Can these be picked up now?

I will try to review some patches this weekend.

Any chance to get these into 7.3?

Can these be picked up now?

Yes, I'll pick them up for v7.4. I have already reviewed and merged some patches for v7.4. I need to perform some more tests but yours should go in with v7.4 as well.

Adrian

Signature · 4 lines
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913