[PATCH v5 0/2] i2c: ls2x: Add clock- related properties and parsing

[PATCH v5 0/2] i2c: ls2x: Add clock- related properties and parsing

[Hongliang Wang]

Hi all:

This patch set adds clock related properties and parsing in dts and acpi.

======
V5:
Patch (1/2):
 - Adjust the position of #include <dt-bindings/clock/loongson,ls2k-clk.h>;
 - Add CC stable; 
 - Fix Signed-off-by.
Patch (2/2):
 - Replace 2K0500/2K1000/2K2000 with LS2K0500/2K1000/2K2000;
 - Replace 7A1000/7A2000 with LS7A1000/7A2000;
 - Replace if (clk && !IS_ERR(clk)) with if(!IS_ERR_OR_NULL(clk));
 - Add document that clocks and clock-div are only ACPI properties in ACPI;
 - Remove unsigned int cast in code (unsigned long)device_get_match_data(dev);
 - Add CC stable; 
 - Fix Signed-off-by.

Link to V4:
https://lore.kernel.org/all/20260526031021.32662-1-wanghongliang@loongson.cn/

V4:
- Add Acked-by tag from Conor Dooley, thanks.
Patch (2/2):
 - Adjust the position of #include <linux/clk.h>;
 - Remove struct ls2x_i2c_chip_data and use macro to describe div;
 - Use div instead of factor in ls2x_i2c_adjust_bus_speed;
 - Reverse the "if & else" code logic in ls2x_i2c_adjust_bus_speed;

Link to V2:
The PATCH v3 is incomplete, v4 is the replacement of v3, so the previous patch link is v2.

[PATCH v2] dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties
https://lore.kernel.org/all/20260507081010.12810-1-wanghongliang@loongson.cn/
[PATCH v2] i2c: ls2x: Add clocks property parsing and adjust bus speed
https://lore.kernel.org/all/20260507081010.12810-2-wanghongliang@loongson.cn/

V2:
[PATCH v2] dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties
- Remove the custom properties clock-input and clock-div, use clock framework;
[PATCH v2] i2c: ls2x: Add clocks property parsing and adjust bus speed
- Use clock framework to obtain the i2c reference clock in dts.

Link to V1:
https://lore.kernel.org/all/20260325011852.19079-1-wanghongliang@loongson.cn/

wanghongliang (2):
  dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties
  i2c: ls2x: Add clocks property parsing and adjust bus speed

 .../bindings/i2c/loongson,ls2x-i2c.yaml       |  3 ++
 drivers/i2c/busses/i2c-ls2x.c                 | 36 +++++++++++++++++--
 2 files changed, 36 insertions(+), 3 deletions(-)

-- 
2.47.2

[PATCH v5 1/2] dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties

[Hongliang Wang]

Add clocks and clock-frequency properties to examples.

Cc: stable@vger.kernel.org
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Hongliang Wang <wanghongliang@loongson.cn>
---
 Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
