[Buildroot] [PATCH 0/5] rock5b additional improvements

[Buildroot] [PATCH 0/5] rock5b additional improvements

[Niklas Cassel via buildroot]

From: Niklas Cassel <cassel@kernel.org>

Hello all,

Here are some additional improvements for the rock5b board that:
-Migrates to GPT partition table instead of hybrid (MBR+GPT).
-Uses more robust rootfs definition on the kernel command line.
-Enables udev such that modules are loaded automatically, which
 also allows us to drop the linux.fragment which forced some
 drivers to be built as built-in.
-Disables udev predictable NIC names such the NIC is still called eth0.
-Enables DHCP for eth0.

Kind regards,
Niklas

Niklas Cassel (5):
  board/radxa/rock5b: use gpt partition table instead of hybrid
  board/radxa/rock5b: use PARTLABEL to specify rootfs
  configs/rock5b: enable eudev to enable automatic module loading
  board/radxa/rock5b: do not use udev predictable NIC names
  configs/rock5b: enable DHCP for eth0

 board/radxa/rock5b/extlinux.conf  | 2 +-
 board/radxa/rock5b/genimage.cfg   | 4 ++--
 board/radxa/rock5b/linux.fragment | 2 --
 configs/rock5b_defconfig          | 3 ++-
 4 files changed, 5 insertions(+), 6 deletions(-)
 delete mode 100644 board/radxa/rock5b/linux.fragment

-- 
2.46.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/5] board/radxa/rock5b: use gpt partition table instead of hybrid

[Niklas Cassel via buildroot]

From: Niklas Cassel <cassel@kernel.org>

Partition table hybrid means GPT + MBR.
There is no need to keep the MBR when using GPT, so migrate to GPT only.

This change also requires us to migrate from partition-type to
partition-type-uuid, otherwise genimage won't generate an image.

Note that GPT itself always writes a "protective MBR" at LBA 0.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 board/radxa/rock5b/genimage.cfg | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/radxa/rock5b/genimage.cfg b/board/radxa/rock5b/genimage.cfg
index 138ec26e3a..43bb65bdd9 100644
--- a/board/radxa/rock5b/genimage.cfg
+++ b/board/radxa/rock5b/genimage.cfg
@@ -2,7 +2,7 @@
 
 image sdcard.img {
 	hdimage {
-		partition-table-type = "hybrid"
+		partition-table-type = "gpt"
 	}
 
 	partition uboot {
@@ -12,7 +12,7 @@ image sdcard.img {
 	}
 
 	partition rootfs {
-		partition-type = 0x83
+		partition-type-uuid = L
 		image = "rootfs.ext2"
 	}
 }
-- 
2.46.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/5] board/radxa/rock5b: use PARTLABEL to specify rootfs

[Niklas Cassel via buildroot]

From: Niklas Cassel <cassel@kernel.org>

This board has an optional eMMC module.

Use PARTLABEL to specify the rootfs, as this works regardless of the mmc
device probe order.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 board/radxa/rock5b/extlinux.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/radxa/rock5b/extlinux.conf b/board/radxa/rock5b/extlinux.conf
index 9832255992..c4198deaa0 100644
--- a/board/radxa/rock5b/extlinux.conf
+++ b/board/radxa/rock5b/extlinux.conf
@@ -1,4 +1,4 @@
 label Radxa Rock 5b Linux
   kernel /boot/Image
   devicetree /boot/rk3588-rock-5b.dtb
-  append root=/dev/mmcblk1p1 earlycon rootwait
+  append root=PARTLABEL=rootfs earlycon rootwait
-- 
2.46.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Niklas Cassel via buildroot]

From: Niklas Cassel <cassel@kernel.org>

The arm64 defconfig enables both the R8169 Ethernet driver and the
PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.

Enable udev so that these modules will get automatically loaded by udev
after the rootfs is mounted from the SD card.

This way, we do not need to build these modules as built-in, and we also
get the benefit that any device plugged in to the PCIe slot will get
automatically loaded.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 board/radxa/rock5b/linux.fragment | 2 --
 configs/rock5b_defconfig          | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)
 delete mode 100644 board/radxa/rock5b/linux.fragment

diff --git a/board/radxa/rock5b/linux.fragment b/board/radxa/rock5b/linux.fragment
deleted file mode 100644
index 90f2f291ba..0000000000
--- a/board/radxa/rock5b/linux.fragment
+++ /dev/null
@@ -1,2 +0,0 @@
-CONFIG_R8169=y
-CONFIG_PHY_ROCKCHIP_NANENG_COMBO_PHY=y
diff --git a/configs/rock5b_defconfig b/configs/rock5b_defconfig
index 899537c001..bcc2b68f6c 100644
--- a/configs/rock5b_defconfig
+++ b/configs/rock5b_defconfig
@@ -34,7 +34,6 @@ BR2_LINUX_KERNEL_DTS_SUPPORT=y
 BR2_LINUX_KERNEL_INSTALL_TARGET=y
 BR2_LINUX_KERNEL_INTREE_DTS_NAME="rockchip/rk3588-rock-5b"
 BR2_LINUX_KERNEL_DTB_OVERLAY_SUPPORT=y
-BR2_LINUX_KERNEL_CONFIG_FRAGMENT_FILES="board/radxa/rock5b/linux.fragment"
 
 # Filesystem
 BR2_TARGET_GENERIC_HOSTNAME="rock5b"
@@ -46,6 +45,7 @@ BR2_PACKAGE_HOST_DOSFSTOOLS=y
 BR2_PACKAGE_HOST_DTC=y
 BR2_PACKAGE_HOST_GENIMAGE=y
 BR2_PACKAGE_HOST_MTOOLS=y
+BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
 BR2_ROOTFS_POST_BUILD_SCRIPT="board/radxa/rock5b/post-build.sh"
 BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/radxa/rock5b/genimage.cfg"
-- 
2.46.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 4/5] board/radxa/rock5b: do not use udev predictable NIC names

[Niklas Cassel via buildroot]

From: Niklas Cassel <cassel@kernel.org>

While udev is great it "unfortunately" enables predictable NIC names by
default.

Predictable NIC names are not bad by itself, but it does not work well
with the networking scripts in buildroot, thus disable udev predictable
NIC names by supplying net.ifnames=0 on the kernel command line.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 board/radxa/rock5b/extlinux.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/radxa/rock5b/extlinux.conf b/board/radxa/rock5b/extlinux.conf
index c4198deaa0..c34b51c936 100644
--- a/board/radxa/rock5b/extlinux.conf
+++ b/board/radxa/rock5b/extlinux.conf
@@ -1,4 +1,4 @@
 label Radxa Rock 5b Linux
   kernel /boot/Image
   devicetree /boot/rk3588-rock-5b.dtb
-  append root=PARTLABEL=rootfs earlycon rootwait
+  append root=PARTLABEL=rootfs earlycon rootwait net.ifnames=0
-- 
2.46.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 5/5] configs/rock5b: enable DHCP for eth0

[Niklas Cassel via buildroot]

From: Niklas Cassel <cassel@kernel.org>

Enable DHCP for eth0.
This is perfectly fine even for users that are not using the Ethernet
on the rock5b, as in that case, udhcpc will simply fork to background
without delaying the boot.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 configs/rock5b_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/rock5b_defconfig b/configs/rock5b_defconfig
index bcc2b68f6c..b720c8989a 100644
--- a/configs/rock5b_defconfig
+++ b/configs/rock5b_defconfig
@@ -46,6 +46,7 @@ BR2_PACKAGE_HOST_DTC=y
 BR2_PACKAGE_HOST_GENIMAGE=y
 BR2_PACKAGE_HOST_MTOOLS=y
 BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
