[net-next,v4,5/5] ARM: mxs_defconfig: Enable CONFIG_FEC_MTIP_L2SW to support MTIP L2 switch

Commit Message

Lukasz Majewski April 7, 2025, 2:51 p.m. UTC

This patch enables support for More Than IP switch available on some
imx28[7] devices.

Signed-off-by: Lukasz Majewski <[e-mail]>
---
Changes for v4:
- New patch
---
 arch/arm/configs/mxs_defconfig | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Stefan Wahren April 9, 2025, 4:01 p.m. UTC | #1

Hi Lukasz,

Am 07.04.25 um 16:51 schrieb Lukasz Majewski:
> This patch enables support for More Than IP switch available on some
> imx28[7] devices.
>
> Signed-off-by: Lukasz Majewski <[e-mail]>
thanks adding the driver to mxs_defconfig. Unfortunately it's not
possible for reviewers to identify the relevant changes, also the commit
messages doesn't provide further information.

In general there are two approaches to solves this:
1) prepend an additional patch which synchronizes mxs_defconfig with
current mainline
2) manually create the relevant changes against mxs_defconfig

The decision about the approaches is up to the maintainer.

Btw driver review will follow ...

Regards
> ---
> Changes for v4:
> - New patch
> ---
>   arch/arm/configs/mxs_defconfig | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig
> index d8a6e43c401e..4dc4306c035f 100644
> --- a/arch/arm/configs/mxs_defconfig
> +++ b/arch/arm/configs/mxs_defconfig
> @@ -32,11 +32,10 @@ CONFIG_INET=y
>   CONFIG_IP_PNP=y
>   CONFIG_IP_PNP_DHCP=y
>   CONFIG_SYN_COOKIES=y
> -# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
> -# CONFIG_INET_XFRM_MODE_TUNNEL is not set
> -# CONFIG_INET_XFRM_MODE_BEET is not set
>   # CONFIG_INET_DIAG is not set
>   # CONFIG_IPV6 is not set
> +CONFIG_BRIDGE=y
> +CONFIG_NET_SWITCHDEV=y
>   CONFIG_CAN=m
>   # CONFIG_WIRELESS is not set
>   CONFIG_DEVTMPFS=y
> @@ -45,7 +44,6 @@ CONFIG_MTD=y
>   CONFIG_MTD_CMDLINE_PARTS=y
>   CONFIG_MTD_BLOCK=y
>   CONFIG_MTD_DATAFLASH=y
> -CONFIG_MTD_M25P80=y
>   CONFIG_MTD_SST25L=y
>   CONFIG_MTD_RAW_NAND=y
>   CONFIG_MTD_NAND_GPMI_NAND=y
> @@ -56,11 +54,11 @@ CONFIG_EEPROM_AT24=y
>   CONFIG_SCSI=y
>   CONFIG_BLK_DEV_SD=y
>   CONFIG_NETDEVICES=y
> +CONFIG_FEC_MTIP_L2SW=y
>   CONFIG_ENC28J60=y
>   CONFIG_ICPLUS_PHY=y
>   CONFIG_MICREL_PHY=y
>   CONFIG_REALTEK_PHY=y
> -CONFIG_SMSC_PHY=y
>   CONFIG_CAN_FLEXCAN=m
>   CONFIG_USB_USBNET=y
>   CONFIG_USB_NET_SMSC95XX=y
> @@ -77,13 +75,11 @@ CONFIG_SERIAL_AMBA_PL011=y
>   CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
>   CONFIG_SERIAL_MXS_AUART=y
>   # CONFIG_HW_RANDOM is not set
> -# CONFIG_I2C_COMPAT is not set
>   CONFIG_I2C_CHARDEV=y
>   CONFIG_I2C_MXS=y
>   CONFIG_SPI=y
>   CONFIG_SPI_GPIO=m
>   CONFIG_SPI_MXS=y
> -CONFIG_GPIO_SYSFS=y
>   # CONFIG_HWMON is not set
>   CONFIG_WATCHDOG=y
>   CONFIG_STMP3XXX_RTC_WATCHDOG=y
> @@ -138,10 +134,6 @@ CONFIG_PWM_MXS=y
>   CONFIG_NVMEM_MXS_OCOTP=y
>   CONFIG_EXT4_FS=y
>   # CONFIG_DNOTIFY is not set
> -CONFIG_NETFS_SUPPORT=m
> -CONFIG_FSCACHE=y
> -CONFIG_FSCACHE_STATS=y
> -CONFIG_CACHEFILES=m
>   CONFIG_VFAT_FS=y
>   CONFIG_TMPFS=y
>   CONFIG_TMPFS_POSIX_ACL=y

