| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AS8PR06MB784537D5C27DE4D867CF5064D7CB9@AS8PR06MB7845.eurprd06.prod.outlook.com>
Date: Thu, 12 May 2022 16:18:20 +0000
From: LI Qingwu <qing-wu.li@...ca-geosystems.com.cn>
To: Alexandru Ardelean <ardeleanalex@...il.com>
CC: Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
"mchehab+huawei@...nel.org" <mchehab+huawei@...nel.org>,
linux-iio <linux-iio@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Rob Herring <robh+dt@...nel.org>,
Mike Looijmans <mike.looijmans@...ic.nl>,
devicetree <devicetree@...r.kernel.org>
Subject: RE: [PATCH V2 3/6] iio: accel: bmi088: modified the device name
> -----Original Message-----
> From: Alexandru Ardelean <ardeleanalex@...il.com>
> Sent: Thursday, May 12, 2022 3:32 PM
> To: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
> Cc: Jonathan Cameron <jic23@...nel.org>; Lars-Peter Clausen
> <lars@...afoo.de>; mchehab+huawei@...nel.org; linux-iio
> <linux-iio@...r.kernel.org>; LKML <linux-kernel@...r.kernel.org>; Rob Herring
> <robh+dt@...nel.org>; Mike Looijmans <mike.looijmans@...ic.nl>; devicetree
> <devicetree@...r.kernel.org>
> 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@...ca-geosystems.com.cn> 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@...ca-geosystems.com.cn>
> > ---
> > 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
> >
Powered by blists - more mailing lists