[PATCH 0/2] Add I2C bus recovery support on Kunpeng 920/930

[PATCH 0/2] Add I2C bus recovery support on Kunpeng 920/930

[Yicong Yang]

This series adds the I2C bus recovery support on Kunpeng 920/930.
We're using HiSilicon I2C controller on Kunpeng 930 and Designware
I2C controller on Kunpeng 920. For both platform, the SCL/SDA pins
are multiplexed with GPIOs. Both driver use generic GPIO recovery
method and we need to switch the pin mutiplexing before/after the
recovery process. We use ACPI method to help on the pin mux switching
which is also introduced in this series.

Yicong Yang (2):
  i2c: hisi: Add generic GPIO bus recovery support
  i2c: designware: Add ACPI assisted recovery support

 drivers/i2c/busses/i2c-designware-core.h    |  2 +
 drivers/i2c/busses/i2c-designware-master.c  | 84 ++++++++++++++++++++-
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 +-
 drivers/i2c/busses/i2c-hisi.c               | 78 +++++++++++++++++++
 4 files changed, 164 insertions(+), 6 deletions(-)

-- 
2.24.0

[PATCH 1/2] i2c: hisi: Add generic GPIO bus recovery support

[Yicong Yang]

Add generic GPIO bus recovery support for i2c-hisi driver
by registering the recovery information with core provided
i2c_generic_scl_recovery() method.

As the SCL/SDA pins are multiplexed with GPIO, we need to
switch the pins mux to GPIO before recovery and switch back
after recovery. It's implemented by the ACPI method in
the i2c_bus_recovery_info->{prepare,unprepare}_recovery()
method.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/i2c/busses/i2c-hisi.c | 78 +++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/i2c/busses/i2c-hisi.c b/drivers/i2c/busses/i2c-hisi.c
index acf394812061..ffc495f426e9 100644
--- a/drivers/i2c/busses/i2c-hisi.c
+++ b/drivers/i2c/busses/i2c-hisi.c
@@ -5,9 +5,11 @@
  * Copyright (c) 2021 HiSilicon Technologies Co., Ltd.
  */
 
+#include <linux/acpi.h>
 #include <linux/bits.h>
 #include <linux/bitfield.h>
 #include <linux/completion.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -106,6 +108,9 @@ struct hisi_i2c_controller {
 	struct i2c_timings t;
 	u32 clk_rate_khz;
 	u32 spk_len;
+
+	/* Bus recovery method */
+	struct i2c_bus_recovery_info rinfo;
 };
 
 static void hisi_i2c_enable_int(struct hisi_i2c_controller *ctlr, u32 mask)
@@ -424,6 +429,77 @@ static void hisi_i2c_configure_bus(struct hisi_i2c_controller *ctlr)
 	writel(reg, ctlr->iobase + HISI_I2C_FIFO_CTRL);
 }
 
