Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-26 Thread Mark Brown
On Mon, Aug 25, 2014 at 08:40:40AM -0700, Doug Anderson wrote:
 On Mon, Aug 25, 2014 at 2:07 AM, Javier Martinez Canillas

  I see, so probably until we have a way to define the operating mode for
  each regulator using DT we should set the opmode to normal when enabling a
  regulator independently of the value the hardware register reported on 
  probe.

  Can you please test the following change [0] so I can post as a proper
  patch? Doug, Mark do you think that forcing the regulator to opmode normal
  when enabling is the right solution here?

 IMHO that makes sense.

No, this doesn't make any obvious sense to me at all.  Picking normal as
a default if the hardware reads back off due to overlapping
impelementation or something *might* make sense but not overwriting the
hardware state without explicit permission from the machine integration
is a key goal for the regulator API.


signature.asc
Description: Digital signature


Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-26 Thread Javier Martinez Canillas
Hello Mark,

On 08/26/2014 09:17 AM, Mark Brown wrote:
 On Mon, Aug 25, 2014 at 08:40:40AM -0700, Doug Anderson wrote:
  Can you please test the following change [0] so I can post as a proper
  patch? Doug, Mark do you think that forcing the regulator to opmode normal
  when enabling is the right solution here?
 
 IMHO that makes sense.
 
 No, this doesn't make any obvious sense to me at all.  Picking normal as
 a default if the hardware reads back off due to overlapping
 impelementation or something *might* make sense but not overwriting the
 hardware state without explicit permission from the machine integration
 is a key goal for the regulator API.
 

Just to be sure I understood you correctly, what might makes sense to you
then is to set the opmode to normal as default on probe only if off is
read back from the hardware register but leaving the enable function as it
is now using the opmode set on probe?

That seems like a safer solution indeed since enable won't overwrite other
values different from off read from the hardware register, I'll prepare a
patch.

Thanks a lot for your help and best regards,
Javier
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-26 Thread Mark Brown
On Tue, Aug 26, 2014 at 11:08:07AM +0200, Javier Martinez Canillas wrote:
 On 08/26/2014 09:17 AM, Mark Brown wrote:

  No, this doesn't make any obvious sense to me at all.  Picking normal as
  a default if the hardware reads back off due to overlapping
  impelementation or something *might* make sense but not overwriting the
  hardware state without explicit permission from the machine integration
  is a key goal for the regulator API.

 Just to be sure I understood you correctly, what might makes sense to you
 then is to set the opmode to normal as default on probe only if off is
 read back from the hardware register but leaving the enable function as it
 is now using the opmode set on probe?

Yes.


signature.asc
Description: Digital signature


Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-25 Thread Yuvaraj Cd
On Sat, Aug 23, 2014 at 3:45 AM, Doug Anderson diand...@chromium.org wrote:
 Hi,

 On Fri, Aug 22, 2014 at 3:02 PM, Javier Martinez Canillas
 javier.marti...@collabora.co.uk wrote:
 Hello Mark,

 On 08/22/2014 08:30 PM, Mark Brown wrote:

 The problem is that one of these regulators is used as the vqmmc-supply
 (VCCQ/VDD_IO) so the mmc host controller driver disables it on
 MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
 shouldn't be an issue since on card detection, the vqmmc supply should be
 enabled again but on Exynos the built-in card detect line is on the same
 power rail as vqmmc. That means that disabling the regulator prevents card
 insertions to be detected.

 If the MMC host controller needs a supply enabling in order to do card
 detection and it's supposed to be doing card detection I'd expect it to
 be enabling that supply.  Why is it not doing that?


 Good question. I'm not that familiar with the dw_mmc host controller nor
 its driver implementation so I'll let Yuvaraj or Doug to answer that.
Well,here it goes!
1. Power ON the board LDO4CTRL1[7:6] 11b
2. dw_mmc driver enable the vqmmc.
3. checks for UHS support, complete the voltage switching t0 1.8V
4. Does warm reset by reboot command.
5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
7.dw_mmc driver enable the vqmmc.
 But after step 7 also, LD4CTRL[7:6] is 00b.

 I haven't seen the issue that Yuvaraj is reporting (but I also haven't
 picked up all of the relevant patches and tried to reproduce), so I'm
 going to have to leave it to Yuvaraj to answer.
