Re: [PATCH v3 0/2] regulator: Add initial suport for max77686

2012-05-22 Thread Kyungmin Park
Hi Mark,

BTW, do you know that you're reviewing the same device driver patch
from different person?
One from Mr. Lee and another from Yadwinder.

I wonder how to handle it finally. which one is choose?

Thank you,
Kyungmin Park

On 5/22/12, yadi.bra...@gmail.com yadi.bra...@gmail.com wrote:
 From: Yadwinder Singh Brar yadi.b...@samsung.com

 This patch series adds support for max77686 which is a multifunction device
 which includes regulator (pmic), rtc and charger sub-blocks within it. The
 support for mfd driver and regulator driver are added by this patch series.
 This
 patch series also includes device tree and irqdomain support for mfd and
 regulator portions.

 Implemented the required modification, stated in the recieved review
 comments.
 changes since v1:
 -added regmap support.
 -implemented .get_voltage_sel, .set_voltage_sel and .set_voltage_time_sel
 after
  removing .get_voltage and .set_voltage in regulator driver.
 -used of_regulator_match() for parsing DT.
 -added Documentation for Devive Tree binding.

 changes since v2:
 -converted to use regulator_get_voltage_sel_regmap,
  regulator_set_voltage_sel_regmap, regulator_enable_regmap,
  regulator_disable_regmap, regulator_is_enabled_regmap.

 This patch series is based on mark_regulator/for-next and has been tested
 on
 GAIA board.

 Yadwinder Singh Brar (2):
   mfd: Add support for MAX77686.
   regulator: Add support for MAX77686.

  Documentation/devicetree/bindings/mfd/max77686.txt |   61 +++
  drivers/mfd/Kconfig|   21 +
  drivers/mfd/Makefile   |1 +
  drivers/mfd/max77686-irq.c |  255 +
  drivers/mfd/max77686.c |  322 
  drivers/regulator/Kconfig  |9 +
  drivers/regulator/Makefile |1 +
  drivers/regulator/max77686.c   |  389
 
  include/linux/mfd/max77686-private.h   |  282 ++
  include/linux/mfd/max77686.h   |  100 +
  10 files changed, 1441 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/mfd/max77686.txt
  create mode 100644 drivers/mfd/max77686-irq.c
  create mode 100644 drivers/mfd/max77686.c
  create mode 100644 drivers/regulator/max77686.c
  create mode 100644 include/linux/mfd/max77686-private.h
  create mode 100644 include/linux/mfd/max77686.h

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

--
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 1/2] ARM: EXYNOS: Update HSOTG PHY clock setting for Exynos4x12

2012-05-22 Thread Sachin Kamat
Adds clock setting entries for Exynos4212 and Exynos4412 platforms.

Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
Based on Kukjin Kim's v3.5-for-usb-2 branch.
---
 arch/arm/mach-exynos/include/mach/regs-usb-phy.h |5 ++
 arch/arm/mach-exynos/setup-usb-phy.c |   47 -
 arch/arm/plat-samsung/include/plat/cpu.h |4 ++
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-exynos/include/mach/regs-usb-phy.h 
b/arch/arm/mach-exynos/include/mach/regs-usb-phy.h
index c337cf3..bab28d9 100644
--- a/arch/arm/mach-exynos/include/mach/regs-usb-phy.h
+++ b/arch/arm/mach-exynos/include/mach/regs-usb-phy.h
@@ -38,8 +38,13 @@
 #define CLKSEL_MASK(0x3  0)
 #define CLKSEL_SHIFT   (0)
 #define CLKSEL_48M (0x0  0)
+#define CLKSEL_9600K   (0x0  0)
+#define CLKSEL_10M (0x1  0)
 #define CLKSEL_12M (0x2  0)
+#define CLKSEL_19200K  (0x3  0)
 #define CLKSEL_24M (0x3  0)
+#define CLKSEL_20M (0x4  0)
+#define EXYNOS4X12_CLKSEL_24M  (0x5  0)
 
 #define EXYNOS4_RSTCON EXYNOS4_HSOTG_PHYREG(0x08)
 #define HOST_LINK_PORT_SWRST_MASK  (0xf  6)
diff --git a/arch/arm/mach-exynos/setup-usb-phy.c 
b/arch/arm/mach-exynos/setup-usb-phy.c
index 1af0a7f..b8cf0f8 100644
--- a/arch/arm/mach-exynos/setup-usb-phy.c
+++ b/arch/arm/mach-exynos/setup-usb-phy.c
@@ -36,17 +36,42 @@ static void exynos4210_usb_phy_clkset(struct 
platform_device *pdev)
 
xusbxti_clk = clk_get(pdev-dev, xusbxti);
if (xusbxti_clk  !IS_ERR(xusbxti_clk)) {
-   switch (clk_get_rate(xusbxti_clk)) {
-   case 12 * MHZ:
-   phyclk |= CLKSEL_12M;
-   break;
-   case 24 * MHZ:
-   phyclk |= CLKSEL_24M;
-   break;
-   default:
-   case 48 * MHZ:
-   /* default reference clock */
-   break;
+   if (soc_is_exynos4210()) {
+   switch (clk_get_rate(xusbxti_clk)) {
+   case 12 * MHZ:
+   phyclk |= CLKSEL_12M;
+   break;
+   case 24 * MHZ:
+   phyclk |= CLKSEL_24M;
+   break;
+   default:
+   case 48 * MHZ:
+   /* default reference clock */
+   break;
+   }
+   } else if (soc_is_exynos4212() || soc_is_exynos4412()) {
+   switch (clk_get_rate(xusbxti_clk)) {
+   case 9600 * KHZ:
+   phyclk |= CLKSEL_9600K;
+   break;
+   case 10 * MHZ:
+   phyclk |= CLKSEL_10M;
+   break;
+   case 12 * MHZ:
+   phyclk |= CLKSEL_12M;
+   break;
+   case 19200 * KHZ:
+   phyclk |= CLKSEL_19200K;
+   break;
+   case 20 * MHZ:
+   phyclk |= CLKSEL_20M;
+   break;
+   default:
+   case 24 * MHZ:
+   /* default reference clock */
+   phyclk |= EXYNOS4X12_CLKSEL_24M;
+   break;
+   }
}
clk_put(xusbxti_clk);
}
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h 
b/arch/arm/plat-samsung/include/plat/cpu.h
index 787ceac..6a6ff7e 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -132,6 +132,10 @@ IS_SAMSUNG_CPU(exynos5250, EXYNOS5250_SOC_ID, 
EXYNOS5_SOC_MASK)
 
 #define IODESC_ENT(x) { (unsigned long)S3C24XX_VA_##x, 
__phys_to_pfn(S3C24XX_PA_##x), S3C24XX_SZ_##x, MT_DEVICE }
 
+#ifndef KHZ
+#define KHZ (1000)
+#endif
+
 #ifndef MHZ
 #define MHZ (1000*1000)
 #endif
-- 
1.7.4.1

--
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 2/2] ARM: EXYNOS: Add HSOTG support to SMDK4X12

2012-05-22 Thread Sachin Kamat
S3C-HSOTG support is added to SMDK4212 and SMDK4412 boards.

Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
Based on Kukjin Kim's v3.5-for-usb-2 branch.
---
 arch/arm/mach-exynos/Kconfig |2 ++
 arch/arm/mach-exynos/mach-smdk4x12.c |8 
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 6682f35..95e3ee3 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -349,6 +349,7 @@ config MACH_SMDK4212
select S3C_DEV_I2C3
select S3C_DEV_I2C7
select S3C_DEV_RTC
+   select S3C_DEV_USB_HSOTG
select S3C_DEV_WDT
select SAMSUNG_DEV_BACKLIGHT
select SAMSUNG_DEV_KEYPAD
@@ -359,6 +360,7 @@ config MACH_SMDK4212
select EXYNOS4_SETUP_I2C7
select EXYNOS4_SETUP_KEYPAD
select EXYNOS4_SETUP_SDHCI
+   select EXYNOS4_SETUP_USB_PHY
help
  Machine support for Samsung SMDK4212
 
diff --git a/arch/arm/mach-exynos/mach-smdk4x12.c 
b/arch/arm/mach-exynos/mach-smdk4x12.c
index d00e4f0..6d8f325 100644
--- a/arch/arm/mach-exynos/mach-smdk4x12.c
+++ b/arch/arm/mach-exynos/mach-smdk4x12.c
@@ -19,6 +19,7 @@
 #include linux/pwm_backlight.h
 #include linux/regulator/machine.h
 #include linux/serial_core.h
+#include linux/platform_data/s3c-hsotg.h
 
 #include asm/mach/arch.h
 #include asm/hardware/gic.h
@@ -235,6 +236,9 @@ static struct samsung_keypad_platdata smdk4x12_keypad_data 
__initdata = {
.cols   = 5,
 };
 
+/* USB OTG */
+static struct s3c_hsotg_plat smdk4x12_hsotg_pdata;
+
 static struct platform_device *smdk4x12_devices[] __initdata = {
s3c_device_hsmmc2,
s3c_device_hsmmc3,
@@ -243,6 +247,7 @@ static struct platform_device *smdk4x12_devices[] 
__initdata = {
s3c_device_i2c3,
s3c_device_i2c7,
s3c_device_rtc,
+   s3c_device_usb_hsotg,
s3c_device_wdt,
samsung_device_keypad,
 };
@@ -281,6 +286,9 @@ static void __init smdk4x12_machine_init(void)
s3c_sdhci2_set_platdata(smdk4x12_hsmmc2_pdata);
s3c_sdhci3_set_platdata(smdk4x12_hsmmc3_pdata);
 
+   s3c_hsotg_set_platdata(smdk4x12_hsotg_pdata);
+   clk_xusbxti.rate = 2400;
+
platform_add_devices(smdk4x12_devices, ARRAY_SIZE(smdk4x12_devices));
 }
 
-- 
1.7.4.1

--
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 0/2] regulator: Add initial suport for max77686