+#ifdef CONFIG_ACPI
+#define HISI_I2C_PIN_MUX_METHOD	"PMUX"
+
+/**
+ * i2c_dw_acpi_pin_mux_change - Change the I2C controller's pin mux through ACPI
+ * @dev: device owns the SCL/SDA pin
+ * @to_gpio: true to switch to GPIO, false to switch to SCL/SDA
+ *
+ * The function invokes the specific ACPI method "PMUX" for changing the
+ * pin mux of I2C controller between SCL/SDA and GPIO in order to help on
+ * the generic GPIO recovery process.
+ */
+static void i2c_hisi_pin_mux_change(struct device *dev, bool to_gpio)
+{
+	acpi_handle handle = ACPI_HANDLE(dev);
+	struct acpi_object_list arg_list;
+	unsigned long long data;
+	union acpi_object arg;
+
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = to_gpio;
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+
+	acpi_evaluate_integer(handle, HISI_I2C_PIN_MUX_METHOD, &arg_list, &data);
+}
+
+static void i2c_hisi_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+
+	i2c_hisi_pin_mux_change(ctlr->dev, true);
+}
+
+static void i2c_hisi_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct hisi_i2c_controller *ctlr = i2c_get_adapdata(adap);
+
+	i2c_hisi_pin_mux_change(ctlr->dev, false);
+}
+
+static void hisi_i2c_init_recovery_info(struct hisi_i2c_controller *ctlr)
+{
+	struct i2c_bus_recovery_info *rinfo = &ctlr->rinfo;
+	struct acpi_device *adev = ACPI_COMPANION(ctlr->dev);
+	struct gpio_desc *gpio;
+
+	if (!acpi_has_method(adev->handle, HISI_I2C_PIN_MUX_METHOD))
+		return;
+
+	gpio = devm_gpiod_get_optional(ctlr->dev, "scl", GPIOD_OUT_HIGH);
+	if (IS_ERR_OR_NULL(gpio))
+		return;
+
+	rinfo->scl_gpiod = gpio;
+
+	gpio = devm_gpiod_get_optional(ctlr->dev, "sda", GPIOD_IN);
+	if (IS_ERR(gpio))
+		return;
+
+	rinfo->sda_gpiod = gpio;
+	rinfo->recover_bus = i2c_generic_scl_recovery;
+	rinfo->prepare_recovery =  i2c_hisi_prepare_recovery;
+	rinfo->unprepare_recovery = i2c_hisi_unprepare_recovery;
+
+	ctlr->adapter.bus_recovery_info = rinfo;
+}
+#else
+static inline void hisi_i2c_init_recovery_info(struct hisi_i2c_controller *ctlr) { }
+#endif /* CONFIG_ACPI */
+
 static int hisi_i2c_probe(struct platform_device *pdev)
 {
 	struct hisi_i2c_controller *ctlr;
@@ -473,6 +549,8 @@ static int hisi_i2c_probe(struct platform_device *pdev)
 	adapter->dev.parent = dev;
 	i2c_set_adapdata(adapter, ctlr);
 
+	hisi_i2c_init_recovery_info(ctlr);
+
 	ret = devm_i2c_add_adapter(dev, adapter);
 	if (ret)
 		return ret;
-- 
2.24.0

[PATCH 2/2] i2c: designware: Add ACPI assisted recovery support

[Yicong Yang]

On HiSilicon Kunpeng 920 the SCL/SDA pins are multiplexed with
GPIO. On bus recovery the driver needs to swich the pin mux
to GPIO to recover and switch back on finished. Currently the
pin mux switch is generally implemented by a pinctrl driver
which is unavailable on our platform.

This patch introduces an ACPI method to switch the pin mux of
SCL/SDA during the recovery process. Then the recovery support
can be enabled with the assistance of the ACPI.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/i2c/busses/i2c-designware-core.h    |  2 +
 drivers/i2c/busses/i2c-designware-master.c  | 84 ++++++++++++++++++++-
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 +-
 3 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 4b26cba40139..bbf4bb9c9d0f 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -297,6 +297,8 @@ struct dw_i2c_dev {
 
 #define ACCESS_INTR_MASK	BIT(0)
 #define ACCESS_NO_IRQ_SUSPEND	BIT(1)
+/* ACPI assists SCL/SDA pin mutiplexing on recovery */
+#define RECOVERY_WITH_ACPI	BIT(2)
 
 #define MODEL_MSCC_OCELOT	BIT(8)
 #define MODEL_BAIKAL_BT1	BIT(9)
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 9177463c2cbb..89fdbc48b4a9 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -818,11 +818,81 @@ static void i2c_dw_unprepare_recovery(struct i2c_adapter *adap)
 	i2c_dw_init_master(dev);
 }
 
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+/*
+ * i2c_dw_acpi_pin_mux_change - Change the I2C controller's pin mux through ACPI
+ * @dev: device owns the SCL/SDA pin
+ * @to_gpio: true to switch to GPIO, false to switch to SCL/SDA
+ *
+ * The function invokes the specific ACPI method "PMUX" for changing the
+ * pin mux of I2C controller between SCL/SDA and GPIO. It's not defined by
+ * the ACPI spec but used on some platforms like HiSilicon's Kunpeng 920
+ * server, help on the generic GPIO recovery process.
+ */
+static void i2c_dw_acpi_pin_mux_change(struct device *dev, bool to_gpio)
+{
+	acpi_handle handle = ACPI_HANDLE(dev);
+	struct acpi_object_list arg_list;
+	unsigned long long data;
+	union acpi_object arg;
+
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = to_gpio;
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+
+	acpi_evaluate_integer(handle, "PMUX", &arg_list, &data);
+}
+
+static void i2c_dw_acpi_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+
+	i2c_dw_prepare_recovery(adap);
+	i2c_dw_acpi_pin_mux_change(dev->dev, true);
+}
+
+static void i2c_dw_acpi_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+
+	i2c_dw_acpi_pin_mux_change(dev->dev, false);
+	i2c_dw_unprepare_recovery(adap);
+}
+
+static int i2c_dw_acpi_init_recovery_info(struct dw_i2c_dev *dev)
+{
+	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
+	struct acpi_device *adev;
+
+	if (acpi_disabled)
+		return -EOPNOTSUPP;
+
+	adev = ACPI_COMPANION(dev->dev);
+	if (!adev || !acpi_has_method(adev->handle, "PMUX"))
+		return -EOPNOTSUPP;
+
+	rinfo->recover_bus = i2c_generic_scl_recovery;
+	rinfo->prepare_recovery = i2c_dw_acpi_prepare_recovery;
+	rinfo->unprepare_recovery = i2c_dw_acpi_unprepare_recovery;
+
+	return 0;
+}
+#else
+static int i2c_dw_acpi_init_recovery_info(struct dw_i2c_dev *dev)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
 {
 	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
 	struct i2c_adapter *adap = &dev->adapter;
 	struct gpio_desc *gpio;
+	int ret = 0;
 
 	gpio = devm_gpiod_get_optional(dev->dev, "scl", GPIOD_OUT_HIGH);
 	if (IS_ERR_OR_NULL(gpio))
@@ -835,9 +905,17 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
 		return PTR_ERR(gpio);
 	rinfo->sda_gpiod = gpio;
 
-	rinfo->recover_bus = i2c_generic_scl_recovery;
-	rinfo->prepare_recovery = i2c_dw_prepare_recovery;
-	rinfo->unprepare_recovery = i2c_dw_unprepare_recovery;
+	if (dev->flags & RECOVERY_WITH_ACPI) {
+		ret = i2c_dw_acpi_init_recovery_info(dev);
+	} else {
+		rinfo->recover_bus = i2c_generic_scl_recovery;
+		rinfo->prepare_recovery = i2c_dw_prepare_recovery;
+		rinfo->unprepare_recovery = i2c_dw_unprepare_recovery;
+	}
+
+	if (ret)
+		return ret;
+
 	adap->bus_recovery_info = rinfo;
 
 	dev_info(dev->dev, "running with gpio recovery mode! scl%s",
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 2bd81abc86f6..a7070867a07f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -52,9 +52,9 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "AMDI0010", ACCESS_INTR_MASK },
 	{ "AMDI0510", 0 },
 	{ "APMC0D0F", 0 },
-	{ "HISI02A1", 0 },
-	{ "HISI02A2", 0 },
-	{ "HISI02A3", 0 },
+	{ "HISI02A1", RECOVERY_WITH_ACPI },
+	{ "HISI02A2", RECOVERY_WITH_ACPI },
+	{ "HISI02A3", RECOVERY_WITH_ACPI },
 	{ "HYGO0010", ACCESS_INTR_MASK },
 	{ }
 };
-- 
2.24.0

Re: [PATCH 0/2] Add I2C bus recovery support on Kunpeng 920/930

[Yicong Yang]

a friendly ping ...

Hi Designware maintainers and Wolfram, any comments for this?

thanks.

On 2022/1/25 20:49, Yicong Yang wrote:
> This series adds the I2C bus recovery support on Kunpeng 920/930.
> We're using HiSilicon I2C controller on Kunpeng 930 and Designware
> I2C controller on Kunpeng 920. For both platform, the SCL/SDA pins
> are multiplexed with GPIOs. Both driver use generic GPIO recovery
> method and we need to switch the pin mutiplexing before/after the
> recovery process. We use ACPI method to help on the pin mux switching
> which is also introduced in this series.
> 
> Yicong Yang (2):
>   i2c: hisi: Add generic GPIO bus recovery support
>   i2c: designware: Add ACPI assisted recovery support
> 
>  drivers/i2c/busses/i2c-designware-core.h    |  2 +
>  drivers/i2c/busses/i2c-designware-master.c  | 84 ++++++++++++++++++++-
>  drivers/i2c/busses/i2c-designware-platdrv.c |  6 +-
>  drivers/i2c/busses/i2c-hisi.c               | 78 +++++++++++++++++++
>  4 files changed, 164 insertions(+), 6 deletions(-)
> 

