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

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

[Hongliang Wang]

From: wanghongliang <wanghongliang@loongson.cn>

Hi all:

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

I'm sorry that the PATCH v3 I previously submitted was incomplete, and
missing changelog and previous patch link. So I resubmit the PATCH v4
instead of v3 for review.

Thanks.

======
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                 | 35 +++++++++++++++++--
 2 files changed, 35 insertions(+), 3 deletions(-)

-- 
2.47.2

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

[Hongliang Wang]

From: wanghongliang <wanghongliang@loongson.cn>

clocks property describes the i2c bus reference clock from APB clock.
clock-frequency property describes i2c bus speed.

Signed-off-by: wanghongliang <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..4bf89bb97e7d 100644
--- a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
@@ -38,10 +38,13 @@ unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/loongson,ls2k-clk.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

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

[Hongliang Wang]

From: wanghongliang <wanghongliang@loongson.cn>

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

Signed-off-by: wanghongliang <wanghongliang@loongson.cn>
---
 drivers/i2c/busses/i2c-ls2x.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
index b475dd27b7af..6d332e59452d 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 2K0500/2K1000/2K2000 */
+#define LS2X_I2C_2K_CLOCK_DIV	40
+
+/* The div of i2c reference clock on 7A1000/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,29 @@ 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 (clk && !IS_ERR(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 {
+		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 +322,8 @@ static int ls2x_i2c_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->div = (unsigned int)(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 +378,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 v4 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 added to the example but is not defined in the `properties` block, which will cause `dt_binding_check` to fail.
--

commit dfdc495c318fa2d081d4ec5526c34bbf035f948f
Author: wanghongliang <wanghongliang@loongson.cn>
dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties

This commit updates the loongson,ls2k-i2c device tree binding example to 
include the clocks and clock-frequency properties. It also adds the 
required clock header file to the example.

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

[Severity: Low]
Will this cause a schema validation error during make dt_binding_check?

Because unevaluatedProperties is set to false, and the clocks property is
not declared in the local properties block of this binding, including it
in the examples section typically triggers a validation failure. Should
clocks be added to the properties block as well?

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

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] Calling `devm_clk_get_optional_enabled()` dynamically inside the hardware initialization function causes unbounded devres memory leaks, unbalanced clock enable counts, and potential memory reclaim deadlocks.
- [High] The driver silently ignores `-EPROBE_DEFER` from `devm_clk_get_optional_enabled()`, failing to defer probe and instead initializing with an incorrect fallback frequency.
- [High] Unvalidated `div` values from device match data or ACPI properties can cause a division by zero kernel panic during prescaler calculation.
- [Medium] Repurposing the standard "clocks" property as a `u32` integer for ACPI violates standard device property bindings.
- [Medium] The I2C clock prescaler calculation uses 32-bit arithmetic, which can overflow if the reference clock is too large.

Pre-existing issues:
- [High] Hardware interrupts are enabled before the IRQ handler is registered, risking unhandled interrupt storms.
--

commit 09494456e295e0b7fa53296c66ae490bc54b296b
Author: wanghongliang <wanghongliang@loongson.cn>

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

This commit adds clock property parsing for the i2c-ls2x driver to
adjust the bus speed based on the reference clock and divider values
obtained from device tree or ACPI.

> diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
> index b475dd27b7af9..6d332e59452d2 100644
> --- a/drivers/i2c/busses/i2c-ls2x.c
> +++ b/drivers/i2c/busses/i2c-ls2x.c
[ ... ]
> @@ -107,12 +117,29 @@ 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]
Does calling devm_clk_get_optional_enabled() during hardware initialization
risk memory leaks and unbalanced clock counts?

ls2x_i2c_adjust_bus_speed() is called from ls2x_i2c_init(), which executes
during probe, resume, and when an I2C transfer times out in
ls2x_i2c_xfer_one().

Calling a devres allocation function on every timeout or resume cycle
allocates new memory and increments the clock enable count each time without
a matching disable, leading to unbounded leaks. Additionally, performing
devres allocations in the transfer path can cause memory reclaim deadlocks.

> +		if (clk && !IS_ERR(clk))
> +			pclk = clk_get_rate(clk);
> +		else
> +			pclk = LS2X_I2C_PCLK_FREQ;

[Severity: High]
If devm_clk_get_optional_enabled() returns -EPROBE_DEFER, will this code
fail to defer the probe?

By catching the error here and silently falling back to LS2X_I2C_PCLK_FREQ,
the hardware will initialize assuming a 50 MHz clock instead of waiting for
the clock provider to be ready. This could leave the clock unenabled by the
framework and lead to incorrect bus speeds.

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

[Severity: High]
Can 'div' be zero here, leading to a division by zero panic?

If the device is bound manually via sysfs without a matching compatible
string, device_get_match_data() will return NULL, leaving priv->div as 0.

[Severity: Medium]
Will the calculation (pclk * 10) overflow 32-bit arithmetic if the reference
clock rate exceeds approximately 429.5 MHz?

Casting to 64-bit for the multiplication would prevent wraparound.

> +	} else {
> +		if (!device_property_read_u32(dev, "clocks", &pclk) &&

[Severity: Medium]
Does reading "clocks" as a raw 32-bit integer violate standard device
bindings?

In generic device properties and Device Tree, the "clocks" property is
strictly standardized to contain an array of references (phandles) to clock
providers. Overloading this standardized property to store an integer can
cause generic parsing code to fail. Should a standard property like
"clock-frequency" be used instead?

> +		    !device_property_read_u32(dev, "clock-div", &div))
> +			val = (pclk * 10) / (div * t->bus_freq_hz) - 1;

[Severity: High]
Could a zero value in the "clock-div" ACPI property cause a division by zero
panic here?

> +		else
> +			val = LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1;
> +	}
> +
[ ... ]
> @@ -295,6 +322,8 @@ static int ls2x_i2c_probe(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	priv->div = (unsigned int)(unsigned long)device_get_match_data(dev);
> +

[Severity: High]
This is a pre-existing issue, but does initializing the hardware before
requesting the shared IRQ risk an unhandled interrupt storm?

ls2x_i2c_probe() currently performs:

	ls2x_i2c_init(priv);

	ret = devm_request_irq(dev, irq, ls2x_i2c_isr, IRQF_SHARED, ...);

ls2x_i2c_init() writes CTR_READY_MASK (which includes LS2X_CTR_IEN, bit 6)
to the hardware control register, enabling the device to assert interrupts.

If the device asserts an interrupt immediately on the shared IRQ line, it
will fire before devm_request_irq() completes. Other drivers sharing the IRQ
will return IRQ_NONE, leading to an unhandled interrupt storm that could
cause the kernel to permanently disable the shared IRQ line.

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