2012-05-22 Thread Mark Brown
On Tue, May 22, 2012 at 03:53:11PM +0900, Kyungmin Park wrote:

 BTW, do you know that you're reviewing the same device driver patch
 from different person?
 One from Mr. Lee and another from Yadwinder.

 I wonder how to handle it finally. which one is choose?

Yes, I realise there's two different versions of it floating around -
personally I'd hope that you guys could come to some agreement about a
common set of code that works for everyone.


signature.asc
Description: Digital signature


Re: [PATCH v3 1/2] mfd: Add support for MAX77686.

2012-05-22 Thread Yadwinder Singh Brar
CC' ing to maintainer.
(I forget to add maintainer in cc, sorry for noise).

On Tue, May 22, 2012 at 11:26 AM,  yadi.bra...@gmail.com wrote:
 From: Yadwinder Singh Brar yadi.b...@samsung.com

 MAX77686 is a mulitifunction device with PMIC, RTC and Charger on chip. Th
 driver provides common support for accessing the device. This is initial
 version of this driver that supports to enable the chip with its primary I
 bus.It also includes IRQ and device tree support for MAX77686 chip.

 Signed-off-by: Yadwinder Singh Brar yadi.b...@samsung.com
 Reviewed-by: Mark Brown broo...@opensource.wolfsonmicro.com
 ---
  Documentation/devicetree/bindings/mfd/max77686.txt |   61 
  drivers/mfd/Kconfig                                |   21 ++
  drivers/mfd/Makefile                               |    1 +
  drivers/mfd/max77686-irq.c                         |  255 
  drivers/mfd/max77686.c                             |  322 
 
  include/linux/mfd/max77686-private.h               |  282 +
  include/linux/mfd/max77686.h                       |  100 ++
  7 files changed, 1042 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/mfd/max77686.txt
  create mode 100644 drivers/mfd/max77686-irq.c
  create mode 100644 drivers/mfd/max77686.c
  create mode 100644 include/linux/mfd/max77686-private.h
  create mode 100644 include/linux/mfd/max77686.h

 diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt 
 b/Documentation/devicetree/bindings/mfd/max77686.txt
 new file mode 100644
 index 000..78a47bd
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
 @@ -0,0 +1,61 @@
 +Maxim MAX77686 multi-function device
 +
 +MAX77686 is a Mulitifunction device with PMIC, RTC and Charger on chip. It is
 +interfaced to host controller using i2c interface. PMIC and Charger 
 submodules
 +are addressed using same i2c slave address where as RTC submodule uses
 +different slave address,presently for which we are statically creating i2c
 +client while probing.This document describes the binding for mfd device and
 +PMIC submodule.
 +
 +Required properties:
 +- compatible : Must be maxim,max77686;
 +- reg : Specifiec the i2c slave address of PMIC block.
 +- interrupts : This i2c device has an IRQ line connected to the main SoC.
 +- interrupt-parent : The parent interrupt controller.
 +
 +Optional node:
 +- max77686,buck_ramp_delay : Ramp delay to be setup for buck2,34.
 +
 +- voltage-regulators : The regulators of max77686 have to be instantiated
 +  under subnode named voltage-regulators uing the following format.
 +
 +       regulator_name {
 +               standard regulator constraints
 +       };
 +       refer Documentation/devicetree/bindings/regulator/regulator.txt
 +
 +  The names of regulator should be as follow:
 +
 +       -LDOn   :       for LDOs, where n can lie in range 1 to 26.
 +                       example: LDO1, LDO2, LDO26.
 +       -BUCKn  :       for BUCKs, where n can lie in range 1 to 9.
 +                       example: BUCK1, BUCK5, BUCK9.
 +
 +
 +Example:
 +
 +       max77686_pmic@09 {
 +               compatible = maxim,max77686;
 +               interrupt-parent = wakeup_eint;
 +               interrupts = 26 0;
 +               reg = 0x09;
 +
 +               max77686,buck_ramp_delay = 2;         /* default */
 +
 +               voltage-regulators {
 +                       ldo11_reg: LDO11 {
 +                               regulator-name = vdd_ldo11;
 +                               regulator-min-microvolt = 190;
 +                               regulator-max-microvolt = 190;
 +                               regulator-always-on;
 +                       };
 +
 +                       buck1_reg: BUCK1 {
 +                               regulator-name = vdd_mif;
 +                               regulator-min-microvolt = 95;
 +                               regulator-max-microvolt = 130;
 +                               regulator-always-on;
 +                               regulator-boot-on;
 +                       };
 +       }
 +
 diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
 index a4fd173..d8285e5 100644
 --- a/drivers/mfd/Kconfig
 +++ b/drivers/mfd/Kconfig
 @@ -442,6 +442,27 @@ config MFD_MAX8998
          additional drivers must be enabled in order to use the functionality
          of the device.

 +config MFD_MAX77686
 +       bool Maxim Semiconductor MAX77686 PMIC Support
 +       depends on I2C=y  GENERIC_HARDIRQS
 +       select MFD_CORE
 +       select REGMAP_I2C
 +       help
 +         Say yes here to support for Maxim Semiconductor MAX77686.
 +         This is a Power Management IC with RTC on chip.
 +         This driver provides common support for accessing the device;
 +         additional drivers must be enabled in order to use the functionality
 +         of the device.
 +
 +config DEBUG_MAX77686
 +       bool MAX77686 PMIC debugging
 +       depends 

Re: [PATCH v3 2/2] regulator: Add support for MAX77686.

2012-05-22 Thread jonghwa3 . lee
Hi, Yadwinder,
As you know, both of us, recently, had competition for one driver
whether you intend or not. And now, i think it is time to stop this and
to find appropriate goal. From now on, i won't update this driver no
more. I recommend you to review my patch and apply feature that you can
apply. And also check comments that i wrote below.

On 2012년 05월 22일 14:57, yadi.bra...@gmail.com wrote:

 From: Yadwinder Singh Brar yadi.b...@samsung.com
 
 Add support for PMIC/regulator portion of MAX77686 multifunction device.
 MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driv
 which supports setting and getting the voltage of a regulator with I2C
 interface.
 
 Signed-off-by: Yadwinder Singh Brar yadi.b...@samsung.com
 ---
  drivers/regulator/Kconfig|9 +
  drivers/regulator/Makefile   |1 +
  drivers/regulator/max77686.c |  387 
 ++
  3 files changed, 397 insertions(+), 0 deletions(-)
  create mode 100644 drivers/regulator/max77686.c
 
 diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
 index c86b886..e8f9417 100644
 --- a/drivers/regulator/Kconfig
 +++ b/drivers/regulator/Kconfig
 @@ -195,6 +195,15 @@ config REGULATOR_MAX8998
 via I2C bus. The provided regulator is suitable for S3C6410
 and S5PC1XX chips to control VCC_CORE and VCC_USIM voltages.
  
 +config REGULATOR_MAX77686
 + tristate Maxim 77686 regulator
 + depends on MFD_MAX77686
 + help
 +   This driver controls a Maxim 77686 voltage regulator via I2C
 +   bus. The provided regulator is suitable for Exynos5 chips to
 +   control VDD_ARM and VDD_INT voltages.It supports LDOs[1-26]
 +   and BUCKs[1-9].
 +
  config REGULATOR_PCAP
   tristate Motorola PCAP2 regulator driver
   depends on EZX_PCAP
 diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
 index 977fd46..d854453 100644
 --- a/drivers/regulator/Makefile
 +++ b/drivers/regulator/Makefile
 @@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_MAX8925) += max8925-regulator.o
  obj-$(CONFIG_REGULATOR_MAX8952) += max8952.o
  obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
  obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
 +obj-$(CONFIG_REGULATOR_MAX77686) += max77686.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/max77686.c b/drivers/regulator/max77686.c
 new file mode 100644
 index 000..98dbd50
 --- /dev/null
 +++ b/drivers/regulator/max77686.c
 @@ -0,0 +1,387 @@
 +/*
 + * max77686.c - Regulator driver for the Maxim 77686
 + *
 + * Copyright (C) 2012 Samsung Electronics Co. Ltd.
 + * Chiwoong Byun woong.b...@samsung.com
 + * Yadwinder Singh Brar yadi.b...@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.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 + *
 + * This driver is based on max8997.c
 + */
 +
 +#include linux/module.h
 +#include linux/bug.h
 +#include linux/delay.h
 +#include linux/err.h
 +#include linux/gpio.h
 +#include linux/slab.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
 +
 +struct max77686_data {
 + struct device *dev;
 + struct max77686_dev *iodev;
 + int num_regulators;
 + struct regulator_dev **rdev;
 + int ramp_delay; /* index of ramp_delay */
 +
 + /*GPIO-DVS feature is not enabled with the
 +  *current version of MAX77686 driver.*/
 +};
 +
 +static int max77686_voltage_dvs_buck_time_sel(struct regulator_dev *rdev,
 + unsigned int old_sel,
 + unsigned int new_sel)
 +{
 + struct max77686_data *max77686 = rdev_get_drvdata(rdev);
 + int ramp[] = {13, 27, 55, 100}; /* ramp_rate in mV/uS */
 +
 + return DIV_ROUND_UP(rdev-desc-uV_step *
 + abs(new_sel - old_sel),
 + ramp[max77686-ramp_delay]);
 +}
 +
 +static int max77686_voltage_time_sel(struct regulator_dev *rdev,
 + unsigned int old_sel,
 + unsigned int new_sel)
 

