[PATCH 1/4] arm: multi_v7: Add AXP20X_POWER

[PATCH 1/4] arm: multi_v7: Add AXP20X_POWER

[Maxime Ripard]

The multi_v7 defconfig is missing the AXP209 USB power supply driver,
resulting in non-working USB controllers because the USB PHY will always
return EPROBE_DEFER.

Make sure it's selected.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/multi_v7_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 4d19c1b4b8e7..6bab339a8dd8 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -432,6 +432,7 @@ CONFIG_GPIO_TPS6586X=y
 CONFIG_GPIO_TPS65910=y
 CONFIG_BATTERY_ACT8945A=y
 CONFIG_BATTERY_SBS=y
+CONFIG_AXP20X_POWER=y
 CONFIG_BATTERY_MAX17040=m
 CONFIG_BATTERY_MAX17042=m
 CONFIG_CHARGER_MAX14577=m
@@ -439,7 +440,6 @@ CONFIG_CHARGER_MAX77693=m
 CONFIG_CHARGER_MAX8997=m
 CONFIG_CHARGER_MAX8998=m
 CONFIG_CHARGER_TPS65090=y
-CONFIG_AXP20X_POWER=m
 CONFIG_POWER_RESET_AS3722=y
 CONFIG_POWER_RESET_GPIO=y
 CONFIG_POWER_RESET_GPIO_RESTART=y
-- 
2.13.3

[PATCH 2/4] arm: sunxi: refresh the defconfig

[Maxime Ripard]

Update the defconfig with the current state of defaults.

This was done using make sunxi_defconfig; make savedefconfig

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/sunxi_defconfig | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 0ec1d1ec130f..400eb9366a66 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -1,4 +1,3 @@
-CONFIG_FHANDLE=y
 CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y
 CONFIG_CGROUPS=y
@@ -56,7 +55,6 @@ CONFIG_STMMAC_ETH=y
 # CONFIG_NET_VENDOR_VIA is not set
 # CONFIG_NET_VENDOR_WIZNET is not set
 # CONFIG_WLAN is not set
-# CONFIG_INPUT_MOUSEDEV is not set
 CONFIG_INPUT_EVDEV=y
 CONFIG_KEYBOARD_SUN4I_LRADC=y
 # CONFIG_INPUT_MOUSE is not set
@@ -71,7 +69,6 @@ CONFIG_SERIAL_8250_RUNTIME_UARTS=8
 CONFIG_SERIAL_8250_DW=y
 CONFIG_SERIAL_OF_PLATFORM=y
 # CONFIG_HW_RANDOM is not set
-CONFIG_I2C=y
 CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_MV64XXX=y
 CONFIG_I2C_SUN6I_P2WI=y
@@ -82,12 +79,10 @@ CONFIG_GPIO_SYSFS=y
 CONFIG_POWER_SUPPLY=y
 CONFIG_AXP20X_POWER=y
 CONFIG_THERMAL=y
-CONFIG_THERMAL_OF=y
 CONFIG_CPU_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_SUNXI_WATCHDOG=y
 CONFIG_MFD_AC100=y
-CONFIG_MFD_AXP20X=y
 CONFIG_MFD_AXP20X_I2C=y
 CONFIG_MFD_AXP20X_RSB=y
 CONFIG_REGULATOR=y
@@ -99,12 +94,9 @@ CONFIG_MEDIA_RC_SUPPORT=y
 CONFIG_RC_DEVICES=y
 CONFIG_IR_SUNXI=y
 CONFIG_DRM=y
-CONFIG_DRM_DUMB_VGA_DAC=y
 CONFIG_DRM_SUN4I=y
-CONFIG_FB=y
+CONFIG_DRM_DUMB_VGA_DAC=y
 CONFIG_FB_SIMPLE=y
-CONFIG_FRAMEBUFFER_CONSOLE=y
-CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
 CONFIG_SOUND=y
 CONFIG_SND=y
 CONFIG_SND_SOC=y
@@ -130,7 +122,6 @@ CONFIG_RTC_CLASS=y
 # CONFIG_RTC_INTF_SYSFS is not set
 # CONFIG_RTC_INTF_PROC is not set
 CONFIG_RTC_DRV_AC100=y
-CONFIG_RTC_DRV_SUN6I=y
 CONFIG_RTC_DRV_SUNXI=y
 CONFIG_DMADEVICES=y
 CONFIG_DMA_SUN6I=y
-- 
2.13.3

[PATCH 2/4] arm: sunxi: refresh the defconfig

[Chen-Yu Tsai]

