[PATCH v5 0/2] HiSilicon I2C bus recovery support

[PATCH v5 0/2] HiSilicon I2C bus recovery support

[Bowen Yu]

Changes since v4:
  - Patch 1 (new): Add a static inline stub for acpi_has_method() in
    <linux/acpi.h> for !CONFIG_ACPI builds. This is the prerequisite
    for dropping the #ifdef in the driver: acpi_has_method() is the
    only ACPI symbol referenced by the recovery code that lacks an
    automatic stub (the ACPICA core helpers already get stubs via
    ACPI_EXTERNAL_RETURN_*).
  - Patch 2: Drop the #ifdef CONFIG_ACPI guard around the recovery
    helpers in i2c-hisi.c. The six recovery functions are now compiled
    unconditionally.

Patch 2 depends on Patch 1 for !CONFIG_ACPI compilation.

Bowen Yu (2):
  ACPI: Add stub for acpi_has_method() for !CONFIG_ACPI
  i2c: hisi: Add I2C bus recovery support

 drivers/i2c/busses/i2c-hisi.c | 142 ++++++++++++++++++++++++++++++++++
 include/linux/acpi.h          |   5 ++
 2 files changed, 147 insertions(+)

-- 
2.33.0

[PATCH v5 1/2] ACPI: Add stub for acpi_has_method() for !CONFIG_ACPI

[Bowen Yu]

acpi_has_method() is declared in <acpi/acpi_bus.h>, which is only
included by <linux/acpi.h> when CONFIG_ACPI is enabled. Unlike the
ACPICA core helpers (acpi_evaluate_object(), acpi_format_exception(),
...) that get automatic static-inline stubs via the
ACPI_EXTERNAL_RETURN_* macros when !CONFIG_ACPI, acpi_has_method() is
Linux glue and has no stub in the #else section of <linux/acpi.h>.

This forces drivers referencing acpi_has_method() to wrap the call
site in #ifdef CONFIG_ACPI to avoid -Werror=implicit-function-declaration
on !CONFIG_ACPI builds -- a valid config for drivers that
"depends on ARM64 || COMPILE_TEST" with no ACPI dependency.

Add a static inline stub returning false, consistent with the existing
stubs for acpi_check_dsm() and friends, so drivers can drop the #ifdef
and rely on the runtime acpi_disabled check instead.

Signed-off-by: Bowen Yu <yubowen8@huawei.com>
---
 include/linux/acpi.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ddacac812094..141a322435c2 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1028,6 +1028,11 @@ static inline bool acpi_driver_match_device(struct device *dev,
 	return false;
 }
 
