Re: [PATCH v2 2/2] OMAP: omap_device: Add a method to build an omap_device from a DT node

2011-09-21 Thread Kevin Hilman 
Cousson, Benoit b-cous...@ti.com writes:

 On 9/17/2011 6:13 PM, Grant Likely wrote:
 On Fri, Sep 16, 2011 at 04:43:19PM +0200, Benoit Cousson wrote:

[...]

 +}
 +
 +static int _omap_device_notifier_call(struct notifier_block *nb,
 + unsigned long event, void *dev)

 Nit: Why the preceding underscore?  Generally that is only done for
 'special' variants of public functions.  ie. for a variant that
 expects a lock to already be held.

 Yeah, the convention in this file is not that strict, and it is used
 for internal static helper function as well.
 I'll let Kevin arbitrate that point :-)

The convention in this file is the leading '_' is used for internal
helper functions.

I'd prefer to keep it that way, and if we decide to change the coding
convention to match a coding convention elsewhere, we should do it all
at the same time.

Kevin
--
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 2/2] OMAP: omap_device: Add a method to build an omap_device from a DT node

2011-09-19 Thread Cousson, Benoit 

On 9/17/2011 6:13 PM, Grant Likely wrote:

On Fri, Sep 16, 2011 at 04:43:19PM +0200, Benoit Cousson wrote:

Add a notifier called during device_add phase. If an of_node is present,
retrieve the hwmod entry in order to populate properly the omap_device
structure.
For the moment the resource from the device-tree are overloaded.
DT does not support named resource yet, and thus, most driver
will not work without that information.

Add two helpers function to parse a property that contains multiple
strings.

Signed-off-by: Benoit Coussonb-cous...@ti.com
Cc: Kevin Hilmankhil...@ti.com
---
  arch/arm/plat-omap/omap_device.c |  144 ++
  1 files changed, 144 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index cac7b9a..da13630 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -85,6 +85,8 @@
  #includelinux/clk.h
  #includelinux/clkdev.h
  #includelinux/pm_runtime.h
+#includelinux/of.h
+#includelinux/notifier.h

  #includeplat/omap_device.h
  #includeplat/omap_hwmod.h
@@ -94,12 +96,15 @@
  #define USE_WAKEUP_LAT0
  #define IGNORE_WAKEUP_LAT 1

+#define MAX_HWMOD_NAME_SIZE32
+
  static int omap_device_register(struct platform_device *pdev);
  static int omap_early_device_register(struct platform_device *pdev);
  static struct omap_device *omap_device_alloc(struct platform_device *pdev,
  struct omap_hwmod **ohs, int oh_cnt,
  struct omap_device_pm_latency *pm_lats,
  int pm_lats_cnt);
+static void omap_device_delete(struct omap_device *od);


  static struct omap_device_pm_latency omap_default_latency[] = {
@@ -315,6 +320,138 @@ static void _add_hwmod_clocks_clkdev(struct omap_device 
*od,
_add_clkdev(od, oh-opt_clks[i].role, oh-opt_clks[i].clk);
  }

+/*
+ * XXX: DT helper functions that should probably move elsewhere if
+ * they become usefull for other needs.
+ */


Lets just *assume* these are useful for other needs and start with
them in drivers/of so that other people know that they actually exist.
Otherwise they will never be made generic. :-)


Good point...

But, meanwhile Stephen Warren proposed a much nicer iterator to deal 
with that. Since these patches are still not in mainline, I didn't want 
to depend on them yet. So my secret plan was to remove these functions 
once the ones from Stephen will be merged.



