Linux IIO development
 help / color / mirror / Atom feed
* [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data()
@ 2023-08-07 17:25 Biju Das
  2023-08-07 17:25 ` [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe() Biju Das
  2023-08-08 13:04 ` [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Andy Shevchenko
  0 siblings, 2 replies; 7+ messages in thread
From: Biju Das @ 2023-08-07 17:25 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Biju Das, Peter Rosin, Lars-Peter Clausen, Andy Shevchenko,
	Andi Shyti, Jakob Hauser, Linus Walleij, Uwe Kleine-König,
	linux-iio, Geert Uytterhoeven, linux-renesas-soc

Simplify the probe() by replacing device_get_match_data() with
i2c_get_match_data().

While at it, drop unnecessary enum chip_ids by splitting the array
yas5xx_chip_info_tbl[] as individual variables.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 150 +++++++++++------------
 1 file changed, 71 insertions(+), 79 deletions(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index c5e485bfc6fc..0ff3d81e24b4 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -133,13 +133,6 @@
 /* Turn off device regulators etc after 5 seconds of inactivity */
 #define YAS5XX_AUTOSUSPEND_DELAY_MS	5000
 
-enum chip_ids {
-	yas530,
-	yas532,
-	yas533,
-	yas537,
-};
-
 static const int yas530_volatile_reg[] = {
 	YAS530_ACTUATE_INIT_COIL,
 	YAS530_MEASURE,
@@ -1321,67 +1314,68 @@ static int yas537_power_on(struct yas5xx *yas5xx)
 	return 0;
 }
 
-static const struct yas5xx_chip_info yas5xx_chip_info_tbl[] = {
-	[yas530] = {
-		.devid = YAS530_DEVICE_ID,
-		.product_name = "YAS530 MS-3E",
-		.version_names = { "A", "B" },
-		.volatile_reg = yas530_volatile_reg,
-		.volatile_reg_qty = ARRAY_SIZE(yas530_volatile_reg),
-		.scaling_val2 = 100000000, /* picotesla to Gauss */
-		.t_ref = 182, /* counts */
-		.min_temp_x10 = -620, /* 1/10:s degrees Celsius */
-		.get_measure = yas530_get_measure,
-		.get_calibration_data = yas530_get_calibration_data,
-		.dump_calibration = yas530_dump_calibration,
-		.measure_offsets = yas530_measure_offsets,
-		.power_on = yas530_power_on,
-	},
-	[yas532] = {
-		.devid = YAS532_DEVICE_ID,
-		.product_name = "YAS532 MS-3R",
-		.version_names = { "AB", "AC" },
-		.volatile_reg = yas530_volatile_reg,
-		.volatile_reg_qty = ARRAY_SIZE(yas530_volatile_reg),
-		.scaling_val2 = 100000, /* nanotesla to Gauss */
-		.t_ref = 390, /* counts */
-		.min_temp_x10 = -500, /* 1/10:s degrees Celsius */
-		.get_measure = yas530_get_measure,
-		.get_calibration_data = yas532_get_calibration_data,
-		.dump_calibration = yas530_dump_calibration,
-		.measure_offsets = yas530_measure_offsets,
-		.power_on = yas530_power_on,
-	},
-	[yas533] = {
-		.devid = YAS532_DEVICE_ID,
-		.product_name = "YAS533 MS-3F",
-		.version_names = { "AB", "AC" },
-		.volatile_reg = yas530_volatile_reg,
-		.volatile_reg_qty = ARRAY_SIZE(yas530_volatile_reg),
-		.scaling_val2 = 100000, /* nanotesla to Gauss */
-		.t_ref = 390, /* counts */
-		.min_temp_x10 = -500, /* 1/10:s degrees Celsius */
-		.get_measure = yas530_get_measure,
-		.get_calibration_data = yas532_get_calibration_data,
-		.dump_calibration = yas530_dump_calibration,
-		.measure_offsets = yas530_measure_offsets,
-		.power_on = yas530_power_on,
-	},
-	[yas537] = {
-		.devid = YAS537_DEVICE_ID,
-		.product_name = "YAS537 MS-3T",
-		.version_names = { "v0", "v1" }, /* version naming unknown */
-		.volatile_reg = yas537_volatile_reg,
-		.volatile_reg_qty = ARRAY_SIZE(yas537_volatile_reg),
-		.scaling_val2 = 100000, /* nanotesla to Gauss */
-		.t_ref = 8120, /* counts */
-		.min_temp_x10 = -3860, /* 1/10:s degrees Celsius */
-		.get_measure = yas537_get_measure,
-		.get_calibration_data = yas537_get_calibration_data,
-		.dump_calibration = yas537_dump_calibration,
-		/* .measure_offets is not needed for yas537 */
-		.power_on = yas537_power_on,
-	},
+static const struct yas5xx_chip_info yas530_chip_info = {
+	.devid = YAS530_DEVICE_ID,
+	.product_name = "YAS530 MS-3E",
+	.version_names = { "A", "B" },
+	.volatile_reg = yas530_volatile_reg,
+	.volatile_reg_qty = ARRAY_SIZE(yas530_volatile_reg),
+	.scaling_val2 = 100000000, /* picotesla to Gauss */
+	.t_ref = 182, /* counts */
+	.min_temp_x10 = -620, /* 1/10:s degrees Celsius */
+	.get_measure = yas530_get_measure,
+	.get_calibration_data = yas530_get_calibration_data,
+	.dump_calibration = yas530_dump_calibration,
+	.measure_offsets = yas530_measure_offsets,
+	.power_on = yas530_power_on,
+};
+
+static const struct yas5xx_chip_info yas532_chip_info = {
+	.devid = YAS532_DEVICE_ID,
+	.product_name = "YAS532 MS-3R",
+	.version_names = { "AB", "AC" },
+	.volatile_reg = yas530_volatile_reg,
+	.volatile_reg_qty = ARRAY_SIZE(yas530_volatile_reg),
+	.scaling_val2 = 100000, /* nanotesla to Gauss */
+	.t_ref = 390, /* counts */
+	.min_temp_x10 = -500, /* 1/10:s degrees Celsius */
+	.get_measure = yas530_get_measure,
+	.get_calibration_data = yas532_get_calibration_data,
+	.dump_calibration = yas530_dump_calibration,
+	.measure_offsets = yas530_measure_offsets,
+	.power_on = yas530_power_on,
+};
+
+static const struct yas5xx_chip_info yas533_chip_info = {
+	.devid = YAS532_DEVICE_ID,
+	.product_name = "YAS533 MS-3F",
+	.version_names = { "AB", "AC" },
+	.volatile_reg = yas530_volatile_reg,
+	.volatile_reg_qty = ARRAY_SIZE(yas530_volatile_reg),
+	.scaling_val2 = 100000, /* nanotesla to Gauss */
+	.t_ref = 390, /* counts */
+	.min_temp_x10 = -500, /* 1/10:s degrees Celsius */
+	.get_measure = yas530_get_measure,
+	.get_calibration_data = yas532_get_calibration_data,
+	.dump_calibration = yas530_dump_calibration,
+	.measure_offsets = yas530_measure_offsets,
+	.power_on = yas530_power_on,
+};
+
+static const struct yas5xx_chip_info yas537_chip_info = {
+	.devid = YAS537_DEVICE_ID,
+	.product_name = "YAS537 MS-3T",
+	.version_names = { "v0", "v1" }, /* version naming unknown */
+	.volatile_reg = yas537_volatile_reg,
+	.volatile_reg_qty = ARRAY_SIZE(yas537_volatile_reg),
+	.scaling_val2 = 100000, /* nanotesla to Gauss */
+	.t_ref = 8120, /* counts */
+	.min_temp_x10 = -3860, /* 1/10:s degrees Celsius */
+	.get_measure = yas537_get_measure,
+	.get_calibration_data = yas537_get_calibration_data,
+	.dump_calibration = yas537_dump_calibration,
+	/* .measure_offets is not needed for yas537 */
+	.power_on = yas537_power_on,
 };
 
 static int yas5xx_probe(struct i2c_client *i2c)
@@ -1434,9 +1428,7 @@ static int yas5xx_probe(struct i2c_client *i2c)
 		goto assert_reset;
 	}
 
-	ci = device_get_match_data(dev);
-	if (!ci)
-		ci = (const struct yas5xx_chip_info *)id->driver_data;
+	ci = i2c_get_match_data(i2c);
 	yas5xx->chip_info = ci;
 
 	ret = regmap_read(yas5xx->map, YAS5XX_DEVICE_ID, &id_check);
@@ -1582,19 +1574,19 @@ static DEFINE_RUNTIME_DEV_PM_OPS(yas5xx_dev_pm_ops, yas5xx_runtime_suspend,
 				 yas5xx_runtime_resume, NULL);
 
 static const struct i2c_device_id yas5xx_id[] = {
-	{"yas530", (kernel_ulong_t)&yas5xx_chip_info_tbl[yas530] },
-	{"yas532", (kernel_ulong_t)&yas5xx_chip_info_tbl[yas532] },
-	{"yas533", (kernel_ulong_t)&yas5xx_chip_info_tbl[yas533] },
-	{"yas537", (kernel_ulong_t)&yas5xx_chip_info_tbl[yas537] },
+	{"yas530", (kernel_ulong_t)&yas530_chip_info },
+	{"yas532", (kernel_ulong_t)&yas532_chip_info },
+	{"yas533", (kernel_ulong_t)&yas533_chip_info },
+	{"yas537", (kernel_ulong_t)&yas537_chip_info },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, yas5xx_id);
 
 static const struct of_device_id yas5xx_of_match[] = {
-	{ .compatible = "yamaha,yas530", &yas5xx_chip_info_tbl[yas530] },
-	{ .compatible = "yamaha,yas532", &yas5xx_chip_info_tbl[yas532] },
-	{ .compatible = "yamaha,yas533", &yas5xx_chip_info_tbl[yas533] },
-	{ .compatible = "yamaha,yas537", &yas5xx_chip_info_tbl[yas537] },
+	{ .compatible = "yamaha,yas530", &yas530_chip_info },
+	{ .compatible = "yamaha,yas532", &yas532_chip_info },
+	{ .compatible = "yamaha,yas533", &yas533_chip_info },
+	{ .compatible = "yamaha,yas537", &yas537_chip_info },
 	{}
 };
 MODULE_DEVICE_TABLE(of, yas5xx_of_match);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe()
  2023-08-07 17:25 [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Biju Das
@ 2023-08-07 17:25 ` Biju Das
  2023-08-08 13:05   ` Andy Shevchenko
  2023-08-08 13:04 ` [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Andy Shevchenko
  1 sibling, 1 reply; 7+ messages in thread
From: Biju Das @ 2023-08-07 17:25 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Biju Das, Peter Rosin, Lars-Peter Clausen, Andy Shevchenko,
	Andi Shyti, Uwe Kleine-König, Crt Mori, Dmitry Torokhov,
	linux-iio, Geert Uytterhoeven, linux-renesas-soc

Simplify the probe() by replacing device_get_match_data() and
i2c_client_get_device_id by i2c_get_match_data() as we have similar I2C,
ACPI and DT matching table.

While at it, replace name->chipset to make error message consistent.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/iio/magnetometer/ak8975.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index eb706d0bf70b..9c4d942ffab3 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -878,16 +878,13 @@ static irqreturn_t ak8975_handle_trigger(int irq, void *p)
 
 static int ak8975_probe(struct i2c_client *client)
 {
-	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	struct ak8975_data *data;
 	struct iio_dev *indio_dev;
 	struct gpio_desc *eoc_gpiod;
 	struct gpio_desc *reset_gpiod;
-	const void *match;
 	unsigned int i;
 	int err;
 	enum asahi_compass_chipset chipset;
-	const char *name = NULL;
 
 	/*
 	 * Grab and set up the supplied GPIO.
@@ -927,15 +924,8 @@ static int ak8975_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	/* id will be NULL when enumerated via ACPI */
-	match = device_get_match_data(&client->dev);
-	if (match) {
-		chipset = (uintptr_t)match;
-		name = dev_name(&client->dev);
-	} else if (id) {
-		chipset = (enum asahi_compass_chipset)(id->driver_data);
-		name = id->name;
-	} else
+	chipset = (uintptr_t)i2c_get_match_data(client);
+	if (!chipset)
 		return -ENOSYS;
 
 	for (i = 0; i < ARRAY_SIZE(ak_def_array); i++)
@@ -967,12 +957,13 @@ static int ak8975_probe(struct i2c_client *client)
 		dev_err(&client->dev, "Unexpected device\n");
 		goto power_off;
 	}
-	dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
+	dev_dbg(&client->dev, "Asahi compass chip %d\n", chipset);
 
 	/* Perform some basic start-of-day setup of the device. */
 	err = ak8975_setup(client);
 	if (err < 0) {
-		dev_err(&client->dev, "%s initialization fails\n", name);
+		dev_err(&client->dev, "Initialization failed for chip %d\n",
+			chipset);
 		goto power_off;
 	}
 
@@ -982,7 +973,7 @@ static int ak8975_probe(struct i2c_client *client)
 	indio_dev->info = &ak8975_info;
 	indio_dev->available_scan_masks = ak8975_scan_masks;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->name = name;
+	indio_dev->name = dev_name(&client->dev);
 
 	err = iio_triggered_buffer_setup(indio_dev, NULL, ak8975_handle_trigger,
 					 NULL);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data()
  2023-08-07 17:25 [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Biju Das
  2023-08-07 17:25 ` [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe() Biju Das
@ 2023-08-08 13:04 ` Andy Shevchenko
  2023-08-11  7:49   ` Biju Das
  1 sibling, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2023-08-08 13:04 UTC (permalink / raw)
  To: Biju Das
  Cc: Jonathan Cameron, Peter Rosin, Lars-Peter Clausen, Andi Shyti,
	Jakob Hauser, Linus Walleij, Uwe Kleine-König, linux-iio,
	Geert Uytterhoeven, linux-renesas-soc

On Mon, Aug 07, 2023 at 06:25:47PM +0100, Biju Das wrote:
> Simplify the probe() by replacing device_get_match_data() with
> i2c_get_match_data().

> While at it, drop unnecessary enum chip_ids by splitting the array
> yas5xx_chip_info_tbl[] as individual variables.

This should be in a separate change.
Personally I see no point in doing this.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe()
  2023-08-07 17:25 ` [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe() Biju Das
@ 2023-08-08 13:05   ` Andy Shevchenko
  2023-08-11 12:17     ` Biju Das
  2023-08-28 13:06     ` Jonathan Cameron
  0 siblings, 2 replies; 7+ messages in thread
From: Andy Shevchenko @ 2023-08-08 13:05 UTC (permalink / raw)
  To: Biju Das
  Cc: Jonathan Cameron, Peter Rosin, Lars-Peter Clausen, Andi Shyti,
	Uwe Kleine-König, Crt Mori, Dmitry Torokhov, linux-iio,
	Geert Uytterhoeven, linux-renesas-soc

On Mon, Aug 07, 2023 at 06:25:48PM +0100, Biju Das wrote:
> Simplify the probe() by replacing device_get_match_data() and
> i2c_client_get_device_id by i2c_get_match_data() as we have similar I2C,
> ACPI and DT matching table.

...

> -		name = dev_name(&client->dev);

> -		name = id->name;

> -	indio_dev->name = name;
> +	indio_dev->name = dev_name(&client->dev);

I believe this is an ABI breakage.

NAK.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data()
  2023-08-08 13:04 ` [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Andy Shevchenko
@ 2023-08-11  7:49   ` Biju Das
  0 siblings, 0 replies; 7+ messages in thread
From: Biju Das @ 2023-08-11  7:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Peter Rosin, Lars-Peter Clausen, Andi Shyti,
	Jakob Hauser, Linus Walleij, Uwe Kleine-König,
	linux-iio@vger.kernel.org, Geert Uytterhoeven,
	linux-renesas-soc@vger.kernel.org

Hi Andy Shevchenko,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use
> i2c_get_match_data()
> 
> On Mon, Aug 07, 2023 at 06:25:47PM +0100, Biju Das wrote:
> > Simplify the probe() by replacing device_get_match_data() with
> > i2c_get_match_data().
> 
> > While at it, drop unnecessary enum chip_ids by splitting the array
> > yas5xx_chip_info_tbl[] as individual variables.
> 
> This should be in a separate change.

OK, will make it as a separate change.

Cheers,
Biju

> Personally I see no point in doing this.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe()
  2023-08-08 13:05   ` Andy Shevchenko
@ 2023-08-11 12:17     ` Biju Das
  2023-08-28 13:06     ` Jonathan Cameron
  1 sibling, 0 replies; 7+ messages in thread
From: Biju Das @ 2023-08-11 12:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Peter Rosin, Lars-Peter Clausen, Andi Shyti,
	Uwe Kleine-König, Crt Mori, Dmitry Torokhov,
	linux-iio@vger.kernel.org, Geert Uytterhoeven,
	linux-renesas-soc@vger.kernel.org

Hi Andy Shevchenko,

Thanks for the feedback.

> Subject: Re: [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe()
> 
> On Mon, Aug 07, 2023 at 06:25:48PM +0100, Biju Das wrote:
> > Simplify the probe() by replacing device_get_match_data() and
> > i2c_client_get_device_id by i2c_get_match_data() as we have similar
> > I2C, ACPI and DT matching table.
> 
> ...
> 
> > -		name = dev_name(&client->dev);
> 
> > -		name = id->name;
> 
> > -	indio_dev->name = name;
> > +	indio_dev->name = dev_name(&client->dev);
> 
> I believe this is an ABI breakage.

OK, will drop this patch as future 
device_get_match_data() any will treat this as legacy table.

Cheers,
Biu



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe()
  2023-08-08 13:05   ` Andy Shevchenko
  2023-08-11 12:17     ` Biju Das
@ 2023-08-28 13:06     ` Jonathan Cameron
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2023-08-28 13:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Biju Das, Peter Rosin, Lars-Peter Clausen, Andi Shyti,
	Uwe Kleine-König, Crt Mori, Dmitry Torokhov, linux-iio,
	Geert Uytterhoeven, linux-renesas-soc

On Tue, 8 Aug 2023 16:05:48 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Mon, Aug 07, 2023 at 06:25:48PM +0100, Biju Das wrote:
> > Simplify the probe() by replacing device_get_match_data() and
> > i2c_client_get_device_id by i2c_get_match_data() as we have similar I2C,
> > ACPI and DT matching table.  
> 
> ...
> 
> > -		name = dev_name(&client->dev);  
> 
> > -		name = id->name;  
> 
> > -	indio_dev->name = name;
> > +	indio_dev->name = dev_name(&client->dev);  
> 
> I believe this is an ABI breakage.

Using dev_name(&client->dev) was an old bug but we missed it
in a few drivers for long enough that we didn't want to risk
breaking userspace by fixing it :( 

With hindsight we should have added more comments to the code
though so people would know to beware.

Jonathan

> 
> NAK.
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-08-28 13:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-07 17:25 [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Biju Das
2023-08-07 17:25 ` [PATCH 2/2] iio: magnetometer: ak8975: Simplify probe() Biju Das
2023-08-08 13:05   ` Andy Shevchenko
2023-08-11 12:17     ` Biju Das
2023-08-28 13:06     ` Jonathan Cameron
2023-08-08 13:04 ` [PATCH 1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() Andy Shevchenko
2023-08-11  7:49   ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox