[PATCH 4/5] dm: adc: Add SPL_ADC Kconfig symbol for use of ADC in SPL

[Jonas Karlman]

Hi Quentin,

On 2024-07-31 14:42, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 7/31/24 8:50 AM, Jonas Karlman wrote:
>  > What model of Radxa ZERO 3W/3E boards can be identified using ADC at
>  > runtime, add a Kconfig symbol to allow use of ADC in SPL.
>  >
>  > This will be used to identify board model in SPL to allow loading
>  > correct FIT configuration and FDT for U-Boot proper at SPL phase.
>  >
>  > Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>  > ---
>  >   drivers/Makefile    | 2 +-
>  >   drivers/adc/Kconfig | 4 ++++
>  >   2 files changed, 5 insertions(+), 1 deletion(-)
>  >
>  > diff --git a/drivers/Makefile b/drivers/Makefile
>  > index 9195dafd37e0..1acd94f3c17e 100644
>  > --- a/drivers/Makefile
>  > +++ b/drivers/Makefile
>  > @@ -1,5 +1,6 @@
>  >   # SPDX-License-Identifier: GPL-2.0+
>  >
>  > +obj-$(CONFIG_$(SPL_TPL_)ADC) += adc/
>  >   obj-$(CONFIG_$(SPL_TPL_)BIOSEMU) += bios_emulator/
>  >   obj-$(CONFIG_$(SPL_TPL_)BLK) += block/
>  >   obj-$(CONFIG_$(SPL_TPL_)BOOTCOUNT_LIMIT) += bootcount/
>  > @@ -81,7 +82,6 @@ endif
>  >
>  >   ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
>  >
>  > -obj-y += adc/
>  >   obj-y += ata/
>  >   obj-$(CONFIG_DM_DEMO) += demo/
>  >   obj-y += block/
>  > diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
>  > index c9cdbe6942de..eb705f9e0fb8 100644
>  > --- a/drivers/adc/Kconfig
>  > +++ b/drivers/adc/Kconfig
>  > @@ -11,6 +11,10 @@ config ADC
>  >          - support supply's phandle with auto-enable
>  >          - supply polarity setting in fdt
>  >
>  > +config SPL_ADC
>  > +     bool "Enable ADC drivers using Driver Model in SPL"
>  > +     depends on ADC
>  > +
> 
> This is just because you didn't modify the drivers/adc/Makefile to have
> obj-$(CONFIG_$(SPL_TPL_)ADC) += adc-uclass.o
> I assume? It's a bit odd to require a "proper" symbol for an SPL symbol.
> 
> Additionally, since you use $(SPL_TPL_) maybe add that TPL symbol too in
> the Kconfig?

Agree, this could have been done differently, I added the depends on ADC
a few minutes before I send the series to ensure next user of ADC in SPL
does not miss it.

The use of $(SPL_TPL_) was to follow rest of the Makefile, and there
are plenty of other places where this is used and a symbol is missing.
Also this helps when next user would like to use ADC in TPL, only the
Kconfig symbol needs to be added.

> 
> Finally, I think it'd be best to have symbols for SPL and TPL for the
> drivers as well and update the Makefile to use $(SPL_TPL_) for those as
> well. I don't see this as being a big issue for ADC specifically right
> now but it's been a pain for me for a few other subsystems (power, pmic,
> i2c, spi, IIRC). This isn't a blocker though I believe.

Yeah, lots of drivers/subsections could use an update to have more
consistency.

For this series I only wanted to include bare minimum change to make it
possible to use adc_channel_single_shot() in SPL to figure out what FIT
config to use.

Full Makefile and Kconfig cleanup should probably be done in a separate
series.

> 
> On a side note, I'm wondering if we're not missing a depends on DM for
> CONFIG_ADC by any chance? c.f. the config option title: "Enable ADC
> drivers using Driver Model"

Probably, guessing no target is using ADC without DM at the moment.

Regards,
Jonas

> 
> Cheers,
> Quentin