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@xxxxxxxxxxxxxx>
> > ---
> > 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/ |
Attachment:
signature.asc
Description: PGP signature
