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

[PATCH v6 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.

Make the clock optional since the driver is designed to operate without
explicit configuration in firmware thus making it useful on platforms
where clock is not or cannot be provided.

Signed-off-by: Abdurrahman Hussain <abdurrahman@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):
      dt-bindings: i2c: xiic: make clocks optional
      i2c: xiic: make the clock optional
      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

 .../bindings/i2c/xlnx,xps-iic-2.00.a.yaml          |  1 -
 drivers/i2c/busses/i2c-xiic.c                      | 92 +++++++++-------------
 2 files changed, 38 insertions(+), 55 deletions(-)
---
base-commit: 63804fed149a6750ffd28610c5c1c98cce6bd377
change-id: 20260122-i2c-xiic-3ba89ff5ea93

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

[PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The xiic driver is designed to operate without explicit clock configuration
when clocks are not specified in the firmware.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml b/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
index 658ae92fa86d..cc500130f41f 100644
--- a/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
+++ b/Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
@@ -43,7 +43,6 @@ required:
   - compatible
   - reg
   - interrupts
-  - clocks
 
 unevaluatedProperties: false
 

-- 
2.52.0

[PATCH v6 2/7] i2c: xiic: make the clock optional

[Abdurrahman Hussain via B4 Relay]

From: Abdurrahman Hussain <abdurrahman@nexthop.ai>

The xiic driver is designed to operate without explicit clock configuration
when clocks are not specified in the firmware. This functionality is
already implemented in xiic_setclk(), which performs an early return when
either i2c_clk or input_clk are zero:

This condition is satisfied when clocks are missing, as clk_get_rate(NULL)
returns zero, allowing the driver to rely on hardware-configured timing.

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 28015d77599d..16202fb89271 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;
@@ -1464,7 +1465,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	mutex_init(&i2c->lock);
 	spin_lock_init(&i2c->atomic_lock);
 
-	i2c->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	i2c->clk = devm_clk_get_optional_enabled(dev, NULL);
 	if (IS_ERR(i2c->clk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
 				     "failed to enable input clock.\n");

-- 
2.52.0

[PATCH v6 3/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.

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

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 16202fb89271..973463a3a7f9 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1462,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_optional_enabled(dev, NULL);
@@ -1473,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);
@@ -1490,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 =
@@ -1509,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(&pdev->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) {
@@ -1530,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;
 }
 
@@ -1556,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 v6 4/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.

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 973463a3a7f9..95242d39f554 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 v6 5/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.

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 95242d39f554..73fc8f8cb5d4 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 v6 6/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.

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

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 73fc8f8cb5d4..9b763d8dbe84 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);
@@ -1464,12 +1464,13 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	i2c->clk = devm_clk_get_optional_enabled(dev, NULL);
 	if (IS_ERR(i2c->clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(i2c->clk),
+		return dev_err_probe(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;
 
@@ -1503,7 +1503,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 
 	ret = xiic_reinit(i2c);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Cannot xiic_reinit\n");
+		return dev_err_probe(dev, ret, "Cannot xiic_reinit\n");
 
 	/* add i2c adapter to i2c tree */
 	ret = i2c_add_adapter(&i2c->adap);
@@ -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);
-
-	if (ret < 0)
-		dev_warn(&pdev->dev, "Failed to activate device for removal (%pe)\n",
+	ret = pm_runtime_get_sync(dev);
+	if (ret)
+		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 v6 7/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.

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 9b763d8dbe84..75b43f469861 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

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

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 09:03:54PM +0000, Abdurrahman Hussain via B4 Relay wrote:
> Switch to generic device property accessors.
> 
> Switch to managed devm_ functions to simplify error handling.
> 
> Make the clock optional since the driver is designed to operate without
> explicit configuration in firmware thus making it useful on platforms
> where clock is not or cannot be provided.

Why the heck you ignored my tags?!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 3/7] i2c: xiic: switch to devres managed APIs

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 09:03:57PM +0000, Abdurrahman Hussain via B4 Relay wrote:

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

...

> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Cannot xiic_reinit\n");

I told you to use slightly different parameter. Please, read reviews carefully.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Krzysztof Kozlowski]

On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> The xiic driver is designed to operate without explicit clock configuration

And if you change this in the driver, then you change bindings?

You miss here explanation based on hardware - how does the hardware work
if nothing ticks it clocks?

Best regards,
Krzysztof

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Michal Simek]

On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
>> The xiic driver is designed to operate without explicit clock configuration
> 
> And if you change this in the driver, then you change bindings?
> 
> You miss here explanation based on hardware - how does the hardware work
> if nothing ticks it clocks?

Hardware obviously have clock input which needs to be connected. Without it it 
won't work.
And there are designs like Microblaze one where clock is shared with cpu itself 
and there is no way to adjust it. It is just working.

Then we have configurations where clock is coming from clock generator which 
needs to be enabled. That's case for SOCs with ARM cpus or with clock generators.

This series targets case around PCIe EP. Clock is likely derived from PCIe 
itself and there is no way how to change it, adjust it. It is just running on 
fixed value.

In DT this is design/described via fixed-clock but I expect this concept is not 
used on systems with ACPI.

Thanks,
Michal

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Andrew Lunn]

