[PATCH v9 0/7] i2c: xiic: use generic device property accessors

[PATCH v9 0/7] i2c: xiic: use generic device property accessors

[Abdurrahman Hussain via B4 Relay]

- Switch to generic device property accessors.
- Switch to managed devm_ functions to simplify error handling.
- General cleanups.
- Skip clock setup on non-OF systems where clock is not specified via
firmware.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Changes in v9:
- Fix an issue found by the test robot in the cleanup patch.
- Reworded the clock change description.
- Add a patch to switch to i2c_add_numbered_adapter().
- Link to v8: https://lore.kernel.org/r/20260201-i2c-xiic-v8-0-deb2dd158d1a@nexthop.ai

Changes in v8:
- Made the clock change the last patch in the series.
- Reworded the clock change description.
- Changed dev->fwnode to dev_fwnode(dev).
- Link to v7: https://lore.kernel.org/r/20260129-i2c-xiic-v7-0-727e434897ef@nexthop.ai

Changes in v7:
- Drop the dt-bindings patch.
- Skip clock setup on non-OF systems.
- Minor commit body rewording.
- Applied code-review trailers with `b4 trailers -u`
- Link to v6: https://lore.kernel.org/r/20260127-i2c-xiic-v6-0-e82e2f6f657c@nexthop.ai

Changes in v6:
- Cosmetic changes to address the comments.
- Added a patch to use resource format specifier in debug log.
- Link to v5: https://lore.kernel.org/r/20260126-i2c-xiic-v5-0-88a16a28721c@nexthop.ai

Changes in v5:
- Reorder the cosmetic patch to be the last in the series.
- Added a documentation patch to describe the optional clock.
- Minor commit body rewording.
- Link to v4: https://lore.kernel.org/r/20260123-i2c-xiic-v4-0-4a3eba3510ce@nexthop.ai

Changes in v4:
- Reorder the cosmetic patch to be the first in the series.
- Amend the mutex_init patch to also switch to the managed pm_runtime_
  variant.
- Link to v3: https://lore.kernel.org/r/20260123-i2c-xiic-v3-0-eb7cd4254dfb@nexthop.ai

Changes in v3:
- Reorder the "optional clock" patch to be the first in the series. 
- Add a patch to switch to devm_mutex_init().
- Remove dup message in error path.
- Cosmetic: use temporary dev variable.
- Link to v2: https://lore.kernel.org/r/20260122-i2c-xiic-v2-0-134f5d743e8b@nexthop.ai

Changes in v2:
- Split the patch into two independent changes.
- Added struct device *dev at the top of probe() and remove() to re-use.
- Switched to device_set_node(...)

---
Abdurrahman Hussain (7):
      i2c: xiic: switch to devres managed APIs
      i2c: xiic: remove duplicate error message
      i2c: xiic: switch to generic device property accessors
      i2c: xiic: cosmetic cleanup
      i2c: xiic: cosmetic: use resource format specifier in debug log
      i2c: xiic: use numbered adapter registration
      i2c: xiic: skip input clock setup on non-OF systems

 drivers/i2c/busses/i2c-xiic.c | 99 +++++++++++++++++++------------------------
 1 file changed, 43 insertions(+), 56 deletions(-)
---
base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
change-id: 20260122-i2c-xiic-3ba89ff5ea93

Best regards,
-- 
Abdurrahman Hussain <abdurrahman@nexthop.ai>

[PATCH v9 1/7] i2c: xiic: switch to devres managed APIs

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Simplify the error code paths by switching to devres managed helper
functions.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 28015d77599d..16ff83fe280b 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1423,6 +1423,7 @@ MODULE_DEVICE_TABLE(of, xiic_of_match);
 
 static int xiic_i2c_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct xiic_i2c *i2c;
 	struct xiic_i2c_platform_data *pdata;
 	const struct of_device_id *match;
@@ -1461,7 +1462,10 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
 		 DRIVER_NAME " %s", pdev->name);
 
-	mutex_init(&i2c->lock);
+	ret = devm_mutex_init(dev, &i2c->lock);
+	if (ret)
+		return ret;
+
 	spin_lock_init(&i2c->atomic_lock);
 
 	i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL);
@@ -1472,8 +1476,9 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	i2c->dev = &pdev->dev;
 	pm_runtime_set_autosuspend_delay(i2c->dev, XIIC_PM_TIMEOUT);
 	pm_runtime_use_autosuspend(i2c->dev);
-	pm_runtime_set_active(i2c->dev);
-	pm_runtime_enable(i2c->dev);
+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
+		return ret;
 
 	/* SCL frequency configuration */
 	i2c->input_clk = clk_get_rate(i2c->clk);
@@ -1489,7 +1494,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	if (ret < 0) {
 		dev_err_probe(&pdev->dev, ret, "Cannot claim IRQ\n");
-		goto err_pm_disable;
+		return ret;
 	}
 
 	i2c->singlemaster =
@@ -1508,16 +1513,14 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 		i2c->endianness = BIG;
 
 	ret = xiic_reinit(i2c);
-	if (ret < 0) {
-		dev_err_probe(&pdev->dev, ret, "Cannot xiic_reinit\n");
-		goto err_pm_disable;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot xiic_reinit\n");
 
 	/* add i2c adapter to i2c tree */
 	ret = i2c_add_adapter(&i2c->adap);
 	if (ret) {
 		xiic_deinit(i2c);
-		goto err_pm_disable;
+		return ret;
 	}
 
 	if (pdata) {
@@ -1529,12 +1532,6 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	dev_dbg(&pdev->dev, "mmio %08lx irq %d scl clock frequency %d\n",
 		(unsigned long)res->start, irq, i2c->i2c_clk);
 
-	return 0;
-
-err_pm_disable:
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_set_suspended(&pdev->dev);
-
 	return ret;
 }
 
@@ -1555,8 +1552,6 @@ static void xiic_i2c_remove(struct platform_device *pdev)
 		xiic_deinit(i2c);
 
 	pm_runtime_put_sync(i2c->dev);
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 }
 

-- 
2.52.0

[PATCH v9 2/7] i2c: xiic: remove duplicate error message

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The devm_request_threaded_irq() already prints an error message. Remove
the duplicate.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 16ff83fe280b..4cb7c40fb565 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1491,11 +1491,8 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 					xiic_process, IRQF_ONESHOT,
 					pdev->name, i2c);
-
-	if (ret < 0) {
-		dev_err_probe(&pdev->dev, ret, "Cannot claim IRQ\n");
+	if (ret)
 		return ret;
-	}
 
 	i2c->singlemaster =
 		of_property_read_bool(pdev->dev.of_node, "single-master");

-- 
2.52.0

[PATCH v9 3/7] i2c: xiic: switch to generic device property accessors

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Use generic device property accessors.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 4cb7c40fb565..60d417a2266e 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -27,7 +27,6 @@
 #include <linux/platform_data/i2c-xiic.h>
 #include <linux/io.h>
 #include <linux/slab.h>
-#include <linux/of.h>
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
 #include <linux/iopoll.h>
@@ -1408,7 +1407,6 @@ static const struct i2c_adapter xiic_adapter = {
 	.algo = &xiic_algorithm,
 };
 
-#if defined(CONFIG_OF)
 static const struct xiic_version_data xiic_2_00 = {
 	.quirks = DYNAMIC_MODE_READ_BROKEN_BIT,
 };
@@ -1419,14 +1417,13 @@ static const struct of_device_id xiic_of_match[] = {
 	{},
 };
 MODULE_DEVICE_TABLE(of, xiic_of_match);
-#endif
 
 static int xiic_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct xiic_i2c *i2c;
 	struct xiic_i2c_platform_data *pdata;
-	const struct of_device_id *match;
+	const struct xiic_version_data *data;
 	struct resource *res;
 	int ret, irq;
 	u8 i;
@@ -1436,12 +1433,9 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
-	match = of_match_node(xiic_of_match, pdev->dev.of_node);
-	if (match && match->data) {
-		const struct xiic_version_data *data = match->data;
-
+	data = device_get_match_data(dev);
+	if (data)
 		i2c->quirks = data->quirks;
-	}
 
 	i2c->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(i2c->base))
@@ -1458,7 +1452,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	i2c->adap = xiic_adapter;
 	i2c_set_adapdata(&i2c->adap, i2c);
 	i2c->adap.dev.parent = &pdev->dev;
-	i2c->adap.dev.of_node = pdev->dev.of_node;
+	device_set_node(&i2c->adap.dev, dev_fwnode(dev));
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
 		 DRIVER_NAME " %s", pdev->name);
 
@@ -1482,8 +1476,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	/* SCL frequency configuration */
 	i2c->input_clk = clk_get_rate(i2c->clk);
-	ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-				   &i2c->i2c_clk);
+	ret = device_property_read_u32(dev, "clock-frequency", &i2c->i2c_clk);
 	/* If clock-frequency not specified in DT, do not configure in SW */
 	if (ret || i2c->i2c_clk > I2C_MAX_FAST_MODE_PLUS_FREQ)
 		i2c->i2c_clk = 0;
@@ -1494,8 +1487,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	i2c->singlemaster =
-		of_property_read_bool(pdev->dev.of_node, "single-master");
+	i2c->singlemaster = device_property_read_bool(dev, "single-master");
 
 	/*
 	 * Detect endianness

-- 
2.52.0

[PATCH v9 4/7] i2c: xiic: cosmetic cleanup

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Re-use dev pointer instead of referencing &pdev->dev everywhere.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 60d417a2266e..ade0fd66451b 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1429,7 +1429,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	u8 i;
 	u32 sr;
 
-	i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
+	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
 
@@ -1445,13 +1445,13 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	pdata = dev_get_platdata(&pdev->dev);
+	pdata = dev_get_platdata(dev);
 
 	/* hook up driver to tree */
 	platform_set_drvdata(pdev, i2c);
 	i2c->adap = xiic_adapter;
 	i2c_set_adapdata(&i2c->adap, i2c);
-	i2c->adap.dev.parent = &pdev->dev;
+	i2c->adap.dev.parent = dev;
 	device_set_node(&i2c->adap.dev, dev_fwnode(dev));
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
 		 DRIVER_NAME " %s", pdev->name);
@@ -1467,9 +1467,10 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
 				     "failed to enable input clock.\n");
 
-	i2c->dev = &pdev->dev;
-	pm_runtime_set_autosuspend_delay(i2c->dev, XIIC_PM_TIMEOUT);
-	pm_runtime_use_autosuspend(i2c->dev);
+	i2c->dev = dev;
+
+	pm_runtime_set_autosuspend_delay(dev, XIIC_PM_TIMEOUT);
+	pm_runtime_use_autosuspend(dev);
 	ret = devm_pm_runtime_set_active_enabled(dev);
 	if (ret)
 		return ret;
@@ -1481,9 +1482,8 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	if (ret || i2c->i2c_clk > I2C_MAX_FAST_MODE_PLUS_FREQ)
 		i2c->i2c_clk = 0;
 
-	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-					xiic_process, IRQF_ONESHOT,
-					pdev->name, i2c);
+	ret = devm_request_threaded_irq(dev, irq, NULL, xiic_process,
+					IRQF_ONESHOT, pdev->name, i2c);
 	if (ret)
 		return ret;
 
@@ -1518,7 +1518,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 			i2c_new_client_device(&i2c->adap, pdata->devices + i);
 	}
 
-	dev_dbg(&pdev->dev, "mmio %08lx irq %d scl clock frequency %d\n",
+	dev_dbg(dev, "mmio %08lx irq %d scl clock frequency %d\n",
 		(unsigned long)res->start, irq, i2c->i2c_clk);
 
 	return ret;
@@ -1526,22 +1526,22 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 static void xiic_i2c_remove(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct xiic_i2c *i2c = platform_get_drvdata(pdev);
 	int ret;
 
 	/* remove adapter & data */
 	i2c_del_adapter(&i2c->adap);
 
-	ret = pm_runtime_get_sync(i2c->dev);
-
+	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
-		dev_warn(&pdev->dev, "Failed to activate device for removal (%pe)\n",
+		dev_warn(dev, "Failed to activate device for removal (%pe)\n",
 			 ERR_PTR(ret));
 	else
 		xiic_deinit(i2c);
 
-	pm_runtime_put_sync(i2c->dev);
-	pm_runtime_dont_use_autosuspend(&pdev->dev);
+	pm_runtime_put_sync(dev);
+	pm_runtime_dont_use_autosuspend(dev);
 }
 
 static const struct dev_pm_ops xiic_dev_pm_ops = {

-- 
2.52.0

[PATCH v9 5/7] i2c: xiic: cosmetic: use resource format specifier in debug log

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Use standard resource format specifier %pR in debug log.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index ade0fd66451b..eeb8ba8e131e 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1518,8 +1518,8 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 			i2c_new_client_device(&i2c->adap, pdata->devices + i);
 	}
 
-	dev_dbg(dev, "mmio %08lx irq %d scl clock frequency %d\n",
-		(unsigned long)res->start, irq, i2c->i2c_clk);
+	dev_dbg(dev, "mmio %pR irq %d scl clock frequency %d\n",
+		res, irq, i2c->i2c_clk);
 
 	return ret;
 }

-- 
2.52.0

[PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Use i2c_add_numbered_adapter() to allow platform devices to specify
fixed bus numbers when needed.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index eeb8ba8e131e..2c0470452221 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1450,6 +1450,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	/* hook up driver to tree */
 	platform_set_drvdata(pdev, i2c);
 	i2c->adap = xiic_adapter;
+	i2c->adap.nr = pdev->id;
 	i2c_set_adapdata(&i2c->adap, i2c);
 	i2c->adap.dev.parent = dev;
 	device_set_node(&i2c->adap.dev, dev_fwnode(dev));
@@ -1506,7 +1507,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret, "Cannot xiic_reinit\n");
 
 	/* add i2c adapter to i2c tree */
-	ret = i2c_add_adapter(&i2c->adap);
+	ret = i2c_add_numbered_adapter(&i2c->adap);
 	if (ret) {
 		xiic_deinit(i2c);
 		return ret;

-- 
2.52.0

[PATCH v9 7/7] i2c: xiic: skip input clock setup on non-OF systems

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Currently Linux does not implement ACPI ClockInput to describe clock
resources, unlike DT. However the xiic driver is happy if something
magically enables the clock before the driver probes, and does not
turn it off again. The clock should always be considered optional for
ACPI.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/i2c/busses/i2c-xiic.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 2c0470452221..017e33d4309c 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1463,10 +1463,12 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	spin_lock_init(&i2c->atomic_lock);
 
-	i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL);
-	if (IS_ERR(i2c->clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
-				     "failed to enable input clock.\n");
+	if (is_of_node(dev_fwnode(dev))) {
+		i2c->clk = devm_clk_get_enabled(dev, NULL);
+		if (IS_ERR(i2c->clk))
+			return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
+					"failed to enable input clock.\n");
+	}
 
 	i2c->dev = dev;
 

-- 
2.52.0

Re: [PATCH v9 4/7] i2c: xiic: cosmetic cleanup

[Andy Shevchenko]

On Mon, Feb 02, 2026 at 08:37:21PM +0000, Abdurrahman Hussain via B4 Relay wrote:

> Re-use dev pointer instead of referencing &pdev->dev everywhere.

...

> -	dev_dbg(&pdev->dev, "mmio %08lx irq %d scl clock frequency %d\n",
> +	dev_dbg(dev, "mmio %08lx irq %d scl clock frequency %d\n",
>  		(unsigned long)res->start, irq, i2c->i2c_clk);

No need to do this here as we change the format string afterwards anyway.

But I leave it up to Andi, I won't prevent this to go in, if he agrees.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Andy Shevchenko]

On Mon, Feb 02, 2026 at 08:37:23PM +0000, Abdurrahman Hussain via B4 Relay wrote:

> Use i2c_add_numbered_adapter() to allow platform devices to specify
> fixed bus numbers when needed.

Not sure about this. Doesn't it break the current approach? Please, double
check that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 7/7] i2c: xiic: skip input clock setup on non-OF systems

[Andy Shevchenko]

On Mon, Feb 02, 2026 at 08:37:24PM +0000, Abdurrahman Hussain via B4 Relay wrote:

> Currently Linux does not implement ACPI ClockInput to describe clock

ClockInput() resource

> resources, unlike DT. However the xiic driver is happy if something
> magically enables the clock before the driver probes, and does not
> turn it off again. The clock should always be considered optional for
> ACPI.

...

> +	if (is_of_node(dev_fwnode(dev))) {
> +		i2c->clk = devm_clk_get_enabled(dev, NULL);
> +		if (IS_ERR(i2c->clk))
> +			return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),

Haven't you seen my previous comments? Why 'pdev'?

> +					"failed to enable input clock.\n");
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Abdurrahman Hussain]

> On Feb 3, 2026, at 7:45 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> On Mon, Feb 02, 2026 at 08:37:23PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> 
>> Use i2c_add_numbered_adapter() to allow platform devices to specify
>> fixed bus numbers when needed.
> 
> Not sure about this. Doesn't it break the current approach? Please, double
> check that.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

If pdev->id is PLATFORM_DEVID_NONE(-1) then i2c_add_numbered_adapter()
falls back to dynamic allocation and calls i2c_add_adapter().

Many existing i2c drivers use the same approach, see i2c-pxa.c and
i2c-pnx.c etc.

Re: [PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Andrew Lunn]

On Tue, Feb 03, 2026 at 10:14:49AM -0800, Abdurrahman Hussain wrote:
> 
> 
> > On Feb 3, 2026, at 7:45 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > 
> > On Mon, Feb 02, 2026 at 08:37:23PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> > 
> >> Use i2c_add_numbered_adapter() to allow platform devices to specify
> >> fixed bus numbers when needed.
> > 
> > Not sure about this. Doesn't it break the current approach? Please, double
> > check that.
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> > 
> 
> If pdev->id is PLATFORM_DEVID_NONE(-1) then i2c_add_numbered_adapter()
> falls back to dynamic allocation and calls i2c_add_adapter().
> 
> Many existing i2c drivers use the same approach, see i2c-pxa.c and
> i2c-pnx.c etc.

It is not about if other drivers do this. Its about does this change
the behaviour of this driver, so that I2C busses get different IDs
then before, and so cause regressions?

You need to explain in the commit message why you think this is safe.

    Andrew

Re: [PATCH v9 7/7] i2c: xiic: skip input clock setup on non-OF systems

[Abdurrahman Hussain]

> On Feb 3, 2026, at 7:46 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> On Mon, Feb 02, 2026 at 08:37:24PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> 
>> Currently Linux does not implement ACPI ClockInput to describe clock
> 
> ClockInput() resource
> 
>> resources, unlike DT. However the xiic driver is happy if something
>> magically enables the clock before the driver probes, and does not
>> turn it off again. The clock should always be considered optional for
>> ACPI.
> 

I just used the wording provided earlier by Andrew verbatim. But can change
to the following if you insist:

Currently Linux does not implement ACPI ClockInput() resource to describe
clocks, unlike DT. However the xiic driver is happy if something
magically enables the clock before the driver probes, and does not
turn it off again. The clock should always be considered optional for
ACPI.

> ...
> 
>> + if (is_of_node(dev_fwnode(dev))) {
>> + i2c->clk = devm_clk_get_enabled(dev, NULL);
>> + if (IS_ERR(i2c->clk))
>> + return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
> 
> Haven't you seen my previous comments? Why 'pdev’?
> 
Sorry, I missed it. Will fix in V10.

>> + "failed to enable input clock.\n");
>> + }
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Abdurrahman Hussain]

> On Feb 3, 2026, at 10:31 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> On Tue, Feb 03, 2026 at 10:14:49AM -0800, Abdurrahman Hussain wrote:
>> 
>> 
>>> On Feb 3, 2026, at 7:45 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>>> 
>>> On Mon, Feb 02, 2026 at 08:37:23PM +0000, Abdurrahman Hussain via B4 Relay wrote:
>>> 
>>>> Use i2c_add_numbered_adapter() to allow platform devices to specify
>>>> fixed bus numbers when needed.
>>> 
>>> Not sure about this. Doesn't it break the current approach? Please, double
>>> check that.
>>> 
>>> -- 
>>> With Best Regards,
>>> Andy Shevchenko
>>> 
>>> 
>> 
>> If pdev->id is PLATFORM_DEVID_NONE(-1) then i2c_add_numbered_adapter()
>> falls back to dynamic allocation and calls i2c_add_adapter().
>> 
>> Many existing i2c drivers use the same approach, see i2c-pxa.c and
>> i2c-pnx.c etc.
> 
> It is not about if other drivers do this. Its about does this change
> the behaviour of this driver, so that I2C busses get different IDs
> then before, and so cause regressions?
> 
> You need to explain in the commit message why you think this is safe.
> 
>    Andrew

Before, the driver was always doing the dynamic allocation due to
i2c_add_adapter(). So there could not have been a system that relied on
consistent i2c bus numbering. Even between reboots bus numbering could change.

The systems that did set pdev->id and were expecting a consistent i2c bus
numbering were broken to begin with.

This patch won’t break any existing systems or cause regressions.

Re: [PATCH v9 4/7] i2c: xiic: cosmetic cleanup

[Abdurrahman Hussain]