On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Update the defconfig with the current state of defaults.
>
> This was done using make sunxi_defconfig; make savedefconfig
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

[PATCH 2/4] arm: sunxi: refresh the defconfig

[Maxime Ripard]

On Sat, Jul 22, 2017 at 10:05:06AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Update the defconfig with the current state of defaults.
> >
> > This was done using make sunxi_defconfig; make savedefconfig
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/473af518/attachment.sig>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Maxime Ripard]

The APX20X_POWER option, that is a dependency of the USB controllers
depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
back.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/sunxi_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 400eb9366a66..456988dd7b0e 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -127,6 +127,8 @@ CONFIG_DMADEVICES=y
 CONFIG_DMA_SUN6I=y
 # CONFIG_IOMMU_SUPPORT is not set
 CONFIG_EXTCON=y
+CONFIG_IIO=y
+CONFIG_AXP20X_ADC=y
 CONFIG_PWM=y
 CONFIG_PWM_SUN4I=y
 CONFIG_PHY_SUN4I_USB=y
-- 
2.13.3

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Chen-Yu Tsai]

On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The APX20X_POWER option, that is a dependency of the USB controllers
> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
> back.

The statement is only half true. AXP20X_POWER can fall back to reading
the ADC registers directly if AXP20X_ADC is not enabled. Is there a plan
to remove this feature?

Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
not the later ones that don't have voltage/current sensing. I would
like to change the hard dependency on AXP20X_ADC to the "imply"
keyword in Kconfig for the AC and battery power supply drivers.
That is another discussion though.

So I think the description should be something like

    AXP20X_POWER depends on IIO. Even though it does not depend on
    AXP20X_ADC, it is the new, preferred way of getting power supply
    readings. The other AXP power supply drivers use it as well.

    Enable both options.

ChenYu

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/configs/sunxi_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
> index 400eb9366a66..456988dd7b0e 100644
> --- a/arch/arm/configs/sunxi_defconfig
> +++ b/arch/arm/configs/sunxi_defconfig
> @@ -127,6 +127,8 @@ CONFIG_DMADEVICES=y
>  CONFIG_DMA_SUN6I=y
>  # CONFIG_IOMMU_SUPPORT is not set
>  CONFIG_EXTCON=y
> +CONFIG_IIO=y
> +CONFIG_AXP20X_ADC=y
>  CONFIG_PWM=y
>  CONFIG_PWM_SUN4I=y
>  CONFIG_PHY_SUN4I_USB=y
> --
> 2.13.3
>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Quentin Schulz]

Hi all,

I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.

On 22/07/2017 04:19, Chen-Yu Tsai wrote:
> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> The APX20X_POWER option, that is a dependency of the USB controllers
>> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
>> back.
> 
> The statement is only half true. AXP20X_POWER can fall back to reading
> the ADC registers directly if AXP20X_ADC is not enabled.

For any PMIC using "x-powers,axp202-usb-power-supply" compatible, that
is true, the others imperatively use IIO channels as there was no prior
support to this driver.

The switch between reading IIO channels or directly reading the
registers is done at probe by checking IS_ENABLED(AXP20X_ADC)
(IS_ENABLED returns 1 when Kconfig is set to y or m). The goal was to
not break the existing system when users would update to a newer kernel
(as they may not have selected AXP20X_ADC).

Since there is no hard dependency between AXP20X_ADC and AXP20X_POWER, I
think we may have a problem when AXP20X_ADC is built as a module. Then
AXP20X_POWER probing is deferred since it waits for the IIO channels.
Also, if you do a modprobe on AXP20X_POWER it'll not probe AXP20X_ADC
because the dependency is not declared. I don't know if there is a way
to declare this dependency programatically only when AXP20X_ADC is built
as a module?

> Is there a plan
> to remove this feature?
> 

As said above (and in the commit log[1]), I added the condition for
backward compatibility so anyone could still use AXP20X_POWER without
the ADC, but maybe that is irrelevant? This could solve a bit of the
"complexity" of the driver, now that we know we want to enforce
AXP20X_ADC to be built with AXP20X_POWER. I'm obviously okay to remove
this feature if my point of backward compatibility is irrelevant.

But now, we still have the problem that AXP20X_ADC is not needed for
AXP20X_POWER for AXP22X (and those that don't have voltage/current
sensing). We don't really need to force AXP20X_ADC to be built when the
user want AXP20X_POWER.

1) We force AXP20X_ADC to be built whenever AXP20X_POWER is built, even
for PMICs that don't have any voltage/current sensing. This way, we can
enforce the dependency in Kconfig and everything's easy,