Lukasz Majewski April 10, 2025, 7:01 a.m. UTC | #2

Hi Stefan,

> Hi Lukasz,
> 
> Am 07.04.25 um 16:51 schrieb Lukasz Majewski:
> > This patch enables support for More Than IP switch available on some
> > imx28[7] devices.
> >
> > Signed-off-by: Lukasz Majewski <[e-mail]>  
> thanks adding the driver to mxs_defconfig. Unfortunately it's not
> possible for reviewers to identify the relevant changes,

Could you be more specific here?
As fair as I see - there is only 14 LOCs changed for review.

Please also be aware that MTIP L2 switch driver has some dependencies -
on e.g. SWITCHDEV and BRIDGE, which had to be enabled to allow the
former one to be active.

> also the
> commit messages doesn't provide further information.
> 

What kind of extra information shall I provide? IMHO the patch is
self-explaining.

> In general there are two approaches to solves this:
> 1) prepend an additional patch which synchronizes mxs_defconfig with
> current mainline
> 2) manually create the relevant changes against mxs_defconfig
> 
> The decision about the approaches is up to the maintainer.

I took the linux-next's (or net-next) mxs defconfig (cp it to be
.config)

Then run CROSS_COMPILE= ... make ARCH=arm menuconfig
Enabled all the relevant Kconfig options and run

CROSS_COMPILE= ... make ARCH=arm savedefconfig
and copy defconfig to mxs_defconfig.
Then I used git to prepare the patch.

Isn't the above procedure correct?


> 
> Btw driver review will follow ...
> 
> Regards
> > ---
> > Changes for v4:
> > - New patch
> > ---
> >   arch/arm/configs/mxs_defconfig | 14 +++-----------
> >   1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm/configs/mxs_defconfig
> > b/arch/arm/configs/mxs_defconfig index d8a6e43c401e..4dc4306c035f
> > 100644 --- a/arch/arm/configs/mxs_defconfig
> > +++ b/arch/arm/configs/mxs_defconfig
> > @@ -32,11 +32,10 @@ CONFIG_INET=y
> >   CONFIG_IP_PNP=y
> >   CONFIG_IP_PNP_DHCP=y
> >   CONFIG_SYN_COOKIES=y
> > -# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
> > -# CONFIG_INET_XFRM_MODE_TUNNEL is not set
> > -# CONFIG_INET_XFRM_MODE_BEET is not set
> >   # CONFIG_INET_DIAG is not set
> >   # CONFIG_IPV6 is not set
> > +CONFIG_BRIDGE=y
> > +CONFIG_NET_SWITCHDEV=y
> >   CONFIG_CAN=m
> >   # CONFIG_WIRELESS is not set
> >   CONFIG_DEVTMPFS=y
> > @@ -45,7 +44,6 @@ CONFIG_MTD=y
> >   CONFIG_MTD_CMDLINE_PARTS=y
> >   CONFIG_MTD_BLOCK=y
> >   CONFIG_MTD_DATAFLASH=y
> > -CONFIG_MTD_M25P80=y
> >   CONFIG_MTD_SST25L=y
> >   CONFIG_MTD_RAW_NAND=y
> >   CONFIG_MTD_NAND_GPMI_NAND=y
> > @@ -56,11 +54,11 @@ CONFIG_EEPROM_AT24=y
> >   CONFIG_SCSI=y
> >   CONFIG_BLK_DEV_SD=y
> >   CONFIG_NETDEVICES=y
> > +CONFIG_FEC_MTIP_L2SW=y
> >   CONFIG_ENC28J60=y
> >   CONFIG_ICPLUS_PHY=y
> >   CONFIG_MICREL_PHY=y
> >   CONFIG_REALTEK_PHY=y
> > -CONFIG_SMSC_PHY=y
> >   CONFIG_CAN_FLEXCAN=m
> >   CONFIG_USB_USBNET=y
> >   CONFIG_USB_NET_SMSC95XX=y
> > @@ -77,13 +75,11 @@ CONFIG_SERIAL_AMBA_PL011=y
> >   CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
> >   CONFIG_SERIAL_MXS_AUART=y
> >   # CONFIG_HW_RANDOM is not set
> > -# CONFIG_I2C_COMPAT is not set
> >   CONFIG_I2C_CHARDEV=y
> >   CONFIG_I2C_MXS=y
> >   CONFIG_SPI=y
> >   CONFIG_SPI_GPIO=m
> >   CONFIG_SPI_MXS=y
> > -CONFIG_GPIO_SYSFS=y
> >   # CONFIG_HWMON is not set
> >   CONFIG_WATCHDOG=y
> >   CONFIG_STMP3XXX_RTC_WATCHDOG=y
> > @@ -138,10 +134,6 @@ CONFIG_PWM_MXS=y
> >   CONFIG_NVMEM_MXS_OCOTP=y
> >   CONFIG_EXT4_FS=y
> >   # CONFIG_DNOTIFY is not set
> > -CONFIG_NETFS_SUPPORT=m
> > -CONFIG_FSCACHE=y
> > -CONFIG_FSCACHE_STATS=y
> > -CONFIG_CACHEFILES=m
> >   CONFIG_VFAT_FS=y
> >   CONFIG_TMPFS=y
> >   CONFIG_TMPFS_POSIX_ACL=y  
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: [phone] Fax: [phone] Email: [e-mail]

