[PATCH v2 09/09] regulator: refresh 88pm8607 driver with updated api

[PATCH v2 09/09] regulator: refresh 88pm8607 driver with updated api

[Haojian Zhuang]

[PATCH v2 09/09] regulator: refresh 88pm8607 driver with updated api

[Mark Brown]

On Wed, Dec 09, 2009 at 08:18:18AM -0500, Haojian Zhuang wrote:

> Since i2c API of mfd 88pm860x driver is changed, refresh 88pm8607 driver.
> Remove the support of 88PM8607 A0/A1 stepping.

> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Might be nice to split the removal of the old devices into a separate
patch but that is definitely a win.  One blocker below, though.

> +		ret = (index < 3) ? (index * 50000 + 1800000) :
> +			((index < 7) ? (index * 50000 + 2550000) :
> +			3300000);

I know the previous code did this but I'm really not loving the multiple
ternery operators here for legibility.

>  	return 0;
>  }
> -subsys_initcall(pm8607_regulator_init);
> +module_init(pm8607_regulator_init);

This should be left at subsys_initcall() to try to ensure that the
regulators are initialsied before the things that use them, otherwise
the handling of init sequencing gets too complex.

[PATCH v2 09/09] regulator: refresh 88pm8607 driver with updated api

[Haojian Zhuang]

On Wed, Dec 9, 2009 at 10:20 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Dec 09, 2009 at 08:18:18AM -0500, Haojian Zhuang wrote:
>
>> Since i2c API of mfd 88pm860x driver is changed, refresh 88pm8607 driver.
>> Remove the support of 88PM8607 A0/A1 stepping.
>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>
> Might be nice to split the removal of the old devices into a separate
> patch but that is definitely a win. ?One blocker below, though.
>
>> + ? ? ? ? ? ? ret = (index < 3) ? (index * 50000 + 1800000) :
>> + ? ? ? ? ? ? ? ? ? ? ((index < 7) ? (index * 50000 + 2550000) :
>> + ? ? ? ? ? ? ? ? ? ? 3300000);
>
> I know the previous code did this but I'm really not loving the multiple
> ternery operators here for legibility.
>
>> ? ? ? return 0;
>> ?}
>> -subsys_initcall(pm8607_regulator_init);
>> +module_init(pm8607_regulator_init);
>
> This should be left at subsys_initcall() to try to ensure that the
> regulators are initialsied before the things that use them, otherwise
> the handling of init sequencing gets too complex.
>

Now split this patch into two. Thanks :)

Best Regards
Haojian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-regulator-refresh-88pm8607-driver-with-updated-api.patch
Type: text/x-patch
Size: 5509 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091209/00e76454/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-regulator-unsupport-88pm8607-A0-and-A1.patch
Type: text/x-patch
Size: 9843 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091209/00e76454/attachment-0003.bin>

[PATCH v2 09/09] regulator: refresh 88pm8607 driver with updated api

[Mark Brown]

On Wed, Dec 09, 2009 at 10:44:15PM -0500, Haojian Zhuang wrote:

> Now split this patch into two. Thanks :)

Both look OK to me:

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

[PATCH v2 09/09] regulator: refresh 88pm8607 driver with updated api

[Liam Girdwood]

On Wed, 2009-12-09 at 22:44 -0500, Haojian Zhuang wrote:
> On Wed, Dec 9, 2009 at 10:20 AM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Wed, Dec 09, 2009 at 08:18:18AM -0500, Haojian Zhuang wrote:
> >
> >> Since i2c API of mfd 88pm860x driver is changed, refresh 88pm8607 driver.
> >> Remove the support of 88PM8607 A0/A1 stepping.
> >
> >> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> >
> > Might be nice to split the removal of the old devices into a separate
> > patch but that is definitely a win.  One blocker below, though.
> >
> >> +             ret = (index < 3) ? (index * 50000 + 1800000) :
> >> +                     ((index < 7) ? (index * 50000 + 2550000) :
> >> +                     3300000);
> >
> > I know the previous code did this but I'm really not loving the multiple
> > ternery operators here for legibility.
> >
> >>       return 0;
> >>  }
> >> -subsys_initcall(pm8607_regulator_init);
> >> +module_init(pm8607_regulator_init);
> >
> > This should be left at subsys_initcall() to try to ensure that the
> > regulators are initialsied before the things that use them, otherwise
> > the handling of init sequencing gets too complex.
> >
> 
> Now split this patch into two. Thanks :)


Applied both.

Thanks

Liam