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 <[email protected]>
---
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
The remove callback never succeeds, which seems odd.
Signed-off-by: Masahiro Yamada <[email protected]>
---
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
On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
Reviewed-by: Alexandre Courbot <[email protected]>
On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
<[email protected]> wrote:
> The remove callback never succeeds, which seems odd.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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?
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
> <[email protected]> wrote:
> > The remove callback never succeeds, which seems odd.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > 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?
On Mon, Jun 22, 2015 at 10:36 AM, Tien Hock Loh <[email protected]> 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 <[email protected]>
>
> On Sun, 2015-06-21 at 16:25 +0900, Alexandre Courbot wrote:
>> On Wed, Jun 17, 2015 at 8:59 PM, Masahiro Yamada
>> <[email protected]> wrote:
>> > The remove callback never succeeds, which seems odd.
>> >
>> > Signed-off-by: Masahiro Yamada <[email protected]>
>> > ---
>> >
>> > 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?
>
Hi Tien,
2015-06-22 10:36 GMT+09:00 Tien Hock Loh <[email protected]>:
> 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
Acked-by: Tien Hock Loh <[email protected]>
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 <[email protected]>
> ---
>
> 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[] = {
Acked-by: Tien Hock Loh <[email protected]>
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 <[email protected]>
> ---
>
> 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;
> }
On Wed, Jun 17, 2015 at 1:59 PM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
Patch applied with the ACKs.
Yours,
Linus Walleij