Re: [PATCH v3 2/2] regulator: Add support for MAX77686.

2012-05-22 Thread jonghwa3 . lee
Hi, again.
On 2012년 05월 22일 14:57, yadi.bra...@gmail.com wrote:


 +static __devinit int max77686_pmic_probe(struct platform_device *pdev)
 +{

 +
 + for (i = 0; i  pdata-num_regulators; i++) {
 + config.dev = max77686-dev;
 + config.init_data = pdata-regulators[i].init_data;
 + config.driver_data = max77686;
 + config.regmap = iodev-regmap;
 +
 + rdev[i] = regulator_register(regulators[i], config);


I'm sorry that i missed one. You have to register all regulators
unconditionally. Mark brown commented about this to my former patch.

'No, you should unconditionally register all regulators the device
physically has.  This is useful for debug and simplifies the code.'
- from Mark Brown



Thanks.
--
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 support for MAX77686.

2012-05-22 Thread Yadwinder Singh Brar
(adding Kyungmin Park and Samuel Ortiz)

Hi,

Yes, It happened unintentionally. I didn't know about your patch
before submitting
the initial version of my patches. I agree with you, I will review
your patches and
will try to incorporate extra features from your patches.

On Wed, May 23, 2012 at 7:10 AM,  jonghwa3@samsung.com wrote:
 Hi, Yadwinder,
 As you know, both of us, recently, had competition for one driver
 whether you intend or not. And now, i think it is time to stop this and
 to find appropriate goal. From now on, i won't update this driver no
 more. I recommend you to review my patch and apply feature that you can
 apply. And also check comments that i wrote below.

 On 2012년 05월 22일 14:57, yadi.bra...@gmail.com wrote:

 From: Yadwinder Singh Brar yadi.b...@samsung.com

 Add support for PMIC/regulator portion of MAX77686 multifunction device.
 MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driv
 which supports setting and getting the voltage of a regulator with I2C
 interface.

 +     return DIV_ROUND_UP(rdev-desc-uV_step *
 +                         abs(new_sel - old_sel),
 +                         100);
 +}
 +


 Does LDO also need waiting for voltage change? I afraid it's not.


Yes, according to technical reference manual which I have, ramp rate for
LDOs is also 100mV/us.

 +
 +     if (pdata-ramp_delay  MAX77686_RAMP_RATE_13MV ||
 +             pdata-ramp_delay  MAX77686_RAMP_RATE_100MV)
 +             pdata-ramp_delay = MAX77686_RAMP_RATE_27MV;    /* default */

If pdata doesn't have proper ramp_delay, it will get value
MAX77686_RAMP_RATE_27MV.

 +
 +     max77686-ramp_delay = pdata-ramp_delay - 1;


 I think it is better to check pdata-ramp_delay is available.
 If pdata doesn't have ramp_delay member it might be error.


Yes, we have taken care of this case above before setting value of
max77686-ramp_delay.

 +     max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
 +             max77686-ramp_delay  6, RAMP_MASK);
 +     max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
 +             max77686-ramp_delay  6, RAMP_MASK);
 +     max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
 +             max77686-ramp_delay  6, RAMP_MASK);
 +


 Why do you use i2c client still? If you registered regmap you can use
 its API. I recommend you to use regmap_update_bits() directly.