+BR2_SYSTEM_DHCP="eth0"
 BR2_ROOTFS_POST_BUILD_SCRIPT="board/radxa/rock5b/post-build.sh"
 BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/radxa/rock5b/genimage.cfg"
-- 
2.46.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/5] rock5b additional improvements

[Thomas Petazzoni via buildroot]

On Mon,  2 Sep 2024 12:44:28 +0200
Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:

> Niklas Cassel (5):
>   board/radxa/rock5b: use gpt partition table instead of hybrid
>   board/radxa/rock5b: use PARTLABEL to specify rootfs
>   configs/rock5b: enable eudev to enable automatic module loading
>   board/radxa/rock5b: do not use udev predictable NIC names
>   configs/rock5b: enable DHCP for eth0

I have applied to our next branch patches 1/5, 2/5 and 5/5, but not 3/5
and 4/5. I'll reply separately to them.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Thomas Petazzoni via buildroot]

On Mon,  2 Sep 2024 12:44:31 +0200
Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:

> From: Niklas Cassel <cassel@kernel.org>
> 
> The arm64 defconfig enables both the R8169 Ethernet driver and the
> PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.
> 
> Enable udev so that these modules will get automatically loaded by udev
> after the rootfs is mounted from the SD card.
> 
> This way, we do not need to build these modules as built-in, and we also
> get the benefit that any device plugged in to the PCIe slot will get
> automatically loaded.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

I'm not sure what we want to do here, especially when this ends up
needing net.ifnames=0 that you add in PATCH 4/5.

We do have already a small number of defconfigs that enable eudev. So
far we only use net.ifnames=0 in 4 configurations.

Basically, I'm trying to make sure we remain consistent and have a
policy that applies to all defconfigs rather than ad-hoc solutions that
are different for each and every defconfig. That's why I'm a bit
reluctant with applying your patches as-is because they only solve the
specific case of this defconfig, in a specific way, without creating a
clear rule that all defconfigs meeting a specific situation should
follow.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Arnout Vandecappelle via buildroot]

On 03/09/2024 21:26, Thomas Petazzoni via buildroot wrote:
> On Mon,  2 Sep 2024 12:44:31 +0200
> Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:
> 
>> From: Niklas Cassel <cassel@kernel.org>
>>
>> The arm64 defconfig enables both the R8169 Ethernet driver and the
>> PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.
>>
>> Enable udev so that these modules will get automatically loaded by udev
>> after the rootfs is mounted from the SD card.
>>
>> This way, we do not need to build these modules as built-in, and we also
>> get the benefit that any device plugged in to the PCIe slot will get
>> automatically loaded.
>>
>> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> 
> I'm not sure what we want to do here, especially when this ends up
> needing net.ifnames=0 that you add in PATCH 4/5.
> 
> We do have already a small number of defconfigs that enable eudev. So
> far we only use net.ifnames=0 in 4 configurations.
> 
> Basically, I'm trying to make sure we remain consistent and have a
> policy that applies to all defconfigs rather than ad-hoc solutions that
> are different for each and every defconfig. That's why I'm a bit
> reluctant with applying your patches as-is because they only solve the
> specific case of this defconfig, in a specific way, without creating a
> clear rule that all defconfigs meeting a specific situation should
> follow.

  We don't really have a policy or rules at the moment. I've added this to the 
topics of the Buildroot Developer meeting in Vienna.

  For the time being, I think we generally prefer to have a kernel config that 
is as much as possible custom for the target board. Thus, we also generally 
prefer to not require loading modules. Certainly for ethernet - having ethernet 
as a module makes nfsroot impossible.

  So I've marked patches 3/5 and 4/5 as Rejected.

  Feel free to give arguments why we should prefer these drivers as module though!

  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Niklas Cassel via buildroot]