index ee09c6d9c5f0..0beb7f2515c8 100644
--- a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
@@ -37,11 +37,14 @@ unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/clock/loongson,ls2k-clk.h>
     #include <dt-bindings/interrupt-controller/irq.h>
 
     i2c0: i2c@1fe21000 {
         compatible = "loongson,ls2k-i2c";
         reg = <0x1fe21000 0x8>;
+        clock-frequency = <100000>;
+        clocks = <&clk LOONGSON2_APB_CLK>;
         interrupt-parent = <&extioiic>;
         interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
         #address-cells = <1>;
-- 
2.47.2

Re: [PATCH v5 1/2] dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `clocks` property is used in the binding example but is not declared in the `properties` block, causing `dt_binding_check` to fail.
--

commit 2429ce4ee39320cf5271a51cef0c60497b97ce80
Author: Hongliang Wang <wanghongliang@loongson.cn>

dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties

This commit adds clocks and clock-frequency properties to the
loongson,ls2x-i2c dt-binding examples.

Cc: stable@vger.kernel.org
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Hongliang Wang <wanghongliang@loongson.cn>

> diff --git a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
> index 67882ec6e06af..baa26684bbed0 100644
> --- a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
> @@ -33,11 +33,14 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/clock/loongson,ls2k-clk.h>
>      #include <dt-bindings/interrupt-controller/irq.h>
>  
>      i2c0: i2c@1fe21000 {
>          compatible = "loongson,ls2k-i2c";
>          reg = <0x1fe21000 0x8>;
> +        clock-frequency = <100000>;
> +        clocks = <&clk LOONGSON2_APB_CLK>;

[Severity: Low]
Will this cause dt_binding_check to fail? The schema specifies
unevaluatedProperties: false, and while clock-frequency is pulled in
from the referenced i2c-controller.yaml schema, clocks is not explicitly
defined in the properties block.

Should the clocks property be added to the properties block to prevent
validation errors on the example?

>          interrupt-parent = <&extioiic>;
>          interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
>          #address-cells = <1>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604015848.18643-1-wanghongliang@loongson.cn?part=1

[PATCH v5 2/2] i2c: ls2x: Add clocks property parsing and adjust bus speed

[Hongliang Wang]

The i2c-ls2x driver supports dts and acpi parameter passing.

In dts, uses clock framework, by parsing clocks property to
get i2c bus reference clock, and define the div of reference
clock by device data.

In acpi, by passing clocks property to describe i2c bus reference
clock and clock-div property to describe the div of reference clock.

Based on i2c bus reference clock(clock_a), i2c bus speed(clock_s)
and div, calculate the prcescale of i2c divider register. The
calculation formula is

prcescale = (clock_a*10)/(div*clock_s)-1

Cc: stable@vger.kernel.org
Signed-off-by: Hongliang Wang <wanghongliang@loongson.cn>
---
 drivers/i2c/busses/i2c-ls2x.c | 36 ++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
index b475dd27b7af..46dafa11b301 100644
--- a/drivers/i2c/busses/i2c-ls2x.c
+++ b/drivers/i2c/busses/i2c-ls2x.c
@@ -12,6 +12,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/iopoll.h>
@@ -63,11 +64,18 @@
 /* The default bus frequency, which is an empirical value */
 #define LS2X_I2C_FREQ_STD	(33 * HZ_PER_KHZ)
 
+/* The div of i2c reference clock on LS2K0500/2K1000/2K2000 */
+#define LS2X_I2C_2K_CLOCK_DIV	40
+
+/* The div of i2c reference clock on LS7A1000/7A2000 */
+#define LS2X_I2C_7A_CLOCK_DIV	50
+
 struct ls2x_i2c_priv {
 	struct i2c_adapter	adapter;
 	void __iomem		*base;
 	struct i2c_timings	i2c_t;
 	struct completion	cmd_complete;
+	unsigned int		div;
 };
 
 /*
@@ -96,6 +104,8 @@ static irqreturn_t ls2x_i2c_isr(int this_irq, void *dev_id)
 static void ls2x_i2c_adjust_bus_speed(struct ls2x_i2c_priv *priv)
 {
 	u16 val;
+	u32 pclk, div;
+	struct clk *clk;
 	struct i2c_timings *t = &priv->i2c_t;
 	struct device *dev = priv->adapter.dev.parent;
 	u32 acpi_speed = i2c_acpi_find_bus_speed(dev);
@@ -107,12 +117,30 @@ static void ls2x_i2c_adjust_bus_speed(struct ls2x_i2c_priv *priv)
 	else
 		t->bus_freq_hz = LS2X_I2C_FREQ_STD;
 
+	if (dev_of_node(dev)) {
+		clk = devm_clk_get_optional_enabled(dev, NULL);
+		if (!IS_ERR_OR_NULL(clk))
+			pclk = clk_get_rate(clk);
+		else
+			pclk = LS2X_I2C_PCLK_FREQ;
+
+		div = priv->div;
+
+		val = (pclk * 10) / (div * t->bus_freq_hz) - 1;
+	} else {
+		/* clocks and clock-div are only ACPI properties. */
+		if (!device_property_read_u32(dev, "clocks", &pclk) &&
+		    !device_property_read_u32(dev, "clock-div", &div))
+			val = (pclk * 10) / (div * t->bus_freq_hz) - 1;
+		else
+			val = LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1;
+	}
+
 	/*
 	 * According to the chip manual, we can only access the registers as bytes,
 	 * otherwise the high bits will be truncated.
 	 * So set the I2C frequency with a sequential writeb() instead of writew().
 	 */
-	val = LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1;
 	writeb(FIELD_GET(GENMASK(7, 0), val), priv->base + I2C_LS2X_PRER_LO);
 	writeb(FIELD_GET(GENMASK(15, 8), val), priv->base + I2C_LS2X_PRER_HI);
 }
@@ -295,6 +323,8 @@ static int ls2x_i2c_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->div = (unsigned long)device_get_match_data(dev);
+
 	/* Map hardware registers */
 	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base))
