[PATCH] ALSA: hda: cs35l41: Make cs35l41_hda_remove() return void

[Uwe Kleine-König]

On Tue, Jan 18, 2022 at 11:59:15AM +0000, Lucas tanure wrote:
> On 1/17/22 22:00, Uwe Kleine-König wrote:
> > Up to now cs35l41_hda_remove() returns zero unconditionally. Make it
> > return void instead which makes it easier to see in the callers that
> > there is no error to handle.
> > 
> > Also the return value of i2c and spi remove callbacks is ignored anyway.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> > ---
> >   sound/pci/hda/cs35l41_hda.c     | 4 +---
> >   sound/pci/hda/cs35l41_hda.h     | 2 +-
> >   sound/pci/hda/cs35l41_hda_i2c.c | 4 +++-
> >   sound/pci/hda/cs35l41_hda_spi.c | 4 +++-
> >   4 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
> > index 30b40d865863..ce3782826830 100644
> > --- a/sound/pci/hda/cs35l41_hda.c
> > +++ b/sound/pci/hda/cs35l41_hda.c
> > @@ -508,7 +508,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i
> >   }
> >   EXPORT_SYMBOL_GPL(cs35l41_hda_probe);
> > -int cs35l41_hda_remove(struct device *dev)
> > +void cs35l41_hda_remove(struct device *dev)
> >   {
> >   	struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev);
> > @@ -517,8 +517,6 @@ int cs35l41_hda_remove(struct device *dev)
> >   	if (!cs35l41->vspk_always_on)
> >   		gpiod_set_value_cansleep(cs35l41->reset_gpio, 0);
> >   	gpiod_put(cs35l41->reset_gpio);
> > -
> > -	return 0;
> >   }
> >   EXPORT_SYMBOL_GPL(cs35l41_hda_remove);
> > diff --git a/sound/pci/hda/cs35l41_hda.h b/sound/pci/hda/cs35l41_hda.h
> > index 76c69a8a22f6..8ecaddf5f132 100644
> > --- a/sound/pci/hda/cs35l41_hda.h
> > +++ b/sound/pci/hda/cs35l41_hda.h
> > @@ -64,6 +64,6 @@ struct cs35l41_hda {
> >   int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int irq,
> >   		      struct regmap *regmap);
> > -int cs35l41_hda_remove(struct device *dev);
> > +void cs35l41_hda_remove(struct device *dev);
> >   #endif /*__CS35L41_HDA_H__*/
> > diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c
> > index 4a9462fb5c14..d4240b8ded10 100644
> > --- a/sound/pci/hda/cs35l41_hda_i2c.c
> > +++ b/sound/pci/hda/cs35l41_hda_i2c.c
> > @@ -32,7 +32,9 @@ static int cs35l41_hda_i2c_probe(struct i2c_client *clt, const struct i2c_device
> >   static int cs35l41_hda_i2c_remove(struct i2c_client *clt)
> >   {
> > -	return cs35l41_hda_remove(&clt->dev);
> > +	cs35l41_hda_remove(&clt->dev);
> > +
> > +	return 0;
> >   }
> >   static const struct i2c_device_id cs35l41_hda_i2c_id[] = {
> > diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c
> > index 77426e96c58f..d63c487bc3a9 100644
> > --- a/sound/pci/hda/cs35l41_hda_spi.c
> > +++ b/sound/pci/hda/cs35l41_hda_spi.c
> > @@ -30,7 +30,9 @@ static int cs35l41_hda_spi_probe(struct spi_device *spi)
> >   static int cs35l41_hda_spi_remove(struct spi_device *spi)
> >   {
> > -	return cs35l41_hda_remove(&spi->dev);
> > +	cs35l41_hda_remove(&spi->dev);
> > +
> > +	return 0;
> >   }
> >   static const struct spi_device_id cs35l41_hda_spi_id[] = {
> > 
> > base-commit: 0c947b893d69231a9add855939da7c66237ab44f
> 
> I don't see much point in this patch. The idea of the core driver is to
> concentrate as much as code as possible can, so the I2C and SPI driver are
> minimal.

How is the spi (or i2c) driver any bigger here? For now the core driver
has a feature (returning an error code, but always 0) that isn't used at
all. How is that any good?

The motivation for this patch is that I want to do this in the near
future:

	diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
	--- a/include/linux/spi/spi.h
	+++ b/include/linux/spi/spi.h
	@@ -280,7 +280,7 @@ struct spi_message;
	 struct spi_driver {
		const struct spi_device_id *id_table;
		int			(*probe)(struct spi_device *spi);
	-	int			(*remove)(struct spi_device *spi);
	+	void			(*remove)(struct spi_device *spi);
		void			(*shutdown)(struct spi_device *spi);
		struct device_driver	driver;
	 };

And as this has to touch all spi drivers in the same commit, I prefer
having only hunks like:

	-static int cs35l41_hda_spi_remove(struct spi_device *spi)
	+static void cs35l41_hda_spi_remove(struct spi_device *spi)
	 {
		cs35l41_hda_remove(&spi->dev);
	-
	-	return 0;
	 }
 
which are obviously correct compared to

	-static int cs35l41_hda_spi_remove(struct spi_device *spi)
	+static void cs35l41_hda_spi_remove(struct spi_device *spi)
	 {
	- 	return cs35l41_hda_remove(&spi->dev);
	+	cs35l41_hda_remove(&spi->dev);
	 }

BTW, the long term plan is to do the same for i2c. 

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20220118/8b496241/attachment.sig>