On Tue, Sep 03, 2024 at 09:26:11PM +0200, Thomas Petazzoni wrote:
> On Mon,  2 Sep 2024 12:44:31 +0200
> Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:
> 
> > From: Niklas Cassel <cassel@kernel.org>
> > 
> > The arm64 defconfig enables both the R8169 Ethernet driver and the
> > PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.
> > 
> > Enable udev so that these modules will get automatically loaded by udev
> > after the rootfs is mounted from the SD card.
> > 
> > This way, we do not need to build these modules as built-in, and we also
> > get the benefit that any device plugged in to the PCIe slot will get
> > automatically loaded.
> > 
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> 
> I'm not sure what we want to do here, especially when this ends up
> needing net.ifnames=0 that you add in PATCH 4/5.
> 
> We do have already a small number of defconfigs that enable eudev. So
> far we only use net.ifnames=0 in 4 configurations.
> 
> Basically, I'm trying to make sure we remain consistent and have a
> policy that applies to all defconfigs rather than ad-hoc solutions that
> are different for each and every defconfig. That's why I'm a bit
> reluctant with applying your patches as-is because they only solve the
> specific case of this defconfig, in a specific way, without creating a
> clear rule that all defconfigs meeting a specific situation should
> follow.

(Replying with the email that I have subscribed to the list this time.)


An alternative to specify net.ifnames=0 on the kernel command line
(to disable udev predictable NIC names), is to create an empty file:
/etc/udev/rules.d/80-net-name-slot.rules

See:
https://wiki.gentoo.org/wiki/Udev#Optional:_Disable_or_override_predictable_network_interface_naming

I guess we could modify package/eudev/eudev.mk and package/eudev/Config.in
to create a new BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES Kconfig which we
default to "not set".

If BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES is "not set", we install/touch
/etc/udev/rules.d/80-net-name-slot.rules in the target rootfs.

That would avoid the need for net.ifnames=0. We could also remove it for
the existing boards that have it set.

Any board that actually wants predictable NIC names can set the new
BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES Kconfig.

Thoughts?


Kind regards,
Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Niklas Cassel via buildroot]

On Tue, Sep 03, 2024 at 09:50:53PM +0200, Arnout Vandecappelle wrote:
> 
> 
> On 03/09/2024 21:26, Thomas Petazzoni via buildroot wrote:
> > On Mon,  2 Sep 2024 12:44:31 +0200
> > Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:
> > 
> > > From: Niklas Cassel <cassel@kernel.org>
> > > 
> > > The arm64 defconfig enables both the R8169 Ethernet driver and the
> > > PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.
> > > 
> > > Enable udev so that these modules will get automatically loaded by udev
> > > after the rootfs is mounted from the SD card.
> > > 
> > > This way, we do not need to build these modules as built-in, and we also
> > > get the benefit that any device plugged in to the PCIe slot will get
> > > automatically loaded.
> > > 
> > > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > 
> > I'm not sure what we want to do here, especially when this ends up
> > needing net.ifnames=0 that you add in PATCH 4/5.
> > 
> > We do have already a small number of defconfigs that enable eudev. So
> > far we only use net.ifnames=0 in 4 configurations.
> > 
> > Basically, I'm trying to make sure we remain consistent and have a
> > policy that applies to all defconfigs rather than ad-hoc solutions that
> > are different for each and every defconfig. That's why I'm a bit
> > reluctant with applying your patches as-is because they only solve the
> > specific case of this defconfig, in a specific way, without creating a
> > clear rule that all defconfigs meeting a specific situation should
> > follow.
> 
>  We don't really have a policy or rules at the moment. I've added this to
> the topics of the Buildroot Developer meeting in Vienna.

See my reply to Thomas where I suggest to create a
BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES Kconfig which is default not set.

I think that would be a nice policy / solution.


> 
>  For the time being, I think we generally prefer to have a kernel config
> that is as much as possible custom for the target board. Thus, we also
> generally prefer to not require loading modules. Certainly for ethernet -
> having ethernet as a module makes nfsroot impossible.
> 
>  So I've marked patches 3/5 and 4/5 as Rejected.
> 
>  Feel free to give arguments why we should prefer these drivers as module though!

Sure, we could keep the Ethernet driver as built-in.