Yes, we are using regmap_update_bits().  max77686_update_reg() is just
a wrapper over it.

 +     platform_set_drvdata(pdev, max77686);
 +


 MAX77686 has crystal oscillator in it. And original version of this
 driver which was written by Chiwoon Byun, registers it as a regulator.
 As Mark says, we have to change it to use generic clock API. Where do
 you think should we put them into? In my opinion, it is proper that just
 leave them in regulator driver because this driver is almost core of
 PMIC. I already applied generic API in my local repository but i
 couldn't test yet. Because it crashed with SOC's private clock API.
 Anyway if you register 32khz clock with generic API ,DEFINE_CLK_GATE()
 will help you out which defined in linux/clk-private.h.


Yes, I was also thinking about where to put it. I am not sure whether this
is a proper place to put them. Anyway I will again think about it.

Thanks,
Yadwinder.
--
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 support for MAX77686.

2012-05-22 Thread Yadwinder Singh Brar
Hi,

On Wed, May 23, 2012 at 7:20 AM,  jonghwa3@samsung.com wrote:
 Hi, again.
 On 2012년 05월 22일 14:57, yadi.bra...@gmail.com wrote:


 +static __devinit int max77686_pmic_probe(struct platform_device *pdev)
 +{

 +
 +     for (i = 0; i  pdata-num_regulators; i++) {
 +             config.dev = max77686-dev;
 +             config.init_data = pdata-regulators[i].init_data;
 +             config.driver_data = max77686;
 +             config.regmap = iodev-regmap;
 +
 +             rdev[i] = regulator_register(regulators[i], config);


 I'm sorry that i missed one. You have to register all regulators
 unconditionally. Mark brown commented about this to my former patch.

 'No, you should unconditionally register all regulators the device
 physically has.  This is useful for debug and simplifies the code.'
                                                - from Mark Brown


Yes, we are registering all regulators here.
As pdata-num_regulators will be equal to ARRAY_SIZE(regulators)

Thanks,
Yadwinder.
--
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 support for MAX77686.

2012-05-22 Thread jonghwa3 . lee
On 2012년 05월 23일 13:16, Yadwinder Singh Brar wrote:

 + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
 + max77686-ramp_delay  6, RAMP_MASK);
 + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
 + max77686-ramp_delay  6, RAMP_MASK);
 + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
 + max77686-ramp_delay  6, RAMP_MASK);
 +


 Why do you use i2c client still? If you registered regmap you can use
 its API. I recommend you to use regmap_update_bits() directly.


 
 Yes, we are using regmap_update_bits().  max77686_update_reg() is just
 a wrapper over it.
 


