Avoid to use constraint name in driver. So use driver_data instead to
store regulator name in platform driver.
Signed-off-by: Haojian Zhuang <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
---
drivers/regulator/88pm8607.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index 784ea77..63fc34e 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -402,12 +402,13 @@ static int __devinit pm8607_regulator_probe(struct platform_device *pdev)
int i;
pdata = pdev->dev.platform_data;
- if (pdata == NULL)
+ if ((pdata == NULL) || (pdata->driver_data == NULL))
return -EINVAL;
for (i = 0; i < ARRAY_SIZE(pm8607_regulator_info); i++) {
info = &pm8607_regulator_info[i];
- if (!strcmp(info->desc.name, pdata->constraints.name))
+ /* pdata->driver_data stores the name of regulator */
+ if (!strcmp(info->desc.name, pdata->driver_data))
break;
}
if (i > ARRAY_SIZE(pm8607_regulator_info)) {
@@ -424,6 +425,7 @@ static int __devinit pm8607_regulator_probe(struct platform_device *pdev)
if (info->chip->buck3_double)
info->slope_double = 1;
+ /* replace driver_data with info */
info->regulator = regulator_register(&info->desc, &pdev->dev,
pdata, info);
if (IS_ERR(info->regulator)) {
--
1.5.6.5
On Mon, Apr 18, 2011 at 10:04:11PM +0800, Haojian Zhuang wrote:
> Avoid to use constraint name in driver. So use driver_data instead to
> store regulator name in platform driver.
If this is something that's present in the current driver it should be
fixed for 2.6.39 so should be a patch at the start of the series which
can be applied separately to the others.
> + /* pdata->driver_data stores the name of regulator */
> + if (!strcmp(info->desc.name, pdata->driver_data))
> break;
Why not just use #defines in the header? It'd be less error prone if
nothing else.
On Mon, Apr 18, 2011 at 10:30 PM, Mark Brown
<[email protected]> wrote:
> On Mon, Apr 18, 2011 at 10:04:11PM +0800, Haojian Zhuang wrote:
>> Avoid to use constraint name in driver. So use driver_data instead to
>> store regulator name in platform driver.
>
> If this is something that's present in the current driver it should be
> fixed for 2.6.39 so should be a patch at the start of the series which
> can be applied separately to the others.
>
>> + ? ? ? ? ? ? /* pdata->driver_data stores the name of regulator */
>> + ? ? ? ? ? ? if (!strcmp(info->desc.name, pdata->driver_data))
>> ? ? ? ? ? ? ? ? ? ? ? break;
>
> Why not just use #defines in the header? ?It'd be less error prone if
> nothing else.
>
I don't understand this. desc.name is assigned in regulator driver.
pdata->driver_data is assigned in platform driver. I need to use same
name to link them together.
On Mon, Apr 18, 2011 at 11:38:00PM +0800, Haojian Zhuang wrote:
> On Mon, Apr 18, 2011 at 10:30 PM, Mark Brown
> > Why not just use #defines in the header? ?It'd be less error prone if
> > nothing else.
> I don't understand this. desc.name is assigned in regulator driver.
> pdata->driver_data is assigned in platform driver. I need to use same
> name to link them together.
I assume that by "platform driver" you mean the machine driver for the
system? What I'm saying is that instead of using a string to match it'd
be more normal to use an enum in the driver header, that way if someone
typos the string or something then there will be a build failure whereas
with strings like this there will only be a runtime failure.
Part of the problem here is that your changelogs aren't really saying
why you're making these changes...
>-----Original Message-----
>From: Mark Brown [mailto:[email protected]]
>Sent: 2011年4月19日 12:03 AM
>To: Haojian Zhuang
>Cc: Haojian Zhuang; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]
>Subject: Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint
>name in driver
>
>On Mon, Apr 18, 2011 at 11:38:00PM +0800, Haojian Zhuang wrote:
>> On Mon, Apr 18, 2011 at 10:30 PM, Mark Brown
>
>> > Why not just use #defines in the header? It'd be less error prone
>if
>> > nothing else.
>
>> I don't understand this. desc.name is assigned in regulator driver.
>> pdata->driver_data is assigned in platform driver. I need to use same
>> name to link them together.
>
>I assume that by "platform driver" you mean the machine driver for the
>system? What I'm saying is that instead of using a string to match it'd
>be more normal to use an enum in the driver header, that way if someone
>typos the string or something then there will be a build failure whereas
>with strings like this there will only be a runtime failure.
>
It's nearly same whatever it's string or index number. In machine driver,
it's more convenient on using string. Otherwise, I still need to create
a new integer array to store index since I transfer them into regulator data
by driver_data parameter in regulator_init_data.
>Part of the problem here is that your changelogs aren't really saying
>why you're making these changes...
Yes, the changelog is too simple.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Mon, Apr 18, 2011 at 07:45:06PM -0700, Haojian Zhuang wrote:
> >I assume that by "platform driver" you mean the machine driver for the
> >system? What I'm saying is that instead of using a string to match it'd
> >be more normal to use an enum in the driver header, that way if someone
> >typos the string or something then there will be a build failure whereas
> >with strings like this there will only be a runtime failure.
> It's nearly same whatever it's string or index number. In machine driver,
> it's more convenient on using string. Otherwise, I still need to create
You're saying it's more convenient but you're not saying why. I'd say
that the lack of build time checking I mentioned is actually unhelpful.
> a new integer array to store index since I transfer them into regulator data
> by driver_data parameter in regulator_init_data.
You already need to add a char * to store the name, adding an enum
instead isn't a substantial difference...
>-----Original Message-----
>From: Mark Brown [mailto:[email protected]]
>Sent: 2011??4??19?? 4:13 PM
>To: Haojian Zhuang
>Cc: Haojian Zhuang; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]
>Subject: Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint
>name in driver
>
>On Mon, Apr 18, 2011 at 07:45:06PM -0700, Haojian Zhuang wrote:
>
>> >I assume that by "platform driver" you mean the machine driver for
>the
>> >system? What I'm saying is that instead of using a string to match
>it'd
>> >be more normal to use an enum in the driver header, that way if
>someone
>> >typos the string or something then there will be a build failure
>whereas
>> >with strings like this there will only be a runtime failure.
>
>> It's nearly same whatever it's string or index number. In machine
>driver,
>> it's more convenient on using string. Otherwise, I still need to
>create
>
>You're saying it's more convenient but you're not saying why. I'd say
>that the lack of build time checking I mentioned is actually unhelpful.
>
>> a new integer array to store index since I transfer them into
>regulator data
>> by driver_data parameter in regulator_init_data.
>
>You already need to add a char * to store the name, adding an enum
>instead isn't a substantial difference...
OK. So I just need to switch to ID instead of name on these two patches.
Is it right?
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Avoid to use constraint name in driver. So use driver_data instead to
store regulator id in platform driver.
Signed-off-by: Haojian Zhuang <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
---
drivers/mfd/88pm860x-core.c | 35 ++++++++---------------------------
drivers/regulator/88pm8607.c | 25 +++++++++++++------------
2 files changed, 21 insertions(+), 39 deletions(-)
diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index 4fbc69b..3147c41 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -544,7 +544,7 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
{
struct regulator_init_data *initdata;
int ret;
- int i, j;
+ int i, seq;
if ((pdata == NULL) || (pdata->regulator == NULL))
return;
@@ -552,31 +552,12 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
if (pdata->num_regulators > ARRAY_SIZE(regulator_devs))
pdata->num_regulators = ARRAY_SIZE(regulator_devs);
- for (i = 0, j = -1; i < pdata->num_regulators; i++) {
+ for (i = 0, seq = -1; i < pdata->num_regulators; i++) {
initdata = &pdata->regulator[i];
- if (strstr(initdata->constraints.name, "BUCK")) {
- sscanf(initdata->constraints.name, "BUCK%d", &j);
- /* BUCK1 ~ BUCK3 */
- if ((j < 1) || (j > 3)) {
- dev_err(chip->dev, "Failed to add constraint "
- "(%s)\n", initdata->constraints.name);
- goto out;
- }
- j = (j - 1) + PM8607_ID_BUCK1;
- }
- if (strstr(initdata->constraints.name, "LDO")) {
- sscanf(initdata->constraints.name, "LDO%d", &j);
- /* LDO1 ~ LDO15 */
- if ((j < 1) || (j > 15)) {
- dev_err(chip->dev, "Failed to add constraint "
- "(%s)\n", initdata->constraints.name);
- goto out;
- }
- j = (j - 1) + PM8607_ID_LDO1;
- }
- if (j == -1) {
- dev_err(chip->dev, "Failed to add constraint (%s)\n",
- initdata->constraints.name);
+ seq = *(unsigned int *)initdata->driver_data;
+ if ((seq < 0) || (seq > PM8607_ID_RG_MAX)) {
+ dev_err(chip->dev, "Wrong ID(%d) on regulator(%s)\n",
+ seq, initdata->constraints.name);
goto out;
}
memcpy(®ulator_pdata[i], &pdata->regulator[i],
@@ -584,10 +565,10 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
regulator_devs[i].platform_data = ®ulator_pdata[i];
regulator_devs[i].pdata_size = sizeof(regulator_pdata[i]);
regulator_devs[i].num_resources = 1;
- regulator_devs[i].resources = ®ulator_resources[j];
+ regulator_devs[i].resources = ®ulator_resources[seq];
ret = mfd_add_devices(chip->dev, 0, ®ulator_devs[i], 1,
- ®ulator_resources[j], 0);
+ ®ulator_resources[seq], 0);
if (ret < 0) {
dev_err(chip->dev, "Failed to add regulator subdev\n");
goto out;
diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index 784ea77..1904be9 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -398,32 +398,33 @@ static int __devinit pm8607_regulator_probe(struct platform_device *pdev)
{
struct pm860x_chip *chip = dev_get_drvdata(pdev->dev.parent);
struct pm8607_regulator_info *info = NULL;
- struct regulator_init_data *pdata;
+ struct regulator_init_data *pdata = pdev->dev.platform_data;
+ struct resource *res;
int i;
- pdata = pdev->dev.platform_data;
- if (pdata == NULL)
+ res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (res == NULL) {
+ dev_err(&pdev->dev, "No I/O resource!\n");
return -EINVAL;
-
+ }
for (i = 0; i < ARRAY_SIZE(pm8607_regulator_info); i++) {
info = &pm8607_regulator_info[i];
- if (!strcmp(info->desc.name, pdata->constraints.name))
+ if (info->desc.id == res->start)
break;
}
- if (i > ARRAY_SIZE(pm8607_regulator_info)) {
- dev_err(&pdev->dev, "Failed to find regulator %s\n",
- pdata->constraints.name);
+ if ((i < 0) || (i > PM8607_ID_RG_MAX)) {
+ dev_err(&pdev->dev, "Failed to find regulator %d\n",
+ res->start);
return -EINVAL;
}
-
info->i2c = (chip->id == CHIP_PM8607) ? chip->client : chip->companion;
info->chip = chip;
/* check DVC ramp slope double */
- if (!strcmp(info->desc.name, "BUCK3"))
- if (info->chip->buck3_double)
- info->slope_double = 1;
+ if ((i == PM8607_ID_BUCK3) && info->chip->buck3_double)
+ info->slope_double = 1;
+ /* replace driver_data with info */
info->regulator = regulator_register(&info->desc, &pdev->dev,
pdata, info);
if (IS_ERR(info->regulator)) {
--
1.5.6.5