[PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

[PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

[Masahiro Yamada]

This driver calls of_mm_gpiochip_add() to add a memory mapped gpio
chip.  So, of_mm_gpiochip_remove() should be used when removing it.

The direct call of gpiochip_remove() misses unmapping the register
and freeing the label.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/gpio/gpio-altera.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
index 449fb46..c653c83 100644
--- a/drivers/gpio/gpio-altera.c
+++ b/drivers/gpio/gpio-altera.c
@@ -337,7 +337,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
 {
 	struct altera_gpio_chip *altera_gc = platform_get_drvdata(pdev);
 
-	gpiochip_remove(&altera_gc->mmchip.gc);
+	of_mm_gpiochip_remove(&altera_gc->mmchip);
 
 	return -EIO;
 }
-- 
1.9.1

[PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Masahiro Yamada]

The remove callback never succeeds, which seems odd.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

I wonder why nobody has pointed this out before me.
I am suspecting -EIO might be intentional.
I hope some Altera guys will give me comments.


 drivers/gpio/gpio-altera.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
index c653c83..5861550 100644
--- a/drivers/gpio/gpio-altera.c
+++ b/drivers/gpio/gpio-altera.c
@@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
 
 	of_mm_gpiochip_remove(&altera_gc->mmchip);
 
-	return -EIO;
+	return 0;
 }
 
 static const struct of_device_id altera_gpio_of_match[] = {
-- 
1.9.1

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Alexandre Courbot]

On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> The remove callback never succeeds, which seems odd.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> I wonder why nobody has pointed this out before me.
> I am suspecting -EIO might be intentional.
> I hope some Altera guys will give me comments.
>
>
>  drivers/gpio/gpio-altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index c653c83..5861550 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>
>         of_mm_gpiochip_remove(&altera_gc->mmchip);
>
> -       return -EIO;
> +       return 0;

That looks weird indeed. Tien, can you comment on this?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Alexandre Courbot]

On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> The remove callback never succeeds, which seems odd.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> I wonder why nobody has pointed this out before me.
> I am suspecting -EIO might be intentional.
> I hope some Altera guys will give me comments.
>
>
>  drivers/gpio/gpio-altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index c653c83..5861550 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>
>         of_mm_gpiochip_remove(&altera_gc->mmchip);
>
> -       return -EIO;
> +       return 0;

That looks weird indeed. Tien, can you comment on this?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Tien Hock Loh]

Sorry I was away from my mail for the past few weeks. 
This isn't intentional, should be a bug I overlook. The fix is correct.

On Sun, 2015-06-21 at 16:25 +0900, Alexandre Courbot wrote:
> On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> > The remove callback never succeeds, which seems odd.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > I wonder why nobody has pointed this out before me.
> > I am suspecting -EIO might be intentional.
> > I hope some Altera guys will give me comments.
> >
> >
> >  drivers/gpio/gpio-altera.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> > index c653c83..5861550 100644
> > --- a/drivers/gpio/gpio-altera.c
> > +++ b/drivers/gpio/gpio-altera.c
> > @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
> >
> >         of_mm_gpiochip_remove(&altera_gc->mmchip);
> >
> > -       return -EIO;
> > +       return 0;
> 
> That looks weird indeed. Tien, can you comment on this?

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Tien Hock Loh]

Sorry I was away from my mail for the past few weeks. 
This isn't intentional, should be a bug I overlook. The fix is correct.

On Sun, 2015-06-21 at 16:25 +0900, Alexandre Courbot wrote:
> On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> > The remove callback never succeeds, which seems odd.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > I wonder why nobody has pointed this out before me.
> > I am suspecting -EIO might be intentional.
> > I hope some Altera guys will give me comments.
> >
> >
> >  drivers/gpio/gpio-altera.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> > index c653c83..5861550 100644
> > --- a/drivers/gpio/gpio-altera.c
> > +++ b/drivers/gpio/gpio-altera.c
> > @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
> >
> >         of_mm_gpiochip_remove(&altera_gc->mmchip);
> >
> > -       return -EIO;
> > +       return 0;
> 
> That looks weird indeed. Tien, can you comment on this?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Alexandre Courbot]

