| Message ID | 20240731065048.1199792-5-jonas@kwiboo.se |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | board: rockchip: Add Radxa ZERO 3W/3E | expand |
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@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
Hi Quentin, On 2024-08-01 11:26, Quentin Schulz wrote: > Hi Jonas, > > On 7/31/24 4:10 PM, Jonas Karlman wrote: > > 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@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. > > > > I would rather not depend on a hack if we can avoid it. I believe: > obj-$(CONFIG_$(SPL_TPL_)ADC) += adc-uclass.o > should be enough of a change here? I thinks so to, will change to that in a v2. > > I'm actually wondering if we can't simply have > obj-y += adc-uclass.o > ? considering that this Makefile is only included if > $(CONFIG_$(SPL_TPL_)ADC)? Should be possible, but it is probably better to be explicit in case the condition in drivers/Makefile ever changes in the future. Regards, Jonas > > Cheers, > Quentin
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 + config ADC_EXYNOS bool "Enable Exynos 54xx ADC driver" depends on ADC
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@kwiboo.se> --- drivers/Makefile | 2 +- drivers/adc/Kconfig | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)