I still think it is a really good idea to enable udev though.

rock5b is currently using arm64 defconfig as kernel config.


Just enabling udev shows this in lsmod:

# lsmod
Module                  Size  Used by    Not tainted
hantro_vpu            270336  0 
rockchipdrm           167936  0 
v4l2_jpeg              16384  1 hantro_vpu
v4l2_vp9               20480  1 hantro_vpu
v4l2_h264              12288  1 hantro_vpu
analogix_dp            36864  1 rockchipdrm
v4l2_mem2mem           36864  1 hantro_vpu
dw_mipi_dsi            20480  1 rockchipdrm
panthor               118784  0 
videobuf2_dma_contig    16384  1 hantro_vpu
dw_hdmi                40960  1 rockchipdrm
videobuf2_memops       12288  1 videobuf2_dma_contig
cec                    53248  1 dw_hdmi
drm_gpuvm              32768  1 panthor
videobuf2_v4l2         28672  2 hantro_vpu,v4l2_mem2mem
drm_exec               12288  2 panthor,drm_gpuvm
drm_display_helper    167936  3 rockchipdrm,analogix_dp,dw_hdmi
drm_shmem_helper       24576  1 panthor
drm_dma_helper         20480  1 rockchipdrm
crct10dif_ce           12288  1 
videodev              253952  3 hantro_vpu,v4l2_mem2mem,videobuf2_v4l2
rockchip_saradc        16384  0 
gpu_sched              40960  1 panthor
videobuf2_common       57344  5 hantro_vpu,v4l2_mem2mem,videobuf2_dma_contig,videobuf2_memops,videobuf2_v4l2
drm_kms_helper        188416  7 rockchipdrm,analogix_dp,dw_mipi_dsi,dw_hdmi,drm_display_helper,drm_shmem_helper,drm_dma_helper
mc                     57344  5 hantro_vpu,v4l2_mem2mem,videobuf2_v4l2,videodev,videobuf2_common
drm                   552960 12 rockchipdrm,analogix_dp,dw_mipi_dsi,panthor,dw_hdmi,drm_gpuvm,drm_exec,drm_display_helper,drm_shmem_helper,drm_dma_helper,gpu_sched,drm_kms_helper
industrialio_triggered_buffer    12288  1 rockchip_saradc
backlight              20480  2 drm_display_helper,drm
phy_rockchip_usbdp     20480  2 
phy_rockchip_naneng_combphy    16384  5 
kfifo_buf              12288  1 industrialio_triggered_buffer
pwm_fan                16384  0 
rtc_hym8563            12288  1 
snd_soc_audio_graph_card    16384  0 
typec                  77824  1 phy_rockchip_usbdp
snd_soc_rockchip_i2s_tdm    20480  2 
snd_soc_simple_card_utils    28672  1 snd_soc_audio_graph_card
snd_soc_es8316         40960  1 
rockchip_thermal       24576  0 
spi_rockchip_sfc       12288  0 
rk805_pwrkey           12288  0 

Without udev (i.e. with the current buildroot rock5b_defconfig)
none of these modules will be loaded.


Kind regards,
Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Arnout Vandecappelle via buildroot]