2) We programmatically set the dependency between AXP20X_ADC and
AXP20X_POWER depending on the compatible used (AXP20X set the
dependency, not AXP22X).

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33863c938caa2538397170f1a355885f1ea1564e

Thanks,
Quentin

> Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
> not the later ones that don't have voltage/current sensing. I would
> like to change the hard dependency on AXP20X_ADC to the "imply"
> keyword in Kconfig for the AC and battery power supply drivers.
> That is another discussion though.
> 
> So I think the description should be something like
> 
>     AXP20X_POWER depends on IIO. Even though it does not depend on
>     AXP20X_ADC, it is the new, preferred way of getting power supply
>     readings. The other AXP power supply drivers use it as well.
> 
>     Enable both options.
> 
> ChenYu
> 
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  arch/arm/configs/sunxi_defconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
>> index 400eb9366a66..456988dd7b0e 100644
>> --- a/arch/arm/configs/sunxi_defconfig
>> +++ b/arch/arm/configs/sunxi_defconfig
>> @@ -127,6 +127,8 @@ CONFIG_DMADEVICES=y
>>  CONFIG_DMA_SUN6I=y
>>  # CONFIG_IOMMU_SUPPORT is not set
>>  CONFIG_EXTCON=y
>> +CONFIG_IIO=y
>> +CONFIG_AXP20X_ADC=y
>>  CONFIG_PWM=y
>>  CONFIG_PWM_SUN4I=y
>>  CONFIG_PHY_SUN4I_USB=y
>> --
>> 2.13.3
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Chen-Yu Tsai]

On Mon, Jul 24, 2017 at 2:43 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> Hi all,
>
> I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
>
> On 22/07/2017 04:19, Chen-Yu Tsai wrote:
>> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>>> The APX20X_POWER option, that is a dependency of the USB controllers
>>> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
>>> back.
>>
>> The statement is only half true. AXP20X_POWER can fall back to reading
>> the ADC registers directly if AXP20X_ADC is not enabled.
>
> For any PMIC using "x-powers,axp202-usb-power-supply" compatible, that
> is true, the others imperatively use IIO channels as there was no prior
> support to this driver.
>
> The switch between reading IIO channels or directly reading the
> registers is done at probe by checking IS_ENABLED(AXP20X_ADC)
> (IS_ENABLED returns 1 when Kconfig is set to y or m). The goal was to
> not break the existing system when users would update to a newer kernel
> (as they may not have selected AXP20X_ADC).
>
> Since there is no hard dependency between AXP20X_ADC and AXP20X_POWER, I
> think we may have a problem when AXP20X_ADC is built as a module. Then
> AXP20X_POWER probing is deferred since it waits for the IIO channels.
> Also, if you do a modprobe on AXP20X_POWER it'll not probe AXP20X_ADC
> because the dependency is not declared. I don't know if there is a way
> to declare this dependency programatically only when AXP20X_ADC is built
> as a module?

There's request_module(), which actually asks the kernel to load a
module, or MODULE_SOFTDEP, which looks like an additional tag for
modprobe. The former gives you more control. The latter doesn't seem
to be used a lot.

>
>> Is there a plan
>> to remove this feature?
>>
>
> As said above (and in the commit log[1]), I added the condition for
> backward compatibility so anyone could still use AXP20X_POWER without
> the ADC, but maybe that is irrelevant? This could solve a bit of the
> "complexity" of the driver, now that we know we want to enforce
> AXP20X_ADC to be built with AXP20X_POWER. I'm obviously okay to remove
> this feature if my point of backward compatibility is irrelevant.

It was a good idea, considering we haven't added the new symbols to
any of the defconfigs. Alternatively, we could have all the AXP20X
sub-drivers have "default MFD_AXP20X". This way no one forgets to
enable new drivers.

>
> But now, we still have the problem that AXP20X_ADC is not needed for
> AXP20X_POWER for AXP22X (and those that don't have voltage/current
> sensing). We don't really need to force AXP20X_ADC to be built when the
> user want AXP20X_POWER.
>
> 1) We force AXP20X_ADC to be built whenever AXP20X_POWER is built, even
> for PMICs that don't have any voltage/current sensing. This way, we can
> enforce the dependency in Kconfig and everything's easy,
>
> 2) We programmatically set the dependency between AXP20X_ADC and
> AXP20X_POWER depending on the compatible used (AXP20X set the
> dependency, not AXP22X).

