linux-kernel - Re: [PATCH] spidev: Make probe to fail early if a spidev compatible is used

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211110074247.g7eaq2z27bwdt4m5@pengutronix.de>
Date:   Wed, 10 Nov 2021 08:42:47 +0100
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Javier Martinez Canillas <javierm@...hat.com>
Cc:     linux-kernel@...r.kernel.org, Mark Brown <broonie@...nel.org>,
        linux-spi@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH] spidev: Make probe to fail early if a spidev compatible
 is used

Hello,

On Tue, Nov 09, 2021 at 11:59:20PM +0100, Javier Martinez Canillas wrote:
> Some Device Trees don't use a real device name in the compatible string
> for SPI devices nodes, abusing the fact that the spidev driver name is
> used to match as a fallback when a SPI device ID table is not defined.
> 
> But since commit 6840615f85f6 ("spi: spidev: Add SPI ID table") a table
> for SPI device IDs was added to the driver breaking the assumption that
> these DTs were relying on.
> 
> There has been a warning message for some time since commit 956b200a846e
> ("spi: spidev: Warn loudly if instantiated from DT as "spidev""), making
> quite clear that this case is not really supported by the spidev driver.
> 
> Since these devices won't match anyways after the mentioned commit, there
> is no point to continue if an spidev compatible is used. Let's just make
> the driver probe to fail early.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>

Up to 6840615f85f6 the choices you had to use the spidev driver were
(assuing a dt machine):

 a) Use compatible = "spidev" and ignore the warning
 b) Use compatible = $chipname and add $chipname to the list of
    supported devices for the spidev driver. (e.g. "rohm,dh2228fv")
 c) Use compatible = $chipname and force binding the spidev driver using

   	echo spidev > /sys/bus/spi/devices/spiX.Y/driver_override
	echo spiX.Y > /sys/bus/spi/drivers/spidev/bind

Commit 6840615f85f6 changed that in situation a) you had to switch to c)
(well, or b) adding "spidev" to the spi id list).

With the change introduced by this patch, you make it impossible to bind
the spidev driver to such a device (without kernel source changes) even
using approach c). I wonder if this is too harsh given that changing the
dtb is difficult on some machines.

I think I prefer the status quo that people who use plain "spidev" as
compatible now have to do c) and get the warning. Maybe they notice that
they could switch to using the right chipname now to improve their
situation where the procedure to use the device is identical but the
warning is gone.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)