Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
No worries :) On Tue, Jan 20, 2015 at 11:41 AM, Peter Korsgaard wrote: >> "Ricardo" == Ricardo Ribalda Delgado writes: > > > Hello Peter > > I thought the logic behind the original driver was: > > > 1) Create gpiochip so it can be used by other modules/userland > > 2) if there is also a irq available for it, create a irqdomain. > > otherwise continue. > > Indeed - Sorry, I must have been confused yesterday. > > -- > Bye, Peter Korsgaard -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
> "Ricardo" == Ricardo Ribalda Delgado writes: > Hello Peter > I thought the logic behind the original driver was: > 1) Create gpiochip so it can be used by other modules/userland > 2) if there is also a irq available for it, create a irqdomain. > otherwise continue. Indeed - Sorry, I must have been confused yesterday. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado wrote: > This way we do not need to transverse the device tree manually. > > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Grant Likely > Cc: Rob Herring > Cc: Peter Korsgaard > Cc: devicet...@vger.kernel.org > Signed-off-by: Ricardo Ribalda Delgado Patch applied with the ACKs, hope nothing explodes. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
On Sun, Jan 18, 2015 at 8:39 PM, Ricardo Ribalda Delgado wrote: > This way we do not need to transverse the device tree manually. ... and this makes things much more legible. Acked-by: Alexandre Courbot -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
Hello Peter
I thought the logic behind the original driver was:
1) Create gpiochip so it can be used by other modules/userland
2) if there is also a irq available for it, create a irqdomain.
otherwise continue.
I can change the code so it does not continue, and exit cleanly if the
irqdomain fails.
You are the original developer of the driver. So: do you think it
makes sense to continue only as a gpiochip? :P
If I need to change it I rather add it as a another patch to the
patchset, so if it causes an issue somewhere it can be found easier
via bisect.
Regards!
On Mon, Jan 19, 2015 at 11:48 PM, Peter Korsgaard wrote:
>> "Ricardo" == Ricardo Ribalda Delgado writes:
>
> > This way we do not need to transverse the device tree manually.
> > Cc: Linus Walleij
> > Cc: Alexandre Courbot
> > Cc: Grant Likely
> > Cc: Rob Herring
> > Cc: Peter Korsgaard
> > Cc: devicet...@vger.kernel.org
> > Signed-off-by: Ricardo Ribalda Delgado
> > ---
> > drivers/gpio/gpio-mpc8xxx.c | 48
> +
> > 1 file changed, 22 insertions(+), 26 deletions(-)
>
> > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> > index d1ff879..57eb794 100644
> > --- a/drivers/gpio/gpio-mpc8xxx.c
> > +++ b/drivers/gpio/gpio-mpc8xxx.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -342,8 +343,9 @@ static struct of_device_id mpc8xxx_gpio_ids[]
> __initdata = {
> > {}
> > };
>
> > -static void __init mpc8xxx_add_controller(struct device_node *np)
> > +static int mpc8xxx_probe(struct platform_device *pdev)
> > {
> > +struct device_node *np = pdev->dev.of_node;
> > struct mpc8xxx_gpio_chip *mpc8xxx_gc;
> > struct of_mm_gpio_chip *mm_gc;
> > struct gpio_chip *gc;
> > @@ -351,11 +353,9 @@ static void __init mpc8xxx_add_controller(struct
> device_node *np)
> > unsigned hwirq;
> > int ret;
>
> > -mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
> > -if (!mpc8xxx_gc) {
> > -ret = -ENOMEM;
> > -goto err;
> > -}
> > +mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc),
> GFP_KERNEL);
> > +if (!mpc8xxx_gc)
> > +return -ENOMEM;
>
> > spin_lock_init(&mpc8xxx_gc->lock);
>
> > @@ -375,16 +375,16 @@ static void __init mpc8xxx_add_controller(struct
> device_node *np)
>
> > ret = of_mm_gpiochip_add(np, mm_gc);
> > if (ret)
> > -goto err;
> > +return ret;
>
> > hwirq = irq_of_parse_and_map(np, 0);
> > if (hwirq == NO_IRQ)
> > -goto skip_irq;
> > +return 0;
>
> Actually, looking closer - Isn't this leaking mm_gc? It was as well
> before, but it would be good to get it fixed.
>
>
> > mpc8xxx_gc-> irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
> > &mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
> > if (!mpc8xxx_gc->irq)
> > -goto skip_irq;
> > +return 0;
>
> And here as well?
>
>
> --
> Bye, Peter Korsgaard
--
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
> "Ricardo" == Ricardo Ribalda Delgado writes:
> This way we do not need to transverse the device tree manually.
> Cc: Linus Walleij
> Cc: Alexandre Courbot
> Cc: Grant Likely
> Cc: Rob Herring
> Cc: Peter Korsgaard
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado
> ---
> drivers/gpio/gpio-mpc8xxx.c | 48
> +
> 1 file changed, 22 insertions(+), 26 deletions(-)
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index d1ff879..57eb794 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -342,8 +343,9 @@ static struct of_device_id mpc8xxx_gpio_ids[] __initdata
> = {
> {}
> };
> -static void __init mpc8xxx_add_controller(struct device_node *np)
> +static int mpc8xxx_probe(struct platform_device *pdev)
> {
> +struct device_node *np = pdev->dev.of_node;
> struct mpc8xxx_gpio_chip *mpc8xxx_gc;
> struct of_mm_gpio_chip *mm_gc;
> struct gpio_chip *gc;
> @@ -351,11 +353,9 @@ static void __init mpc8xxx_add_controller(struct
> device_node *np)
> unsigned hwirq;
> int ret;
> -mpc8xxx_gc = kzalloc(sizeof(*mpc8xxx_gc), GFP_KERNEL);
> -if (!mpc8xxx_gc) {
> -ret = -ENOMEM;
> -goto err;
> -}
> +mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL);
> +if (!mpc8xxx_gc)
> +return -ENOMEM;
> spin_lock_init(&mpc8xxx_gc->lock);
> @@ -375,16 +375,16 @@ static void __init mpc8xxx_add_controller(struct
> device_node *np)
> ret = of_mm_gpiochip_add(np, mm_gc);
> if (ret)
> -goto err;
> +return ret;
> hwirq = irq_of_parse_and_map(np, 0);
> if (hwirq == NO_IRQ)
> -goto skip_irq;
> +return 0;
Actually, looking closer - Isn't this leaking mm_gc? It was as well
before, but it would be good to get it fixed.
> mpc8xxx_gc-> irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS,
> &mpc8xxx_gpio_irq_ops, mpc8xxx_gc);
> if (!mpc8xxx_gc->irq)
> -goto skip_irq;
> +return 0;
And here as well?
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
> "Ricardo" == Ricardo Ribalda Delgado writes: > This way we do not need to transverse the device tree manually. > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Grant Likely > Cc: Rob Herring > Cc: Peter Korsgaard > Cc: devicet...@vger.kernel.org > Signed-off-by: Ricardo Ribalda Delgado Looks sensible. Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