We could also look at it this way: does the user need/want the additional
voltage/current information? If not, then even IIO is not a requirement.
We could wrap all the related blocks in "if (IS_ENABLED(CONFIG_IIO))",
and also trim the exposed power supply properties along with.

The hard dependency can then be dropped to a soft "implies".

I have to say your first option is easier.

Regards
ChenYu

>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33863c938caa2538397170f1a355885f1ea1564e
>
> Thanks,
> Quentin
>
>> Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
>> not the later ones that don't have voltage/current sensing. I would
>> like to change the hard dependency on AXP20X_ADC to the "imply"
>> keyword in Kconfig for the AC and battery power supply drivers.
>> That is another discussion though.
>>
>> So I think the description should be something like
>>
>>     AXP20X_POWER depends on IIO. Even though it does not depend on
>>     AXP20X_ADC, it is the new, preferred way of getting power supply
>>     readings. The other AXP power supply drivers use it as well.
>>
>>     Enable both options.
>>
>> ChenYu
>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>>  arch/arm/configs/sunxi_defconfig | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
>>> index 400eb9366a66..456988dd7b0e 100644
>>> --- a/arch/arm/configs/sunxi_defconfig
>>> +++ b/arch/arm/configs/sunxi_defconfig
>>> @@ -127,6 +127,8 @@ CONFIG_DMADEVICES=y
>>>  CONFIG_DMA_SUN6I=y
>>>  # CONFIG_IOMMU_SUPPORT is not set
>>>  CONFIG_EXTCON=y
>>> +CONFIG_IIO=y
>>> +CONFIG_AXP20X_ADC=y
>>>  CONFIG_PWM=y
>>>  CONFIG_PWM_SUN4I=y
>>>  CONFIG_PHY_SUN4I_USB=y
>>> --
>>> 2.13.3
>>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
> --
> Quentin Schulz, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Maxime Ripard]

On Mon, Jul 24, 2017 at 08:43:56AM +0200, Quentin Schulz wrote:
> Hi all,
> 
> I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
> 
> On 22/07/2017 04:19, Chen-Yu Tsai wrote:
> > On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> >> The APX20X_POWER option, that is a dependency of the USB controllers
> >> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
> >> back.
> > 
> > The statement is only half true. AXP20X_POWER can fall back to reading
> > the ADC registers directly if AXP20X_ADC is not enabled.
> 
> For any PMIC using "x-powers,axp202-usb-power-supply" compatible, that
> is true, the others imperatively use IIO channels as there was no prior
> support to this driver.
> 
> The switch between reading IIO channels or directly reading the
> registers is done at probe by checking IS_ENABLED(AXP20X_ADC)
> (IS_ENABLED returns 1 when Kconfig is set to y or m). The goal was to
> not break the existing system when users would update to a newer kernel
> (as they may not have selected AXP20X_ADC).
> 
> Since there is no hard dependency between AXP20X_ADC and AXP20X_POWER, I
> think we may have a problem when AXP20X_ADC is built as a module. Then
> AXP20X_POWER probing is deferred since it waits for the IIO channels.
> Also, if you do a modprobe on AXP20X_POWER it'll not probe AXP20X_ADC
> because the dependency is not declared. I don't know if there is a way
> to declare this dependency programatically only when AXP20X_ADC is built
> as a module?
> 
> > Is there a plan
> > to remove this feature?
> > 
> 
> As said above (and in the commit log[1]), I added the condition for
> backward compatibility so anyone could still use AXP20X_POWER without
> the ADC, but maybe that is irrelevant?

The only compatibility we care about is the DT one. We don't care
about config options.

> This could solve a bit of the "complexity" of the driver, now that
> we know we want to enforce AXP20X_ADC to be built with
> AXP20X_POWER. I'm obviously okay to remove this feature if my point
> of backward compatibility is irrelevant.
> 
> But now, we still have the problem that AXP20X_ADC is not needed for
> AXP20X_POWER for AXP22X (and those that don't have voltage/current
> sensing). We don't really need to force AXP20X_ADC to be built when the
> user want AXP20X_POWER.
> 
> 1) We force AXP20X_ADC to be built whenever AXP20X_POWER is built, even
> for PMICs that don't have any voltage/current sensing. This way, we can
> enforce the dependency in Kconfig and everything's easy,

That's backward. The dependency is from AXP20X_POWER to
AXP20X_ADC. That's it, we should express it as such.

> 2) We programmatically set the dependency between AXP20X_ADC and
> AXP20X_POWER depending on the compatible used (AXP20X set the
> dependency, not AXP22X).

