2012-06-12 02:21:01

by Axel Lin

[permalink] [raw]
Subject: [PATCH RFT 1/4] regulator: wm831x-dcdc: Convert wm831x_buckp_ops to regulator_list_voltage_linear

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/wm831x-dcdc.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index 099da11..fbcff56 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -599,15 +599,6 @@ static struct platform_driver wm831x_buckv_driver = {
* BUCKP specifics
*/

-static int wm831x_buckp_list_voltage(struct regulator_dev *rdev,
- unsigned selector)
-{
- if (selector <= WM831X_BUCKP_MAX_SELECTOR)
- return 850000 + (selector * 25000);
- else
- return -EINVAL;
-}
-
static int wm831x_buckp_set_voltage_int(struct regulator_dev *rdev, int reg,
int min_uV, int max_uV, int *selector)
{
@@ -620,7 +611,7 @@ static int wm831x_buckp_set_voltage_int(struct regulator_dev *rdev, int reg,
else
return -EINVAL;

- if (wm831x_buckp_list_voltage(rdev, vsel) > max_uV)
+ if (regulator_list_voltage_linear(rdev, vsel) > max_uV)
return -EINVAL;

*selector = vsel;
@@ -652,7 +643,7 @@ static int wm831x_buckp_set_suspend_voltage(struct regulator_dev *rdev,
static struct regulator_ops wm831x_buckp_ops = {
.set_voltage = wm831x_buckp_set_voltage,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
- .list_voltage = wm831x_buckp_list_voltage,
+ .list_voltage = regulator_list_voltage_linear,
.set_suspend_voltage = wm831x_buckp_set_suspend_voltage,

.is_enabled = regulator_is_enabled_regmap,
@@ -715,6 +706,8 @@ static __devinit int wm831x_buckp_probe(struct platform_device *pdev)
dcdc->desc.vsel_mask = WM831X_DC3_ON_VSEL_MASK;
dcdc->desc.enable_reg = WM831X_DCDC_ENABLE;
dcdc->desc.enable_mask = 1 << id;
+ dcdc->desc.min_uV = 850000;
+ dcdc->desc.uV_step = 25000;

config.dev = pdev->dev.parent;
if (pdata)
--
1.7.9.5



2012-06-12 02:21:53

by Axel Lin

[permalink] [raw]
Subject: [PATCH RFT 2/4] regulator: wm831x-dcdc: Convert wm831x_buckp_ops to regulator_set_voltage_sel_regmap

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/wm831x-dcdc.c | 44 ++++++++-------------------------------
1 file changed, 9 insertions(+), 35 deletions(-)

diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index fbcff56..91b5b0a 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -599,51 +599,25 @@ static struct platform_driver wm831x_buckv_driver = {
* BUCKP specifics
*/

-static int wm831x_buckp_set_voltage_int(struct regulator_dev *rdev, int reg,
- int min_uV, int max_uV, int *selector)
+static int wm831x_buckp_set_suspend_voltage(struct regulator_dev *rdev, int uV)
{
struct wm831x_dcdc *dcdc = rdev_get_drvdata(rdev);
struct wm831x *wm831x = dcdc->wm831x;
- u16 vsel;
-
- if (min_uV <= 34000000)
- vsel = (min_uV - 850000) / 25000;
- else
- return -EINVAL;
-
- if (regulator_list_voltage_linear(rdev, vsel) > max_uV)
- return -EINVAL;
-
- *selector = vsel;
-
- return wm831x_set_bits(wm831x, reg, WM831X_DC3_ON_VSEL_MASK, vsel);
-}
-
-static int wm831x_buckp_set_voltage(struct regulator_dev *rdev,
- int min_uV, int max_uV,
- unsigned *selector)
-{
- struct wm831x_dcdc *dcdc = rdev_get_drvdata(rdev);
- u16 reg = dcdc->base + WM831X_DCDC_ON_CONFIG;
-
- return wm831x_buckp_set_voltage_int(rdev, reg, min_uV, max_uV,
- selector);
-}
-
-static int wm831x_buckp_set_suspend_voltage(struct regulator_dev *rdev,
- int uV)
-{
- struct wm831x_dcdc *dcdc = rdev_get_drvdata(rdev);
u16 reg = dcdc->base + WM831X_DCDC_SLEEP_CONTROL;
- unsigned selector;
+ int sel;
+
+ sel = regulator_map_voltage_linear(rdev, uV, uV);
+ if (sel < 0)
+ return sel;

- return wm831x_buckp_set_voltage_int(rdev, reg, uV, uV, &selector);
+ return wm831x_set_bits(wm831x, reg, WM831X_DC3_ON_VSEL_MASK, sel);
}

static struct regulator_ops wm831x_buckp_ops = {
- .set_voltage = wm831x_buckp_set_voltage,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
.list_voltage = regulator_list_voltage_linear,
+ .map_voltage = regulator_map_voltage_linear,
.set_suspend_voltage = wm831x_buckp_set_suspend_voltage,

.is_enabled = regulator_is_enabled_regmap,
--
1.7.9.5


2012-06-12 02:23:16

by Axel Lin

[permalink] [raw]
Subject: [PATCH RFT 3/4] regulator: wm831x-dcdc: Convert wm831x_buckv_ops to set_voltage_sel and map_voltage

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/wm831x-dcdc.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index 91b5b0a..eea53e1 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -215,8 +215,8 @@ static int wm831x_buckv_list_voltage(struct regulator_dev *rdev,
return -EINVAL;
}

-static int wm831x_buckv_select_min_voltage(struct regulator_dev *rdev,
- int min_uV, int max_uV)
+static int wm831x_buckv_map_voltage(struct regulator_dev *rdev,
+ int min_uV, int max_uV)
{
u16 vsel;

@@ -251,33 +251,28 @@ static int wm831x_buckv_set_dvs(struct regulator_dev *rdev, int state)
return 0;
}

-static int wm831x_buckv_set_voltage(struct regulator_dev *rdev,
- int min_uV, int max_uV, unsigned *selector)
+static int wm831x_buckv_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned selector)
{
struct wm831x_dcdc *dcdc = rdev_get_drvdata(rdev);
struct wm831x *wm831x = dcdc->wm831x;
int on_reg = dcdc->base + WM831X_DCDC_ON_CONFIG;
int dvs_reg = dcdc->base + WM831X_DCDC_DVS_CONTROL;
- int vsel, ret;
-
- vsel = wm831x_buckv_select_min_voltage(rdev, min_uV, max_uV);
- if (vsel < 0)
- return vsel;
-
- *selector = vsel;
+ int ret;

/* If this value is already set then do a GPIO update if we can */
- if (dcdc->dvs_gpio && dcdc->on_vsel == vsel)
+ if (dcdc->dvs_gpio && dcdc->on_vsel == selector)
return wm831x_buckv_set_dvs(rdev, 0);

- if (dcdc->dvs_gpio && dcdc->dvs_vsel == vsel)
+ if (dcdc->dvs_gpio && dcdc->dvs_vsel == selector)
return wm831x_buckv_set_dvs(rdev, 1);

/* Always set the ON status to the minimum voltage */
- ret = wm831x_set_bits(wm831x, on_reg, WM831X_DC1_ON_VSEL_MASK, vsel);
+ ret = wm831x_set_bits(wm831x, on_reg, WM831X_DC1_ON_VSEL_MASK,
+ selector);
if (ret < 0)
return ret;
- dcdc->on_vsel = vsel;
+ dcdc->on_vsel = selector;

if (!dcdc->dvs_gpio)
return ret;
@@ -293,12 +288,12 @@ static int wm831x_buckv_set_voltage(struct regulator_dev *rdev,
* usage where we want to get to the highest voltage very
* quickly.
*/
- if (vsel > dcdc->dvs_vsel) {
+ if (selector > dcdc->dvs_vsel) {
ret = wm831x_set_bits(wm831x, dvs_reg,
WM831X_DC1_DVS_VSEL_MASK,
dcdc->dvs_vsel);
if (ret == 0)
- dcdc->dvs_vsel = vsel;
+ dcdc->dvs_vsel = selector;
else
dev_warn(wm831x->dev,
"Failed to set DCDC DVS VSEL: %d\n", ret);
@@ -315,7 +310,7 @@ static int wm831x_buckv_set_suspend_voltage(struct regulator_dev *rdev,
u16 reg = dcdc->base + WM831X_DCDC_SLEEP_CONTROL;
int vsel;

- vsel = wm831x_buckv_select_min_voltage(rdev, uV, uV);
+ vsel = wm831x_buckv_map_voltage(rdev, uV, uV);
if (vsel < 0)
return vsel;

@@ -373,9 +368,10 @@ static int wm831x_buckv_get_current_limit(struct regulator_dev *rdev)
}

static struct regulator_ops wm831x_buckv_ops = {
- .set_voltage = wm831x_buckv_set_voltage,
+ .set_voltage_sel = wm831x_buckv_set_voltage_sel,
.get_voltage_sel = wm831x_buckv_get_voltage_sel,
.list_voltage = wm831x_buckv_list_voltage,
+ .map_voltage = wm831x_buckv_map_voltage,
.set_suspend_voltage = wm831x_buckv_set_suspend_voltage,
.set_current_limit = wm831x_buckv_set_current_limit,
.get_current_limit = wm831x_buckv_get_current_limit,
--
1.7.9.5


2012-06-12 02:24:17

by Axel Lin

[permalink] [raw]
Subject: [PATCH RFT 4/4] regulator: wm831x-dcdc: Convert to devm_request_threaded_irq

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/wm831x-dcdc.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index eea53e1..5ec10d7 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -532,8 +532,9 @@ static __devinit int wm831x_buckv_probe(struct platform_device *pdev)
}

irq = wm831x_irq(wm831x, platform_get_irq_byname(pdev, "UV"));
- ret = request_threaded_irq(irq, NULL, wm831x_dcdc_uv_irq,
- IRQF_TRIGGER_RISING, dcdc->name, dcdc);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ wm831x_dcdc_uv_irq, IRQF_TRIGGER_RISING,
+ dcdc->name, dcdc);
if (ret != 0) {
dev_err(&pdev->dev, "Failed to request UV IRQ %d: %d\n",
irq, ret);
@@ -541,8 +542,9 @@ static __devinit int wm831x_buckv_probe(struct platform_device *pdev)
}

irq = wm831x_irq(wm831x, platform_get_irq_byname(pdev, "HC"));
- ret = request_threaded_irq(irq, NULL, wm831x_dcdc_oc_irq,
- IRQF_TRIGGER_RISING, dcdc->name, dcdc);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ wm831x_dcdc_oc_irq, IRQF_TRIGGER_RISING,
+ dcdc->name, dcdc);
if (ret != 0) {
dev_err(&pdev->dev, "Failed to request HC IRQ %d: %d\n",
irq, ret);
@@ -554,8 +556,6 @@ static __devinit int wm831x_buckv_probe(struct platform_device *pdev)
return 0;

err_uv:
- free_irq(wm831x_irq(wm831x, platform_get_irq_byname(pdev, "UV")),
- dcdc);
err_regulator:
regulator_unregister(dcdc->regulator);
err:
@@ -567,14 +567,8 @@ err:
static __devexit int wm831x_buckv_remove(struct platform_device *pdev)
{
struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);
- struct wm831x *wm831x = dcdc->wm831x;

platform_set_drvdata(pdev, NULL);
-
- free_irq(wm831x_irq(wm831x, platform_get_irq_byname(pdev, "HC")),
- dcdc);
- free_irq(wm831x_irq(wm831x, platform_get_irq_byname(pdev, "UV")),
- dcdc);
regulator_unregister(dcdc->regulator);
if (dcdc->dvs_gpio)
gpio_free(dcdc->dvs_gpio);
@@ -694,8 +688,9 @@ static __devinit int wm831x_buckp_probe(struct platform_device *pdev)
}

irq = wm831x_irq(wm831x, platform_get_irq_byname(pdev, "UV"));
- ret = request_threaded_irq(irq, NULL, wm831x_dcdc_uv_irq,
- IRQF_TRIGGER_RISING, dcdc->name, dcdc);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ wm831x_dcdc_uv_irq, IRQF_TRIGGER_RISING,
+ dcdc->name, dcdc);
if (ret != 0) {
dev_err(&pdev->dev, "Failed to request UV IRQ %d: %d\n",
irq, ret);
@@ -717,9 +712,6 @@ static __devexit int wm831x_buckp_remove(struct platform_device *pdev)
struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);

platform_set_drvdata(pdev, NULL);
-
- free_irq(wm831x_irq(dcdc->wm831x, platform_get_irq_byname(pdev, "UV")),
- dcdc);
regulator_unregister(dcdc->regulator);

return 0;
@@ -828,9 +820,9 @@ static __devinit int wm831x_boostp_probe(struct platform_device *pdev)
}

irq = wm831x_irq(wm831x, platform_get_irq_byname(pdev, "UV"));
- ret = request_threaded_irq(irq, NULL, wm831x_dcdc_uv_irq,
- IRQF_TRIGGER_RISING, dcdc->name,
- dcdc);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ wm831x_dcdc_uv_irq, IRQF_TRIGGER_RISING,
+ dcdc->name, dcdc);
if (ret != 0) {
dev_err(&pdev->dev, "Failed to request UV IRQ %d: %d\n",
irq, ret);
@@ -852,9 +844,6 @@ static __devexit int wm831x_boostp_remove(struct platform_device *pdev)
struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);

platform_set_drvdata(pdev, NULL);
-
- free_irq(wm831x_irq(dcdc->wm831x, platform_get_irq_byname(pdev, "UV")),
- dcdc);
regulator_unregister(dcdc->regulator);

return 0;
--
1.7.9.5


2012-06-13 10:03:45

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH RFT 3/4] regulator: wm831x-dcdc: Convert wm831x_buckv_ops to set_voltage_sel and map_voltage

On Tue, Jun 12, 2012 at 10:23:09AM +0800, Axel Lin wrote:
> Signed-off-by: Axel Lin <[email protected]>

This patch is way more invasive than it needs to be (and therefore hard
to check) because

> - if (dcdc->dvs_gpio && dcdc->on_vsel == vsel)
> + if (dcdc->dvs_gpio && dcdc->on_vsel == selector)

you've replaced vsel with selector throughout the code.


Attachments:
(No filename) (356.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-06-13 11:41:48

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH RFT 3/4] regulator: wm831x-dcdc: Convert wm831x_buckv_ops to set_voltage_sel and map_voltage

2012/6/13 Mark Brown <[email protected]>:
> On Tue, Jun 12, 2012 at 10:23:09AM +0800, Axel Lin wrote:
>> Signed-off-by: Axel Lin <[email protected]>
>
> This patch is way more invasive than it needs to be (and therefore hard
> to check) because
>
>> - ? ? if (dcdc->dvs_gpio && dcdc->on_vsel == vsel)
>> + ? ? if (dcdc->dvs_gpio && dcdc->on_vsel == selector)
>
> you've replaced vsel with selector throughout the code.

Sorry. I don't get it.

In wm831x_buckv_set_voltage(), we call
wm831x_buckv_select_min_voltage() to get the selector ( named vsel).

In this patch, now we convert to set_voltage_sel:

static int wm831x_buckv_set_voltage_sel(struct regulator_dev *rdev,
unsigned selector)

So the selector now named 'selector'.


Or do you prefer I chenge the prototype to:

static int wm831x_buckv_set_voltage_sel(struct regulator_dev *rdev,
unsigned vsel)

Then the diff can be smaller.

Regards,
Axel

2012-06-13 11:51:21

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH RFT 3/4] regulator: wm831x-dcdc: Convert wm831x_buckv_ops to set_voltage_sel and map_voltage

On Wed, Jun 13, 2012 at 07:41:26PM +0800, Axel Lin wrote:

> Or do you prefer I chenge the prototype to:

> static int wm831x_buckv_set_voltage_sel(struct regulator_dev *rdev,
> unsigned vsel)

> Then the diff can be smaller.

Yes, this is the point. A huge proportion of the change is a cosmetic
change for this rename which makes it much harder to read and review.


Attachments:
(No filename) (408.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-06-13 11:51:53

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH RFT 4/4] regulator: wm831x-dcdc: Convert to devm_request_threaded_irq

On Tue, Jun 12, 2012 at 10:24:09AM +0800, Axel Lin wrote:
> Signed-off-by: Axel Lin <[email protected]>

Again, this has issues for driver removal.


Attachments:
(No filename) (149.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments