Date:   Sun, 14 Nov 2021 21:39:54 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Michael Walle <michael@walle.cc>
Cc:     linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH] spi: fsl-dspi: use devm_spi_alloc_master()
Message-ID: <20211114193954.suonysr5lpdgpnrj@skbuf>
References: <20211110161754.3312805-1-michael@walle.cc>
 <20211113180642.7doe6dc5iotw5yez@skbuf>
 <cd4b16c6fb46cbb9670b5a9551bc46be@walle.cc>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cd4b16c6fb46cbb9670b5a9551bc46be@walle.cc>
Precedence: bulk

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.