[-next,0/6] spi: Switch to use devm_spi_alloc_master() in some drivers

This patchset is trying to replace spi_alloc_master() with
devm_spi_alloc_master() in some spi drivers. With this helper,
spi_master_put() is called in devres_release_all() whenever
the device is unbound, so the spi_master_put() in error path
can be removed.

Yang Yingliang (6):
  spi: oc-tiny: Switch to use devm_spi_alloc_master()
  spi: ath79: Switch to use devm_spi_alloc_master()
  spi: omap-uwire: Switch to use devm_spi_alloc_master()
  spi: ppc4xx: Switch to use devm_spi_alloc_master()
  spi: sh-sci: Switch to use devm_spi_alloc_master()
  spi: altera: Switch to use devm_spi_alloc_master()

 drivers/spi/spi-altera-platform.c | 23 ++++++++-------------
 drivers/spi/spi-ath79.c           | 18 ++++++----------
 drivers/spi/spi-oc-tiny.c         | 18 ++++++----------
 drivers/spi/spi-omap-uwire.c      |  6 ++----
 drivers/spi/spi-ppc4xx.c          | 17 ++++++----------
 drivers/spi/spi-sh-sci.c          | 34 ++++++++++++-------------------
 6 files changed, 41 insertions(+), 75 deletions(-)

On Wed, Sep 21, 2022 at 01:37:49PM +0100, Mark Brown wrote:
> On Wed, Sep 21, 2022 at 10:02:25AM +0800, Yang Yingliang wrote:
> > On 2022/9/21 2:33, Mark Brown wrote:
> > > If we're switching please update to the modern naming and use
> > > "controller" rather than the old name.
> 
> > Do you mean to use spi_controller instead of spi_master? Something like
> > this:
> > 'struct spi_controller * ctlr = devm_spi_alloc_master();'
> 
> Or just use devm_spi_alloc_controller() directly.

There's no such thing.  The driver needs to explicitly allocate a
master or slave and that will result in the slave bit being set
correctly in struct spi_controller.

Yang's v2 series now calls __devm_spi_alloc_controller()
but drivers should never call that directly.

Thanks,

Lukas

On Tue, Sep 20, 2022 at 09:48:15PM +0800, Yang Yingliang wrote:
> Switch to use devm_spi_alloc_master() to simpify error path.

Unfortunately you're not removing the spi_master_put() from
ath79_spi_remove() here either.  So another use-after-free. :(

Thanks,

Lukas

On Tue, Sep 20, 2022 at 09:48:13PM +0800, Yang Yingliang wrote:
> This patchset is trying to replace spi_alloc_master() with
> devm_spi_alloc_master() in some spi drivers. With this helper,
> spi_master_put() is called in devres_release_all() whenever
> the device is unbound, so the spi_master_put() in error path
> can be removed.
> 
> Yang Yingliang (6):
>   spi: oc-tiny: Switch to use devm_spi_alloc_master()
>   spi: ath79: Switch to use devm_spi_alloc_master()
>   spi: omap-uwire: Switch to use devm_spi_alloc_master()
>   spi: ppc4xx: Switch to use devm_spi_alloc_master()
>   spi: sh-sci: Switch to use devm_spi_alloc_master()
>   spi: altera: Switch to use devm_spi_alloc_master()

I'm withdrawing my objections to patches 1, 2 and 3:
I failed to appreciate that these drivers use spi_bitbang_start(),
which takes an extra reference on the controller.  Sorry for the noise.

Whole series is
Reviewed-by: Lukas Wunner <lukas@wunner.de>

This pertains to v1 of the series, not v2 (which incorrectly uses
__devm_spi_alloc_controller()).

Thanks,

Lukas

On Fri, Sep 23, 2022 at 06:42:58AM +0200, Lukas Wunner wrote:
> On Wed, Sep 21, 2022 at 01:37:49PM +0100, Mark Brown wrote:

> > Or just use devm_spi_alloc_controller() directly.

> There's no such thing.  The driver needs to explicitly allocate a
> master or slave and that will result in the slave bit being set
> correctly in struct spi_controller.

> Yang's v2 series now calls __devm_spi_alloc_controller()
> but drivers should never call that directly.

Right, we should probably make the actual function to wrap that though -
I'd misremembered that that hadn't been created.