[RFC/PATCH 0/7] arm: omap: move more HWMOD data to DT

[RFC/PATCH 0/7] arm: omap: move more HWMOD data to DT

[Felipe Balbi]

Hi,

here's a more complete series showing the out of debugfs interface.

So far only AM43xx has been converted and only those HWMODs which
play well with DT and/or are not common with other OMAPs/AM33xx.

I plan to work on AM33xx tomorrow, but this RFC shows what we can
achieve WRT HWMOD static data reduction.

As you can see, with AM43xx, data reduction is quite minimal, since
it reuses so much from AM33xx, but well, had to start somewhere.

All patches were boot tested with AM437x SK on top of:

commit 1ca7c606de868d172afb4eb65e04e290dbdb51ff
Author: Stephen Rothwell <sfr@canb.auug.org.au>
Date:   Thu Dec 4 19:49:10 2014 +1100

    Add linux-next specific files for 20141204
    
    Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

cheers

Felipe Balbi (7):
  arm: omap: hwmod: add debugfs interface
  arm: omap: devicetree: add new properties for OMAP devices
  arm: omap: hwmod: drop 'const' qualifier from omap_hwmod_class name
  arm: omap: device: add support for generating sysconfig data from DT
  arm: omap: hwmod: allow for registration of class-less hwmods
  arm: boot: dts: am4372: add sysconfig data to all HWMODs
  arm: omap: hwmod: 43xx: remove sysc and class data

 .../devicetree/bindings/arm/omap/omap.txt          |   7 +
 arch/arm/boot/dts/am4372.dtsi                      | 251 +++++++++++++++++++
 arch/arm/mach-omap2/Makefile                       |   2 +-
 arch/arm/mach-omap2/omap_device.c                  | 117 +++++++++
 arch/arm/mach-omap2/omap_hwmod.c                   |  30 ++-
 arch/arm/mach-omap2/omap_hwmod.h                   |  11 +-
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c         |  61 -----
 arch/arm/mach-omap2/omap_hwmod_debugfs.c           | 269 +++++++++++++++++++++
 8 files changed, 673 insertions(+), 75 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap_hwmod_debugfs.c

-- 
2.2.0

[RFC/PATCH 1/7] arm: omap: hwmod: add debugfs interface

[Felipe Balbi]

By exposing the details of hwmod structures
to debugfs we can much more easily verify
that changes to hwmod data is correct and won't
cause regressions.

The idea is that this can be used to check the
state of one hwmod, verify hwmod sysc fields, etc.

For example, this will be used to move some of
the sysc fields to DT and later verify that they
are correct pre- and post-patch.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/Makefile             |   2 +-
 arch/arm/mach-omap2/omap_device.c        |   2 +
 arch/arm/mach-omap2/omap_hwmod.h         |   9 ++
 arch/arm/mach-omap2/omap_hwmod_debugfs.c | 269 +++++++++++++++++++++++++++++++
 4 files changed, 281 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-omap2/omap_hwmod_debugfs.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 5d27dfd..68c9ae5 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -11,7 +11,7 @@ obj-y := id.o io.o control.o mux.o devices.o fb.o serial.o timer.o pm.o \
 	 omap_device.o sram.o drm.o
 
 hwmod-common				= omap_hwmod.o omap_hwmod_reset.o \
-					  omap_hwmod_common_data.o
+					  omap_hwmod_common_data.o omap_hwmod_debugfs.o
 clock-common				= clock.o clock_common_data.o \
 					  clkt_dpll.o clkt_clksel.o
 secure-common				= omap-smc.o omap-secure.o
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 8c58b71..abd622b 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -156,6 +156,8 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
 		hwmods[i] = oh;
 		if (oh->flags & HWMOD_INIT_NO_IDLE)
 			device_active = true;
+
+		omap_hwmod_register_debugfs(oh);
 	}
 
 	od = omap_device_alloc(pdev, hwmods, oh_cnt);
diff --git a/arch/arm/mach-omap2/omap_hwmod.h b/arch/arm/mach-omap2/omap_hwmod.h
index 35ca6ef..fcf55f5 100644
--- a/arch/arm/mach-omap2/omap_hwmod.h
+++ b/arch/arm/mach-omap2/omap_hwmod.h
@@ -768,4 +768,13 @@ int am43xx_hwmod_init(void);
 
 extern int __init omap_hwmod_register_links(struct omap_hwmod_ocp_if **ois);
 
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+extern int omap_hwmod_register_debugfs(struct omap_hwmod *oh);
+#else
+static inline int omap_hwmod_register_debugfs(struct omap_hwmod *oh)
+{
+	return 0;
+}
+#endif
+
 #endif
diff --git a/arch/arm/mach-omap2/omap_hwmod_debugfs.c b/arch/arm/mach-omap2/omap_hwmod_debugfs.c
new file mode 100644
index 0000000..cc9533b
--- /dev/null
+++ b/arch/arm/mach-omap2/omap_hwmod_debugfs.c
@@ -0,0 +1,269 @@
+/*
+ * omap_hwmod debugfs view
+ *
+ * Copyright (C) 2014 Texas Instruments, Incorporated - http://www.ti.com
+ * Author: Felipe Balbi <balbi@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#undef DEBUG
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#include "clock.h"
+#include "omap_hwmod.h"
+
+#include "soc.h"
+#include "common.h"
+#include "clockdomain.h"
+#include "powerdomain.h"
+#include "cm2xxx.h"
+#include "cm3xxx.h"
+#include "cm33xx.h"
+#include "prm.h"
+#include "prm3xxx.h"
+#include "prm44xx.h"
+#include "prm33xx.h"
+#include "prminst44xx.h"
+#include "mux.h"
+#include "pm.h"
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+static struct dentry *omap_hwmod_debugfs_root;
+
+static int __init omap_hwmod_create_files(struct omap_hwmod *oh,
+		struct dentry *dir)
+{
+	struct dentry *file;
+
+	file = debugfs_create_u16("flags", S_IRUGO, dir, &oh->flags);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("mpu_rt_idx", S_IRUGO, dir, &oh->mpu_rt_idx);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("response_lat", S_IRUGO, dir,
+			&oh->response_lat);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("rst_lines_cnt", S_IRUGO, dir,
+			&oh->rst_lines_cnt);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("opt_clks_cnt", S_IRUGO, dir,
+			&oh->opt_clks_cnt);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("masters_cnt", S_IRUGO, dir, &oh->masters_cnt);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("slaves_cnt", S_IRUGO, dir, &oh->slaves_cnt);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("hwmods_cnt", S_IRUGO, dir, &oh->hwmods_cnt);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("_int_flags", S_IRUGO, dir, &oh->_int_flags);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("_state", S_IRUGO, dir, &oh->_state);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("_postsetup_state", S_IRUGO, dir,
+			&oh->_postsetup_state);
+	if (!file)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int __init
+omap_hwmod_create_sysc_files(struct omap_hwmod_class_sysconfig *sysc,
+		struct dentry *dir)
+{
+	struct dentry *subdir;
+	struct dentry *file;
+
+	if (!sysc)
+		return 0;
+
+	subdir = debugfs_create_dir("sysc", dir);
+	if (!subdir)
+		return -ENOMEM;
+
+	file = debugfs_create_x32("rev_offs", S_IRUGO, subdir,
+			&sysc->rev_offs);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_x32("sysc_offs", S_IRUGO, subdir,
+			&sysc->sysc_offs);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_x32("syss_offs", S_IRUGO, subdir,
+			&sysc->syss_offs);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_x16("sysc_flags", S_IRUGO, subdir,
+			&sysc->sysc_flags);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("srst_udelay", S_IRUGO, subdir,
+			&sysc->srst_udelay);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_x8("idlemodes", S_IRUGO, subdir,
+			&sysc->idlemodes);
+	if (!file)
+		return -ENOMEM;
+
+	file = debugfs_create_u8("clockact", S_IRUGO, subdir, &sysc->clockact);
+	if (!file)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int __init omap_hwmod_create_class_files(struct omap_hwmod_class *class,
+		struct dentry *dir)
+{
+	struct dentry *subdir;
+	struct dentry *file;
+	int ret;
+
+	if (!class)
+		return 0;
+
+	subdir = debugfs_create_dir("class", dir);
+	if (!subdir)
+		return -ENOMEM;
+
+	file = debugfs_create_u32("rev", S_IRUGO, subdir, &class->rev);
+	if (!file)
+		return -ENOMEM;
+
+	ret = omap_hwmod_create_sysc_files(class->sysc, subdir);
+	if (ret)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int __init omap_hwmod_create_irq_files(struct omap_hwmod_irq_info *irqs,
+		struct dentry *dir)
+{
+	struct dentry *subdir;
+	int i = 0;
+
+	if (!irqs)
+		return 0;
+
+	subdir = debugfs_create_dir("irqs", dir);
+	if (!subdir)
+		return -ENOMEM;
+
+	while (irqs[i].irq != -1) {
+		struct dentry *file;
+
+		file = debugfs_create_u16(irqs[i].name, S_IRUGO, subdir,
+				&irqs[i].irq);
+		if (!file)
+			return -ENOMEM;
+		i++;
+	}
+
+	return 0;
+}
+
+static int __init omap_hwmod_create_dma_files(struct omap_hwmod_dma_info *dmas,
+		struct dentry *dir)
+{
+	struct dentry *subdir;
+	int i = 0;
+
+	if (!dmas)
+		return 0;
+
+	subdir = debugfs_create_dir("dmas", dir);
+	if (!subdir)
+		return -ENOMEM;
+
+	while (dmas[i].dma_req != -1) {
+		struct dentry *file;
+
+		file = debugfs_create_u16(dmas[i].name, S_IRUGO, subdir,
+				&dmas[i].dma_req);
+		if (!file)
+			return -ENOMEM;
+		i++;
+	}
+
+	return 0;
+}
+
+int omap_hwmod_register_debugfs(struct omap_hwmod *oh)
+{
+	struct dentry *root;
+	struct dentry *dir;
+	int ret;
+
+	if (!omap_hwmod_debugfs_root) {
+		root = debugfs_create_dir("omap_hwmod", NULL);
+		if (!root ) {
+			pr_debug("omap_hwmod: unable to initialize debugfs\n");
+			return 0;
+		}
+
+		omap_hwmod_debugfs_root = root;
+	} else {
+		root = omap_hwmod_debugfs_root;
+	}
+
+	dir = debugfs_create_dir(oh->name, root);
+	if (!dir)
+		return -ENOMEM;
+
+	ret = omap_hwmod_create_files(oh, dir);
+	if (ret)
+		return ret;
+
+	ret = omap_hwmod_create_class_files(oh->class, dir);
+	if (ret)
+		return ret;
+
+	ret = omap_hwmod_create_irq_files(oh->mpu_irqs, dir);
+	if (ret)
+		return ret;
+
+	ret = omap_hwmod_create_dma_files(oh->sdma_reqs, dir);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+#endif
-- 
2.2.0

[RFC/PATCH 2/7] arm: omap: devicetree: add new properties for OMAP devices

[Felipe Balbi]

In order to get rid of some more hwmod data, we
introduce a few extra properties to OMAP DT
data in order to be able to pass the needed
information through DT.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 Documentation/devicetree/bindings/arm/omap/omap.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
index 4f6a82c..c6b9515 100644
--- a/Documentation/devicetree/bindings/arm/omap/omap.txt
+++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
@@ -23,6 +23,13 @@ Optional properties:
   during suspend.
 - ti,no-reset-on-init: When present, the module should not be reset at init
 - ti,no-idle-on-init: When present, the module should not be idled at init
+- ti,rev_offs: IP block revision register offset (defaults to 0)
+- ti,sysc_offs: OCP_SYSCONFIG register offset (defaults to 0)
+- ti,syss_offs: OCP_SYSSTATUS register offset (defaults to 0)
+- ti,srst_udelay: Delay needed after a softreset in usecs (defaults to 0)
+- ti,idlemodes: Slave and Master supported idling modes
+- ti,clockact: default value of clock activity bits (defaults to 0)
+- ti,sysc_type: OCP_SYSCONFIG type. Valid types are 1, 2 or 3 (defaults to 1)
 
 Example:
 
-- 
2.2.0

[RFC/PATCH 3/7] arm: omap: hwmod: drop 'const' qualifier from omap_hwmod_class name

[Felipe Balbi]

We will be allocating struct omap_hwmod_class for
any DT which passes sysc data. Because of that,
class->name cannot be const.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.h b/arch/arm/mach-omap2/omap_hwmod.h
index fcf55f5..0ee0116 100644
--- a/arch/arm/mach-omap2/omap_hwmod.h
+++ b/arch/arm/mach-omap2/omap_hwmod.h
@@ -588,7 +588,7 @@ struct omap_hwmod_omap4_prcm {
  * unusual reset sequences - usually processor IP blocks like the IVA.
  */
 struct omap_hwmod_class {
-	const char				*name;
+	char					*name;
 	struct omap_hwmod_class_sysconfig	*sysc;
 	u32					rev;
 	int					(*pre_shutdown)(struct omap_hwmod *oh);
-- 
2.2.0

[RFC/PATCH 4/7] arm: omap: device: add support for generating sysconfig data from DT

[Felipe Balbi]

After moving sysconfig data to DT, we need to make
sure we can generate the same data we have today in C
files out of the DT provided properties.

This patch adds support for all optional properties
documented by a previous commit.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/omap_device.c | 115 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index abd622b..ea729aa 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -25,6 +25,7 @@
  */
 #undef DEBUG
 
+#include <linux/ctype.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -109,6 +110,115 @@ static void _add_hwmod_clocks_clkdev(struct omap_device *od,
 		_add_clkdev(od, oh->opt_clks[i].role, oh->opt_clks[i].clk);
 }
 