static int max77802_enable(struct regulator_dev *rdev)
{
  struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
 int id = rdev_get_id(rdev);
 int shift = max77802_get_opmode_shift(id);
 return regmap_update_bits(rdev-regmap,
rdev-desc-enable_reg,rdev-desc-enable_mask,max77802-opmode[id] 
shift);
  }
I think in the above code snippet, the val is what we got it during
the probe.We always write that value for enabling this regulator(which
is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
the MAX77802 manual.

 As far as I know the dw_mmc driver ought to be enabling vqmmc when it
 needs it.  Perhaps there's a bug in your patch series that adds vqmmc
 support to dw_mmc?

 -Doug
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-25 Thread Javier Martinez Canillas
Hello Yuvaraj,

On 08/25/2014 10:22 AM, Yuvaraj Cd wrote:
 Good question. I'm not that familiar with the dw_mmc host controller nor
 its driver implementation so I'll let Yuvaraj or Doug to answer that.
 Well,here it goes!
 1. Power ON the board LDO4CTRL1[7:6] 11b
 2. dw_mmc driver enable the vqmmc.
 3. checks for UHS support, complete the voltage switching t0 1.8V
 4. Does warm reset by reboot command.
 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
 7.dw_mmc driver enable the vqmmc.
  But after step 7 also, LD4CTRL[7:6] is 00b.

Ok, so the dw_mmc driver is enabling vqmmc, that's good.


 I haven't seen the issue that Yuvaraj is reporting (but I also haven't
 picked up all of the relevant patches and tried to reproduce), so I'm
 going to have to leave it to Yuvaraj to answer.
 static int max77802_enable(struct regulator_dev *rdev)
 {
   struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
  int id = rdev_get_id(rdev);
  int shift = max77802_get_opmode_shift(id);
  return regmap_update_bits(rdev-regmap,
 rdev-desc-enable_reg,rdev-desc-enable_mask,max77802-opmode[id] 
 shift);
   }
 I think in the above code snippet, the val is what we got it during
 the probe.We always write that value for enabling this regulator(which
 is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
 the MAX77802 manual.


I see, so probably until we have a way to define the operating mode for
each regulator using DT we should set the opmode to normal when enabling a
regulator independently of the value the hardware register reported on probe.

Can you please test the following change [0] so I can post as a proper
patch? Doug, Mark do you think that forcing the regulator to opmode normal
when enabling is the right solution here?

Best regards,
Javier

[0]
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index ad1caa9..917b5ab 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -180,7 +180,7 @@ static int max77802_enable(struct regulator_dev *rdev)

return regmap_update_bits(rdev-regmap, rdev-desc-enable_reg,
  rdev-desc-enable_mask,
- max77802-opmode[id]  shift);
+ MAX77802_OPMODE_NORMAL  shift);
 }
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-25 Thread Yuvaraj Cd
On Mon, Aug 25, 2014 at 2:37 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
 Hello Yuvaraj,

 On 08/25/2014 10:22 AM, Yuvaraj Cd wrote:
 Good question. I'm not that familiar with the dw_mmc host controller nor
 its driver implementation so I'll let Yuvaraj or Doug to answer that.
 Well,here it goes!
 1. Power ON the board LDO4CTRL1[7:6] 11b
 2. dw_mmc driver enable the vqmmc.
 3. checks for UHS support, complete the voltage switching t0 1.8V
 4. Does warm reset by reboot command.
 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
 7.dw_mmc driver enable the vqmmc.
  But after step 7 also, LD4CTRL[7:6] is 00b.

 Ok, so the dw_mmc driver is enabling vqmmc, that's good.


 I haven't seen the issue that Yuvaraj is reporting (but I also haven't
 picked up all of the relevant patches and tried to reproduce), so I'm
 going to have to leave it to Yuvaraj to answer.
 static int max77802_enable(struct regulator_dev *rdev)
 {
   struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
  int id = rdev_get_id(rdev);
  int shift = max77802_get_opmode_shift(id);
  return regmap_update_bits(rdev-regmap,
 rdev-desc-enable_reg,rdev-desc-enable_mask,max77802-opmode[id] 
 shift);
   }
 I think in the above code snippet, the val is what we got it during
 the probe.We always write that value for enabling this regulator(which
 is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
 the MAX77802 manual.


 I see, so probably until we have a way to define the operating mode for
 each regulator using DT we should set the opmode to normal when enabling a
 regulator independently of the value the hardware register reported on probe.

 Can you please test the following change [0] so I can post as a proper
 patch? Doug, Mark do you think that forcing the regulator to opmode normal
 when enabling is the right solution here?

 Best regards,
 Javier

 [0]
 diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
 index ad1caa9..917b5ab 100644
 --- a/drivers/regulator/max77802.c
 +++ b/drivers/regulator/max77802.c
 @@ -180,7 +180,7 @@ static int max77802_enable(struct regulator_dev *rdev)

 return regmap_update_bits(rdev-regmap, rdev-desc-enable_reg,
   rdev-desc-enable_mask,
 - max77802-opmode[id]  shift);
 + MAX77802_OPMODE_NORMAL  shift);
  }
