* [RFC 0/9] Introduce OPP modifier for ARM SoCs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: Jisheng Zhang, devicetree, Nishanth Menon, Dave Gerlach,
Viresh Kumar, Anson Huang, Rafael J. Wysocki, cpufreq, kernel,
Shawn Guo
There are many SoCs that can have different operating points supported
depending on different conditions even for otherwise identical parts.
It does not make sense to define many different iterations of a device tree
file for each possible permutation of the same device especially when this data
may exist within the part.
This proposal introduces a framework and example driver to help in enabling
or disabling appropriate OPPs based on a register value. The information needed
for deciding which OPPs are to be enabled is defined in the device tree in a
table so that one device tree file is able to support any version of the part.
This series is mostly untested besides 335x and only is intended to demonstrate
the approach for dynamically changing available OPPs.
The framework allows other drivers to register themselves and
then through a single function call modify which OPPs are available for a device.
The included opp-modifier-reg driver can already support many different SoCs
that have a manufacturer modifiable register defining which OPPs are possible
on the SoC. This series includes example entries for am335x, am437x, omap4,
dra7, and imx6q.
The opp-modifier-reg driver expects all possible OPPs to be loaded for the
device already and then is able to modify the table based on the details
passed compared to the defined register.
This RFC only applies to MPU OPPs and only supports modifying the OPP list to
contain supported OPPs during boot but this driver could easily be extended
to support modifying the OPPs of other IPs that have them easily by placing
the hook (in patch 3 here) elsewhere. The hook for opp-modifier was placed
in its current only as a proof of concept; if there is a better location it
should be moved.
This series is tested on am33xx but must be used with 2.x+ silicon as the
earlier revision does not support the same OPPs. Also for the additional OPPs
the maximum voltage supplied by the regulator for the MPU rail on both am335
and am437x would need to be extended but those patches were left out of this
series to focus on opp-modifier.
Comments? Is this a reasonable direction to take?
Dave Gerlach (9):
opp-modifier: Introduce OPP Modifier Framework
opp-modifier: Add opp-modifier-reg driver
PM / OPP: Add hook to modify OPPs after they are loaded.
ARM: dts: AM33XX: Add opp-modifier device entry and add higher OPPs
ARM: dts: AM4372: Add opp-modifier device entry and add higher OPPs
ARM: dts: omap443x: Add opp-modifier entry and add higher OPPs
ARM: dts: omap4460: Add opp-modifier entry and add higher OPPs
ARM: dts: dra7: Add opp-modifier device entry and add higher OPPs
ARM: dts: imx6q: Add opp-modifier device entry
.../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
arch/arm/boot/dts/am33xx.dtsi | 27 ++-
arch/arm/boot/dts/am4372.dtsi | 30 +++
arch/arm/boot/dts/dra7.dtsi | 18 ++
arch/arm/boot/dts/imx6q.dtsi | 18 ++
arch/arm/boot/dts/omap443x.dtsi | 16 ++
arch/arm/boot/dts/omap4460.dtsi | 17 ++
drivers/base/power/opp.c | 8 +
drivers/power/Makefile | 2 +
drivers/power/opp/Makefile | 2 +
drivers/power/opp/core.c | 126 ++++++++++
drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
include/dt-bindings/opp/imx.h | 17 ++
include/dt-bindings/opp/ti.h | 33 +++
include/linux/opp-modifier.h | 35 +++
15 files changed, 717 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
create mode 100644 drivers/power/opp/Makefile
create mode 100644 drivers/power/opp/core.c
create mode 100644 drivers/power/opp/opp-modifier-reg.c
create mode 100644 include/dt-bindings/opp/imx.h
create mode 100644 include/dt-bindings/opp/ti.h
create mode 100644 include/linux/opp-modifier.h
--
1.9.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 0/9] Introduce OPP modifier for ARM SoCs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
There are many SoCs that can have different operating points supported
depending on different conditions even for otherwise identical parts.
It does not make sense to define many different iterations of a device tree
file for each possible permutation of the same device especially when this data
may exist within the part.
This proposal introduces a framework and example driver to help in enabling
or disabling appropriate OPPs based on a register value. The information needed
for deciding which OPPs are to be enabled is defined in the device tree in a
table so that one device tree file is able to support any version of the part.
This series is mostly untested besides 335x and only is intended to demonstrate
the approach for dynamically changing available OPPs.
The framework allows other drivers to register themselves and
then through a single function call modify which OPPs are available for a device.
The included opp-modifier-reg driver can already support many different SoCs
that have a manufacturer modifiable register defining which OPPs are possible
on the SoC. This series includes example entries for am335x, am437x, omap4,
dra7, and imx6q.
The opp-modifier-reg driver expects all possible OPPs to be loaded for the
device already and then is able to modify the table based on the details
passed compared to the defined register.
This RFC only applies to MPU OPPs and only supports modifying the OPP list to
contain supported OPPs during boot but this driver could easily be extended
to support modifying the OPPs of other IPs that have them easily by placing
the hook (in patch 3 here) elsewhere. The hook for opp-modifier was placed
in its current only as a proof of concept; if there is a better location it
should be moved.
This series is tested on am33xx but must be used with 2.x+ silicon as the
earlier revision does not support the same OPPs. Also for the additional OPPs
the maximum voltage supplied by the regulator for the MPU rail on both am335
and am437x would need to be extended but those patches were left out of this
series to focus on opp-modifier.
Comments? Is this a reasonable direction to take?
Dave Gerlach (9):
opp-modifier: Introduce OPP Modifier Framework
opp-modifier: Add opp-modifier-reg driver
PM / OPP: Add hook to modify OPPs after they are loaded.
ARM: dts: AM33XX: Add opp-modifier device entry and add higher OPPs
ARM: dts: AM4372: Add opp-modifier device entry and add higher OPPs
ARM: dts: omap443x: Add opp-modifier entry and add higher OPPs
ARM: dts: omap4460: Add opp-modifier entry and add higher OPPs
ARM: dts: dra7: Add opp-modifier device entry and add higher OPPs
ARM: dts: imx6q: Add opp-modifier device entry
.../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
arch/arm/boot/dts/am33xx.dtsi | 27 ++-
arch/arm/boot/dts/am4372.dtsi | 30 +++
arch/arm/boot/dts/dra7.dtsi | 18 ++
arch/arm/boot/dts/imx6q.dtsi | 18 ++
arch/arm/boot/dts/omap443x.dtsi | 16 ++
arch/arm/boot/dts/omap4460.dtsi | 17 ++
drivers/base/power/opp.c | 8 +
drivers/power/Makefile | 2 +
drivers/power/opp/Makefile | 2 +
drivers/power/opp/core.c | 126 ++++++++++
drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
include/dt-bindings/opp/imx.h | 17 ++
include/dt-bindings/opp/ti.h | 33 +++
include/linux/opp-modifier.h | 35 +++
15 files changed, 717 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
create mode 100644 drivers/power/opp/Makefile
create mode 100644 drivers/power/opp/core.c
create mode 100644 drivers/power/opp/opp-modifier-reg.c
create mode 100644 include/dt-bindings/opp/imx.h
create mode 100644 include/dt-bindings/opp/ti.h
create mode 100644 include/linux/opp-modifier.h
--
1.9.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 1/9] opp-modifier: Introduce OPP Modifier Framework
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Introduce framework to allow an OPP modifier driver to selectively
determine which possible OPPs for an SoC are available based on
register values found in SoC through a common API.
Three functions are exported, a register and unregister function for
the opp modifier drivers to notify the API of their existence, and
opp_modify_dev_table which looks up the appropriate driver and
DT information to use for a device and then uses enable/disable to
change which previously loaded OPPs are available on the device.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/power/Makefile | 2 +
drivers/power/opp/Makefile | 1 +
drivers/power/opp/core.c | 126 +++++++++++++++++++++++++++++++++++++++++++
include/linux/opp-modifier.h | 35 ++++++++++++
4 files changed, 164 insertions(+)
create mode 100644 drivers/power/opp/Makefile
create mode 100644 drivers/power/opp/core.c
create mode 100644 include/linux/opp-modifier.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index ee54a3e..ed379cf 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -58,3 +58,5 @@ obj-$(CONFIG_POWER_AVS) += avs/
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
obj-$(CONFIG_POWER_RESET) += reset/
+
+obj-y += opp/
diff --git a/drivers/power/opp/Makefile b/drivers/power/opp/Makefile
new file mode 100644
index 0000000..820eb10
--- /dev/null
+++ b/drivers/power/opp/Makefile
@@ -0,0 +1 @@
+obj-y += core.o
diff --git a/drivers/power/opp/core.c b/drivers/power/opp/core.c
new file mode 100644
index 0000000..403013b
--- /dev/null
+++ b/drivers/power/opp/core.c
@@ -0,0 +1,126 @@
+/*
+ * OPP Modifier framework
+ *
+ * Copyright (C) 2013 Texas Instruments Inc.
+ * Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/opp-modifier.h>
+
+static DEFINE_MUTEX(opp_modifier_list_mutex);
+static LIST_HEAD(opp_modifier_list);
+
+static int opp_modify_dev_opp_table(struct opp_modifier_dev *opp_dev,
+ struct device *dev)
+{
+ if (opp_dev->ops->modify)
+ return opp_dev->ops->modify(dev);
+
+ return -EINVAL;
+}
+
+static struct opp_modifier_dev *opp_modifier_get(struct device *dev)
+{
+ struct opp_modifier_dev *o, *opp_dev;
+ struct device_node *np;
+
+ if (!dev)
+ return ERR_PTR(-EINVAL);
+
+ np = of_parse_phandle(dev->of_node, "platform-opp-modifier", 0);
+
+ if (!np)
+ return ERR_PTR(-ENOSYS);
+
+ opp_dev = NULL;
+
+ mutex_lock(&opp_modifier_list_mutex);
+
+ list_for_each_entry(o, &opp_modifier_list, list) {
+ if (of_get_parent(np) == o->of_node) {
+ opp_dev = o;
+ break;
+ }
+ }
+
+ if (!opp_dev) {
+ mutex_unlock(&opp_modifier_list_mutex);
+ return ERR_PTR(-EINVAL);
+ }
+
+ of_node_put(np);
+
+ try_module_get(opp_dev->owner);
+ mutex_unlock(&opp_modifier_list_mutex);
+
+ return opp_dev;
+}
+
+static void opp_modifier_put(struct opp_modifier_dev *opp_dev)
+{
+ if (IS_ERR(opp_dev))
+ return;
+
+ module_put(opp_dev->owner);
+}
+
+int opp_modifier_register(struct opp_modifier_dev *opp_dev)
+{
+ mutex_lock(&opp_modifier_list_mutex);
+ list_add(&opp_dev->list, &opp_modifier_list);
+ mutex_unlock(&opp_modifier_list_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(opp_modifier_register);
+
+void opp_modifier_unregister(struct opp_modifier_dev *opp_dev)
+{
+ mutex_lock(&opp_modifier_list_mutex);
+ list_del(&opp_dev->list);
+ mutex_unlock(&opp_modifier_list_mutex);
+}
+EXPORT_SYMBOL_GPL(opp_modifier_unregister);
+
+int opp_modify_dev_table(struct device *dev)
+{
+ struct opp_modifier_dev *opp_dev;
+ int ret;
+
+ opp_dev = opp_modifier_get(dev);
+
+ /*
+ * It is a valid case for a device to not implement
+ * an OPP modifier table so return 0 if not present
+ */
+
+ if (IS_ERR(opp_dev) && PTR_ERR(opp_dev) == -ENOSYS) {
+ dev_dbg(dev, "No platform-opp-modifier entry present\n");
+ return 0;
+ } else if (IS_ERR(opp_dev)) {
+ return PTR_ERR(opp_dev);
+ }
+
+ ret = opp_modify_dev_opp_table(opp_dev, dev);
+
+ opp_modifier_put(opp_dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(opp_modify_dev_table);
diff --git a/include/linux/opp-modifier.h b/include/linux/opp-modifier.h
new file mode 100644
index 0000000..8d50851
--- /dev/null
+++ b/include/linux/opp-modifier.h
@@ -0,0 +1,35 @@
+/*
+ * TI OPP Modifier Core API
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_OPP_MODIFIER_H__
+#define __LINUX_OPP_MODIFIER_H__
+
+struct opp_modifier_dev {
+ struct opp_modifier_ops *ops;
+ struct module *owner;
+ struct list_head list;
+ struct device_node *of_node;
+};
+
+struct opp_modifier_ops {
+ int (*modify)(struct device *dev);
+};
+
+int opp_modifier_register(struct opp_modifier_dev *opp_dev);
+void opp_modifier_unregister(struct opp_modifier_dev *opp_dev);
+int opp_modify_dev_table(struct device *dev);
+
+#endif /* __LINUX_OPP_MODIFIER_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 1/9] opp-modifier: Introduce OPP Modifier Framework
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Introduce framework to allow an OPP modifier driver to selectively
determine which possible OPPs for an SoC are available based on
register values found in SoC through a common API.
Three functions are exported, a register and unregister function for
the opp modifier drivers to notify the API of their existence, and
opp_modify_dev_table which looks up the appropriate driver and
DT information to use for a device and then uses enable/disable to
change which previously loaded OPPs are available on the device.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/power/Makefile | 2 +
drivers/power/opp/Makefile | 1 +
drivers/power/opp/core.c | 126 +++++++++++++++++++++++++++++++++++++++++++
include/linux/opp-modifier.h | 35 ++++++++++++
4 files changed, 164 insertions(+)
create mode 100644 drivers/power/opp/Makefile
create mode 100644 drivers/power/opp/core.c
create mode 100644 include/linux/opp-modifier.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index ee54a3e..ed379cf 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -58,3 +58,5 @@ obj-$(CONFIG_POWER_AVS) += avs/
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
obj-$(CONFIG_POWER_RESET) += reset/
+
+obj-y += opp/
diff --git a/drivers/power/opp/Makefile b/drivers/power/opp/Makefile
new file mode 100644
index 0000000..820eb10
--- /dev/null
+++ b/drivers/power/opp/Makefile
@@ -0,0 +1 @@
+obj-y += core.o
diff --git a/drivers/power/opp/core.c b/drivers/power/opp/core.c
new file mode 100644
index 0000000..403013b
--- /dev/null
+++ b/drivers/power/opp/core.c
@@ -0,0 +1,126 @@
+/*
+ * OPP Modifier framework
+ *
+ * Copyright (C) 2013 Texas Instruments Inc.
+ * Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/opp-modifier.h>
+
+static DEFINE_MUTEX(opp_modifier_list_mutex);
+static LIST_HEAD(opp_modifier_list);
+
+static int opp_modify_dev_opp_table(struct opp_modifier_dev *opp_dev,
+ struct device *dev)
+{
+ if (opp_dev->ops->modify)
+ return opp_dev->ops->modify(dev);
+
+ return -EINVAL;
+}
+
+static struct opp_modifier_dev *opp_modifier_get(struct device *dev)
+{
+ struct opp_modifier_dev *o, *opp_dev;
+ struct device_node *np;
+
+ if (!dev)
+ return ERR_PTR(-EINVAL);
+
+ np = of_parse_phandle(dev->of_node, "platform-opp-modifier", 0);
+
+ if (!np)
+ return ERR_PTR(-ENOSYS);
+
+ opp_dev = NULL;
+
+ mutex_lock(&opp_modifier_list_mutex);
+
+ list_for_each_entry(o, &opp_modifier_list, list) {
+ if (of_get_parent(np) == o->of_node) {
+ opp_dev = o;
+ break;
+ }
+ }
+
+ if (!opp_dev) {
+ mutex_unlock(&opp_modifier_list_mutex);
+ return ERR_PTR(-EINVAL);
+ }
+
+ of_node_put(np);
+
+ try_module_get(opp_dev->owner);
+ mutex_unlock(&opp_modifier_list_mutex);
+
+ return opp_dev;
+}
+
+static void opp_modifier_put(struct opp_modifier_dev *opp_dev)
+{
+ if (IS_ERR(opp_dev))
+ return;
+
+ module_put(opp_dev->owner);
+}
+
+int opp_modifier_register(struct opp_modifier_dev *opp_dev)
+{
+ mutex_lock(&opp_modifier_list_mutex);
+ list_add(&opp_dev->list, &opp_modifier_list);
+ mutex_unlock(&opp_modifier_list_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(opp_modifier_register);
+
+void opp_modifier_unregister(struct opp_modifier_dev *opp_dev)
+{
+ mutex_lock(&opp_modifier_list_mutex);
+ list_del(&opp_dev->list);
+ mutex_unlock(&opp_modifier_list_mutex);
+}
+EXPORT_SYMBOL_GPL(opp_modifier_unregister);
+
+int opp_modify_dev_table(struct device *dev)
+{
+ struct opp_modifier_dev *opp_dev;
+ int ret;
+
+ opp_dev = opp_modifier_get(dev);
+
+ /*
+ * It is a valid case for a device to not implement
+ * an OPP modifier table so return 0 if not present
+ */
+
+ if (IS_ERR(opp_dev) && PTR_ERR(opp_dev) == -ENOSYS) {
+ dev_dbg(dev, "No platform-opp-modifier entry present\n");
+ return 0;
+ } else if (IS_ERR(opp_dev)) {
+ return PTR_ERR(opp_dev);
+ }
+
+ ret = opp_modify_dev_opp_table(opp_dev, dev);
+
+ opp_modifier_put(opp_dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(opp_modify_dev_table);
diff --git a/include/linux/opp-modifier.h b/include/linux/opp-modifier.h
new file mode 100644
index 0000000..8d50851
--- /dev/null
+++ b/include/linux/opp-modifier.h
@@ -0,0 +1,35 @@
+/*
+ * TI OPP Modifier Core API
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_OPP_MODIFIER_H__
+#define __LINUX_OPP_MODIFIER_H__
+
+struct opp_modifier_dev {
+ struct opp_modifier_ops *ops;
+ struct module *owner;
+ struct list_head list;
+ struct device_node *of_node;
+};
+
+struct opp_modifier_ops {
+ int (*modify)(struct device *dev);
+};
+
+int opp_modifier_register(struct opp_modifier_dev *opp_dev);
+void opp_modifier_unregister(struct opp_modifier_dev *opp_dev);
+int opp_modify_dev_table(struct device *dev);
+
+#endif /* __LINUX_OPP_MODIFIER_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Driver to read from a register and depending on either set bits or
a specific known selectively enable or disable OPPs based on DT node.
Can support opp-modifier-reg-bit where single bits within the register
determine the availability of an OPP or opp-modifier-reg-val where a
certain value inside the register or a portion of it determine what the
maximum allowed OPP is.
The driver expects a device that has already has its OPPs loaded
and then will disable the OPPs not matching the criteria specified in
the opp-modifier table.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
.../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
drivers/power/opp/Makefile | 1 +
drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
3 files changed, 371 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
create mode 100644 drivers/power/opp/opp-modifier-reg.c
diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
new file mode 100644
index 0000000..af8a2e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
@@ -0,0 +1,111 @@
+* OPP-Modifier - opp modifier to selectively enable operating points
+
+Many SoCs that have selectively modifiable OPPs can specify
+all available OPPs in their operating-points listing and then define
+opp_modifiers to enable or disable the OPPs that are actually available
+on the specific hardware.
+
+* OPP Modifier Provider
+
+For single bits that define the availability of an OPP:
+-------------------------------------------
+Some SoCs define a bit in a register that indicates whether an OPP is
+available. This will disable any OPP with a frequency corresponding to
+the bit given if it is not set appropriately.
+
+properties:
+- compatible : Should be "opp-modifier-reg-bit"
+- reg : Address and length of the registers needed to identify available
+ OPPs, here we provide just the register containing OPP data.
+
+Optional Properties:
+- opp,reg-bit-enable-low: Take the complement of register before comparing mask
+ defined below under opp-modifier.
+
+Sub-nodes:
+Sub-nodes are defined as a container to hold opp modifier table for a
+specific device with an operating-points table already defined
+
+Sub-node properties:
+- opp-modifier: A collection of rows consisting of the following entries to
+ allow specification of available OPPs:
+ -kHz: The opp to be enabled based on following criteria
+ -offset: Offset into register where relevant bits are located
+ -value: Bit that indicates availability of OPP
+
+Example:
+
+ opp_modifier: opp_modifier@0x44e107fc {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x44e107fc 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 BIT_1
+ 720000 0 BIT_2
+ >;
+ };
+ };
+
+For a value that defines the maximum available OPP:
+-------------------------------------------
+Some SoCs define a value in a register that corresponds to an OPP. If
+that value is matched this will disable all OPPs greater than the
+associated frequency.
+
+properties:
+- compatible : Should be "opp-modifier-reg-val"
+- reg : Address and length of the registers needed to identify available
+ OPPs, here we provide just the register containing OPP data.
+
+Optional Properties:
+- opp,reg-mask: Only compare the bits masked off by this value.
+
+Sub-nodes:
+Sub-nodes are defined as a container to hold opp modifier table for a
+specific device with an operating-points table already defined
+
+Sub-node properties:
+- opp-modifier: A collection of rows consisting of the following entries to
+ allow specification of available OPPs:
+ -kHz: The opp to be enabled based on following criteria
+ -offset: Offset into register where relevant bits are located
+ -value: Value that indicates maximum available OPP
+
+Example:
+
+ opp_modifier: opp_modifier@0x44e107fc {
+ compatible = "opp-modifier-reg-val";
+ reg = <0x44e107fc 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 VAL_1
+ 720000 0 VAL_2
+ >;
+ };
+ };
+
+* OPP Modifier Consumer
+
+Properties:
+- platform-opp-modifier: phandle to the sub-node of the proper opp-modifier
+ provider that contains the appropriate opp-modifier table
+
+Example:
+
+cpu@0 {
+ compatible = "arm,cortex-a8";
+ device_type = "cpu";
+
+ operating-points = <
+ /* kHz uV */
+ 1000000 1351000
+ 720000 1285000
+ >;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
+};
+
diff --git a/drivers/power/opp/Makefile b/drivers/power/opp/Makefile
index 820eb10..7f60adc 100644
--- a/drivers/power/opp/Makefile
+++ b/drivers/power/opp/Makefile
@@ -1 +1,2 @@
obj-y += core.o
+obj-y += opp-modifier-reg.o
diff --git a/drivers/power/opp/opp-modifier-reg.c b/drivers/power/opp/opp-modifier-reg.c
new file mode 100644
index 0000000..f4dcf7a
--- /dev/null
+++ b/drivers/power/opp/opp-modifier-reg.c
@@ -0,0 +1,259 @@
+/*
+ * Single bit OPP Modifier Driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/list.h>
+
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pm_opp.h>
+#include <linux/opp-modifier.h>
+
+static struct of_device_id opp_omap_of_match[];
+
+struct opp_reg_context {
+ struct device *dev;
+ void __iomem *reg;
+ u32 mask;
+ bool enable_low;
+ int (*modify)(struct device *dev, const struct property *prop);
+};
+
+static struct opp_reg_context *opp_reg;
+
+static unsigned long opp_reg_read(int offset)
+{
+ return readl(opp_reg->reg + offset);
+}
+
+static int opp_modifier_reg_bit_enable(struct device *dev,
+ const struct property *prop)
+{
+ const __be32 *val;
+ unsigned long reg_val, freq, offset, bit;
+ int idx;
+
+ val = prop->value;
+ idx = (prop->length / sizeof(u32)) / 3;
+ while (idx--) {
+ freq = be32_to_cpup(val++) * 1000;
+ offset = be32_to_cpup(val++);
+ bit = be32_to_cpup(val++);
+
+ reg_val = opp_reg_read(offset);
+
+ if (opp_reg->enable_low)
+ reg_val = ~reg_val;
+
+ if (!(reg_val & bit))
+ dev_pm_opp_disable(dev, freq);
+ }
+ return 0;
+}
+
+static int opp_modifier_reg_value_enable(struct device *dev,
+ const struct property *prop)
+{
+ const __be32 *val;
+ unsigned long reg_val, freq, offset, bits;
+ unsigned long disable_freq, search_freq;
+ struct dev_pm_opp *disable_opp;
+ int idx, i, opp_count;
+
+ val = prop->value;
+ idx = (prop->length / sizeof(u32)) / 3;
+
+ while (idx--) {
+ freq = be32_to_cpup(val++) * 1000;
+ offset = be32_to_cpup(val++);
+ bits = be32_to_cpup(val++);
+
+ reg_val = opp_reg_read(offset);
+
+ if ((reg_val & opp_reg->mask) == bits) {
+ /*
+ * Find all frequencies greater than current freq
+ */
+ search_freq = freq + 1;
+ rcu_read_lock();
+ opp_count = dev_pm_opp_get_opp_count(dev);
+ rcu_read_unlock();
+
+ for (i = 0; i < opp_count; i++) {
+ rcu_read_lock();
+ disable_opp =
+ dev_pm_opp_find_freq_ceil(dev,
+ &search_freq);
+ if (IS_ERR(disable_opp)) {
+ rcu_read_unlock();
+ break;
+ }
+ disable_freq =
+ dev_pm_opp_get_freq(disable_opp);
+ rcu_read_unlock();
+ dev_pm_opp_disable(dev, disable_freq);
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int of_opp_check_availability(struct device *dev, struct device_node *np)
+{
+ const struct property *prop;
+ int nr;
+
+ if (!dev || !np)
+ return -EINVAL;
+
+ prop = of_find_property(np, "opp-modifier", NULL);
+ if (!prop)
+ return -EINVAL;
+ if (!prop->value)
+ return -EINVAL;
+
+ nr = prop->length / sizeof(u32);
+ if (nr % 3) {
+ pr_err("%s: Invalid OPP Available list\n", __func__);
+ return -EINVAL;
+ }
+
+ return opp_reg->modify(dev, prop);
+}
+
+static int opp_modifier_reg_device_modify(struct device *dev)
+{
+ struct device_node *np;
+ int ret;
+
+ if (!dev)
+ return -EINVAL;
+
+ np = of_parse_phandle(dev->of_node, "platform-opp-modifier", 0);
+
+ if (!np)
+ return -EINVAL;
+
+ ret = of_opp_check_availability(dev, np);
+
+ if (ret)
+ pr_err("Error modifying available OPPs\n");
+
+ of_node_put(np);
+
+ return ret;
+}
+
+static struct opp_modifier_ops opp_modifier_reg_ops = {
+ .modify = opp_modifier_reg_device_modify,
+};
+
+static struct opp_modifier_dev opp_modifier_reg_dev = {
+ .ops = &opp_modifier_reg_ops,
+};
+
+static struct of_device_id opp_modifier_reg_of_match[] = {
+ {
+ .compatible = "opp-modifier-reg-bit",
+ .data = &opp_modifier_reg_bit_enable,
+ },
+ {
+ .compatible = "opp-modifier-reg-val",
+ .data = &opp_modifier_reg_value_enable,
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, opp_modifier_reg_of_match);
+
+static int opp_modifier_reg_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+ struct resource *res;
+ struct device_node *np = pdev->dev.of_node;
+ int ret = 0;
+
+ opp_reg = devm_kzalloc(&pdev->dev, sizeof(*opp_reg), GFP_KERNEL);
+ if (!opp_reg) {
+ dev_err(opp_reg->dev, "reg context memory allocation failed\n");
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ match = of_match_device(opp_modifier_reg_of_match, &pdev->dev);
+
+ if (!match) {
+ dev_err(&pdev->dev, "Invalid match data value\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ opp_reg->modify = (void *)match->data;
+
+ opp_reg->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "no memory resource for opp register\n");
+ ret = -ENXIO;
+ goto err;
+ }
+
+ opp_reg->reg = devm_request_and_ioremap(opp_reg->dev, res);
+ if (!opp_reg->reg) {
+ dev_err(opp_reg->dev, "could not ioremap opp register\n");
+ ret = -EADDRNOTAVAIL;
+ goto err;
+ }
+
+ if (of_get_property(np, "opp,reg-bit-enable-low", NULL))
+ opp_reg->enable_low = true;
+
+ of_property_read_u32(np, "opp,reg-mask", &opp_reg->mask);
+
+ opp_modifier_reg_dev.ops = &opp_modifier_reg_ops;
+ opp_modifier_reg_dev.of_node = pdev->dev.of_node;
+
+ opp_modifier_register(&opp_modifier_reg_dev);
+
+err:
+ return ret;
+}
+
+static int opp_modifier_reg_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver opp_modifier_reg_driver = {
+ .probe = opp_modifier_reg_probe,
+ .remove = opp_modifier_reg_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "opp-modifier-reg",
+ .of_match_table = opp_modifier_reg_of_match,
+ },
+};
+
+module_platform_driver(opp_modifier_reg_driver);
+
+MODULE_AUTHOR("Dave Gerlach <d-gerlach@ti.com>");
+MODULE_DESCRIPTION("OPP Modifier driver for eFuse defined OPPs");
+MODULE_LICENSE("GPL v2");
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Driver to read from a register and depending on either set bits or
a specific known selectively enable or disable OPPs based on DT node.
Can support opp-modifier-reg-bit where single bits within the register
determine the availability of an OPP or opp-modifier-reg-val where a
certain value inside the register or a portion of it determine what the
maximum allowed OPP is.
The driver expects a device that has already has its OPPs loaded
and then will disable the OPPs not matching the criteria specified in
the opp-modifier table.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
.../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
drivers/power/opp/Makefile | 1 +
drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
3 files changed, 371 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
create mode 100644 drivers/power/opp/opp-modifier-reg.c
diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
new file mode 100644
index 0000000..af8a2e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
@@ -0,0 +1,111 @@
+* OPP-Modifier - opp modifier to selectively enable operating points
+
+Many SoCs that have selectively modifiable OPPs can specify
+all available OPPs in their operating-points listing and then define
+opp_modifiers to enable or disable the OPPs that are actually available
+on the specific hardware.
+
+* OPP Modifier Provider
+
+For single bits that define the availability of an OPP:
+-------------------------------------------
+Some SoCs define a bit in a register that indicates whether an OPP is
+available. This will disable any OPP with a frequency corresponding to
+the bit given if it is not set appropriately.
+
+properties:
+- compatible : Should be "opp-modifier-reg-bit"
+- reg : Address and length of the registers needed to identify available
+ OPPs, here we provide just the register containing OPP data.
+
+Optional Properties:
+- opp,reg-bit-enable-low: Take the complement of register before comparing mask
+ defined below under opp-modifier.
+
+Sub-nodes:
+Sub-nodes are defined as a container to hold opp modifier table for a
+specific device with an operating-points table already defined
+
+Sub-node properties:
+- opp-modifier: A collection of rows consisting of the following entries to
+ allow specification of available OPPs:
+ -kHz: The opp to be enabled based on following criteria
+ -offset: Offset into register where relevant bits are located
+ -value: Bit that indicates availability of OPP
+
+Example:
+
+ opp_modifier: opp_modifier at 0x44e107fc {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x44e107fc 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 BIT_1
+ 720000 0 BIT_2
+ >;
+ };
+ };
+
+For a value that defines the maximum available OPP:
+-------------------------------------------
+Some SoCs define a value in a register that corresponds to an OPP. If
+that value is matched this will disable all OPPs greater than the
+associated frequency.
+
+properties:
+- compatible : Should be "opp-modifier-reg-val"
+- reg : Address and length of the registers needed to identify available
+ OPPs, here we provide just the register containing OPP data.
+
+Optional Properties:
+- opp,reg-mask: Only compare the bits masked off by this value.
+
+Sub-nodes:
+Sub-nodes are defined as a container to hold opp modifier table for a
+specific device with an operating-points table already defined
+
+Sub-node properties:
+- opp-modifier: A collection of rows consisting of the following entries to
+ allow specification of available OPPs:
+ -kHz: The opp to be enabled based on following criteria
+ -offset: Offset into register where relevant bits are located
+ -value: Value that indicates maximum available OPP
+
+Example:
+
+ opp_modifier: opp_modifier at 0x44e107fc {
+ compatible = "opp-modifier-reg-val";
+ reg = <0x44e107fc 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 VAL_1
+ 720000 0 VAL_2
+ >;
+ };
+ };
+
+* OPP Modifier Consumer
+
+Properties:
+- platform-opp-modifier: phandle to the sub-node of the proper opp-modifier
+ provider that contains the appropriate opp-modifier table
+
+Example:
+
+cpu at 0 {
+ compatible = "arm,cortex-a8";
+ device_type = "cpu";
+
+ operating-points = <
+ /* kHz uV */
+ 1000000 1351000
+ 720000 1285000
+ >;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
+};
+
diff --git a/drivers/power/opp/Makefile b/drivers/power/opp/Makefile
index 820eb10..7f60adc 100644
--- a/drivers/power/opp/Makefile
+++ b/drivers/power/opp/Makefile
@@ -1 +1,2 @@
obj-y += core.o
+obj-y += opp-modifier-reg.o
diff --git a/drivers/power/opp/opp-modifier-reg.c b/drivers/power/opp/opp-modifier-reg.c
new file mode 100644
index 0000000..f4dcf7a
--- /dev/null
+++ b/drivers/power/opp/opp-modifier-reg.c
@@ -0,0 +1,259 @@
+/*
+ * Single bit OPP Modifier Driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/list.h>
+
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pm_opp.h>
+#include <linux/opp-modifier.h>
+
+static struct of_device_id opp_omap_of_match[];
+
+struct opp_reg_context {
+ struct device *dev;
+ void __iomem *reg;
+ u32 mask;
+ bool enable_low;
+ int (*modify)(struct device *dev, const struct property *prop);
+};
+
+static struct opp_reg_context *opp_reg;
+
+static unsigned long opp_reg_read(int offset)
+{
+ return readl(opp_reg->reg + offset);
+}
+
+static int opp_modifier_reg_bit_enable(struct device *dev,
+ const struct property *prop)
+{
+ const __be32 *val;
+ unsigned long reg_val, freq, offset, bit;
+ int idx;
+
+ val = prop->value;
+ idx = (prop->length / sizeof(u32)) / 3;
+ while (idx--) {
+ freq = be32_to_cpup(val++) * 1000;
+ offset = be32_to_cpup(val++);
+ bit = be32_to_cpup(val++);
+
+ reg_val = opp_reg_read(offset);
+
+ if (opp_reg->enable_low)
+ reg_val = ~reg_val;
+
+ if (!(reg_val & bit))
+ dev_pm_opp_disable(dev, freq);
+ }
+ return 0;
+}
+
+static int opp_modifier_reg_value_enable(struct device *dev,
+ const struct property *prop)
+{
+ const __be32 *val;
+ unsigned long reg_val, freq, offset, bits;
+ unsigned long disable_freq, search_freq;
+ struct dev_pm_opp *disable_opp;
+ int idx, i, opp_count;
+
+ val = prop->value;
+ idx = (prop->length / sizeof(u32)) / 3;
+
+ while (idx--) {
+ freq = be32_to_cpup(val++) * 1000;
+ offset = be32_to_cpup(val++);
+ bits = be32_to_cpup(val++);
+
+ reg_val = opp_reg_read(offset);
+
+ if ((reg_val & opp_reg->mask) == bits) {
+ /*
+ * Find all frequencies greater than current freq
+ */
+ search_freq = freq + 1;
+ rcu_read_lock();
+ opp_count = dev_pm_opp_get_opp_count(dev);
+ rcu_read_unlock();
+
+ for (i = 0; i < opp_count; i++) {
+ rcu_read_lock();
+ disable_opp =
+ dev_pm_opp_find_freq_ceil(dev,
+ &search_freq);
+ if (IS_ERR(disable_opp)) {
+ rcu_read_unlock();
+ break;
+ }
+ disable_freq =
+ dev_pm_opp_get_freq(disable_opp);
+ rcu_read_unlock();
+ dev_pm_opp_disable(dev, disable_freq);
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int of_opp_check_availability(struct device *dev, struct device_node *np)
+{
+ const struct property *prop;
+ int nr;
+
+ if (!dev || !np)
+ return -EINVAL;
+
+ prop = of_find_property(np, "opp-modifier", NULL);
+ if (!prop)
+ return -EINVAL;
+ if (!prop->value)
+ return -EINVAL;
+
+ nr = prop->length / sizeof(u32);
+ if (nr % 3) {
+ pr_err("%s: Invalid OPP Available list\n", __func__);
+ return -EINVAL;
+ }
+
+ return opp_reg->modify(dev, prop);
+}
+
+static int opp_modifier_reg_device_modify(struct device *dev)
+{
+ struct device_node *np;
+ int ret;
+
+ if (!dev)
+ return -EINVAL;
+
+ np = of_parse_phandle(dev->of_node, "platform-opp-modifier", 0);
+
+ if (!np)
+ return -EINVAL;
+
+ ret = of_opp_check_availability(dev, np);
+
+ if (ret)
+ pr_err("Error modifying available OPPs\n");
+
+ of_node_put(np);
+
+ return ret;
+}
+
+static struct opp_modifier_ops opp_modifier_reg_ops = {
+ .modify = opp_modifier_reg_device_modify,
+};
+
+static struct opp_modifier_dev opp_modifier_reg_dev = {
+ .ops = &opp_modifier_reg_ops,
+};
+
+static struct of_device_id opp_modifier_reg_of_match[] = {
+ {
+ .compatible = "opp-modifier-reg-bit",
+ .data = &opp_modifier_reg_bit_enable,
+ },
+ {
+ .compatible = "opp-modifier-reg-val",
+ .data = &opp_modifier_reg_value_enable,
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, opp_modifier_reg_of_match);
+
+static int opp_modifier_reg_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+ struct resource *res;
+ struct device_node *np = pdev->dev.of_node;
+ int ret = 0;
+
+ opp_reg = devm_kzalloc(&pdev->dev, sizeof(*opp_reg), GFP_KERNEL);
+ if (!opp_reg) {
+ dev_err(opp_reg->dev, "reg context memory allocation failed\n");
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ match = of_match_device(opp_modifier_reg_of_match, &pdev->dev);
+
+ if (!match) {
+ dev_err(&pdev->dev, "Invalid match data value\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ opp_reg->modify = (void *)match->data;
+
+ opp_reg->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "no memory resource for opp register\n");
+ ret = -ENXIO;
+ goto err;
+ }
+
+ opp_reg->reg = devm_request_and_ioremap(opp_reg->dev, res);
+ if (!opp_reg->reg) {
+ dev_err(opp_reg->dev, "could not ioremap opp register\n");
+ ret = -EADDRNOTAVAIL;
+ goto err;
+ }
+
+ if (of_get_property(np, "opp,reg-bit-enable-low", NULL))
+ opp_reg->enable_low = true;
+
+ of_property_read_u32(np, "opp,reg-mask", &opp_reg->mask);
+
+ opp_modifier_reg_dev.ops = &opp_modifier_reg_ops;
+ opp_modifier_reg_dev.of_node = pdev->dev.of_node;
+
+ opp_modifier_register(&opp_modifier_reg_dev);
+
+err:
+ return ret;
+}
+
+static int opp_modifier_reg_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver opp_modifier_reg_driver = {
+ .probe = opp_modifier_reg_probe,
+ .remove = opp_modifier_reg_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "opp-modifier-reg",
+ .of_match_table = opp_modifier_reg_of_match,
+ },
+};
+
+module_platform_driver(opp_modifier_reg_driver);
+
+MODULE_AUTHOR("Dave Gerlach <d-gerlach@ti.com>");
+MODULE_DESCRIPTION("OPP Modifier driver for eFuse defined OPPs");
+MODULE_LICENSE("GPL v2");
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 3/9] PM / OPP: Add hook to modify OPPs after they are loaded.
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add a hook inside opp_init_cpufreq_table to allow all cpufreq drivers
to utilize OPP modifier functionality. Hook will return success if no
phandle is present for devices that do not use opp-modifier.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index fa41874..eaedc6b 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -24,6 +24,7 @@
#include <linux/pm_opp.h>
#include <linux/of.h>
#include <linux/export.h>
+#include <linux/opp-modifier.h>
/*
* Internal data structure organization with the OPP layer library is as
@@ -629,6 +630,13 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct dev_pm_opp *opp;
struct cpufreq_frequency_table *freq_table;
int i = 0;
+ int ret;
+
+ ret = opp_modify_dev_table(dev);
+ if (ret) {
+ pr_err("failed to modify OPP table: %d\n", ret);
+ return ret;
+ }
/* Pretend as if I am an updater */
mutex_lock(&dev_opp_list_lock);
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 3/9] PM / OPP: Add hook to modify OPPs after they are loaded.
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add a hook inside opp_init_cpufreq_table to allow all cpufreq drivers
to utilize OPP modifier functionality. Hook will return success if no
phandle is present for devices that do not use opp-modifier.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index fa41874..eaedc6b 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -24,6 +24,7 @@
#include <linux/pm_opp.h>
#include <linux/of.h>
#include <linux/export.h>
+#include <linux/opp-modifier.h>
/*
* Internal data structure organization with the OPP layer library is as
@@ -629,6 +630,13 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct dev_pm_opp *opp;
struct cpufreq_frequency_table *freq_table;
int i = 0;
+ int ret;
+
+ ret = opp_modify_dev_table(dev);
+ if (ret) {
+ pr_err("failed to modify OPP table: %d\n", ret);
+ return ret;
+ }
/* Pretend as if I am an updater */
mutex_lock(&dev_opp_list_lock);
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 4/9] ARM: dts: AM33XX: Add opp-modifier device entry and add higher OPPs
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add an entry for opp_modifier which configures OPPs on am33xx. Within this
nodes are defined with opp-modifier propety that are defined as a list
of frequency, offset from base register, and efuse value. The CPU node
passes a phandle to the appropriate child node of the efuse node.
This patch also adds the ES2.x OPPs for am33xx.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/am33xx.dtsi | 27 +++++++++++++++++++++++++--
include/dt-bindings/opp/ti.h | 22 ++++++++++++++++++++++
2 files changed, 47 insertions(+), 2 deletions(-)
create mode 100644 include/dt-bindings/opp/ti.h
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 6d95d3d..a09f4fb 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -10,6 +10,7 @@
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/pinctrl/am33xx.h>
+#include <dt-bindings/opp/ti.h>
#include "skeleton.dtsi"
@@ -52,12 +53,17 @@
*/
operating-points = <
/* kHz uV */
+ 1000000 1351000
+ 800000 1285000
720000 1285000
600000 1225000
- 500000 1125000
- 275000 1125000
+ 300000 1125000
>;
+
voltage-tolerance = <2>; /* 2 percentage */
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
+
clock-latency = <300000>; /* From omap-cpufreq driver */
};
};
@@ -818,6 +824,23 @@
reg = <0x48310000 0x2000>;
interrupts = <111>;
};
+
+ opp_modifier: opp_modifier@0x44e107fc {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x44e107fc 0x04>;
+
+ opp,reg-bit-enable-low;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 AM33XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT
+ 800000 0 AM33XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT
+ 720000 0 AM33XX_EFUSE_SMA_OPP_120_720MHZ_BIT
+ 600000 0 AM33XX_EFUSE_SMA_OPP_100_600MHZ_BIT
+ >;
+ };
+ };
};
};
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
new file mode 100644
index 0000000..58436c1
--- /dev/null
+++ b/include/dt-bindings/opp/ti.h
@@ -0,0 +1,22 @@
+/*
+ * This header provides constants for TI SoC OPP bindings.
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.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.
+ *
+ */
+
+#ifndef __DT_BINDINGS_OPP_TI_H__
+#define __DT_BINDINGS_OPP_TI_H__
+
+#define AM33XX_EFUSE_SMA_OPP_50_300MHZ_BIT (1 << 4)
+#define AM33XX_EFUSE_SMA_OPP_100_300MHZ_BIT (1 << 5)
+#define AM33XX_EFUSE_SMA_OPP_100_600MHZ_BIT (1 << 6)
+#define AM33XX_EFUSE_SMA_OPP_120_720MHZ_BIT (1 << 7)
+#define AM33XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 8)
+#define AM33XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 9)
+
+#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 4/9] ARM: dts: AM33XX: Add opp-modifier device entry and add higher OPPs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry for opp_modifier which configures OPPs on am33xx. Within this
nodes are defined with opp-modifier propety that are defined as a list
of frequency, offset from base register, and efuse value. The CPU node
passes a phandle to the appropriate child node of the efuse node.
This patch also adds the ES2.x OPPs for am33xx.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/am33xx.dtsi | 27 +++++++++++++++++++++++++--
include/dt-bindings/opp/ti.h | 22 ++++++++++++++++++++++
2 files changed, 47 insertions(+), 2 deletions(-)
create mode 100644 include/dt-bindings/opp/ti.h
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 6d95d3d..a09f4fb 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -10,6 +10,7 @@
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/pinctrl/am33xx.h>
+#include <dt-bindings/opp/ti.h>
#include "skeleton.dtsi"
@@ -52,12 +53,17 @@
*/
operating-points = <
/* kHz uV */
+ 1000000 1351000
+ 800000 1285000
720000 1285000
600000 1225000
- 500000 1125000
- 275000 1125000
+ 300000 1125000
>;
+
voltage-tolerance = <2>; /* 2 percentage */
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
+
clock-latency = <300000>; /* From omap-cpufreq driver */
};
};
@@ -818,6 +824,23 @@
reg = <0x48310000 0x2000>;
interrupts = <111>;
};
+
+ opp_modifier: opp_modifier at 0x44e107fc {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x44e107fc 0x04>;
+
+ opp,reg-bit-enable-low;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 AM33XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT
+ 800000 0 AM33XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT
+ 720000 0 AM33XX_EFUSE_SMA_OPP_120_720MHZ_BIT
+ 600000 0 AM33XX_EFUSE_SMA_OPP_100_600MHZ_BIT
+ >;
+ };
+ };
};
};
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
new file mode 100644
index 0000000..58436c1
--- /dev/null
+++ b/include/dt-bindings/opp/ti.h
@@ -0,0 +1,22 @@
+/*
+ * This header provides constants for TI SoC OPP bindings.
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.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.
+ *
+ */
+
+#ifndef __DT_BINDINGS_OPP_TI_H__
+#define __DT_BINDINGS_OPP_TI_H__
+
+#define AM33XX_EFUSE_SMA_OPP_50_300MHZ_BIT (1 << 4)
+#define AM33XX_EFUSE_SMA_OPP_100_300MHZ_BIT (1 << 5)
+#define AM33XX_EFUSE_SMA_OPP_100_600MHZ_BIT (1 << 6)
+#define AM33XX_EFUSE_SMA_OPP_120_720MHZ_BIT (1 << 7)
+#define AM33XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 8)
+#define AM33XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 9)
+
+#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 5/9] ARM: dts: AM4372: Add opp-modifier device entry and add higher OPPs
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add an entry for opp_modifier which configures OPPs on AM4372. Within
this, nodes are defined with the opp-modifier propety that are defined as
a list of frequency, offset from base register, and efuse value.
The CPU node passes a phandle to the appropriate child node to get the
correct table.
This patch also adds higher eFused OPPs for AM4372.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/am4372.dtsi | 30 ++++++++++++++++++++++++++++++
include/dt-bindings/opp/ti.h | 6 ++++++
2 files changed, 36 insertions(+)
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index c6bd4d9..dbaa2e3 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -9,6 +9,7 @@
*/
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/opp/ti.h>
#include "skeleton.dtsi"
@@ -33,6 +34,17 @@
compatible = "arm,cortex-a9";
device_type = "cpu";
reg = <0>;
+
+ operating-points = <
+ /* kHz uV */
+ 300000 950000
+ 600000 1100000
+ 720000 1200000
+ 800000 1260000
+ 1000000 1325000
+ >;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
};
};
@@ -662,6 +674,24 @@
dma-names = "tx", "rx";
};
+ opp_modifier: opp_modifier@0x44e10610 {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x44e10610 0x04>;
+
+ opp,reg-bit-enable-low;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 AM43XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT
+ 800000 0 AM43XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT
+ 720000 0 AM43XX_EFUSE_SMA_OPP_120_720MHZ_BIT
+ 600000 0 AM43XX_EFUSE_SMA_OPP_100_600MHZ_BIT
+ 300000 0 AM43XX_EFUSE_SMA_OPP_50_300MHZ_BIT
+ >;
+ };
+ };
+
mcasp0: mcasp@48038000 {
compatible = "ti,am33xx-mcasp-audio";
ti,hwmods = "mcasp0";
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
index 58436c1..d3717be 100644
--- a/include/dt-bindings/opp/ti.h
+++ b/include/dt-bindings/opp/ti.h
@@ -19,4 +19,10 @@
#define AM33XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 8)
#define AM33XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 9)
+#define AM43XX_EFUSE_SMA_OPP_50_300MHZ_BIT (1 << 0)
+#define AM43XX_EFUSE_SMA_OPP_100_600MHZ_BIT (1 << 2)
+#define AM43XX_EFUSE_SMA_OPP_120_720MHZ_BIT (1 << 3)
+#define AM43XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 4)
+#define AM43XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 5)
+
#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 5/9] ARM: dts: AM4372: Add opp-modifier device entry and add higher OPPs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry for opp_modifier which configures OPPs on AM4372. Within
this, nodes are defined with the opp-modifier propety that are defined as
a list of frequency, offset from base register, and efuse value.
The CPU node passes a phandle to the appropriate child node to get the
correct table.
This patch also adds higher eFused OPPs for AM4372.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/am4372.dtsi | 30 ++++++++++++++++++++++++++++++
include/dt-bindings/opp/ti.h | 6 ++++++
2 files changed, 36 insertions(+)
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index c6bd4d9..dbaa2e3 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -9,6 +9,7 @@
*/
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/opp/ti.h>
#include "skeleton.dtsi"
@@ -33,6 +34,17 @@
compatible = "arm,cortex-a9";
device_type = "cpu";
reg = <0>;
+
+ operating-points = <
+ /* kHz uV */
+ 300000 950000
+ 600000 1100000
+ 720000 1200000
+ 800000 1260000
+ 1000000 1325000
+ >;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
};
};
@@ -662,6 +674,24 @@
dma-names = "tx", "rx";
};
+ opp_modifier: opp_modifier at 0x44e10610 {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x44e10610 0x04>;
+
+ opp,reg-bit-enable-low;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1000000 0 AM43XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT
+ 800000 0 AM43XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT
+ 720000 0 AM43XX_EFUSE_SMA_OPP_120_720MHZ_BIT
+ 600000 0 AM43XX_EFUSE_SMA_OPP_100_600MHZ_BIT
+ 300000 0 AM43XX_EFUSE_SMA_OPP_50_300MHZ_BIT
+ >;
+ };
+ };
+
mcasp0: mcasp at 48038000 {
compatible = "ti,am33xx-mcasp-audio";
ti,hwmods = "mcasp0";
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
index 58436c1..d3717be 100644
--- a/include/dt-bindings/opp/ti.h
+++ b/include/dt-bindings/opp/ti.h
@@ -19,4 +19,10 @@
#define AM33XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 8)
#define AM33XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 9)
+#define AM43XX_EFUSE_SMA_OPP_50_300MHZ_BIT (1 << 0)
+#define AM43XX_EFUSE_SMA_OPP_100_600MHZ_BIT (1 << 2)
+#define AM43XX_EFUSE_SMA_OPP_120_720MHZ_BIT (1 << 3)
+#define AM43XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 4)
+#define AM43XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 5)
+
#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 6/9] ARM: dts: omap443x: Add opp-modifier entry and add higher OPPs
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add an entry for opp_modifier which configures OPPs on omap443x. Within
this, nodes are defined with the opp-modifier propety that are defined as
a list of frequency, offset from base register, and register value. The
CPU node passes a phandle to the appropriate child node to get the correct
table.
This patch also adds higher eFused OPPs for omap443x.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/omap443x.dtsi | 16 ++++++++++++++++
include/dt-bindings/opp/ti.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi
index 8c1cfad..1c7e730 100644
--- a/arch/arm/boot/dts/omap443x.dtsi
+++ b/arch/arm/boot/dts/omap443x.dtsi
@@ -9,6 +9,7 @@
*/
#include "omap4.dtsi"
+#include <dt-bindings/opp/ti.h>
/ {
cpus {
@@ -20,9 +21,12 @@
600000 1200000
800000 1313000
1008000 1375000
+ 1200000 1388000
>;
clock-latency = <300000>; /* From legacy driver */
+ platform-opp-modifier = <&mpu_opp_modifier>;
+
/* cooling options */
cooling-min-level = <0>;
cooling-max-level = <3>;
@@ -42,6 +46,18 @@
#thermal-sensor-cells = <0>;
};
+
+ opp_modifier: opp_modifier@0x4A002218 {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x4A002218 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1200000 0 OMAP4_EFUSE_HAS_PERF_SILICON_BIT
+ >;
+ };
+ };
};
};
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
index d3717be..cb62c32 100644
--- a/include/dt-bindings/opp/ti.h
+++ b/include/dt-bindings/opp/ti.h
@@ -25,4 +25,6 @@
#define AM43XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 4)
#define AM43XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 5)
+#define OMAP4_EFUSE_HAS_PERF_SILICON_BIT (1 << 17)
+
#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 6/9] ARM: dts: omap443x: Add opp-modifier entry and add higher OPPs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry for opp_modifier which configures OPPs on omap443x. Within
this, nodes are defined with the opp-modifier propety that are defined as
a list of frequency, offset from base register, and register value. The
CPU node passes a phandle to the appropriate child node to get the correct
table.
This patch also adds higher eFused OPPs for omap443x.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/omap443x.dtsi | 16 ++++++++++++++++
include/dt-bindings/opp/ti.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi
index 8c1cfad..1c7e730 100644
--- a/arch/arm/boot/dts/omap443x.dtsi
+++ b/arch/arm/boot/dts/omap443x.dtsi
@@ -9,6 +9,7 @@
*/
#include "omap4.dtsi"
+#include <dt-bindings/opp/ti.h>
/ {
cpus {
@@ -20,9 +21,12 @@
600000 1200000
800000 1313000
1008000 1375000
+ 1200000 1388000
>;
clock-latency = <300000>; /* From legacy driver */
+ platform-opp-modifier = <&mpu_opp_modifier>;
+
/* cooling options */
cooling-min-level = <0>;
cooling-max-level = <3>;
@@ -42,6 +46,18 @@
#thermal-sensor-cells = <0>;
};
+
+ opp_modifier: opp_modifier at 0x4A002218 {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x4A002218 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1200000 0 OMAP4_EFUSE_HAS_PERF_SILICON_BIT
+ >;
+ };
+ };
};
};
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
index d3717be..cb62c32 100644
--- a/include/dt-bindings/opp/ti.h
+++ b/include/dt-bindings/opp/ti.h
@@ -25,4 +25,6 @@
#define AM43XX_EFUSE_SMA_OPP_TURBO_800MHZ_BIT (1 << 4)
#define AM43XX_EFUSE_SMA_OPP_NITRO_1GHZ_BIT (1 << 5)
+#define OMAP4_EFUSE_HAS_PERF_SILICON_BIT (1 << 17)
+
#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 7/9] ARM: dts: omap4460: Add opp-modifier entry and add higher OPPs
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add an entry for opp_modifier which configures OPPs on omap4460. Within
this, nodes are defined with the opp-modifier propety that are defined
as a list of frequency, offset from base register, and register value.
The CPU node passes a phandle to the appropriate child node to get the
correct table.
This patch also adds higher eFused OPPs for omap4460.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/omap4460.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
index 6b32f52..5271121 100644
--- a/arch/arm/boot/dts/omap4460.dtsi
+++ b/arch/arm/boot/dts/omap4460.dtsi
@@ -8,6 +8,7 @@
* kind, whether express or implied.
*/
#include "omap4.dtsi"
+#include <dt-bindings/opp/ti.h>
/ {
cpus {
@@ -18,9 +19,13 @@
350000 1025000
700000 1200000
920000 1313000
+ 1200000 1380000
+ 1500000 1390000
>;
clock-latency = <300000>; /* From legacy driver */
+ platform-opp-modifier = <&mpu_opp_modifier>;
+
/* cooling options */
cooling-min-level = <0>;
cooling-max-level = <2>;
@@ -50,6 +55,18 @@
#thermal-sensor-cells = <0>;
};
+
+ opp_modifier: opp_modifier@0x4A002218 {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x4A002218 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1500000 0 OMAP4_EFUSE_HAS_PERF_SILICON_BIT
+ >;
+ };
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 7/9] ARM: dts: omap4460: Add opp-modifier entry and add higher OPPs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry for opp_modifier which configures OPPs on omap4460. Within
this, nodes are defined with the opp-modifier propety that are defined
as a list of frequency, offset from base register, and register value.
The CPU node passes a phandle to the appropriate child node to get the
correct table.
This patch also adds higher eFused OPPs for omap4460.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/omap4460.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/omap4460.dtsi b/arch/arm/boot/dts/omap4460.dtsi
index 6b32f52..5271121 100644
--- a/arch/arm/boot/dts/omap4460.dtsi
+++ b/arch/arm/boot/dts/omap4460.dtsi
@@ -8,6 +8,7 @@
* kind, whether express or implied.
*/
#include "omap4.dtsi"
+#include <dt-bindings/opp/ti.h>
/ {
cpus {
@@ -18,9 +19,13 @@
350000 1025000
700000 1200000
920000 1313000
+ 1200000 1380000
+ 1500000 1390000
>;
clock-latency = <300000>; /* From legacy driver */
+ platform-opp-modifier = <&mpu_opp_modifier>;
+
/* cooling options */
cooling-min-level = <0>;
cooling-max-level = <2>;
@@ -50,6 +55,18 @@
#thermal-sensor-cells = <0>;
};
+
+ opp_modifier: opp_modifier at 0x4A002218 {
+ compatible = "opp-modifier-reg-bit";
+ reg = <0x4A002218 0x04>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1500000 0 OMAP4_EFUSE_HAS_PERF_SILICON_BIT
+ >;
+ };
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 8/9] ARM: dts: dra7: Add opp-modifier device entry and add higher OPPs
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add an entry for opp-modifier which configures OPPs on 43xx. Within
this, nodes are defined with the opp-modifier propety that are defined as
a list of frequency, offset from base register, and efuse value.
The CPU node passes a phandle to the appropriate child node to get the
correct table.
This patch also adds higher eFused OPPs for dra7.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/dra7.dtsi | 18 ++++++++++++++++++
include/dt-bindings/opp/ti.h | 3 +++
2 files changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 1fd75aa..ffd0bae 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -9,6 +9,7 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/pinctrl/dra.h>
+#include <dt-bindings/opp/ti.h>
#include "skeleton.dtsi"
@@ -46,7 +47,10 @@
/* kHz uV */
1000000 1060000
1176000 1160000
+ 1500000 1260000
>;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
};
cpu@1 {
device_type = "cpu";
@@ -621,6 +625,20 @@
dma-names = "tx0", "rx0";
status = "disabled";
};
+
+ opp_modifier: opp_modifier@0x4AE0C20C {
+ compatible = "opp-modifier-reg-val";
+ reg = <0x4AE0C20C 0x04>;
+ opp,reg-mask = <0x000000F7>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1500000 0 DRA7_EFUSE_HAS_HIGH_MPU_OPP
+ 1176000 0 DRA7_EFUSE_HAS_OD_MPU_OPP
+ >;
+ };
+ };
};
};
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
index cb62c32..0f5efe8 100644
--- a/include/dt-bindings/opp/ti.h
+++ b/include/dt-bindings/opp/ti.h
@@ -27,4 +27,7 @@
#define OMAP4_EFUSE_HAS_PERF_SILICON_BIT (1 << 17)
+#define DRA7_EFUSE_HAS_OD_MPU_OPP 11
+#define DRA7_EFUSE_HAS_HIGH_MPU_OPP 15
+
#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 8/9] ARM: dts: dra7: Add opp-modifier device entry and add higher OPPs
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry for opp-modifier which configures OPPs on 43xx. Within
this, nodes are defined with the opp-modifier propety that are defined as
a list of frequency, offset from base register, and efuse value.
The CPU node passes a phandle to the appropriate child node to get the
correct table.
This patch also adds higher eFused OPPs for dra7.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/dra7.dtsi | 18 ++++++++++++++++++
include/dt-bindings/opp/ti.h | 3 +++
2 files changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 1fd75aa..ffd0bae 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -9,6 +9,7 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/pinctrl/dra.h>
+#include <dt-bindings/opp/ti.h>
#include "skeleton.dtsi"
@@ -46,7 +47,10 @@
/* kHz uV */
1000000 1060000
1176000 1160000
+ 1500000 1260000
>;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
};
cpu at 1 {
device_type = "cpu";
@@ -621,6 +625,20 @@
dma-names = "tx0", "rx0";
status = "disabled";
};
+
+ opp_modifier: opp_modifier at 0x4AE0C20C {
+ compatible = "opp-modifier-reg-val";
+ reg = <0x4AE0C20C 0x04>;
+ opp,reg-mask = <0x000000F7>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1500000 0 DRA7_EFUSE_HAS_HIGH_MPU_OPP
+ 1176000 0 DRA7_EFUSE_HAS_OD_MPU_OPP
+ >;
+ };
+ };
};
};
diff --git a/include/dt-bindings/opp/ti.h b/include/dt-bindings/opp/ti.h
index cb62c32..0f5efe8 100644
--- a/include/dt-bindings/opp/ti.h
+++ b/include/dt-bindings/opp/ti.h
@@ -27,4 +27,7 @@
#define OMAP4_EFUSE_HAS_PERF_SILICON_BIT (1 << 17)
+#define DRA7_EFUSE_HAS_OD_MPU_OPP 11
+#define DRA7_EFUSE_HAS_HIGH_MPU_OPP 15
+
#endif /* __DT_BINDINGS_OPP_TI_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 9/9] ARM: dts: imx6q: Add opp-modifier device entry
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 19:25 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel, linux-omap, linux-pm
Cc: cpufreq, devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar, Nishanth Menon,
Dave Gerlach
Add an entry for opp_modifier which configures OPPs on imx6q. Within
this nodes are defined with opp-modifier propety that are defined as a list
of frequency, offset from base register, and efuse value.
This is an untested example patch to show how opp-modifier could be used
for this platform.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/imx6q.dtsi | 18 ++++++++++++++++++
include/dt-bindings/opp/imx.h | 17 +++++++++++++++++
2 files changed, 35 insertions(+)
create mode 100644 include/dt-bindings/opp/imx.h
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index f024ef2..4cc48ef 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -10,6 +10,7 @@
#include "imx6q-pinfunc.h"
#include "imx6qdl.dtsi"
+#include <dt-bindings/opp/imx.h>
/ {
cpus {
@@ -36,6 +37,8 @@
arm-supply = <®_arm>;
pu-supply = <®_pu>;
soc-supply = <®_soc>;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
};
cpu@1 {
@@ -140,6 +143,21 @@
clock-names = "bus", "di0", "di1";
resets = <&src 4>;
};
+
+ opp_modifier: opp_modifier@0x021bc440 {
+ compatible = "opp-modifier-reg-val";
+ reg = <0x021bc440 0x04>;
+ opp,reg-mask = <0x00030000>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1200000 0 OPP_OCOTP_CFG3_SPEED_1P2GHZ
+ 996000 0 OPP_OCOTP_CFG3_SPEED_996MHZ
+ >;
+ };
+ };
+
};
};
diff --git a/include/dt-bindings/opp/imx.h b/include/dt-bindings/opp/imx.h
new file mode 100644
index 0000000..2700ab9
--- /dev/null
+++ b/include/dt-bindings/opp/imx.h
@@ -0,0 +1,17 @@
+/*
+ * This header provides constants for iMX SoC OPP bindings.
+ *
+ * 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.
+ *
+ */
+
+#ifndef __DT_BINDINGS_OPP_IMX_H__
+#define __DT_BINDINGS_OPP_IMX_H__
+
+#define OPP_OCOTP_CFG3_SPEED_1P2GHZ (0x3 << 16)
+#define OPP_OCOTP_CFG3_SPEED_996MHZ (0x2 << 16)
+#define OPP_OCOTP_CFG3_SPEED_852MHZ (0x1 << 16)
+
+#endif /* __DT_BINDINGS_OPP_IMX_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC 9/9] ARM: dts: imx6q: Add opp-modifier device entry
@ 2014-03-14 19:25 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-14 19:25 UTC (permalink / raw)
To: linux-arm-kernel
Add an entry for opp_modifier which configures OPPs on imx6q. Within
this nodes are defined with opp-modifier propety that are defined as a list
of frequency, offset from base register, and efuse value.
This is an untested example patch to show how opp-modifier could be used
for this platform.
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
arch/arm/boot/dts/imx6q.dtsi | 18 ++++++++++++++++++
include/dt-bindings/opp/imx.h | 17 +++++++++++++++++
2 files changed, 35 insertions(+)
create mode 100644 include/dt-bindings/opp/imx.h
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index f024ef2..4cc48ef 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -10,6 +10,7 @@
#include "imx6q-pinfunc.h"
#include "imx6qdl.dtsi"
+#include <dt-bindings/opp/imx.h>
/ {
cpus {
@@ -36,6 +37,8 @@
arm-supply = <®_arm>;
pu-supply = <®_pu>;
soc-supply = <®_soc>;
+
+ platform-opp-modifier = <&mpu_opp_modifier>;
};
cpu at 1 {
@@ -140,6 +143,21 @@
clock-names = "bus", "di0", "di1";
resets = <&src 4>;
};
+
+ opp_modifier: opp_modifier at 0x021bc440 {
+ compatible = "opp-modifier-reg-val";
+ reg = <0x021bc440 0x04>;
+ opp,reg-mask = <0x00030000>;
+
+ mpu_opp_modifier: mpu_opp_modifier {
+ opp-modifier = <
+ /* kHz offset value */
+ 1200000 0 OPP_OCOTP_CFG3_SPEED_1P2GHZ
+ 996000 0 OPP_OCOTP_CFG3_SPEED_996MHZ
+ >;
+ };
+ };
+
};
};
diff --git a/include/dt-bindings/opp/imx.h b/include/dt-bindings/opp/imx.h
new file mode 100644
index 0000000..2700ab9
--- /dev/null
+++ b/include/dt-bindings/opp/imx.h
@@ -0,0 +1,17 @@
+/*
+ * This header provides constants for iMX SoC OPP bindings.
+ *
+ * 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.
+ *
+ */
+
+#ifndef __DT_BINDINGS_OPP_IMX_H__
+#define __DT_BINDINGS_OPP_IMX_H__
+
+#define OPP_OCOTP_CFG3_SPEED_1P2GHZ (0x3 << 16)
+#define OPP_OCOTP_CFG3_SPEED_996MHZ (0x2 << 16)
+#define OPP_OCOTP_CFG3_SPEED_852MHZ (0x1 << 16)
+
+#endif /* __DT_BINDINGS_OPP_IMX_H__ */
--
1.9.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
2014-03-14 19:25 ` Dave Gerlach
@ 2014-03-14 21:00 ` Rob Herring
-1 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2014-03-14 21:00 UTC (permalink / raw)
To: Dave Gerlach
Cc: linux-arm-kernel, linux-omap, linux-pm, cpufreq, devicetree,
kernel, Rafael J. Wysocki, Jisheng Zhang, Anson Huang, Shawn Guo,
Viresh Kumar, Nishanth Menon
On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> Driver to read from a register and depending on either set bits or
> a specific known selectively enable or disable OPPs based on DT node.
>
> Can support opp-modifier-reg-bit where single bits within the register
> determine the availability of an OPP or opp-modifier-reg-val where a
> certain value inside the register or a portion of it determine what the
> maximum allowed OPP is.
>
> The driver expects a device that has already has its OPPs loaded
> and then will disable the OPPs not matching the criteria specified in
> the opp-modifier table.
>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
> drivers/power/opp/Makefile | 1 +
> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
> 3 files changed, 371 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>
> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
> new file mode 100644
> index 0000000..af8a2e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
> @@ -0,0 +1,111 @@
> +* OPP-Modifier - opp modifier to selectively enable operating points
> +
> +Many SoCs that have selectively modifiable OPPs can specify
> +all available OPPs in their operating-points listing and then define
> +opp_modifiers to enable or disable the OPPs that are actually available
> +on the specific hardware.
> +
> +* OPP Modifier Provider
Uggg. Please stop designing around the current OPP binding which has
the problem that the OPP table is not extensible to add more data.
Define a new OPP binding that solves these problems. This is at least
the 3rd OPP related binding addition I've seen recently. But I
wouldn't spend a lot of effort on a new OPP binding just to add the
functionality you are adding here because I don't like the whole
concept in general. This might be a common way to determine valid OPPs
on TI chips, but I think it is too low level and I don't want to see
bindings for every different possible way. Just add platform code to
do the OPP setup you need.
Frankly, I prefer the bootloader/firmware fixup the OPP table approach
mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
kernel could do the fixups as well (via of_update_property).
Rob
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
@ 2014-03-14 21:00 ` Rob Herring
0 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2014-03-14 21:00 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> Driver to read from a register and depending on either set bits or
> a specific known selectively enable or disable OPPs based on DT node.
>
> Can support opp-modifier-reg-bit where single bits within the register
> determine the availability of an OPP or opp-modifier-reg-val where a
> certain value inside the register or a portion of it determine what the
> maximum allowed OPP is.
>
> The driver expects a device that has already has its OPPs loaded
> and then will disable the OPPs not matching the criteria specified in
> the opp-modifier table.
>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
> drivers/power/opp/Makefile | 1 +
> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
> 3 files changed, 371 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>
> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
> new file mode 100644
> index 0000000..af8a2e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
> @@ -0,0 +1,111 @@
> +* OPP-Modifier - opp modifier to selectively enable operating points
> +
> +Many SoCs that have selectively modifiable OPPs can specify
> +all available OPPs in their operating-points listing and then define
> +opp_modifiers to enable or disable the OPPs that are actually available
> +on the specific hardware.
> +
> +* OPP Modifier Provider
Uggg. Please stop designing around the current OPP binding which has
the problem that the OPP table is not extensible to add more data.
Define a new OPP binding that solves these problems. This is at least
the 3rd OPP related binding addition I've seen recently. But I
wouldn't spend a lot of effort on a new OPP binding just to add the
functionality you are adding here because I don't like the whole
concept in general. This might be a common way to determine valid OPPs
on TI chips, but I think it is too low level and I don't want to see
bindings for every different possible way. Just add platform code to
do the OPP setup you need.
Frankly, I prefer the bootloader/firmware fixup the OPP table approach
mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
kernel could do the fixups as well (via of_update_property).
Rob
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
2014-03-14 21:00 ` Rob Herring
@ 2014-03-17 14:30 ` Nishanth Menon
-1 siblings, 0 replies; 30+ messages in thread
From: Nishanth Menon @ 2014-03-17 14:30 UTC (permalink / raw)
To: Rob Herring, Dave Gerlach
Cc: Jisheng Zhang, devicetree, Shawn Guo, linux-pm, Viresh Kumar,
Anson Huang, Rafael J. Wysocki, cpufreq, kernel, linux-omap,
linux-arm-kernel
On 03/14/2014 04:00 PM, Rob Herring wrote:
> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Driver to read from a register and depending on either set bits or
>> a specific known selectively enable or disable OPPs based on DT node.
>>
>> Can support opp-modifier-reg-bit where single bits within the register
>> determine the availability of an OPP or opp-modifier-reg-val where a
>> certain value inside the register or a portion of it determine what the
>> maximum allowed OPP is.
>>
>> The driver expects a device that has already has its OPPs loaded
>> and then will disable the OPPs not matching the criteria specified in
>> the opp-modifier table.
>>
>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> ---
>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>> drivers/power/opp/Makefile | 1 +
>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>> 3 files changed, 371 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>
>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>> new file mode 100644
>> index 0000000..af8a2e9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>> @@ -0,0 +1,111 @@
>> +* OPP-Modifier - opp modifier to selectively enable operating points
>> +
>> +Many SoCs that have selectively modifiable OPPs can specify
>> +all available OPPs in their operating-points listing and then define
>> +opp_modifiers to enable or disable the OPPs that are actually available
>> +on the specific hardware.
>> +
>> +* OPP Modifier Provider
>
> Uggg. Please stop designing around the current OPP binding which has
> the problem that the OPP table is not extensible to add more data.
> Define a new OPP binding that solves these problems. This is at least
Generically, there are three different issues with current OPP bindings:
a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
settings.
b) ability to reuse OPPs defined for one device node for another (cpu1
to reuse OPP definitions of cpu0)
c) ability to add additional information per OPP. we can argue this is
a superset of (a), but really, the problems are different.
Previous proposals include making each OPP as a phandle, but there
does not seem much traction in that direction either. - proposal here
has nothing to do with (b) or (c).
> the 3rd OPP related binding addition I've seen recently. But I
> wouldn't spend a lot of effort on a new OPP binding just to add the
> functionality you are adding here because I don't like the whole
> concept in general. This might be a common way to determine valid OPPs
> on TI chips, but I think it is too low level and I don't want to see
Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
more as well. doing OTP/Efuse based decision on which OPPs are valid
on a chip is not a TI specific thing. This was the reason for us to
try to define something generic enough to be reused by more SoCs than
just TI.
> bindings for every different possible way. Just add platform code to
> do the OPP setup you need.
Errr.. adding platform code means the hardware description goes back
to kernel. is'nt that giving up on device tree binding for describing
hardware?
>
> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
> kernel could do the fixups as well (via of_update_property).
a) Trying to move the hardware definition away from device tree seems
to me a major step backwards.
b) Allowing for definitions in platform code is a step backwards again
for a generic solution that works for more than 1 vendor.
c) moving the logic away to bootloader when it can easily be done in
kernel again is adding burden to bootloader for data it does need to
handle.
OPP is a hardware behavior, which OPPs are enabled are described in
hardware on certain SoCs. the current proposal is to provide a generic
solution for those devices that allow for dynamic definition of OPPs
based on SoC efuse definition.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
@ 2014-03-17 14:30 ` Nishanth Menon
0 siblings, 0 replies; 30+ messages in thread
From: Nishanth Menon @ 2014-03-17 14:30 UTC (permalink / raw)
To: linux-arm-kernel
On 03/14/2014 04:00 PM, Rob Herring wrote:
> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Driver to read from a register and depending on either set bits or
>> a specific known selectively enable or disable OPPs based on DT node.
>>
>> Can support opp-modifier-reg-bit where single bits within the register
>> determine the availability of an OPP or opp-modifier-reg-val where a
>> certain value inside the register or a portion of it determine what the
>> maximum allowed OPP is.
>>
>> The driver expects a device that has already has its OPPs loaded
>> and then will disable the OPPs not matching the criteria specified in
>> the opp-modifier table.
>>
>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> ---
>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>> drivers/power/opp/Makefile | 1 +
>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>> 3 files changed, 371 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>
>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>> new file mode 100644
>> index 0000000..af8a2e9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>> @@ -0,0 +1,111 @@
>> +* OPP-Modifier - opp modifier to selectively enable operating points
>> +
>> +Many SoCs that have selectively modifiable OPPs can specify
>> +all available OPPs in their operating-points listing and then define
>> +opp_modifiers to enable or disable the OPPs that are actually available
>> +on the specific hardware.
>> +
>> +* OPP Modifier Provider
>
> Uggg. Please stop designing around the current OPP binding which has
> the problem that the OPP table is not extensible to add more data.
> Define a new OPP binding that solves these problems. This is at least
Generically, there are three different issues with current OPP bindings:
a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
settings.
b) ability to reuse OPPs defined for one device node for another (cpu1
to reuse OPP definitions of cpu0)
c) ability to add additional information per OPP. we can argue this is
a superset of (a), but really, the problems are different.
Previous proposals include making each OPP as a phandle, but there
does not seem much traction in that direction either. - proposal here
has nothing to do with (b) or (c).
> the 3rd OPP related binding addition I've seen recently. But I
> wouldn't spend a lot of effort on a new OPP binding just to add the
> functionality you are adding here because I don't like the whole
> concept in general. This might be a common way to determine valid OPPs
> on TI chips, but I think it is too low level and I don't want to see
Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
more as well. doing OTP/Efuse based decision on which OPPs are valid
on a chip is not a TI specific thing. This was the reason for us to
try to define something generic enough to be reused by more SoCs than
just TI.
> bindings for every different possible way. Just add platform code to
> do the OPP setup you need.
Errr.. adding platform code means the hardware description goes back
to kernel. is'nt that giving up on device tree binding for describing
hardware?
>
> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
> kernel could do the fixups as well (via of_update_property).
a) Trying to move the hardware definition away from device tree seems
to me a major step backwards.
b) Allowing for definitions in platform code is a step backwards again
for a generic solution that works for more than 1 vendor.
c) moving the logic away to bootloader when it can easily be done in
kernel again is adding burden to bootloader for data it does need to
handle.
OPP is a hardware behavior, which OPPs are enabled are described in
hardware on certain SoCs. the current proposal is to provide a generic
solution for those devices that allow for dynamic definition of OPPs
based on SoC efuse definition.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
2014-03-17 14:30 ` Nishanth Menon
@ 2014-03-17 18:37 ` Rob Herring
-1 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2014-03-17 18:37 UTC (permalink / raw)
To: Nishanth Menon
Cc: Dave Gerlach, linux-arm-kernel, linux-omap, linux-pm, cpufreq,
devicetree, kernel, Rafael J. Wysocki, Jisheng Zhang,
Anson Huang, Shawn Guo, Viresh Kumar
On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon <nm@ti.com> wrote:
> On 03/14/2014 04:00 PM, Rob Herring wrote:
>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>> Driver to read from a register and depending on either set bits or
>>> a specific known selectively enable or disable OPPs based on DT node.
>>>
>>> Can support opp-modifier-reg-bit where single bits within the register
>>> determine the availability of an OPP or opp-modifier-reg-val where a
>>> certain value inside the register or a portion of it determine what the
>>> maximum allowed OPP is.
>>>
>>> The driver expects a device that has already has its OPPs loaded
>>> and then will disable the OPPs not matching the criteria specified in
>>> the opp-modifier table.
>>>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> ---
>>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>>> drivers/power/opp/Makefile | 1 +
>>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>>> 3 files changed, 371 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>> new file mode 100644
>>> index 0000000..af8a2e9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>> @@ -0,0 +1,111 @@
>>> +* OPP-Modifier - opp modifier to selectively enable operating points
>>> +
>>> +Many SoCs that have selectively modifiable OPPs can specify
>>> +all available OPPs in their operating-points listing and then define
>>> +opp_modifiers to enable or disable the OPPs that are actually available
>>> +on the specific hardware.
>>> +
>>> +* OPP Modifier Provider
>>
>> Uggg. Please stop designing around the current OPP binding which has
>> the problem that the OPP table is not extensible to add more data.
>> Define a new OPP binding that solves these problems. This is at least
> Generically, there are three different issues with current OPP bindings:
> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
> settings.
More generically: ...depending on variety of factors.
> b) ability to reuse OPPs defined for one device node for another (cpu1
> to reuse OPP definitions of cpu0)
> c) ability to add additional information per OPP. we can argue this is
> a superset of (a), but really, the problems are different.
It is all additional data per OPP. Additional different information is
of course for different problems. That doesn't mean we need different
solutions.
> Previous proposals include making each OPP as a phandle, but there
> does not seem much traction in that direction either. - proposal here
> has nothing to do with (b) or (c).
They may have nothing to do with each other, but they all have to do
with the OPP binding. If we're going to change/extend the binding,
then all issues need to be taken into account.
>> the 3rd OPP related binding addition I've seen recently. But I
>> wouldn't spend a lot of effort on a new OPP binding just to add the
>> functionality you are adding here because I don't like the whole
>> concept in general. This might be a common way to determine valid OPPs
>> on TI chips, but I think it is too low level and I don't want to see
>
> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
> more as well. doing OTP/Efuse based decision on which OPPs are valid
> on a chip is not a TI specific thing. This was the reason for us to
> try to define something generic enough to be reused by more SoCs than
> just TI.
Agreed, but I'm not convinced how different SOCs determine valid OPPs
is common enough. Certainly how to mark an entry disabled is common
though.
>> bindings for every different possible way. Just add platform code to
>> do the OPP setup you need.
> Errr.. adding platform code means the hardware description goes back
> to kernel. is'nt that giving up on device tree binding for describing
> hardware?
We're always going to have some platform code. I'm not saying you have
to in this case. I'm saying either come up with an OPP binding
addressing all these issues or live with the existing one and fix it
up in the kernel or bootloader.
>> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
>> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
>> kernel could do the fixups as well (via of_update_property).
>
> a) Trying to move the hardware definition away from device tree seems
> to me a major step backwards.
> b) Allowing for definitions in platform code is a step backwards again
> for a generic solution that works for more than 1 vendor.
> c) moving the logic away to bootloader when it can easily be done in
> kernel again is adding burden to bootloader for data it does need to
> handle.
The burden has to be somewhere. Maintaining a binding forever in the
kernel is a burden as well if it is poorly designed.
Valid OPPs are not going to just be random. There's probably on a few
combinations and they'll be based on part# or speed grade or something
(which in turn defines the efuses in your case). While a dev board may
have random parts on it, an actual product would not. I could argue
that your DTB just needs to be correct to begin with for a given
part/design. Obviously, managing minor differences in a DTB like this
can be a pain. This is why firmware or bootloaders do adjustments to
the DTB at runtime and it is quite common.
> OPP is a hardware behavior, which OPPs are enabled are described in
> hardware on certain SoCs. the current proposal is to provide a generic
> solution for those devices that allow for dynamic definition of OPPs
> based on SoC efuse definition.
What if the decision is not based on a single register bit? Perhaps
efuses are not directly memory mapped. Maybe it is based on Si
revision. Or you need to limit frequency because a certain board can't
supply adequate current. You call this generic, but it is not. It
doesn't even solve the part that is generic which is marking some OPPs
disabled.
Rob
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
@ 2014-03-17 18:37 ` Rob Herring
0 siblings, 0 replies; 30+ messages in thread
From: Rob Herring @ 2014-03-17 18:37 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon <nm@ti.com> wrote:
> On 03/14/2014 04:00 PM, Rob Herring wrote:
>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>> Driver to read from a register and depending on either set bits or
>>> a specific known selectively enable or disable OPPs based on DT node.
>>>
>>> Can support opp-modifier-reg-bit where single bits within the register
>>> determine the availability of an OPP or opp-modifier-reg-val where a
>>> certain value inside the register or a portion of it determine what the
>>> maximum allowed OPP is.
>>>
>>> The driver expects a device that has already has its OPPs loaded
>>> and then will disable the OPPs not matching the criteria specified in
>>> the opp-modifier table.
>>>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> ---
>>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>>> drivers/power/opp/Makefile | 1 +
>>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>>> 3 files changed, 371 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>> new file mode 100644
>>> index 0000000..af8a2e9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>> @@ -0,0 +1,111 @@
>>> +* OPP-Modifier - opp modifier to selectively enable operating points
>>> +
>>> +Many SoCs that have selectively modifiable OPPs can specify
>>> +all available OPPs in their operating-points listing and then define
>>> +opp_modifiers to enable or disable the OPPs that are actually available
>>> +on the specific hardware.
>>> +
>>> +* OPP Modifier Provider
>>
>> Uggg. Please stop designing around the current OPP binding which has
>> the problem that the OPP table is not extensible to add more data.
>> Define a new OPP binding that solves these problems. This is at least
> Generically, there are three different issues with current OPP bindings:
> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
> settings.
More generically: ...depending on variety of factors.
> b) ability to reuse OPPs defined for one device node for another (cpu1
> to reuse OPP definitions of cpu0)
> c) ability to add additional information per OPP. we can argue this is
> a superset of (a), but really, the problems are different.
It is all additional data per OPP. Additional different information is
of course for different problems. That doesn't mean we need different
solutions.
> Previous proposals include making each OPP as a phandle, but there
> does not seem much traction in that direction either. - proposal here
> has nothing to do with (b) or (c).
They may have nothing to do with each other, but they all have to do
with the OPP binding. If we're going to change/extend the binding,
then all issues need to be taken into account.
>> the 3rd OPP related binding addition I've seen recently. But I
>> wouldn't spend a lot of effort on a new OPP binding just to add the
>> functionality you are adding here because I don't like the whole
>> concept in general. This might be a common way to determine valid OPPs
>> on TI chips, but I think it is too low level and I don't want to see
>
> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
> more as well. doing OTP/Efuse based decision on which OPPs are valid
> on a chip is not a TI specific thing. This was the reason for us to
> try to define something generic enough to be reused by more SoCs than
> just TI.
Agreed, but I'm not convinced how different SOCs determine valid OPPs
is common enough. Certainly how to mark an entry disabled is common
though.
>> bindings for every different possible way. Just add platform code to
>> do the OPP setup you need.
> Errr.. adding platform code means the hardware description goes back
> to kernel. is'nt that giving up on device tree binding for describing
> hardware?
We're always going to have some platform code. I'm not saying you have
to in this case. I'm saying either come up with an OPP binding
addressing all these issues or live with the existing one and fix it
up in the kernel or bootloader.
>> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
>> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
>> kernel could do the fixups as well (via of_update_property).
>
> a) Trying to move the hardware definition away from device tree seems
> to me a major step backwards.
> b) Allowing for definitions in platform code is a step backwards again
> for a generic solution that works for more than 1 vendor.
> c) moving the logic away to bootloader when it can easily be done in
> kernel again is adding burden to bootloader for data it does need to
> handle.
The burden has to be somewhere. Maintaining a binding forever in the
kernel is a burden as well if it is poorly designed.
Valid OPPs are not going to just be random. There's probably on a few
combinations and they'll be based on part# or speed grade or something
(which in turn defines the efuses in your case). While a dev board may
have random parts on it, an actual product would not. I could argue
that your DTB just needs to be correct to begin with for a given
part/design. Obviously, managing minor differences in a DTB like this
can be a pain. This is why firmware or bootloaders do adjustments to
the DTB at runtime and it is quite common.
> OPP is a hardware behavior, which OPPs are enabled are described in
> hardware on certain SoCs. the current proposal is to provide a generic
> solution for those devices that allow for dynamic definition of OPPs
> based on SoC efuse definition.
What if the decision is not based on a single register bit? Perhaps
efuses are not directly memory mapped. Maybe it is based on Si
revision. Or you need to limit frequency because a certain board can't
supply adequate current. You call this generic, but it is not. It
doesn't even solve the part that is generic which is marking some OPPs
disabled.
Rob
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
2014-03-17 18:37 ` Rob Herring
@ 2014-03-18 15:36 ` Nishanth Menon
-1 siblings, 0 replies; 30+ messages in thread
From: Nishanth Menon @ 2014-03-18 15:36 UTC (permalink / raw)
To: Rob Herring
Cc: Jisheng Zhang, devicetree, Shawn Guo, linux-pm, Viresh Kumar,
Anson Huang, Dave Gerlach, Rafael J. Wysocki, cpufreq, kernel,
linux-omap, linux-arm-kernel
On 03/17/2014 01:37 PM, Rob Herring wrote:
> On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon <nm@ti.com> wrote:
>> On 03/14/2014 04:00 PM, Rob Herring wrote:
>>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>> Driver to read from a register and depending on either set bits or
>>>> a specific known selectively enable or disable OPPs based on DT node.
>>>>
>>>> Can support opp-modifier-reg-bit where single bits within the register
>>>> determine the availability of an OPP or opp-modifier-reg-val where a
>>>> certain value inside the register or a portion of it determine what the
>>>> maximum allowed OPP is.
>>>>
>>>> The driver expects a device that has already has its OPPs loaded
>>>> and then will disable the OPPs not matching the criteria specified in
>>>> the opp-modifier table.
>>>>
>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>> ---
>>>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>>>> drivers/power/opp/Makefile | 1 +
>>>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>>>> 3 files changed, 371 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>>>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>> new file mode 100644
>>>> index 0000000..af8a2e9
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>> @@ -0,0 +1,111 @@
>>>> +* OPP-Modifier - opp modifier to selectively enable operating points
>>>> +
>>>> +Many SoCs that have selectively modifiable OPPs can specify
>>>> +all available OPPs in their operating-points listing and then define
>>>> +opp_modifiers to enable or disable the OPPs that are actually available
>>>> +on the specific hardware.
>>>> +
>>>> +* OPP Modifier Provider
>>>
>>> Uggg. Please stop designing around the current OPP binding which has
>>> the problem that the OPP table is not extensible to add more data.
>>> Define a new OPP binding that solves these problems. This is at least
>> Generically, there are three different issues with current OPP bindings:
>> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
>> settings.
>
> More generically: ...depending on variety of factors.
Agreed.
>
>> b) ability to reuse OPPs defined for one device node for another (cpu1
>> to reuse OPP definitions of cpu0)
>> c) ability to add additional information per OPP. we can argue this is
>> a superset of (a), but really, the problems are different.
>
> It is all additional data per OPP. Additional different information is
> of course for different problems. That doesn't mean we need different
> solutions.
>
>> Previous proposals include making each OPP as a phandle, but there
>> does not seem much traction in that direction either. - proposal here
>> has nothing to do with (b) or (c).
>
> They may have nothing to do with each other, but they all have to do
> with the OPP binding. If we're going to change/extend the binding,
> then all issues need to be taken into account.
We aren't extending the existing binding in this series. We are just
defining how hardware description of which OPPs are valid here.
>>> the 3rd OPP related binding addition I've seen recently. But I
>>> wouldn't spend a lot of effort on a new OPP binding just to add the
>>> functionality you are adding here because I don't like the whole
>>> concept in general. This might be a common way to determine valid OPPs
>>> on TI chips, but I think it is too low level and I don't want to see
>>
>> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
>> more as well. doing OTP/Efuse based decision on which OPPs are valid
>> on a chip is not a TI specific thing. This was the reason for us to
>> try to define something generic enough to be reused by more SoCs than
>> just TI.
>
> Agreed, but I'm not convinced how different SOCs determine valid OPPs
> is common enough. Certainly how to mark an entry disabled is common
> though.
Fair enough, without procuring NDA documents for all the SoCs, I
cannot comment much either, all we can do is see threads such as
http://marc.info/?t=139470791100003&r=1&w=2 and propose.
This series does include iMx as well which seems to have equivalent
challenges.
I have given examples here on how the current driver at least tries to
make generic the instances of SoCs that we have today, further, the
driver in no way constraints us from using opp_modifier_register with
proper ops in case we do something weirdly different (example: use non
memory mapped operations) - it is just a simple framework.
>
>>> bindings for every different possible way. Just add platform code to
>>> do the OPP setup you need.
>> Errr.. adding platform code means the hardware description goes back
>> to kernel. is'nt that giving up on device tree binding for describing
>> hardware?
>
> We're always going to have some platform code. I'm not saying you have
> to in this case. I'm saying either come up with an OPP binding
> addressing all these issues or live with the existing one and fix it
> up in the kernel or bootloader.
bootloader is out of the picture considering most of the platforms
need to deal with legacy bootloaders.
then tying part of the data in kernel and part in dts!
>
>>> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
>>> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
>>> kernel could do the fixups as well (via of_update_property).
>>
>> a) Trying to move the hardware definition away from device tree seems
>> to me a major step backwards.
>> b) Allowing for definitions in platform code is a step backwards again
>> for a generic solution that works for more than 1 vendor.
>> c) moving the logic away to bootloader when it can easily be done in
>> kernel again is adding burden to bootloader for data it does need to
>> handle.
>
> The burden has to be somewhere. Maintaining a binding forever in the
> kernel is a burden as well if it is poorly designed.
>
> Valid OPPs are not going to just be random. There's probably on a few
> combinations and they'll be based on part# or speed grade or something
> (which in turn defines the efuses in your case). While a dev board may
> have random parts on it, an actual product would not. I could argue
> that your DTB just needs to be correct to begin with for a given
> part/design. Obviously, managing minor differences in a DTB like this
> can be a pain. This is why firmware or bootloaders do adjustments to
> the DTB at runtime and it is quite common.
Bootloaders may not always be capable of doing things or may just be
legacy bootloader that were created in a world where kernel was self
sustaining (opp data was in the kernel previously in OMAP as an
example). asking bootloader to change to ensure dtbs are proper is
just opening up another can of worms here.
>
>> OPP is a hardware behavior, which OPPs are enabled are described in
>> hardware on certain SoCs. the current proposal is to provide a generic
>> solution for those devices that allow for dynamic definition of OPPs
>> based on SoC efuse definition.
>
> What if the decision is not based on a single register bit? Perhaps
> efuses are not directly memory mapped. Maybe it is based on Si
> revision. Or you need to limit frequency because a certain board can't
> supply adequate current. You call this generic, but it is not. It
> doesn't even solve the part that is generic which is marking some OPPs
> disabled.
Are we saying that having a generic layer which may decide on which
OPPs are valid and which are not is a no-no? the RFC has a few issues,
I agree, but that is part of our review process to help improve if we
think the over all concept is good enough to carry forward for next
patch iteration. You dont seem convinced enough to think that makes
sense here.
As I mentioned, patch #1 is the framework, patch #2 is a specific
implementation(and there are improvements possible)-> if we need to
add sil revision based logic OR have current supply based
implementation OR have non memory mapped based decision making,
there'd be specific drivers for them.
The key question is this: do we have an conceptual agreement that
making the decision on which OPPs are valid is a decision for the
kernel? if yes, lets make that as the standard, if kernel should not
do it, then we enforce discipline that bootloaders will mandatorily
implement dtb modification for OPP entries for all SoCs. If I
understand your thought, I think your push is for dtbs containing the
right OPP entries always.
If we agree that kernel should be standalone capable of handling valid
OPPs (which happens to be my view), then lets debate if a generic
layer such as the one proposed should be created helping all SoCs to
operate generically. So far, none of the arguments you have presented
seems to indicate such a generic layer is impossible to do.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
@ 2014-03-18 15:36 ` Nishanth Menon
0 siblings, 0 replies; 30+ messages in thread
From: Nishanth Menon @ 2014-03-18 15:36 UTC (permalink / raw)
To: linux-arm-kernel
On 03/17/2014 01:37 PM, Rob Herring wrote:
> On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon <nm@ti.com> wrote:
>> On 03/14/2014 04:00 PM, Rob Herring wrote:
>>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>> Driver to read from a register and depending on either set bits or
>>>> a specific known selectively enable or disable OPPs based on DT node.
>>>>
>>>> Can support opp-modifier-reg-bit where single bits within the register
>>>> determine the availability of an OPP or opp-modifier-reg-val where a
>>>> certain value inside the register or a portion of it determine what the
>>>> maximum allowed OPP is.
>>>>
>>>> The driver expects a device that has already has its OPPs loaded
>>>> and then will disable the OPPs not matching the criteria specified in
>>>> the opp-modifier table.
>>>>
>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>> ---
>>>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>>>> drivers/power/opp/Makefile | 1 +
>>>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>>>> 3 files changed, 371 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>>>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>> new file mode 100644
>>>> index 0000000..af8a2e9
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>> @@ -0,0 +1,111 @@
>>>> +* OPP-Modifier - opp modifier to selectively enable operating points
>>>> +
>>>> +Many SoCs that have selectively modifiable OPPs can specify
>>>> +all available OPPs in their operating-points listing and then define
>>>> +opp_modifiers to enable or disable the OPPs that are actually available
>>>> +on the specific hardware.
>>>> +
>>>> +* OPP Modifier Provider
>>>
>>> Uggg. Please stop designing around the current OPP binding which has
>>> the problem that the OPP table is not extensible to add more data.
>>> Define a new OPP binding that solves these problems. This is at least
>> Generically, there are three different issues with current OPP bindings:
>> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
>> settings.
>
> More generically: ...depending on variety of factors.
Agreed.
>
>> b) ability to reuse OPPs defined for one device node for another (cpu1
>> to reuse OPP definitions of cpu0)
>> c) ability to add additional information per OPP. we can argue this is
>> a superset of (a), but really, the problems are different.
>
> It is all additional data per OPP. Additional different information is
> of course for different problems. That doesn't mean we need different
> solutions.
>
>> Previous proposals include making each OPP as a phandle, but there
>> does not seem much traction in that direction either. - proposal here
>> has nothing to do with (b) or (c).
>
> They may have nothing to do with each other, but they all have to do
> with the OPP binding. If we're going to change/extend the binding,
> then all issues need to be taken into account.
We aren't extending the existing binding in this series. We are just
defining how hardware description of which OPPs are valid here.
>>> the 3rd OPP related binding addition I've seen recently. But I
>>> wouldn't spend a lot of effort on a new OPP binding just to add the
>>> functionality you are adding here because I don't like the whole
>>> concept in general. This might be a common way to determine valid OPPs
>>> on TI chips, but I think it is too low level and I don't want to see
>>
>> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
>> more as well. doing OTP/Efuse based decision on which OPPs are valid
>> on a chip is not a TI specific thing. This was the reason for us to
>> try to define something generic enough to be reused by more SoCs than
>> just TI.
>
> Agreed, but I'm not convinced how different SOCs determine valid OPPs
> is common enough. Certainly how to mark an entry disabled is common
> though.
Fair enough, without procuring NDA documents for all the SoCs, I
cannot comment much either, all we can do is see threads such as
http://marc.info/?t=139470791100003&r=1&w=2 and propose.
This series does include iMx as well which seems to have equivalent
challenges.
I have given examples here on how the current driver at least tries to
make generic the instances of SoCs that we have today, further, the
driver in no way constraints us from using opp_modifier_register with
proper ops in case we do something weirdly different (example: use non
memory mapped operations) - it is just a simple framework.
>
>>> bindings for every different possible way. Just add platform code to
>>> do the OPP setup you need.
>> Errr.. adding platform code means the hardware description goes back
>> to kernel. is'nt that giving up on device tree binding for describing
>> hardware?
>
> We're always going to have some platform code. I'm not saying you have
> to in this case. I'm saying either come up with an OPP binding
> addressing all these issues or live with the existing one and fix it
> up in the kernel or bootloader.
bootloader is out of the picture considering most of the platforms
need to deal with legacy bootloaders.
then tying part of the data in kernel and part in dts!
>
>>> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
>>> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
>>> kernel could do the fixups as well (via of_update_property).
>>
>> a) Trying to move the hardware definition away from device tree seems
>> to me a major step backwards.
>> b) Allowing for definitions in platform code is a step backwards again
>> for a generic solution that works for more than 1 vendor.
>> c) moving the logic away to bootloader when it can easily be done in
>> kernel again is adding burden to bootloader for data it does need to
>> handle.
>
> The burden has to be somewhere. Maintaining a binding forever in the
> kernel is a burden as well if it is poorly designed.
>
> Valid OPPs are not going to just be random. There's probably on a few
> combinations and they'll be based on part# or speed grade or something
> (which in turn defines the efuses in your case). While a dev board may
> have random parts on it, an actual product would not. I could argue
> that your DTB just needs to be correct to begin with for a given
> part/design. Obviously, managing minor differences in a DTB like this
> can be a pain. This is why firmware or bootloaders do adjustments to
> the DTB at runtime and it is quite common.
Bootloaders may not always be capable of doing things or may just be
legacy bootloader that were created in a world where kernel was self
sustaining (opp data was in the kernel previously in OMAP as an
example). asking bootloader to change to ensure dtbs are proper is
just opening up another can of worms here.
>
>> OPP is a hardware behavior, which OPPs are enabled are described in
>> hardware on certain SoCs. the current proposal is to provide a generic
>> solution for those devices that allow for dynamic definition of OPPs
>> based on SoC efuse definition.
>
> What if the decision is not based on a single register bit? Perhaps
> efuses are not directly memory mapped. Maybe it is based on Si
> revision. Or you need to limit frequency because a certain board can't
> supply adequate current. You call this generic, but it is not. It
> doesn't even solve the part that is generic which is marking some OPPs
> disabled.
Are we saying that having a generic layer which may decide on which
OPPs are valid and which are not is a no-no? the RFC has a few issues,
I agree, but that is part of our review process to help improve if we
think the over all concept is good enough to carry forward for next
patch iteration. You dont seem convinced enough to think that makes
sense here.
As I mentioned, patch #1 is the framework, patch #2 is a specific
implementation(and there are improvements possible)-> if we need to
add sil revision based logic OR have current supply based
implementation OR have non memory mapped based decision making,
there'd be specific drivers for them.
The key question is this: do we have an conceptual agreement that
making the decision on which OPPs are valid is a decision for the
kernel? if yes, lets make that as the standard, if kernel should not
do it, then we enforce discipline that bootloaders will mandatorily
implement dtb modification for OPP entries for all SoCs. If I
understand your thought, I think your push is for dtbs containing the
right OPP entries always.
If we agree that kernel should be standalone capable of handling valid
OPPs (which happens to be my view), then lets debate if a generic
layer such as the one proposed should be created helping all SoCs to
operate generically. So far, none of the arguments you have presented
seems to indicate such a generic layer is impossible to do.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
2014-03-18 15:36 ` Nishanth Menon
@ 2014-03-25 3:24 ` Dave Gerlach
-1 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-25 3:24 UTC (permalink / raw)
To: Nishanth Menon
Cc: Jisheng Zhang, devicetree, Shawn Guo, linux-pm, Viresh Kumar,
Anson Huang, Rafael J. Wysocki, cpufreq, kernel, linux-omap,
linux-arm-kernel
On 03/18/2014 10:36 AM, Nishanth Menon wrote:
> On 03/17/2014 01:37 PM, Rob Herring wrote:
>> On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon <nm@ti.com> wrote:
>>> On 03/14/2014 04:00 PM, Rob Herring wrote:
>>>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>> Driver to read from a register and depending on either set bits or
>>>>> a specific known selectively enable or disable OPPs based on DT node.
>>>>>
>>>>> Can support opp-modifier-reg-bit where single bits within the register
>>>>> determine the availability of an OPP or opp-modifier-reg-val where a
>>>>> certain value inside the register or a portion of it determine what the
>>>>> maximum allowed OPP is.
>>>>>
>>>>> The driver expects a device that has already has its OPPs loaded
>>>>> and then will disable the OPPs not matching the criteria specified in
>>>>> the opp-modifier table.
>>>>>
>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>> ---
>>>>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>>>>> drivers/power/opp/Makefile | 1 +
>>>>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>>>>> 3 files changed, 371 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>>>>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>>> new file mode 100644
>>>>> index 0000000..af8a2e9
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>>> @@ -0,0 +1,111 @@
>>>>> +* OPP-Modifier - opp modifier to selectively enable operating points
>>>>> +
>>>>> +Many SoCs that have selectively modifiable OPPs can specify
>>>>> +all available OPPs in their operating-points listing and then define
>>>>> +opp_modifiers to enable or disable the OPPs that are actually available
>>>>> +on the specific hardware.
>>>>> +
>>>>> +* OPP Modifier Provider
>>>>
>>>> Uggg. Please stop designing around the current OPP binding which has
>>>> the problem that the OPP table is not extensible to add more data.
>>>> Define a new OPP binding that solves these problems. This is at least
>>> Generically, there are three different issues with current OPP bindings:
>>> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
>>> settings.
>>
>> More generically: ...depending on variety of factors.
>
> Agreed.
The idea here was not to touch the existing OPP bindings, hence the opp
"modifier" name. As Nishanth stated, this is not extending the binding;
the opp-modifier just uses the frequency values as an identifier but the
driver does not necessarily care where the OPPs already in the table
came from, just that they correspond to the same frequencies it
describes. And again, just to reiterate, nothing is binding the user to
use the opp-modifier-reg child driver, any driver could be written to
decide which OPPs to enable or disable.
With that said, I do understand that this is far from a perfect solution
to the issue of defining which OPPs are available, I meant it as a
suggestion for a possible way forward. This could be used as a starting
point for something even more generic. It's a common problem on many
SoCs even if it is defined in completely different ways so this
framework or one like it could give a common point to branch out from.
If we don't want to move forward with a generic layer to handle OPP
availability, what is the best option? Does anybody else have opinions
on this? Regardless of what is decided if everyone can agree on a
direction we can all move forward.
Regards,
Dave
>
>>
>>> b) ability to reuse OPPs defined for one device node for another (cpu1
>>> to reuse OPP definitions of cpu0)
>>> c) ability to add additional information per OPP. we can argue this is
>>> a superset of (a), but really, the problems are different.
>>
>> It is all additional data per OPP. Additional different information is
>> of course for different problems. That doesn't mean we need different
>> solutions.
>>
>>> Previous proposals include making each OPP as a phandle, but there
>>> does not seem much traction in that direction either. - proposal here
>>> has nothing to do with (b) or (c).
>>
>> They may have nothing to do with each other, but they all have to do
>> with the OPP binding. If we're going to change/extend the binding,
>> then all issues need to be taken into account.
>
> We aren't extending the existing binding in this series. We are just
> defining how hardware description of which OPPs are valid here.
>
>>>> the 3rd OPP related binding addition I've seen recently. But I
>>>> wouldn't spend a lot of effort on a new OPP binding just to add the
>>>> functionality you are adding here because I don't like the whole
>>>> concept in general. This might be a common way to determine valid OPPs
>>>> on TI chips, but I think it is too low level and I don't want to see
>>>
>>> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
>>> more as well. doing OTP/Efuse based decision on which OPPs are valid
>>> on a chip is not a TI specific thing. This was the reason for us to
>>> try to define something generic enough to be reused by more SoCs than
>>> just TI.
>>
>> Agreed, but I'm not convinced how different SOCs determine valid OPPs
>> is common enough. Certainly how to mark an entry disabled is common
>> though.
>
> Fair enough, without procuring NDA documents for all the SoCs, I
> cannot comment much either, all we can do is see threads such as
> http://marc.info/?t=139470791100003&r=1&w=2 and propose.
>
> This series does include iMx as well which seems to have equivalent
> challenges.
>
> I have given examples here on how the current driver at least tries to
> make generic the instances of SoCs that we have today, further, the
> driver in no way constraints us from using opp_modifier_register with
> proper ops in case we do something weirdly different (example: use non
> memory mapped operations) - it is just a simple framework.
>
>>
>>>> bindings for every different possible way. Just add platform code to
>>>> do the OPP setup you need.
>>> Errr.. adding platform code means the hardware description goes back
>>> to kernel. is'nt that giving up on device tree binding for describing
>>> hardware?
>>
>> We're always going to have some platform code. I'm not saying you have
>> to in this case. I'm saying either come up with an OPP binding
>> addressing all these issues or live with the existing one and fix it
>> up in the kernel or bootloader.
>
> bootloader is out of the picture considering most of the platforms
> need to deal with legacy bootloaders.
>
> then tying part of the data in kernel and part in dts!
>>
>>>> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
>>>> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
>>>> kernel could do the fixups as well (via of_update_property).
>>>
>>> a) Trying to move the hardware definition away from device tree seems
>>> to me a major step backwards.
>>> b) Allowing for definitions in platform code is a step backwards again
>>> for a generic solution that works for more than 1 vendor.
>>> c) moving the logic away to bootloader when it can easily be done in
>>> kernel again is adding burden to bootloader for data it does need to
>>> handle.
>>
>> The burden has to be somewhere. Maintaining a binding forever in the
>> kernel is a burden as well if it is poorly designed.
>
>>
>> Valid OPPs are not going to just be random. There's probably on a few
>> combinations and they'll be based on part# or speed grade or something
>> (which in turn defines the efuses in your case). While a dev board may
>> have random parts on it, an actual product would not. I could argue
>> that your DTB just needs to be correct to begin with for a given
>> part/design. Obviously, managing minor differences in a DTB like this
>> can be a pain. This is why firmware or bootloaders do adjustments to
>> the DTB at runtime and it is quite common.
> Bootloaders may not always be capable of doing things or may just be
> legacy bootloader that were created in a world where kernel was self
> sustaining (opp data was in the kernel previously in OMAP as an
> example). asking bootloader to change to ensure dtbs are proper is
> just opening up another can of worms here.
>
>>
>>> OPP is a hardware behavior, which OPPs are enabled are described in
>>> hardware on certain SoCs. the current proposal is to provide a generic
>>> solution for those devices that allow for dynamic definition of OPPs
>>> based on SoC efuse definition.
>>
>> What if the decision is not based on a single register bit? Perhaps
>> efuses are not directly memory mapped. Maybe it is based on Si
>> revision. Or you need to limit frequency because a certain board can't
>> supply adequate current. You call this generic, but it is not. It
>> doesn't even solve the part that is generic which is marking some OPPs
>> disabled.
>
> Are we saying that having a generic layer which may decide on which
> OPPs are valid and which are not is a no-no? the RFC has a few issues,
> I agree, but that is part of our review process to help improve if we
> think the over all concept is good enough to carry forward for next
> patch iteration. You dont seem convinced enough to think that makes
> sense here.
>
> As I mentioned, patch #1 is the framework, patch #2 is a specific
> implementation(and there are improvements possible)-> if we need to
> add sil revision based logic OR have current supply based
> implementation OR have non memory mapped based decision making,
> there'd be specific drivers for them.
>
> The key question is this: do we have an conceptual agreement that
> making the decision on which OPPs are valid is a decision for the
> kernel? if yes, lets make that as the standard, if kernel should not
> do it, then we enforce discipline that bootloaders will mandatorily
> implement dtb modification for OPP entries for all SoCs. If I
> understand your thought, I think your push is for dtbs containing the
> right OPP entries always.
>
> If we agree that kernel should be standalone capable of handling valid
> OPPs (which happens to be my view), then lets debate if a generic
> layer such as the one proposed should be created helping all SoCs to
> operate generically. So far, none of the arguments you have presented
> seems to indicate such a generic layer is impossible to do.
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
@ 2014-03-25 3:24 ` Dave Gerlach
0 siblings, 0 replies; 30+ messages in thread
From: Dave Gerlach @ 2014-03-25 3:24 UTC (permalink / raw)
To: linux-arm-kernel
On 03/18/2014 10:36 AM, Nishanth Menon wrote:
> On 03/17/2014 01:37 PM, Rob Herring wrote:
>> On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon <nm@ti.com> wrote:
>>> On 03/14/2014 04:00 PM, Rob Herring wrote:
>>>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>> Driver to read from a register and depending on either set bits or
>>>>> a specific known selectively enable or disable OPPs based on DT node.
>>>>>
>>>>> Can support opp-modifier-reg-bit where single bits within the register
>>>>> determine the availability of an OPP or opp-modifier-reg-val where a
>>>>> certain value inside the register or a portion of it determine what the
>>>>> maximum allowed OPP is.
>>>>>
>>>>> The driver expects a device that has already has its OPPs loaded
>>>>> and then will disable the OPPs not matching the criteria specified in
>>>>> the opp-modifier table.
>>>>>
>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>> ---
>>>>> .../devicetree/bindings/power/opp-modifier.txt | 111 +++++++++
>>>>> drivers/power/opp/Makefile | 1 +
>>>>> drivers/power/opp/opp-modifier-reg.c | 259 +++++++++++++++++++++
>>>>> 3 files changed, 371 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt
>>>>> create mode 100644 drivers/power/opp/opp-modifier-reg.c
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>>> new file mode 100644
>>>>> index 0000000..af8a2e9
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt
>>>>> @@ -0,0 +1,111 @@
>>>>> +* OPP-Modifier - opp modifier to selectively enable operating points
>>>>> +
>>>>> +Many SoCs that have selectively modifiable OPPs can specify
>>>>> +all available OPPs in their operating-points listing and then define
>>>>> +opp_modifiers to enable or disable the OPPs that are actually available
>>>>> +on the specific hardware.
>>>>> +
>>>>> +* OPP Modifier Provider
>>>>
>>>> Uggg. Please stop designing around the current OPP binding which has
>>>> the problem that the OPP table is not extensible to add more data.
>>>> Define a new OPP binding that solves these problems. This is at least
>>> Generically, there are three different issues with current OPP bindings:
>>> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse
>>> settings.
>>
>> More generically: ...depending on variety of factors.
>
> Agreed.
The idea here was not to touch the existing OPP bindings, hence the opp
"modifier" name. As Nishanth stated, this is not extending the binding;
the opp-modifier just uses the frequency values as an identifier but the
driver does not necessarily care where the OPPs already in the table
came from, just that they correspond to the same frequencies it
describes. And again, just to reiterate, nothing is binding the user to
use the opp-modifier-reg child driver, any driver could be written to
decide which OPPs to enable or disable.
With that said, I do understand that this is far from a perfect solution
to the issue of defining which OPPs are available, I meant it as a
suggestion for a possible way forward. This could be used as a starting
point for something even more generic. It's a common problem on many
SoCs even if it is defined in completely different ways so this
framework or one like it could give a common point to branch out from.
If we don't want to move forward with a generic layer to handle OPP
availability, what is the best option? Does anybody else have opinions
on this? Regardless of what is decided if everyone can agree on a
direction we can all move forward.
Regards,
Dave
>
>>
>>> b) ability to reuse OPPs defined for one device node for another (cpu1
>>> to reuse OPP definitions of cpu0)
>>> c) ability to add additional information per OPP. we can argue this is
>>> a superset of (a), but really, the problems are different.
>>
>> It is all additional data per OPP. Additional different information is
>> of course for different problems. That doesn't mean we need different
>> solutions.
>>
>>> Previous proposals include making each OPP as a phandle, but there
>>> does not seem much traction in that direction either. - proposal here
>>> has nothing to do with (b) or (c).
>>
>> They may have nothing to do with each other, but they all have to do
>> with the OPP binding. If we're going to change/extend the binding,
>> then all issues need to be taken into account.
>
> We aren't extending the existing binding in this series. We are just
> defining how hardware description of which OPPs are valid here.
>
>>>> the 3rd OPP related binding addition I've seen recently. But I
>>>> wouldn't spend a lot of effort on a new OPP binding just to add the
>>>> functionality you are adding here because I don't like the whole
>>>> concept in general. This might be a common way to determine valid OPPs
>>>> on TI chips, but I think it is too low level and I don't want to see
>>>
>>> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially
>>> more as well. doing OTP/Efuse based decision on which OPPs are valid
>>> on a chip is not a TI specific thing. This was the reason for us to
>>> try to define something generic enough to be reused by more SoCs than
>>> just TI.
>>
>> Agreed, but I'm not convinced how different SOCs determine valid OPPs
>> is common enough. Certainly how to mark an entry disabled is common
>> though.
>
> Fair enough, without procuring NDA documents for all the SoCs, I
> cannot comment much either, all we can do is see threads such as
> http://marc.info/?t=139470791100003&r=1&w=2 and propose.
>
> This series does include iMx as well which seems to have equivalent
> challenges.
>
> I have given examples here on how the current driver at least tries to
> make generic the instances of SoCs that we have today, further, the
> driver in no way constraints us from using opp_modifier_register with
> proper ops in case we do something weirdly different (example: use non
> memory mapped operations) - it is just a simple framework.
>
>>
>>>> bindings for every different possible way. Just add platform code to
>>>> do the OPP setup you need.
>>> Errr.. adding platform code means the hardware description goes back
>>> to kernel. is'nt that giving up on device tree binding for describing
>>> hardware?
>>
>> We're always going to have some platform code. I'm not saying you have
>> to in this case. I'm saying either come up with an OPP binding
>> addressing all these issues or live with the existing one and fix it
>> up in the kernel or bootloader.
>
> bootloader is out of the picture considering most of the platforms
> need to deal with legacy bootloaders.
>
> then tying part of the data in kernel and part in dts!
>>
>>>> Frankly, I prefer the bootloader/firmware fixup the OPP table approach
>>>> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the
>>>> kernel could do the fixups as well (via of_update_property).
>>>
>>> a) Trying to move the hardware definition away from device tree seems
>>> to me a major step backwards.
>>> b) Allowing for definitions in platform code is a step backwards again
>>> for a generic solution that works for more than 1 vendor.
>>> c) moving the logic away to bootloader when it can easily be done in
>>> kernel again is adding burden to bootloader for data it does need to
>>> handle.
>>
>> The burden has to be somewhere. Maintaining a binding forever in the
>> kernel is a burden as well if it is poorly designed.
>
>>
>> Valid OPPs are not going to just be random. There's probably on a few
>> combinations and they'll be based on part# or speed grade or something
>> (which in turn defines the efuses in your case). While a dev board may
>> have random parts on it, an actual product would not. I could argue
>> that your DTB just needs to be correct to begin with for a given
>> part/design. Obviously, managing minor differences in a DTB like this
>> can be a pain. This is why firmware or bootloaders do adjustments to
>> the DTB at runtime and it is quite common.
> Bootloaders may not always be capable of doing things or may just be
> legacy bootloader that were created in a world where kernel was self
> sustaining (opp data was in the kernel previously in OMAP as an
> example). asking bootloader to change to ensure dtbs are proper is
> just opening up another can of worms here.
>
>>
>>> OPP is a hardware behavior, which OPPs are enabled are described in
>>> hardware on certain SoCs. the current proposal is to provide a generic
>>> solution for those devices that allow for dynamic definition of OPPs
>>> based on SoC efuse definition.
>>
>> What if the decision is not based on a single register bit? Perhaps
>> efuses are not directly memory mapped. Maybe it is based on Si
>> revision. Or you need to limit frequency because a certain board can't
>> supply adequate current. You call this generic, but it is not. It
>> doesn't even solve the part that is generic which is marking some OPPs
>> disabled.
>
> Are we saying that having a generic layer which may decide on which
> OPPs are valid and which are not is a no-no? the RFC has a few issues,
> I agree, but that is part of our review process to help improve if we
> think the over all concept is good enough to carry forward for next
> patch iteration. You dont seem convinced enough to think that makes
> sense here.
>
> As I mentioned, patch #1 is the framework, patch #2 is a specific
> implementation(and there are improvements possible)-> if we need to
> add sil revision based logic OR have current supply based
> implementation OR have non memory mapped based decision making,
> there'd be specific drivers for them.
>
> The key question is this: do we have an conceptual agreement that
> making the decision on which OPPs are valid is a decision for the
> kernel? if yes, lets make that as the standard, if kernel should not
> do it, then we enforce discipline that bootloaders will mandatorily
> implement dtb modification for OPP entries for all SoCs. If I
> understand your thought, I think your push is for dtbs containing the
> right OPP entries always.
>
> If we agree that kernel should be standalone capable of handling valid
> OPPs (which happens to be my view), then lets debate if a generic
> layer such as the one proposed should be created helping all SoCs to
> operate generically. So far, none of the arguments you have presented
> seems to indicate such a generic layer is impossible to do.
>
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2014-03-25 3:24 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-14 19:25 [RFC 0/9] Introduce OPP modifier for ARM SoCs Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 1/9] opp-modifier: Introduce OPP Modifier Framework Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 2/9] opp-modifier: Add opp-modifier-reg driver Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 21:00 ` Rob Herring
2014-03-14 21:00 ` Rob Herring
2014-03-17 14:30 ` Nishanth Menon
2014-03-17 14:30 ` Nishanth Menon
2014-03-17 18:37 ` Rob Herring
2014-03-17 18:37 ` Rob Herring
2014-03-18 15:36 ` Nishanth Menon
2014-03-18 15:36 ` Nishanth Menon
2014-03-25 3:24 ` Dave Gerlach
2014-03-25 3:24 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 3/9] PM / OPP: Add hook to modify OPPs after they are loaded Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 4/9] ARM: dts: AM33XX: Add opp-modifier device entry and add higher OPPs Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 5/9] ARM: dts: AM4372: " Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 6/9] ARM: dts: omap443x: Add opp-modifier " Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 7/9] ARM: dts: omap4460: " Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 8/9] ARM: dts: dra7: Add opp-modifier device " Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
2014-03-14 19:25 ` [RFC 9/9] ARM: dts: imx6q: Add opp-modifier device entry Dave Gerlach
2014-03-14 19:25 ` Dave Gerlach
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.