All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/23] iio: accel: Convert to i2c's .probe_new
@ 2022-10-23 13:22 Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new() Uwe Kleine-König
                   ` (22 more replies)
  0 siblings, 23 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Cosmin Tanislav,
	Jonathan Cameron, Dan Robertson
  Cc: Wolfram Sang, linux-iio, kernel, Srinivas Pandruvada,
	Benjamin Mugnier, Peter Senna Tschudin, Paul Cercueil,
	Gwendal Grignou, Paul Lemmermann, Andy Shevchenko,
	Dmitry Rokosov, Vladimir Oltean, Yang Yingliang, wangjianli,
	Miaoqian Lin, Greg Kroah-Hartman, Jean Delvare, Ajay Gupta,
	Heikki Krogerus, Jeremy Kerr, Maximilian Luz, Dmitry Torokhov,
	Hans de Goede, Haibo Chen, Peter Rosin, Hans Verkuil,
	Antoniu Miclaus, Andy Shevchenko, Sebastian Reichel

Hello,

this series converts all drivers below iio/accel to use the (not very new)
.probe_new() probe routine. The motivation is to get rid of the implicit
i2c_match_id call for each .probe invokation that isn't used by a big part of
the drivers anyhow.

Best regards
Uwe

Uwe Kleine-König (23):
  iio: accel: adxl367: Convert to i2c's .probe_new()
  iio: accel: adxl372: Convert to i2c's .probe_new
  iio: accel: bma400: Convert to i2c's .probe_new
  iio: accel: bmc150: Convert to i2c's .probe_new
  iio: accel: da280: Convert to i2c's .probe_new
  iio: accel: da311: Convert to i2c's .probe_new()
  iio: accel: dmard06: Convert to i2c's .probe_new()
  iio: accel: dmard09: Convert to i2c's .probe_new()
  iio: accel: dmard10: Convert to i2c's .probe_new()
  iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  iio: accel: kxsd9: Convert to i2c's .probe_new()
  iio: accel: mc3230: Convert to i2c's .probe_new()
  iio: accel: mma7455: Convert to i2c's .probe_new
  iio: accel: mma7660: Convert to i2c's .probe_new()
  iio: accel: mma8452: Convert to i2c's .probe_new
  iio: accel: mma9551: Convert to i2c's .probe_new
  iio: accel: mma9553: Convert to i2c's .probe_new
  iio: accel: mxc4005: Convert to i2c's .probe_new()
  iio: accel: mxc6255: Convert to i2c's .probe_new()
  iio: accel: stk8312: Convert to i2c's .probe_new()
  iio: accel: stk8ba50: Convert to i2c's .probe_new()
  iio: accel: st_magn: Convert to i2c's .probe_new()
  iio: accel: vl6180: Convert to i2c's .probe_new()

 drivers/iio/accel/adxl367_i2c.c        |  5 ++--
 drivers/iio/accel/adxl372_i2c.c        | 18 ++++++-------
 drivers/iio/accel/bma400_i2c.c         | 18 ++++++-------
 drivers/iio/accel/bmc150-accel-i2c.c   | 37 +++++++++++++-------------
 drivers/iio/accel/da280.c              | 22 ++++++++-------
 drivers/iio/accel/da311.c              |  5 ++--
 drivers/iio/accel/dmard06.c            |  5 ++--
 drivers/iio/accel/dmard09.c            |  5 ++--
 drivers/iio/accel/dmard10.c            |  5 ++--
 drivers/iio/accel/kxcjk-1013.c         | 29 ++++++++++----------
 drivers/iio/accel/kxsd9-i2c.c          |  5 ++--
 drivers/iio/accel/mc3230.c             |  5 ++--
 drivers/iio/accel/mma7455_i2c.c        | 20 +++++++-------
 drivers/iio/accel/mma7660.c            |  5 ++--
 drivers/iio/accel/mma8452.c            | 28 +++++++++----------
 drivers/iio/accel/mma9551.c            | 19 +++++++------
 drivers/iio/accel/mma9553.c            | 21 +++++++--------
 drivers/iio/accel/mxc4005.c            |  5 ++--
 drivers/iio/accel/mxc6255.c            |  5 ++--
 drivers/iio/accel/stk8312.c            |  5 ++--
 drivers/iio/accel/stk8ba50.c           |  5 ++--
 drivers/iio/light/vl6180.c             |  5 ++--
 drivers/iio/magnetometer/st_magn_i2c.c |  5 ++--
 23 files changed, 133 insertions(+), 149 deletions(-)


base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780
-- 
2.37.2


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

* [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:43   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 02/23] iio: accel: adxl372: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (21 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Cosmin Tanislav, Jonathan Cameron
  Cc: Wolfram Sang, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/adxl367_i2c.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/adxl367_i2c.c b/drivers/iio/accel/adxl367_i2c.c
index 3606efa25835..070aad724abd 100644
--- a/drivers/iio/accel/adxl367_i2c.c
+++ b/drivers/iio/accel/adxl367_i2c.c
@@ -41,8 +41,7 @@ static const struct adxl367_ops adxl367_i2c_ops = {
 	.read_fifo = adxl367_i2c_read_fifo,
 };
 
-static int adxl367_i2c_probe(struct i2c_client *client,
-			     const struct i2c_device_id *id)
+static int adxl367_i2c_probe(struct i2c_client *client)
 {
 	struct adxl367_i2c_state *st;
 	struct regmap *regmap;
@@ -78,7 +77,7 @@ static struct i2c_driver adxl367_i2c_driver = {
 		.name = "adxl367_i2c",
 		.of_match_table = adxl367_of_match,
 	},
-	.probe = adxl367_i2c_probe,
+	.probe_new = adxl367_i2c_probe,
 	.id_table = adxl367_i2c_id,
 };
 
-- 
2.37.2


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

* [PATCH 02/23] iio: accel: adxl372: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 03/23] iio: accel: bma400: " Uwe Kleine-König
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron
  Cc: Wolfram Sang, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/adxl372_i2c.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/adxl372_i2c.c b/drivers/iio/accel/adxl372_i2c.c
index 4efb70a5fe40..5a21c1bcf2d8 100644
--- a/drivers/iio/accel/adxl372_i2c.c
+++ b/drivers/iio/accel/adxl372_i2c.c
@@ -18,9 +18,15 @@ static const struct regmap_config adxl372_regmap_config = {
 	.readable_noinc_reg = adxl372_readable_noinc_reg,
 };
 
-static int adxl372_i2c_probe(struct i2c_client *client,
-			     const struct i2c_device_id *id)
+static const struct i2c_device_id adxl372_i2c_id[] = {
+	{ "adxl372", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, adxl372_i2c_id);
+
+static int adxl372_i2c_probe(struct i2c_client *client)
 {
+	const struct i2c_device_id *id = i2c_match_id(adxl372_i2c_id, client);
 	struct regmap *regmap;
 	unsigned int regval;
 	int ret;
@@ -41,12 +47,6 @@ static int adxl372_i2c_probe(struct i2c_client *client,
 	return adxl372_probe(&client->dev, regmap, client->irq, id->name);
 }
 
-static const struct i2c_device_id adxl372_i2c_id[] = {
-	{ "adxl372", 0 },
-	{}
-};
-MODULE_DEVICE_TABLE(i2c, adxl372_i2c_id);
-
 static const struct of_device_id adxl372_of_match[] = {
 	{ .compatible = "adi,adxl372" },
 	{ }
@@ -58,7 +58,7 @@ static struct i2c_driver adxl372_i2c_driver = {
 		.name = "adxl372_i2c",
 		.of_match_table = adxl372_of_match,
 	},
-	.probe = adxl372_i2c_probe,
+	.probe_new = adxl372_i2c_probe,
 	.id_table = adxl372_i2c_id,
 };
 
-- 
2.37.2


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

* [PATCH 03/23] iio: accel: bma400: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new() Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 02/23] iio: accel: adxl372: Convert to i2c's .probe_new Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 04/23] iio: accel: bmc150: " Uwe Kleine-König
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Dan Robertson, Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/bma400_i2c.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/bma400_i2c.c b/drivers/iio/accel/bma400_i2c.c
index 1ba2a982ea73..cf7a830d6baf 100644
--- a/drivers/iio/accel/bma400_i2c.c
+++ b/drivers/iio/accel/bma400_i2c.c
@@ -13,10 +13,16 @@
 
 #include "bma400.h"
 
-static int bma400_i2c_probe(struct i2c_client *client,
-			    const struct i2c_device_id *id)
+static const struct i2c_device_id bma400_i2c_ids[] = {
+	{ "bma400", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, bma400_i2c_ids);
+
+static int bma400_i2c_probe(struct i2c_client *client)
 {
 	struct regmap *regmap;
+	const struct i2c_device_id *id = i2c_match_id(bma400_i2c_ids, client);
 
 	regmap = devm_regmap_init_i2c(client, &bma400_regmap_config);
 	if (IS_ERR(regmap)) {
@@ -27,12 +33,6 @@ static int bma400_i2c_probe(struct i2c_client *client,
 	return bma400_probe(&client->dev, regmap, client->irq, id->name);
 }
 
-static const struct i2c_device_id bma400_i2c_ids[] = {
-	{ "bma400", 0 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, bma400_i2c_ids);
-
 static const struct of_device_id bma400_of_i2c_match[] = {
 	{ .compatible = "bosch,bma400" },
 	{ }
@@ -44,7 +44,7 @@ static struct i2c_driver bma400_i2c_driver = {
 		.name = "bma400",
 		.of_match_table = bma400_of_i2c_match,
 	},
-	.probe    = bma400_i2c_probe,
+	.probe_new = bma400_i2c_probe,
 	.id_table = bma400_i2c_ids,
 };
 
-- 
2.37.2


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

* [PATCH 04/23] iio: accel: bmc150: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 03/23] iio: accel: bma400: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 05/23] iio: accel: da280: " Uwe Kleine-König
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen
  Cc: Wolfram Sang, Srinivas Pandruvada, Benjamin Mugnier,
	Peter Senna Tschudin, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/bmc150-accel-i2c.c | 37 ++++++++++++++--------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index be8cc598b88e..0184106d0b65 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -171,9 +171,24 @@ static void bmc150_acpi_dual_accel_probe(struct i2c_client *client) {}
 static void bmc150_acpi_dual_accel_remove(struct i2c_client *client) {}
 #endif
 
-static int bmc150_accel_probe(struct i2c_client *client,
-			      const struct i2c_device_id *id)
+static const struct i2c_device_id bmc150_accel_id[] = {
+	{"bma222"},
+	{"bma222e"},
+	{"bma250e"},
+	{"bma253"},
+	{"bma254"},
+	{"bma255"},
+	{"bma280"},
+	{"bmc150_accel"},
+	{"bmc156_accel", BOSCH_BMC156},
+	{"bmi055_accel"},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, bmc150_accel_id);
+
+static int bmc150_accel_probe(struct i2c_client *client)
 {
+	const struct i2c_device_id *id = i2c_match_id(bmc150_accel_id, client);
 	struct regmap *regmap;
 	const char *name = NULL;
 	enum bmc150_type type = BOSCH_UNKNOWN;
@@ -231,22 +246,6 @@ static const struct acpi_device_id bmc150_accel_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, bmc150_accel_acpi_match);
 
-static const struct i2c_device_id bmc150_accel_id[] = {
-	{"bma222"},
-	{"bma222e"},
-	{"bma250e"},
-	{"bma253"},
-	{"bma254"},
-	{"bma255"},
-	{"bma280"},
-	{"bmc150_accel"},
-	{"bmc156_accel", BOSCH_BMC156},
-	{"bmi055_accel"},
-	{}
-};
-
-MODULE_DEVICE_TABLE(i2c, bmc150_accel_id);
-
 static const struct of_device_id bmc150_accel_of_match[] = {
 	{ .compatible = "bosch,bma222" },
 	{ .compatible = "bosch,bma222e" },
@@ -269,7 +268,7 @@ static struct i2c_driver bmc150_accel_driver = {
 		.acpi_match_table = ACPI_PTR(bmc150_accel_acpi_match),
 		.pm	= &bmc150_accel_pm_ops,
 	},
-	.probe		= bmc150_accel_probe,
+	.probe_new	= bmc150_accel_probe,
 	.remove		= bmc150_accel_remove,
 	.id_table	= bmc150_accel_id,
 };
-- 
2.37.2


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

* [PATCH 05/23] iio: accel: da280: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (3 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 04/23] iio: accel: bmc150: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 06/23] iio: accel: da311: Convert to i2c's .probe_new() Uwe Kleine-König
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/da280.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/da280.c b/drivers/iio/accel/da280.c
index 04e9c5678964..72a6a835ee74 100644
--- a/drivers/iio/accel/da280.c
+++ b/drivers/iio/accel/da280.c
@@ -105,8 +105,14 @@ static void da280_disable(void *client)
 	da280_enable(client, false);
 }
 
-static int da280_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static const struct i2c_device_id da280_i2c_id[] = {
+	{ "da226", da226 },
+	{ "da280", da280 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, da280_i2c_id);
+
+static int da280_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -131,6 +137,9 @@ static int da280_probe(struct i2c_client *client,
 	if (ACPI_HANDLE(&client->dev)) {
 		chip = da280_match_acpi_device(&client->dev);
 	} else {
+		const struct i2c_device_id *id =
+			i2c_match_id(da280_i2c_id, client);
+
 		chip = id->driver_data;
 	}
 
@@ -171,20 +180,13 @@ static const struct acpi_device_id da280_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, da280_acpi_match);
 
-static const struct i2c_device_id da280_i2c_id[] = {
-	{ "da226", da226 },
-	{ "da280", da280 },
-	{}
-};
-MODULE_DEVICE_TABLE(i2c, da280_i2c_id);
-
 static struct i2c_driver da280_driver = {
 	.driver = {
 		.name = "da280",
 		.acpi_match_table = ACPI_PTR(da280_acpi_match),
 		.pm = pm_sleep_ptr(&da280_pm_ops),
 	},
-	.probe		= da280_probe,
+	.probe_new	= da280_probe,
 	.id_table	= da280_i2c_id,
 };
 
-- 
2.37.2


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

* [PATCH 06/23] iio: accel: da311: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (4 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 05/23] iio: accel: da280: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:51   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 07/23] iio: accel: dmard06: " Uwe Kleine-König
                   ` (16 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/da311.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/da311.c b/drivers/iio/accel/da311.c
index ec4e29d260f7..080335fa2ad6 100644
--- a/drivers/iio/accel/da311.c
+++ b/drivers/iio/accel/da311.c
@@ -217,8 +217,7 @@ static void da311_disable(void *client)
 	da311_enable(client, false);
 }
 
