[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 at 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>