+static int omap_hwmod_has_sysc_bindings(struct device *dev)
+{
+	struct device_node *node = dev->of_node;
+	char *properties[] = {
+		"ti,rev_offs",
+		"ti,sysc_offs",
+		"ti,syss_offs",
+		"ti,sysc_flags",
+		"ti,srst_udelay",
+		"ti,idlemodes",
+		"ti,clockact",
+		"ti,sysc_type",
+		NULL
+	};
+	char **tmp = properties;
+
+	while (*tmp) {
+		if (of_property_read_bool(node, *tmp)) {
+			return true;
+		}
+		tmp++;
+	}
+
+	return 0;
+}
+
+static int omap_hwmod_init_sysc(struct device *dev,
+		struct omap_hwmod *oh, int index)
+{
+	struct device_node *node = dev->of_node;
+	struct omap_hwmod_class *class = oh->class;
+	struct omap_hwmod_class_sysconfig *sysc;
+	int ret;
+	int i;
+	char name[128];
+	const char *tmp = oh->name;
+	u32 prop;
+
+	/* if data isn't provided by DT, skip */
+	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(dev))
+		return 0;
+
+	class = kzalloc(sizeof(*class), GFP_KERNEL);
+	if (!class)
+		return -ENOMEM;
+
+	i = 0;
+	while (*tmp) {
+		if (isalpha(*tmp))
+			name[i++] = *tmp;
+		tmp++;
+	}
+	name[i] = '\0';
+
+	class->name = kzalloc(sizeof(name), GFP_KERNEL);
+	if (!class->name)
+		return -ENOMEM;
+	strncpy(class->name, name, sizeof(name) - 1);
+
+	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
+	if (!sysc)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
+	if (!ret)
+		sysc->rev_offs = prop;
+
+	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
+	if (!ret)
+		sysc->sysc_offs = prop;
+
+	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
+	if (!ret)
+		sysc->syss_offs = prop;
+
+	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
+	if (!ret)
+		sysc->sysc_flags = prop & 0xffff;
+
+	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
+	if (!ret)
+		sysc->srst_udelay = prop & 0xff;
+
+	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
+	if (!ret)
+		sysc->idlemodes = prop & 0xff;
+
+	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
+	if (!ret)
+		sysc->clockact = prop & 0xff;
+
+	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
+	if (ret)
+		prop = 1;
+
+	switch (prop) {
+	case 2:
+		sysc->sysc_fields = &omap_hwmod_sysc_type2;
+		break;
+	case 3:
+		sysc->sysc_fields = &omap_hwmod_sysc_type3;
+		break;
+	case 1:
+	default:
+		sysc->sysc_fields = &omap_hwmod_sysc_type1;
+	}
+
+	return 0;
+}
 
 /**
  * omap_device_build_from_dt - build an omap_device with multiple hwmods
@@ -154,6 +264,11 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
 			goto odbfd_exit1;
 		}
 		hwmods[i] = oh;
+
+		ret = omap_hwmod_init_sysc(&pdev->dev, oh, i);
+		if (ret)
+			goto odbfd_exit1;
+
 		if (oh->flags & HWMOD_INIT_NO_IDLE)
 			device_active = true;
 
-- 
2.2.0

[RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Felipe Balbi]

Before this patch, HWMOD requires the existence
of a struct omap_hwmod_class very early. It just
so happens that we're moving that data to DeviceTree
in a follow-up commit so we need to allow the
registration of HWMODs with a valid omap_hwmod_class
pointer.

When the device is built from DT, we will allocate and
populate struct omap_hwmod_class based on DT data.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index cbb908d..2355764 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -504,6 +504,9 @@ static int _set_dmadisable(struct omap_hwmod *oh)
 	u32 v;
 	u32 dmadisable_mask;
 
+	if (!oh->class)
+		return -EINVAL;
+
 	if (!oh->class->sysc ||
 	    !(oh->class->sysc->sysc_flags & SYSC_HAS_DMADISABLE))
 		return -EINVAL;
@@ -1843,7 +1846,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
 	int c = 0;
 	int ret = 0;
 
-	if (!oh->class->sysc ||
+	if (!oh->class || !oh->class->sysc ||
 	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET))
 		return -ENOENT;
 
@@ -1937,12 +1940,13 @@ static int _reset(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
-	if (oh->class->reset) {
+	if (oh->class && oh->class->reset) {
 		r = oh->class->reset(oh);
 	} else {
 		if (oh->rst_lines_cnt > 0) {
-			for (i = 0; i < oh->rst_lines_cnt; i++)
+			for (i = 0; i < oh->rst_lines_cnt; i++) {
 				_assert_hardreset(oh, oh->rst_lines[i].name);
+			}
 			return 0;
 		} else {
 			r = _ocp_softreset(oh);
@@ -1958,7 +1962,7 @@ static int _reset(struct omap_hwmod *oh)
 	 * softreset.  The _enable() function should be split to avoid
 	 * the rewrite of the OCP_SYSCONFIG register.
 	 */
-	if (oh->class->sysc) {
+	if (oh->class && oh->class->sysc) {
 		_update_sysc_cache(oh);
 		_enable_sysc(oh);
 	}
@@ -2036,7 +2040,7 @@ static int _omap4_get_context_lost(struct omap_hwmod *oh)
  */
 static int _enable_preprogram(struct omap_hwmod *oh)
 {
-	if (!oh->class->enable_preprogram)
+	if (!oh->class || !oh->class->enable_preprogram)
 		return 0;
 
 	return oh->class->enable_preprogram(oh);
@@ -2145,7 +2149,7 @@ static int _enable(struct omap_hwmod *oh)
 		oh->_state = _HWMOD_STATE_ENABLED;
 
 		/* Access the sysconfig only if the target is ready */
-		if (oh->class->sysc) {
+		if (oh->class && oh->class->sysc) {
 			if (!(oh->_int_flags & _HWMOD_SYSCONFIG_LOADED))
 				_update_sysc_cache(oh);
 			_enable_sysc(oh);
@@ -2186,7 +2190,7 @@ static int _idle(struct omap_hwmod *oh)
 	if (_are_all_hardreset_lines_asserted(oh))
 		return 0;
 
-	if (oh->class->sysc)
+	if (oh->class && oh->class->sysc)
 		_idle_sysc(oh);
 	_del_initiator_dep(oh, mpu_oh);
 
@@ -2244,7 +2248,7 @@ static int _shutdown(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
 
-	if (oh->class->pre_shutdown) {
+	if (oh->class && oh->class->pre_shutdown) {
 		prev_state = oh->_state;
 		if (oh->_state == _HWMOD_STATE_IDLE)
 			_enable(oh);
@@ -2256,7 +2260,7 @@ static int _shutdown(struct omap_hwmod *oh)
 		}
 	}
 
-	if (oh->class->sysc) {
+	if (oh->class && oh->class->sysc) {
 		if (oh->_state == _HWMOD_STATE_IDLE)
 			_enable(oh);
 		_shutdown_sysc(oh);
@@ -2451,7 +2455,7 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 				oh->name, np->name);
 	}
 
-	if (oh->class->sysc) {
+	if (oh->class && oh->class->sysc) {
 		r = _init_mpu_rt_base(oh, NULL, index, np);
 		if (r < 0) {
 			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
@@ -2684,8 +2688,7 @@ static int __init _setup(struct omap_hwmod *oh, void *data)
  */
 static int __init _register(struct omap_hwmod *oh)
 {
-	if (!oh || !oh->name || !oh->class || !oh->class->name ||
-	    (oh->_state != _HWMOD_STATE_UNKNOWN))
+	if (!oh || !oh->name || (oh->_state != _HWMOD_STATE_UNKNOWN))
 		return -EINVAL;
 
 	pr_debug("omap_hwmod: %s: registering\n", oh->name);
@@ -3942,6 +3945,9 @@ int omap_hwmod_for_each_by_class(const char *classname,
 		 __func__, classname);
 
 	list_for_each_entry(temp_oh, &omap_hwmod_list, node) {
+		if (!temp_oh->class)
+			continue;
+
 		if (!strcmp(temp_oh->class->name, classname)) {
 			pr_debug("omap_hwmod: %s: %s: calling callback fn\n",
 				 __func__, temp_oh->name);
-- 
2.2.0

[RFC/PATCH 6/7] arm: boot: dts: am4372: add sysconfig data to all HWMODs

[Felipe Balbi]

This patch adds all new sysconfig DT properties
so they can later be removed from hwmod*data.c files

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Note here, there's still an idea of converting the flags to use
macros which would get preprocessed away, I'm yet to do that.

 arch/arm/boot/dts/am4372.dtsi | 251 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 251 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index b62a1cd..5a14405 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -126,6 +126,11 @@
 			reg = <0x44e09000 0x2000>;
 			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart1";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x27>;
+			ti,syss_offs = <0x58>;
+			ti,sysc_offs = <0x54>;
+			ti,rev_offs = <0x50>;
 		};
 
 		uart1: serial@48022000 {
@@ -133,6 +138,11 @@
 			reg = <0x48022000 0x2000>;
 			interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart2";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x27>;
+			ti,syss_offs = <0x58>;
+			ti,sysc_offs = <0x54>;
+			ti,rev_offs = <0x50>;
 			status = "disabled";
 		};
 
@@ -141,6 +151,11 @@
 			reg = <0x48024000 0x2000>;
 			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart3";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x27>;
+			ti,syss_offs = <0x58>;
+			ti,sysc_offs = <0x54>;
+			ti,rev_offs = <0x50>;
 			status = "disabled";
 		};
 
@@ -149,6 +164,11 @@
 			reg = <0x481a6000 0x2000>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart4";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x27>;
+			ti,syss_offs = <0x58>;
+			ti,sysc_offs = <0x54>;
+			ti,rev_offs = <0x50>;
 			status = "disabled";
 		};
 
@@ -157,6 +177,11 @@
 			reg = <0x481a8000 0x2000>;
 			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart5";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x27>;
+			ti,syss_offs = <0x58>;
+			ti,sysc_offs = <0x54>;
+			ti,rev_offs = <0x50>;
 			status = "disabled";
 		};
 
@@ -165,6 +190,11 @@
 			reg = <0x481aa000 0x2000>;
 			interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "uart6";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x27>;
+			ti,syss_offs = <0x58>;
+			ti,sysc_offs = <0x54>;
+			ti,rev_offs = <0x50>;
 			status = "disabled";
 		};
 
@@ -173,6 +203,10 @@
 			reg = <0x480C8000 0x200>;
 			interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "mailbox";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0x222>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			#mbox-cells = <1>;
 			ti,mbox-num-users = <4>;
 			ti,mbox-num-fifos = <8>;
@@ -188,6 +222,11 @@
 			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-alwon;
 			ti,hwmods = "timer1";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 		};
 
 		timer2: timer@48040000  {
@@ -195,6 +234,11 @@
 			reg = <0x48040000  0x400>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "timer2";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 		};
 
 		timer3: timer@48042000 {
@@ -202,7 +246,12 @@
 			reg = <0x48042000 0x400>;
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "timer3";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
+			ti,sysc_type = <2>;
 		};
 
 		timer4: timer@48044000 {
@@ -211,6 +260,11 @@
 			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-pwm;
 			ti,hwmods = "timer4";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -220,6 +274,11 @@
 			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-pwm;
 			ti,hwmods = "timer5";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -229,6 +288,11 @@
 			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-pwm;
 			ti,hwmods = "timer6";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -238,6 +302,11 @@
 			interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-pwm;
 			ti,hwmods = "timer7";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -246,6 +315,11 @@
 			reg = <0x481c1000 0x400>;
 			interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "timer8";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -254,6 +328,11 @@
 			reg = <0x4833d000 0x400>;
 			interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "timer9";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -262,6 +341,11 @@
 			reg = <0x4833f000 0x400>;
 			interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "timer10";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -270,6 +354,11 @@
 			reg = <0x48341000 0x400>;
 			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "timer11";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x22>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 		};
 
@@ -277,6 +366,9 @@
 			compatible = "ti,am4372-counter32k","ti,omap-counter32k";
 			reg = <0x44e86000 0x40>;
 			ti,hwmods = "counter_32k";
+			ti,idlemodes = <0x3>;
+			ti,sysc_flags = <0x20>;
+			ti,sysc_offs = <0x4>;
 		};
 
 		rtc: rtc@44e3e000 {
@@ -285,6 +377,11 @@
 			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH
 				      GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "rtc";
+			ti,rev_offs = <0x74>;
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x20>;
+			ti,sysc_offs = <0x78>;
+			ti,sysc_type = <3>;
 			status = "disabled";
 		};
 
@@ -293,6 +390,10 @@
 			reg = <0x44e35000 0x1000>;
 			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "wd_timer2";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xaa>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
 		};
 
 		gpio0: gpio@44e07000 {
@@ -304,6 +405,10 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			ti,hwmods = "gpio1";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xa7>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
 		};
 
@@ -316,6 +421,10 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			ti,hwmods = "gpio2";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xa7>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
 		};
 
@@ -328,6 +437,10 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			ti,hwmods = "gpio3";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xa7>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
 		};
 
@@ -340,6 +453,10 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			ti,hwmods = "gpio4";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xa7>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
 		};
 