On Mon, Jun 22, 2015 at 10:36 AM, Tien Hock Loh <thloh@altera.com> wrote:
> Sorry I was away from my mail for the past few weeks.
> This isn't intentional, should be a bug I overlook. The fix is correct.

Interestingly that has never been caught by reviewers despite 10
respins of your series!

Thanks for confirming.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

>
> On Sun, 2015-06-21 at 16:25 +0900, Alexandre Courbot wrote:
>> On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>> > The remove callback never succeeds, which seems odd.
>> >
>> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > ---
>> >
>> > I wonder why nobody has pointed this out before me.
>> > I am suspecting -EIO might be intentional.
>> > I hope some Altera guys will give me comments.
>> >
>> >
>> >  drivers/gpio/gpio-altera.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
>> > index c653c83..5861550 100644
>> > --- a/drivers/gpio/gpio-altera.c
>> > +++ b/drivers/gpio/gpio-altera.c
>> > @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>> >
>> >         of_mm_gpiochip_remove(&altera_gc->mmchip);
>> >
>> > -       return -EIO;
>> > +       return 0;
>>
>> That looks weird indeed. Tien, can you comment on this?
>

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Alexandre Courbot]

On Mon, Jun 22, 2015 at 10:36 AM, Tien Hock Loh <thloh@altera.com> wrote:
> Sorry I was away from my mail for the past few weeks.
> This isn't intentional, should be a bug I overlook. The fix is correct.

Interestingly that has never been caught by reviewers despite 10
respins of your series!

Thanks for confirming.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

>
> On Sun, 2015-06-21 at 16:25 +0900, Alexandre Courbot wrote:
>> On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>> > The remove callback never succeeds, which seems odd.
>> >
>> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > ---
>> >
>> > I wonder why nobody has pointed this out before me.
>> > I am suspecting -EIO might be intentional.
>> > I hope some Altera guys will give me comments.
>> >
>> >
>> >  drivers/gpio/gpio-altera.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
>> > index c653c83..5861550 100644
>> > --- a/drivers/gpio/gpio-altera.c
>> > +++ b/drivers/gpio/gpio-altera.c
>> > @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>> >
>> >         of_mm_gpiochip_remove(&altera_gc->mmchip);
>> >
>> > -       return -EIO;
>> > +       return 0;
>>
>> That looks weird indeed. Tien, can you comment on this?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Masahiro Yamada]

Hi Tien,


2015-06-22 10:36 GMT+09:00 Tien Hock Loh <thloh@altera.com>:
> Sorry I was away from my mail for the past few weeks.
> This isn't intentional, should be a bug I overlook. The fix is correct.
>

OK, then could you issue your Acked-by credit?


Also, do the followings look good?

http://patchwork.ozlabs.org/patch/485370/
http://patchwork.ozlabs.org/patch/486669/

Thanks.

-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Masahiro Yamada]

Hi Tien,


2015-06-22 10:36 GMT+09:00 Tien Hock Loh <thloh@altera.com>:
> Sorry I was away from my mail for the past few weeks.
> This isn't intentional, should be a bug I overlook. The fix is correct.
>

OK, then could you issue your Acked-by credit?


Also, do the followings look good?

http://patchwork.ozlabs.org/patch/485370/
http://patchwork.ozlabs.org/patch/486669/

Thanks.

-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Tien Hock Loh]

Acked-by: Tien Hock Loh <thloh@altera.com>

