Discussion:
[PATCH 1/4] arm: multi_v7: Add AXP20X_POWER
Maxime Ripard
2017-07-21 16:20:02 UTC
Permalink
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 <***@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
Maxime Ripard
2017-07-21 16:20:04 UTC
Permalink
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 <***@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
Chen-Yu Tsai
2017-07-22 02:19:08 UTC
Permalink
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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
Post by Maxime Ripard
---
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
Quentin Schulz
2017-07-24 06:43:56 UTC
Permalink
Hi all,

I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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?
Post by Chen-Yu Tsai
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
Post by Chen-Yu Tsai
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
Post by Maxime Ripard
---
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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Chen-Yu Tsai
2017-07-24 07:48:54 UTC
Permalink
On Mon, Jul 24, 2017 at 2:43 PM, Quentin Schulz
Post by Quentin Schulz
Hi all,
I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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.
Post by Quentin Schulz
Post by Chen-Yu Tsai
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.
Post by Quentin Schulz
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
Post by Quentin Schulz
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33863c938caa2538397170f1a355885f1ea1564e
Thanks,
Quentin
Post by Chen-Yu Tsai
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
Post by Maxime Ripard
---
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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Maxime Ripard
2017-07-24 08:10:13 UTC
Permalink
Post by Quentin Schulz
Hi all,
I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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?
Post by Chen-Yu Tsai
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.
Post by Quentin Schulz
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.
Post by Quentin Schulz
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
Quentin Schulz
2017-07-24 08:41:05 UTC
Permalink
Hi Maxime,
Post by Maxime Ripard
Post by Quentin Schulz
Hi all,
I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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?
Post by Chen-Yu Tsai
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.
Post by Maxime Ripard
Post by Quentin Schulz
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.
Post by Maxime Ripard
Post by Quentin Schulz
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
Maxime Ripard
2017-07-24 09:33:16 UTC
Permalink
Post by Quentin Schulz
Hi Maxime,
Post by Maxime Ripard
Post by Quentin Schulz
Hi all,
I've been Cc'ed by Chen-Yu I think, thanks for the heads-up.
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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?
Post by Chen-Yu Tsai
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.
Post by Maxime Ripard
Post by Quentin Schulz
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.
Post by Quentin Schulz
Post by Maxime Ripard
Post by Quentin Schulz
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
Maxime Ripard
2017-07-24 08:18:46 UTC
Permalink
Hi,
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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 :)
Post by Chen-Yu Tsai
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.
Post by Chen-Yu Tsai
That is another discussion though.
Indeed.
Post by Chen-Yu Tsai
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
Chen-Yu Tsai
2017-07-24 08:22:09 UTC
Permalink
On Mon, Jul 24, 2017 at 4:18 PM, Maxime Ripard
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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 :)
Post by Chen-Yu Tsai
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.
Post by Chen-Yu Tsai
That is another discussion though.
Indeed.
Post by Chen-Yu Tsai
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
Maxime Ripard
2017-07-24 09:44:24 UTC
Permalink
Post by Chen-Yu Tsai
On Mon, Jul 24, 2017 at 4:18 PM, Maxime Ripard
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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 :)
Post by Chen-Yu Tsai
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.
Post by Chen-Yu Tsai
That is another discussion though.
Indeed.
Post by Chen-Yu Tsai
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
Quentin Schulz
2017-07-24 08:47:58 UTC
Permalink
Hi Maxime,
Post by Maxime Ripard
Hi,
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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
Post by Maxime Ripard
Post by Chen-Yu Tsai
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.
Post by Chen-Yu Tsai
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.
Post by Chen-Yu Tsai
That is another discussion though.
Indeed.
Post by Chen-Yu Tsai
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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Maxime Ripard
2017-07-21 16:20:05 UTC
Permalink
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 <***@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
Chen-Yu Tsai
2017-07-22 02:20:58 UTC
Permalink
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
Acked-by: Chen-Yu Tsai <***@csie.org>
Maxime Ripard
2017-07-24 08:13:08 UTC
Permalink
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Maxime Ripard
2017-07-21 16:20:03 UTC
Permalink
Update the defconfig with the current state of defaults.

This was done using make sunxi_defconfig; make savedefconfig

Signed-off-by: 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
Chen-Yu Tsai
2017-07-22 02:05:06 UTC
Permalink
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by Maxime Ripard
Update the defconfig with the current state of defaults.
This was done using make sunxi_defconfig; make savedefconfig
Acked-by: Chen-Yu Tsai <***@csie.org>
Maxime Ripard
2017-07-24 08:12:42 UTC
Permalink
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by Maxime Ripard
Update the defconfig with the current state of defaults.
This was done using make sunxi_defconfig; make savedefconfig
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Chen-Yu Tsai
2017-07-24 08:14:17 UTC
Permalink
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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
Post by Maxime Ripard
---
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
Maxime Ripard
2017-07-25 14:30:16 UTC
Permalink
Post by Chen-Yu Tsai
On Sat, Jul 22, 2017 at 12:20 AM, Maxime Ripard
Post by 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.
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
Loading...