* [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16
[not found] <1421581173-28416-1-git-send-email-ricardo.ribalda@gmail.com>
@ 2015-01-18 11:39 ` Ricardo Ribalda Delgado
[not found] ` <1421581173-28416-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[not found] ` <1421581173-28416-1-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 13+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-01-18 11:39 UTC (permalink / raw)
To: linux-gpio, linux-kernel
Cc: Ricardo Ribalda Delgado, Linus Walleij, Alexandre Courbot,
Grant Likely, Rob Herring, John Crispin, devicetree
Instead of parsing manually the shadow content, use the much simpler
helper of_property_read_u16.
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Crispin <blogic@openwrt.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
drivers/gpio/gpio-mm-lantiq.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index 7d55161..b511a6b 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
static int ltq_mm_probe(struct platform_device *pdev)
{
struct ltq_mm *chip;
- const __be32 *shadow;
chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
if (!chip)
@@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
chip->mmchip.save_regs = ltq_mm_save_regs;
/* store the shadow value if one was passed by the devicetree */
- shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
- if (shadow)
- chip->shadow = be32_to_cpu(*shadow);
+ of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
+ &chip->shadow);
return of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
[not found] ` <1421581173-28416-1-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2015-01-18 11:39 ` Ricardo Ribalda Delgado
2015-01-19 22:36 ` Peter Korsgaard
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-01-18 11:39 UTC (permalink / raw)
To: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ricardo Ribalda Delgado, Linus Walleij, Alexandre Courbot,
Grant Likely, Rob Herring, Peter Korsgaard,
devicetree-u79uwXL29TY76Z2rM5mHXA
This way we do not need to transverse the device tree manually.
Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Peter Korsgaard <jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
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 <linux/of.h>
#include <linux/of_gpio.h>
#include <linux/of_irq.h>
+#include <linux/of_platform.h>
#include <linux/gpio.h>
#include <linux/slab.h>
#include <linux/irq.h>
@@ -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;
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;
id = of_match_node(mpc8xxx_gpio_ids, np);
if (id)
@@ -397,24 +397,20 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
irq_set_handler_data(hwirq, mpc8xxx_gc);
irq_set_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade);
-skip_irq:
- return;
-
-err:
- pr_err("%s: registration failed with status %d\n",
- np->full_name, ret);
- kfree(mpc8xxx_gc);
-
- return;
+ return 0;
}
-static int __init mpc8xxx_add_gpiochips(void)
-{
- struct device_node *np;
-
- for_each_matching_node(np, mpc8xxx_gpio_ids)
- mpc8xxx_add_controller(np);
+static struct platform_driver mpc8xxx_plat_driver = {
+ .probe = mpc8xxx_probe,
+ .driver = {
+ .name = "gpio-mpc8xxx",
+ .of_match_table = mpc8xxx_gpio_ids,
+ },
+};
- return 0;
+static int __init mpc8xxx_init(void)
+{
+ return platform_driver_register(&mpc8xxx_plat_driver);
}
-arch_initcall(mpc8xxx_add_gpiochips);
+
+arch_initcall(mpc8xxx_init);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16
[not found] ` <1421581173-28416-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2015-01-19 10:57 ` Mark Rutland
2015-01-19 11:27 ` [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32 Ricardo Ribalda Delgado
2015-01-19 11:28 ` [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16 Ricardo Ribalda Delgado
0 siblings, 2 replies; 13+ messages in thread
From: Mark Rutland @ 2015-01-19 10:57 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linus Walleij, Alexandre Courbot,
grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Rob Herring,
John Crispin, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On Sun, Jan 18, 2015 at 11:39:29AM +0000, Ricardo Ribalda Delgado wrote:
> Instead of parsing manually the shadow content, use the much simpler
> helper of_property_read_u16.
>
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: John Crispin <blogic-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/gpio/gpio-mm-lantiq.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
> index 7d55161..b511a6b 100644
> --- a/drivers/gpio/gpio-mm-lantiq.c
> +++ b/drivers/gpio/gpio-mm-lantiq.c
> @@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
> static int ltq_mm_probe(struct platform_device *pdev)
> {
> struct ltq_mm *chip;
> - const __be32 *shadow;
>
> chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> if (!chip)
> @@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
> chip->mmchip.save_regs = ltq_mm_save_regs;
>
> /* store the shadow value if one was passed by the devicetree */
> - shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
> - if (shadow)
> - chip->shadow = be32_to_cpu(*shadow);
> + of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
> + &chip->shadow);
The old code and new code aren't equivalent:
The binding doesn't specify the use of /bits/ 16 <...>, nor does the
only in-tree dts use it:
arch/mips/boot/dts/easy50712.dts: lantiq,shadow = <0xfff>;
So in the DTB, lantiq-shadow will be stored as a be32 value. The byte
stream will look like: [00 00 0f ff].
The existing code reads all 4 bytes, and converts them as required,
reading the expected value of 0xfff. If you use of_property_read_u16,
this will only take the first 2 bytes, and will incorrectly read 0.
You can instead use of_property_read_u32, which will function
equivalently to the existing code.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32
2015-01-19 10:57 ` Mark Rutland
@ 2015-01-19 11:27 ` Ricardo Ribalda Delgado
2015-01-20 10:10 ` Linus Walleij
2015-01-19 11:28 ` [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16 Ricardo Ribalda Delgado
1 sibling, 1 reply; 13+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-01-19 11:27 UTC (permalink / raw)
To: linux-gpio, linux-kernel
Cc: Ricardo Ribalda Delgado, Linus Walleij, Alexandre Courbot,
Grant Likely, Rob Herring, John Crispin, devicetree, Mark Rutland
Instead of parsing manually the shadow content, use the much simpler
helper of_property_read_u32.
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Crispin <blogic@openwrt.org>
Cc: devicetree@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
v2: Reported by Mark Rutland <mark.rutland@arm.com>
-use of_property_read_u32 instead of u16
drivers/gpio/gpio-mm-lantiq.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
index 7d55161..95ff180 100644
--- a/drivers/gpio/gpio-mm-lantiq.c
+++ b/drivers/gpio/gpio-mm-lantiq.c
@@ -105,7 +105,7 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
static int ltq_mm_probe(struct platform_device *pdev)
{
struct ltq_mm *chip;
- const __be32 *shadow;
+ u32 shadow;
chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
if (!chip)
@@ -117,9 +117,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
chip->mmchip.save_regs = ltq_mm_save_regs;
/* store the shadow value if one was passed by the devicetree */
- shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
- if (shadow)
- chip->shadow = be32_to_cpu(*shadow);
+ if (!of_property_read_u32(pdev->dev.of_node, "lantiq,shadow", &shadow))
+ chip->shadow = shadow;
return of_mm_gpiochip_add(pdev->dev.of_node, &chip->mmchip);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16
2015-01-19 10:57 ` Mark Rutland
2015-01-19 11:27 ` [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32 Ricardo Ribalda Delgado
@ 2015-01-19 11:28 ` Ricardo Ribalda Delgado
1 sibling, 0 replies; 13+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-01-19 11:28 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Linus Walleij, Alexandre Courbot, grant.likely@linaro.org,
Rob Herring, John Crispin, devicetree@vger.kernel.org
Hello Mark
Thanks for noticing. I have just sent the v2 of the patch.
Regards!
On Mon, Jan 19, 2015 at 11:57 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Sun, Jan 18, 2015 at 11:39:29AM +0000, Ricardo Ribalda Delgado wrote:
>> Instead of parsing manually the shadow content, use the much simpler
>> helper of_property_read_u16.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Alexandre Courbot <gnurou@gmail.com>
>> Cc: Grant Likely <grant.likely@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: John Crispin <blogic@openwrt.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>> drivers/gpio/gpio-mm-lantiq.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mm-lantiq.c b/drivers/gpio/gpio-mm-lantiq.c
>> index 7d55161..b511a6b 100644
>> --- a/drivers/gpio/gpio-mm-lantiq.c
>> +++ b/drivers/gpio/gpio-mm-lantiq.c
>> @@ -105,7 +105,6 @@ static void ltq_mm_save_regs(struct of_mm_gpio_chip *mm_gc)
>> static int ltq_mm_probe(struct platform_device *pdev)
>> {
>> struct ltq_mm *chip;
>> - const __be32 *shadow;
>>
>> chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>> if (!chip)
>> @@ -117,9 +116,8 @@ static int ltq_mm_probe(struct platform_device *pdev)
>> chip->mmchip.save_regs = ltq_mm_save_regs;
>>
>> /* store the shadow value if one was passed by the devicetree */
>> - shadow = of_get_property(pdev->dev.of_node, "lantiq,shadow", NULL);
>> - if (shadow)
>> - chip->shadow = be32_to_cpu(*shadow);
>> + of_property_read_u16(pdev->dev.of_node, "lantiq,shadow",
>> + &chip->shadow);
>
> The old code and new code aren't equivalent:
>
> The binding doesn't specify the use of /bits/ 16 <...>, nor does the
> only in-tree dts use it:
>
> arch/mips/boot/dts/easy50712.dts: lantiq,shadow = <0xfff>;
>
> So in the DTB, lantiq-shadow will be stored as a be32 value. The byte
> stream will look like: [00 00 0f ff].
>
> The existing code reads all 4 bytes, and converts them as required,
> reading the expected value of 0xfff. If you use of_property_read_u16,
> this will only take the first 2 bytes, and will incorrectly read 0.
>
> You can instead use of_property_read_u32, which will function
> equivalently to the existing code.
>
> Thanks,
> Mark.
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-18 11:39 ` [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface Ricardo Ribalda Delgado
@ 2015-01-19 22:36 ` Peter Korsgaard
2015-01-19 22:48 ` Peter Korsgaard
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Peter Korsgaard @ 2015-01-19 22:36 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio, linux-kernel, Linus Walleij, Alexandre Courbot,
Grant Likely, Rob Herring, Peter Korsgaard, devicetree
>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:
> This way we do not need to transverse the device tree manually.
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Looks sensible.
Acked-by: Peter Korsgaard <peter@korsgaard.com>
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-18 11:39 ` [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface Ricardo Ribalda Delgado
2015-01-19 22:36 ` Peter Korsgaard
@ 2015-01-19 22:48 ` Peter Korsgaard
2015-01-19 23:37 ` Ricardo Ribalda Delgado
2015-01-20 2:03 ` Alexandre Courbot
2015-01-20 10:15 ` Linus Walleij
3 siblings, 1 reply; 13+ messages in thread
From: Peter Korsgaard @ 2015-01-19 22:48 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio, linux-kernel, Linus Walleij, Alexandre Courbot,
Grant Likely, Rob Herring, Peter Korsgaard, devicetree
>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:
> This way we do not need to transverse the device tree manually.
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> 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 <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> #include <linux/gpio.h>
> #include <linux/slab.h>
> #include <linux/irq.h>
> @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-19 22:48 ` Peter Korsgaard
@ 2015-01-19 23:37 ` Ricardo Ribalda Delgado
2015-01-20 10:41 ` Peter Korsgaard
0 siblings, 1 reply; 13+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-01-19 23:37 UTC (permalink / raw)
To: Peter Korsgaard
Cc: linux-gpio@vger.kernel.org, LKML, Linus Walleij,
Alexandre Courbot, Grant Likely, Rob Herring, Peter Korsgaard,
devicetree@vger.kernel.org
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 <peter@korsgaard.com> wrote:
>>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes:
>
> > This way we do not need to transverse the device tree manually.
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Alexandre Courbot <gnurou@gmail.com>
> > Cc: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Peter Korsgaard <jacmet@sunsite.dk>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > ---
> > 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 <linux/of.h>
> > #include <linux/of_gpio.h>
> > #include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > #include <linux/gpio.h>
> > #include <linux/slab.h>
> > #include <linux/irq.h>
> > @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-18 11:39 ` [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface Ricardo Ribalda Delgado
2015-01-19 22:36 ` Peter Korsgaard
2015-01-19 22:48 ` Peter Korsgaard
@ 2015-01-20 2:03 ` Alexandre Courbot
2015-01-20 10:15 ` Linus Walleij
3 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2015-01-20 2:03 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio@vger.kernel.org, Linux Kernel Mailing List,
Linus Walleij, Grant Likely, Rob Herring, Peter Korsgaard,
devicetree@vger.kernel.org
On Sun, Jan 18, 2015 at 8:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> This way we do not need to transverse the device tree manually.
... and this makes things much more legible.
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32
2015-01-19 11:27 ` [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32 Ricardo Ribalda Delgado
@ 2015-01-20 10:10 ` Linus Walleij
0 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2015-01-20 10:10 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Alexandre Courbot, Grant Likely, Rob Herring, John Crispin,
devicetree@vger.kernel.org, Mark Rutland
On Mon, Jan 19, 2015 at 12:27 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Instead of parsing manually the shadow content, use the much simpler
> helper of_property_read_u32.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: John Crispin <blogic@openwrt.org>
> Cc: devicetree@vger.kernel.org
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>
> v2: Reported by Mark Rutland <mark.rutland@arm.com>
> -use of_property_read_u32 instead of u16
This v2 version applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-18 11:39 ` [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface Ricardo Ribalda Delgado
` (2 preceding siblings ...)
2015-01-20 2:03 ` Alexandre Courbot
@ 2015-01-20 10:15 ` Linus Walleij
3 siblings, 0 replies; 13+ messages in thread
From: Linus Walleij @ 2015-01-20 10:15 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Alexandre Courbot, Grant Likely, Rob Herring, Peter Korsgaard,
devicetree@vger.kernel.org
On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> This way we do not need to transverse the device tree manually.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Peter Korsgaard <jacmet@sunsite.dk>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Patch applied with the ACKs, hope nothing explodes.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-19 23:37 ` Ricardo Ribalda Delgado
@ 2015-01-20 10:41 ` Peter Korsgaard
2015-01-20 10:54 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 13+ messages in thread
From: Peter Korsgaard @ 2015-01-20 10:41 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: linux-gpio@vger.kernel.org, LKML, Linus Walleij,
Alexandre Courbot, Grant Likely, Rob Herring, Peter Korsgaard,
devicetree@vger.kernel.org
>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface.
2015-01-20 10:41 ` Peter Korsgaard
@ 2015-01-20 10:54 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 13+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-01-20 10:54 UTC (permalink / raw)
To: Peter Korsgaard
Cc: linux-gpio@vger.kernel.org, LKML, Linus Walleij,
Alexandre Courbot, Grant Likely, Rob Herring, Peter Korsgaard,
devicetree@vger.kernel.org
No worries :)
On Tue, Jan 20, 2015 at 11:41 AM, Peter Korsgaard <peter@korsgaard.com> wrote:
>>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-01-20 10:54 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1421581173-28416-1-git-send-email-ricardo.ribalda@gmail.com>
2015-01-18 11:39 ` [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16 Ricardo Ribalda Delgado
[not found] ` <1421581173-28416-5-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-01-19 10:57 ` Mark Rutland
2015-01-19 11:27 ` [PATCH v2 4/8] gpio-mm-lantiq: Use of_property_read_u32 Ricardo Ribalda Delgado
2015-01-20 10:10 ` Linus Walleij
2015-01-19 11:28 ` [PATCH 4/8] gpio-mm-lantiq: Use of_propery_read_u16 Ricardo Ribalda Delgado
[not found] ` <1421581173-28416-1-git-send-email-ricardo.ribalda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-01-18 11:39 ` [PATCH 7/8] gpio/mpc8xxx: Convert to platform device interface Ricardo Ribalda Delgado
2015-01-19 22:36 ` Peter Korsgaard
2015-01-19 22:48 ` Peter Korsgaard
2015-01-19 23:37 ` Ricardo Ribalda Delgado
2015-01-20 10:41 ` Peter Korsgaard
2015-01-20 10:54 ` Ricardo Ribalda Delgado
2015-01-20 2:03 ` Alexandre Courbot
2015-01-20 10:15 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).