From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-16 08:30:11
This patch allows the Samsung ADC driver to enable VDD regulator at
probe and resume and to disable at exit and suspend.
In a platform where ADC's VDD regulator is not "always-on", this control
is required although this patch does not provide fine-grained power
control (turning on the regulator only when being accessed).
However, if VDD regulator ("vdd" for the adc device) is not provided,
the regulator control will not be activated because there are platforms
that do not provide regulator for ADC device.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
arch/arm/plat-samsung/adc.c | 20 +++++++++++++++++++-
1 files changed, 19 insertions(+), 1 deletions(-)
@@ -338,6 +342,12 @@ static int s3c_adc_probe(struct platform_device *pdev)adc->pdev=pdev;adc->prescale=S3C2410_ADCCON_PRSCVL(49);+adc->vdd=regulator_get(dev,S3C_ADC_REGULATOR_NAME);+if(IS_ERR_OR_NULL(adc->vdd)){+dev_dbg(dev,"operating without regulator %s.\n",S3C_ADC_REGULATOR_NAME);+adc->vdd=NULL;/* Do not control regulator */+}+adc->irq=platform_get_irq(pdev,1);if(adc->irq<=0){dev_err(dev,"failed to get adc irq\n");
@@ -372,6 +382,8 @@ static int s3c_adc_probe(struct platform_device *pdev)gotoerr_clk;}+if(adc->vdd)+regulator_enable(adc->vdd);clk_enable(adc->clk);tmp=adc->prescale|S3C2410_ADCCON_PRSCEN;
@@ -406,6 +418,8 @@ static int __devexit s3c_adc_remove(struct platform_device *pdev)iounmap(adc->regs);free_irq(adc->irq,adc);clk_disable(adc->clk);+if(adc->vdd)+regulator_disable(adc->vdd);clk_put(adc->clk);kfree(adc);
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-16 08:30:13
In S5PV210/S5PC110/Exynos4, ADCMUX channel selection uses ADCMUX
register, not ADCCON register. This patch corrects the behavior of
Samsung-ADC for such cpus.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
arch/arm/plat-samsung/adc.c | 24 +++++++++++++++++-------
1 files changed, 17 insertions(+), 7 deletions(-)
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-16 08:47:47
There has been no #ifndef - #define - #endif protection for this header
file. The patch adds it for Exynos4-ADC support
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
arch/arm/plat-samsung/include/plat/devs.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
I'm not convinced that the #define for the name is terribly good style
here, especially given that you actually call it vdd in the code.
+ if (IS_ERR_OR_NULL(adc->vdd)) {
+ dev_dbg(dev, "operating without regulator %s.\n", S3C_ADC_REGULATOR_NAME);
+ adc->vdd = NULL; /* Do not control regulator */
+ }
+
No, don't do this. Just unconditionally assume the regulator is present
if power is essential for use of the device. The regulator API will
stub out correctly if it's not in use to allow things to proceed and if
vdd is genuinely not hooked up then the driver can't function.
+ if (adc->vdd)
+ regulator_enable(adc->vdd);
You're not checking the return value here or anywhere else after the
inital get().
I'm not convinced that the #define for the name is terribly good style
here, especially given that you actually call it vdd in the code.
Then, would it be fine to use as [ regulator_get(dev, "vdd"); ] ?
quoted
+ ? ? if (IS_ERR_OR_NULL(adc->vdd)) {
+ ? ? ? ? ? ? dev_dbg(dev, "operating without regulator %s.\n", S3C_ADC_REGULATOR_NAME);
+ ? ? ? ? ? ? adc->vdd = NULL; /* Do not control regulator */
+ ? ? }
+
No, don't do this. ?Just unconditionally assume the regulator is present
if power is essential for use of the device. ?The regulator API will
stub out correctly if it's not in use to allow things to proceed and if
vdd is genuinely not hooked up then the driver can't function.
This ADC driver is for every ADC from S3C24xx series to Exynos4 (and
its successors as well).
The regulator (VDD for ADC) is essential for the recent chips
(S5PC110, S5PV210, and Exynos4).
I was just worried about the old boards using the same ADC driver
(mach-s3c2410/mach-*.c, mach-s3c6410/mach-*.c, and so on) without
ADC-VDD regulators defined.
However, no s3c compliance defconfigs have ever used CONFIG_REGULATOR.
Thus, it seems that it's safe to enforce using "vdd" with regulators
in plat-samsung's ADC driver.
I'll proceed as you have commented.
You're not checking the return value here or anywhere else after the
inital get().
Ok. I'll let it handle errors from regulator_enable.
Thank you!
- MyungJoo.
--
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
I'm not convinced that the #define for the name is terribly good style
here, especially given that you actually call it vdd in the code.
Then, would it be fine to use as [ regulator_get(dev, "vdd"); ] ?
Yes.
quoted
quoted
+ ? ? if (IS_ERR_OR_NULL(adc->vdd)) {
+ ? ? ? ? ? ? dev_dbg(dev, "operating without regulator %s.\n", S3C_ADC_REGULATOR_NAME);
+ ? ? ? ? ? ? adc->vdd = NULL; /* Do not control regulator */
+ ? ? }
+
quoted
No, don't do this. ?Just unconditionally assume the regulator is present
if power is essential for use of the device. ?The regulator API will
stub out correctly if it's not in use to allow things to proceed and if
vdd is genuinely not hooked up then the driver can't function.
This ADC driver is for every ADC from S3C24xx series to Exynos4 (and
its successors as well).
The regulator (VDD for ADC) is essential for the recent chips
(S5PC110, S5PV210, and Exynos4).
I was just worried about the old boards using the same ADC driver
(mach-s3c2410/mach-*.c, mach-s3c6410/mach-*.c, and so on) without
ADC-VDD regulators defined.
If the regulator API is in use on a system it is reasonable to expect it
to be set up correctly for the system.
However, no s3c compliance defconfigs have ever used CONFIG_REGULATOR.
Thus, it seems that it's safe to enforce using "vdd" with regulators
in plat-samsung's ADC driver.
I'll proceed as you have commented.
Note that SMDK6410 and Cragganmore are both using regulators fairly
extensively.
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-21 01:58:52
In S5PV210/S5PC110/Exynos4, ADCMUX channel selection uses ADCMUX
register, not ADCCON register. This patch corrects the behavior of
Samsung-ADC for such cpus.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
v2: No changes from v1. Resubmitted as a series of patches
---
arch/arm/plat-samsung/adc.c | 24 +++++++++++++++++-------
arch/arm/plat-samsung/include/plat/regs-adc.h | 1 +
2 files changed, 18 insertions(+), 7 deletions(-)
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-21 01:58:55
Patch 1/5: Add regulator support in ADC driver.
If CONFIG_REGULATOR is enabled, "vdd" regulator for the ADC driver
(e.g., "s5p-adc") should exist for the adc driver.
Patch 2/5: Channel selection method for S5PC110 and Exynos4
Recent Samsung SoCs have different register addresses for
channel selection. Use "s5p-adc" to support such chips.
Patch 3/5: Support ADC at Exynos4
Define register addresses and device name for Exynos4
Patch 4/5: Support ADC at S5PC110/S5PV210
Correct ADC device name for S5PC110/S5PV210
Patch 5/5: Header file correction (plat/devs.h)
The long-overdue bugfix for compiler errors. ADC for Exynos4 fails to
be compiled without this patch.
MyungJoo Ham (5):
Samsung SoC ADC: use regulator (VDD for ADC).
Samsung SoC ADC: Channel selection for S5PV210, S5PC110, and Exynos4
ARM: Exynos4: Support ADC
ARM: S5PC110/S5PV210: Support ADC
Samsung SoC: header file revised to prevent declaring duplicated.
arch/arm/mach-exynos4/Kconfig | 1 +
arch/arm/mach-exynos4/cpu.c | 4 ++
arch/arm/mach-exynos4/include/mach/irqs.h | 8 ++++
arch/arm/mach-exynos4/include/mach/map.h | 5 ++
arch/arm/mach-s5pv210/cpu.c | 2 +-
arch/arm/plat-samsung/adc.c | 55 +++++++++++++++++++-----
arch/arm/plat-samsung/include/plat/devs.h | 5 ++
arch/arm/plat-samsung/include/plat/regs-adc.h | 1 +
8 files changed, 68 insertions(+), 13 deletions(-)
--
1.7.4.1
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-21 01:58:59
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
v2: No changes from v1. Resubmitted as a series of patches
---
arch/arm/mach-s5pv210/cpu.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-21 01:59:05
There has been no #ifndef - #define - #endif protection for this header
file.
To compile EXYNOS4 with adc support without compiler errors, this patch is essential.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
v2: No changes from v1. Resubmitted as a series of patches
---
arch/arm/plat-samsung/include/plat/devs.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-21 01:59:53
This patch allows the Samsung ADC driver to enable VDD regulator at
probe and resume and to disable at exit and suspend.
In a platform where ADC's VDD regulator is not "always-on", this control
is required although this patch does not provide fine-grained power
control (turning on the regulator only when being accessed).
However, if VDD regulator ("vdd" for the adc device) is not provided,
the regulator control will not be activated because there are platforms
that do not provide regulator for ADC device.
arch_initcall has been modified to module_init in order to allow
regulators to be available at probe.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
changes from v1
- Removed macro defining the name of regulator.
- Handle error from regulator_enable.
- Do not allow not to have the regulator if CONFIG_REGULATOR.
- Seperate a patch dealing with "arch_initcall->module_init"
---
arch/arm/plat-samsung/adc.c | 31 ++++++++++++++++++++++++++-----
1 files changed, 26 insertions(+), 5 deletions(-)
@@ -338,17 +340,24 @@ static int s3c_adc_probe(struct platform_device *pdev)adc->pdev=pdev;adc->prescale=S3C2410_ADCCON_PRSCVL(49);+adc->vdd=regulator_get(dev,"vdd");+if(IS_ERR(adc->vdd)){+dev_err(dev,"operating without regulator \"vdd\" .\n");+ret=PTR_ERR(adc->vdd);+gotoerr_alloc;+}+adc->irq=platform_get_irq(pdev,1);if(adc->irq<=0){dev_err(dev,"failed to get adc irq\n");ret=-ENOENT;-gotoerr_alloc;+gotoerr_reg;}ret=request_irq(adc->irq,s3c_adc_irq,0,dev_name(dev),adc);if(ret<0){dev_err(dev,"failed to attach adc irq\n");-gotoerr_alloc;+gotoerr_reg;}adc->clk=clk_get(dev,"adc");
@@ -372,6 +381,10 @@ static int s3c_adc_probe(struct platform_device *pdev)gotoerr_clk;}+ret=regulator_enable(adc->vdd);+if(!ret)+gotoerr_ioremap;+clk_enable(adc->clk);tmp=adc->prescale|S3C2410_ADCCON_PRSCEN;
@@ -388,12 +401,15 @@ static int s3c_adc_probe(struct platform_device *pdev)return0;+err_ioremap:+iounmap(adc->regs);err_clk:clk_put(adc->clk);err_irq:free_irq(adc->irq,adc);-+err_reg:+regulator_put(adc->vdd);err_alloc:kfree(adc);returnret;
@@ -406,6 +422,8 @@ static int __devexit s3c_adc_remove(struct platform_device *pdev)iounmap(adc->regs);free_irq(adc->irq,adc);clk_disable(adc->clk);+regulator_disable(adc->vdd);+regulator_put(adc->vdd);clk_put(adc->clk);kfree(adc);
Seems better to return as soon as we notice the error, no point in
starting anything else up if we don't have power.
Ok. I see.
--
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
From: Kukjin Kim <hidden> Date: 2011-06-22 09:10:49
MyungJoo Ham wrote:
quoted hunk
There has been no #ifndef - #define - #endif protection for this header
file. The patch adds it for Exynos4-ADC support
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
arch/arm/plat-samsung/include/plat/devs.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
From: Kukjin Kim <hidden> Date: 2011-06-29 13:42:36
MyungJoo Ham wrote:
quoted hunk
In S5PV210/S5PC110/Exynos4, ADCMUX channel selection uses ADCMUX
register, not ADCCON register. This patch corrects the behavior of
Samsung-ADC for such cpus.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
v2: No changes from v1. Resubmitted as a series of patches
---
arch/arm/plat-samsung/adc.c | 24
+++++++++++++++++-------
arch/arm/plat-samsung/include/plat/regs-adc.h | 1 +
2 files changed, 18 insertions(+), 7 deletions(-)
How about S5PC100? Following is better for next if we cannot distinguish it
by CPU.
-enum s3c_cpu_type {
- TYPE_S3C24XX,
- TYPE_S3C64XX
+enum samsung_adc_type {
+ TYPE_ADC24, /* S3C24XX */
+ TYPE_ADC64, /* S3C64XX, S5P64X0, S5PC100 */
+ TYPE_ADCV3 /* S5PV210, EXYNOS4210 */
};
...
TYPE_ADCV1 and V2?...
Of course, the name can be changed...
As you know, we need to use external GIC instead of current internal on
EXYNOS4210.
So could you please re-make this based on that?
I will make some branch or will apply external GIC patches into for-next
soon.
(snip)
Thanks.
Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
From: MyungJoo Ham <myungjoo.ham@samsung.com> Date: 2011-06-30 07:50:56
On Wed, Jun 29, 2011 at 10:42 PM, Kukjin Kim [off-list ref] wrote:
MyungJoo Ham wrote:
quoted
In S5PV210/S5PC110/Exynos4, ADCMUX channel selection uses ADCMUX
register, not ADCCON register. This patch corrects the behavior of
Samsung-ADC for such cpus.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
v2: No changes from v1. Resubmitted as a series of patches
---
How about S5PC100? Following is better for next if we cannot distinguish it
by CPU.
-enum s3c_cpu_type {
- ? ? ? TYPE_S3C24XX,
- ? ? ? TYPE_S3C64XX
+enum samsung_adc_type {
+ ? ? ? TYPE_ADC24, ? ? ?/* S3C24XX */
+ ? ? ? TYPE_ADC64, ? ? ?/* S3C64XX, S5P64X0, S5PC100 */
+ ? ? ? TYPE_ADCV3 ? ? ?/* S5PV210, EXYNOS4210 */
?};
...
TYPE_ADCV1 and V2?...
Of course, the name can be changed...
I don't mind about these names as they are just internal values of adc.c.
Anyway, I prefer the last suggestion and I'll use TYPE_ADCV1/V2/V3.
Thanks.
--
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858