Why not just adding a depends on?

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/eca59c21/attachment.sig>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Quentin Schulz]

Hi Maxime,

On 24/07/2017 10:10, Maxime Ripard wrote:
> On Mon, Jul 24, 2017 at 08:43:56AM +0200, Quentin Schulz wrote:
>> Hi all,
>>
>> I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
>>
>> On 22/07/2017 04:19, Chen-Yu Tsai wrote:
>>> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
>>> <maxime.ripard@free-electrons.com> wrote:
>>>> The APX20X_POWER option, that is a dependency of the USB controllers
>>>> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
>>>> back.
>>>
>>> The statement is only half true. AXP20X_POWER can fall back to reading
>>> the ADC registers directly if AXP20X_ADC is not enabled.
>>
>> For any PMIC using "x-powers,axp202-usb-power-supply" compatible, that
>> is true, the others imperatively use IIO channels as there was no prior
>> support to this driver.
>>
>> The switch between reading IIO channels or directly reading the
>> registers is done at probe by checking IS_ENABLED(AXP20X_ADC)
>> (IS_ENABLED returns 1 when Kconfig is set to y or m). The goal was to
>> not break the existing system when users would update to a newer kernel
>> (as they may not have selected AXP20X_ADC).
>>
>> Since there is no hard dependency between AXP20X_ADC and AXP20X_POWER, I
>> think we may have a problem when AXP20X_ADC is built as a module. Then
>> AXP20X_POWER probing is deferred since it waits for the IIO channels.
>> Also, if you do a modprobe on AXP20X_POWER it'll not probe AXP20X_ADC
>> because the dependency is not declared. I don't know if there is a way
>> to declare this dependency programatically only when AXP20X_ADC is built
>> as a module?
>>
>>> Is there a plan
>>> to remove this feature?
>>>
>>
>> As said above (and in the commit log[1]), I added the condition for
>> backward compatibility so anyone could still use AXP20X_POWER without
>> the ADC, but maybe that is irrelevant?
> 
> The only compatibility we care about is the DT one. We don't care
> about config options.
> 

ACK.

>> This could solve a bit of the "complexity" of the driver, now that
>> we know we want to enforce AXP20X_ADC to be built with
>> AXP20X_POWER. I'm obviously okay to remove this feature if my point
>> of backward compatibility is irrelevant.
>>
>> But now, we still have the problem that AXP20X_ADC is not needed for
>> AXP20X_POWER for AXP22X (and those that don't have voltage/current
>> sensing). We don't really need to force AXP20X_ADC to be built when the
>> user want AXP20X_POWER.
>>
>> 1) We force AXP20X_ADC to be built whenever AXP20X_POWER is built, even
>> for PMICs that don't have any voltage/current sensing. This way, we can
>> enforce the dependency in Kconfig and everything's easy,
> 
> That's backward. The dependency is from AXP20X_POWER to
> AXP20X_ADC. That's it, we should express it as such.
> 

You want AXP20X_POWER to depends on AXP20X_ADC (AXP20X_ADC built when
AXP20X_POWER is built, which is what I said). We agree on the same thing
here.

>> 2) We programmatically set the dependency between AXP20X_ADC and
>> AXP20X_POWER depending on the compatible used (AXP20X set the
>> dependency, not AXP22X).
> 
> Why not just adding a depends on?
> 

Because there is no dependency for AXP22X PMICs' USB power supply driver
on AXP20X_ADC. AXP22X PMICs do not receive any information from the ADC.

Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/77511074/attachment.sig>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Maxime Ripard]

