Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
Hi Tony, Mark, Ezequiel,
Tony Lindgren t...@atomide.com wrote:
* Ezequiel Garcia elezegar...@gmail.com [130121 09:00]:
Hi Mark,
On Mon, Jan 21, 2013 at 9:30 AM, Mark Rutland mark.rutl...@arm.com
wrote:
[...]
diff --git a/arch/arm/mach-omap2/gpmc.c
b/arch/arm/mach-omap2/gpmc.c
index 01ce462..f7de9eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
#include omap_device.h
#include gpmc.h
#include gpmc-nand.h
+#include gpmc-onenand.h
#define DEVICE_NAME omap-gpmc
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct
platform_device *pdev,
}
#endif
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ u32 val;
+ struct omap_onenand_platform_data *gpmc_onenand_data;
+
+ if (of_property_read_u32(child, reg, val) 0) {
+ dev_err(pdev-dev, %s has no 'reg' property\n,
+ child-full_name);
+ return -ENODEV;
+ }
+
+ gpmc_onenand_data = devm_kzalloc(pdev-dev,
sizeof(*gpmc_onenand_data),
+ GFP_KERNEL);
+ if (!gpmc_onenand_data)
+ return -ENOMEM;
+
+ gpmc_onenand_data-cs = val;
+ gpmc_onenand_data-of_node = child;
+ gpmc_onenand_data-dma_channel = -1;
+
+ if (!of_property_read_u32(child, dma-channel, val))
+ gpmc_onenand_data-dma_channel = val;
+
+ gpmc_onenand_init(gpmc_onenand_data);
+
+ return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ return 0;
+}
+#endif
+
static int gpmc_probe_dt(struct platform_device *pdev)
{
int ret;
@@ -1276,6 +1314,12 @@ static int gpmc_probe_dt(struct
platform_device *pdev)
return ret;
}
This doesn't look right to me:
+ for_each_node_by_name(child, onenand) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ of_node_put(child);
+ if (ret 0)
+ return ret;
+ }
for_each_node_by_name automatically calls of_node_put on each node
once passed,
and as far as I can tell, gpmc_probe_onenand_child doesn't do
anything that'd
increment a node's refcount.
As far as I can see, you only need the of_node_put in the error
case:
for_each_node_by_name(child, onenand) {
ret = gpmc_probe_onenand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
Have I missed something here?
Mmm... perhaps I've overlooked that code.
After some digging through source and reading for_each_node_by_name()
it seems to me you're right.
@Daniel: It seems this would also apply to the NAND binding.
What do you think?
Would prefer this done as a fix against the omap-for-v3.9/gpmc
branch before we apply Ezequiel's patches.
I'm currently far away from my computer and can't prepare a patch for this,
sorry. But I think you are right, so please just submit a patch for that,
anyone :-)
Best regards,
Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
On Mon, Jan 21, 2013 at 10:32 PM, Daniel Mack zon...@gmail.com wrote:
Hi Tony, Mark, Ezequiel,
Tony Lindgren t...@atomide.com wrote:
* Ezequiel Garcia elezegar...@gmail.com [130121 09:00]:
Hi Mark,
On Mon, Jan 21, 2013 at 9:30 AM, Mark Rutland mark.rutl...@arm.com
wrote:
[...]
diff --git a/arch/arm/mach-omap2/gpmc.c
b/arch/arm/mach-omap2/gpmc.c
index 01ce462..f7de9eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
#include omap_device.h
#include gpmc.h
#include gpmc-nand.h
+#include gpmc-onenand.h
#define DEVICE_NAME omap-gpmc
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct
platform_device *pdev,
}
#endif
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ u32 val;
+ struct omap_onenand_platform_data *gpmc_onenand_data;
+
+ if (of_property_read_u32(child, reg, val) 0) {
+ dev_err(pdev-dev, %s has no 'reg' property\n,
+ child-full_name);
+ return -ENODEV;
+ }
+
+ gpmc_onenand_data = devm_kzalloc(pdev-dev,
sizeof(*gpmc_onenand_data),
+ GFP_KERNEL);
+ if (!gpmc_onenand_data)
+ return -ENOMEM;
+
+ gpmc_onenand_data-cs = val;
+ gpmc_onenand_data-of_node = child;
+ gpmc_onenand_data-dma_channel = -1;
+
+ if (!of_property_read_u32(child, dma-channel, val))
+ gpmc_onenand_data-dma_channel = val;
+
+ gpmc_onenand_init(gpmc_onenand_data);
+
+ return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ return 0;
+}
+#endif
+
static int gpmc_probe_dt(struct platform_device *pdev)
{
int ret;
@@ -1276,6 +1314,12 @@ static int gpmc_probe_dt(struct
platform_device *pdev)
return ret;
}
This doesn't look right to me:
+ for_each_node_by_name(child, onenand) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ of_node_put(child);
+ if (ret 0)
+ return ret;
+ }
for_each_node_by_name automatically calls of_node_put on each node
once passed,
and as far as I can tell, gpmc_probe_onenand_child doesn't do
anything that'd
increment a node's refcount.
As far as I can see, you only need the of_node_put in the error
case:
for_each_node_by_name(child, onenand) {
ret = gpmc_probe_onenand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
Have I missed something here?
Mmm... perhaps I've overlooked that code.
After some digging through source and reading for_each_node_by_name()
it seems to me you're right.
@Daniel: It seems this would also apply to the NAND binding.
What do you think?
Would prefer this done as a fix against the omap-for-v3.9/gpmc
branch before we apply Ezequiel's patches.
I'm currently far away from my computer and can't prepare a patch for this,
sorry. But I think you are right, so please just submit a patch for that,
anyone :-)
Ok, I'll try to submit a patch as soon as possible. If anyone wants to
do it instead, fine by me.
--
Ezequiel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
* Ezequiel Garcia elezegar...@gmail.com [130122 10:17]: On Mon, Jan 21, 2013 at 10:32 PM, Daniel Mack zon...@gmail.com wrote: I'm currently far away from my computer and can't prepare a patch for this, sorry. But I think you are right, so please just submit a patch for that, anyone :-) Ok, I'll try to submit a patch as soon as possible. If anyone wants to do it instead, fine by me. No please go ahead as it seems that you can easily test it too. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
On Tue, Jan 22, 2013 at 3:27 PM, Tony Lindgren t...@atomide.com wrote:
* Ezequiel Garcia elezegar...@gmail.com [130122 10:17]:
On Mon, Jan 21, 2013 at 10:32 PM, Daniel Mack zon...@gmail.com wrote:
I'm currently far away from my computer and can't prepare a patch for
this, sorry. But I think you are right, so please just submit a patch for
that, anyone :-)
Ok, I'll try to submit a patch as soon as possible. If anyone wants to
do it instead, fine by me.
No please go ahead as it seems that you can easily test it too.
No problem.
I now wonder if it's okey to exit upon probe failure.
In particular, the for_each should be like this:
for_each_node_by_name(child, nand) {
ret = gpmc_probe_nand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
or like this:
for_each_node_by_name(child, nand) {
ret = gpmc_probe_nand_child(pdev, child);
WARN_ON(ret 0);
}
Ideas?
--
Ezequiel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
* Ezequiel Garcia elezegar...@gmail.com [130122 11:46]:
On Tue, Jan 22, 2013 at 3:27 PM, Tony Lindgren t...@atomide.com wrote:
* Ezequiel Garcia elezegar...@gmail.com [130122 10:17]:
On Mon, Jan 21, 2013 at 10:32 PM, Daniel Mack zon...@gmail.com wrote:
I'm currently far away from my computer and can't prepare a patch for
this, sorry. But I think you are right, so please just submit a patch
for that, anyone :-)
Ok, I'll try to submit a patch as soon as possible. If anyone wants to
do it instead, fine by me.
No please go ahead as it seems that you can easily test it too.
No problem.
I now wonder if it's okey to exit upon probe failure.
In particular, the for_each should be like this:
for_each_node_by_name(child, nand) {
ret = gpmc_probe_nand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
or like this:
for_each_node_by_name(child, nand) {
ret = gpmc_probe_nand_child(pdev, child);
WARN_ON(ret 0);
}
Ideas?
Well I would return and make sure the resources are freed.
However, if this relates to using bootloader configured values
for the few cases where we don't have the timing information
for calculations available, then just doing a warning is
the way to go.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
[...]
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 01ce462..f7de9eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
#include omap_device.h
#include gpmc.h
#include gpmc-nand.h
+#include gpmc-onenand.h
#define DEVICE_NAME omap-gpmc
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct
platform_device *pdev,
}
#endif
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ u32 val;
+ struct omap_onenand_platform_data *gpmc_onenand_data;
+
+ if (of_property_read_u32(child, reg, val) 0) {
+ dev_err(pdev-dev, %s has no 'reg' property\n,
+ child-full_name);
+ return -ENODEV;
+ }
+
+ gpmc_onenand_data = devm_kzalloc(pdev-dev, sizeof(*gpmc_onenand_data),
+ GFP_KERNEL);
+ if (!gpmc_onenand_data)
+ return -ENOMEM;
+
+ gpmc_onenand_data-cs = val;
+ gpmc_onenand_data-of_node = child;
+ gpmc_onenand_data-dma_channel = -1;
+
+ if (!of_property_read_u32(child, dma-channel, val))
+ gpmc_onenand_data-dma_channel = val;
+
+ gpmc_onenand_init(gpmc_onenand_data);
+
+ return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ return 0;
+}
+#endif
+
static int gpmc_probe_dt(struct platform_device *pdev)
{
int ret;
@@ -1276,6 +1314,12 @@ static int gpmc_probe_dt(struct platform_device *pdev)
return ret;
}
This doesn't look right to me:
+ for_each_node_by_name(child, onenand) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ of_node_put(child);
+ if (ret 0)
+ return ret;
+ }
for_each_node_by_name automatically calls of_node_put on each node once passed,
and as far as I can tell, gpmc_probe_onenand_child doesn't do anything that'd
increment a node's refcount.
As far as I can see, you only need the of_node_put in the error case:
for_each_node_by_name(child, onenand) {
ret = gpmc_probe_onenand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
Have I missed something here?
return 0;
}
#else
--
1.7.8.6
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
Hi Mark,
On Mon, Jan 21, 2013 at 9:30 AM, Mark Rutland mark.rutl...@arm.com wrote:
[...]
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 01ce462..f7de9eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
#include omap_device.h
#include gpmc.h
#include gpmc-nand.h
+#include gpmc-onenand.h
#define DEVICE_NAME omap-gpmc
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct
platform_device *pdev,
}
#endif
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ u32 val;
+ struct omap_onenand_platform_data *gpmc_onenand_data;
+
+ if (of_property_read_u32(child, reg, val) 0) {
+ dev_err(pdev-dev, %s has no 'reg' property\n,
+ child-full_name);
+ return -ENODEV;
+ }
+
+ gpmc_onenand_data = devm_kzalloc(pdev-dev,
sizeof(*gpmc_onenand_data),
+ GFP_KERNEL);
+ if (!gpmc_onenand_data)
+ return -ENOMEM;
+
+ gpmc_onenand_data-cs = val;
+ gpmc_onenand_data-of_node = child;
+ gpmc_onenand_data-dma_channel = -1;
+
+ if (!of_property_read_u32(child, dma-channel, val))
+ gpmc_onenand_data-dma_channel = val;
+
+ gpmc_onenand_init(gpmc_onenand_data);
+
+ return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ return 0;
+}
+#endif
+
static int gpmc_probe_dt(struct platform_device *pdev)
{
int ret;
@@ -1276,6 +1314,12 @@ static int gpmc_probe_dt(struct platform_device *pdev)
return ret;
}
This doesn't look right to me:
+ for_each_node_by_name(child, onenand) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ of_node_put(child);
+ if (ret 0)
+ return ret;
+ }
for_each_node_by_name automatically calls of_node_put on each node once
passed,
and as far as I can tell, gpmc_probe_onenand_child doesn't do anything that'd
increment a node's refcount.
As far as I can see, you only need the of_node_put in the error case:
for_each_node_by_name(child, onenand) {
ret = gpmc_probe_onenand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
Have I missed something here?
Mmm... perhaps I've overlooked that code.
After some digging through source and reading for_each_node_by_name()
it seems to me you're right.
@Daniel: It seems this would also apply to the NAND binding.
What do you think?
--
Ezequiel
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] arm: omap2: gpmc: add DT bindings for OneNAND
* Ezequiel Garcia elezegar...@gmail.com [130121 09:00]:
Hi Mark,
On Mon, Jan 21, 2013 at 9:30 AM, Mark Rutland mark.rutl...@arm.com wrote:
[...]
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 01ce462..f7de9eb 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
#include omap_device.h
#include gpmc.h
#include gpmc-nand.h
+#include gpmc-onenand.h
#define DEVICE_NAME omap-gpmc
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct
platform_device *pdev,
}
#endif
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ u32 val;
+ struct omap_onenand_platform_data *gpmc_onenand_data;
+
+ if (of_property_read_u32(child, reg, val) 0) {
+ dev_err(pdev-dev, %s has no 'reg' property\n,
+ child-full_name);
+ return -ENODEV;
+ }
+
+ gpmc_onenand_data = devm_kzalloc(pdev-dev,
sizeof(*gpmc_onenand_data),
+ GFP_KERNEL);
+ if (!gpmc_onenand_data)
+ return -ENOMEM;
+
+ gpmc_onenand_data-cs = val;
+ gpmc_onenand_data-of_node = child;
+ gpmc_onenand_data-dma_channel = -1;
+
+ if (!of_property_read_u32(child, dma-channel, val))
+ gpmc_onenand_data-dma_channel = val;
+
+ gpmc_onenand_init(gpmc_onenand_data);
+
+ return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+ struct device_node *child)
+{
+ return 0;
+}
+#endif
+
static int gpmc_probe_dt(struct platform_device *pdev)
{
int ret;
@@ -1276,6 +1314,12 @@ static int gpmc_probe_dt(struct platform_device
*pdev)
return ret;
}
This doesn't look right to me:
+ for_each_node_by_name(child, onenand) {
+ ret = gpmc_probe_onenand_child(pdev, child);
+ of_node_put(child);
+ if (ret 0)
+ return ret;
+ }
for_each_node_by_name automatically calls of_node_put on each node once
passed,
and as far as I can tell, gpmc_probe_onenand_child doesn't do anything
that'd
increment a node's refcount.
As far as I can see, you only need the of_node_put in the error case:
for_each_node_by_name(child, onenand) {
ret = gpmc_probe_onenand_child(pdev, child);
if (ret 0) {
of_node_put(child);
return ret;
}
}
Have I missed something here?
Mmm... perhaps I've overlooked that code.
After some digging through source and reading for_each_node_by_name()
it seems to me you're right.
@Daniel: It seems this would also apply to the NAND binding.
What do you think?
Would prefer this done as a fix against the omap-for-v3.9/gpmc
branch before we apply Ezequiel's patches.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
