Re: [PATCH v3 2/2] regulator: add device tree support for max8997
On 13 March 2012 09:13, Grant Likely grant.lik...@secretlab.ca wrote:
On Thu, 23 Feb 2012 18:08:08 +0530, Thomas Abraham
thomas.abra...@linaro.org wrote:
Add device tree based discovery support for max8997.
Cc: MyungJoo Ham myungjoo@samsung.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
.../devicetree/bindings/regulator/max8997-pmic.txt | 134
+++
drivers/mfd/max8997.c | 72 ++-
drivers/regulator/max8997.c | 139
+++-
include/linux/mfd/max8997.h | 1 +
4 files changed, 344 insertions(+), 2 deletions(-)
create mode 100644
Documentation/devicetree/bindings/regulator/max8997-pmic.txt
[...]
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 20ecad3..13180be 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -23,6 +23,7 @@
#include linux/slab.h
#include linux/i2c.h
+#include linux/of_irq.h
#include linux/interrupt.h
#include linux/pm_runtime.h
#include linux/module.h
@@ -123,6 +124,60 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg,
u8 val, u8 mask)
}
EXPORT_SYMBOL_GPL(max8997_update_reg);
+#ifdef CONFIG_OF
+/*
+ * Only the common platform data elements for max8997 are parsed here from
the
+ * device tree. Other sub-modules of max8997 such as pmic, rtc and others
have
+ * to parse their own platform data elements from device tree.
+ *
+ * The max8997 platform data structure is instantiated here and the drivers
for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static int max8997_i2c_parse_dt_pdata(struct device *dev,
+ struct max8997_platform_data **pdata)
+{
+ struct max8997_platform_data *pd;
+
+ pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+ if (!pd) {
+ dev_err(dev, could not allocate memory for pdata\n);
+ return -ENOMEM;
+ }
+
+ pd-ono = irq_of_parse_and_map(dev-of_node, 1);
+
+ /*
+ * ToDo: the 'wakeup' member in the platform data is more of a linux
+ * specfic information. Hence, there is no binding for that yet and
+ * not parsed here.
+ */
+
+ *pdata = pd;
+ return 0;
+}
+#else
+static int max8997_i2c_parse_dt_pdata(struct device *dev,
+ struct max8997_platform_data **pdata)
+{
+ return 0;
+}
+#endif
+
+static struct of_device_id __devinitdata max8997_pmic_dt_match[];
Move the actual definition of this structure up to here so that a forward
declaration becomes unnecessary.
Ok.
+static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+#ifdef CONFIG_OF
+ if (i2c-dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_node(max8997_pmic_dt_match, i2c-dev.of_node);
+ return (int)match-data;
+ }
+#endif
+ return (int)id-driver_data;
+}
+
static int max8997_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -137,9 +192,16 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
i2c_set_clientdata(i2c, max8997);
max8997-dev = i2c-dev;
max8997-i2c = i2c;
- max8997-type = id-driver_data;
+ max8997-type = max8997_i2c_get_driver_data(i2c, id);
max8997-irq = i2c-irq;
+ if (max8997-dev-of_node) {
+ ret = max8997_i2c_parse_dt_pdata(max8997-dev, pdata);
+ if (ret)
+ goto err;
+ i2c-dev.platform_data = pdata;
This line is illegal. Drivers must *not* change the value of
dev-platform_data.
instead the valid pdata pointer needs to be stored in the max8997 private data
structure.
The intention here was to let the max8897 driver's mfd portion create
an 'common' instance of max8897 platform data and allow the max8997
sub-drivers such as the pmic driver and charger driver use that
'common' instance to populate their respective platform data portions
from dt. But, assigning that allocated pdata pointer to
i2c-dev.platform_data was not correct, I agree.
Instead, a new pointer to 'struct max8997_platform_data' will be added
to 'struct max8997_dev', assign the pdata in the mfd driver to this
new member and let the sub-drivers use the pdata pointer in 'struct
max8997_dev'. I will do this change and repost this patch.
+ }
+
If you tweak the definition of max8997_i2c_parse_dt_pdata() then this block
can
be simplified to:
pdata = max8997_i2c_parse_dt_pdata(max8997-dev, pdata);
if (!pdata)
pdata = i2c-dev.platform_data,
if (!pdata)
Re: [PATCH v3 2/2] regulator: add device tree support for max8997
On Tue, 13 Mar 2012 12:14:42 +0530, Thomas Abraham thomas.abra...@linaro.org wrote: On 13 March 2012 09:13, Grant Likely grant.lik...@secretlab.ca wrote: On Thu, 23 Feb 2012 18:08:08 +0530, Thomas Abraham thomas.abra...@linaro.org wrote: Otherwise, the patch looks pretty good. Â (although seeing the decode function has got me thinking that we need a much better way of decoding the dt data). The parsing function looks huge since there is lot of data to pick up from the dt node. Oh, I understand that you have to do it this way; I just think the core code should make it a lot easier. :-) g. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] regulator: add device tree support for max8997
On Thu, 23 Feb 2012 18:08:08 +0530, Thomas Abraham thomas.abra...@linaro.org
wrote:
Add device tree based discovery support for max8997.
Cc: MyungJoo Ham myungjoo@samsung.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
.../devicetree/bindings/regulator/max8997-pmic.txt | 134 +++
drivers/mfd/max8997.c | 72 ++-
drivers/regulator/max8997.c| 139
+++-
include/linux/mfd/max8997.h|1 +
4 files changed, 344 insertions(+), 2 deletions(-)
create mode 100644
Documentation/devicetree/bindings/regulator/max8997-pmic.txt
diff --git a/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
new file mode 100644
index 000..d282635
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
@@ -0,0 +1,134 @@
+* Maxim MAX8997 Voltage and Current Regulator
+
+The Maxim MAX8997 is a multi-function device which includes volatage and
+current regulators, rtc, charger controller and other sub-blocks. It is
+interfaced to the host controller using a i2c interface. Each sub-block is
+addressed by the host system using different i2c slave address. This document
+describes the bindings for 'pmic' sub-block of max8997.
+
+Required properties:
+- compatible: Should be maxim,max8997-pmic.
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+- max8997,pmic-buck1-dvs-voltage: A set of 8 voltage values in micro-volt
(uV)
+ units for buck1 when changing voltage using gpio dvs. Refer to [1] below
+ for additional information.
+
+- max8997,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt
(uV)
+ units for buck2 when changing voltage using gpio dvs. Refer to [1] below
+ for additional information.
+
+- max8997,pmic-buck5-dvs-voltage: A set of 8 voltage values in micro-volt
(uV)
+ units for buck5 when changing voltage using gpio dvs. Refer to [1] below
+ for additional information.
+
+[1] If none of the 'max8997,pmic-buck[1/2/5]-uses-gpio-dvs' optional
+property is specified, the 'max8997,pmic-buck[1/2/5]-dvs-voltage'
+property should specify atleast one voltage level (which would be a
+safe operating voltage).
+
+If either of the 'max8997,pmic-buck[1/2/5]-uses-gpio-dvs' optional
+property is specified, then all the eigth voltage values for the
+'max8997,pmic-buck[1/2/5]-dvs-voltage' should be specified.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to
which
+ the interrupts from max8997 are delivered to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+ - First interrupt specifier is for 'irq1' interrupt.
+ - Second interrupt specifier is for 'alert' interrupt.
+- max8997,pmic-buck1-uses-gpio-dvs: 'buck1' can be controlled by gpio dvs.
+- max8997,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
+- max8997,pmic-buck5-uses-gpio-dvs: 'buck5' can be controlled by gpio dvs.
+
+Additional properties required if either of the optional properties are used:
+- max8997,pmic-ignore-gpiodvs-side-effect: When GPIO-DVS mode is used for
+ multiple bucks, changing the voltage value of one of the bucks may affect
+ that of another buck, which is the side effect of the change (set_voltage).
+ Use this property to ignore such side effects and change the voltage.
+
+- max8997,pmic-buck125-default-dvs-idx: Default voltage setting selected from
+ the possible 8 options selectable by the dvs gpios. The value of this
+ property should be between 0 and 7. If not specified or if out of range,
the
+ default value of this property is set to 0.
+
+- max8997,pmic-buck125-dvs-gpios: GPIO specifiers for three host gpio's used
+ for dvs. The format of the gpio specifier depends in the gpio controller.
+
+
+Regulators: The regulators of max8997 that have to be instantiated should be
+included in a sub-node named 'regulators'. Regulator nodes included in this
+sub-node should be of the format as below. Note: The 'n' in LDOn and BUCKn
+represents the LDO or BUCK number as per the datasheet of max8997.
+
+For LDO's:
+ LDOn {
+ standard regulator bindings here
+ };
+
+For BUCK's:
+ BUCKn {
+ standard regulator bindings here
+ };
+
+The bindings inside the regulator nodes use the standard regulator bindings
+which are documented elsewhere.
+
+Example:
+
+ max8997_pmic@66 {
+ compatible = maxim,max8997-pmic;
+ interrupt-parent = wakeup_eint;
+ reg = 0x66;
+ interrupts = 4 0, 3 0;
+
+
[PATCH v3 2/2] regulator: add device tree support for max8997
Add device tree based discovery support for max8997.
Cc: MyungJoo Ham myungjoo@samsung.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
.../devicetree/bindings/regulator/max8997-pmic.txt | 134 +++
drivers/mfd/max8997.c | 72 ++-
drivers/regulator/max8997.c| 139 +++-
include/linux/mfd/max8997.h|1 +
4 files changed, 344 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/max8997-pmic.txt
diff --git a/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
new file mode 100644
index 000..d282635
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
@@ -0,0 +1,134 @@
+* Maxim MAX8997 Voltage and Current Regulator
+
+The Maxim MAX8997 is a multi-function device which includes volatage and
+current regulators, rtc, charger controller and other sub-blocks. It is
+interfaced to the host controller using a i2c interface. Each sub-block is
+addressed by the host system using different i2c slave address. This document
+describes the bindings for 'pmic' sub-block of max8997.
+
+Required properties:
+- compatible: Should be maxim,max8997-pmic.
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+- max8997,pmic-buck1-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+ units for buck1 when changing voltage using gpio dvs. Refer to [1] below
+ for additional information.
+
+- max8997,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+ units for buck2 when changing voltage using gpio dvs. Refer to [1] below
+ for additional information.
+
+- max8997,pmic-buck5-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+ units for buck5 when changing voltage using gpio dvs. Refer to [1] below
+ for additional information.
+
+[1] If none of the 'max8997,pmic-buck[1/2/5]-uses-gpio-dvs' optional
+property is specified, the 'max8997,pmic-buck[1/2/5]-dvs-voltage'
+property should specify atleast one voltage level (which would be a
+safe operating voltage).
+
+If either of the 'max8997,pmic-buck[1/2/5]-uses-gpio-dvs' optional
+property is specified, then all the eigth voltage values for the
+'max8997,pmic-buck[1/2/5]-dvs-voltage' should be specified.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+ the interrupts from max8997 are delivered to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+ - First interrupt specifier is for 'irq1' interrupt.
+ - Second interrupt specifier is for 'alert' interrupt.
+- max8997,pmic-buck1-uses-gpio-dvs: 'buck1' can be controlled by gpio dvs.
+- max8997,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
+- max8997,pmic-buck5-uses-gpio-dvs: 'buck5' can be controlled by gpio dvs.
+
+Additional properties required if either of the optional properties are used:
+- max8997,pmic-ignore-gpiodvs-side-effect: When GPIO-DVS mode is used for
+ multiple bucks, changing the voltage value of one of the bucks may affect
+ that of another buck, which is the side effect of the change (set_voltage).
+ Use this property to ignore such side effects and change the voltage.
+
+- max8997,pmic-buck125-default-dvs-idx: Default voltage setting selected from
+ the possible 8 options selectable by the dvs gpios. The value of this
+ property should be between 0 and 7. If not specified or if out of range, the
+ default value of this property is set to 0.
+
+- max8997,pmic-buck125-dvs-gpios: GPIO specifiers for three host gpio's used
+ for dvs. The format of the gpio specifier depends in the gpio controller.
+
+
+Regulators: The regulators of max8997 that have to be instantiated should be
+included in a sub-node named 'regulators'. Regulator nodes included in this
+sub-node should be of the format as below. Note: The 'n' in LDOn and BUCKn
+represents the LDO or BUCK number as per the datasheet of max8997.
+
+For LDO's:
+ LDOn {
+ standard regulator bindings here
+ };
+
+For BUCK's:
+ BUCKn {
+ standard regulator bindings here
+ };
+
+The bindings inside the regulator nodes use the standard regulator bindings
+which are documented elsewhere.
+
+Example:
+
+ max8997_pmic@66 {
+ compatible = maxim,max8997-pmic;
+ interrupt-parent = wakeup_eint;
+ reg = 0x66;
+ interrupts = 4 0, 3 0;
+
+ max8997,pmic-buck1-uses-gpio-dvs;
+ max8997,pmic-buck2-uses-gpio-dvs;
+ max8997,pmic-buck5-uses-gpio-dvs;
+
+
