Re: REGRESSION: "spi: add of_device_uevent_modalias support" and following "fix" breaks Macchiatobin

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Sep 20, 2021 at 11:56:31PM +0200, Andreas Schwab wrote:
> On Sep 20 2021, Russell King (Oracle) wrote:
> 
> > On Mon, Sep 20, 2021 at 09:41:47PM +0200, Andreas Schwab wrote:
> >> On Sep 20 2021, Russell King (Oracle) wrote:
> >> 
> >> > Therefore, this change breaks module autoloading.
> >> 
> >> Reverting this change breaks module autoloading.
> >
> > No.
> >
> > Module autoloading worked before.
> 
> Nope.

Sorry, but you are wrong. Let me take a random built kernel I have
laying around. 5.4.0+:

-rw-r--r-- 1 rmk rmk 17164877 Jan 26  2020 /home/rmk/systems/juno-host-5.4.0+.tar.bz2

and throw it onto the Macchiatobin:

[  OK  ] Finished Suspend/Resume Running libvirt Guests.
[  OK  ] Started LSB: exim Mail Transport Agent.
[  OK  ] Started Samba SMB Daemon.
[  OK  ] Reached target Multi-User System.
[  OK  ] Reached target Graphical Interface.
         Starting Update UTMP about System Runlevel Changes...
[  OK  ] Finished Update UTMP about System Runlevel Changes.

Debian GNU/Linux 11 mcbin-ss ttyS2

mcbin-ss login: root
Password:
Linux mcbin-ss 5.4.0+ #608 SMP PREEMPT Sun Jan 26 15:44:51 GMT 2020 aarch64

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Mon Sep 20 22:17:58 BST 2021 on ttyS2
root@mcbin-ss:~# lsmod
Module                  Size  Used by
nfnetlink              16384  0
8021q                  36864  0
garp                   16384  1 8021q
mrp                    20480  1 8021q
crct10dif_ce           16384  1
spi_nor                49152  0
armada_thermal         16384  0
sbsa_gwdt              16384  0
ip_tables              32768  0
x_tables               45056  1 ip_tables
root@mcbin-ss:~#

That's strange, spi_nor has been _autoloaded_ under 5.4.0+. If I boot
exactly the same userspace with 5.13.0+, it gets _autoloaded_:

[  OK  ] Finished Suspend/Resume Running libvirt Guests.
[  OK  ] Started Samba SMB Daemon.P■ower device driver controller.
[  OK  ] Started LSB: exim Mail Transport Agent.tails.
[  OK  ] Reached target Multi-User System.
[  OK  ] Reached target Graphical Interface.ring sensors.
         Starting Update UTMP about System Runlevel Changes...
[  OK  ] Finished Update UTMP about System Runlevel Changes.rvice.
[  OK  ] Started LSB: rng-tools (Debian variant).

Debian GNU/Linux 11 mcbin-ss ttyS2

mcbin-ss login: root
Password:
Linux mcbin-ss 5.13.0+ #958 SMP PREEMPT Tue Sep 14 16:17:44 BST 2021 aarch64

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Mon Sep 20 23:19:05 BST 2021 on ttyS2
root@mcbin-ss:~# lsmod
Module                  Size  Used by
nfnetlink              20480  0
8021q                  36864  0
garp                   16384  1 8021q
mrp                    20480  1 8021q
crct10dif_ce           20480  1
spi_nor                73728  0
leds_gpio              16384  0
pwm_fan                20480  0
armada_thermal         16384  0
sbsa_gwdt              20480  0
ip_tables              32768  0
x_tables               45056  1 ip_tables

I'll prove it by moving the modules so they can't be loaded by the
normal system boot, and then trigger udev:

root@mcbin-ss:~# uname -a
Linux mcbin-ss 5.13.0+ #958 SMP PREEMPT Tue Sep 14 16:17:44 BST 2021 aarch64 GNU/Linux
root@mcbin-ss:~# lsmod
Module                  Size  Used by
root@mcbin-ss:~# mv 5.13.0+ /lib/modules
root@mcbin-ss:~# udevadm trigger --action=add
root@mcbin-ss:~# sbsa-gwdt f0610000.watchdog: Initialized with 10s timeout @ 25000000 Hz, action=0.
spi-nor spi4.0: w25q32 (4096 Kbytes)
lsmod
Module                  Size  Used by
crct10dif_ce           20480  1
pwm_fan                20480  0
spi_nor                73728  0
leds_gpio              16384  0
armada_thermal         16384  0
sbsa_gwdt              20480  0

Why does this happen?

root@mcbin-ss:~# cat /sys/bus/spi/devices/spi4.0/modalias
spi:w25q32
root@mcbin-ss:~# cat /sys/bus/spi/devices/spi4.0/uevent
DRIVER=spi-nor
OF_NAME=spi-flash
OF_FULLNAME=/cp1/config-space@f4000000/spi@700680/spi-flash@0
OF_COMPATIBLE_0=st,w25q32
OF_COMPATIBLE_N=1
MODALIAS=spi:w25q32
root@mcbin-ss:~# modinfo spi-nor
filename:       /lib/modules/5.13.0+/kernel/drivers/mtd/spi-nor/spi-nor.ko
description:    framework for SPI NOR
author:         Mike Lavender
author:         Huang Shijie <shijie8@gmail.com>
license:        GPL v2
alias:          of:N*T*Cjedec,spi-norC*
alias:          of:N*T*Cjedec,spi-nor
alias:          spi:mr25h40
...
alias:          spi:w25q32		<=======================
...
alias:          spi:spi-nor
depends:
intree:         Y
name:           spi_nor
vermagic:       5.13.0+ SMP preempt mod_unload aarch64

If I boot the exact same userspace with 5.14.0+, it does _not_ get
autoloaded, because the modinfo and uevent files contain different
contents that the spi-nor module does not have an alias for.

Yet you say without your change module autoloading doesn't work - that
may be true for you, but the point here is that:

   Your change plus another during the 5.14 cycle broke previously
   working module autoloading. It broke a previously working setup.
   This is a _regression_.

Maybe you could explain why you think otherwise - and possibly accept
that it _did_ used to work for others.

Yes, I get it that _your_ patch (which is the later one of the two
that I mentioned) was merely bringing the modalias file into line with
the uevent file - but that doesn't change the fact that 5.13 and
earlier kernels worked, 5.14 does not.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!