+static inline bool acpi_has_method(acpi_handle handle, char *name)
+{
+	return false;
+}
+
 static inline bool acpi_check_dsm(acpi_handle handle, const guid_t *guid,
 				  u64 rev, u64 funcs)
 {
-- 
2.33.0

Re: [PATCH v5 1/2] ACPI: Add stub for acpi_has_method() for !CONFIG_ACPI

[Rafael J. Wysocki (Intel)]

On Tue, Sep 29, 2026 at 2:13 PM Bowen Yu <yubowen8@huawei.com> wrote:
>
> acpi_has_method() is declared in <acpi/acpi_bus.h>, which is only
> included by <linux/acpi.h> when CONFIG_ACPI is enabled. Unlike the
> ACPICA core helpers (acpi_evaluate_object(), acpi_format_exception(),
> ...) that get automatic static-inline stubs via the
> ACPI_EXTERNAL_RETURN_* macros when !CONFIG_ACPI, acpi_has_method() is
> Linux glue and has no stub in the #else section of <linux/acpi.h>.
>
> This forces drivers referencing acpi_has_method() to wrap the call
> site in #ifdef CONFIG_ACPI to avoid -Werror=implicit-function-declaration
> on !CONFIG_ACPI builds -- a valid config for drivers that
> "depends on ARM64 || COMPILE_TEST" with no ACPI dependency.
>
> Add a static inline stub returning false, consistent with the existing
> stubs for acpi_check_dsm() and friends, so drivers can drop the #ifdef
> and rely on the runtime acpi_disabled check instead.
>
> Signed-off-by: Bowen Yu <yubowen8@huawei.com>
> ---
>  include/linux/acpi.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index ddacac812094..141a322435c2 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1028,6 +1028,11 @@ static inline bool acpi_driver_match_device(struct device *dev,
>         return false;
>  }
>
> +static inline bool acpi_has_method(acpi_handle handle, char *name)
> +{
> +       return false;
> +}
> +
>  static inline bool acpi_check_dsm(acpi_handle handle, const guid_t *guid,
>                                   u64 rev, u64 funcs)
>  {
> --

Please see

https://lore.kernel.org/linux-acpi/3717753.iIbC2pHGDl@rafael.j.wysocki/

in linux-next now.

Thanks!

[PATCH v5 2/2] i2c: hisi: Add I2C bus recovery support

[Bowen Yu]

When the I2C bus is stuck due to a slave device holding SDA low
(e.g. during an incomplete transfer), the master has no way to recover
the bus through normal operation. Add bus recovery support using the
subctrl register to manually toggle SCL and generate clock pulses to
release the bus.

The recovery is performed via a second register resource (sctrl_base)
that provides direct control over SCL/SDA lines through mux and
output-enable bits. After recovery, the I2C controller is reset through
the ACPI _RST method and reconfigured.

Recovery is only registered when the subctrl resource is provided in
the firmware description, keeping backward compatibility with existing
platforms.

Signed-off-by: Bowen Yu <yubowen8@huawei.com>
---
 drivers/i2c/busses/i2c-hisi.c | 142 ++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
index ba5c9579ae19..5b4e92d23032 100644
--- a/drivers/i2c/busses/i2c-hisi.c
+++ b/drivers/i2c/busses/i2c-hisi.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2021 HiSilicon Technologies Co., Ltd.
  */
 
+#include <linux/acpi.h>
 #include <linux/bits.h>
 #include <linux/bitfield.h>
 #include <linux/clk.h>
@@ -62,6 +63,8 @@
 #define HISI_I2C_INT_CLR		0x0048
 #define HISI_I2C_INT_MASK		0x004C
 #define HISI_I2C_TRANS_STATE		0x0050
+#define   HISI_I2C_TRANS_STATE_SDA_LEVEL	BIT(5)
+#define   HISI_I2C_TRANS_STATE_SCL_LEVEL	BIT(6)
 #define HISI_I2C_TRANS_ERR		0x0054
 #define HISI_I2C_VERSION		0x0058
 
@@ -86,9 +89,26 @@
 #define NSEC_TO_CYCLES(ns, clk_rate_khz) \
 	DIV_ROUND_UP_ULL((clk_rate_khz) * (ns), NSEC_PER_MSEC)
 
+/*
+ * SUBCTRL SC_I2C_CTRL register
+ * Set HISI_I2C_CTRL_DAT_CFG_EN and HISI_I2C_CTRL_SCL_CFG_EN to control
+ * I2C pin behavior by subctrl controller; use HISI_I2C_CTRL_DAT_OE and
+ * HISI_I2C_CTRL_CLK_OE to control input or output; use HISI_I2C_CTRL_SDA_OUT
+ * and HISI_I2C_CTRL_SCL_OUT to control output value.
+ */
+#define HISI_I2C_CTRL_DAT_CFG_EN BIT(5)
+#define HISI_I2C_CTRL_SCL_CFG_EN BIT(4)
+#define HISI_I2C_CTRL_DAT_OE BIT(3)
+#define HISI_I2C_CTRL_CLK_OE BIT(2)
+#define HISI_I2C_CTRL_SDA_OUT BIT(1)
+#define HISI_I2C_CTRL_SCL_OUT BIT(0)
+
+#define HISI_I2C_RECOVERY_REG_SIZE 4
+
 struct hisi_i2c_controller {
 	struct i2c_adapter adapter;
 	void __iomem *iobase;
+	void __iomem *sctrl_addr;
 	struct device *dev;
 	struct clk *clk;
 	int irq;
@@ -108,8 +128,14 @@ struct hisi_i2c_controller {
 	struct i2c_timings t;
 	u32 clk_rate_khz;
 	u32 spk_len;
+
+	/* Bus recovery */
+	struct i2c_bus_recovery_info rinfo;
+	acpi_handle acpi_handle;
 };
 
+static void hisi_i2c_configure_bus(struct hisi_i2c_controller *ctlr);
+
 static void hisi_i2c_enable_int(struct hisi_i2c_controller *ctlr, u32 mask)
 {
 	writel_relaxed(mask, ctlr->iobase + HISI_I2C_INT_MASK);
@@ -151,6 +177,118 @@ static void hisi_i2c_handle_errors(struct hisi_i2c_controller *ctlr)
 	}
 }
 
+static int hisi_i2c_recovery_get_scl(struct i2c_adapter *adap)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+	u32 reg = readl(ctlr->iobase + HISI_I2C_TRANS_STATE);
+
+	return !!(reg & HISI_I2C_TRANS_STATE_SCL_LEVEL);
+}
+
+static int hisi_i2c_recovery_get_sda(struct i2c_adapter *adap)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+	u32 reg = readl(ctlr->iobase + HISI_I2C_TRANS_STATE);
+
+	return !!(reg & HISI_I2C_TRANS_STATE_SDA_LEVEL);
+}
+
+static void hisi_i2c_recovery_set_scl(struct i2c_adapter *adap, int val)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = readl(ctlr->sctrl_addr);
+	if (val)
+		reg |= HISI_I2C_CTRL_SCL_OUT;
+	else
+		reg &= ~HISI_I2C_CTRL_SCL_OUT;
+	writel(reg, ctlr->sctrl_addr);
+}
+
+static void hisi_i2c_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = readl(ctlr->sctrl_addr);
+	reg |= HISI_I2C_CTRL_SCL_CFG_EN | HISI_I2C_CTRL_DAT_CFG_EN |
+		   HISI_I2C_CTRL_CLK_OE | HISI_I2C_CTRL_SCL_OUT;
+	reg &= ~HISI_I2C_CTRL_DAT_OE;
+	writel(reg, ctlr->sctrl_addr);
+}
+
+static void hisi_i2c_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = readl(ctlr->sctrl_addr);
+	reg &= ~(HISI_I2C_CTRL_SCL_CFG_EN | HISI_I2C_CTRL_DAT_CFG_EN);
+	writel(reg, ctlr->sctrl_addr);
+
+	/*
+	 * Invokes the specific ACPI method "_RST" to trigger a soft reset
+	 * of the I2C controller to help the I2C controller recover from
+	 * the abnormal state after the bus recovery process.
+	 */
+	if (ctlr->acpi_handle && acpi_has_method(ctlr->acpi_handle, "_RST")) {
+		acpi_status status;
+
+		status = acpi_evaluate_object(ctlr->acpi_handle, "_RST", NULL, NULL);
+		if (ACPI_FAILURE(status))
+			dev_err(ctlr->dev, "_RST method failed: %s\n",
+				acpi_format_exception(status));
+	}
+	hisi_i2c_configure_bus(ctlr);
+}
+
+static int hisi_i2c_get_bus_recovery_res(struct hisi_i2c_controller *ctlr,
+					 struct platform_device *pdev)
+{
+	struct resource *res0;
+
+	res0 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+
+	if (!res0 || resource_size(res0) != HISI_I2C_RECOVERY_REG_SIZE)
+		return -ENODEV;
+
+	ctlr->sctrl_addr = devm_ioremap_resource(&pdev->dev, res0);
+	if (IS_ERR(ctlr->sctrl_addr)) {
+		ctlr->sctrl_addr = NULL;
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int hisi_i2c_recovery_init(struct hisi_i2c_controller *ctlr)
+{
+	struct platform_device *pdev = to_platform_device(ctlr->dev);
+	struct i2c_adapter *adapter = &ctlr->adapter;
+	int ret;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	ret = hisi_i2c_get_bus_recovery_res(ctlr, pdev);
+	if (ret)
+		return ret;
+
+	ctlr->rinfo = (struct i2c_bus_recovery_info) {
+		.get_scl = hisi_i2c_recovery_get_scl,
+		.get_sda = hisi_i2c_recovery_get_sda,
+		.set_scl = hisi_i2c_recovery_set_scl,
+		.prepare_recovery = hisi_i2c_prepare_recovery,
+		.unprepare_recovery = hisi_i2c_unprepare_recovery,
+		.recover_bus = i2c_generic_scl_recovery,
+	};
+	adapter->bus_recovery_info = &ctlr->rinfo;
+	ctlr->acpi_handle = ACPI_HANDLE(ctlr->dev);
+
+	return 0;
+}
+
 static int hisi_i2c_start_xfer(struct hisi_i2c_controller *ctlr)
 {
 	struct i2c_msg *msg = ctlr->msgs;
@@ -496,6 +634,10 @@ static int hisi_i2c_probe(struct platform_device *pdev)
 
 	hisi_i2c_configure_bus(ctlr);
 
+	ret = hisi_i2c_recovery_init(ctlr);
+	if (ret)
+		dev_info(ctlr->dev, "I2C bus recovery not available\n");
+
 	adapter = &ctlr->adapter;
 	snprintf(adapter->name, sizeof(adapter->name),
 		 "HiSilicon I2C Controller %s", dev_name(dev));
-- 
2.33.0

Re: [PATCH v5 2/2] i2c: hisi: Add I2C bus recovery support

[Jie Zhan]

On 9/29/2026 8:13 PM, Bowen Yu wrote:
> When the I2C bus is stuck due to a slave device holding SDA low
> (e.g. during an incomplete transfer), the master has no way to recover
> the bus through normal operation. Add bus recovery support using the
> subctrl register to manually toggle SCL and generate clock pulses to
> release the bus.
> 
> The recovery is performed via a second register resource (sctrl_base)
> that provides direct control over SCL/SDA lines through mux and
> output-enable bits. After recovery, the I2C controller is reset through
> the ACPI _RST method and reconfigured.
> 
> Recovery is only registered when the subctrl resource is provided in
> the firmware description, keeping backward compatibility with existing
> platforms.
> 
> Signed-off-by: Bowen Yu <yubowen8@huawei.com>
> ---
>  drivers/i2c/busses/i2c-hisi.c | 142 ++++++++++++++++++++++++++++++++++
>  1 file changed, 142 insertions(+)
This version looks good to me now.  Thanks!

Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com> 

Re: [PATCH v5 2/2] i2c: hisi: Add I2C bus recovery support

[Andi Shyti]

Hi Bowen,

the patch looks good and it can be taken without 1/2.

Just few nitpicks,

...

> +static int hisi_i2c_get_bus_recovery_res(struct hisi_i2c_controller *ctlr,
> +					 struct platform_device *pdev)
> +{
> +	struct resource *res0;

/res0/res/

> +
> +	res0 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +
> +	if (!res0 || resource_size(res0) != HISI_I2C_RECOVERY_REG_SIZE)
> +		return -ENODEV;
> +
> +	ctlr->sctrl_addr = devm_ioremap_resource(&pdev->dev, res0);
> +	if (IS_ERR(ctlr->sctrl_addr)) {
> +		ctlr->sctrl_addr = NULL;
> +		return -ENOMEM;

return PTR_ERR(ctlr->sctrl_addr);

> +	}
> +
> +	return 0;
> +}

...

>  static int hisi_i2c_start_xfer(struct hisi_i2c_controller *ctlr)
>  {
>  	struct i2c_msg *msg = ctlr->msgs;
> @@ -496,6 +634,10 @@ static int hisi_i2c_probe(struct platform_device *pdev)
>  
>  	hisi_i2c_configure_bus(ctlr);
>  
> +	ret = hisi_i2c_recovery_init(ctlr);
> +	if (ret)
> +		dev_info(ctlr->dev, "I2C bus recovery not available\n");

I would use here dev_dbg(...);

If it's OK with you, I can change the ones above and apply,
otherwise, please send a v6.

Andi

> +
>  	adapter = &ctlr->adapter;
>  	snprintf(adapter->name, sizeof(adapter->name),
>  		 "HiSilicon I2C Controller %s", dev_name(dev));
> -- 
> 2.33.0
> 

Re: [PATCH v5 2/2] i2c: hisi: Add I2C bus recovery support

[yubowen (H)]

在 2026/10/27 18:51, Andi Shyti 写道:
> Hi Bowen,
>
> the patch looks good and it can be taken without 1/2.
>
> Just few nitpicks,
>
> ...
>
>> +static int hisi_i2c_get_bus_recovery_res(struct hisi_i2c_controller *ctlr,
>> +					 struct platform_device *pdev)
>> +{
>> +	struct resource *res0;
> /res0/res/
>
>> +
>> +	res0 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +
>> +	if (!res0 || resource_size(res0) != HISI_I2C_RECOVERY_REG_SIZE)
>> +		return -ENODEV;
>> +
>> +	ctlr->sctrl_addr = devm_ioremap_resource(&pdev->dev, res0);
>> +	if (IS_ERR(ctlr->sctrl_addr)) {
>> +		ctlr->sctrl_addr = NULL;
>> +		return -ENOMEM;
> return PTR_ERR(ctlr->sctrl_addr);
>
>> +	}
>> +
>> +	return 0;
>> +}
> ...
>
>>   static int hisi_i2c_start_xfer(struct hisi_i2c_controller *ctlr)
>>   {
>>   	struct i2c_msg *msg = ctlr->msgs;
>> @@ -496,6 +634,10 @@ static int hisi_i2c_probe(struct platform_device *pdev)
>>   
>>   	hisi_i2c_configure_bus(ctlr);
>>   
>> +	ret = hisi_i2c_recovery_init(ctlr);
>> +	if (ret)
>> +		dev_info(ctlr->dev, "I2C bus recovery not available\n");
> I would use here dev_dbg(...);
>
> If it's OK with you, I can change the ones above and apply,
> otherwise, please send a v6.
>
> Andi
>
>> +
>>   	adapter = &ctlr->adapter;
>>   	snprintf(adapter->name, sizeof(adapter->name),
>>   		 "HiSilicon I2C Controller %s", dev_name(dev));
>> -- 
>> 2.33.0
>>
Thank you for the feedback, I be back with V6.

Best Regards,
Bowen