@@ -349,8 +379,8 @@ static DEFINE_RUNTIME_DEV_PM_OPS(ls2x_i2c_pm_ops,
 				 ls2x_i2c_suspend, ls2x_i2c_resume, NULL);
 
 static const struct of_device_id ls2x_i2c_id_table[] = {
-	{ .compatible = "loongson,ls2k-i2c" },
-	{ .compatible = "loongson,ls7a-i2c" },
+	{ .compatible = "loongson,ls2k-i2c", .data = (void *)LS2X_I2C_2K_CLOCK_DIV, },
+	{ .compatible = "loongson,ls7a-i2c", .data = (void *)LS2X_I2C_7A_CLOCK_DIV, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ls2x_i2c_id_table);
-- 
2.47.2

Re: [PATCH v5 2/2] i2c: ls2x: Add clocks property parsing and adjust bus speed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Calling `devm_clk_get_optional_enabled()` dynamically inside `ls2x_i2c_adjust_bus_speed()` causes unbounded devres memory leaks, clock reference/enable leaks, and introduces an AB-BA deadlock during I2C transfers.
- [High] The prescale calculation divides by `div` without verifying it is non-zero, creating a division-by-zero vulnerability.
- [High] The driver silently ignores clock acquisition errors and fails to propagate `-EPROBE_DEFER`, breaking probe deferral.
--

commit feb6cc9d7c19ebc975910d8ee72838e62efa3d45
Author: Hongliang Wang <wanghongliang@loongson.cn>

i2c: ls2x: Add clocks property parsing and adjust bus speed

The i2c-ls2x driver supports dts and acpi parameter passing.
In dts, uses clock framework, by parsing clocks property to
get i2c bus reference clock, and define the div of reference
clock by device data.

> diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
> index b475dd27b7af9..46dafa11b3013 100644
> --- a/drivers/i2c/busses/i2c-ls2x.c
> +++ b/drivers/i2c/busses/i2c-ls2x.c
[ ... ]
> @@ -107,12 +117,30 @@ static void ls2x_i2c_adjust_bus_speed(struct ls2x_i2c_priv *priv)
>  	else
>  		t->bus_freq_hz = LS2X_I2C_FREQ_STD;
>  
> +	if (dev_of_node(dev)) {
> +		clk = devm_clk_get_optional_enabled(dev, NULL);

[Severity: High]
Is it safe to call devm_clk_get_optional_enabled() here?

ls2x_i2c_adjust_bus_speed() is called by ls2x_i2c_init(), which runs during
probe(), system resume(), and error recovery paths in ls2x_i2c_xfer_one().

Calling a devm_ function repeatedly at runtime will cause unbounded devres
memory leaks and clock reference/enable leaks.

Additionally, clk_prepare_enable() acquires the global clock framework's
prepare_lock. Since ls2x_i2c_xfer_one() executes while the I2C core holds
adap->bus_lock, this creates a bus_lock -> prepare_lock dependency. If an
I2C-based clock provider exists on the system, it will try to acquire locks in
the reverse order, which could result in an AB-BA deadlock.

> +		if (!IS_ERR_OR_NULL(clk))

[Severity: High]
Does masking the error here break probe deferral?

If devm_clk_get_optional_enabled() returns -EPROBE_DEFER because the clock
provider hasn't initialized yet, the error is swallowed instead of being
propagated to the probe() function. This could result in the driver binding
successfully without its necessary clock being enabled.

> +			pclk = clk_get_rate(clk);
> +		else
> +			pclk = LS2X_I2C_PCLK_FREQ;
> +
> +		div = priv->div;
> +
> +		val = (pclk * 10) / (div * t->bus_freq_hz) - 1;

[Severity: High]
Can div be zero here, causing a divide-by-zero panic?

div is populated via device_get_match_data(). If the device is dynamically
bound via sysfs or matches a fallback compatible without data,
device_get_match_data() returns NULL, making div 0.

> +	} else {
> +		/* clocks and clock-div are only ACPI properties. */
> +		if (!device_property_read_u32(dev, "clocks", &pclk) &&
> +		    !device_property_read_u32(dev, "clock-div", &div))
> +			val = (pclk * 10) / (div * t->bus_freq_hz) - 1;

[Severity: High]
Can clock-div be zero in the ACPI properties, also causing a divide-by-zero
panic?

If the firmware provides clock-div = 0, this will result in an unhandled
division-by-zero exception.

> +		else
> +			val = LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1;
> +	}
> +
>  	/*
>  	 * According to the chip manual, we can only access the registers as bytes,
>  	 * otherwise the high bits will be truncated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604015848.18643-1-wanghongliang@loongson.cn?part=2