Re: [PATCH v4 2/2] regulator: add device tree support for max8997
Hi Mark, On 18 April 2012 00:08, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Wed, Apr 18, 2012 at 12:05:59AM +0530, Thomas Abraham wrote: On 28 March 2012 22:33, Karol Lewandowski k.lewando...@samsung.com wrote: + For BUCK's: No 's here, BTW. Ok. - EN32KHz_AP - EN32KHz_CP - ENVICHG - ESAFEOUT1 - ESAFEOUT2 - CHARGER - CHARGER_CV - CHARGER_TOPOFF I wonder if these should be mentioned in documentation too. Yes, I missed the above regulators in the documentation. I have included them now and will resubmit this patch. Please omit the clocks; these are obviously a bodge due to the inability to support clocks off-SoC so we shouldn't be enshrining them in the device tree bindings. Thanks for the suggestion. I have removed EN32KHz_AP and EN32KHz_CP from the list. The rest are either voltage (fixed) or current regulators. Regards, Thomas. -- 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 v4 2/2] regulator: add device tree support for max8997
On 28 March 2012 22:33, Karol Lewandowski k.lewando...@samsung.com wrote:
On 24.03.2012 10:49, Thomas Abraham wrote:
Hi Thomas!
Add device tree based discovery support for max8997.
...
+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
+ };
+
Small note - driver supports[1] configuring following regulators by
using respective DT node names:
- EN32KHz_AP
- EN32KHz_CP
- ENVICHG
- ESAFEOUT1
- ESAFEOUT2
- CHARGER
- CHARGER_CV
- CHARGER_TOPOFF
I wonder if these should be mentioned in documentation too.
[1] These are used in e.g. mach-nuri.c
Yes, I missed the above regulators in the documentation. I have
included them now and will resubmit this patch.
diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
index 9657929..dce8aaf 100644
--- a/drivers/regulator/max8997.c
+++ b/drivers/regulator/max8997.c
..
+static int max8997_pmic_dt_parse_pdata(struct max8997_dev *iodev,
+ struct max8997_platform_data *pdata)
+{
+ struct device_node *pmic_np, *regulators_np, *reg_np;
+ struct max8997_regulator_data *rdata;
+ unsigned int i, dvs_voltage_nr = 1, ret;
+
+ pmic_np = iodev-dev-of_node;
+ if (!pmic_np) {
+ dev_err(iodev-dev, could not find pmic sub-node\n);
+ return -ENODEV;
+ }
+
+ regulators_np = of_find_node_by_name(pmic_np, regulators);
+ if (!regulators_np) {
+ dev_err(iodev-dev, could not find regulators sub-node\n);
+ return -EINVAL;
+ }
+
+ /* count the number of regulators to be supported in pmic */
+ pdata-num_regulators = 0;
+ for_each_child_of_node(regulators_np, reg_np)
+ pdata-num_regulators++;
+
+ rdata = devm_kzalloc(iodev-dev, sizeof(*rdata) *
+ pdata-num_regulators, GFP_KERNEL);
+ if (!rdata) {
+ dev_err(iodev-dev, could not allocate memory for
+ regulator data\n);
+ return -ENOMEM;
+ }
+ pdata-regulators = rdata;
+ for_each_child_of_node(regulators_np, reg_np) {
+ for (i = 0; i ARRAY_SIZE(regulators); i++)
+ if (!of_node_cmp(reg_np-name, regulators[i].name))
+ break;
+ rdata-id = i;
rdata-id will be equal to ARRAY_SIZE(regulators) when one adds DT node
name (below regulators) which is different from what can be found in
regulators[] table.
On my test machine this results in hard lockup - possibly because
something tries to access regulators[ARRAY_SIZE(regulators)]
later on.
Following patch fixes this on my machine (using DTS with misspelled LDO1 for
LDx1):
diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
index dce8aaf..c20fd72 100644
--- a/drivers/regulator/max8997.c
+++ b/drivers/regulator/max8997.c
@@ -1011,6 +1011,13 @@ static int max8997_pmic_dt_parse_pdata(struct
max8997_dev *iodev,
for (i = 0; i ARRAY_SIZE(regulators); i++)
if (!of_node_cmp(reg_np-name, regulators[i].name))
break;
+
+ if (i == ARRAY_SIZE(regulators)) {
+ dev_warn(iodev-dev, don't know how to configure
regulator %s\n,
+ reg_np-name);
+ continue;
+ }
+
rdata-id = i;
rdata-initdata = of_get_regulator_init_data(
iodev-dev, reg_np);
Thanks for this fix. I have merged this change into this patch.
Regards,
Thomas.
Regards,
--
Karol Lewandowski | Samsung Poland RD Center | Linux/Platform
--
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 v4 2/2] regulator: add device tree support for max8997
On Wed, Apr 18, 2012 at 12:05:59AM +0530, Thomas Abraham wrote: On 28 March 2012 22:33, Karol Lewandowski k.lewando...@samsung.com wrote: + For BUCK's: No 's here, BTW. - EN32KHz_AP - EN32KHz_CP - ENVICHG - ESAFEOUT1 - ESAFEOUT2 - CHARGER - CHARGER_CV - CHARGER_TOPOFF I wonder if these should be mentioned in documentation too. Yes, I missed the above regulators in the documentation. I have included them now and will resubmit this patch. Please omit the clocks; these are obviously a bodge due to the inability to support clocks off-SoC so we shouldn't be enshrining them in the device tree bindings. signature.asc Description: Digital signature
Re: [PATCH v4 2/2] regulator: add device tree support for max8997
On Sat, Mar 24, 2012 at 03:19:50PM +0530, Thomas Abraham wrote: Add device tree based discovery support for max8997. I tried to apply this but it's collided with some other changes in the driver which have arrived in the meantime and the rejects were too large to fix up. I suspect it's mostly just the change in parameters for regulator_register(). Can you please regenerate against my current topic/drivers branch? signature.asc Description: Digital signature
Re: [PATCH v4 2/2] regulator: add device tree support for max8997
On 17 April 2012 00:21, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Sat, Mar 24, 2012 at 03:19:50PM +0530, Thomas Abraham wrote: Add device tree based discovery support for max8997. I tried to apply this but it's collided with some other changes in the driver which have arrived in the meantime and the rejects were too large to fix up. I suspect it's mostly just the change in parameters for regulator_register(). Can you please regenerate against my current topic/drivers branch? Hi Mark, Sure, I will do redo this patch based on your current 'topic/drivers' branch. Thanks, Thomas. -- 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 v4 2/2] regulator: add device tree support for max8997
On Sat, 24 Mar 2012 15:19:50 +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 | 133 ++ drivers/mfd/max8997.c | 73 ++- drivers/regulator/max8997.c| 143 +++- include/linux/mfd/max8997-private.h|1 + include/linux/mfd/max8997.h|1 + 5 files changed, 347 insertions(+), 4 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..90a730b --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt @@ -0,0 +1,133 @@ +* 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. These are *really long* property names. Anything over 32 characters seems excessive to me. Other than that the binding looks good. 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 v4 2/2] regulator: add device tree support for max8997
On Sat, Mar 24, 2012 at 6:49 PM, 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
Acked-by: MyungJoo Ham myungjoo@samsung.com
---
.../devicetree/bindings/regulator/max8997-pmic.txt | 133 ++
drivers/mfd/max8997.c | 73 ++-
drivers/regulator/max8997.c | 143
+++-
include/linux/mfd/max8997-private.h | 1 +
include/linux/mfd/max8997.h | 1 +
5 files changed, 347 insertions(+), 4 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..90a730b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
@@ -0,0 +1,133 @@
+* 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;
+
[PATCH v4 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 | 133 ++
drivers/mfd/max8997.c | 73 ++-
drivers/regulator/max8997.c| 143 +++-
include/linux/mfd/max8997-private.h|1 +
include/linux/mfd/max8997.h|1 +
5 files changed, 347 insertions(+), 4 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..90a730b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
@@ -0,0 +1,133 @@
+* 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;
+