On Mon, Jul 24, 2017 at 10:41:05AM +0200, Quentin Schulz wrote:
> Hi Maxime,
> 
> On 24/07/2017 10:10, Maxime Ripard wrote:
> > On Mon, Jul 24, 2017 at 08:43:56AM +0200, Quentin Schulz wrote:
> >> Hi all,
> >>
> >> I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
> >>
> >> On 22/07/2017 04:19, Chen-Yu Tsai wrote:
> >>> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> >>> <maxime.ripard@free-electrons.com> wrote:
> >>>> The APX20X_POWER option, that is a dependency of the USB controllers
> >>>> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
> >>>> back.
> >>>
> >>> The statement is only half true. AXP20X_POWER can fall back to reading
> >>> the ADC registers directly if AXP20X_ADC is not enabled.
> >>
> >> For any PMIC using "x-powers,axp202-usb-power-supply" compatible, that
> >> is true, the others imperatively use IIO channels as there was no prior
> >> support to this driver.
> >>
> >> The switch between reading IIO channels or directly reading the
> >> registers is done at probe by checking IS_ENABLED(AXP20X_ADC)
> >> (IS_ENABLED returns 1 when Kconfig is set to y or m). The goal was to
> >> not break the existing system when users would update to a newer kernel
> >> (as they may not have selected AXP20X_ADC).
> >>
> >> Since there is no hard dependency between AXP20X_ADC and AXP20X_POWER, I
> >> think we may have a problem when AXP20X_ADC is built as a module. Then
> >> AXP20X_POWER probing is deferred since it waits for the IIO channels.
> >> Also, if you do a modprobe on AXP20X_POWER it'll not probe AXP20X_ADC
> >> because the dependency is not declared. I don't know if there is a way
> >> to declare this dependency programatically only when AXP20X_ADC is built
> >> as a module?
> >>
> >>> Is there a plan
> >>> to remove this feature?
> >>>
> >>
> >> As said above (and in the commit log[1]), I added the condition for
> >> backward compatibility so anyone could still use AXP20X_POWER without
> >> the ADC, but maybe that is irrelevant?
> > 
> > The only compatibility we care about is the DT one. We don't care
> > about config options.
> > 
> 
> ACK.
> 
> >> This could solve a bit of the "complexity" of the driver, now that
> >> we know we want to enforce AXP20X_ADC to be built with
> >> AXP20X_POWER. I'm obviously okay to remove this feature if my point
> >> of backward compatibility is irrelevant.
> >>
> >> But now, we still have the problem that AXP20X_ADC is not needed for
> >> AXP20X_POWER for AXP22X (and those that don't have voltage/current
> >> sensing). We don't really need to force AXP20X_ADC to be built when the
> >> user want AXP20X_POWER.
> >>
> >> 1) We force AXP20X_ADC to be built whenever AXP20X_POWER is built, even
> >> for PMICs that don't have any voltage/current sensing. This way, we can
> >> enforce the dependency in Kconfig and everything's easy,
> > 
> > That's backward. The dependency is from AXP20X_POWER to
> > AXP20X_ADC. That's it, we should express it as such.
> > 
> 
> You want AXP20X_POWER to depends on AXP20X_ADC (AXP20X_ADC built when
> AXP20X_POWER is built, which is what I said). We agree on the same thing
> here.

Not really, or at least it's not what it sounded like. "Forcing
AXP20X_ADC to be built whenever AXP20X_POWER is built" is a select
dependency, not a depends on.

Anyway... it's great to know that we were both thinking to add a
depends on.

> >> 2) We programmatically set the dependency between AXP20X_ADC and
> >> AXP20X_POWER depending on the compatible used (AXP20X set the
> >> dependency, not AXP22X).
> > 
> > Why not just adding a depends on?
> > 
> 
> Because there is no dependency for AXP22X PMICs' USB power supply driver
> on AXP20X_ADC. AXP22X PMICs do not receive any information from the ADC.

You won't be able to deal with all cases properly...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/f3f736d5/attachment-0001.sig>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Maxime Ripard]

Hi,

On Sat, Jul 22, 2017 at 10:19:08AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The APX20X_POWER option, that is a dependency of the USB controllers
> > depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
> > back.
> 
> The statement is only half true. AXP20X_POWER can fall back to reading
> the ADC registers directly if AXP20X_ADC is not enabled. Is there a plan
> to remove this feature?

I never really considered this a feature, but some compatibility for
older DTs to be honest :)

> Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
> not the later ones that don't have voltage/current sensing. I would
> like to change the hard dependency on AXP20X_ADC to the "imply"
> keyword in Kconfig for the AC and battery power supply drivers.

I'm not sure. It's also needed when you want to read the battery
voltage and current, or the AC-IN voltage. And that's a feature found
on all PMICs.

And since the battery and the AC-IN power supplies also have a
dependency on AXP20X_ADC, I guess we can just add a depnds on. In most
configuration, it's going to be enabled anyway.

> That is another discussion though.

Indeed.

> So I think the description should be something like
> 
>     AXP20X_POWER depends on IIO. Even though it does not depend on
>     AXP20X_ADC, it is the new, preferred way of getting power supply
>     readings. The other AXP power supply drivers use it as well.
> 
>     Enable both options.

That works for me.

With that comment, do I have your Acked-By?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/bed2243b/attachment.sig>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Chen-Yu Tsai]

