Re: [PATCH 08/11] Kconfig: i2c: Depend on correct xPL symbol

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 5886 bytes --]

On Sat, Jul 11, 2026 at 01:52:26AM +0200, Jonas Karlman wrote:
> Hi Tom,
> 
> On 7/10/2026 5:56 PM, Tom Rini wrote:
> > On Fri, Jul 10, 2026 at 05:39:08PM +0200, Jonas Karlman wrote:
> >> Hi Tom,
> >>
> >> On 7/10/2026 3:50 PM, Tom Rini wrote:
> >>> On Thu, Jul 09, 2026 at 09:12:24AM +0200, Jonas Karlman wrote:
> >>>> Hi Tom,
> >>>>
> >>>> On 7/9/2026 3:52 AM, Tom Rini wrote:
> >>>>> On Wed, Jul 08, 2026 at 10:05:34PM +0000, Jonas Karlman wrote:
> >>>>>
> >>>>>> Change to depend on the xPL symbol instead of the SUPPORT_xPL symbol to
> >>>>>> only make the xPL_SYS_I2C_LEGACY symbols available when xPL is enabled.
> >>>>>>
> >>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >>>>>> ---
> >>>>>>  drivers/i2c/Kconfig | 4 ++--
> >>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> >>>>>> index ab5af17858c8..125dc84819e6 100644
> >>>>>> --- a/drivers/i2c/Kconfig
> >>>>>> +++ b/drivers/i2c/Kconfig
> >>>>>> @@ -78,14 +78,14 @@ config SYS_I2C_LEGACY
> >>>>>>  
> >>>>>>  config SPL_SYS_I2C_LEGACY
> >>>>>>  	bool "Enable legacy I2C subsystem and drivers in SPL"
> >>>>>> -	depends on SUPPORT_SPL && !SPL_DM_I2C
> >>>>>> +	depends on SPL && !SPL_DM_I2C
> >>>>>>  	help
> >>>>>>  	  Enable the legacy I2C subsystem and drivers in SPL.  This is useful
> >>>>>>  	  in some size constrained situations.
> >>>>>>  
> >>>>>>  config TPL_SYS_I2C_LEGACY
> >>>>>>  	bool "Enable legacy I2C subsystem and drivers in TPL"
> >>>>>> -	depends on SUPPORT_TPL && !SPL_DM_I2C
> >>>>>> +	depends on TPL && !TPL_DM_I2C
> >>>>>>  	help
> >>>>>>  	  Enable the legacy I2C subsystem and drivers in TPL.  This is useful
> >>>>>>  	  in some size constrained situations.
> >>>>>
> >>>>> Building on the last email, I think these might need to be
> >>>>> [ST]PL_FRAMEWORK not [ST]PL.
> >>>>>
> >>>>
> >>>> I have mostly tested this series with additional changes that alias
> >>>> putc()/puts() to printch()/printascii() in xPL builds when
> >>>> common/console.o is unavailable and DEBUG_UART=y, patches that will
> >>>> be sent as a separate series.
> >>>>
> >>>> It is possible such change may have hidden some latent issues, now
> >>>> trying to push a branch that contains changes that ends at the patches
> >>>> included in this series, but due to timeouts it was not possible prior
> >>>> to sending this reply.
> >>>>
> >>>> This patch can also be dropped if it causes any issues, I only added it
> >>>> after a search for SUPPORT_xPL and noticed that these depends seem to be
> >>>> incorrect. SUPPORT_xPL is used to tell if xPL can be enabled or not,
> >>>> not if xPL is enabled/disabled.
> >>>
> >>> I don't want to drop this, but there are a handful of platforms that
> >>> change:
> >>> 10: Kconfig: i2c: Depend on correct xPL symbol
> >>>    aarch64: (for 1/1 boards) all -60.0 text -60.0
> >>>             ls1043aqds_tfa : all -60 text -60
> >>>                u-boot: add: 0/-1, grow: 0/-1 bytes: 0/-60 (-60)
> >>>                  function                                   old     new   delta
> >>>                  board_early_init_f                          68      64      -4
> >>>                  i2c_early_init_f                            56       -     -56
> >>>
> >>> Can you please look in to it and see what needs to be changed? Thanks.
> >>
> >> From a quick look this size change may be caused due to use of an
> >> 'imply SPL_SYS_I2C_LEGACY' instead of using an 'imply SYS_I2C_LEGACY'
> >> in arch/arm/cpu/armv8/fsl-layerscape/Kconfig.
> >>
> >> 'config MACH_SUN8I_R40' is another platform that also possible miss
> >> an 'imply SYS_I2C_LEGACY' to match other similar sunxi platforms.
> 
> I have now done some more digging and it looks like SYS_I2C_EARLY_INIT
> is the culprit.
> 
>   config SYS_I2C_EARLY_INIT
> 	bool "Enable legacy I2C subsystem early in boot"
> 	depends on BOARD_EARLY_INIT_F && SPL_SYS_I2C_LEGACY && SYS_I2C_MXC
> 	help
> 	  Add the function prototype for i2c_early_init_f which is called in
> 	  board_early_init_f.
> 
> Not sure what to do here, it is possible that just dropping
> SPL_SYS_I2C_LEGACY from depends would ensure the code is restored.

Ah, good work. Yes, this is tricky.

> However, that also feels strange as the intent of i2c_early_init_f() is
> to initialize I2C_LEGACY, and most of the affected boards use DM_I2C:
> 
>   > tools/qconfig.py -f ~SPL SYS_I2C_EARLY_INIT
>   5 matches
>   ls1043aqds_tfa_SECURE_BOOT ls1043aqds_tfa ls1046aqds_tfa_SECURE_BOOT ls1046aqds_tfa ls2080aqds_qspi
> 
>   > tools/qconfig.py -f ~SPL SYS_I2C_EARLY_INIT ~DM_I2C
>   1 matches
>   ls2080aqds_qspi
> 
> (Please note that my qconfig.db does not cover 100% of all defconfigs.)
> 
> All other code seem to be protected by CONFIG_IS_ENABLED(SYS_I2C_LEGACY)
> so I am not expecting any other change for targets that use xPL=n.

This is tricky, yes. Looking at all the boards which *have*
SYS_I2C_EARLY_INIT guards for calling i2c_early_init_f some presumably
used to, but do not now, set SPL_SYS_I2C_LEGACY. And going back to my
initial conversion of this symbol to Kconfig, I'm not sure it's 100%
right, now. I think we need to add SPL_SYS_I2C_EARLY_INIT and make both
of these symbols be "select ..." in the right board Kconfig, rather than
a prompted question. And with that, we should have:
ls1043aqds_tfa_SECURE_BOOT ls1043aqds_tfa ls1046aqds_tfa_SECURE_BOOT ls1046aqds_tfa
be "select SPL_SYS_I2C_EARLY_INIT" and then:
ls2080aqds_nand ls2080aqds_qspi
be doing both "select SPL_SYS_I2C_EARLY_INIT" and "select
SYS_I2C_EARLY_INIT".

There's also (and so I've cc'd Peng) some dead code to remove on other
layerscape platforms that don't call i2c_early_init_f (they're both
full and SPL DM i2c) and so have some dead code now.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]