On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
> 
> 
> On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> > On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> > > The xiic driver is designed to operate without explicit clock configuration
> > 
> > And if you change this in the driver, then you change bindings?
> > 
> > You miss here explanation based on hardware - how does the hardware work
> > if nothing ticks it clocks?
> 
> Hardware obviously have clock input which needs to be connected. Without it
> it won't work.

Should ACPI potential limitations be making the DT description less
accurate?

Would it not be better that the driver has an DT binding and an ACPI
binding? Where there are common properties, common functions can be
used to retrieve them. However, if ACPI lacks usable clocks, use the
of_ method to get the clock from DT, and skip it for ACPI.

    Andrew

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Michal Simek]

On 1/28/26 15:34, Andrew Lunn wrote:
> On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
>>
>>
>> On 1/28/26 11:37, Krzysztof Kozlowski wrote:
>>> On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
>>>> The xiic driver is designed to operate without explicit clock configuration
>>>
>>> And if you change this in the driver, then you change bindings?
>>>
>>> You miss here explanation based on hardware - how does the hardware work
>>> if nothing ticks it clocks?
>>
>> Hardware obviously have clock input which needs to be connected. Without it
>> it won't work.
> 
> Should ACPI potential limitations be making the DT description less
> accurate?
> 
> Would it not be better that the driver has an DT binding and an ACPI
> binding? Where there are common properties, common functions can be
> used to retrieve them. However, if ACPI lacks usable clocks, use the
> of_ method to get the clock from DT, and skip it for ACPI.

I found is_of_node()/is_acpi_node() to detect if this driver is probed on 
OF/ACPI system. If this is the right way to go then clocks can stay required on 
DT system and not required on ACPI.

Thanks,
Michal

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Conor Dooley]

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

On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
> On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
> > 
> > 
> > On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> > > On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> > > > The xiic driver is designed to operate without explicit clock configuration
> > > 
> > > And if you change this in the driver, then you change bindings?
> > > 
> > > You miss here explanation based on hardware - how does the hardware work
> > > if nothing ticks it clocks?
> > 
> > Hardware obviously have clock input which needs to be connected. Without it
> > it won't work.
> 
> Should ACPI potential limitations be making the DT description less
> accurate?

We absolutely should not be cocking up bindings because ACPI requires
less complete descriptions.

> Would it not be better that the driver has an DT binding and an ACPI
> binding? Where there are common properties, common functions can be
> used to retrieve them. However, if ACPI lacks usable clocks, use the
> of_ method to get the clock from DT, and skip it for ACPI.
> 
>     Andrew
> 
> 

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

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
> On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
> > On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> > > On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> > > > The xiic driver is designed to operate without explicit clock configuration
> > > 
> > > And if you change this in the driver, then you change bindings?
> > > 
> > > You miss here explanation based on hardware - how does the hardware work
> > > if nothing ticks it clocks?
> > 
> > Hardware obviously have clock input which needs to be connected. Without it
> > it won't work.
> 
> Should ACPI potential limitations be making the DT description less
> accurate?
> 
> Would it not be better that the driver has an DT binding and an ACPI
> binding? Where there are common properties, common functions can be
> used to retrieve them. However, if ACPI lacks usable clocks, use the
> of_ method to get the clock from DT, and skip it for ACPI.

Why should we use of_ methods? If this is required we can check the type of
fwnode and act accordingly, but I think this should go deeper into some
treewide available helpers, because now some drivers repeat the mantra.

But how do the driver get the clock frequency (if needed for some register
settings and/or calculations)? DT seems to have well established property
'clock-frequency' for that. Can we consider it as "ACPI binding" as well?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Michal Simek]

