Re: [PATCH net-next 1/4] net: ti: Kconfig: Select HSR for ICSSG Driver

[MD Danish Anwar <danishanwar@ti.com>]

On 18/12/24 10:29 pm, Larysa Zaremba wrote:
> On Mon, Dec 16, 2024 at 03:30:41PM +0530, MD Danish Anwar wrote:
>> HSR offloading is supported by ICSSG driver. Select the symbol HSR for
>> TI_ICSSG_PRUETH. Also select NET_SWITCHDEV instead of depending on it to
>> remove recursive dependency.
>>
> 
> 2 things:
> 1) The explanation from the cover should have been included in the commit 
>    message.

I wanted to keep the commit message brief so I provided the actual
errors in cover letter. I will add the logs here as well.

> 2) Why not `depends on HSR`?

Adding `depends on HSR` in `config TI_ICSSG_PRUETH` is not setting HSR.
I have tried below scenarios and only one of them work.

1) depends on NET_SWITCHDEV
   depends on HSR

	HSR doesn't get set in .config - `# CONFIG_HSR is not set`. Even the
CONFIG_TI_ICSSG_PRUETH also gets unset although this is set to =m in
defconfig. But keeping both as `depends on` makes CONFIG_TI_ICSSG_PRUETH
disabled.

2) select NET_SWITCHDEV
   depends on HSR

	HSR doesn't get set in .config - `# CONFIG_HSR is not set`. Even the
CONFIG_TI_ICSSG_PRUETH also gets unset although this is set to =m in
defconfig. But keeping both as `depends on` makes CONFIG_TI_ICSSG_PRUETH
disabled.

3) depends on NET_SWITCHDEV
   select HSR
	
	Results in recursive dependency

error: recursive dependency detected!
	symbol NET_DSA depends on HSR
	symbol HSR is selected by TI_ICSSG_PRUETH
	symbol TI_ICSSG_PRUETH depends on NET_SWITCHDEV
	symbol NET_SWITCHDEV is selected by NET_DSA
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"

make[2]: *** [scripts/kconfig/Makefile:95: defconfig] Error 1
make[1]: *** [/home/danish/workspace/net-next/Makefile:733: defconfig]
Error 2
make: *** [Makefile:251: __sub-make] Error 2

4) select NET_SWITCHDEV
   select HSR

	HSR is set as `m` along with `CONFIG_TI_ICSSG_PRUETH`

CONFIG_HSR=m
CONFIG_NET_SWITCHDEV=y
CONFIG_TI_ICSSG_PRUETH=m

#4 is the only secnario where HSR gets built. That's why I sent the
patch with `select NET_SWITCHDEV` and `select HSR`

>  
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  drivers/net/ethernet/ti/Kconfig | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
>> index 0d5a862cd78a..ad366abfa746 100644
>> --- a/drivers/net/ethernet/ti/Kconfig
>> +++ b/drivers/net/ethernet/ti/Kconfig
>> @@ -187,8 +187,9 @@ config TI_ICSSG_PRUETH
>>  	select PHYLIB
>>  	select TI_ICSS_IEP
>>  	select TI_K3_CPPI_DESC_POOL
>> +	select NET_SWITCHDEV
>> +	select HSR
>>  	depends on PRU_REMOTEPROC
>> -	depends on NET_SWITCHDEV
>>  	depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER
>>  	depends on PTP_1588_CLOCK_OPTIONAL
>>  	help
>> -- 
>> 2.34.1
>>
>>

-- 
Thanks and Regards,
Danish