linux-kernel - Re: [PATCH] spi: fsl-dspi: use devm_spi_alloc

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] [day] [month] [year] [list]

Message-ID: <20211114193954.suonysr5lpdgpnrj@skbuf>
Date:   Sun, 14 Nov 2021 21:39:54 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Michael Walle <michael@...le.cc>
Cc:     linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH] spi: fsl-dspi: use devm_spi_alloc_master()

On Sun, Nov 14, 2021 at 04:44:18PM +0100, Michael Walle wrote:
> Let me know if I should respin the patch or if you like to
> keep the devm_kzalloc() as it is now, because you've mentioned
> you don't like the spi devres mappers. During earlier debugging
> I just noticed the following comment in drivers/spi/spi.c and
> noticed that this driver isn't converted:
> 
> /* Release the last reference on the controller if its driver
>  * has not yet been converted to devm_spi_alloc_master/slave().
>  */

I'm not a huge fan of cargo cult refactoring. I admit I haven't followed
Lukas' work too closely and I don't see why drivers _should_ be
converted to devm_spi_alloc_controller. Traditionally, the devres
helpers were riddled with various gotchas related to deregistration
being performed way too late (leading to peripheral drivers being unable
to access their hardware because the controller's ->remove had already
executed). I said that not making any use of devres would be great
because I sadly don't have enough time to keep up with all the
subtleties of the devm_spi_* helpers, and having everything implemented
in a plain and simple way makes errors more obvious for me to reason
about. So if there isn't any technical reason for this change, I'd
personally pass on it, but it's up to Mark, really.