Re: [PATCH v2 2/2] regulator: add device tree support for max8997

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Mr. Ham,

On 12 January 2012 15:19, MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> wrote:
> On Thu, Jan 12, 2012 at 4:35 PM, Thomas Abraham
> <thomas.abraham@xxxxxxxxxx> wrote:
>> Add device tree based discovery support for max8997.
>>
>> Cc: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx>
>> Cc: Rajendra Nayak <rnayak@xxxxxx>
>> Cc: Rob Herring <rob.herring@xxxxxxxxxxx>
>> Cc: Grant Likely <grant.likely@xxxxxxxxxxxx>
>> Signed-off-by: Thomas Abraham <thomas.abraham@xxxxxxxxxx>
>> ---
>>  .../devicetree/bindings/regulator/max8997-pmic.txt |  120 ++++++++++++++++
>>  drivers/mfd/max8997.c                              |   72 ++++++++++-
>>  drivers/regulator/max8997.c                        |  143 +++++++++++++++++++-
>>  include/linux/mfd/max8997.h                        |    1 +
>>  4 files changed, 334 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 0000000..0c4559d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
>> @@ -0,0 +1,120 @@
>> +* 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.
>> +
>> +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.
>> +
>> +- max8997,pmic-buck1-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
>> +  units for buck1 when changing voltage using gpio dvs.
>> +
>> +- max8997,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
>> +  units for buck2 when changing voltage using gpio dvs.
>> +
>> +- max8997,pmic-buck5-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
>> +  units for buck5 when changing voltage using gpio dvs.
>> +
>
> These three values are _not_ optional.
>
> If they are omitted and at least one of buck1/2/5 is supplying
> critical power to the system, which they do in Exynos boards, system
> will not boot properly.
>
> In order to work properly, at least the first element of each array
> should have a proper value (preferrably the maximum safety voltage if
> this gpio-dvs feature is not used) if others are filled with zeros.
>
> If GPIO-DVS mode is enabled, all the 8 elements of each array should
> be filled with proper values.

Thanks for this information. I was not aware of that. I assumed that
if GPIO DVS is not used for BUCK 1/2/5, then there is no need to
supply the DVS voltage options. I will modify this accordingly.

>
>
> []
>> diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
>> index d26e864..053d0b7 100644
>> --- a/drivers/regulator/max8997.c
>> +++ b/drivers/regulator/max8997.c
> []
>> @@ -32,6 +33,7 @@
>>  struct max8997_data {
>>        struct device *dev;
>> @@ -958,6 +960,138 @@ static struct regulator_desc regulators[] = {
>>        },
>>  };
>>
>> +#if CONFIG_OF
>
> #ifdef?

Yes, I will fix this.

>
> []
>> +static int max8997_pmic_dt_parse_pdata(struct max8997_dev *iodev,
>> +                                       struct max8997_platform_data *pdata)
>> +{
> []
>> +       if (pdata->buck1_gpiodvs) {
>> +               if (of_property_read_u32_array(pmic_np,
>> +                               "max8997,pmic-buck1-dvs-voltage",
>> +                               pdata->buck1_voltage, 8)) {
>> +                       dev_err(iodev->dev, "buck1 voltages not specified\n");
>> +                       return -EINVAL;
>> +               }
>> +       }
>> +
>> +       if (pdata->buck2_gpiodvs) {
>> +               if (of_property_read_u32_array(pmic_np,
>> +                               "max8997,pmic-buck2-dvs-voltage",
>> +                               pdata->buck2_voltage, 8)) {
>> +                       dev_err(iodev->dev, "buck2 voltages not specified\n");
>> +                       return -EINVAL;
>> +               }
>> +       }
>> +
>> +       if (pdata->buck5_gpiodvs) {
>> +               if (of_property_read_u32_array(pmic_np,
>> +                               "max8997,pmic-buck5-dvs-voltage",
>> +                               pdata->buck5_voltage, 8)) {
>> +                       dev_err(iodev->dev, "buck5 voltages not specified\n");
>> +                       return -EINVAL;
>> +               }
>> +       }
>
> As mentioned above, these array should be loaded to probe function
> even if buck?_gpiodvs is false.

Ok.

Thanks for reviewing this patch.

Regards,
Thomas.

>
> []
>
>
>
> Cheers!
> MyungJoo
>
>
> --
> MyungJoo Ham, Ph.D.
> Mobile Software Platform Lab, DMC Business, Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux