> -----Original Message----- > From: Alexandru Ardelean <ardeleanalex@xxxxxxxxx> > Sent: Thursday, May 12, 2022 3:32 PM > To: LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx> > Cc: Jonathan Cameron <jic23@xxxxxxxxxx>; Lars-Peter Clausen > <lars@xxxxxxxxxx>; mchehab+huawei@xxxxxxxxxx; linux-iio > <linux-iio@xxxxxxxxxxxxxxx>; LKML <linux-kernel@xxxxxxxxxxxxxxx>; Rob Herring > <robh+dt@xxxxxxxxxx>; Mike Looijmans <mike.looijmans@xxxxxxxx>; devicetree > <devicetree@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH V2 3/6] iio: accel: bmi088: modified the device name > > This email is not from Hexagon’s Office 365 instance. Please be careful while > clicking links, opening attachments, or replying to this email. > > > On Tue, May 10, 2022 at 5:18 PM LI Qingwu > <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > It is possible to have multiple sensors connected on the same > > platform, To support multiple sensors, the commit makes it possible to > > obtain the device name by reading the chip ID instead of the device-tree > name. > > To be compatible with previous versions, renam bmi088a to bmi088-accel. > > // my spellcheck in GMail found this :p > > typo: renam -> rename > > I also have a comment about a duplication that is highlighted by this change. > > You can disregard my comment about the duplication and leave this change > as-is. > > > > > Signed-off-by: LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx> > > --- > > drivers/iio/accel/bmi088-accel-core.c | 6 +++--- > > drivers/iio/accel/bmi088-accel-spi.c | 4 +--- > > drivers/iio/accel/bmi088-accel.h | 2 +- > > 3 files changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/iio/accel/bmi088-accel-core.c > > b/drivers/iio/accel/bmi088-accel-core.c > > index 8fee1d02e773..de2385e4dad5 100644 > > --- a/drivers/iio/accel/bmi088-accel-core.c > > +++ b/drivers/iio/accel/bmi088-accel-core.c > > @@ -459,7 +459,7 @@ static const struct iio_chan_spec > > bmi088_accel_channels[] = { > > > > static const struct bmi088_accel_chip_info bmi088_accel_chip_info_tbl[] = { > > [0] = { > > - .name = "bmi088a", > > + .name = "bmi088-accel", > > .chip_id = 0x1E, > > .channels = bmi088_accel_channels, > > .num_channels = ARRAY_SIZE(bmi088_accel_channels), > > @@ -524,7 +524,7 @@ static int bmi088_accel_chip_init(struct > > bmi088_accel_data *data) } > > > > int bmi088_accel_core_probe(struct device *dev, struct regmap *regmap, > > - int irq, const char *name, bool block_supported) > > + int irq, bool block_supported) > > { > > struct bmi088_accel_data *data; > > struct iio_dev *indio_dev; > > @@ -545,7 +545,7 @@ int bmi088_accel_core_probe(struct device *dev, > > struct regmap *regmap, > > > > indio_dev->channels = data->chip_info->channels; > > indio_dev->num_channels = data->chip_info->num_channels; > > - indio_dev->name = name ? name : data->chip_info->name; > > + indio_dev->name = data->chip_info->name; > > (with this change) i can better see, a bit of duplication between the spi_device > table and the chip_info table > > this was not introduced by this change, but it was made a bit more obvious by > this change; > > one way to address this, is to remove the `const char *name;` and continue > using the `name` provided as a parameter from bmi088_accel_core_probe(); > (apologies if I seem to have changed my mind (from the previous changeset), but > I did not see it too well before) > > and we can convert > > enum { > ID_BMI088, > ID_BMI085, > ... > }; > > static const struct bmi088_accel_chip_info bmi088_accel_chip_info_tbl[] = { > [ID_BMI088] = { > .chip_id = 0x1E, > .channels = bmi088_accel_channels, > .num_channels = ARRAY_SIZE(bmi088_accel_channels), > }, > [ID_BMI085] = { > ........ > Thanks Ardelean, There is a case where some different sensors are connected to one system, For user space is nice if can detect which sensor is present, if using the name in spi_device table, the name may be inconsistent with the connected sensor. What's your opinion? > > indio_dev->available_scan_masks = bmi088_accel_scan_masks; > > indio_dev->modes = INDIO_DIRECT_MODE; > > indio_dev->info = &bmi088_accel_info; diff --git > > a/drivers/iio/accel/bmi088-accel-spi.c > > b/drivers/iio/accel/bmi088-accel-spi.c > > index dd1e3f6cf211..0fed0081e1fd 100644 > > --- a/drivers/iio/accel/bmi088-accel-spi.c > > +++ b/drivers/iio/accel/bmi088-accel-spi.c > > @@ -42,7 +42,6 @@ static struct regmap_bus bmi088_regmap_bus = { > > static int bmi088_accel_probe(struct spi_device *spi) { > > struct regmap *regmap; > > - const struct spi_device_id *id = spi_get_device_id(spi); > > > > regmap = devm_regmap_init(&spi->dev, &bmi088_regmap_bus, > > spi, &bmi088_regmap_conf); @@ -52,8 +51,7 > @@ > > static int bmi088_accel_probe(struct spi_device *spi) > > return PTR_ERR(regmap); > > } > > > > - return bmi088_accel_core_probe(&spi->dev, regmap, spi->irq, > id->name, > > - true); > > + return bmi088_accel_core_probe(&spi->dev, regmap, spi->irq, > > + true); > > } > > > > static int bmi088_accel_remove(struct spi_device *spi) diff --git > > a/drivers/iio/accel/bmi088-accel.h b/drivers/iio/accel/bmi088-accel.h > > index 5c25f16b672c..c32afe9606a8 100644 > > --- a/drivers/iio/accel/bmi088-accel.h > > +++ b/drivers/iio/accel/bmi088-accel.h > > @@ -12,7 +12,7 @@ extern const struct regmap_config > > bmi088_regmap_conf; extern const struct dev_pm_ops > > bmi088_accel_pm_ops; > > > > int bmi088_accel_core_probe(struct device *dev, struct regmap *regmap, > int irq, > > - const char *name, bool block_supported); > > + bool block_supported); > > int bmi088_accel_core_remove(struct device *dev); > > > > #endif /* BMI088_ACCEL_H */ > > -- > > 2.25.1 > >