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

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

======
V6:
- Add Reviewed-by tag from Huacai, thanks. 
Patch (1/2):
 - Remove CC stable. 

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

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 v6 1/2] dt-bindings: i2c: ls2x-i2c: Add clocks and clock-frequency properties

[Hongliang Wang]

Add clocks and clock-frequency properties to examples.

Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
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

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

Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
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 v6 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] Adding the `clocks` property to the device tree example without defining it in the `properties` block causes schema validation (`make dt_binding_check`) to fail.
--

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

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

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

> diff --git a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
> --- a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
[ ... ]
>  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 addition to the examples block cause make dt_binding_check to fail?

The loongson,ls2x-i2c.yaml schema sets unevaluatedProperties: false. This
strictly limits allowed properties to those explicitly defined in the schema
itself or inherited via references. While clock-frequency is inherited from
the standard i2c-controller.yaml reference, the clocks property does not
appear to be defined anywhere in the local properties block.

Because clocks is undeclared, adding it here will likely cause the schema
validation tooling to reject the example and fail the build.

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

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Calling `devm_clk_get_optional_enabled()` dynamically causes unbounded memory leaks and unbalanced clock enable counts.
- [High] Calculation of the I2C prescale value lacks bounds checking, leading to a kernel panic if `div` evaluates to 0.
- [Medium] The driver improperly repurposes the standard `clocks` property to read a raw 32-bit integer frequency in ACPI setups.
- [Medium] The strict check for `dev_of_node(dev)` breaks compatibility with ACPI PRP0001 matching, causing valid match data to be discarded.
- [Low] The numerator calculation `pclk * 10` can overflow a 32-bit integer, resulting in a miscalculated I2C bus speed.
--

commit 3bc9e29253442765caa0c735b574051e32efaa27
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. It adds
clock framework parsing for device tree configurations, and reads
clock and clock-div properties directly for ACPI setups.

> 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)) {

[Severity: Medium]
Does this check break compatibility with ACPI PRP0001 configurations?

During probe(), the driver correctly reads priv->div from the compatible
string using device_get_match_data(), which supports both native Device Tree
and ACPI PRP0001. However, since dev_of_node() evaluates to NULL for ACPI
nodes, PRP0001 setups will be incorrectly routed to the native ACPI fallback
block.

This will discard the valid matched priv->div and attempt to read non-existent
native ACPI properties. Could this check for the presence of priv->div
instead?

> +		clk = devm_clk_get_optional_enabled(dev, NULL);

[Severity: Critical]
Does calling devm_clk_get_optional_enabled() here cause a devres memory leak
and unbalanced clock enable counts?

ls2x_i2c_adjust_bus_speed() is called by ls2x_i2c_init(), which is called not
only during probe, but also during normal operation and error recovery:

ls2x_i2c_resume() -> ls2x_i2c_init()
ls2x_i2c_xfer_one() -> ls2x_i2c_init()

Since devm_clk_get_optional_enabled() dynamically allocates a managed resource
and increments the clock reference and enable counts, calling it repeatedly
without releasing previous allocations will eventually exhaust system memory
and overflow the clock reference tracking.

Can the clock be acquired exactly once in probe() instead?

> +		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;

[Severity: Low]
Can the numerator calculation pclk * 10 overflow a 32-bit integer?

Since pclk is a 32-bit unsigned integer, if the reference clock exceeds
approximately 429.49 MHz, the multiplication will wrap around and yield an
artificially small numerator.

While typical reference clocks might be lower, would it be safer to use 64-bit
precision like div_u64((u64)pclk * 10, ...) here?


[Severity: High]
Does this division lack bounds checking for a zero divisor?

If div evaluates to 0, this will result in a division by zero kernel panic.

This regression can happen if the device is manually bound via sysfs
driver_override without an ID match, causing device_get_match_data() to
return NULL. It can also happen in the ACPI branch below if a misconfigured
ACPI table sets clock-div to 0.

Should div be explicitly validated as greater than 0 before performing the
division?

> +	} else {
> +		/* clocks and clock-div are only ACPI properties. */
> +		if (!device_property_read_u32(dev, "clocks", &pclk) &&

[Severity: Medium]
Does this reuse the standard clocks property incorrectly for ACPI setups?

The clocks property is strictly defined in Device Tree and unified ACPI _DSD
environments as an array of phandles to clock providers. Reusing this exact
name to read a raw 32-bit integer frequency creates a binding conflict and
could break generic property parsers.

Could a standard, non-conflicting property name like clock-frequency or
ref-clock-frequency be used instead?

> +		    !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;
> +	}
> +

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