On 03/09/2024 21:56, Niklas Cassel wrote:
> On Tue, Sep 03, 2024 at 09:26:11PM +0200, Thomas Petazzoni wrote:
>> On Mon,  2 Sep 2024 12:44:31 +0200
>> Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:
>>
>>> From: Niklas Cassel <cassel@kernel.org>
>>>
>>> The arm64 defconfig enables both the R8169 Ethernet driver and the
>>> PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.
>>>
>>> Enable udev so that these modules will get automatically loaded by udev
>>> after the rootfs is mounted from the SD card.
>>>
>>> This way, we do not need to build these modules as built-in, and we also
>>> get the benefit that any device plugged in to the PCIe slot will get
>>> automatically loaded.
>>>
>>> Signed-off-by: Niklas Cassel <cassel@kernel.org>
>>
>> I'm not sure what we want to do here, especially when this ends up
>> needing net.ifnames=0 that you add in PATCH 4/5.
>>
>> We do have already a small number of defconfigs that enable eudev. So
>> far we only use net.ifnames=0 in 4 configurations.
>>
>> Basically, I'm trying to make sure we remain consistent and have a
>> policy that applies to all defconfigs rather than ad-hoc solutions that
>> are different for each and every defconfig. That's why I'm a bit
>> reluctant with applying your patches as-is because they only solve the
>> specific case of this defconfig, in a specific way, without creating a
>> clear rule that all defconfigs meeting a specific situation should
>> follow.
> 
> (Replying with the email that I have subscribed to the list this time.)
> 
> 
> An alternative to specify net.ifnames=0 on the kernel command line
> (to disable udev predictable NIC names), is to create an empty file:
> /etc/udev/rules.d/80-net-name-slot.rules

  Well, I think Thomas' comment was primarily about whether we should enable 
eudev and dynamically load modules rather than using builtins.

  Passing net.ifnames=0 is probably the easiest way to disable predictable NIC 
names. Only, it's surprising that we have something like 25 defconfigs that 
enable eudev, but only 4 of them enable that option... We should preferably have 
one "normal" way that this is done.


  What is BTW not clear to me is _why_ you don't want predictable names. What's 
wrong with BR2_SYSTEM_DHCP="enp0s31f6"?


  Regards,
  Arnout

> 
> See:
> https://wiki.gentoo.org/wiki/Udev#Optional:_Disable_or_override_predictable_network_interface_naming
> 
> I guess we could modify package/eudev/eudev.mk and package/eudev/Config.in
> to create a new BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES Kconfig which we
> default to "not set".
> 
> If BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES is "not set", we install/touch
> /etc/udev/rules.d/80-net-name-slot.rules in the target rootfs.
> 
> That would avoid the need for net.ifnames=0. We could also remove it for
> the existing boards that have it set.
> 
> Any board that actually wants predictable NIC names can set the new
> BR2_PACKAGE_EUDEV_PREDICTABLE_NIC_NAMES Kconfig.
> 
> Thoughts?
> 
> 
> Kind regards,
> Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Arnout Vandecappelle via buildroot]

On 03/09/2024 22:06, Niklas Cassel wrote:
> On Tue, Sep 03, 2024 at 09:50:53PM +0200, Arnout Vandecappelle wrote:
>>
>>
[snip]

>>   For the time being, I think we generally prefer to have a kernel config
>> that is as much as possible custom for the target board. Thus, we also
>> generally prefer to not require loading modules. Certainly for ethernet -
>> having ethernet as a module makes nfsroot impossible.
>>
>>   So I've marked patches 3/5 and 4/5 as Rejected.
>>
>>   Feel free to give arguments why we should prefer these drivers as module though!
> 
> Sure, we could keep the Ethernet driver as built-in.
> 
> I still think it is a really good idea to enable udev though.
> 
> rock5b is currently using arm64 defconfig as kernel config.
> 
> 
> Just enabling udev shows this in lsmod:
> 
> # lsmod
> Module                  Size  Used by    Not tainted
> hantro_vpu            270336  0
> rockchipdrm           167936  0
> v4l2_jpeg              16384  1 hantro_vpu
> v4l2_vp9               20480  1 hantro_vpu
> v4l2_h264              12288  1 hantro_vpu
> analogix_dp            36864  1 rockchipdrm
> v4l2_mem2mem           36864  1 hantro_vpu
> dw_mipi_dsi            20480  1 rockchipdrm
> panthor               118784  0
> videobuf2_dma_contig    16384  1 hantro_vpu
> dw_hdmi                40960  1 rockchipdrm
> videobuf2_memops       12288  1 videobuf2_dma_contig
> cec                    53248  1 dw_hdmi
> drm_gpuvm              32768  1 panthor
> videobuf2_v4l2         28672  2 hantro_vpu,v4l2_mem2mem
> drm_exec               12288  2 panthor,drm_gpuvm
> drm_display_helper    167936  3 rockchipdrm,analogix_dp,dw_hdmi
> drm_shmem_helper       24576  1 panthor
> drm_dma_helper         20480  1 rockchipdrm
> crct10dif_ce           12288  1
> videodev              253952  3 hantro_vpu,v4l2_mem2mem,videobuf2_v4l2
> rockchip_saradc        16384  0
> gpu_sched              40960  1 panthor
> videobuf2_common       57344  5 hantro_vpu,v4l2_mem2mem,videobuf2_dma_contig,videobuf2_memops,videobuf2_v4l2
> drm_kms_helper        188416  7 rockchipdrm,analogix_dp,dw_mipi_dsi,dw_hdmi,drm_display_helper,drm_shmem_helper,drm_dma_helper
> mc                     57344  5 hantro_vpu,v4l2_mem2mem,videobuf2_v4l2,videodev,videobuf2_common
> drm                   552960 12 rockchipdrm,analogix_dp,dw_mipi_dsi,panthor,dw_hdmi,drm_gpuvm,drm_exec,drm_display_helper,drm_shmem_helper,drm_dma_helper,gpu_sched,drm_kms_helper
> industrialio_triggered_buffer    12288  1 rockchip_saradc
> backlight              20480  2 drm_display_helper,drm
> phy_rockchip_usbdp     20480  2
> phy_rockchip_naneng_combphy    16384  5
> kfifo_buf              12288  1 industrialio_triggered_buffer
> pwm_fan                16384  0
> rtc_hym8563            12288  1
> snd_soc_audio_graph_card    16384  0
> typec                  77824  1 phy_rockchip_usbdp
> snd_soc_rockchip_i2s_tdm    20480  2
> snd_soc_simple_card_utils    28672  1 snd_soc_audio_graph_card
> snd_soc_es8316         40960  1
> rockchip_thermal       24576  0
> spi_rockchip_sfc       12288  0
> rk805_pwrkey           12288  0
> 
> Without udev (i.e. with the current buildroot rock5b_defconfig)
> none of these modules will be loaded.

  That is a very good reaosn to want eudev, indeed! The alternative is to extend 
the linux config fragment to include all of these as builtins, but I guess 
that's not even possible for hantro_vpu...


  Then another question is whether we should use eudev or mdev. We currently 
have 25 confgs that enable mdev and 30 that enable eudev. Some consistency there 
would be nice as well!


  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Niklas Cassel via buildroot]

On Tue, Sep 03, 2024 at 11:06:13PM +0200, Arnout Vandecappelle wrote:
> 
> 
> On 03/09/2024 21:56, Niklas Cassel wrote:
> > On Tue, Sep 03, 2024 at 09:26:11PM +0200, Thomas Petazzoni wrote:
> > > On Mon,  2 Sep 2024 12:44:31 +0200
> > > Niklas Cassel via buildroot <buildroot@buildroot.org> wrote:
> > > 
> > > > From: Niklas Cassel <cassel@kernel.org>
> > > > 
> > > > The arm64 defconfig enables both the R8169 Ethernet driver and the
> > > > PHY_ROCKCHIP_NANENG_COMBO_PHY PHY driver as modules.
> > > > 
> > > > Enable udev so that these modules will get automatically loaded by udev
> > > > after the rootfs is mounted from the SD card.
> > > > 
> > > > This way, we do not need to build these modules as built-in, and we also
> > > > get the benefit that any device plugged in to the PCIe slot will get
> > > > automatically loaded.
> > > > 
> > > > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > > 
> > > I'm not sure what we want to do here, especially when this ends up
> > > needing net.ifnames=0 that you add in PATCH 4/5.
> > > 
> > > We do have already a small number of defconfigs that enable eudev. So
> > > far we only use net.ifnames=0 in 4 configurations.
> > > 
> > > Basically, I'm trying to make sure we remain consistent and have a
> > > policy that applies to all defconfigs rather than ad-hoc solutions that
> > > are different for each and every defconfig. That's why I'm a bit
> > > reluctant with applying your patches as-is because they only solve the
> > > specific case of this defconfig, in a specific way, without creating a
> > > clear rule that all defconfigs meeting a specific situation should
> > > follow.
> > 
> > (Replying with the email that I have subscribed to the list this time.)
> > 
> > 
> > An alternative to specify net.ifnames=0 on the kernel command line
> > (to disable udev predictable NIC names), is to create an empty file:
> > /etc/udev/rules.d/80-net-name-slot.rules
> 
>  Well, I think Thomas' comment was primarily about whether we should enable
> eudev and dynamically load modules rather than using builtins.

For boards that support USB, PCIe and HDMI, I think it is a good default
to enable udev (or mdev), as you could see from my other email, enabling
udev will load 45 different kernel modules. If we don't enable udev,
lsmod will show 0 loaded kernel modules.

I don't think we can enable everything as built-in, especially since a
user can plug in basically anything in the PCIe slot or USB slot.
(But I agree that keeping e.g. Ethernet as built-in is good for NFS root.)

E.g. I can plug in a USB to Ethernet adapter, and with udev enabled, the
correct kernel module gets loaded automatically.


> 
>  Passing net.ifnames=0 is probably the easiest way to disable predictable
> NIC names. Only, it's surprising that we have something like 25 defconfigs
> that enable eudev, but only 4 of them enable that option... We should
> preferably have one "normal" way that this is done.
> 
> 
>  What is BTW not clear to me is _why_ you don't want predictable names.
> What's wrong with BR2_SYSTEM_DHCP="enp0s31f6"?

Well, if you do a:
$ git grep BR2_SYSTEM_DHCP
you see that everyone except:
configs/ls1028ardb_defconfig:BR2_SYSTEM_DHCP="eno0"
is using eth0.

But sure, it would be possible to specify the predictable ifname.

I guess that it is just a bit unintuitive that you would need to
change BR2_SYSTEM_DHCP="eth0" to something else if you happen to enable
eudev. (While if you have mdev or neither mdev or udev, eth0 works fine.)


Kind regards,
Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] configs/rock5b: enable eudev to enable automatic module loading

[Niklas Cassel via buildroot]

On Tue, Sep 03, 2024 at 11:10:44PM +0200, Arnout Vandecappelle wrote:
> > 
> > Without udev (i.e. with the current buildroot rock5b_defconfig)
> > none of these modules will be loaded.
> 
>  That is a very good reaosn to want eudev, indeed! The alternative is to
> extend the linux config fragment to include all of these as builtins, but I
> guess that's not even possible for hantro_vpu...
> 
> 
>  Then another question is whether we should use eudev or mdev. We currently
> have 25 confgs that enable mdev and 30 that enable eudev. Some consistency
> there would be nice as well!

I remember that long ago, mdev did not support automatically loading kernel
modules when hotplugging devices.

See e.g.:
https://wiki.gentoo.org/wiki/Mdev#Miscellaneous
that still says:
"mdev unlike udev does not support auto-modules loading."

However, it appears that since:
https://github.com/buildroot/buildroot/commit/07f46c2b6daec44a6176039c90be67e66c4c2e42

mdev does support automatic module loading.


I changed from:
BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
to
BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV=y

in my defconfig, and did a:
make clean && make rock5b_defconfig && make

and:
$ lsmod | wc -l
45

shows the same amout of loaded kernel modules both with udev and mdev.

Plugging in a USB to Ethernet also loads the correct kernel module with mdev.


Using mdev will also avoid "udev predictable NIC names",
so I the patch in this series that adds net.ifnames=0 is no longer needed.


I will resubmit a patch that enables MDEV and will simply drop the patch
that adds net.ifnames=0 to the kernel command line.


Kind regards,
Niklas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot