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 <jo...@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