> On Feb 3, 2026, at 7:42 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> On Mon, Feb 02, 2026 at 08:37:21PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> 
>> Re-use dev pointer instead of referencing &pdev->dev everywhere.
> 
> ...
> 
>> - dev_dbg(&pdev->dev, "mmio %08lx irq %d scl clock frequency %d\n",
>> + dev_dbg(dev, "mmio %08lx irq %d scl clock frequency %d\n",
>> (unsigned long)res->start, irq, i2c->i2c_clk);
> 
> No need to do this here as we change the format string afterwards anyway.
> 
> But I leave it up to Andi, I won't prevent this to go in, if he agrees.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

No worries, I can do this in the following patch, since there is going to be
a v 10 anyways.

Re: [PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Abdurrahman Hussain]

> On Feb 3, 2026, at 10:50 AM, Abdurrahman Hussain <abdurrahman@nexthop.ai> wrote:
> 
> 
> 
>> On Feb 3, 2026, at 10:31 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> 
>> On Tue, Feb 03, 2026 at 10:14:49AM -0800, Abdurrahman Hussain wrote:
>>> 
>>> 
>>>> On Feb 3, 2026, at 7:45 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>>>> 
>>>> On Mon, Feb 02, 2026 at 08:37:23PM +0000, Abdurrahman Hussain via B4 Relay wrote:
>>>> 
>>>>> Use i2c_add_numbered_adapter() to allow platform devices to specify
>>>>> fixed bus numbers when needed.
>>>> 
>>>> Not sure about this. Doesn't it break the current approach? Please, double
>>>> check that.
>>>> 
>>>> -- 
>>>> With Best Regards,
>>>> Andy Shevchenko
>>>> 
>>>> 
>>> 
>>> If pdev->id is PLATFORM_DEVID_NONE(-1) then i2c_add_numbered_adapter()
>>> falls back to dynamic allocation and calls i2c_add_adapter().
>>> 
>>> Many existing i2c drivers use the same approach, see i2c-pxa.c and
>>> i2c-pnx.c etc.
>> 
>> It is not about if other drivers do this. Its about does this change
>> the behaviour of this driver, so that I2C busses get different IDs
>> then before, and so cause regressions?
>> 
>> You need to explain in the commit message why you think this is safe.
>> 
>>   Andrew
> 
> Before, the driver was always doing the dynamic allocation due to
> i2c_add_adapter(). So there could not have been a system that relied on
> consistent i2c bus numbering. Even between reboots bus numbering could change.
> 
> The systems that did set pdev->id and were expecting a consistent i2c bus
> numbering were broken to begin with.
> 
> This patch won’t break any existing systems or cause regressions.
> 

I can add the following to the commit if you find the wording acceptable:

Previously, the i2c bus numbers were dynamically allocated because of the use of
the i2c_add_adapter() call. This meant there were no guarantees about consistent
bus numbering. However, with this change, on systems that don’t set their platform
device ID and leave it at the default PLATFORM_DEVID_NONE, the bus numbers will
still be dynamically allocated because of the check in i2c_add_numbered_adapter().
Therefore there won’t be any change in behavior.

Re: [PATCH v9 6/7] i2c: xiic: use numbered adapter registration

[Andrew Lunn]

> Before, the driver was always doing the dynamic allocation due to
> i2c_add_adapter(). So there could not have been a system that relied on
> consistent i2c bus numbering. Even between reboots bus numbering could change.
> 
> The systems that did set pdev->id and were expecting a consistent i2c bus
> numbering were broken to begin with.
> 
> This patch won’t break any existing systems or cause regressions.

Great, now please add that to the commit message. These are the sort
of things reviewers want to know, is a change going to cause
regressions. If you have done your homework, mentioning it in the
commit message will help avoid the questions being asked.

       Andrew

Re: [PATCH v9 7/7] i2c: xiic: skip input clock setup on non-OF systems

[Andy Shevchenko]

On Tue, Feb 03, 2026 at 10:38:51AM -0800, Abdurrahman Hussain wrote:
> > On Feb 3, 2026, at 7:46 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Mon, Feb 02, 2026 at 08:37:24PM +0000, Abdurrahman Hussain via B4 Relay wrote:

> >> Currently Linux does not implement ACPI ClockInput to describe clock
> > 
> > ClockInput() resource
> > 
> >> resources, unlike DT. However the xiic driver is happy if something
> >> magically enables the clock before the driver probes, and does not
> >> turn it off again. The clock should always be considered optional for
> >> ACPI.
> 
> I just used the wording provided earlier by Andrew verbatim. But can change
> to the following if you insist:

I do not alter the wording, I correct the mention of the respective resource.
And yes, I insist.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 4/7] i2c: xiic: cosmetic cleanup

[Andi Shyti]

> >> Re-use dev pointer instead of referencing &pdev->dev everywhere.
> > 
> > ...
> > 
> >> - dev_dbg(&pdev->dev, "mmio %08lx irq %d scl clock frequency %d\n",
> >> + dev_dbg(dev, "mmio %08lx irq %d scl clock frequency %d\n",
> >> (unsigned long)res->start, irq, i2c->i2c_clk);
> > 
> > No need to do this here as we change the format string afterwards anyway.
> > 
> > But I leave it up to Andi, I won't prevent this to go in, if he agrees.
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> > 
> 
> No worries, I can do this in the following patch, since there is going to be
> a v 10 anyways.

This would be merged next week anyway, so, please, send a v10 as
there are many little changes (if we were out of time I would
have manually changed things before merging, but that's not the
case, yet).

Andi