It works.
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-25 Thread Doug Anderson
Javier,

On Mon, Aug 25, 2014 at 2:07 AM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
 Hello Yuvaraj,

 On 08/25/2014 10:22 AM, Yuvaraj Cd wrote:
 Good question. I'm not that familiar with the dw_mmc host controller nor
 its driver implementation so I'll let Yuvaraj or Doug to answer that.
 Well,here it goes!
 1. Power ON the board LDO4CTRL1[7:6] 11b
 2. dw_mmc driver enable the vqmmc.
 3. checks for UHS support, complete the voltage switching t0 1.8V
 4. Does warm reset by reboot command.
 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF.
 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b
 7.dw_mmc driver enable the vqmmc.
  But after step 7 also, LD4CTRL[7:6] is 00b.

 Ok, so the dw_mmc driver is enabling vqmmc, that's good.


 I haven't seen the issue that Yuvaraj is reporting (but I also haven't
 picked up all of the relevant patches and tried to reproduce), so I'm
 going to have to leave it to Yuvaraj to answer.
 static int max77802_enable(struct regulator_dev *rdev)
 {
   struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
  int id = rdev_get_id(rdev);
  int shift = max77802_get_opmode_shift(id);
  return regmap_update_bits(rdev-regmap,
 rdev-desc-enable_reg,rdev-desc-enable_mask,max77802-opmode[id] 
 shift);
   }
 I think in the above code snippet, the val is what we got it during
 the probe.We always write that value for enabling this regulator(which
 is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according
 the MAX77802 manual.


 I see, so probably until we have a way to define the operating mode for
 each regulator using DT we should set the opmode to normal when enabling a
 regulator independently of the value the hardware register reported on probe.

 Can you please test the following change [0] so I can post as a proper
 patch? Doug, Mark do you think that forcing the regulator to opmode normal
 when enabling is the right solution here?

IMHO that makes sense.
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-25 Thread Javier Martinez Canillas
Hello Doug,

On 08/25/2014 05:40 PM, Doug Anderson wrote:

 I see, so probably until we have a way to define the operating mode for
 each regulator using DT we should set the opmode to normal when enabling a
 regulator independently of the value the hardware register reported on probe.

 Can you please test the following change [0] so I can post as a proper
 patch? Doug, Mark do you think that forcing the regulator to opmode normal
 when enabling is the right solution here?
 
 IMHO that makes sense.
 

Thanks, I'll post it as a proper patch tomorrow then.

Best regards,
Javier
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Yuvaraj Cd
On Mon, Aug 18, 2014 at 2:02 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
 The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
 (LDO) regulators. This patch adds support for all these regulators
 found on the MAX77802 PMIC and is based on a driver added by Simon
 Glass to the Chrome OS kernel 3.8 tree.

 Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 Tested-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 ---

 Changes since v7:
  - Remove DVS support since that can be added as a follow up.

 Changes since v6: None

 Changes since v5:
  - Take out the mfd changes from v4 that were squashed by mistake.
Suggested by Lee Jones.

 Changes since v4: None

 Changes since v3:
  - Set the supply_name for regulators to lookup their parent supply node.
Suggested by Mark Brown.
  - Change Exyno5 for Exynos5420/Exynos5800 in regulator driver Kconfig.
Suggested by Doug Anderson.
 ---
  drivers/regulator/Kconfig|   9 +
  drivers/regulator/Makefile   |   1 +
  drivers/regulator/max77802.c | 578 
 +++
  3 files changed, 588 insertions(+)
  create mode 100644 drivers/regulator/max77802.c

 diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
 index 2dc8289..8134a99 100644
 --- a/drivers/regulator/Kconfig
 +++ b/drivers/regulator/Kconfig
 @@ -387,6 +387,15 @@ config REGULATOR_MAX77693
   and one current regulator 'CHARGER'. This is suitable for
   Exynos-4x12 chips.

 +config REGULATOR_MAX77802
 +   tristate Maxim 77802 regulator
 +   depends on MFD_MAX77686
 +   help
 + This driver controls a Maxim 77802 regulator
 + via I2C bus. The provided regulator is suitable for
 + Exynos5420/Exynos5800 SoCs to control various voltages.
 + It includes support for control of voltage and ramp speed.
 +
  config REGULATOR_MC13XXX_CORE
 tristate

 diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
 index aa4a6aa..b4ec6c8 100644
 --- a/drivers/regulator/Makefile
 +++ b/drivers/regulator/Makefile
 @@ -52,6 +52,7 @@ obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
  obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
  obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
  obj-$(CONFIG_REGULATOR_MAX77693) += max77693.o
 +obj-$(CONFIG_REGULATOR_MAX77802) += max77802.o
  obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
  obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
  obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
 new file mode 100644
 index 000..5f022f8
 --- /dev/null
 +++ b/drivers/regulator/max77802.c
 @@ -0,0 +1,578 @@
 +/*
 + * max77802.c - Regulator driver for the Maxim 77802
 + *
 + * Copyright (C) 2013-2014 Google, Inc
 + * Simon Glass s...@chromium.org
 + *
 + * Copyright (C) 2012 Samsung Electronics
 + * Chiwoong Byun woong.b...@smasung.com
 + * Jonghwa Lee jonghwa3@samsung.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; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * This driver is based on max8997.c
 + */
 +
 +#include linux/kernel.h
 +#include linux/bug.h
 +#include linux/err.h
 +#include linux/gpio.h
 +#include linux/slab.h
 +#include linux/gpio/consumer.h
 +#include linux/platform_device.h
 +#include linux/regulator/driver.h
 +#include linux/regulator/machine.h
 +#include linux/regulator/of_regulator.h
 +#include linux/mfd/max77686.h
 +#include linux/mfd/max77686-private.h
 +
 +/* Default ramp delay in case it is not manually set */
 +#define MAX77802_RAMP_DELAY10  /* uV/us */
 +
 +#define MAX77802_OPMODE_SHIFT_LDO  6
 +#define MAX77802_OPMODE_BUCK234_SHIFT  4
 +#define MAX77802_OPMODE_MASK   0x3
 +
 +#define MAX77802_VSEL_MASK 0x3F
 +#define MAX77802_DVS_VSEL_MASK 0xFF
 +
 +#define MAX77802_RAMP_RATE_MASK_2BIT   0xC0
 +#define MAX77802_RAMP_RATE_SHIFT_2BIT  6
 +#define MAX77802_RAMP_RATE_MASK_4BIT   0xF0
 +#define MAX77802_RAMP_RATE_SHIFT_4BIT  4
 +
 +/* MAX77802 has two register formats: 2-bit and 4-bit */
 +static const unsigned int ramp_table_77802_2bit[] = {
 +   12500,
 +   25000,
 +   5,
 +   10,
 +};
 +
 +static unsigned int ramp_table_77802_4bit[] = {
 +   1000,   2000,   3030,   4000,
 +   5000,   5880,   7140,   8330,
 +   9090,   1,  0,  12500,
 +   16670,  25000,  5,  10,
 +};
 +
 +struct max77802_regulator_prv {
 +   int num_regulators;
 +   

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Javier Martinez Canillas
Hello Yuvaraj,

On 08/22/2014 08:01 AM, Yuvaraj Cd wrote:
 +
 +static int max77802_pmic_probe(struct platform_device *pdev)
 +{
 +   struct max77686_dev *iodev = dev_get_drvdata(pdev-dev.parent);
 +   struct max77686_platform_data *pdata = dev_get_platdata(iodev-dev);
 +   struct max77802_regulator_prv *max77802;
 +   int i, ret = 0, val;
 +   struct regulator_config config = { };
 +
 +   /* This is allocated by the MFD driver */
 +   if (!pdata) {
 +   dev_err(pdev-dev, no platform data found for 
 regulator\n);
 +   return -ENODEV;
 +   }
 +
 +   max77802 = devm_kzalloc(pdev-dev,
 +   sizeof(struct max77802_regulator_prv),
 +   GFP_KERNEL);
 +   if (!max77802)
 +   return -ENOMEM;
 +
 +   if (iodev-dev-of_node) {
 +   ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
 +   if (ret)
 +   return ret;
 +   }
 +
 +   config.dev = iodev-dev;
 +   config.regmap = iodev-regmap;
 +   config.driver_data = max77802;
 +   platform_set_drvdata(pdev, max77802);
 +
 +   for (i = 0; i  MAX77802_REG_MAX; i++) {
 +   struct regulator_dev *rdev;
 +   int id = pdata-regulators[i].id;
 +   int shift = max77802_get_opmode_shift(id);
 +
 +   config.init_data = pdata-regulators[i].initdata;
 +   config.of_node = pdata-regulators[i].of_node;
 +
 +   ret = regmap_read(iodev-regmap, regulators[i].enable_reg, 
 val);
 +   max77802-opmode[id] = val  shift  MAX77802_OPMODE_MASK;
 
 I have been using this patch series for adding UHS support for dw_mmc
 driver. During reboot testing I came across an issue where card
 detection fails due to vqmmc regulator not getting enabled. On
 debugging further, I found that PMIC driver is reading the operating
 mode during probe and reusing it in the enable function. With the UHS
 patches vqmmc regulator gets disabled during POWER_OFF and if we do
 warm reboot, an incorrect operating mode(OFF) is read. This leads to
 the vqmmc regulator staying disabled. I have referred to 77686 driver
 and observed that they are handling this a little differently. With
 the following change in the driver above issue is resolved:
 
 -   ret = regmap_read(iodev-regmap,
 regulators[i].enable_reg, val);
 -   max77802-opmode[id] = val  shift  MAX77802_OPMODE_MASK;
 +   max77802-opmode[i] = regulators[i].enable_mask  shift;
 

I don't think this change is correct it its current form since
.enable_mask is initialized to MAX77802_OPMODE_MASK  shift so this is
actually setting the opmode to MAX77802_OPMODE_MASK.

Now, MAX77802_OPMODE_MASK has the same value than MAX77802_OPMODE_NORMAL
so what you are really doing here is initializing the opmode to
MAX77802_OPMODE_NORMAL.

 Please have a look and let me know, if there is any better way of handling 
 this.


The first versions of this driver did in fact set the opmode to
MAX77802_OPMODE_NORMAL on probe but Mark asked me to read it from the
hardware instead [0]. I was indeed worried at the time that something like
this could happen on a warm reset [1].

Mark, any opinions on how this should be solved will be highly appreciated.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/6/16/576
[1]: https://lkml.org/lkml/2014/6/17/174
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Mark Brown
On Fri, Aug 22, 2014 at 02:15:46PM +0200, Javier Martinez Canillas wrote:

 Mark, any opinions on how this should be solved will be highly appreciated.

If someone could tell me what this is that'd help...


signature.asc
Description: Digital signature


Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Javier Martinez Canillas
Hello Mark,

On 08/22/2014 04:45 PM, Mark Brown wrote:
 On Fri, Aug 22, 2014 at 02:15:46PM +0200, Javier Martinez Canillas wrote:
 
 Mark, any opinions on how this should be solved will be highly appreciated.
 
 If someone could tell me what this is that'd help...
 

Sorry for not being clear on that regard. By this I meant the problem
reported by Yuvaraj.

The regulators operating mode is read from the hardware registers on probe
as you suggested but that means that if the regulator is disabled and the
machine rebooted (warm reset) the opmode reported by the hardware is
MAX77802_OPMODE_OFF.

The problem is that one of these regulators is used as the vqmmc-supply
(VCCQ/VDD_IO) so the mmc host controller driver disables it on
MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
shouldn't be an issue since on card detection, the vqmmc supply should be
enabled again but on Exynos the built-in card detect line is on the same
power rail as vqmmc. That means that disabling the regulator prevents card
insertions to be detected.

This does not happen on the downstream Chrome OS kernel because the
max77802 regulator driver has some ad-hoc DT bindings were you can define
the operating mode to be set on probe. For this particular regulator is
MAX77802_OPMODE_STANDBY.

Yuvaraj solution was to set the operating mode to MAX77802_OPMODE_NORMAL
on probe() which was what I did on a previous version of the driver but
you explained to me that it was not safe to do that.

That's why I'm asking for suggestions.

Thanks a lot and best regards,
Javier

--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Mark Brown
On Fri, Aug 22, 2014 at 07:53:19PM +0200, Javier Martinez Canillas wrote:
 On 08/22/2014 04:45 PM, Mark Brown wrote:
  On Fri, Aug 22, 2014 at 02:15:46PM +0200, Javier Martinez Canillas wrote:

  Mark, any opinions on how this should be solved will be highly appreciated.

  If someone could tell me what this is that'd help...

 Sorry for not being clear on that regard. By this I meant the problem
 reported by Yuvaraj.

That mail was rather lengthy and seemed to be discussing several issues.

 The problem is that one of these regulators is used as the vqmmc-supply
 (VCCQ/VDD_IO) so the mmc host controller driver disables it on
 MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
 shouldn't be an issue since on card detection, the vqmmc supply should be
 enabled again but on Exynos the built-in card detect line is on the same
 power rail as vqmmc. That means that disabling the regulator prevents card
 insertions to be detected.

If the MMC host controller needs a supply enabling in order to do card
detection and it's supposed to be doing card detection I'd expect it to
be enabling that supply.  Why is it not doing that?


signature.asc
Description: Digital signature


Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Javier Martinez Canillas
Hello Mark,

On 08/22/2014 08:30 PM, Mark Brown wrote:
 
 The problem is that one of these regulators is used as the vqmmc-supply
 (VCCQ/VDD_IO) so the mmc host controller driver disables it on
 MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
 shouldn't be an issue since on card detection, the vqmmc supply should be
 enabled again but on Exynos the built-in card detect line is on the same
 power rail as vqmmc. That means that disabling the regulator prevents card
 insertions to be detected.
 
 If the MMC host controller needs a supply enabling in order to do card
 detection and it's supposed to be doing card detection I'd expect it to
 be enabling that supply.  Why is it not doing that?
 

Good question. I'm not that familiar with the dw_mmc host controller nor
its driver implementation so I'll let Yuvaraj or Doug to answer that.

Best regards,
Javier
--
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 v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-22 Thread Doug Anderson
Hi,

On Fri, Aug 22, 2014 at 3:02 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
 Hello Mark,

 On 08/22/2014 08:30 PM, Mark Brown wrote:

 The problem is that one of these regulators is used as the vqmmc-supply
 (VCCQ/VDD_IO) so the mmc host controller driver disables it on
 MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this
 shouldn't be an issue since on card detection, the vqmmc supply should be
 enabled again but on Exynos the built-in card detect line is on the same
 power rail as vqmmc. That means that disabling the regulator prevents card
 insertions to be detected.

 If the MMC host controller needs a supply enabling in order to do card
 detection and it's supposed to be doing card detection I'd expect it to
 be enabling that supply.  Why is it not doing that?


 Good question. I'm not that familiar with the dw_mmc host controller nor
 its driver implementation so I'll let Yuvaraj or Doug to answer that.

I haven't seen the issue that Yuvaraj is reporting (but I also haven't
picked up all of the relevant patches and tried to reproduce), so I'm
going to have to leave it to Yuvaraj to answer.

As far as I know the dw_mmc driver ought to be enabling vqmmc when it
needs it.  Perhaps there's a bug in your patch series that adds vqmmc
support to dw_mmc?

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


[PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-18 Thread Javier Martinez Canillas
The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
(LDO) regulators. This patch adds support for all these regulators
found on the MAX77802 PMIC and is based on a driver added by Simon
Glass to the Chrome OS kernel 3.8 tree.

Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Tested-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
---

Changes since v7:
 - Remove DVS support since that can be added as a follow up.

Changes since v6: None

Changes since v5:
 - Take out the mfd changes from v4 that were squashed by mistake.
   Suggested by Lee Jones.

Changes since v4: None

Changes since v3:
 - Set the supply_name for regulators to lookup their parent supply node.
   Suggested by Mark Brown.
 - Change Exyno5 for Exynos5420/Exynos5800 in regulator driver Kconfig.
   Suggested by Doug Anderson.
---
 drivers/regulator/Kconfig|   9 +
 drivers/regulator/Makefile   |   1 +
 drivers/regulator/max77802.c | 578 +++
 3 files changed, 588 insertions(+)
 create mode 100644 drivers/regulator/max77802.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 2dc8289..8134a99 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -387,6 +387,15 @@ config REGULATOR_MAX77693
  and one current regulator 'CHARGER'. This is suitable for
  Exynos-4x12 chips.
 
+config REGULATOR_MAX77802
+   tristate Maxim 77802 regulator
+   depends on MFD_MAX77686
+   help
+ This driver controls a Maxim 77802 regulator
+ via I2C bus. The provided regulator is suitable for
+ Exynos5420/Exynos5800 SoCs to control various voltages.
+ It includes support for control of voltage and ramp speed.
+
 config REGULATOR_MC13XXX_CORE
tristate
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index aa4a6aa..b4ec6c8 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
 obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
 obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
 obj-$(CONFIG_REGULATOR_MAX77693) += max77693.o
+obj-$(CONFIG_REGULATOR_MAX77802) += max77802.o
 obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
new file mode 100644
index 000..5f022f8
--- /dev/null
+++ b/drivers/regulator/max77802.c
@@ -0,0 +1,578 @@
+/*
+ * max77802.c - Regulator driver for the Maxim 77802
+ *
+ * Copyright (C) 2013-2014 Google, Inc
+ * Simon Glass s...@chromium.org
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Chiwoong Byun woong.b...@smasung.com
+ * Jonghwa Lee jonghwa3@samsung.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; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver is based on max8997.c
+ */
+
+#include linux/kernel.h
+#include linux/bug.h
+#include linux/err.h
+#include linux/gpio.h
+#include linux/slab.h
+#include linux/gpio/consumer.h
+#include linux/platform_device.h
+#include linux/regulator/driver.h
+#include linux/regulator/machine.h
+#include linux/regulator/of_regulator.h
+#include linux/mfd/max77686.h
+#include linux/mfd/max77686-private.h
+
+/* Default ramp delay in case it is not manually set */
+#define MAX77802_RAMP_DELAY10  /* uV/us */
+
+#define MAX77802_OPMODE_SHIFT_LDO  6
+#define MAX77802_OPMODE_BUCK234_SHIFT  4
+#define MAX77802_OPMODE_MASK   0x3
+
+#define MAX77802_VSEL_MASK 0x3F
+#define MAX77802_DVS_VSEL_MASK 0xFF
+
+#define MAX77802_RAMP_RATE_MASK_2BIT   0xC0
+#define MAX77802_RAMP_RATE_SHIFT_2BIT  6
+#define MAX77802_RAMP_RATE_MASK_4BIT   0xF0
+#define MAX77802_RAMP_RATE_SHIFT_4BIT  4
+
+/* MAX77802 has two register formats: 2-bit and 4-bit */
+static const unsigned int ramp_table_77802_2bit[] = {
+   12500,
+   25000,
+   5,
+   10,
+};
+
+static unsigned int ramp_table_77802_4bit[] = {
+   1000,   2000,   3030,   4000,
+   5000,   5880,   7140,   8330,
+   9090,   1,  0,  12500,
+   16670,  25000,  5,  10,
+};
+
+struct max77802_regulator_prv {
+   int num_regulators;
+   struct regulator_dev *rdev[MAX77802_REG_MAX];
+   unsigned int opmode[MAX77802_REG_MAX];
+};
+
+static int max77802_get_opmode_shift(int id)
+{
+   if (id == MAX77802_BUCK1 || (id = MAX77802_BUCK5 
+  

Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators

2014-08-18 Thread Mark Brown
On Mon, Aug 18, 2014 at 10:32:41AM +0200, Javier Martinez Canillas wrote:
 The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
 (LDO) regulators. This patch adds support for all these regulators
 found on the MAX77802 PMIC and is based on a driver added by Simon
 Glass to the Chrome OS kernel 3.8 tree.

Applied, thanks.


signature.asc
Description: Digital signature