| Message ID | 20181120175211.3913-9-oskari@lemmela.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AXP8x3 AC and battery power supply support | expand |
On Tue, 20 Nov 2018, Oskari Lemmela wrote: > Parts of the AXP803 are compatible with their counterparts on the AXP813. > These include the GPIO, ADC, AC and battery power supplies. > > Signed-off-by: Oskari Lemmela <oskari@lemmela.net> > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > Tested-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > drivers/mfd/axp20x.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index dfc3cff1d08b..e415b967d38c 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -728,11 +728,26 @@ static const struct mfd_cell axp288_cells[] = { > > static const struct mfd_cell axp803_cells[] = { > { > + .name = "axp20x-gpio", > + .of_compatible = "x-powers,axp813-gpio", > + }, { > .name = "axp221-pek", > .num_resources = ARRAY_SIZE(axp803_pek_resources), > .resources = axp803_pek_resources, > }, > { .name = "axp20x-regulator" }, > + { > + .name = "axp813-adc", > + .of_compatible = "x-powers,axp813-adc", > + }, { > + .name = "axp20x-battery-power-supply", > + .of_compatible = "x-powers,axp813-battery-power-supply", > + }, { > + .name = "axp20x-ac-power-supply", > + .of_compatible = "x-powers,axp813-ac-power-supply", > + .num_resources = ARRAY_SIZE(axp20x_ac_power_supply_resources), > + .resources = axp20x_ac_power_supply_resources, > + }, > }; My OCD-dar is going crazy. Why haven't you used the same alignment as is already there? If it starts to run over 80-chars then bring the others back. Also why is there a single liner shoved in the middle of the multi-line entries? Please move the singles to the top or the bottom.
On Fri, Dec 7, 2018 at 8:40 AM Lee Jones <lee.jones@linaro.org> wrote: > My OCD-dar is going crazy. > > Why haven't you used the same alignment as is already there? > > If it starts to run over 80-chars then bring the others back. > > Also why is there a single liner shoved in the middle of the > multi-line entries? Please move the singles to the top or the > bottom. Hi Lee, Could you please reformat it in the way that makes your OCD-dar happy? It would be really nice to get AC and battery support for APX8x3 merged -- it'll make Pinebook and Teres-I pretty well supported by mainline kernel. Thanks, Vasily > > -- > Lee Jones [李琼斯] > Linaro Services Technical Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog
On Fri, 07 Dec 2018, Vasily Khoruzhick wrote: > On Fri, Dec 7, 2018 at 8:40 AM Lee Jones <lee.jones@linaro.org> wrote: > > > My OCD-dar is going crazy. > > > > Why haven't you used the same alignment as is already there? > > > > If it starts to run over 80-chars then bring the others back. > > > > Also why is there a single liner shoved in the middle of the > > multi-line entries? Please move the singles to the top or the > > bottom. > > Hi Lee, > > Could you please reformat it in the way that makes your OCD-dar happy? > It would be really nice to get I'm afraid not, for a multitude of reasons. The most important of which surround testing. > AC and battery support for APX8x3 merged -- it'll make Pinebook and > Teres-I pretty well supported by mainline kernel. That's great. A worthy cause indeed. So I'm sure you guys will want to turn the patch around in short order so that it's applied in time for the next merge window.
Hi Lee, On Fri, Dec 07, 2018 at 07:22:37PM +0000, Lee Jones wrote: > On Fri, 07 Dec 2018, Vasily Khoruzhick wrote: > > > On Fri, Dec 7, 2018 at 8:40 AM Lee Jones <lee.jones@linaro.org> wrote: > > > > > My OCD-dar is going crazy. > > > > > > Why haven't you used the same alignment as is already there? > > > > > > If it starts to run over 80-chars then bring the others back. > > > > > > Also why is there a single liner shoved in the middle of the > > > multi-line entries? Please move the singles to the top or the > > > bottom. > > > > Hi Lee, > > > > Could you please reformat it in the way that makes your OCD-dar happy? > > It would be really nice to get > > I'm afraid not, for a multitude of reasons. > > The most important of which surround testing. > > > AC and battery support for APX8x3 merged -- it'll make Pinebook and > > Teres-I pretty well supported by mainline kernel. > > That's great. A worthy cause indeed. So I'm sure you guys will want > to turn the patch around in short order so that it's applied in time > for the next merge window. > Aren't the MFD cells probed in order? In that case, it makes little sense to short order them for this particular device (X-Powers PMICs in general). It will just make the system boot slower because of probe deferring. Why? As explained by Chen-Yu in v3[1], axp-gpios can be muxed as regulators, thus should be probed before axp-regulators. axp-adc is often used by axp-battery, axp-usb-power, axp-ac-power, thus should be probed beforehand as well. For the alignment that also triggered your OCD, I can send you a patch the day you merge this one if it can help. I sent a few patches for this driver that didn't respect the alignment so I'm fine fixing the mfd cells (and eventually re-order them as I saw a few axp-gpio cells being declared after axp-regulators). Does that make this patch OK for you, Lee? Thanks, Quentin [1] https://lkml.org/lkml/2018/10/11/338
Hi Lee, On 12/7/18 6:40 PM, Lee Jones wrote: > On Tue, 20 Nov 2018, Oskari Lemmela wrote: > >> Parts of the AXP803 are compatible with their counterparts on the AXP813. >> These include the GPIO, ADC, AC and battery power supplies. >> >> Signed-off-by: Oskari Lemmela <oskari@lemmela.net> >> Reviewed-by: Chen-Yu Tsai <wens@csie.org> >> Tested-by: Vasily Khoruzhick <anarsoul@gmail.com> >> --- >> drivers/mfd/axp20x.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c >> index dfc3cff1d08b..e415b967d38c 100644 >> --- a/drivers/mfd/axp20x.c >> +++ b/drivers/mfd/axp20x.c >> @@ -728,11 +728,26 @@ static const struct mfd_cell axp288_cells[] = { >> >> static const struct mfd_cell axp803_cells[] = { >> { >> + .name = "axp20x-gpio", >> + .of_compatible = "x-powers,axp813-gpio", >> + }, { >> .name = "axp221-pek", >> .num_resources = ARRAY_SIZE(axp803_pek_resources), >> .resources = axp803_pek_resources, >> }, >> { .name = "axp20x-regulator" }, >> + { >> + .name = "axp813-adc", >> + .of_compatible = "x-powers,axp813-adc", >> + }, { >> + .name = "axp20x-battery-power-supply", >> + .of_compatible = "x-powers,axp813-battery-power-supply", >> + }, { >> + .name = "axp20x-ac-power-supply", >> + .of_compatible = "x-powers,axp813-ac-power-supply", >> + .num_resources = ARRAY_SIZE(axp20x_ac_power_supply_resources), >> + .resources = axp20x_ac_power_supply_resources, >> + }, >> }; > My OCD-dar is going crazy. > > Why haven't you used the same alignment as is already there? > > If it starts to run over 80-chars then bring the others back. > > Also why is there a single liner shoved in the middle of the > multi-line entries? Please move the singles to the top or the > bottom. > I sent patch set v8 which contains ChenYu's re-align patch and this patch rebased top of it. Re-align patch will make number of whitespaces consistent in axp20x.c Thanks, Oskari
On Sat, 08 Dec 2018, Quentin Schulz wrote: > Hi Lee, > > On Fri, Dec 07, 2018 at 07:22:37PM +0000, Lee Jones wrote: > > On Fri, 07 Dec 2018, Vasily Khoruzhick wrote: > > > > > On Fri, Dec 7, 2018 at 8:40 AM Lee Jones <lee.jones@linaro.org> wrote: > > > > > > > My OCD-dar is going crazy. > > > > > > > > Why haven't you used the same alignment as is already there? > > > > > > > > If it starts to run over 80-chars then bring the others back. > > > > > > > > Also why is there a single liner shoved in the middle of the > > > > multi-line entries? Please move the singles to the top or the > > > > bottom. > > > > > > Hi Lee, > > > > > > Could you please reformat it in the way that makes your OCD-dar happy? > > > It would be really nice to get > > > > I'm afraid not, for a multitude of reasons. > > > > The most important of which surround testing. > > > > > AC and battery support for APX8x3 merged -- it'll make Pinebook and > > > Teres-I pretty well supported by mainline kernel. > > > > That's great. A worthy cause indeed. So I'm sure you guys will want > > to turn the patch around in short order so that it's applied in time > > for the next merge window. > > > > Aren't the MFD cells probed in order? > > In that case, it makes little sense to short order them for this > particular device (X-Powers PMICs in general). It will just make the > system boot slower because of probe deferring. > > Why? As explained by Chen-Yu in v3[1], axp-gpios can be muxed as > regulators, thus should be probed before axp-regulators. axp-adc is > often used by axp-battery, axp-usb-power, axp-ac-power, thus should be > probed beforehand as well. If there are inter-dependencies between the devices, it makes sense to keep them in the most efficient order. > For the alignment that also triggered your OCD, I can send you a patch > the day you merge this one if it can help. I sent a few patches for this > driver that didn't respect the alignment so I'm fine fixing the mfd > cells (and eventually re-order them as I saw a few axp-gpio cells being > declared after axp-regulators). That's fine. Please send the patch (based on this set) right away. > Does that make this patch OK for you, Lee? Yes, thank you.
On Tue, 20 Nov 2018, Oskari Lemmela wrote: > Parts of the AXP803 are compatible with their counterparts on the AXP813. > These include the GPIO, ADC, AC and battery power supplies. > > Signed-off-by: Oskari Lemmela <oskari@lemmela.net> > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > Tested-by: Vasily Khoruzhick <anarsoul@gmail.com> > --- > drivers/mfd/axp20x.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) Applied, thanks.
Hi Lee, On Mon, Dec 10, 2018 at 06:27:18AM +0000, Lee Jones wrote: > On Sat, 08 Dec 2018, Quentin Schulz wrote: > > > Hi Lee, > > > > On Fri, Dec 07, 2018 at 07:22:37PM +0000, Lee Jones wrote: > > > On Fri, 07 Dec 2018, Vasily Khoruzhick wrote: > > > > > > > On Fri, Dec 7, 2018 at 8:40 AM Lee Jones <lee.jones@linaro.org> wrote: > > > > > > > > > My OCD-dar is going crazy. > > > > > > > > > > Why haven't you used the same alignment as is already there? > > > > > > > > > > If it starts to run over 80-chars then bring the others back. > > > > > > > > > > Also why is there a single liner shoved in the middle of the > > > > > multi-line entries? Please move the singles to the top or the > > > > > bottom. > > > > > > > > Hi Lee, > > > > > > > > Could you please reformat it in the way that makes your OCD-dar happy? > > > > It would be really nice to get > > > > > > I'm afraid not, for a multitude of reasons. > > > > > > The most important of which surround testing. > > > > > > > AC and battery support for APX8x3 merged -- it'll make Pinebook and > > > > Teres-I pretty well supported by mainline kernel. > > > > > > That's great. A worthy cause indeed. So I'm sure you guys will want > > > to turn the patch around in short order so that it's applied in time > > > for the next merge window. > > > > > > > Aren't the MFD cells probed in order? > > > > In that case, it makes little sense to short order them for this > > particular device (X-Powers PMICs in general). It will just make the > > system boot slower because of probe deferring. > > > > Why? As explained by Chen-Yu in v3[1], axp-gpios can be muxed as > > regulators, thus should be probed before axp-regulators. axp-adc is > > often used by axp-battery, axp-usb-power, axp-ac-power, thus should be > > probed beforehand as well. > > If there are inter-dependencies between the devices, it makes sense to > keep them in the most efficient order. > > > For the alignment that also triggered your OCD, I can send you a patch > > the day you merge this one if it can help. I sent a few patches for this > > driver that didn't respect the alignment so I'm fine fixing the mfd > > cells (and eventually re-order them as I saw a few axp-gpio cells being > > declared after axp-regulators). > > That's fine. Please send the patch (based on this set) right away. > Since Oskari sent a new version with an alignment fix patch and you merged that version, I assume I do not have any work to do anymore on this matter. Thank you all for the patches, Quentin
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c index dfc3cff1d08b..e415b967d38c 100644 --- a/drivers/mfd/axp20x.c +++ b/drivers/mfd/axp20x.c @@ -728,11 +728,26 @@ static const struct mfd_cell axp288_cells[] = { static const struct mfd_cell axp803_cells[] = { { + .name = "axp20x-gpio", + .of_compatible = "x-powers,axp813-gpio", + }, { .name = "axp221-pek", .num_resources = ARRAY_SIZE(axp803_pek_resources), .resources = axp803_pek_resources, }, { .name = "axp20x-regulator" }, + { + .name = "axp813-adc", + .of_compatible = "x-powers,axp813-adc", + }, { + .name = "axp20x-battery-power-supply", + .of_compatible = "x-powers,axp813-battery-power-supply", + }, { + .name = "axp20x-ac-power-supply", + .of_compatible = "x-powers,axp813-ac-power-supply", + .num_resources = ARRAY_SIZE(axp20x_ac_power_supply_resources), + .resources = axp20x_ac_power_supply_resources, + }, }; static const struct mfd_cell axp806_self_working_cells[] = {