On 1/28/26 15:45, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
>> On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
>>> On 1/28/26 11:37, Krzysztof Kozlowski wrote:
>>>> On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
>>>>> The xiic driver is designed to operate without explicit clock configuration
>>>>
>>>> And if you change this in the driver, then you change bindings?
>>>>
>>>> You miss here explanation based on hardware - how does the hardware work
>>>> if nothing ticks it clocks?
>>>
>>> Hardware obviously have clock input which needs to be connected. Without it
>>> it won't work.
>>
>> Should ACPI potential limitations be making the DT description less
>> accurate?
>>
>> Would it not be better that the driver has an DT binding and an ACPI
>> binding? Where there are common properties, common functions can be
>> used to retrieve them. However, if ACPI lacks usable clocks, use the
>> of_ method to get the clock from DT, and skip it for ACPI.
> 
> Why should we use of_ methods? If this is required we can check the type of
> fwnode and act accordingly, but I think this should go deeper into some
> treewide available helpers, because now some drivers repeat the mantra.
> 
> But how do the driver get the clock frequency (if needed for some register
> settings and/or calculations)? DT seems to have well established property
> 'clock-frequency' for that. Can we consider it as "ACPI binding" as well?

"clock-frequency" property in i2c is used for selecting i2c speed 100/400kHz.

Clock frequency in this driver is about describing clock coming to IP itself.
Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml

Thanks,
Michal

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 04:00:30PM +0100, Michal Simek wrote:
> On 1/28/26 15:45, Andy Shevchenko wrote:
> > On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
> > > On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
> > > > On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> > > > > On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> > > > > > The xiic driver is designed to operate without explicit clock configuration
> > > > > 
> > > > > And if you change this in the driver, then you change bindings?
> > > > > 
> > > > > You miss here explanation based on hardware - how does the hardware work
> > > > > if nothing ticks it clocks?
> > > > 
> > > > Hardware obviously have clock input which needs to be connected. Without it
> > > > it won't work.
> > > 
> > > Should ACPI potential limitations be making the DT description less
> > > accurate?
> > > 
> > > Would it not be better that the driver has an DT binding and an ACPI
> > > binding? Where there are common properties, common functions can be
> > > used to retrieve them. However, if ACPI lacks usable clocks, use the
> > > of_ method to get the clock from DT, and skip it for ACPI.
> > 
> > Why should we use of_ methods? If this is required we can check the type of
> > fwnode and act accordingly, but I think this should go deeper into some
> > treewide available helpers, because now some drivers repeat the mantra.
> > 
> > But how do the driver get the clock frequency (if needed for some register
> > settings and/or calculations)? DT seems to have well established property
> > 'clock-frequency' for that. Can we consider it as "ACPI binding" as well?
> 
> "clock-frequency" property in i2c is used for selecting i2c speed 100/400kHz.
> 
> Clock frequency in this driver is about describing clock coming to IP itself.
> Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml

Ah, I see, then is_of_node() probably is the compromise how to deal with
this setup.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Abdurrahman Hussain]

> On Jan 28, 2026, at 7:43 AM, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> On Wed, Jan 28, 2026 at 04:00:30PM +0100, Michal Simek wrote:
>> On 1/28/26 15:45, Andy Shevchenko wrote:
>>> On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
>>>> On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
>>>>> On 1/28/26 11:37, Krzysztof Kozlowski wrote:
>>>>>> On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
>>>>>>> The xiic driver is designed to operate without explicit clock configuration
>>>>>> 
>>>>>> And if you change this in the driver, then you change bindings?
>>>>>> 
>>>>>> You miss here explanation based on hardware - how does the hardware work
>>>>>> if nothing ticks it clocks?
>>>>> 
>>>>> Hardware obviously have clock input which needs to be connected. Without it
>>>>> it won't work.
>>>> 
>>>> Should ACPI potential limitations be making the DT description less
>>>> accurate?
>>>> 
>>>> Would it not be better that the driver has an DT binding and an ACPI
>>>> binding? Where there are common properties, common functions can be
>>>> used to retrieve them. However, if ACPI lacks usable clocks, use the
>>>> of_ method to get the clock from DT, and skip it for ACPI.
>>> 
>>> Why should we use of_ methods? If this is required we can check the type of
>>> fwnode and act accordingly, but I think this should go deeper into some
>>> treewide available helpers, because now some drivers repeat the mantra.
>>> 
>>> But how do the driver get the clock frequency (if needed for some register
>>> settings and/or calculations)? DT seems to have well established property
>>> 'clock-frequency' for that. Can we consider it as "ACPI binding" as well?
>> 
>> "clock-frequency" property in i2c is used for selecting i2c speed 100/400kHz.
>> 
>> Clock frequency in this driver is about describing clock coming to IP itself.
>> Documentation/devicetree/bindings/i2c/xlnx,xps-iic-2.00.a.yaml
> 
> Ah, I see, then is_of_node() probably is the compromise how to deal with
> this setup.
> 

Thanks for all the feedback!

So, if everyone agrees, I can make the changes to:

