diff mbox series

iio: proximity: sx9310: Convert enum->pointer for match data table

Message ID 20230812130130.123243-1-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Headers show
Series iio: proximity: sx9310: Convert enum->pointer for match data table | expand

Commit Message

Biju Das Aug. 12, 2023, 1:01 p.m. UTC
Convert enum->pointer for data in match data table, so that
device_get_match_data() can do match against OF/ACPI/I2C tables, once i2c
bus type match support added to it.

Add struct sx931x_info and replace enum->sx931x_info in the match table
and simplify sx9310_check_whoami().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/iio/proximity/sx9310.c | 44 +++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 19 deletions(-)

Comments

Andy Shevchenko Aug. 13, 2023, 8:09 p.m. UTC | #1
On Sat, Aug 12, 2023 at 4:01 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Convert enum->pointer for data in match data table, so that
> device_get_match_data() can do match against OF/ACPI/I2C tables, once i2c
> bus type match support added to it.
>
> Add struct sx931x_info and replace enum->sx931x_info in the match table
> and simplify sx9310_check_whoami().

...

> +       ddata = device_get_match_data(dev);
> +       if (ddata->whoami != whoami)
>                 return -EINVAL;

-ENODEV looks better.

...

> +static const struct sx931x_info sx9310_info = {
> +       .name = "sx9310",
> +       .whoami = SX9310_WHOAMI_VALUE

Keep trailing comma.

> +};
> +
> +static const struct sx931x_info sx9311_info = {
> +       .name = "sx9311",
> +       .whoami = SX9311_WHOAMI_VALUE

Ditto.

> +};

...

Otherwise looks good to me
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Biju Das Aug. 14, 2023, 8:11 a.m. UTC | #2
Hi Andy,

Thanks for the feedback.

> Subject: Re: [PATCH] iio: proximity: sx9310: Convert enum->pointer for
> match data table
> 
> On Sat, Aug 12, 2023 at 4:01 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> >
> > Convert enum->pointer for data in match data table, so that
> > device_get_match_data() can do match against OF/ACPI/I2C tables, once
> > i2c bus type match support added to it.
> >
> > Add struct sx931x_info and replace enum->sx931x_info in the match
> > table and simplify sx9310_check_whoami().
> 
> ...
> 
> > +       ddata = device_get_match_data(dev);
> > +       if (ddata->whoami != whoami)
> >                 return -EINVAL;
> 
> -ENODEV looks better.

OK.

> 
> ...
> 
> > +static const struct sx931x_info sx9310_info = {
> > +       .name = "sx9310",
> > +       .whoami = SX9310_WHOAMI_VALUE
> 
> Keep trailing comma.

OK.

> 
> > +};
> > +
> > +static const struct sx931x_info sx9311_info = {
> > +       .name = "sx9311",
> > +       .whoami = SX9311_WHOAMI_VALUE
> 
> Ditto.

OK.

Will fix this in next version, if there is no objections.

Cheers,
Biju

> 
> > +};
> 
> ...
> 
> Otherwise looks good to me
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> --
> With Best Regards,
> Andy Shevchenko
diff mbox series

Patch

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index d977aacb7491..0f1d5226a585 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -159,6 +159,11 @@  static_assert(SX9310_NUM_CHANNELS <= SX_COMMON_MAX_NUM_CHANNELS);
 }
 #define SX9310_CHANNEL(idx) SX9310_NAMED_CHANNEL(idx, NULL)
 
+struct sx931x_info {
+	const char *name;
+	unsigned int whoami;
+};
+
 static const struct iio_chan_spec sx9310_channels[] = {
 	SX9310_CHANNEL(0),			/* CS0 */
 	SX9310_CHANNEL(1),			/* CS1 */
@@ -902,7 +907,7 @@  static int sx9310_check_whoami(struct device *dev,
 			       struct iio_dev *indio_dev)
 {
 	struct sx_common_data *data = iio_priv(indio_dev);
-	unsigned int long ddata;
+	const struct sx931x_info *ddata;
 	unsigned int whoami;
 	int ret;
 
@@ -910,20 +915,11 @@  static int sx9310_check_whoami(struct device *dev,
 	if (ret)
 		return ret;
 
-	ddata = (uintptr_t)device_get_match_data(dev);
-	if (ddata != whoami)
+	ddata = device_get_match_data(dev);
+	if (ddata->whoami != whoami)
 		return -EINVAL;
 
-	switch (whoami) {
-	case SX9310_WHOAMI_VALUE:
-		indio_dev->name = "sx9310";
-		break;
-	case SX9311_WHOAMI_VALUE:
-		indio_dev->name = "sx9311";
-		break;
-	default:
-		return -ENODEV;
-	}
+	indio_dev->name = ddata->name;
 
 	return 0;
 }
@@ -1015,23 +1011,33 @@  static int sx9310_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(sx9310_pm_ops, sx9310_suspend, sx9310_resume);
 
+static const struct sx931x_info sx9310_info = {
+	.name = "sx9310",
+	.whoami = SX9310_WHOAMI_VALUE
+};
+
+static const struct sx931x_info sx9311_info = {
+	.name = "sx9311",
+	.whoami = SX9311_WHOAMI_VALUE
+};
+
 static const struct acpi_device_id sx9310_acpi_match[] = {
-	{ "STH9310", SX9310_WHOAMI_VALUE },
-	{ "STH9311", SX9311_WHOAMI_VALUE },
+	{ "STH9310", (kernel_ulong_t)&sx9310_info },
+	{ "STH9311", (kernel_ulong_t)&sx9311_info },
 	{}
 };
 MODULE_DEVICE_TABLE(acpi, sx9310_acpi_match);
 
 static const struct of_device_id sx9310_of_match[] = {
-	{ .compatible = "semtech,sx9310", (void *)SX9310_WHOAMI_VALUE },
-	{ .compatible = "semtech,sx9311", (void *)SX9311_WHOAMI_VALUE },
+	{ .compatible = "semtech,sx9310", &sx9310_info },
+	{ .compatible = "semtech,sx9311", &sx9311_info },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sx9310_of_match);
 
 static const struct i2c_device_id sx9310_id[] = {
-	{ "sx9310", SX9310_WHOAMI_VALUE },
-	{ "sx9311", SX9311_WHOAMI_VALUE },
+	{ "sx9310", (kernel_ulong_t)&sx9310_info },
+	{ "sx9311", (kernel_ulong_t)&sx9311_info },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, sx9310_id);