2022-06-15 12:14:15

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v2 1/2] iio: proximity: sx_common: Don't use IIO device for properties

It's not correct to use artificial device created by IIO core to
retrieve device properties. Even ->get_default_reg() callback
takes a simple struct device pointer which suggests it wants to
operate on the real device.

Correct this by replacing pointer to IIO device by a real device
pointer in the caller of ->get_default_reg().

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: new patch (necessary prerequisite for the following change)
drivers/iio/proximity/sx_common.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/proximity/sx_common.c b/drivers/iio/proximity/sx_common.c
index 8ad814d96b7e..9f2e47385198 100644
--- a/drivers/iio/proximity/sx_common.c
+++ b/drivers/iio/proximity/sx_common.c
@@ -434,7 +434,7 @@ static void sx_common_regulator_disable(void *_data)

#define SX_COMMON_SOFT_RESET 0xde

-static int sx_common_init_device(struct iio_dev *indio_dev)
+static int sx_common_init_device(struct device *dev, struct iio_dev *indio_dev)
{
struct sx_common_data *data = iio_priv(indio_dev);
struct sx_common_reg_default tmp;
@@ -456,8 +456,7 @@ static int sx_common_init_device(struct iio_dev *indio_dev)

/* Program defaults from constant or BIOS. */
for (i = 0; i < data->chip_info->num_default_regs; i++) {
- initval = data->chip_info->ops.get_default_reg(&indio_dev->dev,
- i, &tmp);
+ initval = data->chip_info->ops.get_default_reg(dev, i, &tmp);
ret = regmap_write(data->regmap, initval->reg, initval->def);
if (ret)
return ret;
@@ -530,7 +529,7 @@ int sx_common_probe(struct i2c_client *client,

i2c_set_clientdata(client, indio_dev);

- ret = sx_common_init_device(indio_dev);
+ ret = sx_common_init_device(dev, indio_dev);
if (ret)
return dev_err_probe(dev, ret, "Unable to initialize sensor\n");

--
2.35.1


2022-06-15 12:35:33

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v2 2/2] iio: proximity: sx_common: Allow IIO core to take care of firmware node

IIO core correctly will take care of firmware node if it's not set in
the driver. Drop ACPI and OF specifics from the driver and allow IIO
core to handle this.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no changes
drivers/iio/proximity/sx_common.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/iio/proximity/sx_common.c b/drivers/iio/proximity/sx_common.c
index 9f2e47385198..d70a6b4f0bf8 100644
--- a/drivers/iio/proximity/sx_common.c
+++ b/drivers/iio/proximity/sx_common.c
@@ -5,7 +5,6 @@
* Common part of most Semtech SAR sensor.
*/

-#include <linux/acpi.h>
#include <linux/bitops.h>
#include <linux/byteorder/generic.h>
#include <linux/delay.h>
@@ -519,8 +518,6 @@ int sx_common_probe(struct i2c_client *client,
if (ret)
return dev_err_probe(dev, ret, "error reading WHOAMI\n");

- ACPI_COMPANION_SET(&indio_dev->dev, ACPI_COMPANION(dev));
- indio_dev->dev.of_node = client->dev.of_node;
indio_dev->modes = INDIO_DIRECT_MODE;

indio_dev->channels = data->chip_info->iio_channels;
--
2.35.1

2022-06-28 22:11:43

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: proximity: sx_common: Don't use IIO device for properties

On Wed, Jun 15, 2022 at 02:47:45PM +0300, Andy Shevchenko wrote:
> It's not correct to use artificial device created by IIO core to
> retrieve device properties. Even ->get_default_reg() callback
> takes a simple struct device pointer which suggests it wants to
> operate on the real device.
>
> Correct this by replacing pointer to IIO device by a real device
> pointer in the caller of ->get_default_reg().

Gwendal, any comments on this version?

--
With Best Regards,
Andy Shevchenko


2022-06-28 23:42:17

by Gwendal Grignou

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: proximity: sx_common: Don't use IIO device for properties

On Tue, Jun 28, 2022 at 3:01 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Wed, Jun 15, 2022 at 02:47:45PM +0300, Andy Shevchenko wrote:
> > It's not correct to use artificial device created by IIO core to
> > retrieve device properties. Even ->get_default_reg() callback
> > takes a simple struct device pointer which suggests it wants to
> > operate on the real device.
> >
> > Correct this by replacing pointer to IIO device by a real device
> > pointer in the caller of ->get_default_reg().
>
> Gwendal, any comments on this version?
We can also replace '&indio_dev->dev' with 'dev' when setting
ACPI_COMPANION_SET() in the probe routine.

Reviewed-by: Gwendal Grignou <[email protected]>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

2022-06-29 10:01:58

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: proximity: sx_common: Don't use IIO device for properties

On Wed, Jun 29, 2022 at 1:41 AM Gwendal Grignou <[email protected]> wrote:
>
> On Tue, Jun 28, 2022 at 3:01 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Wed, Jun 15, 2022 at 02:47:45PM +0300, Andy Shevchenko wrote:
> > > It's not correct to use artificial device created by IIO core to
> > > retrieve device properties. Even ->get_default_reg() callback
> > > takes a simple struct device pointer which suggests it wants to
> > > operate on the real device.
> > >
> > > Correct this by replacing pointer to IIO device by a real device
> > > pointer in the caller of ->get_default_reg().
> >
> > Gwendal, any comments on this version?
> We can also replace '&indio_dev->dev' with 'dev' when setting
> ACPI_COMPANION_SET() in the probe routine.

It would make no sense, IIUC. In any case, the second patch in the
series, which was the same in v1 removes that completely.

> Reviewed-by: Gwendal Grignou <[email protected]>

Thanks!

--
With Best Regards,
Andy Shevchenko

2022-06-29 17:42:01

by Gwendal Grignou

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: proximity: sx_common: Allow IIO core to take care of firmware node

On Wed, Jun 15, 2022 at 4:47 AM Andy Shevchenko
<[email protected]> wrote:
>
> IIO core correctly will take care of firmware node if it's not set in
> the driver. Drop ACPI and OF specifics from the driver and allow IIO
> core to handle this.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Gwendal Grignou <[email protected]>
> ---
> v2: no changes
> drivers/iio/proximity/sx_common.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/iio/proximity/sx_common.c b/drivers/iio/proximity/sx_common.c
> index 9f2e47385198..d70a6b4f0bf8 100644
> --- a/drivers/iio/proximity/sx_common.c
> +++ b/drivers/iio/proximity/sx_common.c
> @@ -5,7 +5,6 @@
> * Common part of most Semtech SAR sensor.
> */
>
> -#include <linux/acpi.h>
> #include <linux/bitops.h>
> #include <linux/byteorder/generic.h>
> #include <linux/delay.h>
> @@ -519,8 +518,6 @@ int sx_common_probe(struct i2c_client *client,
> if (ret)
> return dev_err_probe(dev, ret, "error reading WHOAMI\n");
>
> - ACPI_COMPANION_SET(&indio_dev->dev, ACPI_COMPANION(dev));
> - indio_dev->dev.of_node = client->dev.of_node;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> indio_dev->channels = data->chip_info->iio_channels;
> --
> 2.35.1
>

2022-07-03 16:02:22

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: proximity: sx_common: Allow IIO core to take care of firmware node

On Wed, Jun 29, 2022 at 10:07:40AM -0700, Gwendal Grignou wrote:
> On Wed, Jun 15, 2022 at 4:47 AM Andy Shevchenko
> <[email protected]> wrote:
> >
> > IIO core correctly will take care of firmware node if it's not set in
> > the driver. Drop ACPI and OF specifics from the driver and allow IIO
> > core to handle this.
> >
> > Signed-off-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Gwendal Grignou <[email protected]>

Thanks!

Jonathan, I guess we are ready with this series. What do you think?

--
With Best Regards,
Andy Shevchenko


2022-07-16 15:59:55

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: proximity: sx_common: Allow IIO core to take care of firmware node

On Sun, 3 Jul 2022 18:45:44 +0300
Andy Shevchenko <[email protected]> wrote:

> On Wed, Jun 29, 2022 at 10:07:40AM -0700, Gwendal Grignou wrote:
> > On Wed, Jun 15, 2022 at 4:47 AM Andy Shevchenko
> > <[email protected]> wrote:
> > >
> > > IIO core correctly will take care of firmware node if it's not set in
> > > the driver. Drop ACPI and OF specifics from the driver and allow IIO
> > > core to handle this.
> > >
> > > Signed-off-by: Andy Shevchenko <[email protected]>
> > Reviewed-by: Gwendal Grignou <[email protected]>
>
> Thanks!
>
> Jonathan, I guess we are ready with this series. What do you think?
>
Gwendal's happy and seems straight forwards to me.

Applied to the togreg branch of iio.git and pushed out as testing.
Note, unlikely to make this cycle so I'll probably hold it in testing only
until I can rebase on rc1.

Thanks,

Jonathan