Krzysztof Kozlowski April 10, 2025, 7:08 a.m. UTC | #3

On 10/04/2025 09:01, Lukasz Majewski wrote:
> Hi Stefan,
> 
>> Hi Lukasz,
>>
>> Am 07.04.25 um 16:51 schrieb Lukasz Majewski:
>>> This patch enables support for More Than IP switch available on some
>>> imx28[7] devices.
>>>
>>> Signed-off-by: Lukasz Majewski <[e-mail]>  
>> thanks adding the driver to mxs_defconfig. Unfortunately it's not
>> possible for reviewers to identify the relevant changes,
> 
> Could you be more specific here?
> As fair as I see - there is only 14 LOCs changed for review.

Really, the comment was very specific. You make multiple independent,
looking irrelevant changes to the file.

> 
> Please also be aware that MTIP L2 switch driver has some dependencies -
> on e.g. SWITCHDEV and BRIDGE, which had to be enabled to allow the
> former one to be active.
> 
>> also the
>> commit messages doesn't provide further information.
>>
> 
> What kind of extra information shall I provide? IMHO the patch is
> self-explaining.

For example explain why do you think GPIO_SYSFS should be dropped.

> 
>> In general there are two approaches to solves this:
>> 1) prepend an additional patch which synchronizes mxs_defconfig with
>> current mainline
>> 2) manually create the relevant changes against mxs_defconfig
>>
>> The decision about the approaches is up to the maintainer.
> 
> I took the linux-next's (or net-next) mxs defconfig (cp it to be
> .config)
> 
> Then run CROSS_COMPILE= ... make ARCH=arm menuconfig
> Enabled all the relevant Kconfig options and run
> 
> CROSS_COMPILE= ... make ARCH=arm savedefconfig
> and copy defconfig to mxs_defconfig.
> Then I used git to prepare the patch.
> 
> Isn't the above procedure correct?

No, it is not correct. Do not make any changes in the "Enabled all the
relevant Kconfig options and run" step and check the result. Do you see
difference in result file? If yes, then why such difference should be
part of this commit?

Best regards,
Krzysztof

Stefan Wahren April 10, 2025, 7:58 a.m. UTC | #4

Am 10.04.25 um 09:01 schrieb Lukasz Majewski:
> Hi Stefan,
>
>> Hi Lukasz,
>>
>> Am 07.04.25 um 16:51 schrieb Lukasz Majewski:
>>> This patch enables support for More Than IP switch available on some
>>> imx28[7] devices.
>>>
>>> Signed-off-by: Lukasz Majewski <[e-mail]>
>> thanks adding the driver to mxs_defconfig. Unfortunately it's not
>> possible for reviewers to identify the relevant changes,
> Could you be more specific here?
> As fair as I see - there is only 14 LOCs changed for review.
Yes, this is exactly the problem. The subject suggests enable one
driver, but the changes are not self-explaining.
> Please also be aware that MTIP L2 switch driver has some dependencies -
> on e.g. SWITCHDEV and BRIDGE, which had to be enabled to allow the
> former one to be active.
This should be part of the commit message.
>
>> also the
>> commit messages doesn't provide further information.
>>
> What kind of extra information shall I provide? IMHO the patch is
> self-explaining.
No, it is not. How should a reviewer know that