- Remove the DT bindings change.
- Make the driver fetch the clock only on OF by checking is_of_node().

Does this sound acceptable?

Best regards,
Abdurrahman

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Rob Herring]

On Wed, Jan 28, 2026 at 04:45:12PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
> > On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
> > > On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> > > > On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> > > > > The xiic driver is designed to operate without explicit clock configuration
> > > > 
> > > > And if you change this in the driver, then you change bindings?
> > > > 
> > > > You miss here explanation based on hardware - how does the hardware work
> > > > if nothing ticks it clocks?
> > > 
> > > Hardware obviously have clock input which needs to be connected. Without it
> > > it won't work.
> > 
> > Should ACPI potential limitations be making the DT description less
> > accurate?
> > 
> > Would it not be better that the driver has an DT binding and an ACPI
> > binding? Where there are common properties, common functions can be
> > used to retrieve them. However, if ACPI lacks usable clocks, use the
> > of_ method to get the clock from DT, and skip it for ACPI.
> 
> Why should we use of_ methods? If this is required we can check the type of
> fwnode and act accordingly, but I think this should go deeper into some
> treewide available helpers, because now some drivers repeat the mantra.
> 
> But how do the driver get the clock frequency (if needed for some register
> settings and/or calculations)? DT seems to have well established property
> 'clock-frequency' for that. Can we consider it as "ACPI binding" as well?

Well established and somewhat deprecated. Generally, 'clocks' should be 
used instead. There are some exceptions like I2C buses here to set the 
bus frequency (or max freq). We probably should have used 
'bus-frequency' in this case, but that predates me.

ACPI can use whatever the ACPI binding maintainers want. If you know who 
they are, please let me know. :)

Rob

Re: [PATCH v6 1/7] dt-bindings: i2c: xiic: make clocks optional

[Andy Shevchenko]

On Thu, Jan 29, 2026 at 10:14:04AM -0600, Rob Herring wrote:
> On Wed, Jan 28, 2026 at 04:45:12PM +0200, Andy Shevchenko wrote:
> > On Wed, Jan 28, 2026 at 03:34:02PM +0100, Andrew Lunn wrote:
> > > On Wed, Jan 28, 2026 at 12:21:41PM +0100, Michal Simek wrote:
> > > > On 1/28/26 11:37, Krzysztof Kozlowski wrote:
> > > > > On Tue, Jan 27, 2026 at 09:03:55PM +0000, Abdurrahman Hussain wrote:
> > > > > > The xiic driver is designed to operate without explicit clock configuration
> > > > > 
> > > > > And if you change this in the driver, then you change bindings?
> > > > > 
> > > > > You miss here explanation based on hardware - how does the hardware work
> > > > > if nothing ticks it clocks?
> > > > 
> > > > Hardware obviously have clock input which needs to be connected. Without it
> > > > it won't work.
> > > 
> > > Should ACPI potential limitations be making the DT description less
> > > accurate?
> > > 
> > > Would it not be better that the driver has an DT binding and an ACPI
> > > binding? Where there are common properties, common functions can be
> > > used to retrieve them. However, if ACPI lacks usable clocks, use the
> > > of_ method to get the clock from DT, and skip it for ACPI.
> > 
> > Why should we use of_ methods? If this is required we can check the type of
> > fwnode and act accordingly, but I think this should go deeper into some
> > treewide available helpers, because now some drivers repeat the mantra.
> > 
> > But how do the driver get the clock frequency (if needed for some register
> > settings and/or calculations)? DT seems to have well established property
> > 'clock-frequency' for that. Can we consider it as "ACPI binding" as well?
> 
> Well established and somewhat deprecated. Generally, 'clocks' should be 
> used instead. There are some exceptions like I2C buses here to set the 
> bus frequency (or max freq). We probably should have used 
> 'bus-frequency' in this case, but that predates me.
> 
> ACPI can use whatever the ACPI binding maintainers want. If you know who 
> they are, please let me know. :)

There is no such "entity" AFAIK, as on ACPI side we try pushing vendors to make
their efforts to bring a standard. Nevertheless, we have players, like MSFT, who
make decisions on their own (hence we have tons of "extensions" to the ACPI and
other specifications. Speaking of the standard, we have UEFI, MIPI and problably
others that have their own properties (but standard) and they usually use dedicated
prefixes (uefi-*, mipi-*). Also we have custom vendor specific ones, for example,
Intel in some cases (legit I suppose, as it's about Intel's IP) provides intc-*
properties. In many cases, though we have utilisation of the DT properties and
vendors more or less follow that. (Disclaimer: I described how I see it's done,
and not how I think it has to be done)

-- 
With Best Regards,
Andy Shevchenko