Yes, i know what you mean. However it doesn't need max77686_update_reg()
any more since it uses regmap API. Why don't you just pass iodev-regmap
to regmap_update_bits(). It is clear that there is no reason for using
i2c client as a medium. Please check regulator and mfd driver of my
previous patch.


Thanks.
--
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 support for MAX77686.

2012-05-22 Thread Yadwinder Singh Brar
On Wed, May 23, 2012 at 10:10 AM,  jonghwa3@samsung.com wrote:
 On 2012년 05월 23일 13:16, Yadwinder Singh Brar wrote:

 +     max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
 +             max77686-ramp_delay  6, RAMP_MASK);
 +     max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
 +             max77686-ramp_delay  6, RAMP_MASK);
 +     max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
 +             max77686-ramp_delay  6, RAMP_MASK);
 +


 Why do you use i2c client still? If you registered regmap you can use
 its API. I recommend you to use regmap_update_bits() directly.



 Yes, we are using regmap_update_bits().  max77686_update_reg() is just
 a wrapper over it.



 Yes, i know what you mean. However it doesn't need max77686_update_reg()
 any more since it uses regmap API. Why don't you just pass iodev-regmap
 to regmap_update_bits(). It is clear that there is no reason for using
 i2c client as a medium. Please check regulator and mfd driver of my
 previous patch.