-static int da311_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int da311_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -279,7 +278,7 @@ static struct i2c_driver da311_driver = {
 		.name = "da311",
 		.pm = pm_sleep_ptr(&da311_pm_ops),
 	},
-	.probe		= da311_probe,
+	.probe_new	= da311_probe,
 	.id_table	= da311_i2c_id,
 };
 
-- 
2.37.2


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

* [PATCH 07/23] iio: accel: dmard06: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (5 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 06/23] iio: accel: da311: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:51   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 08/23] iio: accel: dmard09: " Uwe Kleine-König
                   ` (15 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/dmard06.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/dmard06.c b/drivers/iio/accel/dmard06.c
index 4b69c8530f5e..7390509aaac0 100644
--- a/drivers/iio/accel/dmard06.c
+++ b/drivers/iio/accel/dmard06.c
@@ -125,8 +125,7 @@ static const struct iio_info dmard06_info = {
 	.read_raw	= dmard06_read_raw,
 };
 
-static int dmard06_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int dmard06_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -218,7 +217,7 @@ static const struct of_device_id dmard06_of_match[] = {
 MODULE_DEVICE_TABLE(of, dmard06_of_match);
 
 static struct i2c_driver dmard06_driver = {
-	.probe = dmard06_probe,
+	.probe_new = dmard06_probe,
 	.id_table = dmard06_id,
 	.driver = {
 		.name = DMARD06_DRV_NAME,
-- 
2.37.2


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

* [PATCH 08/23] iio: accel: dmard09: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (6 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 07/23] iio: accel: dmard06: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:52   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 09/23] iio: accel: dmard10: " Uwe Kleine-König
                   ` (14 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Gwendal Grignou,
	Paul Lemmermann, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/dmard09.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/dmard09.c b/drivers/iio/accel/dmard09.c
index cb0246ca72f3..4b7a537f617d 100644
--- a/drivers/iio/accel/dmard09.c
+++ b/drivers/iio/accel/dmard09.c
@@ -88,8 +88,7 @@ static const struct iio_info dmard09_info = {
 	.read_raw	= dmard09_read_raw,
 };
 
-static int dmard09_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int dmard09_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -136,7 +135,7 @@ static struct i2c_driver dmard09_driver = {
 	.driver = {
 		.name = DMARD09_DRV_NAME
 	},
-	.probe = dmard09_probe,
+	.probe_new = dmard09_probe,
 	.id_table = dmard09_id,
 };
 
-- 
2.37.2


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

* [PATCH 09/23] iio: accel: dmard10: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (7 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 08/23] iio: accel: dmard09: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:53   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (13 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/dmard10.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/dmard10.c b/drivers/iio/accel/dmard10.c
index 8ac62ec0a04a..07e68aed8a13 100644
--- a/drivers/iio/accel/dmard10.c
+++ b/drivers/iio/accel/dmard10.c
@@ -175,8 +175,7 @@ static void dmard10_shutdown_cleanup(void *client)
 	dmard10_shutdown(client);
 }
 
-static int dmard10_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int dmard10_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -242,7 +241,7 @@ static struct i2c_driver dmard10_driver = {
 		.name = "dmard10",
 		.pm = pm_sleep_ptr(&dmard10_pm_ops),
 	},
-	.probe		= dmard10_probe,
+	.probe_new	= dmard10_probe,
 	.id_table	= dmard10_i2c_id,
 };
 
-- 
2.37.2


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

* [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (8 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 09/23] iio: accel: dmard10: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 19:06   ` Andy Shevchenko
  2022-10-23 13:22 ` [PATCH 11/23] iio: accel: kxsd9: Convert to i2c's .probe_new() Uwe Kleine-König
                   ` (12 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Dmitry Rokosov, Vladimir Oltean, Yang Yingliang, wangjianli,
	Miaoqian Lin, Gwendal Grignou, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/kxcjk-1013.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index adc66b3615c0..e043dd698747 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1432,9 +1432,20 @@ static void kxcjk1013_disable_regulators(void *d)
 	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
 }
 
-static int kxcjk1013_probe(struct i2c_client *client,
-			   const struct i2c_device_id *id)
+static const struct i2c_device_id kxcjk1013_id[] = {
+	{"kxcjk1013", KXCJK1013},
+	{"kxcj91008", KXCJ91008},
+	{"kxtj21009", KXTJ21009},
+	{"kxtf9",     KXTF9},
+	{"kx023-1025", KX0231025},
+	{"SMO8500",   KXCJ91008},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
+
+static int kxcjk1013_probe(struct i2c_client *client)
 {
+	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
 	struct kxcjk1013_data *data;
 	struct iio_dev *indio_dev;
 	struct kxcjk_1013_platform_data *pdata;
@@ -1720,18 +1731,6 @@ static const struct acpi_device_id kx_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
 
-static const struct i2c_device_id kxcjk1013_id[] = {
-	{"kxcjk1013", KXCJK1013},
-	{"kxcj91008", KXCJ91008},
-	{"kxtj21009", KXTJ21009},
-	{"kxtf9",     KXTF9},
-	{"kx023-1025", KX0231025},
-	{"SMO8500",   KXCJ91008},
-	{}
-};
-
-MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
-
 static const struct of_device_id kxcjk1013_of_match[] = {
 	{ .compatible = "kionix,kxcjk1013", },
 	{ .compatible = "kionix,kxcj91008", },
@@ -1749,7 +1748,7 @@ static struct i2c_driver kxcjk1013_driver = {
 		.of_match_table = kxcjk1013_of_match,
 		.pm	= &kxcjk1013_pm_ops,
 	},
-	.probe		= kxcjk1013_probe,
+	.probe_new	= kxcjk1013_probe,
 	.remove		= kxcjk1013_remove,
 	.id_table	= kxcjk1013_id,
 };
-- 
2.37.2


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

* [PATCH 11/23] iio: accel: kxsd9: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (9 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:54   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 12/23] iio: accel: mc3230: " Uwe Kleine-König
                   ` (11 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Greg Kroah-Hartman,
	Jean Delvare, Ajay Gupta, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/kxsd9-i2c.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
index 61346ea8ef19..6b3683ddce36 100644
--- a/drivers/iio/accel/kxsd9-i2c.c
+++ b/drivers/iio/accel/kxsd9-i2c.c
@@ -10,8 +10,7 @@
 
 #include "kxsd9.h"
 
-static int kxsd9_i2c_probe(struct i2c_client *i2c,
-			   const struct i2c_device_id *id)
+static int kxsd9_i2c_probe(struct i2c_client *i2c)
 {
 	static const struct regmap_config config = {
 		.reg_bits = 8,
@@ -55,7 +54,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
 		.of_match_table = kxsd9_of_match,
 		.pm = pm_ptr(&kxsd9_dev_pm_ops),
 	},
-	.probe		= kxsd9_i2c_probe,
+	.probe_new	= kxsd9_i2c_probe,
 	.remove		= kxsd9_i2c_remove,
 	.id_table	= kxsd9_i2c_id,
 };
-- 
2.37.2


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

* [PATCH 12/23] iio: accel: mc3230: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (10 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 11/23] iio: accel: kxsd9: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:57   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 13/23] iio: accel: mma7455: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (10 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Heikki Krogerus, Ajay Gupta,
	Jeremy Kerr, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mc3230.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c
index 2462000e0519..efc21871de42 100644
--- a/drivers/iio/accel/mc3230.c
+++ b/drivers/iio/accel/mc3230.c
@@ -106,8 +106,7 @@ static const struct iio_info mc3230_info = {
 	.read_raw	= mc3230_read_raw,
 };
 
-static int mc3230_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int mc3230_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -191,7 +190,7 @@ static struct i2c_driver mc3230_driver = {
 		.name = "mc3230",
 		.pm = pm_sleep_ptr(&mc3230_pm_ops),
 	},
-	.probe		= mc3230_probe,
+	.probe_new	= mc3230_probe,
 	.remove		= mc3230_remove,
 	.id_table	= mc3230_i2c_id,
 };
-- 
2.37.2


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

* [PATCH 13/23] iio: accel: mma7455: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (11 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 12/23] iio: accel: mc3230: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 14/23] iio: accel: mma7660: Convert to i2c's .probe_new() Uwe Kleine-König
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Vladimir Oltean,
	Maximilian Luz, Jeremy Kerr, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Alternatively it would be possible to just pass i2c->name to
mma7455_core_probe(). That's a simpler patch but that changes things
in case id == NULL, so I didn't do that here even though it might be
considered an improvement.
---
 drivers/iio/accel/mma7455_i2c.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
index c63b321b01cd..9518bd81f9e5 100644
--- a/drivers/iio/accel/mma7455_i2c.c
+++ b/drivers/iio/accel/mma7455_i2c.c
@@ -10,9 +10,16 @@
 
 #include "mma7455.h"
 
-static int mma7455_i2c_probe(struct i2c_client *i2c,
-			     const struct i2c_device_id *id)
+static const struct i2c_device_id mma7455_i2c_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
+
+static int mma7455_i2c_probe(struct i2c_client *i2c)
 {
+	const struct i2c_device_id *id = i2c_match_id(mma7455_i2c_ids, i2c);
 	struct regmap *regmap;
 	const char *name = NULL;
 
@@ -31,13 +38,6 @@ static void mma7455_i2c_remove(struct i2c_client *i2c)
 	mma7455_core_remove(&i2c->dev);
 }
 
-static const struct i2c_device_id mma7455_i2c_ids[] = {
-	{ "mma7455", 0 },
-	{ "mma7456", 0 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
-
 static const struct of_device_id mma7455_of_match[] = {
 	{ .compatible = "fsl,mma7455" },
 	{ .compatible = "fsl,mma7456" },
@@ -46,7 +46,7 @@ static const struct of_device_id mma7455_of_match[] = {
 MODULE_DEVICE_TABLE(of, mma7455_of_match);
 
 static struct i2c_driver mma7455_i2c_driver = {
-	.probe = mma7455_i2c_probe,
+	.probe_new = mma7455_i2c_probe,
 	.remove = mma7455_i2c_remove,
 	.id_table = mma7455_i2c_ids,
 	.driver = {
-- 
2.37.2


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

* [PATCH 14/23] iio: accel: mma7660: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (12 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 13/23] iio: accel: mma7455: Convert to i2c's .probe_new Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:55   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 15/23] iio: accel: mma8452: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (8 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Dmitry Torokhov, Vladimir Oltean, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mma7660.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
index 85829990bbad..b279ca4dcdc0 100644
--- a/drivers/iio/accel/mma7660.c
+++ b/drivers/iio/accel/mma7660.c
@@ -169,8 +169,7 @@ static const struct iio_info mma7660_info = {
 	.attrs			= &mma7660_attribute_group,
 };
 
-static int mma7660_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int mma7660_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -267,7 +266,7 @@ static struct i2c_driver mma7660_driver = {
 		.of_match_table = mma7660_of_match,
 		.acpi_match_table = mma7660_acpi_id,
 	},
-	.probe		= mma7660_probe,
+	.probe_new	= mma7660_probe,
 	.remove		= mma7660_remove,
 	.id_table	= mma7660_i2c_id,
 };
-- 
2.37.2


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

* [PATCH 15/23] iio: accel: mma8452: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (13 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 14/23] iio: accel: mma7660: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 13:22 ` [PATCH 16/23] iio: accel: mma9551: " Uwe Kleine-König
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Hans de Goede, Haibo Chen,
	linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mma8452.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3ba28c2ff68a..2ce13c051363 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1545,8 +1545,18 @@ static const struct of_device_id mma8452_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mma8452_dt_ids);
 
-static int mma8452_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static const struct i2c_device_id mma8452_id[] = {
+	{ "mma8451", mma8451 },
+	{ "mma8452", mma8452 },
+	{ "mma8453", mma8453 },
+	{ "mma8652", mma8652 },
+	{ "mma8653", mma8653 },
+	{ "fxls8471", fxls8471 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma8452_id);
+
+static int mma8452_probe(struct i2c_client *client)
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
@@ -1562,6 +1572,7 @@ static int mma8452_probe(struct i2c_client *client,
 
 	data->chip_info = device_get_match_data(&client->dev);
 	if (!data->chip_info) {
+		const struct i2c_device_id *id = i2c_match_id(mma8452_id, client);
 		if (id) {
 			data->chip_info = &mma_chip_info_table[id->driver_data];
 		} else {
@@ -1829,24 +1840,13 @@ static const struct dev_pm_ops mma8452_pm_ops = {
 			   mma8452_runtime_resume, NULL)
 };
 
-static const struct i2c_device_id mma8452_id[] = {
-	{ "mma8451", mma8451 },
-	{ "mma8452", mma8452 },
-	{ "mma8453", mma8453 },
-	{ "mma8652", mma8652 },
-	{ "mma8653", mma8653 },
-	{ "fxls8471", fxls8471 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, mma8452_id);
-
 static struct i2c_driver mma8452_driver = {
 	.driver = {
 		.name	= "mma8452",
 		.of_match_table = mma8452_dt_ids,
 		.pm	= &mma8452_pm_ops,
 	},
-	.probe = mma8452_probe,
+	.probe_new = mma8452_probe,
 	.remove = mma8452_remove,
 	.id_table = mma8452_id,
 };
-- 
2.37.2


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

* [PATCH 16/23] iio: accel: mma9551: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (14 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 15/23] iio: accel: mma8452: Convert to i2c's .probe_new Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 19:06   ` Andy Shevchenko
  2022-10-23 13:22 ` [PATCH 17/23] iio: accel: mma9553: " Uwe Kleine-König
                   ` (6 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko, Peter Rosin,
	Maximilian Luz, Miaoqian Lin, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mma9551.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
index f7a793f4a8e3..f7c72e8d9fb8 100644
--- a/drivers/iio/accel/mma9551.c
+++ b/drivers/iio/accel/mma9551.c
@@ -446,9 +446,15 @@ static const char *mma9551_match_acpi_device(struct device *dev)
 	return dev_name(dev);
 }
 
-static int mma9551_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static const struct i2c_device_id mma9551_id[] = {
+	{"mma9551", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, mma9551_id);
+
+static int mma9551_probe(struct i2c_client *client)
 {
+	const struct i2c_device_id *id = i2c_match_id(mma9551_id, client);
 	struct mma9551_data *data;
 	struct iio_dev *indio_dev;
 	const char *name = NULL;
@@ -594,20 +600,13 @@ static const struct acpi_device_id mma9551_acpi_match[] = {
 
 MODULE_DEVICE_TABLE(acpi, mma9551_acpi_match);
 
-static const struct i2c_device_id mma9551_id[] = {
-	{"mma9551", 0},
-	{}
-};
-
-MODULE_DEVICE_TABLE(i2c, mma9551_id);
-
 static struct i2c_driver mma9551_driver = {
 	.driver = {
 		   .name = MMA9551_DRV_NAME,
 		   .acpi_match_table = ACPI_PTR(mma9551_acpi_match),
 		   .pm = pm_ptr(&mma9551_pm_ops),
 		   },
-	.probe = mma9551_probe,
+	.probe_new = mma9551_probe,
 	.remove = mma9551_remove,
 	.id_table = mma9551_id,
 };
-- 
2.37.2


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

* [PATCH 17/23] iio: accel: mma9553: Convert to i2c's .probe_new
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (15 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 16/23] iio: accel: mma9551: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-23 19:07   ` Andy Shevchenko
  2022-10-23 13:22 ` [PATCH 18/23] iio: accel: mxc4005: Convert to i2c's .probe_new() Uwe Kleine-König
                   ` (5 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Peter Senna Tschudin, Hans Verkuil, Miaoqian Lin,
	Antoniu Miclaus, linux-iio, kernel

.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in .probe(). The device_id array has to move up for that
to work.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mma9553.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 2da0e005b13e..0aa6f2092108 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -1073,9 +1073,16 @@ static const char *mma9553_match_acpi_device(struct device *dev)
 	return dev_name(dev);
 }
 
-static int mma9553_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+
+static const struct i2c_device_id mma9553_id[] = {
+	{"mma9553", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, mma9553_id);
+
+static int mma9553_probe(struct i2c_client *client)
 {
+	const struct i2c_device_id *id = i2c_match_id(mma9553_id, client);
 	struct mma9553_data *data;
 	struct iio_dev *indio_dev;
 	const char *name = NULL;
@@ -1230,23 +1237,15 @@ static const struct acpi_device_id mma9553_acpi_match[] = {
 	{"MMA9553", 0},
 	{},
 };
-
 MODULE_DEVICE_TABLE(acpi, mma9553_acpi_match);
 
-static const struct i2c_device_id mma9553_id[] = {
-	{"mma9553", 0},
-	{},
-};
-
-MODULE_DEVICE_TABLE(i2c, mma9553_id);
-
 static struct i2c_driver mma9553_driver = {
 	.driver = {
 		   .name = MMA9553_DRV_NAME,
 		   .acpi_match_table = ACPI_PTR(mma9553_acpi_match),
 		   .pm = pm_ptr(&mma9553_pm_ops),
 		   },
-	.probe = mma9553_probe,
+	.probe_new = mma9553_probe,
 	.remove = mma9553_remove,
 	.id_table = mma9553_id,
 };
-- 
2.37.2


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

* [PATCH 18/23] iio: accel: mxc4005: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (16 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 17/23] iio: accel: mma9553: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:56   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 19/23] iio: accel: mxc6255: " Uwe Kleine-König
                   ` (4 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Dmitry Rokosov, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mxc4005.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
index df600d2917c0..b146fc82738f 100644
--- a/drivers/iio/accel/mxc4005.c
+++ b/drivers/iio/accel/mxc4005.c
@@ -385,8 +385,7 @@ static int mxc4005_chip_init(struct mxc4005_data *data)
 	return 0;
 }
 
-static int mxc4005_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int mxc4005_probe(struct i2c_client *client)
 {
 	struct mxc4005_data *data;
 	struct iio_dev *indio_dev;
@@ -489,7 +488,7 @@ static struct i2c_driver mxc4005_driver = {
 		.name = MXC4005_DRV_NAME,
 		.acpi_match_table = ACPI_PTR(mxc4005_acpi_match),
 	},
-	.probe		= mxc4005_probe,
+	.probe_new	= mxc4005_probe,
 	.id_table	= mxc4005_id,
 };
 
-- 
2.37.2


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

* [PATCH 19/23] iio: accel: mxc6255: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (17 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 18/23] iio: accel: mxc4005: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:57   ` Jonathan Cameron
  2022-10-23 13:22 ` [PATCH 20/23] iio: accel: stk8312: " Uwe Kleine-König
                   ` (3 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/mxc6255.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mxc6255.c b/drivers/iio/accel/mxc6255.c
index 9aeeadc420d3..aa2e660545f8 100644
--- a/drivers/iio/accel/mxc6255.c
+++ b/drivers/iio/accel/mxc6255.c
@@ -113,8 +113,7 @@ static const struct regmap_config mxc6255_regmap_config = {
 	.readable_reg = mxc6255_is_readable_reg,
 };
 
-static int mxc6255_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int mxc6255_probe(struct i2c_client *client)
 {
 	struct mxc6255_data *data;
 	struct iio_dev *indio_dev;
@@ -184,7 +183,7 @@ static struct i2c_driver mxc6255_driver = {
 		.name = MXC6255_DRV_NAME,
 		.acpi_match_table = ACPI_PTR(mxc6255_acpi_match),
 	},
-	.probe		= mxc6255_probe,
+	.probe_new	= mxc6255_probe,
 	.id_table	= mxc6255_id,
 };
 
-- 
2.37.2


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

* [PATCH 20/23] iio: accel: stk8312: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (18 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 19/23] iio: accel: mxc6255: " Uwe Kleine-König
@ 2022-10-23 13:22 ` Uwe Kleine-König
  2022-10-29 11:58   ` Jonathan Cameron
  2022-10-23 13:23 ` [PATCH 21/23] iio: accel: stk8ba50: " Uwe Kleine-König
                   ` (2 subsequent siblings)
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Jean Delvare, Gwendal Grignou,
	Peter Rosin, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/stk8312.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/stk8312.c b/drivers/iio/accel/stk8312.c
index 7b1d6fb692b3..68f680db7505 100644
--- a/drivers/iio/accel/stk8312.c
+++ b/drivers/iio/accel/stk8312.c
@@ -498,8 +498,7 @@ static const struct iio_buffer_setup_ops stk8312_buffer_setup_ops = {
 	.postdisable = stk8312_buffer_postdisable,
 };
 
-static int stk8312_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int stk8312_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -645,7 +644,7 @@ static struct i2c_driver stk8312_driver = {
 		.name = STK8312_DRIVER_NAME,
 		.pm = pm_sleep_ptr(&stk8312_pm_ops),
 	},
-	.probe =            stk8312_probe,
+	.probe_new =        stk8312_probe,
 	.remove =           stk8312_remove,
 	.id_table =         stk8312_i2c_id,
 };
-- 
2.37.2


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

* [PATCH 21/23] iio: accel: stk8ba50: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (19 preceding siblings ...)
  2022-10-23 13:22 ` [PATCH 20/23] iio: accel: stk8312: " Uwe Kleine-König
@ 2022-10-23 13:23 ` Uwe Kleine-König
  2022-10-29 11:59   ` Jonathan Cameron
  2022-10-23 13:23 ` [PATCH 22/23] iio: accel: st_magn: " Uwe Kleine-König
  2022-10-23 13:23 ` [PATCH 23/23] iio: accel: vl6180: " Uwe Kleine-König
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:23 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Sebastian Reichel, Peter Rosin, Gwendal Grignou, linux-iio,
	kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/accel/stk8ba50.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/stk8ba50.c b/drivers/iio/accel/stk8ba50.c
index 2f5e4ab2a6e7..44f6e0fbdfcc 100644
--- a/drivers/iio/accel/stk8ba50.c
+++ b/drivers/iio/accel/stk8ba50.c
@@ -379,8 +379,7 @@ static const struct iio_buffer_setup_ops stk8ba50_buffer_setup_ops = {
 	.postdisable = stk8ba50_buffer_postdisable,
 };
 
-static int stk8ba50_probe(struct i2c_client *client,
-			  const struct i2c_device_id *id)
+static int stk8ba50_probe(struct i2c_client *client)
 {
 	int ret;
 	struct iio_dev *indio_dev;
@@ -544,7 +543,7 @@ static struct i2c_driver stk8ba50_driver = {
 		.pm = pm_sleep_ptr(&stk8ba50_pm_ops),
 		.acpi_match_table = ACPI_PTR(stk8ba50_acpi_id),
 	},
-	.probe =            stk8ba50_probe,
+	.probe_new =        stk8ba50_probe,
 	.remove =           stk8ba50_remove,
 	.id_table =         stk8ba50_i2c_id,
 };
-- 
2.37.2


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

* [PATCH 22/23] iio: accel: st_magn: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (20 preceding siblings ...)
  2022-10-23 13:23 ` [PATCH 21/23] iio: accel: stk8ba50: " Uwe Kleine-König
@ 2022-10-23 13:23 ` Uwe Kleine-König
  2022-10-29 12:00   ` Jonathan Cameron
  2022-10-23 13:23 ` [PATCH 23/23] iio: accel: vl6180: " Uwe Kleine-König
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:23 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/magnetometer/st_magn_i2c.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
index c5d8c303db4e..b4098d3b3813 100644
--- a/drivers/iio/magnetometer/st_magn_i2c.c
+++ b/drivers/iio/magnetometer/st_magn_i2c.c
@@ -54,8 +54,7 @@ static const struct of_device_id st_magn_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, st_magn_of_match);
 
-static int st_magn_i2c_probe(struct i2c_client *client,
-			     const struct i2c_device_id *id)
+static int st_magn_i2c_probe(struct i2c_client *client)
 {
 	const struct st_sensor_settings *settings;
 	struct st_sensor_data *mdata;
@@ -107,7 +106,7 @@ static struct i2c_driver st_magn_driver = {
 		.name = "st-magn-i2c",
 		.of_match_table = st_magn_of_match,
 	},
-	.probe = st_magn_i2c_probe,
+	.probe_new = st_magn_i2c_probe,
 	.id_table = st_magn_id_table,
 };
 module_i2c_driver(st_magn_driver);
-- 
2.37.2


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

* [PATCH 23/23] iio: accel: vl6180: Convert to i2c's .probe_new()
  2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
                   ` (21 preceding siblings ...)
  2022-10-23 13:23 ` [PATCH 22/23] iio: accel: st_magn: " Uwe Kleine-König
@ 2022-10-23 13:23 ` Uwe Kleine-König
  2022-10-29 12:01   ` Jonathan Cameron
  22 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-23 13:23 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/light/vl6180.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index d47a4f6f4e87..8b56df26c59e 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -493,8 +493,7 @@ static int vl6180_init(struct vl6180_data *data)
 	return vl6180_hold(data, false);
 }
 
-static int vl6180_probe(struct i2c_client *client,
-			  const struct i2c_device_id *id)
+static int vl6180_probe(struct i2c_client *client)
 {
 	struct vl6180_data *data;
 	struct iio_dev *indio_dev;
@@ -539,7 +538,7 @@ static struct i2c_driver vl6180_driver = {
 		.name   = VL6180_DRV_NAME,
 		.of_match_table = vl6180_of_match,
 	},
-	.probe  = vl6180_probe,
+	.probe_new = vl6180_probe,
 	.id_table = vl6180_id,
 };
 
-- 
2.37.2


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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-23 13:22 ` [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new Uwe Kleine-König
@ 2022-10-23 19:06   ` Andy Shevchenko
  2022-10-24  7:05     ` Uwe Kleine-König
  0 siblings, 1 reply; 53+ messages in thread
From: Andy Shevchenko @ 2022-10-23 19:06 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jonathan Cameron, Wolfram Sang, Lars-Peter Clausen,
	Dmitry Rokosov, Vladimir Oltean, Yang Yingliang, wangjianli,
	Miaoqian Lin, Gwendal Grignou, linux-iio, kernel

On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:
> .probe_new() doesn't get the i2c_device_id * parameter, so determine
> that explicitly in .probe(). The device_id array has to move up for that
> to work.

...

> +static const struct i2c_device_id kxcjk1013_id[] = {
> +	{"kxcjk1013", KXCJK1013},
> +	{"kxcj91008", KXCJ91008},
> +	{"kxtj21009", KXTJ21009},
> +	{"kxtf9",     KXTF9},
> +	{"kx023-1025", KX0231025},
> +	{"SMO8500",   KXCJ91008},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);

I don't like this part. Can we, please, find a way how to dereference this
table via struct i2c_client, please?

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 16/23] iio: accel: mma9551: Convert to i2c's .probe_new
  2022-10-23 13:22 ` [PATCH 16/23] iio: accel: mma9551: " Uwe Kleine-König
@ 2022-10-23 19:06   ` Andy Shevchenko
  0 siblings, 0 replies; 53+ messages in thread
From: Andy Shevchenko @ 2022-10-23 19:06 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jonathan Cameron, Wolfram Sang, Lars-Peter Clausen, Peter Rosin,
	Maximilian Luz, Miaoqian Lin, linux-iio, kernel

On Sun, Oct 23, 2022 at 03:22:55PM +0200, Uwe Kleine-König wrote:
> .probe_new() doesn't get the i2c_device_id * parameter, so determine
> that explicitly in .probe(). The device_id array has to move up for that
> to work.

> +static const struct i2c_device_id mma9551_id[] = {
> +	{"mma9551", 0},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, mma9551_id);

Same, please find a way to leave this as it is right now.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 17/23] iio: accel: mma9553: Convert to i2c's .probe_new
  2022-10-23 13:22 ` [PATCH 17/23] iio: accel: mma9553: " Uwe Kleine-König
@ 2022-10-23 19:07   ` Andy Shevchenko
  0 siblings, 0 replies; 53+ messages in thread
From: Andy Shevchenko @ 2022-10-23 19:07 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jonathan Cameron, Wolfram Sang, Lars-Peter Clausen,
	Peter Senna Tschudin, Hans Verkuil, Miaoqian Lin,
	Antoniu Miclaus, linux-iio, kernel

On Sun, Oct 23, 2022 at 03:22:56PM +0200, Uwe Kleine-König wrote:
> .probe_new() doesn't get the i2c_device_id * parameter, so determine
> that explicitly in .probe(). The device_id array has to move up for that
> to work.

...

> +static const struct i2c_device_id mma9553_id[] = {
> +	{"mma9553", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, mma9553_id);

Same, no shuffling device ID tables, please.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-23 19:06   ` Andy Shevchenko
@ 2022-10-24  7:05     ` Uwe Kleine-König
  2022-10-24  8:39       ` Andy Shevchenko
  0 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-24  7:05 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Miaoqian Lin, Lars-Peter Clausen, Dmitry Rokosov, linux-iio,
	Gwendal Grignou, Wolfram Sang, kernel, Yang Yingliang,
	wangjianli, Vladimir Oltean, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 2282 bytes --]

On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko wrote:
> On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:
> > .probe_new() doesn't get the i2c_device_id * parameter, so determine
> > that explicitly in .probe(). The device_id array has to move up for that
> > to work.
> 
> ...
> 
> > +static const struct i2c_device_id kxcjk1013_id[] = {
> > +	{"kxcjk1013", KXCJK1013},
> > +	{"kxcj91008", KXCJ91008},
> > +	{"kxtj21009", KXTJ21009},
> > +	{"kxtf9",     KXTF9},
> > +	{"kx023-1025", KX0231025},
> > +	{"SMO8500",   KXCJ91008},
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> 
> I don't like this part. Can we, please, find a way how to dereference this
> table via struct i2c_client, please?

It would be possible to do (on top of my patch here as PoC):

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index e043dd698747..00269b25af99 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
 
 static int kxcjk1013_probe(struct i2c_client *client)
 {
-	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
+	const struct i2c_device_id *id = i2c_match_id(to_i2c_driver(client->dev.driver)->id_table, client);
 	struct kxcjk1013_data *data;
 	struct iio_dev *indio_dev;
 	struct kxcjk_1013_platform_data *pdata;

(only compile tested), you could even create a function or macro to make
this a bit prettier on the source level. For the compiler loading the
address from a local symbol instead of from two pointer dereferences is
(I guess) a bit more effective and IMHO more natural.

*shrug*, I don't care much, but I don't like to have to rework this
series just because you don't like this part. You even didn't give a
rationale, I can imagine several different ones:

 [ ] it makes the patch bigger
 [ ] it results in an unnatural order of symbols in the driver
 [ ] it's some kind of duplication
 [ ] something else
     please elaborate: ________________________________

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24  7:05     ` Uwe Kleine-König
@ 2022-10-24  8:39       ` Andy Shevchenko
  2022-10-24  9:14         ` Uwe Kleine-König
  0 siblings, 1 reply; 53+ messages in thread
From: Andy Shevchenko @ 2022-10-24  8:39 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Miaoqian Lin, Lars-Peter Clausen, Dmitry Rokosov, linux-iio,
	Gwendal Grignou, Wolfram Sang, kernel, Yang Yingliang,
	wangjianli, Vladimir Oltean, Jonathan Cameron

On Mon, Oct 24, 2022 at 09:05:18AM +0200, Uwe Kleine-König wrote:
> On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko wrote:
> > On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:

...

> > > +static const struct i2c_device_id kxcjk1013_id[] = {
> > > +	{"kxcjk1013", KXCJK1013},
> > > +	{"kxcj91008", KXCJ91008},
> > > +	{"kxtj21009", KXTJ21009},
> > > +	{"kxtf9",     KXTF9},
> > > +	{"kx023-1025", KX0231025},
> > > +	{"SMO8500",   KXCJ91008},
> > > +	{}
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > 
> > I don't like this part. Can we, please, find a way how to dereference this
> > table via struct i2c_client, please?
> 
> It would be possible to do (on top of my patch here as PoC):
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index e043dd698747..00269b25af99 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
>  
>  static int kxcjk1013_probe(struct i2c_client *client)
>  {
> -	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
> +	const struct i2c_device_id *id = i2c_match_id(to_i2c_driver(client->dev.driver)->id_table, client);
>  	struct kxcjk1013_data *data;
>  	struct iio_dev *indio_dev;
>  	struct kxcjk_1013_platform_data *pdata;
> 
> (only compile tested), you could even create a function or macro to make
> this a bit prettier on the source level. For the compiler loading the
> address from a local symbol instead of from two pointer dereferences is
> (I guess) a bit more effective and IMHO more natural.
> 
> *shrug*, I don't care much, but I don't like to have to rework this
> series just because you don't like this part. You even didn't give a
> rationale, I can imagine several different ones:

And I don't want to have ping-ponging the pieces of code (ID tables) because
some API has to be fixes or so.

>  [ ] it makes the patch bigger
>  [ ] it results in an unnatural order of symbols in the driver
>  [ ] it's some kind of duplication
>  [ ] something else
>      please elaborate: ________________________________

It adds a burden to the future work with no good justification along with
a churn in _this_ series.

While I like the rest of the series, these things I would rather postpone
or rework.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24  8:39       ` Andy Shevchenko
@ 2022-10-24  9:14         ` Uwe Kleine-König
  2022-10-24  9:46           ` Andy Shevchenko
  0 siblings, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-24  9:14 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Miaoqian Lin, Lars-Peter Clausen, linux-iio, Gwendal Grignou,
	Wolfram Sang, Vladimir Oltean, kernel, Yang Yingliang,
	wangjianli, Dmitry Rokosov, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 3692 bytes --]

Hello Andy,

On Mon, Oct 24, 2022 at 11:39:51AM +0300, Andy Shevchenko wrote:
> On Mon, Oct 24, 2022 at 09:05:18AM +0200, Uwe Kleine-König wrote:
> > On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko wrote:
> > > On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:
> 
> ...
> 
> > > > +static const struct i2c_device_id kxcjk1013_id[] = {
> > > > +	{"kxcjk1013", KXCJK1013},
> > > > +	{"kxcj91008", KXCJ91008},
> > > > +	{"kxtj21009", KXTJ21009},
> > > > +	{"kxtf9",     KXTF9},
> > > > +	{"kx023-1025", KX0231025},
> > > > +	{"SMO8500",   KXCJ91008},
> > > > +	{}
> > > > +};
> > > > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > > 
> > > I don't like this part. Can we, please, find a way how to dereference this
> > > table via struct i2c_client, please?
> > 
> > It would be possible to do (on top of my patch here as PoC):
> > 
> > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > index e043dd698747..00269b25af99 100644
> > --- a/drivers/iio/accel/kxcjk-1013.c
> > +++ b/drivers/iio/accel/kxcjk-1013.c
> > @@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> >  
> >  static int kxcjk1013_probe(struct i2c_client *client)
> >  {
> > -	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
> > +	const struct i2c_device_id *id = i2c_match_id(to_i2c_driver(client->dev.driver)->id_table, client);
> >  	struct kxcjk1013_data *data;
> >  	struct iio_dev *indio_dev;
> >  	struct kxcjk_1013_platform_data *pdata;
> > 
> > (only compile tested), you could even create a function or macro to make
> > this a bit prettier on the source level. For the compiler loading the
> > address from a local symbol instead of from two pointer dereferences is
> > (I guess) a bit more effective and IMHO more natural.
> > 
> > *shrug*, I don't care much, but I don't like to have to rework this
> > series just because you don't like this part. You even didn't give a
> > rationale, I can imagine several different ones:
> 
> And I don't want to have ping-ponging the pieces of code (ID tables) because
> some API has to be fixes or so.

In this series it's only ping without pong. To benefit from the local
table instead of fishing the table out of client, the table must be
declared already when it's used.

> >  [ ] it makes the patch bigger
> >  [ ] it results in an unnatural order of symbols in the driver
> >  [ ] it's some kind of duplication
> >  [ ] something else
> >      please elaborate: ________________________________
> 
> It adds a burden to the future work with no good justification along with

This burden exists in the drivers that already today have the table
above the probe function? (Ok, there are none in this series, but it
happens, see for example

	https://lore.kernel.org/linux-rtc/20221021130706.178687-4-u.kleine-koenig@pengutronix.de

and a few more in the rtc series.) I don't see a burden here, we're
talking about the id table being defined before the probe function, right?
How is that a burden? What am I missing?

> a churn in _this_ series.

The alternatives are: Split the patch into reorder + convert to
.probe_new, or add a declaration for the id table. Among these I like
the current approach best.

> While I like the rest of the series, these things I would rather postpone
> or rework.

There is no win in postponing, is there?[1] What would be your preferred
way to rework?

Best regards
Uwe

[1] .probe_new was created in 2016!

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24  9:14         ` Uwe Kleine-König
@ 2022-10-24  9:46           ` Andy Shevchenko
  2022-10-24 10:22             ` Nuno Sá
  2022-10-31 23:38             ` Uwe Kleine-König
  0 siblings, 2 replies; 53+ messages in thread
From: Andy Shevchenko @ 2022-10-24  9:46 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Miaoqian Lin, Lars-Peter Clausen, linux-iio, Gwendal Grignou,
	Wolfram Sang, Vladimir Oltean, kernel, Yang Yingliang,
	wangjianli, Dmitry Rokosov, Jonathan Cameron

On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:
> On Mon, Oct 24, 2022 at 11:39:51AM +0300, Andy Shevchenko wrote:
> > On Mon, Oct 24, 2022 at 09:05:18AM +0200, Uwe Kleine-König wrote:
> > > On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko wrote:
> > > > On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:

...

> > > > > +static const struct i2c_device_id kxcjk1013_id[] = {
> > > > > +	{"kxcjk1013", KXCJK1013},
> > > > > +	{"kxcj91008", KXCJ91008},
> > > > > +	{"kxtj21009", KXTJ21009},
> > > > > +	{"kxtf9",     KXTF9},
> > > > > +	{"kx023-1025", KX0231025},
> > > > > +	{"SMO8500",   KXCJ91008},
> > > > > +	{}
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > > > 
> > > > I don't like this part. Can we, please, find a way how to dereference this
> > > > table via struct i2c_client, please?
> > > 
> > > It would be possible to do (on top of my patch here as PoC):
> > > 
> > > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > > index e043dd698747..00269b25af99 100644
> > > --- a/drivers/iio/accel/kxcjk-1013.c
> > > +++ b/drivers/iio/accel/kxcjk-1013.c
> > > @@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > >  
> > >  static int kxcjk1013_probe(struct i2c_client *client)
> > >  {
> > > -	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
> > > +	const struct i2c_device_id *id = i2c_match_id(to_i2c_driver(client->dev.driver)->id_table, client);
> > >  	struct kxcjk1013_data *data;
> > >  	struct iio_dev *indio_dev;
> > >  	struct kxcjk_1013_platform_data *pdata;
> > > 
> > > (only compile tested), you could even create a function or macro to make
> > > this a bit prettier on the source level. For the compiler loading the
> > > address from a local symbol instead of from two pointer dereferences is
> > > (I guess) a bit more effective and IMHO more natural.
> > > 
> > > *shrug*, I don't care much, but I don't like to have to rework this
> > > series just because you don't like this part. You even didn't give a
> > > rationale, I can imagine several different ones:
> > 
> > And I don't want to have ping-ponging the pieces of code (ID tables) because
> > some API has to be fixes or so.
> 
> In this series it's only ping without pong.

Exactly. And it means let's put my problem to someone's else shoulders.

> To benefit from the local
> table instead of fishing the table out of client, the table must be
> declared already when it's used.

I don't see benefit of dereferencing tables by name. The table has to be
available via struct driver, otherwise, how the heck we even got into the
->probe() there.

> > >  [ ] it makes the patch bigger
> > >  [ ] it results in an unnatural order of symbols in the driver
> > >  [ ] it's some kind of duplication
> > >  [ ] something else
> > >      please elaborate: ________________________________
> > 
> > It adds a burden to the future work with no good justification along with
> 
> This burden exists in the drivers that already today have the table
> above the probe function? (Ok, there are none in this series, but it
> happens, see for example
> 
> 	https://lore.kernel.org/linux-rtc/20221021130706.178687-4-u.kleine-koenig@pengutronix.de
> 
> and a few more in the rtc series.) I don't see a burden here, we're
> talking about the id table being defined before the probe function, right?
> How is that a burden? What am I missing?

Yeah, people haven't had no idea about accessing tables via struct driver,
reviewers of that code neither. Should it be excuse for us to follow that
example?

> > a churn in _this_ series.
> 
> The alternatives are: Split the patch into reorder + convert to
> .probe_new, or add a declaration for the id table. Among these I like
> the current approach besto.

Alternative is to avoid reordering to begin with, no?

> > While I like the rest of the series, these things I would rather postpone
> > or rework.
> 
> There is no win in postponing, is there?[1] What would be your preferred
> way to rework?

My understand of the probe_new is that an attempt to unify i2c with how spi
does. So, why not teach i2c_match_id() to handle this nicely for the caller?

This will allow to leave tables where they are (or move closer to struct
driver), reduce churn with the using of current i2c_match_id() as you
showed the long line to get that table. This might need a new API to avoid
changing many drivers at once. But it's business as usual.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24  9:46           ` Andy Shevchenko
@ 2022-10-24 10:22             ` Nuno Sá
  2022-10-24 11:40               ` Andy Shevchenko
  2022-10-31 23:38             ` Uwe Kleine-König
  1 sibling, 1 reply; 53+ messages in thread
From: Nuno Sá @ 2022-10-24 10:22 UTC (permalink / raw)
  To: Andy Shevchenko, Uwe Kleine-König
  Cc: Miaoqian Lin, Lars-Peter Clausen, linux-iio, Gwendal Grignou,
	Wolfram Sang, Vladimir Oltean, kernel, Yang Yingliang,
	wangjianli, Dmitry Rokosov, Jonathan Cameron

On Mon, 2022-10-24 at 12:46 +0300, Andy Shevchenko wrote:
> On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:
> > On Mon, Oct 24, 2022 at 11:39:51AM +0300, Andy Shevchenko wrote:
> > > On Mon, Oct 24, 2022 at 09:05:18AM +0200, Uwe Kleine-König wrote:
> > > > On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko
> > > > wrote:
> > > > > On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König
> > > > > wrote:
> 
> ...
> 
> > > > > > +static const struct i2c_device_id kxcjk1013_id[] = {
> > > > > > +       {"kxcjk1013", KXCJK1013},
> > > > > > +       {"kxcj91008", KXCJ91008},
> > > > > > +       {"kxtj21009", KXTJ21009},
> > > > > > +       {"kxtf9",     KXTF9},
> > > > > > +       {"kx023-1025", KX0231025},
> > > > > > +       {"SMO8500",   KXCJ91008},
> > > > > > +       {}
> > > > > > +};
> > > > > > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > > > > 
> > > > > I don't like this part. Can we, please, find a way how to
> > > > > dereference this
> > > > > table via struct i2c_client, please?
> > > > 
> > > > It would be possible to do (on top of my patch here as PoC):
> > > > 
> > > > diff --git a/drivers/iio/accel/kxcjk-1013.c
> > > > b/drivers/iio/accel/kxcjk-1013.c
> > > > index e043dd698747..00269b25af99 100644
> > > > --- a/drivers/iio/accel/kxcjk-1013.c
> > > > +++ b/drivers/iio/accel/kxcjk-1013.c
> > > > @@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > > >  
> > > >  static int kxcjk1013_probe(struct i2c_client *client)
> > > >  {
> > > > -       const struct i2c_device_id *id =
> > > > i2c_match_id(kxcjk1013_id, client);
> > > > +       const struct i2c_device_id *id =
> > > > i2c_match_id(to_i2c_driver(client->dev.driver)->id_table,
> > > > client);
> > > >         struct kxcjk1013_data *data;
> > > >         struct iio_dev *indio_dev;
> > > >         struct kxcjk_1013_platform_data *pdata;
> > > > 
> > > > (only compile tested), you could even create a function or
> > > > macro to make
> > > > this a bit prettier on the source level. For the compiler
> > > > loading the
> > > > address from a local symbol instead of from two pointer
> > > > dereferences is
> > > > (I guess) a bit more effective and IMHO more natural.
> > > > 
> > > > *shrug*, I don't care much, but I don't like to have to rework
> > > > this
> > > > series just because you don't like this part. You even didn't
> > > > give a
> > > > rationale, I can imagine several different ones:
> > > 
> > > And I don't want to have ping-ponging the pieces of code (ID
> > > tables) because
> > > some API has to be fixes or so.
> > 
> > In this series it's only ping without pong.
> 
> Exactly. And it means let's put my problem to someone's else
> shoulders.
> 
> > To benefit from the local
> > table instead of fishing the table out of client, the table must be
> > declared already when it's used.
> 
> I don't see benefit of dereferencing tables by name. The table has to
> be
> available via struct driver, otherwise, how the heck we even got into
> the
> ->probe() there.
> 
> > > >  [ ] it makes the patch bigger
> > > >  [ ] it results in an unnatural order of symbols in the driver
> > > >  [ ] it's some kind of duplication
> > > >  [ ] something else
> > > >      please elaborate: ________________________________
> > > 
> > > It adds a burden to the future work with no good justification
> > > along with
> > 
> > This burden exists in the drivers that already today have the table
> > above the probe function? (Ok, there are none in this series, but
> > it
> > happens, see for example
> > 
> >         
> > https://lore.kernel.org/linux-rtc/20221021130706.178687-4-u.kleine-koenig@pengutronix.de
> > 
> > and a few more in the rtc series.) I don't see a burden here, we're
> > talking about the id table being defined before the probe function,
> > right?
> > How is that a burden? What am I missing?
> 
> Yeah, people haven't had no idea about accessing tables via struct
> driver,
> reviewers of that code neither. Should it be excuse for us to follow
> that
> example?
> 
> > > a churn in _this_ series.
> > 
> > The alternatives are: Split the patch into reorder + convert to
> > .probe_new, or add a declaration for the id table. Among these I
> > like
> > the current approach besto.
> 
> Alternative is to avoid reordering to begin with, no?
> 
> > > While I like the rest of the series, these things I would rather
> > > postpone
> > > or rework.
> > 
> > There is no win in postponing, is there?[1] What would be your
> > preferred
> > way to rework?
> 
> My understand of the probe_new is that an attempt to unify i2c with
> how spi
> does. So, why not teach i2c_match_id() to handle this nicely for the
> caller?
> 
> This will allow to leave tables where they are (or move closer to
> struct
> driver), reduce churn with the using of current i2c_match_id() as you
> showed the long line to get that table. This might need a new API to
> avoid
> changing many drivers at once. But it's business as usual.
> 

I guess something like spi_get_device_id()

- Nuno Sá


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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24 10:22             ` Nuno Sá
@ 2022-10-24 11:40               ` Andy Shevchenko
  2022-10-29 11:49                 ` Jonathan Cameron
  0 siblings, 1 reply; 53+ messages in thread
From: Andy Shevchenko @ 2022-10-24 11:40 UTC (permalink / raw)
  To: Nuno Sá
  Cc: Uwe Kleine-König, Miaoqian Lin, Lars-Peter Clausen,
	linux-iio, Gwendal Grignou, Wolfram Sang, Vladimir Oltean,
	kernel, Yang Yingliang, wangjianli, Dmitry Rokosov,
	Jonathan Cameron

On Mon, Oct 24, 2022 at 12:22:19PM +0200, Nuno Sá wrote:
> On Mon, 2022-10-24 at 12:46 +0300, Andy Shevchenko wrote:
> > On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:

...

> > > There is no win in postponing, is there?[1] What would be your
> > > preferred
> > > way to rework?
> > 
> > My understand of the probe_new is that an attempt to unify i2c with
> > how spi
> > does. So, why not teach i2c_match_id() to handle this nicely for the
> > caller?
> > 
> > This will allow to leave tables where they are (or move closer to
> > struct
> > driver), reduce churn with the using of current i2c_match_id() as you
> > showed the long line to get that table. This might need a new API to
> > avoid
> > changing many drivers at once. But it's business as usual.
> 
> I guess something like spi_get_device_id()

Right, that one I have had in mind when responding.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-29 11:43   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:43 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Lars-Peter Clausen, Michael Hennerich, Cosmin Tanislav,
	Wolfram Sang, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:40 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Given many of these are completely uncontroversial, I'll pick them up
one by one and then swing back around to consider the more complex ones.

Applied to the togreg branch of iio.git and pushed out as testing for 0-day to
take a look at it.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/adxl367_i2c.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl367_i2c.c b/drivers/iio/accel/adxl367_i2c.c
> index 3606efa25835..070aad724abd 100644
> --- a/drivers/iio/accel/adxl367_i2c.c
> +++ b/drivers/iio/accel/adxl367_i2c.c
> @@ -41,8 +41,7 @@ static const struct adxl367_ops adxl367_i2c_ops = {
>  	.read_fifo = adxl367_i2c_read_fifo,
>  };
>  
> -static int adxl367_i2c_probe(struct i2c_client *client,
> -			     const struct i2c_device_id *id)
> +static int adxl367_i2c_probe(struct i2c_client *client)
>  {
>  	struct adxl367_i2c_state *st;
>  	struct regmap *regmap;
> @@ -78,7 +77,7 @@ static struct i2c_driver adxl367_i2c_driver = {
>  		.name = "adxl367_i2c",
>  		.of_match_table = adxl367_of_match,
>  	},
> -	.probe = adxl367_i2c_probe,
> +	.probe_new = adxl367_i2c_probe,
>  	.id_table = adxl367_i2c_id,
>  };
>  


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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24 11:40               ` Andy Shevchenko
@ 2022-10-29 11:49                 ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Nuno Sá,
	Uwe Kleine-König, Miaoqian Lin, Lars-Peter Clausen,
	linux-iio, Gwendal Grignou, Wolfram Sang, Vladimir Oltean,
	kernel, Yang Yingliang, wangjianli, Dmitry Rokosov

On Mon, 24 Oct 2022 14:40:20 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Mon, Oct 24, 2022 at 12:22:19PM +0200, Nuno Sá wrote:
> > On Mon, 2022-10-24 at 12:46 +0300, Andy Shevchenko wrote:  
> > > On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:  
> 
> ...
> 
> > > > There is no win in postponing, is there?[1] What would be your
> > > > preferred
> > > > way to rework?  
> > > 
> > > My understand of the probe_new is that an attempt to unify i2c with
> > > how spi
> > > does. So, why not teach i2c_match_id() to handle this nicely for the
> > > caller?
> > > 
> > > This will allow to leave tables where they are (or move closer to
> > > struct
> > > driver), reduce churn with the using of current i2c_match_id() as you
> > > showed the long line to get that table. This might need a new API to
> > > avoid
> > > changing many drivers at once. But it's business as usual.  
> > 
> > I guess something like spi_get_device_id()  
> 
> Right, that one I have had in mind when responding.
> 

Agreed an i2c_client_get_device_id() seems like a good addition to me.

Jonathan

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

* Re: [PATCH 07/23] iio: accel: dmard06: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 07/23] iio: accel: dmard06: " Uwe Kleine-König
@ 2022-10-29 11:51   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:51 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:46 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/dmard06.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/dmard06.c b/drivers/iio/accel/dmard06.c
> index 4b69c8530f5e..7390509aaac0 100644
> --- a/drivers/iio/accel/dmard06.c
> +++ b/drivers/iio/accel/dmard06.c
> @@ -125,8 +125,7 @@ static const struct iio_info dmard06_info = {
>  	.read_raw	= dmard06_read_raw,
>  };
>  
> -static int dmard06_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> +static int dmard06_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -218,7 +217,7 @@ static const struct of_device_id dmard06_of_match[] = {
>  MODULE_DEVICE_TABLE(of, dmard06_of_match);
>  
>  static struct i2c_driver dmard06_driver = {
> -	.probe = dmard06_probe,
> +	.probe_new = dmard06_probe,
>  	.id_table = dmard06_id,
>  	.driver = {
>  		.name = DMARD06_DRV_NAME,


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

* Re: [PATCH 06/23] iio: accel: da311: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 06/23] iio: accel: da311: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-29 11:51   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:51 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:45 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied
> ---
>  drivers/iio/accel/da311.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/da311.c b/drivers/iio/accel/da311.c
> index ec4e29d260f7..080335fa2ad6 100644
> --- a/drivers/iio/accel/da311.c
> +++ b/drivers/iio/accel/da311.c
> @@ -217,8 +217,7 @@ static void da311_disable(void *client)
>  	da311_enable(client, false);
>  }
>  
> -static int da311_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> +static int da311_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -279,7 +278,7 @@ static struct i2c_driver da311_driver = {
>  		.name = "da311",
>  		.pm = pm_sleep_ptr(&da311_pm_ops),
>  	},
> -	.probe		= da311_probe,
> +	.probe_new	= da311_probe,
>  	.id_table	= da311_i2c_id,
>  };
>  


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

* Re: [PATCH 08/23] iio: accel: dmard09: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 08/23] iio: accel: dmard09: " Uwe Kleine-König
@ 2022-10-29 11:52   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:52 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Gwendal Grignou,
	Paul Lemmermann, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:47 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied
> ---
>  drivers/iio/accel/dmard09.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/dmard09.c b/drivers/iio/accel/dmard09.c
> index cb0246ca72f3..4b7a537f617d 100644
> --- a/drivers/iio/accel/dmard09.c
> +++ b/drivers/iio/accel/dmard09.c
> @@ -88,8 +88,7 @@ static const struct iio_info dmard09_info = {
>  	.read_raw	= dmard09_read_raw,
>  };
>  
> -static int dmard09_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> +static int dmard09_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -136,7 +135,7 @@ static struct i2c_driver dmard09_driver = {
>  	.driver = {
>  		.name = DMARD09_DRV_NAME
>  	},
> -	.probe = dmard09_probe,
> +	.probe_new = dmard09_probe,
>  	.id_table = dmard09_id,
>  };
>  


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

* Re: [PATCH 09/23] iio: accel: dmard10: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 09/23] iio: accel: dmard10: " Uwe Kleine-König
@ 2022-10-29 11:53   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:53 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Paul Cercueil, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:48 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/dmard10.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/dmard10.c b/drivers/iio/accel/dmard10.c
> index 8ac62ec0a04a..07e68aed8a13 100644
> --- a/drivers/iio/accel/dmard10.c
> +++ b/drivers/iio/accel/dmard10.c
> @@ -175,8 +175,7 @@ static void dmard10_shutdown_cleanup(void *client)
>  	dmard10_shutdown(client);
>  }
>  
> -static int dmard10_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> +static int dmard10_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -242,7 +241,7 @@ static struct i2c_driver dmard10_driver = {
>  		.name = "dmard10",
>  		.pm = pm_sleep_ptr(&dmard10_pm_ops),
>  	},
> -	.probe		= dmard10_probe,
> +	.probe_new	= dmard10_probe,
>  	.id_table	= dmard10_i2c_id,
>  };
>  


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

* Re: [PATCH 11/23] iio: accel: kxsd9: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 11/23] iio: accel: kxsd9: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-29 11:54   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:54 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Greg Kroah-Hartman,
	Jean Delvare, Ajay Gupta, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:50 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/kxsd9-i2c.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
> index 61346ea8ef19..6b3683ddce36 100644
> --- a/drivers/iio/accel/kxsd9-i2c.c
> +++ b/drivers/iio/accel/kxsd9-i2c.c
> @@ -10,8 +10,7 @@
>  
>  #include "kxsd9.h"
>  
> -static int kxsd9_i2c_probe(struct i2c_client *i2c,
> -			   const struct i2c_device_id *id)
> +static int kxsd9_i2c_probe(struct i2c_client *i2c)
>  {
>  	static const struct regmap_config config = {
>  		.reg_bits = 8,
> @@ -55,7 +54,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
>  		.of_match_table = kxsd9_of_match,
>  		.pm = pm_ptr(&kxsd9_dev_pm_ops),
>  	},
> -	.probe		= kxsd9_i2c_probe,
> +	.probe_new	= kxsd9_i2c_probe,
>  	.remove		= kxsd9_i2c_remove,
>  	.id_table	= kxsd9_i2c_id,
>  };


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

* Re: [PATCH 14/23] iio: accel: mma7660: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 14/23] iio: accel: mma7660: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-29 11:55   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:55 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Dmitry Torokhov, Vladimir Oltean, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:53 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/mma7660.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
> index 85829990bbad..b279ca4dcdc0 100644
> --- a/drivers/iio/accel/mma7660.c
> +++ b/drivers/iio/accel/mma7660.c
> @@ -169,8 +169,7 @@ static const struct iio_info mma7660_info = {
>  	.attrs			= &mma7660_attribute_group,
>  };
>  
> -static int mma7660_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> +static int mma7660_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -267,7 +266,7 @@ static struct i2c_driver mma7660_driver = {
>  		.of_match_table = mma7660_of_match,
>  		.acpi_match_table = mma7660_acpi_id,
>  	},
> -	.probe		= mma7660_probe,
> +	.probe_new	= mma7660_probe,
>  	.remove		= mma7660_remove,
>  	.id_table	= mma7660_i2c_id,
>  };


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

* Re: [PATCH 18/23] iio: accel: mxc4005: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 18/23] iio: accel: mxc4005: Convert to i2c's .probe_new() Uwe Kleine-König
@ 2022-10-29 11:56   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:56 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Dmitry Rokosov, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:57 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/mxc4005.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
> index df600d2917c0..b146fc82738f 100644
> --- a/drivers/iio/accel/mxc4005.c
> +++ b/drivers/iio/accel/mxc4005.c
> @@ -385,8 +385,7 @@ static int mxc4005_chip_init(struct mxc4005_data *data)
>  	return 0;
>  }
>  
> -static int mxc4005_probe(struct i2c_client *client,
> -			 const struct i2c_device_id *id)
> +static int mxc4005_probe(struct i2c_client *client)
>  {
>  	struct mxc4005_data *data;
>  	struct iio_dev *indio_dev;
> @@ -489,7 +488,7 @@ static struct i2c_driver mxc4005_driver = {
>  		.name = MXC4005_DRV_NAME,
>  		.acpi_match_table = ACPI_PTR(mxc4005_acpi_match),
>  	},
> -	.probe		= mxc4005_probe,
> +	.probe_new	= mxc4005_probe,
>  	.id_table	= mxc4005_id,
>  };
>  


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

* Re: [PATCH 12/23] iio: accel: mc3230: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 12/23] iio: accel: mc3230: " Uwe Kleine-König
@ 2022-10-29 11:57   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:57 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Heikki Krogerus, Ajay Gupta,
	Jeremy Kerr, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:51 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied
> ---
>  drivers/iio/accel/mc3230.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c
> index 2462000e0519..efc21871de42 100644
> --- a/drivers/iio/accel/mc3230.c
> +++ b/drivers/iio/accel/mc3230.c
> @@ -106,8 +106,7 @@ static const struct iio_info mc3230_info = {
>  	.read_raw	= mc3230_read_raw,
>  };
>  
> -static int mc3230_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> +static int mc3230_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -191,7 +190,7 @@ static struct i2c_driver mc3230_driver = {
>  		.name = "mc3230",
>  		.pm = pm_sleep_ptr(&mc3230_pm_ops),
>  	},
> -	.probe		= mc3230_probe,
> +	.probe_new	= mc3230_probe,
>  	.remove		= mc3230_remove,
>  	.id_table	= mc3230_i2c_id,
>  };


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

* Re: [PATCH 19/23] iio: accel: mxc6255: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 19/23] iio: accel: mxc6255: " Uwe Kleine-König
@ 2022-10-29 11:57   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:57 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:58 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/mxc6255.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mxc6255.c b/drivers/iio/accel/mxc6255.c
> index 9aeeadc420d3..aa2e660545f8 100644
> --- a/drivers/iio/accel/mxc6255.c
> +++ b/drivers/iio/accel/mxc6255.c
> @@ -113,8 +113,7 @@ static const struct regmap_config mxc6255_regmap_config = {
>  	.readable_reg = mxc6255_is_readable_reg,
>  };
>  
> -static int mxc6255_probe(struct i2c_client *client,
> -			 const struct i2c_device_id *id)
> +static int mxc6255_probe(struct i2c_client *client)
>  {
>  	struct mxc6255_data *data;
>  	struct iio_dev *indio_dev;
> @@ -184,7 +183,7 @@ static struct i2c_driver mxc6255_driver = {
>  		.name = MXC6255_DRV_NAME,
>  		.acpi_match_table = ACPI_PTR(mxc6255_acpi_match),
>  	},
> -	.probe		= mxc6255_probe,
> +	.probe_new	= mxc6255_probe,
>  	.id_table	= mxc6255_id,
>  };
>  


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

* Re: [PATCH 20/23] iio: accel: stk8312: Convert to i2c's .probe_new()
  2022-10-23 13:22 ` [PATCH 20/23] iio: accel: stk8312: " Uwe Kleine-König
@ 2022-10-29 11:58   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:58 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Jean Delvare, Gwendal Grignou,
	Peter Rosin, linux-iio, kernel

On Sun, 23 Oct 2022 15:22:59 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/stk8312.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/stk8312.c b/drivers/iio/accel/stk8312.c
> index 7b1d6fb692b3..68f680db7505 100644
> --- a/drivers/iio/accel/stk8312.c
> +++ b/drivers/iio/accel/stk8312.c
> @@ -498,8 +498,7 @@ static const struct iio_buffer_setup_ops stk8312_buffer_setup_ops = {
>  	.postdisable = stk8312_buffer_postdisable,
>  };
>  
> -static int stk8312_probe(struct i2c_client *client,
> -			 const struct i2c_device_id *id)
> +static int stk8312_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -645,7 +644,7 @@ static struct i2c_driver stk8312_driver = {
>  		.name = STK8312_DRIVER_NAME,
>  		.pm = pm_sleep_ptr(&stk8312_pm_ops),
>  	},
> -	.probe =            stk8312_probe,
> +	.probe_new =        stk8312_probe,
>  	.remove =           stk8312_remove,
>  	.id_table =         stk8312_i2c_id,
>  };


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

* Re: [PATCH 21/23] iio: accel: stk8ba50: Convert to i2c's .probe_new()
  2022-10-23 13:23 ` [PATCH 21/23] iio: accel: stk8ba50: " Uwe Kleine-König
@ 2022-10-29 11:59   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 11:59 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Lars-Peter Clausen, Andy Shevchenko,
	Sebastian Reichel, Peter Rosin, Gwendal Grignou, linux-iio,
	kernel

On Sun, 23 Oct 2022 15:23:00 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/accel/stk8ba50.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/stk8ba50.c b/drivers/iio/accel/stk8ba50.c
> index 2f5e4ab2a6e7..44f6e0fbdfcc 100644
> --- a/drivers/iio/accel/stk8ba50.c
> +++ b/drivers/iio/accel/stk8ba50.c
> @@ -379,8 +379,7 @@ static const struct iio_buffer_setup_ops stk8ba50_buffer_setup_ops = {
>  	.postdisable = stk8ba50_buffer_postdisable,
>  };
>  
> -static int stk8ba50_probe(struct i2c_client *client,
> -			  const struct i2c_device_id *id)
> +static int stk8ba50_probe(struct i2c_client *client)
>  {
>  	int ret;
>  	struct iio_dev *indio_dev;
> @@ -544,7 +543,7 @@ static struct i2c_driver stk8ba50_driver = {
>  		.pm = pm_sleep_ptr(&stk8ba50_pm_ops),
>  		.acpi_match_table = ACPI_PTR(stk8ba50_acpi_id),
>  	},
> -	.probe =            stk8ba50_probe,
> +	.probe_new =        stk8ba50_probe,
>  	.remove =           stk8ba50_remove,
>  	.id_table =         stk8ba50_i2c_id,
>  };


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

* Re: [PATCH 22/23] iio: accel: st_magn: Convert to i2c's .probe_new()
  2022-10-23 13:23 ` [PATCH 22/23] iio: accel: st_magn: " Uwe Kleine-König
@ 2022-10-29 12:00   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 12:00 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

On Sun, 23 Oct 2022 15:23:01 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied

> ---
>  drivers/iio/magnetometer/st_magn_i2c.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> index c5d8c303db4e..b4098d3b3813 100644
> --- a/drivers/iio/magnetometer/st_magn_i2c.c
> +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> @@ -54,8 +54,7 @@ static const struct of_device_id st_magn_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, st_magn_of_match);
>  
> -static int st_magn_i2c_probe(struct i2c_client *client,
> -			     const struct i2c_device_id *id)
> +static int st_magn_i2c_probe(struct i2c_client *client)
>  {
>  	const struct st_sensor_settings *settings;
>  	struct st_sensor_data *mdata;
> @@ -107,7 +106,7 @@ static struct i2c_driver st_magn_driver = {
>  		.name = "st-magn-i2c",
>  		.of_match_table = st_magn_of_match,
>  	},
> -	.probe = st_magn_i2c_probe,
> +	.probe_new = st_magn_i2c_probe,
>  	.id_table = st_magn_id_table,
>  };
>  module_i2c_driver(st_magn_driver);


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

* Re: [PATCH 23/23] iio: accel: vl6180: Convert to i2c's .probe_new()
  2022-10-23 13:23 ` [PATCH 23/23] iio: accel: vl6180: " Uwe Kleine-König
@ 2022-10-29 12:01   ` Jonathan Cameron
  0 siblings, 0 replies; 53+ messages in thread
From: Jonathan Cameron @ 2022-10-29 12:01 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Wolfram Sang, Lars-Peter Clausen, linux-iio, kernel

On Sun, 23 Oct 2022 15:23:02 +0200
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied.

For the cases where and i2c_client_get_device_id() type call would be
useful, I'd like you to explore that approach as it'll reduce the refactoring
needed and is generally neater.

Jonathan

> ---
>  drivers/iio/light/vl6180.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index d47a4f6f4e87..8b56df26c59e 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -493,8 +493,7 @@ static int vl6180_init(struct vl6180_data *data)
>  	return vl6180_hold(data, false);
>  }
>  
> -static int vl6180_probe(struct i2c_client *client,
> -			  const struct i2c_device_id *id)
> +static int vl6180_probe(struct i2c_client *client)
>  {
>  	struct vl6180_data *data;
>  	struct iio_dev *indio_dev;
> @@ -539,7 +538,7 @@ static struct i2c_driver vl6180_driver = {
>  		.name   = VL6180_DRV_NAME,
>  		.of_match_table = vl6180_of_match,
>  	},
> -	.probe  = vl6180_probe,
> +	.probe_new = vl6180_probe,
>  	.id_table = vl6180_id,
>  };
>  


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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-24  9:46           ` Andy Shevchenko
  2022-10-24 10:22             ` Nuno Sá
@ 2022-10-31 23:38             ` Uwe Kleine-König
  2022-11-01 14:54               ` Andy Shevchenko
  1 sibling, 1 reply; 53+ messages in thread
From: Uwe Kleine-König @ 2022-10-31 23:38 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Miaoqian Lin, Lars-Peter Clausen, Dmitry Rokosov, linux-iio,
	Gwendal Grignou, Wolfram Sang, kernel, Yang Yingliang,
	wangjianli, Vladimir Oltean, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 10318 bytes --]

Hello Andy,

On Mon, Oct 24, 2022 at 12:46:02PM +0300, Andy Shevchenko wrote:
> On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:
> > On Mon, Oct 24, 2022 at 11:39:51AM +0300, Andy Shevchenko wrote:
> > > On Mon, Oct 24, 2022 at 09:05:18AM +0200, Uwe Kleine-König wrote:
> > > > On Sun, Oct 23, 2022 at 10:06:11PM +0300, Andy Shevchenko wrote:
> > > > > On Sun, Oct 23, 2022 at 03:22:49PM +0200, Uwe Kleine-König wrote:
> 
> ...
> 
> > > > > > +static const struct i2c_device_id kxcjk1013_id[] = {
> > > > > > +	{"kxcjk1013", KXCJK1013},
> > > > > > +	{"kxcj91008", KXCJ91008},
> > > > > > +	{"kxtj21009", KXTJ21009},
> > > > > > +	{"kxtf9",     KXTF9},
> > > > > > +	{"kx023-1025", KX0231025},
> > > > > > +	{"SMO8500",   KXCJ91008},
> > > > > > +	{}
> > > > > > +};
> > > > > > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > > > > 
> > > > > I don't like this part. Can we, please, find a way how to dereference this
> > > > > table via struct i2c_client, please?
> > > > 
> > > > It would be possible to do (on top of my patch here as PoC):
> > > > 
> > > > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > > > index e043dd698747..00269b25af99 100644
> > > > --- a/drivers/iio/accel/kxcjk-1013.c
> > > > +++ b/drivers/iio/accel/kxcjk-1013.c
> > > > @@ -1445,7 +1445,7 @@ MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > > >  
> > > >  static int kxcjk1013_probe(struct i2c_client *client)
> > > >  {
> > > > -	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
> > > > +	const struct i2c_device_id *id = i2c_match_id(to_i2c_driver(client->dev.driver)->id_table, client);
> > > >  	struct kxcjk1013_data *data;
> > > >  	struct iio_dev *indio_dev;
> > > >  	struct kxcjk_1013_platform_data *pdata;
> > > > 
> > > > (only compile tested), you could even create a function or macro to make
> > > > this a bit prettier on the source level. For the compiler loading the
> > > > address from a local symbol instead of from two pointer dereferences is
> > > > (I guess) a bit more effective and IMHO more natural.
> > > > 
> > > > *shrug*, I don't care much, but I don't like to have to rework this
> > > > series just because you don't like this part. You even didn't give a
> > > > rationale, I can imagine several different ones:
> > > 
> > > And I don't want to have ping-ponging the pieces of code (ID tables) because
> > > some API has to be fixes or so.
> > 
> > In this series it's only ping without pong.
> 
> Exactly. And it means let's put my problem to someone's else shoulders.

You have a problem that I fail to see. Why is defining the id table
before the probe function bad?

Unless I misunderstand you, you seem to assume that in the nearer future
someone will have the urge to put the id table below the probe function
again. What would you think is their motivation?

> > To benefit from the local
> > table instead of fishing the table out of client, the table must be
> > declared already when it's used.
> 
> I don't see benefit of dereferencing tables by name. The table has to be
> available via struct driver, otherwise, how the heck we even got into the
> ->probe() there.

It is possible, it's just cheaper (in cpu cycles) to calculate the
address of the table directly (i.e. via PC + $offset) instead of via
dereferencing two pointers.

> > > >  [ ] it makes the patch bigger
> > > >  [ ] it results in an unnatural order of symbols in the driver
> > > >  [ ] it's some kind of duplication
> > > >  [ ] something else
> > > >      please elaborate: ________________________________
> > > 
> > > It adds a burden to the future work with no good justification along with
> > 
> > This burden exists in the drivers that already today have the table
> > above the probe function? (Ok, there are none in this series, but it
> > happens, see for example
> > 
> > 	https://lore.kernel.org/linux-rtc/20221021130706.178687-4-u.kleine-koenig@pengutronix.de
> > 
> > and a few more in the rtc series.) I don't see a burden here, we're
> > talking about the id table being defined before the probe function, right?
> > How is that a burden? What am I missing?
> 
> Yeah, people haven't had no idea about accessing tables via struct driver,
> reviewers of that code neither. Should it be excuse for us to follow that
> example?

I fail to follow you again. I talked about drivers/rtc/rtc-isl1208.c as
it is in v6.1-rc1. There the probe function doesn't access the table at
all. Neither via the driver link nor by name. That driver just defines
the id table before the probe function.

> > > a churn in _this_ series.
> > 
> > The alternatives are: Split the patch into reorder + convert to
> > .probe_new, or add a declaration for the id table. Among these I like
> > the current approach besto.
> 
> Alternative is to avoid reordering to begin with, no?

Yeah, that could be done. But I don't see the advantage and you fail to
explain it in a way for me to understand.

So if i2c_match_id() was changed as follows:

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -101,9 +101,17 @@ EXPORT_SYMBOL_GPL(i2c_freq_mode_string);
 const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 						const struct i2c_client *client)
 {
-	if (!(id && client))
+	if (!client)
 		return NULL;
 
+	if (!id) {
+		struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
+
+		id = driver->id_table;
+		if (!id)
+			return NULL;
+	}
+
 	while (id->name[0]) {
 		if (strcmp(client->name, id->name) == 0)
 			return id;


the patch under discussion could be reduced to:

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index adc66b3615c0..b1d11c96597d 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1432,9 +1432,9 @@ static void kxcjk1013_disable_regulators(void *d)
 	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
 }
 
-static int kxcjk1013_probe(struct i2c_client *client,
-			   const struct i2c_device_id *id)
+static int kxcjk1013_probe(struct i2c_client *client)
 {
+	const struct i2c_device_id *id = i2c_match_id(NULL, client);
 	struct kxcjk1013_data *data;
 	struct iio_dev *indio_dev;
 	struct kxcjk_1013_platform_data *pdata;
@@ -1749,7 +1749,7 @@ static struct i2c_driver kxcjk1013_driver = {
 		.of_match_table = kxcjk1013_of_match,
 		.pm	= &kxcjk1013_pm_ops,
 	},
-	.probe		= kxcjk1013_probe,
+	.probe_new	= kxcjk1013_probe,
 	.remove		= kxcjk1013_remove,
 	.id_table	= kxcjk1013_id,
 };

I assume you agree up to here.

After that change there is some incentive to do

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index b1d11c96597d..e043dd698747 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1432,9 +1432,20 @@ static void kxcjk1013_disable_regulators(void *d)
 	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
 }
 
+static const struct i2c_device_id kxcjk1013_id[] = {
+	{"kxcjk1013", KXCJK1013},
+	{"kxcj91008", KXCJ91008},
+	{"kxtj21009", KXTJ21009},
+	{"kxtf9",     KXTF9},
+	{"kx023-1025", KX0231025},
+	{"SMO8500",   KXCJ91008},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
+
 static int kxcjk1013_probe(struct i2c_client *client)
 {
-	const struct i2c_device_id *id = i2c_match_id(NULL, client);
+	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
 	struct kxcjk1013_data *data;
 	struct iio_dev *indio_dev;
 	struct kxcjk_1013_platform_data *pdata;
@@ -1720,18 +1731,6 @@ static const struct acpi_device_id kx_acpi_match[] = {
 };
 MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
 
-static const struct i2c_device_id kxcjk1013_id[] = {
-	{"kxcjk1013", KXCJK1013},
-	{"kxcj91008", KXCJ91008},
-	{"kxtj21009", KXTJ21009},
-	{"kxtf9",     KXTF9},
-	{"kx023-1025", KX0231025},
-	{"SMO8500",   KXCJ91008},
-	{}
-};
-
-MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
-
 static const struct of_device_id kxcjk1013_of_match[] = {
 	{ .compatible = "kionix,kxcjk1013", },
 	{ .compatible = "kionix,kxcj91008", },

on top to safe two pointer dereferences. The sum of the two driver
patches is exactly the effect of my patch just without the i2c core
change. (And the two pointer dereferences that are saved by the 2nd
patch are introduced by the first, so it's fine to not split that
change into two parts.)

> > > While I like the rest of the series, these things I would rather postpone
> > > or rework.
> > 
> > There is no win in postponing, is there?[1] What would be your preferred
> > way to rework?
> 
> My understand of the probe_new is that an attempt to unify i2c with how spi
> does.

My understanding is a bit different, but this detail doesn't matter.

> So, why not teach i2c_match_id() to handle this nicely for the caller?

The metric for "nice" is obviously subjective. For me it's nice to pass
a local symbol to an API function to make the function's job a tad
easier and more effective to solve. And that even if I have to reorder
the caller a bit.

> This will allow to leave tables where they are (or move closer to struct
> driver),

As written above, reordering the driver is (IMHO) cheap enough given the
benefit.

> reduce churn with the using of current i2c_match_id() as you
> showed the long line to get that table.

Do you still remember the original patch? That one doesn't have the long
i2c_match_id() line.

(Do you see your statement is an argument for my approach? The long line
is an indication that it's complicated to determine the address of the
table via ->driver. You can hide that by pushing the needed effort into
i2c_match_id() or a macro, but in the end the complexity remains for the
CPU.)

> This might need a new API to avoid
> changing many drivers at once. But it's business as usual.

My approach doesn't need a new API. That's nice, isn't it?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-10-31 23:38             ` Uwe Kleine-König
@ 2022-11-01 14:54               ` Andy Shevchenko
  2022-11-01 21:49                 ` Uwe Kleine-König
  0 siblings, 1 reply; 53+ messages in thread
From: Andy Shevchenko @ 2022-11-01 14:54 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Miaoqian Lin, Lars-Peter Clausen, Dmitry Rokosov, linux-iio,
	Gwendal Grignou, Wolfram Sang, kernel, Yang Yingliang,
	wangjianli, Vladimir Oltean, Jonathan Cameron

On Tue, Nov 01, 2022 at 12:38:43AM +0100, Uwe Kleine-König wrote:
> On Mon, Oct 24, 2022 at 12:46:02PM +0300, Andy Shevchenko wrote:
> > On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:

...

> > Exactly. And it means let's put my problem to someone's else shoulders.
> 
> You have a problem that I fail to see. Why is defining the id table
> before the probe function bad?
> 
> Unless I misunderstand you, you seem to assume that in the nearer future
> someone will have the urge to put the id table below the probe function
> again. What would you think is their motivation?

The problem with moving the table is the sparse locations in the code for
semantically relative things, like all ID tables to be near to each other. With
your approach you can easily break that and go for let's put one ID table on
top, because some code fails to indirectly access it, and leave another
somewhere else. I do not like this.

Besides, your change making unneeded churn of "I like to move it, move it" for
no real gain.

...

> > I don't see benefit of dereferencing tables by name. The table has to be
> > available via struct driver, otherwise, how the heck we even got into the
> > ->probe() there.
> 
> It is possible, it's just cheaper (in cpu cycles) to calculate the
> address of the table directly (i.e. via PC + $offset) instead of via
> dereferencing two pointers.

It's a slow path.

...

> I fail to follow you again. I talked about drivers/rtc/rtc-isl1208.c as
> it is in v6.1-rc1. There the probe function doesn't access the table at
> all. Neither via the driver link nor by name. That driver just defines
> the id table before the probe function.

So, if it is on top, fine, it would be just an inconsistent style.

...

> > Alternative is to avoid reordering to begin with, no?
> 
> Yeah, that could be done. But I don't see the advantage and you fail to
> explain it in a way for me to understand.
> 
> So if i2c_match_id() was changed as follows:

There is another approach in the discussion and Wolfram acknowledged it already
(with a new API to retrieve the necessary data).

...

> +static const struct i2c_device_id kxcjk1013_id[] = {
> +	{"kxcjk1013", KXCJK1013},
> +	{"kxcj91008", KXCJ91008},
> +	{"kxtj21009", KXTJ21009},
> +	{"kxtf9",     KXTF9},
> +	{"kx023-1025", KX0231025},
> +	{"SMO8500",   KXCJ91008},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> +
>  static int kxcjk1013_probe(struct i2c_client *client)
>  {
> -	const struct i2c_device_id *id = i2c_match_id(NULL, client);
> +	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
>  	struct kxcjk1013_data *data;
>  	struct iio_dev *indio_dev;
>  	struct kxcjk_1013_platform_data *pdata;
> @@ -1720,18 +1731,6 @@ static const struct acpi_device_id kx_acpi_match[] = {
>  };
>  MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
>  
> -static const struct i2c_device_id kxcjk1013_id[] = {
> -	{"kxcjk1013", KXCJK1013},
> -	{"kxcj91008", KXCJ91008},
> -	{"kxtj21009", KXTJ21009},
> -	{"kxtf9",     KXTF9},
> -	{"kx023-1025", KX0231025},
> -	{"SMO8500",   KXCJ91008},
> -	{}
> -};
> -
> -MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);

Can we avoid moving it?

>  static const struct of_device_id kxcjk1013_of_match[] = {
>  	{ .compatible = "kionix,kxcjk1013", },
>  	{ .compatible = "kionix,kxcj91008", },
> 
> on top to safe two pointer dereferences. The sum of the two driver
> patches is exactly the effect of my patch just without the i2c core
> change. (And the two pointer dereferences that are saved by the 2nd
> patch are introduced by the first, so it's fine to not split that
> change into two parts.)

...

> > So, why not teach i2c_match_id() to handle this nicely for the caller?
> 
> The metric for "nice" is obviously subjective. For me it's nice to pass
> a local symbol to an API function to make the function's job a tad
> easier and more effective to solve. And that even if I have to reorder
> the caller a bit.

So, it seems a preferred design paradigm: straightforward vs. OOP.
Kernel is written with OOP in mind, why to avoid that?

...

> > reduce churn with the using of current i2c_match_id() as you
> > showed the long line to get that table.
> 
> Do you still remember the original patch? That one doesn't have the long
> i2c_match_id() line.
> 
> (Do you see your statement is an argument for my approach? The long line
> is an indication that it's complicated to determine the address of the
> table via ->driver. You can hide that by pushing the needed effort into
> i2c_match_id() or a macro, but in the end the complexity remains for the
> CPU.)

Does it matter?

OTOH that will be aligned with SPI framework and idea behind ->probe_new()
as I understood it.

...

> > This might need a new API to avoid
> > changing many drivers at once. But it's business as usual.
> 
> My approach doesn't need a new API. That's nice, isn't it?

It depends. In this case it's not nice since it requires
"I like to move it, move it".

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-11-01 14:54               ` Andy Shevchenko
@ 2022-11-01 21:49                 ` Uwe Kleine-König
  2022-11-02 13:57                   ` Andy Shevchenko
  2022-11-02 20:46                   ` Wolfram Sang
  0 siblings, 2 replies; 53+ messages in thread
From: Uwe Kleine-König @ 2022-11-01 21:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Miaoqian Lin, Lars-Peter Clausen, linux-iio, Gwendal Grignou,
	Wolfram Sang, Vladimir Oltean, kernel, Yang Yingliang,
	wangjianli, Dmitry Rokosov, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 7679 bytes --]

Hello Andy,

On Tue, Nov 01, 2022 at 04:54:52PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 01, 2022 at 12:38:43AM +0100, Uwe Kleine-König wrote:
> > On Mon, Oct 24, 2022 at 12:46:02PM +0300, Andy Shevchenko wrote:
> > > On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:
> 
> ...
> 
> > > Exactly. And it means let's put my problem to someone's else shoulders.
> > 
> > You have a problem that I fail to see. Why is defining the id table
> > before the probe function bad?
> > 
> > Unless I misunderstand you, you seem to assume that in the nearer future
> > someone will have the urge to put the id table below the probe function
> > again. What would you think is their motivation?
> 
> The problem with moving the table is the sparse locations in the code for
> semantically relative things, like all ID tables to be near to each other.

I don't understand that reasoning. Is that important for the compiler or
the human reader? What is "semantically relative"?

> With
> your approach you can easily break that and go for let's put one ID table on
> top, because some code fails to indirectly access it, and leave another
> somewhere else. I do not like this.
> 
> Besides, your change making unneeded churn of "I like to move it, move it" for
> no real gain.

That's not true. It's not that I like to move it. Moving is necessary to
make use of the local symbol in .probe() without a forward declaration.
(If you claimed that adding a forward declaration was churn, I'd agree.)
 
> ...
> 
> > > I don't see benefit of dereferencing tables by name. The table has to be
> > > available via struct driver, otherwise, how the heck we even got into the
> > > ->probe() there.
> > 
> > It is possible, it's just cheaper (in cpu cycles) to calculate the
> > address of the table directly (i.e. via PC + $offset) instead of via
> > dereferencing two pointers.
> 
> It's a slow path.

Somewhat agreed. It influences boot time though. (Well I guess, maybe
it's too little to be actually measureable for a single driver? Still in
my bubble people are sensitive to boot time and I'd consider this a low
hanging fruit for (admittedly small) optimisation.)

> ...
> 
> > I fail to follow you again. I talked about drivers/rtc/rtc-isl1208.c as
> > it is in v6.1-rc1. There the probe function doesn't access the table at
> > all. Neither via the driver link nor by name. That driver just defines
> > the id table before the probe function.
> 
> So, if it is on top, fine, it would be just an inconsistent style.
> 
> ...
> 
> > > Alternative is to avoid reordering to begin with, no?
> > 
> > Yeah, that could be done. But I don't see the advantage and you fail to
> > explain it in a way for me to understand.
> > 
> > So if i2c_match_id() was changed as follows:
> 
> There is another approach in the discussion and Wolfram acknowledged it already
> (with a new API to retrieve the necessary data).

Yeah, saw it. And as expected the follow up patch converting
drivers/iio/pressure/bmp280-i2c.c "suffers" from the double pointer
dereference. But it looks nice because the effort to determine the table
via driver is well hidden.

> ...
> 
> > +static const struct i2c_device_id kxcjk1013_id[] = {
> > +	{"kxcjk1013", KXCJK1013},
> > +	{"kxcj91008", KXCJ91008},
> > +	{"kxtj21009", KXTJ21009},
> > +	{"kxtf9",     KXTF9},
> > +	{"kx023-1025", KX0231025},
> > +	{"SMO8500",   KXCJ91008},
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> > +
> >  static int kxcjk1013_probe(struct i2c_client *client)
> >  {
> > -	const struct i2c_device_id *id = i2c_match_id(NULL, client);
> > +	const struct i2c_device_id *id = i2c_match_id(kxcjk1013_id, client);
> >  	struct kxcjk1013_data *data;
> >  	struct iio_dev *indio_dev;
> >  	struct kxcjk_1013_platform_data *pdata;
> > @@ -1720,18 +1731,6 @@ static const struct acpi_device_id kx_acpi_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(acpi, kx_acpi_match);
> >  
> > -static const struct i2c_device_id kxcjk1013_id[] = {
> > -	{"kxcjk1013", KXCJK1013},
> > -	{"kxcj91008", KXCJ91008},
> > -	{"kxtj21009", KXTJ21009},
> > -	{"kxtf9",     KXTF9},
> > -	{"kx023-1025", KX0231025},
> > -	{"SMO8500",   KXCJ91008},
> > -	{}
> > -};
> > -
> > -MODULE_DEVICE_TABLE(i2c, kxcjk1013_id);
> 
> Can we avoid moving it?

Yes, but the alternatives are not better in my eyes. (You could use
i2c_client_get_device_id() or add a forward declaration.) And moving it
is easy enough and I still don't understand the downsides you care
about.
 
> >  static const struct of_device_id kxcjk1013_of_match[] = {
> >  	{ .compatible = "kionix,kxcjk1013", },
> >  	{ .compatible = "kionix,kxcj91008", },
> > 
> > on top to safe two pointer dereferences. The sum of the two driver
> > patches is exactly the effect of my patch just without the i2c core
> > change. (And the two pointer dereferences that are saved by the 2nd
> > patch are introduced by the first, so it's fine to not split that
> > change into two parts.)
> 
> ...
> 
> > > So, why not teach i2c_match_id() to handle this nicely for the caller?
> > 
> > The metric for "nice" is obviously subjective. For me it's nice to pass
> > a local symbol to an API function to make the function's job a tad
> > easier and more effective to solve. And that even if I have to reorder
> > the caller a bit.
> 
> So, it seems a preferred design paradigm: straightforward vs. OOP.
> Kernel is written with OOP in mind, why to avoid that?
> 
> ...
> 
> > > reduce churn with the using of current i2c_match_id() as you
> > > showed the long line to get that table.
> > 
> > Do you still remember the original patch? That one doesn't have the long
> > i2c_match_id() line.
> > 
> > (Do you see your statement is an argument for my approach? The long line
> > is an indication that it's complicated to determine the address of the
> > table via ->driver. You can hide that by pushing the needed effort into
> > i2c_match_id() or a macro, but in the end the complexity remains for the
> > CPU.)
> 
> Does it matter?

What is "it"? The long line? (Then no, it doesn't matter because it
doesn't appear in neither your nor my approach.) The effort to pull the
table's address out of ->driver? (We seem to disagree. I think it's easy
enough not to do it to justify the optimisation.) Or to hide the effort?
(I think hiding effort and so making it easy to pick a more expensive
approach is at least questionable.)
 
> OTOH that will be aligned with SPI framework and idea behind ->probe_new()
> as I understood it.
> 
> ...
> 
> > > This might need a new API to avoid
> > > changing many drivers at once. But it's business as usual.
> > 
> > My approach doesn't need a new API. That's nice, isn't it?
> 
> It depends. In this case it's not nice since it requires
> "I like to move it, move it".

I still don't get your reasoning and I think we have to agree to not
agree. IMHO the judgement is subjective, you call the move of the id
table "churn" and I call it "tiny effort for a little optimisation".

What I consider "churn" though is this discussion. I will stop my
participation here. That's a bit sad because in my eyes all patches in
this series have a positive value and the discussion about (from my POV)
incomprehensible minor details destroyed my motivation to work on the
quest to convert all drivers to .probe_new() :-\

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-11-01 21:49                 ` Uwe Kleine-König
@ 2022-11-02 13:57                   ` Andy Shevchenko
  2022-11-02 20:46                   ` Wolfram Sang
  1 sibling, 0 replies; 53+ messages in thread
From: Andy Shevchenko @ 2022-11-02 13:57 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Miaoqian Lin, Lars-Peter Clausen, linux-iio, Gwendal Grignou,
	Wolfram Sang, Vladimir Oltean, kernel, Yang Yingliang,
	wangjianli, Dmitry Rokosov, Jonathan Cameron

On Tue, Nov 01, 2022 at 10:49:39PM +0100, Uwe Kleine-König wrote:
> On Tue, Nov 01, 2022 at 04:54:52PM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 01, 2022 at 12:38:43AM +0100, Uwe Kleine-König wrote:
> > > On Mon, Oct 24, 2022 at 12:46:02PM +0300, Andy Shevchenko wrote:
> > > > On Mon, Oct 24, 2022 at 11:14:56AM +0200, Uwe Kleine-König wrote:

I really do appreciate your work on this, but it's pity that my point
is still unclear to you. As a beginning point I assume that the idea
of ->probe_new() is to mimic what SPI core does. That's why I consider
moving the tables is smelling like a half-baked work. Besides that
I tried to explain again on the concerns I have below.

...

> > > > Exactly. And it means let's put my problem to someone's else shoulders.
> > > 
> > > You have a problem that I fail to see. Why is defining the id table
> > > before the probe function bad?
> > > 
> > > Unless I misunderstand you, you seem to assume that in the nearer future
> > > someone will have the urge to put the id table below the probe function
> > > again. What would you think is their motivation?
> > 
> > The problem with moving the table is the sparse locations in the code for
> > semantically relative things, like all ID tables to be near to each other.
> 
> I don't understand that reasoning. Is that important for the compiler or
> the human reader? What is "semantically relative"?

If we have let's say 2+ ID tables, without your patches

...module code...

...TABLE 1...
...TABLE 2...

static struct foo_driver *drv = {
	...references to the tables...
};

With your patches (that move the ID table):


...module header...
...module code...

...TABLE 1...

...more module code...

...TABLE 2...

static struct foo_driver *drv = {
	...references to the tables...
};

which I consider as unneeded sparse for like you said micro-optimization
of the direct access to the table.

There are two ways to solve this:
- move all tables together
- provide an API as done by SPI core

I prefer the latter over the former.

> > With
> > your approach you can easily break that and go for let's put one ID table on
> > top, because some code fails to indirectly access it, and leave another
> > somewhere else. I do not like this.
> > 
> > Besides, your change making unneeded churn of "I like to move it, move it" for
> > no real gain.
> 
> That's not true. It's not that I like to move it. Moving is necessary to
> make use of the local symbol in .probe() without a forward declaration.
> (If you claimed that adding a forward declaration was churn, I'd agree.)

On forward declaration we agree.

But using local symbol directly as a shortcut to access some field of the
instance of some object is against OOP paradigm.

I.o.w. some shortcuts maybe nice, but this kind of approach leads to
layering violation and similar problems to begin with.

...

> > There is another approach in the discussion and Wolfram acknowledged it already
> > (with a new API to retrieve the necessary data).
> 
> Yeah, saw it. And as expected the follow up patch converting
> drivers/iio/pressure/bmp280-i2c.c "suffers" from the double pointer
> dereference. But it looks nice because the effort to determine the table
> via driver is well hidden.

Yes, that is the downside of OOP, I agree.

...

> > > > reduce churn with the using of current i2c_match_id() as you
> > > > showed the long line to get that table.
> > > 
> > > Do you still remember the original patch? That one doesn't have the long
> > > i2c_match_id() line.
> > > 
> > > (Do you see your statement is an argument for my approach? The long line
> > > is an indication that it's complicated to determine the address of the
> > > table via ->driver. You can hide that by pushing the needed effort into
> > > i2c_match_id() or a macro, but in the end the complexity remains for the
> > > CPU.)
> > 
> > Does it matter?
> 
> What is "it"? The long line? (Then no, it doesn't matter because it
> doesn't appear in neither your nor my approach.) The effort to pull the
> table's address out of ->driver? (We seem to disagree. I think it's easy
> enough not to do it to justify the optimisation.) Or to hide the effort?
> (I think hiding effort and so making it easy to pick a more expensive
> approach is at least questionable.)

IT == The length of dereferencing chain to get the pointer to the data
structure in question.

So, it is OOP model versus micro-optimization on a slow path.
That's how I can class the basis of our disagreement.

> > OTOH that will be aligned with SPI framework and idea behind ->probe_new()
> > as I understood it.

...

> What I consider "churn" though is this discussion.

I agree on that as well. Nut it's partially my issue and I'm sorry for that.

> I will stop my participation here. That's a bit sad because in my eyes all
> patches in this series have a positive value

Your patches (that don't move the tables) are nice job!

> and the discussion about (from
> my POV) incomprehensible minor details destroyed my motivation to work on
> the quest to convert all drivers to .probe_new() :-\

It's pity to hear, but you may also imagine how many times I was in
the similar situation and for some cases I lost my motivation, indeed.
I feel your pain.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new
  2022-11-01 21:49                 ` Uwe Kleine-König
  2022-11-02 13:57                   ` Andy Shevchenko
@ 2022-11-02 20:46                   ` Wolfram Sang
  1 sibling, 0 replies; 53+ messages in thread
From: Wolfram Sang @ 2022-11-02 20:46 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Andy Shevchenko, Miaoqian Lin, Lars-Peter Clausen, linux-iio,
	Gwendal Grignou, Vladimir Oltean, kernel, Yang Yingliang,
	wangjianli, Dmitry Rokosov, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

Hi Uwe,

> What I consider "churn" though is this discussion. I will stop my
> participation here. That's a bit sad because in my eyes all patches in
> this series have a positive value and the discussion about (from my POV)
> incomprehensible minor details destroyed my motivation to work on the
> quest to convert all drivers to .probe_new() :-\

To get less pain of such a conversion, team up with Stephen Kitt who
also works on converting the drivers to probe_new[1].

Happy hacking,

   Wolfram

[1] http://patchwork.ozlabs.org/project/linux-i2c/list/?submitter=Kitt&series=&q=&archive=&delegate=&state=*
(but it might be that he did more and just not CC the i2c list anymore)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2022-11-02 20:46 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-23 13:22 [PATCH 00/23] iio: accel: Convert to i2c's .probe_new Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 01/23] iio: accel: adxl367: Convert to i2c's .probe_new() Uwe Kleine-König
2022-10-29 11:43   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 02/23] iio: accel: adxl372: Convert to i2c's .probe_new Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 03/23] iio: accel: bma400: " Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 04/23] iio: accel: bmc150: " Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 05/23] iio: accel: da280: " Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 06/23] iio: accel: da311: Convert to i2c's .probe_new() Uwe Kleine-König
2022-10-29 11:51   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 07/23] iio: accel: dmard06: " Uwe Kleine-König
2022-10-29 11:51   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 08/23] iio: accel: dmard09: " Uwe Kleine-König
2022-10-29 11:52   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 09/23] iio: accel: dmard10: " Uwe Kleine-König
2022-10-29 11:53   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 10/23] iio: accel: kxcjk-1013: Convert to i2c's .probe_new Uwe Kleine-König
2022-10-23 19:06   ` Andy Shevchenko
2022-10-24  7:05     ` Uwe Kleine-König
2022-10-24  8:39       ` Andy Shevchenko
2022-10-24  9:14         ` Uwe Kleine-König
2022-10-24  9:46           ` Andy Shevchenko
2022-10-24 10:22             ` Nuno Sá
2022-10-24 11:40               ` Andy Shevchenko
2022-10-29 11:49                 ` Jonathan Cameron
2022-10-31 23:38             ` Uwe Kleine-König
2022-11-01 14:54               ` Andy Shevchenko
2022-11-01 21:49                 ` Uwe Kleine-König
2022-11-02 13:57                   ` Andy Shevchenko
2022-11-02 20:46                   ` Wolfram Sang
2022-10-23 13:22 ` [PATCH 11/23] iio: accel: kxsd9: Convert to i2c's .probe_new() Uwe Kleine-König
2022-10-29 11:54   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 12/23] iio: accel: mc3230: " Uwe Kleine-König
2022-10-29 11:57   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 13/23] iio: accel: mma7455: Convert to i2c's .probe_new Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 14/23] iio: accel: mma7660: Convert to i2c's .probe_new() Uwe Kleine-König
2022-10-29 11:55   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 15/23] iio: accel: mma8452: Convert to i2c's .probe_new Uwe Kleine-König
2022-10-23 13:22 ` [PATCH 16/23] iio: accel: mma9551: " Uwe Kleine-König
2022-10-23 19:06   ` Andy Shevchenko
2022-10-23 13:22 ` [PATCH 17/23] iio: accel: mma9553: " Uwe Kleine-König
2022-10-23 19:07   ` Andy Shevchenko
2022-10-23 13:22 ` [PATCH 18/23] iio: accel: mxc4005: Convert to i2c's .probe_new() Uwe Kleine-König
2022-10-29 11:56   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 19/23] iio: accel: mxc6255: " Uwe Kleine-König
2022-10-29 11:57   ` Jonathan Cameron
2022-10-23 13:22 ` [PATCH 20/23] iio: accel: stk8312: " Uwe Kleine-König
2022-10-29 11:58   ` Jonathan Cameron
2022-10-23 13:23 ` [PATCH 21/23] iio: accel: stk8ba50: " Uwe Kleine-König
2022-10-29 11:59   ` Jonathan Cameron
2022-10-23 13:23 ` [PATCH 22/23] iio: accel: st_magn: " Uwe Kleine-König
2022-10-29 12:00   ` Jonathan Cameron
2022-10-23 13:23 ` [PATCH 23/23] iio: accel: vl6180: " Uwe Kleine-König
2022-10-29 12:01   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.