On Mon, Jul 24, 2017 at 4:18 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Sat, Jul 22, 2017 at 10:19:08AM +0800, Chen-Yu Tsai wrote:
>> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The APX20X_POWER option, that is a dependency of the USB controllers
>> > depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
>> > back.
>>
>> The statement is only half true. AXP20X_POWER can fall back to reading
>> the ADC registers directly if AXP20X_ADC is not enabled. Is there a plan
>> to remove this feature?
>
> I never really considered this a feature, but some compatibility for
> older DTs to be honest :)
>
>> Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
>> not the later ones that don't have voltage/current sensing. I would
>> like to change the hard dependency on AXP20X_ADC to the "imply"
>> keyword in Kconfig for the AC and battery power supply drivers.
>
> I'm not sure. It's also needed when you want to read the battery
> voltage and current, or the AC-IN voltage. And that's a feature found
> on all PMICs.
>
> And since the battery and the AC-IN power supplies also have a
> dependency on AXP20X_ADC, I guess we can just add a depnds on. In most
> configuration, it's going to be enabled anyway.
>
>> That is another discussion though.
>
> Indeed.
>
>> So I think the description should be something like
>>
>>     AXP20X_POWER depends on IIO. Even though it does not depend on
>>     AXP20X_ADC, it is the new, preferred way of getting power supply
>>     readings. The other AXP power supply drivers use it as well.
>>
>>     Enable both options.
>
> That works for me.
>
> With that comment, do I have your Acked-By?

Yes. We'll figure out the rest later on.

Thanks
ChenYu

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Maxime Ripard]

On Mon, Jul 24, 2017 at 04:22:09PM +0800, Chen-Yu Tsai wrote:
> On Mon, Jul 24, 2017 at 4:18 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Sat, Jul 22, 2017 at 10:19:08AM +0800, Chen-Yu Tsai wrote:
> >> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > The APX20X_POWER option, that is a dependency of the USB controllers
> >> > depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
> >> > back.
> >>
> >> The statement is only half true. AXP20X_POWER can fall back to reading
> >> the ADC registers directly if AXP20X_ADC is not enabled. Is there a plan
> >> to remove this feature?
> >
> > I never really considered this a feature, but some compatibility for
> > older DTs to be honest :)
> >
> >> Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
> >> not the later ones that don't have voltage/current sensing. I would
> >> like to change the hard dependency on AXP20X_ADC to the "imply"
> >> keyword in Kconfig for the AC and battery power supply drivers.
> >
> > I'm not sure. It's also needed when you want to read the battery
> > voltage and current, or the AC-IN voltage. And that's a feature found
> > on all PMICs.
> >
> > And since the battery and the AC-IN power supplies also have a
> > dependency on AXP20X_ADC, I guess we can just add a depnds on. In most
> > configuration, it's going to be enabled anyway.
> >
> >> That is another discussion though.
> >
> > Indeed.
> >
> >> So I think the description should be something like
> >>
> >>     AXP20X_POWER depends on IIO. Even though it does not depend on
> >>     AXP20X_ADC, it is the new, preferred way of getting power supply
> >>     readings. The other AXP power supply drivers use it as well.
> >>
> >>     Enable both options.
> >
> > That works for me.
> >
> > With that comment, do I have your Acked-By?
> 
> Yes. We'll figure out the rest later on.

Applied with that commit log, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/53283b0f/attachment.sig>

[PATCH 3/4] arm: sunxi: Add AXP20X_ADC

[Quentin Schulz]

Hi Maxime,

On 24/07/2017 10:18, Maxime Ripard wrote:
> Hi,
> 
> On Sat, Jul 22, 2017 at 10:19:08AM +0800, Chen-Yu Tsai wrote:
>> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>>> The APX20X_POWER option, that is a dependency of the USB controllers
>>> depends on IIO and AXP20X_ADC. Add it so that we can get a working USB
>>> back.
>>
>> The statement is only half true. AXP20X_POWER can fall back to reading
>> the ADC registers directly if AXP20X_ADC is not enabled. Is there a plan
>> to remove this feature?
> 
> I never really considered this a feature, but some compatibility for
> older DTs to be honest :)
> 

Have nothing to do with backward compatibility for DT as the DT hasn't
changed. The IIO channels are exposed via an IIO map in the ADC driver
and retrieved from the map in the USB power supply driver. No need for
DT, no changes in DT.

Quentin

>> Also, AXP20X_ADC is only really needed if the hardware is AXP20x, and
>> not the later ones that don't have voltage/current sensing. I would
>> like to change the hard dependency on AXP20X_ADC to the "imply"
>> keyword in Kconfig for the AC and battery power supply drivers.
> 
> I'm not sure. It's also needed when you want to read the battery
> voltage and current, or the AC-IN voltage. And that's a feature found
> on all PMICs.
>> And since the battery and the AC-IN power supplies also have a
> dependency on AXP20X_ADC, I guess we can just add a depnds on. In most
> configuration, it's going to be enabled anyway.
> 
>> That is another discussion though.
> 
> Indeed.
> 
>> So I think the description should be something like
>>
>>     AXP20X_POWER depends on IIO. Even though it does not depend on
>>     AXP20X_ADC, it is the new, preferred way of getting power supply
>>     readings. The other AXP power supply drivers use it as well.
>>
>>     Enable both options.
> 
> That works for me.
> 
> With that comment, do I have your Acked-By?
> 
> Maxime
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/b1c053d3/attachment.sig>

[PATCH 4/4] arm: sunxi: Add additional power supplies

[Maxime Ripard]

A bunch of new power supplies have been added recently to handle the
batteries and the AC-IN plugs. Add them to our defconfig.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/sunxi_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 456988dd7b0e..0d29834b9688 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -77,6 +77,8 @@ CONFIG_SPI_SUN4I=y
 CONFIG_SPI_SUN6I=y
 CONFIG_GPIO_SYSFS=y
 CONFIG_POWER_SUPPLY=y
+CONFIG_CHARGER_AXP20X=y
+CONFIG_BATTERY_AXP20X=y
 CONFIG_AXP20X_POWER=y
 CONFIG_THERMAL=y
 CONFIG_CPU_THERMAL=y
-- 
2.13.3

[PATCH 4/4] arm: sunxi: Add additional power supplies

[Chen-Yu Tsai]

On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> A bunch of new power supplies have been added recently to handle the
> batteries and the AC-IN plugs. Add them to our defconfig.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

[PATCH 4/4] arm: sunxi: Add additional power supplies

[Maxime Ripard]

On Sat, Jul 22, 2017 at 10:20:58AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > A bunch of new power supplies have been added recently to handle the
> > batteries and the AC-IN plugs. Add them to our defconfig.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170724/4ac06547/attachment.sig>

[PATCH 1/4] arm: multi_v7: Add AXP20X_POWER

[Chen-Yu Tsai]

On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The multi_v7 defconfig is missing the AXP209 USB power supply driver,
> resulting in non-working USB controllers because the USB PHY will always
> return EPROBE_DEFER.
>
> Make sure it's selected.

This seems weird. You only change it from a module to built-in.
AFAIK, the module does have proper module aliases, and the mfd
core does give out proper uevents for modaliases.

Is the module not getting loaded? Or is it loaded too late?

ChenYu

>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/configs/multi_v7_defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 4d19c1b4b8e7..6bab339a8dd8 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -432,6 +432,7 @@ CONFIG_GPIO_TPS6586X=y
>  CONFIG_GPIO_TPS65910=y
>  CONFIG_BATTERY_ACT8945A=y
>  CONFIG_BATTERY_SBS=y
> +CONFIG_AXP20X_POWER=y
>  CONFIG_BATTERY_MAX17040=m
>  CONFIG_BATTERY_MAX17042=m
>  CONFIG_CHARGER_MAX14577=m
> @@ -439,7 +440,6 @@ CONFIG_CHARGER_MAX77693=m
>  CONFIG_CHARGER_MAX8997=m
>  CONFIG_CHARGER_MAX8998=m
>  CONFIG_CHARGER_TPS65090=y
> -CONFIG_AXP20X_POWER=m
>  CONFIG_POWER_RESET_AS3722=y
>  CONFIG_POWER_RESET_GPIO=y
>  CONFIG_POWER_RESET_GPIO_RESTART=y
> --
> 2.13.3
>

[PATCH 1/4] arm: multi_v7: Add AXP20X_POWER

[Maxime Ripard]

On Mon, Jul 24, 2017 at 04:14:17PM +0800, Chen-Yu Tsai wrote:
> On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The multi_v7 defconfig is missing the AXP209 USB power supply driver,
> > resulting in non-working USB controllers because the USB PHY will always
> > return EPROBE_DEFER.
> >
> > Make sure it's selected.
> 
> This seems weird. You only change it from a module to built-in.
> AFAIK, the module does have proper module aliases, and the mfd
> core does give out proper uevents for modaliases.
> 
> Is the module not getting loaded? Or is it loaded too late?

Nevermind, after reading this back, the commit log is wrong, like you
suggested. The only odd thing is that everything else in order to have
USB working without modules is there but the PHY.

But I guess it's fine.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170725/07cba3fc/attachment.sig>