CONFIG_MTD_M25P80=y

should be dropped? This is completely unrelated to the change.
>
>> In general there are two approaches to solves this:
>> 1) prepend an additional patch which synchronizes mxs_defconfig with
>> current mainline
>> 2) manually create the relevant changes against mxs_defconfig
>>
>> The decision about the approaches is up to the maintainer.
> I took the linux-next's (or net-next) mxs defconfig (cp it to be
> .config)
>
> Then run CROSS_COMPILE= ... make ARCH=arm menuconfig
> Enabled all the relevant Kconfig options and run
>
> CROSS_COMPILE= ... make ARCH=arm savedefconfig
> and copy defconfig to mxs_defconfig.
> Then I used git to prepare the patch.
>
> Isn't the above procedure correct?
The problem here is that you send a patch for two steps. First is to
synchronize mxs_defconfig against current tree and the second is to
enable the driver.

Regards
>
>
>> Btw driver review will follow ...
>>
>> Regards
>>> ---
>>> Changes for v4:
>>> - New patch
>>> ---
>>>    arch/arm/configs/mxs_defconfig | 14 +++-----------
>>>    1 file changed, 3 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/arm/configs/mxs_defconfig
>>> b/arch/arm/configs/mxs_defconfig index d8a6e43c401e..4dc4306c035f
>>> 100644 --- a/arch/arm/configs/mxs_defconfig
>>> +++ b/arch/arm/configs/mxs_defconfig
>>> @@ -32,11 +32,10 @@ CONFIG_INET=y
>>>    CONFIG_IP_PNP=y
>>>    CONFIG_IP_PNP_DHCP=y
>>>    CONFIG_SYN_COOKIES=y
>>> -# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
>>> -# CONFIG_INET_XFRM_MODE_TUNNEL is not set
>>> -# CONFIG_INET_XFRM_MODE_BEET is not set
>>>    # CONFIG_INET_DIAG is not set
>>>    # CONFIG_IPV6 is not set
>>> +CONFIG_BRIDGE=y
>>> +CONFIG_NET_SWITCHDEV=y
>>>    CONFIG_CAN=m
>>>    # CONFIG_WIRELESS is not set
>>>    CONFIG_DEVTMPFS=y
>>> @@ -45,7 +44,6 @@ CONFIG_MTD=y
>>>    CONFIG_MTD_CMDLINE_PARTS=y
>>>    CONFIG_MTD_BLOCK=y
>>>    CONFIG_MTD_DATAFLASH=y
>>> -CONFIG_MTD_M25P80=y
>>>    CONFIG_MTD_SST25L=y
>>>    CONFIG_MTD_RAW_NAND=y
>>>    CONFIG_MTD_NAND_GPMI_NAND=y
>>> @@ -56,11 +54,11 @@ CONFIG_EEPROM_AT24=y
>>>    CONFIG_SCSI=y
>>>    CONFIG_BLK_DEV_SD=y
>>>    CONFIG_NETDEVICES=y
>>> +CONFIG_FEC_MTIP_L2SW=y
>>>    CONFIG_ENC28J60=y
>>>    CONFIG_ICPLUS_PHY=y
>>>    CONFIG_MICREL_PHY=y
>>>    CONFIG_REALTEK_PHY=y
>>> -CONFIG_SMSC_PHY=y
>>>    CONFIG_CAN_FLEXCAN=m
>>>    CONFIG_USB_USBNET=y
>>>    CONFIG_USB_NET_SMSC95XX=y
>>> @@ -77,13 +75,11 @@ CONFIG_SERIAL_AMBA_PL011=y
>>>    CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
>>>    CONFIG_SERIAL_MXS_AUART=y
>>>    # CONFIG_HW_RANDOM is not set
>>> -# CONFIG_I2C_COMPAT is not set
>>>    CONFIG_I2C_CHARDEV=y
>>>    CONFIG_I2C_MXS=y
>>>    CONFIG_SPI=y
>>>    CONFIG_SPI_GPIO=m
>>>    CONFIG_SPI_MXS=y
>>> -CONFIG_GPIO_SYSFS=y
>>>    # CONFIG_HWMON is not set
>>>    CONFIG_WATCHDOG=y
>>>    CONFIG_STMP3XXX_RTC_WATCHDOG=y
>>> @@ -138,10 +134,6 @@ CONFIG_PWM_MXS=y
>>>    CONFIG_NVMEM_MXS_OCOTP=y
>>>    CONFIG_EXT4_FS=y
>>>    # CONFIG_DNOTIFY is not set
>>> -CONFIG_NETFS_SUPPORT=m
>>> -CONFIG_FSCACHE=y
>>> -CONFIG_FSCACHE_STATS=y
>>> -CONFIG_CACHEFILES=m
>>>    CONFIG_VFAT_FS=y
>>>    CONFIG_TMPFS=y
>>>    CONFIG_TMPFS_POSIX_ACL=y
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: [phone] Fax: [phone] Email: [e-mail]