I agree with you we can use directly  regmap API. But I preferred
max77686_update_reg() because its a common practice to use
common read/write API which we define in mfd driver to access
that particular mfd device from other drivers.

Regards,
Yadwinder.
--
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 support for MAX77686.

2012-05-22 Thread jonghwa3 . lee
On 2012년 05월 23일 14:23, Yadwinder Singh Brar wrote:

 On Wed, May 23, 2012 at 10:10 AM,  jonghwa3@samsung.com wrote:
 On 2012년 05월 23일 13:16, Yadwinder Singh Brar wrote:

 + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
 + max77686-ramp_delay  6, RAMP_MASK);
 + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
 + max77686-ramp_delay  6, RAMP_MASK);
 + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
 + max77686-ramp_delay  6, RAMP_MASK);
 +


 Why do you use i2c client still? If you registered regmap you can use
 its API. I recommend you to use regmap_update_bits() directly.



 Yes, we are using regmap_update_bits().  max77686_update_reg() is just
 a wrapper over it.



 Yes, i know what you mean. However it doesn't need max77686_update_reg()
 any more since it uses regmap API. Why don't you just pass iodev-regmap
 to regmap_update_bits(). It is clear that there is no reason for using
 i2c client as a medium. Please check regulator and mfd driver of my
 previous patch.

 
 I agree with you we can use directly  regmap API. But I preferred
 max77686_update_reg() because its a common practice to use
 common read/write API which we define in mfd driver to access
 that particular mfd device from other drivers.
 
 Regards,
 Yadwinder.
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 


I inform you my mfd driver has been confirmed by Samuel Oritz and there
is no mfd private API. This situation looks unusual that we registers
mfd driver and regulator driver separately. But how should we do? For
corporation , i'm asking you to consider my suggestion.

Thanks.


Thanks.
--
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