@@ -352,6 +469,10 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			ti,hwmods = "gpio5";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xa7>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
 		};
 
@@ -364,6 +485,10 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			ti,hwmods = "gpio6";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xa7>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x10>;
 			status = "disabled";
 		};
 
@@ -371,6 +496,10 @@
 			compatible = "ti,omap4-hwspinlock";
 			reg = <0x480ca000 0x1000>;
 			ti,hwmods = "spinlock";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
 			#hwlock-cells = <1>;
 		};
 
@@ -379,6 +508,10 @@
 			reg = <0x44e0b000 0x1000>;
 			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "i2c1";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x90>;
+			ti,sysc_offs = <0x10>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -389,6 +522,10 @@
 			reg = <0x4802a000 0x1000>;
 			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "i2c2";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x90>;
+			ti,sysc_offs = <0x10>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -399,6 +536,10 @@
 			reg = <0x4819c000 0x1000>;
 			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "i2c3";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x90>;
+			ti,sysc_offs = <0x10>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -409,6 +550,10 @@
 			reg = <0x48030000 0x400>;
 			interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "spi0";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x110>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -418,6 +563,11 @@
 			compatible = "ti,omap4-hsmmc";
 			reg = <0x48060000 0x1000>;
 			ti,hwmods = "mmc1";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,rev_offs = <0x1fc>;
 			ti,dual-volt;
 			ti,needs-special-reset;
 			dmas = <&edma 24
@@ -431,6 +581,11 @@
 			compatible = "ti,omap4-hsmmc";
 			reg = <0x481d8000 0x1000>;
 			ti,hwmods = "mmc2";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,rev_offs = <0x1fc>;
 			ti,needs-special-reset;
 			dmas = <&edma 2
 				&edma 3>;
@@ -443,6 +598,11 @@
 			compatible = "ti,omap4-hsmmc";
 			reg = <0x47810000 0x1000>;
 			ti,hwmods = "mmc3";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb7>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
+			ti,rev_offs = <0x1fc>;
 			ti,needs-special-reset;
 			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
@@ -453,6 +613,10 @@
 			reg = <0x481a0000 0x400>;
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "spi1";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x110>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -463,6 +627,10 @@
 			reg = <0x481a2000 0x400>;
 			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "spi2";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x110>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -473,6 +641,10 @@
 			reg = <0x481a4000 0x400>;
 			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "spi3";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x110>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -483,6 +655,10 @@
 			reg = <0x48345000 0x400>;
 			interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "spi4";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x110>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			status = "disabled";
@@ -499,6 +675,11 @@
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ti,hwmods = "cpgmac0";
+			ti,idlemodes = <0x33>;
+			ti,sysc_flags = <0xe0>;
+			ti,syss_offs = <0x4>;
+			ti,sysc_offs = <0x8>;
+			ti,sysc_type = <3>;
 			clocks = <&cpsw_125mhz_gclk>, <&cpsw_cpts_rft_clk>;
 			clock-names = "fck", "cpts";
 			status = "disabled";
