[PATCH v1] ARM: dts: aspeed: g6: Use KCS AST2600 compatible string

[PATCH v1] ARM: dts: aspeed: g6: Use KCS AST2600 compatible string

[Haiyue Wang]

As the commit 2596f6b93a60 ("ipmi: kcs: aspeed: Add AST2600 compatible
string") does, change the AST2600 DTS file to use kcs ast2600 compatible
string.

Signed-off-by: Haiyue Wang <haiyuewa@163.com>
---
 arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
index 56bb3b0444f7..774d5af495fb 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
@@ -585,7 +585,7 @@ lpc: lpc@1e789000 {
 				ranges = <0x0 0x1e789000 0x1000>;
 
 				kcs1: kcs@24 {
-					compatible = "aspeed,ast2500-kcs-bmc-v2";
+					compatible = "aspeed,ast2600-kcs-bmc";
 					reg = <0x24 0x1>, <0x30 0x1>, <0x3c 0x1>;
 					interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
@@ -594,7 +594,7 @@ kcs1: kcs@24 {
 				};
 
 				kcs2: kcs@28 {
-					compatible = "aspeed,ast2500-kcs-bmc-v2";
+					compatible = "aspeed,ast2600-kcs-bmc";
 					reg = <0x28 0x1>, <0x34 0x1>, <0x40 0x1>;
 					interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
@@ -602,7 +602,7 @@ kcs2: kcs@28 {
 				};
 
 				kcs3: kcs@2c {
-					compatible = "aspeed,ast2500-kcs-bmc-v2";
+					compatible = "aspeed,ast2600-kcs-bmc";
 					reg = <0x2c 0x1>, <0x38 0x1>, <0x44 0x1>;
 					interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
@@ -610,7 +610,7 @@ kcs3: kcs@2c {
 				};
 
 				kcs4: kcs@114 {
-					compatible = "aspeed,ast2500-kcs-bmc-v2";
+					compatible = "aspeed,ast2600-kcs-bmc";
 					reg = <0x114 0x1>, <0x118 0x1>, <0x11c 0x1>;
 					interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
 					clocks = <&syscon ASPEED_CLK_GATE_LCLK>;
-- 
2.54.0

Re: [PATCH v1] ARM: dts: aspeed: g6: Use KCS AST2600 compatible string

[sashiko-bot]

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

Pre-existing issues:
- [High] The KCS BMC drivers register and enable their IRQ handler before initializing the device spinlock, leading to a potential crash or lockdep splat on an uninitialized spinlock.
--

commit a51ce0f5a1fd9344e679346b6911e87b35021057
Author: Haiyue Wang <haiyuewa@163.com>

ARM: dts: aspeed: g6: Use KCS AST2600 compatible string

This commit updates the device tree source for the AST2600 to use the
specific aspeed,ast2600-kcs-bmc compatible string instead of the older
ast2500 compatible string.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index 189bc3bbb47c9..9d09d6e0092a8 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> @@ -576,7 +576,7 @@ lpc: lpc@1e789000 {
>  				ranges = <0x0 0x1e789000 0x1000>;
>  
>  				kcs1: kcs@24 {
> -					compatible = "aspeed,ast2500-kcs-bmc-v2";
> +					compatible = "aspeed,ast2600-kcs-bmc";

[Severity: High]
This is a pre-existing issue, but modifying the compatible string draws
attention to an issue in the underlying driver's probe routine.

Does the driver initialization sequence have a race condition that could
lead to a crash on an uninitialized spinlock?

Looking at the driver for this compatible string:

drivers/char/ipmi/kcs_bmc_aspeed.c:aspeed_kcs_probe() {
    ...
    rc = aspeed_kcs_config_downstream_irq(kcs_bmc, pdev);
    if (rc)
        return rc;
    ...
    aspeed_kcs_enable_channel(kcs_bmc, true);

    rc = kcs_bmc_add_device(&priv->kcs_bmc);
    ...
}

The IRQ handler is registered in aspeed_kcs_config_downstream_irq() and
hardware interrupts are explicitly enabled via aspeed_kcs_enable_channel()
before kcs_bmc_add_device() is called. However, kcs_bmc_add_device() is
where spin_lock_init() initializes the device lock. 

If an interrupt fires (either from the hardware or on a shared IRQ line)
in the small window between enabling the channel and initializing the
spinlock, will kcs_bmc_handle_event() unconditionally attempt to acquire
the uninitialized spinlock?

As an aside, the same initialization order issue appears to be present in
the npcm7xx_kcs_probe() routine as well:

drivers/char/ipmi/kcs_bmc_npcm7xx.c:npcm7xx_kcs_probe() {
    ...
    rc = npcm7xx_kcs_config_irq(kcs_bmc, pdev);
    if (rc)
        return rc;

    npcm7xx_kcs_irq_mask_update(kcs_bmc, ...);
    npcm7xx_kcs_enable_channel(kcs_bmc, true);

    rc = kcs_bmc_add_device(kcs_bmc);
    ...
}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627052708.172918-1-haiyuewa@163.com?part=1