diff mbox series

[2/2] iio: magnetometer: ak8975: Simplify probe()

Message ID 20230807172548.258247-2-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Headers show
Series [1/2] iio: magnetometer: yamaha-yas530: Use i2c_get_match_data() | expand

Commit Message

Biju Das Aug. 7, 2023, 5:25 p.m. UTC
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(-)

Comments

Andy Shevchenko Aug. 8, 2023, 1:05 p.m. UTC | #1
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.
Biju Das Aug. 11, 2023, 12:17 p.m. UTC | #2
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
Jonathan Cameron Aug. 28, 2023, 1:06 p.m. UTC | #3
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.
>
diff mbox series

Patch

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);