+static int _dt_count_property_string(const char *prop, int len)
+{
+   int i = 0;
+   size_t l = 0, total = 0;
+
+   if (!prop || !len)
+   return -EINVAL;
+
+   for (i = 0; len= total; total += l, prop += l) {
+   l = strlen(prop) + 1;
+   if (*prop != 0)
+   i++;
+   }
+   return i;
+}
+
+static int _dt_get_property(const char *prop, int len, int index, char *output,
+   int size)
+{
+   int i = 0;
+   size_t l = 0, total = 0;
+
+   if (!prop || !len)
+   return -EINVAL;
+
+   for (i = 0; len= total; total += l, prop += l) {
+   l = strlcpy(output, prop, size) + 1;
+   if (*prop != 0) {
+   if (i++ == index)
+   return 0;
+   }
+   }
+   return -ENODEV;
+}
+
+static struct dev_pm_domain omap_device_pm_domain;
+
+/**
+ * omap_device_build_from_dt - build an omap_device with multiple hwmods
+ * @pdev_name: name of the platform_device driver to use
+ * @pdev_id: this platform_device's connection ID
+ * @oh: ptr to the single omap_hwmod that backs this omap_device
+ * @pdata: platform_data ptr to associate with the platform_device
+ * @pdata_len: amount of memory pointed to by @pdata
+ * @pm_lats: pointer to a omap_device_pm_latency array for this device
+ * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
+ * @is_early_device: should the device be registered as an early device or not
+ *
+ * Function for building an omap_device already registered from device-tree
+ *
+ * Returns 0 or PTR_ERR() on error.
+ */
+static int omap_device_build_from_dt(struct platform_device *pdev)
+{
+   struct omap_hwmod **hwmods;
+   struct omap_device *od;
+   struct omap_hwmod *oh;
+   char oh_name[MAX_HWMOD_NAME_SIZE];
+   const char *prop;
+   int oh_cnt, i, prop_len;
+   int ret = 0;
+
+   prop = of_get_property(pdev-dev.of_node, hwmods,prop_len);


I know you've posted it before, but what is the status of the hwmods
binding documentation.  I would expect it to be part of this patch.


It was part of the series that introduced the first node with hwmods 
attribute 
(http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-August/007621.html).

I do agree, it makes sense to add it here.


+   oh_cnt = _dt_count_property_string(prop, prop_len);
+   if

Re: [PATCH v2 2/2] OMAP: omap_device: Add a method to build an omap_device from a DT node

2011-09-17 Thread Grant Likely 
On Fri, Sep 16, 2011 at 04:43:19PM +0200, Benoit Cousson wrote:
 Add a notifier called during device_add phase. If an of_node is present,
 retrieve the hwmod entry in order to populate properly the omap_device
 structure.
 For the moment the resource from the device-tree are overloaded.
 DT does not support named resource yet, and thus, most driver
 will not work without that information.
 
 Add two helpers function to parse a property that contains multiple
 strings.
 
 Signed-off-by: Benoit Cousson b-cous...@ti.com
 Cc: Kevin Hilman khil...@ti.com
 ---
  arch/arm/plat-omap/omap_device.c |  144 
 ++
  1 files changed, 144 insertions(+), 0 deletions(-)
 
 diff --git a/arch/arm/plat-omap/omap_device.c 
 b/arch/arm/plat-omap/omap_device.c
 index cac7b9a..da13630 100644
 --- a/arch/arm/plat-omap/omap_device.c
 +++ b/arch/arm/plat-omap/omap_device.c
 @@ -85,6 +85,8 @@
  #include linux/clk.h
  #include linux/clkdev.h
  #include linux/pm_runtime.h
 +#include linux/of.h
 +#include linux/notifier.h
  
  #include plat/omap_device.h
  #include plat/omap_hwmod.h
 @@ -94,12 +96,15 @@
  #define USE_WAKEUP_LAT   0
  #define IGNORE_WAKEUP_LAT1
  
 +#define MAX_HWMOD_NAME_SIZE  32
 +
  static int omap_device_register(struct platform_device *pdev);
  static int omap_early_device_register(struct platform_device *pdev);
  static struct omap_device *omap_device_alloc(struct platform_device *pdev,
 struct omap_hwmod **ohs, int oh_cnt,
 struct omap_device_pm_latency *pm_lats,
 int pm_lats_cnt);
 +static void omap_device_delete(struct omap_device *od);
  
  
  static struct omap_device_pm_latency omap_default_latency[] = {
 @@ -315,6 +320,138 @@ static void _add_hwmod_clocks_clkdev(struct omap_device 
 *od,
   _add_clkdev(od, oh-opt_clks[i].role, oh-opt_clks[i].clk);
  }
  
 +/*
 + * XXX: DT helper functions that should probably move elsewhere if
 + * they become usefull for other needs.
 + */

Lets just *assume* these are useful for other needs and start with
them in drivers/of so that other people know that they actually exist.
Otherwise they will never be made generic. :-)

 +static int _dt_count_property_string(const char *prop, int len)
 +{
 + int i = 0;
 + size_t l = 0, total = 0;
 +
 + if (!prop || !len)
 + return -EINVAL;
 +
 + for (i = 0; len = total; total += l, prop += l) {
 + l = strlen(prop) + 1;
 + if (*prop != 0)
 + i++;
 + }
 + return i;
 +}
 +
 +static int _dt_get_property(const char *prop, int len, int index, char 
 *output,
 + int size)
 +{
 + int i = 0;
 + size_t l = 0, total = 0;
 +
 + if (!prop || !len)
 + return -EINVAL;
 +
 + for (i = 0; len = total; total += l, prop += l) {
 + l = strlcpy(output, prop, size) + 1;
 + if (*prop != 0) {
 + if (i++ == index)
 + return 0;
 + }
 + }
 + return -ENODEV;
 +}
 +
 +static struct dev_pm_domain omap_device_pm_domain;
 +
 +/**
 + * omap_device_build_from_dt - build an omap_device with multiple hwmods
 + * @pdev_name: name of the platform_device driver to use
 + * @pdev_id: this platform_device's connection ID
 + * @oh: ptr to the single omap_hwmod that backs this omap_device
 + * @pdata: platform_data ptr to associate with the platform_device
 + * @pdata_len: amount of memory pointed to by @pdata
 + * @pm_lats: pointer to a omap_device_pm_latency array for this device
 + * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
 + * @is_early_device: should the device be registered as an early device or 
 not
 + *
 + * Function for building an omap_device already registered from device-tree
 + *
 + * Returns 0 or PTR_ERR() on error.
 + */
 +static int omap_device_build_from_dt(struct platform_device *pdev)
 +{
 + struct omap_hwmod **hwmods;
 + struct omap_device *od;
 + struct omap_hwmod *oh;
 + char oh_name[MAX_HWMOD_NAME_SIZE];
 + const char *prop;
 + int oh_cnt, i, prop_len;
 + int ret = 0;
 +
 + prop = of_get_property(pdev-dev.of_node, hwmods, prop_len);

I know you've posted it before, but what is the status of the hwmods
binding documentation.  I would expect it to be part of this patch.

 + oh_cnt = _dt_count_property_string(prop, prop_len);
 + if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
 + dev_warn(pdev-dev, No 'hwmods' to build omap_device\n);
 + return -ENODEV;
 + }
 +
 + hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
 + if (!hwmods) {
 + ret = -ENOMEM;
 + goto odbfd_exit;
 + }
 +
 + for (i = 0; i  oh_cnt; i++) {
 + _dt_get_property(prop, prop_len, i, oh_name,
 +  MAX_HWMOD_NAME_SIZE);
 +
 +

[PATCH v2 2/2] OMAP: omap_device: Add a method to build an omap_device from a DT node

2011-09-16 Thread Benoit Cousson 
Add a notifier called during device_add phase. If an of_node is present,
retrieve the hwmod entry in order to populate properly the omap_device
structure.
For the moment the resource from the device-tree are overloaded.
DT does not support named resource yet, and thus, most driver
will not work without that information.

Add two helpers function to parse a property that contains multiple
strings.

Signed-off-by: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@ti.com
---
 arch/arm/plat-omap/omap_device.c |  144 ++
 1 files changed, 144 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index cac7b9a..da13630 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -85,6 +85,8 @@
 #include linux/clk.h
 #include linux/clkdev.h
 #include linux/pm_runtime.h
+#include linux/of.h
+#include linux/notifier.h
 
 #include plat/omap_device.h
 #include plat/omap_hwmod.h
@@ -94,12 +96,15 @@
 #define USE_WAKEUP_LAT 0
 #define IGNORE_WAKEUP_LAT  1
 
+#define MAX_HWMOD_NAME_SIZE32
+
 static int omap_device_register(struct platform_device *pdev);
 static int omap_early_device_register(struct platform_device *pdev);
 static struct omap_device *omap_device_alloc(struct platform_device *pdev,
  struct omap_hwmod **ohs, int oh_cnt,
  struct omap_device_pm_latency *pm_lats,
  int pm_lats_cnt);
+static void omap_device_delete(struct omap_device *od);
 
 
 static struct omap_device_pm_latency omap_default_latency[] = {
@@ -315,6 +320,138 @@ static void _add_hwmod_clocks_clkdev(struct omap_device 
*od,
_add_clkdev(od, oh-opt_clks[i].role, oh-opt_clks[i].clk);
 }
 
+/*
+ * XXX: DT helper functions that should probably move elsewhere if
+ * they become usefull for other needs.
+ */
+static int _dt_count_property_string(const char *prop, int len)
+{
+   int i = 0;
+   size_t l = 0, total = 0;
+
+   if (!prop || !len)
+   return -EINVAL;
+
+   for (i = 0; len = total; total += l, prop += l) {
+   l = strlen(prop) + 1;
+   if (*prop != 0)
+   i++;
+   }
+   return i;
+}
+
+static int _dt_get_property(const char *prop, int len, int index, char *output,
+   int size)
+{
+   int i = 0;
+   size_t l = 0, total = 0;
+
+   if (!prop || !len)
+   return -EINVAL;
+
+   for (i = 0; len = total; total += l, prop += l) {
+   l = strlcpy(output, prop, size) + 1;
+   if (*prop != 0) {
+   if (i++ == index)
+   return 0;
+   }
+   }
+   return -ENODEV;
+}
+
+static struct dev_pm_domain omap_device_pm_domain;
+
+/**
+ * omap_device_build_from_dt - build an omap_device with multiple hwmods
+ * @pdev_name: name of the platform_device driver to use
+ * @pdev_id: this platform_device's connection ID
+ * @oh: ptr to the single omap_hwmod that backs this omap_device
+ * @pdata: platform_data ptr to associate with the platform_device
+ * @pdata_len: amount of memory pointed to by @pdata
+ * @pm_lats: pointer to a omap_device_pm_latency array for this device
+ * @pm_lats_cnt: ARRAY_SIZE() of @pm_lats
+ * @is_early_device: should the device be registered as an early device or not
+ *
+ * Function for building an omap_device already registered from device-tree
+ *
+ * Returns 0 or PTR_ERR() on error.
+ */
+static int omap_device_build_from_dt(struct platform_device *pdev)
+{
+   struct omap_hwmod **hwmods;
+   struct omap_device *od;
+   struct omap_hwmod *oh;
+   char oh_name[MAX_HWMOD_NAME_SIZE];
+   const char *prop;
+   int oh_cnt, i, prop_len;
+   int ret = 0;
+
+   prop = of_get_property(pdev-dev.of_node, hwmods, prop_len);
+   oh_cnt = _dt_count_property_string(prop, prop_len);
+   if (!oh_cnt || IS_ERR_VALUE(oh_cnt)) {
+   dev_warn(pdev-dev, No 'hwmods' to build omap_device\n);
+   return -ENODEV;
+   }
+
+   hwmods = kzalloc(sizeof(struct omap_hwmod *) * oh_cnt, GFP_KERNEL);
+   if (!hwmods) {
+   ret = -ENOMEM;
+   goto odbfd_exit;
+   }
+
+   for (i = 0; i  oh_cnt; i++) {
+   _dt_get_property(prop, prop_len, i, oh_name,
+MAX_HWMOD_NAME_SIZE);
+
+   oh = omap_hwmod_lookup(oh_name);
+   if (!oh) {
+   dev_err(pdev-dev, Cannot lookup hwmod '%s'\n,
+   oh_name);
+   ret = -EINVAL;
+   goto odbfd_exit1;
+   }
+   hwmods[i] = oh;
+   }
+
+   od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
+   if (!od) {
+   dev_err(pdev-dev,