From mboxrd@z Thu Jan 1 00:00:00 1970 From: maxime.ripard@free-electrons.com (Maxime Ripard) Date: Mon, 24 Jul 2017 10:10:13 +0200 Subject: [PATCH 3/4] arm: sunxi: Add AXP20X_ADC In-Reply-To: References: <20170721162005.18892-1-maxime.ripard@free-electrons.com> <20170721162005.18892-3-maxime.ripard@free-electrons.com> Message-ID: <20170724081013.uyfaetzkj5iz7uhj@flea> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > > 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: