[PATCH v1 0/3] LoongArch: dts: i2c: Add clocks and clock-frequency properties

[PATCH v1 0/3] LoongArch: dts: i2c: Add clocks and clock-frequency properties

[Hongliang Wang]

Hi all:

This patch set adds clocks and clock-frequency properties to i2c nodes of
LS2K0500/2K1000/2K2000 dts.

Hongliang Wang (3):
  LoongArch: dts: i2c: Add clocks and clock-frequency properties to
    2K0500
  LoongArch: dts: i2c: Add clocks and clock-frequency properties to
    2K1000
  LoongArch: dts: i2c: Add clocks and clock-frequency properties to
    2K2000

 arch/loongarch/boot/dts/loongson-2k0500.dtsi | 12 ++++++++++++
 arch/loongarch/boot/dts/loongson-2k1000.dtsi |  4 ++++
 arch/loongarch/boot/dts/loongson-2k2000.dtsi |  4 ++++
 3 files changed, 20 insertions(+)

-- 
2.47.2

[PATCH v1 1/3] LoongArch: dts: i2c: Add clocks and clock-frequency properties to 2K0500

[Hongliang Wang]

Add clocks and clock-frequency properties to i2c nodes.

Signed-off-by: Hongliang Wang <wanghongliang@loongson.cn>
---
 arch/loongarch/boot/dts/loongson-2k0500.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/loongarch/boot/dts/loongson-2k0500.dtsi b/arch/loongarch/boot/dts/loongson-2k0500.dtsi
index 1b502064df11..992b1d0de9ec 100644
--- a/arch/loongarch/boot/dts/loongson-2k0500.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k0500.dtsi
@@ -405,6 +405,8 @@ uart0: serial@1ff40800 {
 		i2c0: i2c@1ff48000 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1ff48000 0x0 0x0800>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&eiointc>;
 			interrupts = <14>;
 			status = "disabled";
@@ -413,6 +415,8 @@ i2c0: i2c@1ff48000 {
 		i2c@1ff48800 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1ff48800 0x0 0x0800>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&eiointc>;
 			interrupts = <15>;
 			status = "disabled";
@@ -421,6 +425,8 @@ i2c@1ff48800 {
 		i2c@1ff49000 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1ff49000 0x0 0x0800>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&eiointc>;
 			interrupts = <16>;
 			status = "disabled";
@@ -429,6 +435,8 @@ i2c@1ff49000 {
 		i2c@1ff49800 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1ff49800 0x0 0x0800>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&eiointc>;
 			interrupts = <17>;
 			status = "disabled";
@@ -437,6 +445,8 @@ i2c@1ff49800 {
 		i2c@1ff4a000 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1ff4a000 0x0 0x0800>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&eiointc>;
 			interrupts = <18>;
 			status = "disabled";
@@ -445,6 +455,8 @@ i2c@1ff4a000 {
 		i2c@1ff4a800 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1ff4a800 0x0 0x0800>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&eiointc>;
 			interrupts = <19>;
 			status = "disabled";
-- 
2.47.2

[PATCH v1 2/3] LoongArch: dts: i2c: Add clocks and clock-frequency properties to 2K1000

[Hongliang Wang]

Add clocks and clock-frequency properties to i2c nodes.

Signed-off-by: Hongliang Wang <wanghongliang@loongson.cn>
---
 arch/loongarch/boot/dts/loongson-2k1000.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/loongarch/boot/dts/loongson-2k1000.dtsi b/arch/loongarch/boot/dts/loongson-2k1000.dtsi
index ab6a55937e9e..e6f0fe093ab3 100644
--- a/arch/loongarch/boot/dts/loongson-2k1000.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k1000.dtsi
@@ -311,6 +311,8 @@ uart0: serial@1fe20000 {
 		i2c2: i2c@1fe21000 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1fe21000 0x0 0x8>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&liointc0>;
 			interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
@@ -319,6 +321,8 @@ i2c2: i2c@1fe21000 {
 		i2c3: i2c@1fe21800 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1fe21800 0x0 0x8>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
 			interrupt-parent = <&liointc0>;
 			interrupts = <23 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
-- 
2.47.2

[PATCH v1 3/3] LoongArch: dts: i2c: Add clocks and clock-frequency properties to 2K2000

[Hongliang Wang]

Add clocks and clock-frequency properties to i2c nodes.

Signed-off-by: Hongliang Wang <wanghongliang@loongson.cn>
---
 arch/loongarch/boot/dts/loongson-2k2000.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/loongarch/boot/dts/loongson-2k2000.dtsi b/arch/loongarch/boot/dts/loongson-2k2000.dtsi
index 3678c084adf7..7fdc4192d2f2 100644
--- a/arch/loongarch/boot/dts/loongson-2k2000.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k2000.dtsi
@@ -239,6 +239,8 @@ rtc0: rtc@100d0100 {
 		i2c@1fe00120 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1fe00120 0x0 0x8>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_MISC_CLK>;
 			interrupt-parent = <&liointc>;
 			interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
@@ -247,6 +249,8 @@ i2c@1fe00120 {
 		i2c@1fe00130 {
 			compatible = "loongson,ls2k-i2c";
 			reg = <0x0 0x1fe00130 0x0 0x8>;
+			clock-frequency = <100000>;
+			clocks = <&clk LOONGSON2_MISC_CLK>;
 			interrupt-parent = <&liointc>;
 			interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
-- 
2.47.2

Re: [PATCH v1 2/3] LoongArch: dts: i2c: Add clocks and clock-frequency properties to 2K1000

[sashiko-bot]

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

Pre-existing issues:
- [High] The `ls2x-i2c` driver ignores the `clocks` property from the device tree, uses a hardcoded 50 MHz input frequency, and fails to enable the APB clock.
- [High] The driver enables hardware interrupts during probe but fails to disable them if probe fails or when the module is unloaded, risking a shared interrupt storm.
--

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

LoongArch: dts: i2c: Add clocks and clock-frequency properties to 2K1000

This commit adds the clock-frequency and clocks properties to the i2c
nodes for the Loongson 2K1000 device tree.

> diff --git a/arch/loongarch/boot/dts/loongson-2k1000.dtsi b/arch/loongarch/boot/dts/loongson-2k1000.dtsi
> index ab6a55937e9eb..e6f0fe093ab3c 100644
> --- a/arch/loongarch/boot/dts/loongson-2k1000.dtsi
> +++ b/arch/loongarch/boot/dts/loongson-2k1000.dtsi
> @@ -311,6 +311,8 @@ uart0: serial@1fe20000 {
>  		i2c2: i2c@1fe21000 {
>  			compatible = "loongson,ls2k-i2c";
>  			reg = <0x0 0x1fe21000 0x0 0x8>;
> +			clock-frequency = <100000>;
> +			clocks = <&clk LOONGSON2_APB_CLK>;

[Severity: High]
This is a pre-existing issue, but does the ls2x-i2c driver actually use
these clock properties?

Looking at drivers/i2c/busses/i2c-ls2x.c, the driver appears to hardcode
the input clock frequency to 50 MHz:

drivers/i2c/busses/i2c-ls2x.c:
#define LS2X_I2C_PCLK_FREQ  (50 * HZ_PER_MHZ)

It uses this hardcoded value in ls2x_i2c_adjust_bus_speed() to calculate
the prescaler. Because the APB clock can scale, wouldn't ignoring the
clock frequency from the DT lead to out-of-spec I2C bus speeds?

Additionally, the driver never seems to call clk_prepare_enable().
If no other drivers request the shared APB clock, could the clock
framework disable it via clk_disable_unused, hanging the system on
register access?

>  			interrupt-parent = <&liointc0>;
>  			interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;

[Severity: High]
This is a pre-existing issue, but does the ls2x-i2c driver correctly
manage these interrupts during probe failures or module removal?

In drivers/i2c/busses/i2c-ls2x.c:ls2x_i2c_probe(), hardware interrupts
are enabled via ls2x_i2c_init() before the fallible devm_request_irq()
and devm_i2c_add_adapter() operations:

drivers/i2c/busses/i2c-ls2x.c:ls2x_i2c_probe() {
    ...
    ls2x_i2c_init(priv);

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

If probe fails later, or if the driver is unbound (there is no .remove
callback in ls2x_i2c_driver), the devm framework will free the IRQ handler
but leave the hardware interrupts enabled at the controller level.

Because the interrupt is requested with IRQF_SHARED, won't any subsequent
interrupt asserted by the I2C controller go unhandled and cause an
interrupt storm?

>  			status = "disabled";
>  		};

[ ... ]

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