@@ -548,6 +729,10 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "epwmss0";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x60>;
+			ti,sysc_offs = <0x4>;
+			ti,sysc_type = <2>;
 			status = "disabled";
 
 			ecap0: ecap@48300100 {
@@ -574,6 +759,9 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "epwmss1";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x60>;
+			ti,sysc_offs = <0x4>;
 			status = "disabled";
 
 			ecap1: ecap@48302100 {
@@ -600,6 +788,9 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "epwmss2";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x60>;
+			ti,sysc_offs = <0x4>;
 			status = "disabled";
 
 			ecap2: ecap@48304100 {
@@ -626,6 +817,9 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "epwmss3";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x60>;
+			ti,sysc_offs = <0x4>;
 			status = "disabled";
 
 			ehrpwm3: ehrpwm@48306200 {
@@ -644,6 +838,9 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "epwmss4";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x60>;
+			ti,sysc_offs = <0x4>;
 			status = "disabled";
 
 			ehrpwm4: ehrpwm@48308200 {
@@ -662,6 +859,9 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "epwmss5";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x60>;
+			ti,sysc_offs = <0x4>;
 			status = "disabled";
 
 			ehrpwm5: ehrpwm@4830a200 {
@@ -677,6 +877,10 @@
 			compatible = "ti,am3359-tscadc";
 			reg = <0x44e0d000 0x1000>;
 			ti,hwmods = "adc_tsc";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x20>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&adc_tsc_fck>;
 			clock-names = "fck";
@@ -696,6 +900,10 @@
 		sham: sham@53100000 {
 			compatible = "ti,omap5-sham";
 			ti,hwmods = "sham";
+			ti,sysc_flags = <0x80>;
+			ti,syss_offs = <0x114>;
+			ti,sysc_offs = <0x110>;
+			ti,rev_offs = <0x100>;
 			reg = <0x53100000 0x300>;
 			dmas = <&edma 36>;
 			dma-names = "rx";
@@ -705,6 +913,10 @@
 		aes: aes@53501000 {
 			compatible = "ti,omap4-aes";
 			ti,hwmods = "aes";
+			ti,sysc_flags = <0x80>;
+			ti,syss_offs = <0x88>;
+			ti,sysc_offs = <0x84>;
+			ti,rev_offs = <0x80>;
 			reg = <0x53501000 0xa0>;
 			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
 			dmas = <&edma 6
@@ -725,6 +937,10 @@
 		mcasp0: mcasp@48038000 {
 			compatible = "ti,am33xx-mcasp-audio";
 			ti,hwmods = "mcasp0";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0x20>;
+			ti,sysc_offs = <0x4>;
+			ti,sysc_type = <3>;
 			reg = <0x48038000 0x2000>,
 			      <0x46000000 0x400000>;
 			reg-names = "mpu", "dat";
@@ -739,6 +955,10 @@
 		mcasp1: mcasp@4803C000 {
 			compatible = "ti,am33xx-mcasp-audio";
 			ti,hwmods = "mcasp1";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0x20>;
+			ti,sysc_offs = <0x4>;
+			ti,sysc_type = <3>;
 			reg = <0x4803C000 0x2000>,
 			      <0x46400000 0x400000>;
 			reg-names = "mpu", "dat";
@@ -755,6 +975,10 @@
 			reg = <0x48080000 0x2000>;
 			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "elm";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xb3>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
 			clocks = <&l4ls_gclk>;
 			clock-names = "fck";
 			status = "disabled";
@@ -763,6 +987,10 @@
 		gpmc: gpmc@50000000 {
 			compatible = "ti,am3352-gpmc";
 			ti,hwmods = "gpmc";
+			ti,idlemodes = <0x7>;
+			ti,sysc_flags = <0xa3>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
 			clocks = <&l3s_gclk>;
 			clock-names = "fck";
 			reg = <0x50000000 0x2000>;
@@ -827,6 +1055,10 @@
 		dwc3_1: omap_dwc3@48380000 {
 			compatible = "ti,am437x-dwc3";
 			ti,hwmods = "usb_otg_ss0";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x460>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			reg = <0x48380000 0x10000>;
 			interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>;
 			#address-cells = <1>;
@@ -851,6 +1083,10 @@
 		dwc3_2: omap_dwc3@483c0000 {
 			compatible = "ti,am437x-dwc3";
 			ti,hwmods = "usb_otg_ss1";
+			ti,idlemodes = <0xff>;
+			ti,sysc_flags = <0x460>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			reg = <0x483c0000 0x10000>;
 			interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
 			#address-cells = <1>;
@@ -878,6 +1114,10 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "qspi";
+			ti,idlemodes = <0xf>;
+			ti,sysc_flags = <0x20>;
+			ti,sysc_offs = <0x10>;
+			ti,sysc_type = <2>;
 			interrupts = <0 138 0x4>;
 			num-cs = <4>;
 			status = "disabled";
@@ -898,6 +1138,9 @@
 			reg = <0x4832a000 0x200>;
 			status = "disabled";
 			ti,hwmods = "dss_core";
+			ti,sysc_flags = <0x83>;
+			ti,syss_offs = <0x14>;
+			ti,sysc_offs = <0x10>;
 			clocks = <&disp_clk>;
 			clock-names = "fck";
 			#address-cells = <1>;
@@ -909,6 +1152,10 @@
 				reg = <0x4832a400 0x400>;
 				interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 				ti,hwmods = "dss_dispc";
+				ti,idlemodes = <0x77>;
+				ti,sysc_flags = <0x77>;
+				ti,syss_offs = <0x14>;
+				ti,sysc_offs = <0x10>;
 				clocks = <&disp_clk>;
 				clock-names = "fck";
 			};
@@ -917,6 +1164,10 @@
 				compatible = "ti,omap3-rfbi";
 				reg = <0x4832a800 0x100>;
 				ti,hwmods = "dss_rfbi";
+				ti,idlemodes = <0x7>;
+				ti,sysc_flags = <0x23>;
+				ti,syss_offs = <0x14>;
+				ti,sysc_offs = <0x10>;
 				clocks = <&disp_clk>;
 				clock-names = "fck";
 			};
-- 
2.2.0

[RFC/PATCH 7/7] arm: omap: hwmod: 43xx: remove sysc and class data

[Felipe Balbi]

now that this data comes from DT, we can safely
remove them from this file.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c | 61 ------------------------------
 1 file changed, 61 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_43xx_data.c b/arch/arm/mach-omap2/omap_hwmod_43xx_data.c
index 5c6c841..f990782 100644
--- a/arch/arm/mach-omap2/omap_hwmod_43xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_43xx_data.c
@@ -95,22 +95,10 @@ static struct omap_hwmod am43xx_gpio0_hwmod = {
 	.dev_attr	= &gpio_dev_attr,
 };
 
-static struct omap_hwmod_class_sysconfig am43xx_synctimer_sysc = {
-	.rev_offs	= 0x0,
-	.sysc_offs	= 0x4,
-	.sysc_flags	= SYSC_HAS_SIDLEMODE,
-	.idlemodes	= (SIDLE_FORCE | SIDLE_NO),
-	.sysc_fields	= &omap_hwmod_sysc_type1,
-};
 
-static struct omap_hwmod_class am43xx_synctimer_hwmod_class = {
-	.name	= "synctimer",
-	.sysc	= &am43xx_synctimer_sysc,
-};
 
 static struct omap_hwmod am43xx_synctimer_hwmod = {
 	.name		= "counter_32k",
-	.class		= &am43xx_synctimer_hwmod_class,
 	.clkdm_name	= "l4_wkup_aon_clkdm",
 	.flags		= HWMOD_SWSUP_SIDLE,
 	.main_clk	= "synctimer_32kclk",
@@ -318,13 +306,9 @@ static struct omap_hwmod am43xx_gpio5_hwmod = {
 	.dev_attr	= &gpio_dev_attr,
 };
 
-static struct omap_hwmod_class am43xx_ocp2scp_hwmod_class = {
-	.name	= "ocp2scp",
-};
 
 static struct omap_hwmod am43xx_ocp2scp0_hwmod = {
 	.name		= "ocp2scp0",
-	.class		= &am43xx_ocp2scp_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
 	.main_clk	= "l4ls_gclk",
 	.prcm = {
@@ -337,7 +321,6 @@ static struct omap_hwmod am43xx_ocp2scp0_hwmod = {
 
 static struct omap_hwmod am43xx_ocp2scp1_hwmod = {
 	.name		= "ocp2scp1",
-	.class		= &am43xx_ocp2scp_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
 	.main_clk	= "l4ls_gclk",
 	.prcm = {
@@ -348,26 +331,8 @@ static struct omap_hwmod am43xx_ocp2scp1_hwmod = {
 	},
 };
 
-static struct omap_hwmod_class_sysconfig am43xx_usb_otg_ss_sysc = {
-	.rev_offs	= 0x0000,
-	.sysc_offs	= 0x0010,
-	.sysc_flags	= (SYSC_HAS_DMADISABLE | SYSC_HAS_MIDLEMODE |
-				SYSC_HAS_SIDLEMODE),
-	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
-				SIDLE_SMART_WKUP | MSTANDBY_FORCE |
-				MSTANDBY_NO | MSTANDBY_SMART |
-				MSTANDBY_SMART_WKUP),
-	.sysc_fields	= &omap_hwmod_sysc_type2,
-};
-
-static struct omap_hwmod_class am43xx_usb_otg_ss_hwmod_class = {
-	.name	= "usb_otg_ss",
-	.sysc	= &am43xx_usb_otg_ss_sysc,
-};
-
 static struct omap_hwmod am43xx_usb_otg_ss0_hwmod = {
 	.name		= "usb_otg_ss0",
-	.class		= &am43xx_usb_otg_ss_hwmod_class,
 	.clkdm_name	= "l3s_clkdm",
 	.main_clk	= "l3s_gclk",
 	.prcm = {
@@ -380,7 +345,6 @@ static struct omap_hwmod am43xx_usb_otg_ss0_hwmod = {
 
 static struct omap_hwmod am43xx_usb_otg_ss1_hwmod = {
 	.name		= "usb_otg_ss1",
-	.class		= &am43xx_usb_otg_ss_hwmod_class,
 	.clkdm_name	= "l3s_clkdm",
 	.main_clk	= "l3s_gclk",
 	.prcm = {
@@ -391,22 +355,10 @@ static struct omap_hwmod am43xx_usb_otg_ss1_hwmod = {
 	},
 };
 
-static struct omap_hwmod_class_sysconfig am43xx_qspi_sysc = {
-	.sysc_offs      = 0x0010,
-	.sysc_flags     = SYSC_HAS_SIDLEMODE,
-	.idlemodes      = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
-				SIDLE_SMART_WKUP),
-	.sysc_fields    = &omap_hwmod_sysc_type2,
-};
 
-static struct omap_hwmod_class am43xx_qspi_hwmod_class = {
-	.name   = "qspi",
-	.sysc   = &am43xx_qspi_sysc,
-};
 
 static struct omap_hwmod am43xx_qspi_hwmod = {
 	.name           = "qspi",
-	.class          = &am43xx_qspi_hwmod_class,
 	.clkdm_name     = "l3s_clkdm",
 	.main_clk       = "l3s_gclk",
 	.prcm = {
@@ -421,23 +373,10 @@ static struct omap_hwmod am43xx_qspi_hwmod = {
  * 'adc/tsc' class
  * TouchScreen Controller (Analog-To-Digital Converter)
  */
-static struct omap_hwmod_class_sysconfig am43xx_adc_tsc_sysc = {
-	.rev_offs	= 0x00,
-	.sysc_offs	= 0x10,
-	.sysc_flags	= SYSC_HAS_SIDLEMODE,
-	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
-			  SIDLE_SMART_WKUP),
-	.sysc_fields	= &omap_hwmod_sysc_type2,
-};
 
-static struct omap_hwmod_class am43xx_adc_tsc_hwmod_class = {
-	.name		= "adc_tsc",
-	.sysc		= &am43xx_adc_tsc_sysc,
-};
 
 static struct omap_hwmod am43xx_adc_tsc_hwmod = {
 	.name		= "adc_tsc",
-	.class		= &am43xx_adc_tsc_hwmod_class,
 	.clkdm_name	= "l3s_tsc_clkdm",
 	.main_clk	= "adc_tsc_fck",
 	.prcm		= {
-- 
2.2.0

Re: [RFC/PATCH 0/7] arm: omap: move more HWMOD data to DT

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]

On Tue, Dec 09, 2014 at 04:27:45PM -0600, Felipe Balbi wrote:
> Hi,
> 
> here's a more complete series showing the out of debugfs interface.
> 
> So far only AM43xx has been converted and only those HWMODs which
> play well with DT and/or are not common with other OMAPs/AM33xx.
> 
> I plan to work on AM33xx tomorrow, but this RFC shows what we can
> achieve WRT HWMOD static data reduction.
> 
> As you can see, with AM43xx, data reduction is quite minimal, since
> it reuses so much from AM33xx, but well, had to start somewhere.
> 
> All patches were boot tested with AM437x SK on top of:
> 
> commit 1ca7c606de868d172afb4eb65e04e290dbdb51ff
> Author: Stephen Rothwell <sfr@canb.auug.org.au>
> Date:   Thu Dec 4 19:49:10 2014 +1100
> 
>     Add linux-next specific files for 20141204
>     
>     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> cheers
> 
> Felipe Balbi (7):
>   arm: omap: hwmod: add debugfs interface
>   arm: omap: devicetree: add new properties for OMAP devices
>   arm: omap: hwmod: drop 'const' qualifier from omap_hwmod_class name
>   arm: omap: device: add support for generating sysconfig data from DT
>   arm: omap: hwmod: allow for registration of class-less hwmods
>   arm: boot: dts: am4372: add sysconfig data to all HWMODs
>   arm: omap: hwmod: 43xx: remove sysc and class data
> 
>  .../devicetree/bindings/arm/omap/omap.txt          |   7 +
>  arch/arm/boot/dts/am4372.dtsi                      | 251 +++++++++++++++++++
>  arch/arm/mach-omap2/Makefile                       |   2 +-
>  arch/arm/mach-omap2/omap_device.c                  | 117 +++++++++
>  arch/arm/mach-omap2/omap_hwmod.c                   |  30 ++-
>  arch/arm/mach-omap2/omap_hwmod.h                   |  11 +-
>  arch/arm/mach-omap2/omap_hwmod_43xx_data.c         |  61 -----
>  arch/arm/mach-omap2/omap_hwmod_debugfs.c           | 269 +++++++++++++++++++++
>  8 files changed, 673 insertions(+), 75 deletions(-)
>  create mode 100644 arch/arm/mach-omap2/omap_hwmod_debugfs.c

btw, after removing all class and sysconfig static definitions, we get:

$ git diff --stat
 arch/arm/mach-omap2/omap_hwmod_2420_data.c         |  44 --
 arch/arm/mach-omap2/omap_hwmod_2430_data.c         |  70 --
 .../mach-omap2/omap_hwmod_2xxx_3xxx_ipblock_data.c |  42 --
 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c | 188 ------
 .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c | 370 +----------
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |  78 ---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         | 399 ------------
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c         |  37 --
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         | 713 ---------------------
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c         | 469 --------------
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c          | 488 +-------------
 .../mach-omap2/omap_hwmod_common_ipblock_data.c    |  30 +-
 12 files changed, 5 insertions(+), 2923 deletions(-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 4/7] arm: omap: device: add support for generating sysconfig data from DT

[Lokesh Vutla]

Hi Felipe,
On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> After moving sysconfig data to DT, we need to make
> sure we can generate the same data we have today in C
> files out of the DT provided properties.
> 
> This patch adds support for all optional properties
> documented by a previous commit.
> 
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> ---
>  arch/arm/mach-omap2/omap_device.c | 115 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
> index abd622b..ea729aa 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -25,6 +25,7 @@
>   */
>  #undef DEBUG

[...snip..]

> +
> +static int omap_hwmod_init_sysc(struct device *dev,
> +		struct omap_hwmod *oh, int index)
> +{
> +	struct device_node *node = dev->of_node;
> +	struct omap_hwmod_class *class = oh->class;
> +	struct omap_hwmod_class_sysconfig *sysc;
> +	int ret;
> +	int i;
> +	char name[128];
> +	const char *tmp = oh->name;
> +	u32 prop;
> +
> +	/* if data isn't provided by DT, skip */
> +	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(dev))
> +		return 0;
> +
> +	class = kzalloc(sizeof(*class), GFP_KERNEL);
> +	if (!class)
> +		return -ENOMEM;
> +
> +	i = 0;
> +	while (*tmp) {
> +		if (isalpha(*tmp))
> +			name[i++] = *tmp;
> +		tmp++;
> +	}
> +	name[i] = '\0';
> +
> +	class->name = kzalloc(sizeof(name), GFP_KERNEL);
> +	if (!class->name)
> +		return -ENOMEM;
> +	strncpy(class->name, name, sizeof(name) - 1);
> +
> +	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
> +	if (!sysc)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
> +	if (!ret)
> +		sysc->rev_offs = prop;
> +
> +	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
> +	if (!ret)
> +		sysc->sysc_offs = prop;
> +
> +	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
> +	if (!ret)
> +		sysc->syss_offs = prop;
> +
> +	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
> +	if (!ret)
> +		sysc->sysc_flags = prop & 0xffff;
> +
> +	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
> +	if (!ret)
> +		sysc->srst_udelay = prop & 0xff;
> +
> +	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
> +	if (!ret)
> +		sysc->idlemodes = prop & 0xff;
> +
> +	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
> +	if (!ret)
> +		sysc->clockact = prop & 0xff;
> +
> +	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
> +	if (ret)
> +		prop = 1;
> +
> +	switch (prop) {
> +	case 2:
> +		sysc->sysc_fields = &omap_hwmod_sysc_type2;
> +		break;
> +	case 3:
> +		sysc->sysc_fields = &omap_hwmod_sysc_type3;
> +		break;
> +	case 1:
> +	default:
> +		sysc->sysc_fields = &omap_hwmod_sysc_type1;
> +	}
Missed hooking the class to oh?
	class->sysc = sysc;
	oh->class = class;

Thanks and regards,
Lokesh
> +
> +	return 0;
> +}
>  
>  /**
>   * omap_device_build_from_dt - build an omap_device with multiple hwmods
> @@ -154,6 +264,11 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
>  			goto odbfd_exit1;
>  		}
>  		hwmods[i] = oh;
> +
> +		ret = omap_hwmod_init_sysc(&pdev->dev, oh, i);
> +		if (ret)
> +			goto odbfd_exit1;
> +
>  		if (oh->flags & HWMOD_INIT_NO_IDLE)
>  			device_active = true;
>  
> 

--
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

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Lokesh Vutla]

Hi Felipe,

On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> Before this patch, HWMOD requires the existence
> of a struct omap_hwmod_class very early.
Yes, hwmod code looks for omap_hwmod_class entry before registering any hwmod.

With the patch 4/7 omap_hwmod_class gets populated from dt very late after registration of the hwmod.
So all the hwmod which gets class data from dt never gets registered and state is always UNKNOWN.
Mostly making it unusable.
IMO this patch just masks the problem.

In order to register hwmod we need to populate class data very early.
We can populate at the same place as how reg property is being parsed.
Call omap_hwmod_init_sysc() in _init() of the particular hwmod:
The below diff will help:

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index cbb908d..05ecf8a 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2415,6 +2415,116 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
 	return 0;
 }
 
+static int omap_hwmod_has_sysc_bindings(struct device_node *node)
+{
+	char *properties[] = {
+		"ti,rev_offs",
+		"ti,sysc_offs",
+		"ti,syss_offs",
+		"ti,sysc_flags",
+		"ti,srst_udelay",
+		"ti,idlemodes",
+		"ti,clockact",
+		"ti,sysc_type",
+		NULL
+	};
+	char **tmp = properties;
+
+	while (*tmp) {
+		if (of_property_read_bool(node, *tmp)) {
+			return true;
+		}
+		tmp++;
+	}
+
+	return 0;
+}
+
+static int omap_hwmod_init_sysc(struct device_node *node,
+		struct omap_hwmod *oh, int index)
+{
+	struct omap_hwmod_class *class = oh->class;
+	struct omap_hwmod_class_sysconfig *sysc;
+	int ret;
+	int i;
+	char name[128];
+	const char *tmp = oh->name;
+	u32 prop;
+
+	/* if data isn't provided by DT, skip */
+	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(node))
+		return 0;
+
+	class = kzalloc(sizeof(*class), GFP_KERNEL);
+	if (!class)
+		return -ENOMEM;
+
+	i = 0;
+	while (*tmp) {
+		if (isalpha(*tmp))
+			name[i++] = *tmp;
+		tmp++;
+	}
+	name[i] = '\0';
+
+	class->name = kzalloc(sizeof(name), GFP_KERNEL);
+	if (!class->name)
+		return -ENOMEM;
+	strncpy(class->name, name, sizeof(name) - 1);
+
+	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
+	if (!sysc)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
+	if (!ret)
+		sysc->rev_offs = prop;
+
+	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
+	if (!ret)
+		sysc->sysc_offs = prop;
+
+	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
+	if (!ret)
+		sysc->syss_offs = prop;
+
+	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
+	if (!ret)
+		sysc->sysc_flags = prop & 0xffff;
+
+	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
+	if (!ret)
+		sysc->srst_udelay = prop & 0xff;
+
+	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
+	if (!ret)
+		sysc->idlemodes = prop & 0xff;
+
+	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
+	if (!ret)
+		sysc->clockact = prop & 0xff;
+
+	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
+	if (ret)
+		prop = 1;
+
+	switch (prop) {
+	case 2:
+		sysc->sysc_fields = &omap_hwmod_sysc_type2;
+		break;
+	case 3:
+		sysc->sysc_fields = &omap_hwmod_sysc_type3;
+		break;
+	case 1:
+	default:
+		sysc->sysc_fields = &omap_hwmod_sysc_type1;
+	}
+	class->sysc = sysc;
+	oh->class = class;
+
+	return 0;
+}
+
 /**
  * _init - initialize internal data for the hwmod @oh
  * @oh: struct omap_hwmod *
@@ -2449,6 +2559,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 		else if (np && index)
 			pr_warn("omap_hwmod: %s using broken dt data from %s\n",
 				oh->name, np->name);
+
+		if (np) {
+			r = omap_hwmod_init_sysc(np, oh, 0);
+			if (r)
+				pr_warn("omap_hwmod: %s missing sysc dt data\n", oh->name);
+		}
 	}
 
 	if (oh->class->sysc) {
@@ -2673,8 +2789,7 @@ static int __init _setup(struct omap_hwmod *oh, void *data)
  * already has been registered by the same name; -EINVAL if the
  * omap_hwmod is in the wrong state, if @oh is NULL, if the
  * omap_hwmod's class field is NULL; if the omap_hwmod is missing a
- * name, or if the omap_hwmod's class is missing a name; or 0 upon
- * success.
+ * name, or 0 upon success.
  *
  * XXX The data should be copied into bootmem, so the original data
  * should be marked __initdata and freed after init.  This would allow
@@ -2684,8 +2799,7 @@ static int __init _setup(struct omap_hwmod *oh, void *data)
  */
 static int __init _register(struct omap_hwmod *oh)
 {
-	if (!oh || !oh->name || !oh->class || !oh->class->name ||
-	    (oh->_state != _HWMOD_STATE_UNKNOWN))
+	if (!oh || !oh->name || oh->_state != _HWMOD_STATE_UNKNOWN)
 		return -EINVAL;
 
 	pr_debug("omap_hwmod: %s: registering\n", oh->name);

Thanks and regards,
Lokesh


 It just
> so happens that we're moving that data to DeviceTree
> in a follow-up commit so we need to allow the
> registration of HWMODs with a valid omap_hwmod_class
> pointer.
> 
> When the device is built from DT, we will allocate and
> populate struct omap_hwmod_class based on DT data.
> 
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index cbb908d..2355764 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -504,6 +504,9 @@ static int _set_dmadisable(struct omap_hwmod *oh)
>  	u32 v;
>  	u32 dmadisable_mask;
>  
> +	if (!oh->class)
> +		return -EINVAL;
> +
>  	if (!oh->class->sysc ||
>  	    !(oh->class->sysc->sysc_flags & SYSC_HAS_DMADISABLE))
>  		return -EINVAL;
> @@ -1843,7 +1846,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
>  	int c = 0;
>  	int ret = 0;
>  
> -	if (!oh->class->sysc ||
> +	if (!oh->class || !oh->class->sysc ||
>  	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET))
>  		return -ENOENT;
>  
> @@ -1937,12 +1940,13 @@ static int _reset(struct omap_hwmod *oh)
>  
>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>  
> -	if (oh->class->reset) {
> +	if (oh->class && oh->class->reset) {
>  		r = oh->class->reset(oh);
>  	} else {
>  		if (oh->rst_lines_cnt > 0) {
> -			for (i = 0; i < oh->rst_lines_cnt; i++)
> +			for (i = 0; i < oh->rst_lines_cnt; i++) {
>  				_assert_hardreset(oh, oh->rst_lines[i].name);
> +			}
>  			return 0;
>  		} else {
>  			r = _ocp_softreset(oh);
> @@ -1958,7 +1962,7 @@ static int _reset(struct omap_hwmod *oh)
>  	 * softreset.  The _enable() function should be split to avoid
>  	 * the rewrite of the OCP_SYSCONFIG register.
>  	 */
> -	if (oh->class->sysc) {
> +	if (oh->class && oh->class->sysc) {
>  		_update_sysc_cache(oh);
>  		_enable_sysc(oh);
>  	}
> @@ -2036,7 +2040,7 @@ static int _omap4_get_context_lost(struct omap_hwmod *oh)
>   */
>  static int _enable_preprogram(struct omap_hwmod *oh)
>  {
> -	if (!oh->class->enable_preprogram)
> +	if (!oh->class || !oh->class->enable_preprogram)
>  		return 0;
>  
>  	return oh->class->enable_preprogram(oh);
> @@ -2145,7 +2149,7 @@ static int _enable(struct omap_hwmod *oh)
>  		oh->_state = _HWMOD_STATE_ENABLED;
>  
>  		/* Access the sysconfig only if the target is ready */
> -		if (oh->class->sysc) {
> +		if (oh->class && oh->class->sysc) {
>  			if (!(oh->_int_flags & _HWMOD_SYSCONFIG_LOADED))
>  				_update_sysc_cache(oh);
>  			_enable_sysc(oh);
> @@ -2186,7 +2190,7 @@ static int _idle(struct omap_hwmod *oh)
>  	if (_are_all_hardreset_lines_asserted(oh))
>  		return 0;
>  
> -	if (oh->class->sysc)
> +	if (oh->class && oh->class->sysc)
>  		_idle_sysc(oh);
>  	_del_initiator_dep(oh, mpu_oh);
>  
> @@ -2244,7 +2248,7 @@ static int _shutdown(struct omap_hwmod *oh)
>  
>  	pr_debug("omap_hwmod: %s: disabling\n", oh->name);
>  
> -	if (oh->class->pre_shutdown) {
> +	if (oh->class && oh->class->pre_shutdown) {
>  		prev_state = oh->_state;
>  		if (oh->_state == _HWMOD_STATE_IDLE)
>  			_enable(oh);
> @@ -2256,7 +2260,7 @@ static int _shutdown(struct omap_hwmod *oh)
>  		}
>  	}
>  
> -	if (oh->class->sysc) {
> +	if (oh->class && oh->class->sysc) {
>  		if (oh->_state == _HWMOD_STATE_IDLE)
>  			_enable(oh);
>  		_shutdown_sysc(oh);
> @@ -2451,7 +2455,7 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>  				oh->name, np->name);
>  	}
>  
> -	if (oh->class->sysc) {
> +	if (oh->class && oh->class->sysc) {
>  		r = _init_mpu_rt_base(oh, NULL, index, np);
>  		if (r < 0) {
>  			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
> @@ -2684,8 +2688,7 @@ static int __init _setup(struct omap_hwmod *oh, void *data)
>   */
>  static int __init _register(struct omap_hwmod *oh)
>  {
> -	if (!oh || !oh->name || !oh->class || !oh->class->name ||
> -	    (oh->_state != _HWMOD_STATE_UNKNOWN))
> +	if (!oh || !oh->name || (oh->_state != _HWMOD_STATE_UNKNOWN))
>  		return -EINVAL;
>  
>  	pr_debug("omap_hwmod: %s: registering\n", oh->name);
> @@ -3942,6 +3945,9 @@ int omap_hwmod_for_each_by_class(const char *classname,
>  		 __func__, classname);
>  
>  	list_for_each_entry(temp_oh, &omap_hwmod_list, node) {
> +		if (!temp_oh->class)
> +			continue;
> +
>  		if (!strcmp(temp_oh->class->name, classname)) {
>  			pr_debug("omap_hwmod: %s: %s: calling callback fn\n",
>  				 __func__, temp_oh->name);
> 

--
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

Re: [RFC/PATCH 2/7] arm: omap: devicetree: add new properties for OMAP devices

[Lokesh Vutla]

Hi Felipe,

On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> In order to get rid of some more hwmod data, we
> introduce a few extra properties to OMAP DT
> data in order to be able to pass the needed
> information through DT.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  Documentation/devicetree/bindings/arm/omap/omap.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> index 4f6a82c..c6b9515 100644
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -23,6 +23,13 @@ Optional properties:
>    during suspend.
>  - ti,no-reset-on-init: When present, the module should not be reset at init
>  - ti,no-idle-on-init: When present, the module should not be idled at init
> +- ti,rev_offs: IP block revision register offset (defaults to 0)
> +- ti,sysc_offs: OCP_SYSCONFIG register offset (defaults to 0)
> +- ti,syss_offs: OCP_SYSSTATUS register offset (defaults to 0)
> +- ti,srst_udelay: Delay needed after a softreset in usecs (defaults to 0)
> +- ti,idlemodes: Slave and Master supported idling modes
> +- ti,clockact: default value of clock activity bits (defaults to 0)
> +- ti,sysc_type: OCP_SYSCONFIG type. Valid types are 1, 2 or 3 (defaults to 1)
Once if all the hwmod data comes to dt it will be very dufficult to see all the data
together. Can we separate out hwmod class data like clocks data is separated out into different node
or something of the sort like below?

	ti, hwmod_class {
		ti,class_name = <>;
		ti,rev_offs = <>;
		ti,sysc_offs = <>;
		ti,syss_offs = <>;
		ti,srst_udelay = <>;
		ti,idlemodes = <>;
		ti,clockact = <>;
		ti,sysc_type = <>;
	};

example:
	counter32k: counter@44e86000 {
			compatible = "ti,am4372-counter32k","ti,omap-counter32k";
			reg = <0x44e86000 0x40>;
			ti,hwmods = "counter_32k";
			ti,hwmod_class {
				ti,class_name = "synctimer";
				ti,idlemodes = <0x3>;
				ti,sysc_flags = <0x20>;
				ti,sysc_offs = <0x4>;
			};
	};

Thanks and regards,
Lokesh
		
>  
>  Example:
>  
> 

Re: [RFC/PATCH 4/7] arm: omap: device: add support for generating sysconfig data from DT

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Hi,

adding linux-omap back

On Wed, Dec 10, 2014 at 04:19:06PM +0530, Lokesh Vutla wrote:
> > +	switch (prop) {
> > +	case 2:
> > +		sysc->sysc_fields = &omap_hwmod_sysc_type2;
> > +		break;
> > +	case 3:
> > +		sysc->sysc_fields = &omap_hwmod_sysc_type3;
> > +		break;
> > +	case 1:
> > +	default:
> > +		sysc->sysc_fields = &omap_hwmod_sysc_type1;
> > +	}
> Missed hooking the class to oh?
> 	class->sysc = sysc;

yeah, I'll fix that.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4450 bytes --]

Hi,

(adding linux-omap back to the loop)

On Wed, Dec 10, 2014 at 04:20:30PM +0530, Lokesh Vutla wrote:
> Hi Felipe,
> 
> On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > Before this patch, HWMOD requires the existence
> > of a struct omap_hwmod_class very early.
> Yes, hwmod code looks for omap_hwmod_class entry before registering any hwmod.
> 
> With the patch 4/7 omap_hwmod_class gets populated from dt very late after registration of the hwmod.
> So all the hwmod which gets class data from dt never gets registered and state is always UNKNOWN.
> Mostly making it unusable.
> IMO this patch just masks the problem.
> 
> In order to register hwmod we need to populate class data very early.
> We can populate at the same place as how reg property is being parsed.
> Call omap_hwmod_init_sysc() in _init() of the particular hwmod:
> The below diff will help:
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index cbb908d..05ecf8a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2415,6 +2415,116 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>  	return 0;
>  }
>  
> +static int omap_hwmod_has_sysc_bindings(struct device_node *node)
> +{
> +	char *properties[] = {
> +		"ti,rev_offs",
> +		"ti,sysc_offs",
> +		"ti,syss_offs",
> +		"ti,sysc_flags",
> +		"ti,srst_udelay",
> +		"ti,idlemodes",
> +		"ti,clockact",
> +		"ti,sysc_type",
> +		NULL
> +	};
> +	char **tmp = properties;
> +
> +	while (*tmp) {
> +		if (of_property_read_bool(node, *tmp)) {
> +			return true;
> +		}
> +		tmp++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int omap_hwmod_init_sysc(struct device_node *node,
> +		struct omap_hwmod *oh, int index)
> +{
> +	struct omap_hwmod_class *class = oh->class;
> +	struct omap_hwmod_class_sysconfig *sysc;
> +	int ret;
> +	int i;
> +	char name[128];
> +	const char *tmp = oh->name;
> +	u32 prop;
> +
> +	/* if data isn't provided by DT, skip */
> +	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(node))
> +		return 0;
> +
> +	class = kzalloc(sizeof(*class), GFP_KERNEL);
> +	if (!class)
> +		return -ENOMEM;
> +
> +	i = 0;
> +	while (*tmp) {
> +		if (isalpha(*tmp))
> +			name[i++] = *tmp;
> +		tmp++;
> +	}
> +	name[i] = '\0';
> +
> +	class->name = kzalloc(sizeof(name), GFP_KERNEL);
> +	if (!class->name)
> +		return -ENOMEM;
> +	strncpy(class->name, name, sizeof(name) - 1);
> +
> +	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
> +	if (!sysc)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
> +	if (!ret)
> +		sysc->rev_offs = prop;
> +
> +	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
> +	if (!ret)
> +		sysc->sysc_offs = prop;
> +
> +	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
> +	if (!ret)
> +		sysc->syss_offs = prop;
> +
> +	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
> +	if (!ret)
> +		sysc->sysc_flags = prop & 0xffff;
> +
> +	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
> +	if (!ret)
> +		sysc->srst_udelay = prop & 0xff;
> +
> +	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
> +	if (!ret)
> +		sysc->idlemodes = prop & 0xff;
> +
> +	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
> +	if (!ret)
> +		sysc->clockact = prop & 0xff;
> +
> +	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
> +	if (ret)
> +		prop = 1;
> +
> +	switch (prop) {
> +	case 2:
> +		sysc->sysc_fields = &omap_hwmod_sysc_type2;
> +		break;
> +	case 3:
> +		sysc->sysc_fields = &omap_hwmod_sysc_type3;
> +		break;
> +	case 1:
> +	default:
> +		sysc->sysc_fields = &omap_hwmod_sysc_type1;
> +	}
> +	class->sysc = sysc;
> +	oh->class = class;
> +
> +	return 0;
> +}
> +
>  /**
>   * _init - initialize internal data for the hwmod @oh
>   * @oh: struct omap_hwmod *
> @@ -2449,6 +2559,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>  		else if (np && index)
>  			pr_warn("omap_hwmod: %s using broken dt data from %s\n",
>  				oh->name, np->name);
> +
> +		if (np) {
> +			r = omap_hwmod_init_sysc(np, oh, 0);

this won't handle any binding which lists more than one hwmod on its
ti,hwmods property.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 2/7] arm: omap: devicetree: add new properties for OMAP devices

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]

Hi,

On Wed, Dec 10, 2014 at 04:37:19PM +0530, Lokesh Vutla wrote:
> Hi Felipe,
> 
> On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > In order to get rid of some more hwmod data, we
> > introduce a few extra properties to OMAP DT
> > data in order to be able to pass the needed
> > information through DT.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  Documentation/devicetree/bindings/arm/omap/omap.txt | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> > index 4f6a82c..c6b9515 100644
> > --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> > +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> > @@ -23,6 +23,13 @@ Optional properties:
> >    during suspend.
> >  - ti,no-reset-on-init: When present, the module should not be reset at init
> >  - ti,no-idle-on-init: When present, the module should not be idled at init
> > +- ti,rev_offs: IP block revision register offset (defaults to 0)
> > +- ti,sysc_offs: OCP_SYSCONFIG register offset (defaults to 0)
> > +- ti,syss_offs: OCP_SYSSTATUS register offset (defaults to 0)
> > +- ti,srst_udelay: Delay needed after a softreset in usecs (defaults to 0)
> > +- ti,idlemodes: Slave and Master supported idling modes
> > +- ti,clockact: default value of clock activity bits (defaults to 0)
> > +- ti,sysc_type: OCP_SYSCONFIG type. Valid types are 1, 2 or 3 (defaults to 1)
> Once if all the hwmod data comes to dt it will be very dufficult to see all the data
> together. Can we separate out hwmod class data like clocks data is separated out into different node
> or something of the sort like below?
> 
> 	ti, hwmod_class {
> 		ti,class_name = <>;

this can be done, but I don't want to pass a class name. It's anyways
just the hwmod name without the digits. For those which are not, I would
rather change the class name to match.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 2/7] arm: omap: devicetree: add new properties for OMAP devices

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 4252 bytes --]

Hi,

On Wed, Dec 10, 2014 at 09:00:49AM -0600, Felipe Balbi wrote:
> On Wed, Dec 10, 2014 at 04:37:19PM +0530, Lokesh Vutla wrote:
> > On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > > In order to get rid of some more hwmod data, we
> > > introduce a few extra properties to OMAP DT
> > > data in order to be able to pass the needed
> > > information through DT.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > >  Documentation/devicetree/bindings/arm/omap/omap.txt | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> > > index 4f6a82c..c6b9515 100644
> > > --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> > > +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> > > @@ -23,6 +23,13 @@ Optional properties:
> > >    during suspend.
> > >  - ti,no-reset-on-init: When present, the module should not be reset at init
> > >  - ti,no-idle-on-init: When present, the module should not be idled at init
> > > +- ti,rev_offs: IP block revision register offset (defaults to 0)
> > > +- ti,sysc_offs: OCP_SYSCONFIG register offset (defaults to 0)
> > > +- ti,syss_offs: OCP_SYSSTATUS register offset (defaults to 0)
> > > +- ti,srst_udelay: Delay needed after a softreset in usecs (defaults to 0)
> > > +- ti,idlemodes: Slave and Master supported idling modes
> > > +- ti,clockact: default value of clock activity bits (defaults to 0)
> > > +- ti,sysc_type: OCP_SYSCONFIG type. Valid types are 1, 2 or 3 (defaults to 1)
> > Once if all the hwmod data comes to dt it will be very dufficult to see all the data
> > together. Can we separate out hwmod class data like clocks data is separated out into different node
> > or something of the sort like below?
> > 
> > 	ti, hwmod_class {
> > 		ti,class_name = <>;
> 
> this can be done, but I don't want to pass a class name. It's anyways
> just the hwmod name without the digits. For those which are not, I would
> rather change the class name to match.

I did some initial thinking on this some weeks ago, too (and then
was short of time, so I did not continue). My suggestion for the
binding would be:

* OCP_SYSCONFIG (power-management of IP-Cores connected to OMAP processors)

Each IP-Core connected to the bus of OMAP processors has
three registers, which specify the IP-Core's version, its
status and setup of PM features.

Required Properties:
- ti,prcm-type: must be one of the following:
   1 for OMAP2+ register style,
   2 for OMAP4+ register style,
   3 for AM33xx register style
- reg: offset to revision, config and status registers
       relative to module base address

Optional Properties:
- ti,idlemodes:                bit field of flags (SIDLE)
   PRCM_IDLE_FORCE               (1 << 0)
   PRCM_IDLE_NO                  (1 << 1)
   PRCM_IDLE_SMART               (1 << 2)
   PRCM_IDLE_SMART_WKUP          (1 << 3)
- ti,standbymodes:             bit field of flags (MIDLE)
   PRCM_STANDBY_FORCE            (1 << 0)
   PRCM_STANDBY_NO               (1 << 1)
   PRCM_STANDBY_SMART            (1 << 2)
   PRCM_STANDBY_SMART_WKUP       (1 << 3)
- ti,sysc-has-autoidle:        config register has AUTOIDLE bit
- ti,sysc-has-softreset:       config register has SOFTRESET bit
- ti,sysc-has-enawakeup:       config register has ENAWAKEUP bit
- ti,sysc-has-emufree:         config register has EMUFREE bit
- ti,sysc-has-clock-activity:  config register has CLOCKACTIVITY bit
- ti,sysc-has-dma-disable:     config register has DMADISABLE bit
- ti,sysc-has-reset-status:    config register has RESETDONE bit
- ti,syss-has-reset-status:    status register has RESETDONE bit
- ti,reset-delay-us:           reset delay in us

Example:

ocp {
	gpio1: gpio@48310000 {
		compatible = "ti,omap3-gpio";

		... /* IP-Core specific properties */

		ti,sysconfig {
			ti,prcm-type = <1>;
			reg = <0x00>, <0x10>, <0x14>;
			ti,idlemodes = <(PRCM_IDLE_FORCE | PRCM_IDLE_NO | PRCM_IDLE_SMART)>;
			ti,sysc-has-enawakeup;
			ti,sysc-has-autoidle;
			ti,sysc-has-softreset;
			ti,syss-has-reset-status;
		};
	};
};

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 5066 bytes --]

On Wed, Dec 10, 2014 at 08:54:33AM -0600, Felipe Balbi wrote:
> Hi,
> 
> (adding linux-omap back to the loop)
> 
> On Wed, Dec 10, 2014 at 04:20:30PM +0530, Lokesh Vutla wrote:
> > Hi Felipe,
> > 
> > On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > > Before this patch, HWMOD requires the existence
> > > of a struct omap_hwmod_class very early.
> > Yes, hwmod code looks for omap_hwmod_class entry before registering any hwmod.
> > 
> > With the patch 4/7 omap_hwmod_class gets populated from dt very late after registration of the hwmod.
> > So all the hwmod which gets class data from dt never gets registered and state is always UNKNOWN.
> > Mostly making it unusable.
> > IMO this patch just masks the problem.
> > 
> > In order to register hwmod we need to populate class data very early.
> > We can populate at the same place as how reg property is being parsed.
> > Call omap_hwmod_init_sysc() in _init() of the particular hwmod:
> > The below diff will help:
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> > index cbb908d..05ecf8a 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > @@ -2415,6 +2415,116 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> >  	return 0;
> >  }
> >  
> > +static int omap_hwmod_has_sysc_bindings(struct device_node *node)
> > +{
> > +	char *properties[] = {
> > +		"ti,rev_offs",
> > +		"ti,sysc_offs",
> > +		"ti,syss_offs",
> > +		"ti,sysc_flags",
> > +		"ti,srst_udelay",
> > +		"ti,idlemodes",
> > +		"ti,clockact",
> > +		"ti,sysc_type",
> > +		NULL
> > +	};
> > +	char **tmp = properties;
> > +
> > +	while (*tmp) {
> > +		if (of_property_read_bool(node, *tmp)) {
> > +			return true;
> > +		}
> > +		tmp++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int omap_hwmod_init_sysc(struct device_node *node,
> > +		struct omap_hwmod *oh, int index)
> > +{
> > +	struct omap_hwmod_class *class = oh->class;
> > +	struct omap_hwmod_class_sysconfig *sysc;
> > +	int ret;
> > +	int i;
> > +	char name[128];
> > +	const char *tmp = oh->name;
> > +	u32 prop;
> > +
> > +	/* if data isn't provided by DT, skip */
> > +	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(node))
> > +		return 0;
> > +
> > +	class = kzalloc(sizeof(*class), GFP_KERNEL);
> > +	if (!class)
> > +		return -ENOMEM;
> > +
> > +	i = 0;
> > +	while (*tmp) {
> > +		if (isalpha(*tmp))
> > +			name[i++] = *tmp;
> > +		tmp++;
> > +	}
> > +	name[i] = '\0';
> > +
> > +	class->name = kzalloc(sizeof(name), GFP_KERNEL);
> > +	if (!class->name)
> > +		return -ENOMEM;
> > +	strncpy(class->name, name, sizeof(name) - 1);
> > +
> > +	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
> > +	if (!sysc)
> > +		return -ENOMEM;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
> > +	if (!ret)
> > +		sysc->rev_offs = prop;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
> > +	if (!ret)
> > +		sysc->sysc_offs = prop;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
> > +	if (!ret)
> > +		sysc->syss_offs = prop;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
> > +	if (!ret)
> > +		sysc->sysc_flags = prop & 0xffff;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
> > +	if (!ret)
> > +		sysc->srst_udelay = prop & 0xff;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
> > +	if (!ret)
> > +		sysc->idlemodes = prop & 0xff;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
> > +	if (!ret)
> > +		sysc->clockact = prop & 0xff;
> > +
> > +	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
> > +	if (ret)
> > +		prop = 1;
> > +
> > +	switch (prop) {
> > +	case 2:
> > +		sysc->sysc_fields = &omap_hwmod_sysc_type2;
> > +		break;
> > +	case 3:
> > +		sysc->sysc_fields = &omap_hwmod_sysc_type3;
> > +		break;
> > +	case 1:
> > +	default:
> > +		sysc->sysc_fields = &omap_hwmod_sysc_type1;
> > +	}
> > +	class->sysc = sysc;
> > +	oh->class = class;
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * _init - initialize internal data for the hwmod @oh
> >   * @oh: struct omap_hwmod *
> > @@ -2449,6 +2559,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
> >  		else if (np && index)
> >  			pr_warn("omap_hwmod: %s using broken dt data from %s\n",
> >  				oh->name, np->name);
> > +
> > +		if (np) {
> > +			r = omap_hwmod_init_sysc(np, oh, 0);
> 
> this won't handle any binding which lists more than one hwmod on its
> ti,hwmods property.

I think Tony planned to remove this kind of multi hwmod references.

IMHO instead of DT referencing the hwmod stuff using ti,hwmods the
hwmod database should reference the compatible values. This depends
on omap3 being DT only of course.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 2/7] arm: omap: devicetree: add new properties for OMAP devices

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4670 bytes --]

Hi,

On Thu, Dec 11, 2014 at 01:46:28AM +0100, Sebastian Reichel wrote:
> On Wed, Dec 10, 2014 at 09:00:49AM -0600, Felipe Balbi wrote:
> > On Wed, Dec 10, 2014 at 04:37:19PM +0530, Lokesh Vutla wrote:
> > > On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > > > In order to get rid of some more hwmod data, we
> > > > introduce a few extra properties to OMAP DT
> > > > data in order to be able to pass the needed
> > > > information through DT.
> > > > 
> > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/omap/omap.txt | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> > > > index 4f6a82c..c6b9515 100644
> > > > --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> > > > +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> > > > @@ -23,6 +23,13 @@ Optional properties:
> > > >    during suspend.
> > > >  - ti,no-reset-on-init: When present, the module should not be reset at init
> > > >  - ti,no-idle-on-init: When present, the module should not be idled at init
> > > > +- ti,rev_offs: IP block revision register offset (defaults to 0)
> > > > +- ti,sysc_offs: OCP_SYSCONFIG register offset (defaults to 0)
> > > > +- ti,syss_offs: OCP_SYSSTATUS register offset (defaults to 0)
> > > > +- ti,srst_udelay: Delay needed after a softreset in usecs (defaults to 0)
> > > > +- ti,idlemodes: Slave and Master supported idling modes
> > > > +- ti,clockact: default value of clock activity bits (defaults to 0)
> > > > +- ti,sysc_type: OCP_SYSCONFIG type. Valid types are 1, 2 or 3 (defaults to 1)
> > > Once if all the hwmod data comes to dt it will be very dufficult to see all the data
> > > together. Can we separate out hwmod class data like clocks data is separated out into different node
> > > or something of the sort like below?
> > > 
> > > 	ti, hwmod_class {
> > > 		ti,class_name = <>;
> > 
> > this can be done, but I don't want to pass a class name. It's anyways
> > just the hwmod name without the digits. For those which are not, I would
> > rather change the class name to match.
> 
> I did some initial thinking on this some weeks ago, too (and then
> was short of time, so I did not continue). My suggestion for the
> binding would be:
> 
> * OCP_SYSCONFIG (power-management of IP-Cores connected to OMAP processors)
> 
> Each IP-Core connected to the bus of OMAP processors has
> three registers, which specify the IP-Core's version, its
> status and setup of PM features.
> 
> Required Properties:
> - ti,prcm-type: must be one of the following:
>    1 for OMAP2+ register style,
>    2 for OMAP4+ register style,
>    3 for AM33xx register style
> - reg: offset to revision, config and status registers
>        relative to module base address
> 
> Optional Properties:
> - ti,idlemodes:                bit field of flags (SIDLE)
>    PRCM_IDLE_FORCE               (1 << 0)
>    PRCM_IDLE_NO                  (1 << 1)
>    PRCM_IDLE_SMART               (1 << 2)
>    PRCM_IDLE_SMART_WKUP          (1 << 3)
> - ti,standbymodes:             bit field of flags (MIDLE)
>    PRCM_STANDBY_FORCE            (1 << 0)
>    PRCM_STANDBY_NO               (1 << 1)
>    PRCM_STANDBY_SMART            (1 << 2)
>    PRCM_STANDBY_SMART_WKUP       (1 << 3)
> - ti,sysc-has-autoidle:        config register has AUTOIDLE bit
> - ti,sysc-has-softreset:       config register has SOFTRESET bit
> - ti,sysc-has-enawakeup:       config register has ENAWAKEUP bit
> - ti,sysc-has-emufree:         config register has EMUFREE bit
> - ti,sysc-has-clock-activity:  config register has CLOCKACTIVITY bit
> - ti,sysc-has-dma-disable:     config register has DMADISABLE bit
> - ti,sysc-has-reset-status:    config register has RESETDONE bit
> - ti,syss-has-reset-status:    status register has RESETDONE bit

I thought about having boolean flags like this but after talking to Tony
we agreed that it would just increase the amount of string parsing
during early initialization. Besides, why have two properties using an
integer bitfield and another huge number of boolean properties ?

> - ti,reset-delay-us:           reset delay in us
> 
> Example:
> 
> ocp {
> 	gpio1: gpio@48310000 {
> 		compatible = "ti,omap3-gpio";
> 
> 		... /* IP-Core specific properties */
> 
> 		ti,sysconfig {

this is what I'm still waiting for comments from Tony. I'm not convinced
we need this extra indentation level. It really brings nothing of value.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 5660 bytes --]

Hi,

On Thu, Dec 11, 2014 at 01:52:38AM +0100, Sebastian Reichel wrote:
> On Wed, Dec 10, 2014 at 08:54:33AM -0600, Felipe Balbi wrote:
> > Hi,
> > 
> > (adding linux-omap back to the loop)
> > 
> > On Wed, Dec 10, 2014 at 04:20:30PM +0530, Lokesh Vutla wrote:
> > > Hi Felipe,
> > > 
> > > On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > > > Before this patch, HWMOD requires the existence
> > > > of a struct omap_hwmod_class very early.
> > > Yes, hwmod code looks for omap_hwmod_class entry before registering any hwmod.
> > > 
> > > With the patch 4/7 omap_hwmod_class gets populated from dt very late after registration of the hwmod.
> > > So all the hwmod which gets class data from dt never gets registered and state is always UNKNOWN.
> > > Mostly making it unusable.
> > > IMO this patch just masks the problem.
> > > 
> > > In order to register hwmod we need to populate class data very early.
> > > We can populate at the same place as how reg property is being parsed.
> > > Call omap_hwmod_init_sysc() in _init() of the particular hwmod:
> > > The below diff will help:
> > >
> > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> > > index cbb908d..05ecf8a 100644
> > > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > > @@ -2415,6 +2415,116 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> > >  	return 0;
> > >  }
> > >  
> > > +static int omap_hwmod_has_sysc_bindings(struct device_node *node)
> > > +{
> > > +	char *properties[] = {
> > > +		"ti,rev_offs",
> > > +		"ti,sysc_offs",
> > > +		"ti,syss_offs",
> > > +		"ti,sysc_flags",
> > > +		"ti,srst_udelay",
> > > +		"ti,idlemodes",
> > > +		"ti,clockact",
> > > +		"ti,sysc_type",
> > > +		NULL
> > > +	};
> > > +	char **tmp = properties;
> > > +
> > > +	while (*tmp) {
> > > +		if (of_property_read_bool(node, *tmp)) {
> > > +			return true;
> > > +		}
> > > +		tmp++;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int omap_hwmod_init_sysc(struct device_node *node,
> > > +		struct omap_hwmod *oh, int index)
> > > +{
> > > +	struct omap_hwmod_class *class = oh->class;
> > > +	struct omap_hwmod_class_sysconfig *sysc;
> > > +	int ret;
> > > +	int i;
> > > +	char name[128];
> > > +	const char *tmp = oh->name;
> > > +	u32 prop;
> > > +
> > > +	/* if data isn't provided by DT, skip */
> > > +	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(node))
> > > +		return 0;
> > > +
> > > +	class = kzalloc(sizeof(*class), GFP_KERNEL);
> > > +	if (!class)
> > > +		return -ENOMEM;
> > > +
> > > +	i = 0;
> > > +	while (*tmp) {
> > > +		if (isalpha(*tmp))
> > > +			name[i++] = *tmp;
> > > +		tmp++;
> > > +	}
> > > +	name[i] = '\0';
> > > +
> > > +	class->name = kzalloc(sizeof(name), GFP_KERNEL);
> > > +	if (!class->name)
> > > +		return -ENOMEM;
> > > +	strncpy(class->name, name, sizeof(name) - 1);
> > > +
> > > +	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
> > > +	if (!sysc)
> > > +		return -ENOMEM;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
> > > +	if (!ret)
> > > +		sysc->rev_offs = prop;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
> > > +	if (!ret)
> > > +		sysc->sysc_offs = prop;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
> > > +	if (!ret)
> > > +		sysc->syss_offs = prop;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
> > > +	if (!ret)
> > > +		sysc->sysc_flags = prop & 0xffff;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
> > > +	if (!ret)
> > > +		sysc->srst_udelay = prop & 0xff;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
> > > +	if (!ret)
> > > +		sysc->idlemodes = prop & 0xff;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
> > > +	if (!ret)
> > > +		sysc->clockact = prop & 0xff;
> > > +
> > > +	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
> > > +	if (ret)
> > > +		prop = 1;
> > > +
> > > +	switch (prop) {
> > > +	case 2:
> > > +		sysc->sysc_fields = &omap_hwmod_sysc_type2;
> > > +		break;
> > > +	case 3:
> > > +		sysc->sysc_fields = &omap_hwmod_sysc_type3;
> > > +		break;
> > > +	case 1:
> > > +	default:
> > > +		sysc->sysc_fields = &omap_hwmod_sysc_type1;
> > > +	}
> > > +	class->sysc = sysc;
> > > +	oh->class = class;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  /**
> > >   * _init - initialize internal data for the hwmod @oh
> > >   * @oh: struct omap_hwmod *
> > > @@ -2449,6 +2559,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
> > >  		else if (np && index)
> > >  			pr_warn("omap_hwmod: %s using broken dt data from %s\n",
> > >  				oh->name, np->name);
> > > +
> > > +		if (np) {
> > > +			r = omap_hwmod_init_sysc(np, oh, 0);
> > 
> > this won't handle any binding which lists more than one hwmod on its
> > ti,hwmods property.
> 
> I think Tony planned to remove this kind of multi hwmod references.

true, but I'm still a bit iffy wrt that. ABI compatibility breaks and
all

> IMHO instead of DT referencing the hwmod stuff using ti,hwmods the
> hwmod database should reference the compatible values. This depends
> on omap3 being DT only of course.

don't you think it's too late for that ? We need to support the current
form of dts files forever. It's an ABI afterall.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 2/7] arm: omap: devicetree: add new properties for OMAP devices

[Tony Lindgren]

Hi,

* Felipe Balbi <balbi@ti.com> [141211 06:24]:
> On Thu, Dec 11, 2014 at 01:46:28AM +0100, Sebastian Reichel wrote:
> > 
> > Each IP-Core connected to the bus of OMAP processors has
> > three registers, which specify the IP-Core's version, its
> > status and setup of PM features.
> > 
> > Required Properties:
> > - ti,prcm-type: must be one of the following:
> >    1 for OMAP2+ register style,
> >    2 for OMAP4+ register style,
> >    3 for AM33xx register style

This is the simple part and tells the type of the wiring of
the device :)

> > - reg: offset to revision, config and status registers
> >        relative to module base address

I don't think we should set up the sysconf registers as a child
of the device. These registers are not a child of the device on
the bus. They are sprinkled within the driver reg space. And some
of these sysc registers are way into the driver registers, it's
not always 0-0x10-0x14 layout for the sysconfig register offsets.

> > Optional Properties:
> > - ti,idlemodes:                bit field of flags (SIDLE)
> >    PRCM_IDLE_FORCE               (1 << 0)
> >    PRCM_IDLE_NO                  (1 << 1)
> >    PRCM_IDLE_SMART               (1 << 2)
> >    PRCM_IDLE_SMART_WKUP          (1 << 3)
> > - ti,standbymodes:             bit field of flags (MIDLE)
> >    PRCM_STANDBY_FORCE            (1 << 0)
> >    PRCM_STANDBY_NO               (1 << 1)
> >    PRCM_STANDBY_SMART            (1 << 2)
> >    PRCM_STANDBY_SMART_WKUP       (1 << 3)

These look OK to me. They describe the features available in the
sysconfig registers that map in a different way depending of the
sysconfig type 1, 2 or 3.

Too bad we cannot autoprobe this information from the hardware.

> > - ti,sysc-has-autoidle:        config register has AUTOIDLE bit
> > - ti,sysc-has-softreset:       config register has SOFTRESET bit
> > - ti,sysc-has-enawakeup:       config register has ENAWAKEUP bit
> > - ti,sysc-has-emufree:         config register has EMUFREE bit
> > - ti,sysc-has-clock-activity:  config register has CLOCKACTIVITY bit
> > - ti,sysc-has-dma-disable:     config register has DMADISABLE bit
> > - ti,sysc-has-reset-status:    config register has RESETDONE bit
> > - ti,syss-has-reset-status:    status register has RESETDONE bit
> 
> I thought about having boolean flags like this but after talking to Tony
> we agreed that it would just increase the amount of string parsing
> during early initialization. Besides, why have two properties using an
> integer bitfield and another huge number of boolean properties ?

Yeah setting up individual register bits as strings is just too
much java like.. Setting them up like this means we are doing
devices * nr_sysc_feature_bits of string parsing. So I'd like to
avoid that by using bits like Felipe has done now that we have the
dts preprocessing happening.

> > - ti,reset-delay-us:           reset delay in us
> > 
> > Example:
> > 
> > ocp {
> > 	gpio1: gpio@48310000 {
> > 		compatible = "ti,omap3-gpio";
> > 
> > 		... /* IP-Core specific properties */
> > 
> > 		ti,sysconfig {
> 
> this is what I'm still waiting for comments from Tony. I'm not convinced
> we need this extra indentation level. It really brings nothing of value.

Me neither. It's also wrong from the register mapping point of
view like I explained above.

Cheers,

Tony

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [141210 16:55]:
> On Wed, Dec 10, 2014 at 08:54:33AM -0600, Felipe Balbi wrote:
> > > @@ -2449,6 +2559,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
> > >  		else if (np && index)
> > >  			pr_warn("omap_hwmod: %s using broken dt data from %s\n",
> > >  				oh->name, np->name);
> > > +
> > > +		if (np) {
> > > +			r = omap_hwmod_init_sysc(np, oh, 0);
> > 
> > this won't handle any binding which lists more than one hwmod on its
> > ti,hwmods property.
> 
> I think Tony planned to remove this kind of multi hwmod references.

Yes we really should do that. But that involves repairs in the drivers,
dts files, and hwmod.

We really should aim for 1 to 1 to 1 mapping of the dts compatible
property to device hwmod to device sysc registers.

Once all the devices are mapped that way, we can deprecate
ti,hwmods in the dts files and set it up automatically based on
the compatible and the instance number of the device.

For an example of how to fix the interlaced device dts entries,
let's look at the EDMA. Instead of the current setup in
am33xx.dtsi:

edma: edma@49000000 {
	compatible = "ti,edma3";
	ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
	reg =   <0x49000000 0x10000>,
		<0x44e10f90 0x40>;
		interrupts = <12 13 14>;
		#dma-cells = <1>;
};

It really should be set up in the following way (eventually
without ti,hwmods):

edma: edma@49000000 {
	compatible = "ti,edma3";
	reg =   <0x49000000 0x10000>,
		<0x44e10f90 0x40>;
	interrupts = <12>;
	#dma-cells = <1>;
};

tptc0 {
	compatible = "ti,edma3-tptc";
	reg = <0x49800000 0x100000>;
	interrupts = <13>;
};

tptc1 {
	compatible = "ti,edma3-tptc";
	reg = <0x49900000 0x100000>;
	interrupts = <14>;
};

tptc2 {
	compatible = "ti,edma3-tptc";
	reg = <0x49900000 0x100000>;
	interrupts = <15>;
};

This is because each tptc entry has it's own sysc register and
can idle on it's own. With the above we can have 1 to 1 to 1 mapping
of compatible property to hwmod to sysc registers for each device.

> IMHO instead of DT referencing the hwmod stuff using ti,hwmods the
> hwmod database should reference the compatible values. This depends
> on omap3 being DT only of course.

Yes agreed.

Regards,

Tony

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 6688 bytes --]

On Thu, Dec 11, 2014 at 08:23:33AM -0600, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Dec 11, 2014 at 01:52:38AM +0100, Sebastian Reichel wrote:
> > On Wed, Dec 10, 2014 at 08:54:33AM -0600, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > (adding linux-omap back to the loop)
> > > 
> > > On Wed, Dec 10, 2014 at 04:20:30PM +0530, Lokesh Vutla wrote:
> > > > Hi Felipe,
> > > > 
> > > > On Wednesday 10 December 2014 03:57 AM, Felipe Balbi wrote:
> > > > > Before this patch, HWMOD requires the existence
> > > > > of a struct omap_hwmod_class very early.
> > > > Yes, hwmod code looks for omap_hwmod_class entry before registering any hwmod.
> > > > 
> > > > With the patch 4/7 omap_hwmod_class gets populated from dt very late after registration of the hwmod.
> > > > So all the hwmod which gets class data from dt never gets registered and state is always UNKNOWN.
> > > > Mostly making it unusable.
> > > > IMO this patch just masks the problem.
> > > > 
> > > > In order to register hwmod we need to populate class data very early.
> > > > We can populate at the same place as how reg property is being parsed.
> > > > Call omap_hwmod_init_sysc() in _init() of the particular hwmod:
> > > > The below diff will help:
> > > >
> > > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> > > > index cbb908d..05ecf8a 100644
> > > > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > > > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > > > @@ -2415,6 +2415,116 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +static int omap_hwmod_has_sysc_bindings(struct device_node *node)
> > > > +{
> > > > +	char *properties[] = {
> > > > +		"ti,rev_offs",
> > > > +		"ti,sysc_offs",
> > > > +		"ti,syss_offs",
> > > > +		"ti,sysc_flags",
> > > > +		"ti,srst_udelay",
> > > > +		"ti,idlemodes",
> > > > +		"ti,clockact",
> > > > +		"ti,sysc_type",
> > > > +		NULL
> > > > +	};
> > > > +	char **tmp = properties;
> > > > +
> > > > +	while (*tmp) {
> > > > +		if (of_property_read_bool(node, *tmp)) {
> > > > +			return true;
> > > > +		}
> > > > +		tmp++;
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int omap_hwmod_init_sysc(struct device_node *node,
> > > > +		struct omap_hwmod *oh, int index)
> > > > +{
> > > > +	struct omap_hwmod_class *class = oh->class;
> > > > +	struct omap_hwmod_class_sysconfig *sysc;
> > > > +	int ret;
> > > > +	int i;
> > > > +	char name[128];
> > > > +	const char *tmp = oh->name;
> > > > +	u32 prop;
> > > > +
> > > > +	/* if data isn't provided by DT, skip */
> > > > +	if ((class && class->sysc) || !omap_hwmod_has_sysc_bindings(node))
> > > > +		return 0;
> > > > +
> > > > +	class = kzalloc(sizeof(*class), GFP_KERNEL);
> > > > +	if (!class)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	i = 0;
> > > > +	while (*tmp) {
> > > > +		if (isalpha(*tmp))
> > > > +			name[i++] = *tmp;
> > > > +		tmp++;
> > > > +	}
> > > > +	name[i] = '\0';
> > > > +
> > > > +	class->name = kzalloc(sizeof(name), GFP_KERNEL);
> > > > +	if (!class->name)
> > > > +		return -ENOMEM;
> > > > +	strncpy(class->name, name, sizeof(name) - 1);
> > > > +
> > > > +	sysc = kzalloc(sizeof(*sysc), GFP_KERNEL);
> > > > +	if (!sysc)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,rev_offs", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->rev_offs = prop;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,sysc_offs", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->sysc_offs = prop;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,syss_offs", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->syss_offs = prop;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,sysc_flags", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->sysc_flags = prop & 0xffff;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,srst_udelay", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->srst_udelay = prop & 0xff;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,idlemodes", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->idlemodes = prop & 0xff;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,clockact", index, &prop);
> > > > +	if (!ret)
> > > > +		sysc->clockact = prop & 0xff;
> > > > +
> > > > +	ret = of_property_read_u32_index(node, "ti,sysc_type", index, &prop);
> > > > +	if (ret)
> > > > +		prop = 1;
> > > > +
> > > > +	switch (prop) {
> > > > +	case 2:
> > > > +		sysc->sysc_fields = &omap_hwmod_sysc_type2;
> > > > +		break;
> > > > +	case 3:
> > > > +		sysc->sysc_fields = &omap_hwmod_sysc_type3;
> > > > +		break;
> > > > +	case 1:
> > > > +	default:
> > > > +		sysc->sysc_fields = &omap_hwmod_sysc_type1;
> > > > +	}
> > > > +	class->sysc = sysc;
> > > > +	oh->class = class;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  /**
> > > >   * _init - initialize internal data for the hwmod @oh
> > > >   * @oh: struct omap_hwmod *
> > > > @@ -2449,6 +2559,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
> > > >  		else if (np && index)
> > > >  			pr_warn("omap_hwmod: %s using broken dt data from %s\n",
> > > >  				oh->name, np->name);
> > > > +
> > > > +		if (np) {
> > > > +			r = omap_hwmod_init_sysc(np, oh, 0);
> > > 
> > > this won't handle any binding which lists more than one hwmod on its
> > > ti,hwmods property.
> > 
> > I think Tony planned to remove this kind of multi hwmod references.
> 
> true, but I'm still a bit iffy wrt that. ABI compatibility breaks and
> all

Moving the hwmod data to device tree will break the ABI
compatibility anyways. You wont be able to use an old DT with the
new kernel, since then you are missing the hwmod data (assuming
there won't be a fallback hwmod data kept in the kernel source).

> > IMHO instead of DT referencing the hwmod stuff using ti,hwmods the
> > hwmod database should reference the compatible values. This depends
> > on omap3 being DT only of course.
> 
> don't you think it's too late for that ? We need to support the current
> form of dts files forever. It's an ABI afterall.

As I described above the ABI *will* break if hwmod data is removed from
the kernel.

OTOH where is the ABI breakage when hwmod database in kernel is
built from the compatible value? The compatible values are already
part of the ABI, so there are no new properties needed. The ti,hwmod
property can be marked as deprecated (and maybe removed after some
years).

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC/PATCH 5/7] arm: omap: hwmod: allow for registration of class-less hwmods

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [141211 09:46]:
> On Thu, Dec 11, 2014 at 08:23:33AM -0600, Felipe Balbi wrote:
> > true, but I'm still a bit iffy wrt that. ABI compatibility breaks and
> > all
> 
> Moving the hwmod data to device tree will break the ABI
> compatibility anyways. You wont be able to use an old DT with the
> new kernel, since then you are missing the hwmod data (assuming
> there won't be a fallback hwmod data kept in the kernel source).

Right, the way to deal with that is to do the following:

1. Once we have the binding in place, start printing out warnings
   and deprecate the old built in data.

2. For missing sysc data, we just keep printing warnings and won't
   idle or enable the devices. This keeps the optional boot console
   UART working based on the bootloader settings.

3. We may want to keep the UART and system timer data around forever
   to be able to print sane error messages.

> > > IMHO instead of DT referencing the hwmod stuff using ti,hwmods the
> > > hwmod database should reference the compatible values. This depends
> > > on omap3 being DT only of course.
> > 
> > don't you think it's too late for that ? We need to support the current
> > form of dts files forever. It's an ABI afterall.
> 
> As I described above the ABI *will* break if hwmod data is removed from
> the kernel.

Right, there is no way we can support incomplete device tree
bindings except for devices that are enabled by the bootloader
already. But we can take our time removing the built-in device
data, there's no need to do it all at once.

> OTOH where is the ABI breakage when hwmod database in kernel is
> built from the compatible value? The compatible values are already
> part of the ABI, so there are no new properties needed. The ti,hwmod
> property can be marked as deprecated (and maybe removed after some
> years).

Yes we can keep it around but don't have to do anything with it
eventually. For the legacy systems, we can have built in mapping
of compatible values to ti,hwmods along the clock aliases.

Regards,

Tony