On Wed, 2015-06-17 at 20:59 +0900, Masahiro Yamada wrote:
> The remove callback never succeeds, which seems odd.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> I wonder why nobody has pointed this out before me.
> I am suspecting -EIO might be intentional.
> I hope some Altera guys will give me comments.
> 
> 
>  drivers/gpio/gpio-altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index c653c83..5861550 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>  
>  	of_mm_gpiochip_remove(&altera_gc->mmchip);
>  
> -	return -EIO;
> +	return 0;
>  }
>  
>  static const struct of_device_id altera_gpio_of_match[] = {

Re: [PATCH 2/2] gpio: altera: fix return value of altera_gpio_remove()

[Tien Hock Loh]

Acked-by: Tien Hock Loh <thloh@altera.com>

On Wed, 2015-06-17 at 20:59 +0900, Masahiro Yamada wrote:
> The remove callback never succeeds, which seems odd.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> I wonder why nobody has pointed this out before me.
> I am suspecting -EIO might be intentional.
> I hope some Altera guys will give me comments.
> 
> 
>  drivers/gpio/gpio-altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index c653c83..5861550 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -339,7 +339,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>  
>  	of_mm_gpiochip_remove(&altera_gc->mmchip);
>  
> -	return -EIO;
> +	return 0;
>  }
>  
>  static const struct of_device_id altera_gpio_of_match[] = {

Re: [PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> This driver calls of_mm_gpiochip_add() to add a memory mapped gpio
> chip.  So, of_mm_gpiochip_remove() should be used when removing it.
>
> The direct call of gpiochip_remove() misses unmapping the register
> and freeing the label.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in

Re: [PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

[Alexandre Courbot]

On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> This driver calls of_mm_gpiochip_add() to add a memory mapped gpio
> chip.  So, of_mm_gpiochip_remove() should be used when removing it.
>
> The direct call of gpiochip_remove() misses unmapping the register
> and freeing the label.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

[Tien Hock Loh]

Acked-by: Tien Hock Loh <thloh@altera.com>

On Wed, 2015-06-17 at 20:59 +0900, Masahiro Yamada wrote:
> This driver calls of_mm_gpiochip_add() to add a memory mapped gpio
> chip.  So, of_mm_gpiochip_remove() should be used when removing it.
> 
> The direct call of gpiochip_remove() misses unmapping the register
> and freeing the label.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/gpio/gpio-altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index 449fb46..c653c83 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -337,7 +337,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>  {
>  	struct altera_gpio_chip *altera_gc = platform_get_drvdata(pdev);
>  
> -	gpiochip_remove(&altera_gc->mmchip.gc);
> +	of_mm_gpiochip_remove(&altera_gc->mmchip);
>  
>  	return -EIO;
>  }

Re: [PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

[Tien Hock Loh]

Acked-by: Tien Hock Loh <thloh@altera.com>

On Wed, 2015-06-17 at 20:59 +0900, Masahiro Yamada wrote:
> This driver calls of_mm_gpiochip_add() to add a memory mapped gpio
> chip.  So, of_mm_gpiochip_remove() should be used when removing it.
> 
> The direct call of gpiochip_remove() misses unmapping the register
> and freeing the label.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/gpio/gpio-altera.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-altera.c b/drivers/gpio/gpio-altera.c
> index 449fb46..c653c83 100644
> --- a/drivers/gpio/gpio-altera.c
> +++ b/drivers/gpio/gpio-altera.c
> @@ -337,7 +337,7 @@ static int altera_gpio_remove(struct platform_device *pdev)
>  {
>  	struct altera_gpio_chip *altera_gc = platform_get_drvdata(pdev);
>  
> -	gpiochip_remove(&altera_gc->mmchip.gc);
> +	of_mm_gpiochip_remove(&altera_gc->mmchip);
>  
>  	return -EIO;
>  }

Re: [PATCH 1/2] gpio: altera: use of_mm_gpiochip_remove() to fix memory leak

[Linus Walleij]

On Wed, Jun 17, 2015 at 1:59 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:

> This driver calls of_mm_gpiochip_add() to add a memory mapped gpio
> chip.  So, of_mm_gpiochip_remove() should be used when removing it.
>
> The direct call of gpiochip_remove() misses unmapping the register
> and freeing the label.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Patch applied with the ACKs.

Yours,
Linus Walleij