Re: [PATCH 1/2] i2c: hisi: Add generic GPIO bus recovery support

[Andy Shevchenko]

On Tue, Jan 25, 2022 at 08:49:29PM +0800, Yicong Yang wrote:
> Add generic GPIO bus recovery support for i2c-hisi driver
> by registering the recovery information with core provided
> i2c_generic_scl_recovery() method.
> 
> As the SCL/SDA pins are multiplexed with GPIO, we need to
> switch the pins mux to GPIO before recovery and switch back
> after recovery. It's implemented by the ACPI method in
> the i2c_bus_recovery_info->{prepare,unprepare}_recovery()
> method.

NAK.

ACPI has its own resources for that. What is missed is the layer between ACPI
and pin control.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] i2c: hisi: Add generic GPIO bus recovery support

[Andy Shevchenko]

On Mon, Feb 07, 2022 at 01:17:49PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 25, 2022 at 08:49:29PM +0800, Yicong Yang wrote:
> > Add generic GPIO bus recovery support for i2c-hisi driver
> > by registering the recovery information with core provided
> > i2c_generic_scl_recovery() method.
> > 
> > As the SCL/SDA pins are multiplexed with GPIO, we need to
> > switch the pins mux to GPIO before recovery and switch back
> > after recovery. It's implemented by the ACPI method in
> > the i2c_bus_recovery_info->{prepare,unprepare}_recovery()
> > method.
> 
> NAK.
> 
> ACPI has its own resources for that. What is missed is the layer between ACPI
> and pin control.

To be more precise,

https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pinconfig-pin-configuration-descriptor-macro

https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pinfunction-pin-function-descriptor-macro

https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pingroup-pin-group-descriptor-macro

https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pingroupconfig-pin-group-configuration-descriptor-macro

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] Add I2C bus recovery support on Kunpeng 920/930

[Andy Shevchenko]

On Mon, Feb 07, 2022 at 03:37:17PM +0800, Yicong Yang wrote:
> a friendly ping ...
> 
> Hi Designware maintainers and Wolfram, any comments for this?

Answered, sorry for the delay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] i2c: hisi: Add generic GPIO bus recovery support

[Yicong Yang]

[ +cc pinctrl list for visible ]

Hi Andy,

Thanks for the reply.

On 2022/2/7 19:20, Andy Shevchenko wrote:
> On Mon, Feb 07, 2022 at 01:17:49PM +0200, Andy Shevchenko wrote:
>> On Tue, Jan 25, 2022 at 08:49:29PM +0800, Yicong Yang wrote:
>>> Add generic GPIO bus recovery support for i2c-hisi driver
>>> by registering the recovery information with core provided
>>> i2c_generic_scl_recovery() method.
>>>
>>> As the SCL/SDA pins are multiplexed with GPIO, we need to
>>> switch the pins mux to GPIO before recovery and switch back
>>> after recovery. It's implemented by the ACPI method in
>>> the i2c_bus_recovery_info->{prepare,unprepare}_recovery()
>>> method.
>>
>> NAK.
>>
>> ACPI has its own resources for that. What is missed is the layer between ACPI
>> and pin control.
> 