Patch

diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig
index d8a6e43c401e..4dc4306c035f 100644
--- a/arch/arm/configs/mxs_defconfig
+++ b/arch/arm/configs/mxs_defconfig
@@ -32,11 +32,10 @@  CONFIG_INET=y
 CONFIG_IP_PNP=y
 CONFIG_IP_PNP_DHCP=y
 CONFIG_SYN_COOKIES=y
-# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
-# CONFIG_INET_XFRM_MODE_TUNNEL is not set
-# CONFIG_INET_XFRM_MODE_BEET is not set
 # CONFIG_INET_DIAG is not set
 # CONFIG_IPV6 is not set
+CONFIG_BRIDGE=y
+CONFIG_NET_SWITCHDEV=y
 CONFIG_CAN=m
 # CONFIG_WIRELESS is not set
 CONFIG_DEVTMPFS=y
@@ -45,7 +44,6 @@  CONFIG_MTD=y
 CONFIG_MTD_CMDLINE_PARTS=y
 CONFIG_MTD_BLOCK=y
 CONFIG_MTD_DATAFLASH=y
-CONFIG_MTD_M25P80=y
 CONFIG_MTD_SST25L=y
 CONFIG_MTD_RAW_NAND=y
 CONFIG_MTD_NAND_GPMI_NAND=y
@@ -56,11 +54,11 @@  CONFIG_EEPROM_AT24=y
 CONFIG_SCSI=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_NETDEVICES=y
+CONFIG_FEC_MTIP_L2SW=y
 CONFIG_ENC28J60=y
 CONFIG_ICPLUS_PHY=y
 CONFIG_MICREL_PHY=y
 CONFIG_REALTEK_PHY=y
-CONFIG_SMSC_PHY=y
 CONFIG_CAN_FLEXCAN=m
 CONFIG_USB_USBNET=y
 CONFIG_USB_NET_SMSC95XX=y
@@ -77,13 +75,11 @@  CONFIG_SERIAL_AMBA_PL011=y
 CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
 CONFIG_SERIAL_MXS_AUART=y
 # CONFIG_HW_RANDOM is not set
-# CONFIG_I2C_COMPAT is not set
 CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_MXS=y
 CONFIG_SPI=y
 CONFIG_SPI_GPIO=m
 CONFIG_SPI_MXS=y
-CONFIG_GPIO_SYSFS=y
 # CONFIG_HWMON is not set
 CONFIG_WATCHDOG=y
 CONFIG_STMP3XXX_RTC_WATCHDOG=y
@@ -138,10 +134,6 @@  CONFIG_PWM_MXS=y
 CONFIG_NVMEM_MXS_OCOTP=y
 CONFIG_EXT4_FS=y
 # CONFIG_DNOTIFY is not set
-CONFIG_NETFS_SUPPORT=m
-CONFIG_FSCACHE=y
-CONFIG_FSCACHE_STATS=y
-CONFIG_CACHEFILES=m
 CONFIG_VFAT_FS=y
 CONFIG_TMPFS=y
 CONFIG_TMPFS_POSIX_ACL=y