I think that's where the problem is and why we use this approach. When I looked into
devm_pinctrl_get(), it stops when fails to retrieve the pin info from device tree.
So I cannot use it on our ACPI server for the pinmux.

I looked into the history that Irina raised an RFC for adding the ACPI support in pinctrl[1],
but at that time seems it lacks some standard support. But maybe now we can support it?

But based on the current situation maybe the implementation of this patch suits best.
And currently we don't have a pinctrl driver for doing the multiplexing (though I think it's
easy to add one but maybe only configures 2 pins for now).

[1] https://lore.kernel.org/all/CACRpkdbj==q5wp2Z5-ZXkbfeXa4y+beLF_3YN-vS3CtyAKGwkg@mail.gmail.com/

Thanks,
Yicong

> To be more precise,
> 
> https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pinconfig-pin-configuration-descriptor-macro
> 
> https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pinfunction-pin-function-descriptor-macro
> 
> https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pingroup-pin-group-descriptor-macro
> 
> https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pingroupconfig-pin-group-configuration-descriptor-macro
> 

Re: [PATCH 1/2] i2c: hisi: Add generic GPIO bus recovery support

[Andy Shevchenko]

On Tue, Feb 08, 2022 at 10:15:44AM +0800, Yicong Yang wrote:
> On 2022/2/7 19:20, Andy Shevchenko wrote:
> > On Mon, Feb 07, 2022 at 01:17:49PM +0200, Andy Shevchenko wrote:
> >> On Tue, Jan 25, 2022 at 08:49:29PM +0800, Yicong Yang wrote:
> >>> Add generic GPIO bus recovery support for i2c-hisi driver
> >>> by registering the recovery information with core provided
> >>> i2c_generic_scl_recovery() method.
> >>>
> >>> As the SCL/SDA pins are multiplexed with GPIO, we need to
> >>> switch the pins mux to GPIO before recovery and switch back
> >>> after recovery. It's implemented by the ACPI method in
> >>> the i2c_bus_recovery_info->{prepare,unprepare}_recovery()
> >>> method.
> >>
> >> NAK.
> >>
> >> ACPI has its own resources for that. What is missed is the layer between ACPI
> >> and pin control.
> 
> I think that's where the problem is and why we use this approach. When I looked into
> devm_pinctrl_get(), it stops when fails to retrieve the pin info from device tree.
> So I cannot use it on our ACPI server for the pinmux.
> 
> I looked into the history that Irina raised an RFC for adding the ACPI support in pinctrl[1],
> but at that time seems it lacks some standard support. But maybe now we can support it?

Not that way. ACPI has its own resources as I pointed out. Irina tried to
mimic DT, which has been refused by maintainers. Since that ACPI gained
the native resources.

> But based on the current situation maybe the implementation of this patch suits best.
> And currently we don't have a pinctrl driver for doing the multiplexing (though I think it's
> easy to add one but maybe only configures 2 pins for now).

This patch (i2c hisi) is a hack on all levels, starting with ACPI DSDT.
Please, fix tables to use PinConfig() / PinFunction() (or corresponding
*Group() variants) in the DSDT for the starter.

> [1] https://lore.kernel.org/all/CACRpkdbj==q5wp2Z5-ZXkbfeXa4y+beLF_3YN-vS3CtyAKGwkg@mail.gmail.com/
> 
> Thanks,
> Yicong
> 
> > To be more precise,
> > 
> > https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pinconfig-pin-configuration-descriptor-macro
> > 
> > https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pinfunction-pin-function-descriptor-macro
> > 
> > https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pingroup-pin-group-descriptor-macro
> > 
> > https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html?highlight=pinfunction#pingroupconfig-pin-group-configuration-descriptor-macro

-- 
With Best Regards,
Andy Shevchenko