[PATCH 0/5] i2c: ls2x: Add support for the Loongson-2K/LS7A I2C

[PATCH 0/5] i2c: ls2x: Add support for the Loongson-2K/LS7A I2C

[Binbin Zhou]

Hi all:

This patch series adds support for the I2C module found on various
Loongson systems with the Loongson-2K SoC or the Loongson LS7A bridge chip.

For now, the I2C driver is suitable for DT-based or ACPI-based systems.

BTW:
I have only tested on the Loongson-3A5000+LS7A1000/LS7A2000 under LoongArch
architecture.

Thanks.

Binbin Zhou (5):
  i2c: core: Pick i2c bus number from ACPI if present
  i2c: gpio: Add support on ACPI-based system
  dt-bindings: i2c: add bindings for Loongson LS2X I2C
  i2c: Add driver for Loongson-2K/LS7A I2C controller
  LoongArch: Enable LS2X I2C in loongson3_defconfig

 .../bindings/i2c/loongson,ls2x-i2c.yaml       |  48 +++
 arch/loongarch/configs/loongson3_defconfig    |   1 +
 drivers/i2c/busses/Kconfig                    |   7 +
 drivers/i2c/busses/Makefile                   |   1 +
 drivers/i2c/busses/i2c-gpio.c                 |  41 +-
 drivers/i2c/busses/i2c-ls2x.c                 | 364 ++++++++++++++++++
 drivers/i2c/i2c-core-base.c                   |  10 +-
 7 files changed, 470 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
 create mode 100644 drivers/i2c/busses/i2c-ls2x.c

-- 
2.31.1

[PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[Binbin Zhou]

Under LoongARCH based on ACPI(such as Loongson-3A + LS7A), the ls2x i2c
driver obtains the i2c bus number from ACPI table.

Similar to the DT-base system, this is also a static bus number.

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/i2c/i2c-core-base.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 91007558bcb2..ffab4cc2c6ba 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1559,7 +1559,8 @@ static int __i2c_add_numbered_adapter(struct i2c_adapter *adap)
 int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	struct device *dev = &adapter->dev;
-	int id;
+	acpi_status status;
+	unsigned long long id;
 
 	if (dev->of_node) {
 		id = of_alias_get_id(dev->of_node, "i2c");
@@ -1567,6 +1568,13 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 			adapter->nr = id;
 			return __i2c_add_numbered_adapter(adapter);
 		}
+	} else if (dev->parent->fwnode) {
+		status = acpi_evaluate_integer(ACPI_HANDLE(dev->parent),
+						"_UID", NULL, &id);
+		if (ACPI_SUCCESS(status) && (id >= 0)) {
+			adapter->nr = id;
+			return __i2c_add_numbered_adapter(adapter);
+		}
 	}
 
 	mutex_lock(&core_lock);
-- 
2.31.1

Re: [PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[Mika Westerberg]

Hi,

On Thu, Sep 22, 2022 at 07:39:54PM +0800, Binbin Zhou wrote:
> Under LoongARCH based on ACPI(such as Loongson-3A + LS7A), the ls2x i2c
> driver obtains the i2c bus number from ACPI table.

Why this is needed? The I2CSerialBusV2() resource should be enough to
identify the adapter, and I don't see why static number would be needed
for anything?

Re: [PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[Huacai Chen]

Hi, Mika,

On Thu, Sep 22, 2022 at 8:23 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi,
>
> On Thu, Sep 22, 2022 at 07:39:54PM +0800, Binbin Zhou wrote:
> > Under LoongARCH based on ACPI(such as Loongson-3A + LS7A), the ls2x i2c
> > driver obtains the i2c bus number from ACPI table.
>
> Why this is needed? The I2CSerialBusV2() resource should be enough to
> identify the adapter, and I don't see why static number would be needed
> for anything?
>
In later patches we will add LS7A i2c driver, this driver is shared by
MIPS-based Loongson-3A4000 system (use FDT) and LoongArch-based
Loongson-3A5000 system (use ACPI).

FDT systems support static bus numbers, so we want to do the same
thing on ACPI systems. I think keep this consistency can make user
feel better


Huacai

Re: [PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[Mika Westerberg]

Hi,

On Fri, Sep 23, 2022 at 03:16:03PM +0800, Huacai Chen wrote:
> Hi, Mika,
> 
> On Thu, Sep 22, 2022 at 8:23 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > Hi,
> >
> > On Thu, Sep 22, 2022 at 07:39:54PM +0800, Binbin Zhou wrote:
> > > Under LoongARCH based on ACPI(such as Loongson-3A + LS7A), the ls2x i2c
> > > driver obtains the i2c bus number from ACPI table.
> >
> > Why this is needed? The I2CSerialBusV2() resource should be enough to
> > identify the adapter, and I don't see why static number would be needed
> > for anything?
> >
> In later patches we will add LS7A i2c driver, this driver is shared by
> MIPS-based Loongson-3A4000 system (use FDT) and LoongArch-based
> Loongson-3A5000 system (use ACPI).
> 
> FDT systems support static bus numbers, so we want to do the same
> thing on ACPI systems. I think keep this consistency can make user
> feel better

I don't think the user cares to be honest. As long as all the devices
work as expected ;-) And this saves a couple of lines of code too so if
not really needed, I would just drop that part.

Re: [PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[Jinyang He]

On 2022/9/22 下午7:39, Binbin Zhou wrote:

> Under LoongARCH based on ACPI(such as Loongson-3A + LS7A), the ls2x i2c
> driver obtains the i2c bus number from ACPI table.
>
> Similar to the DT-base system, this is also a static bus number.
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>   drivers/i2c/i2c-core-base.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 91007558bcb2..ffab4cc2c6ba 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1559,7 +1559,8 @@ static int __i2c_add_numbered_adapter(struct i2c_adapter *adap)
>   int i2c_add_adapter(struct i2c_adapter *adapter)
>   {
>   	struct device *dev = &adapter->dev;
> -	int id;
> +	acpi_status status;
> +	unsigned long long id;
>   
>   	if (dev->of_node) {
>   		id = of_alias_get_id(dev->of_node, "i2c");
> @@ -1567,6 +1568,13 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
>   			adapter->nr = id;
>   			return __i2c_add_numbered_adapter(adapter);
>   		}
> +	} else if (dev->parent->fwnode) {
> +		status = acpi_evaluate_integer(ACPI_HANDLE(dev->parent),
> +						"_UID", NULL, &id);
> +		if (ACPI_SUCCESS(status) && (id >= 0)) {

Hi, Binbin,


Emm, the id is unsigned and it always return true if (id>=0). And I think

you should check the other patches.


Jinyang


> +			adapter->nr = id;
> +			return __i2c_add_numbered_adapter(adapter);
> +		}
>   	}
>   
>   	mutex_lock(&core_lock);

Re: [PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[kernel test robot]

Hi Binbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: arc-randconfig-r043-20220922 (https://download.01.org/0day-ci/archive/20220923/202209230216.faCwluIB-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/749fc796eb66dc42c209c6a5808c6b2a5e47fbb6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
        git checkout 749fc796eb66dc42c209c6a5808c6b2a5e47fbb6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/i2c/i2c-core-base.c: In function 'i2c_add_adapter':
>> drivers/i2c/i2c-core-base.c:1568:26: error: implicit declaration of function 'acpi_evaluate_integer'; did you mean 'acpi_evaluate_object'? [-Werror=implicit-function-declaration]
    1568 |                 status = acpi_evaluate_integer(ACPI_HANDLE(dev->parent),
         |                          ^~~~~~~~~~~~~~~~~~~~~
         |                          acpi_evaluate_object
   cc1: some warnings being treated as errors


vim +1568 drivers/i2c/i2c-core-base.c

  1540	
  1541	/**
  1542	 * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  1543	 * @adapter: the adapter to add
  1544	 * Context: can sleep
  1545	 *
  1546	 * This routine is used to declare an I2C adapter when its bus number
  1547	 * doesn't matter or when its bus number is specified by an dt alias.
  1548	 * Examples of bases when the bus number doesn't matter: I2C adapters
  1549	 * dynamically added by USB links or PCI plugin cards.
  1550	 *
  1551	 * When this returns zero, a new bus number was allocated and stored
  1552	 * in adap->nr, and the specified adapter became available for clients.
  1553	 * Otherwise, a negative errno value is returned.
  1554	 */
  1555	int i2c_add_adapter(struct i2c_adapter *adapter)
  1556	{
  1557		struct device *dev = &adapter->dev;
  1558		acpi_status status;
  1559		unsigned long long id;
  1560	
  1561		if (dev->of_node) {
  1562			id = of_alias_get_id(dev->of_node, "i2c");
  1563			if (id >= 0) {
  1564				adapter->nr = id;
  1565				return __i2c_add_numbered_adapter(adapter);
  1566			}
  1567		} else if (dev->parent->fwnode) {
> 1568			status = acpi_evaluate_integer(ACPI_HANDLE(dev->parent),
  1569							"_UID", NULL, &id);
  1570			if (ACPI_SUCCESS(status) && (id >= 0)) {
  1571				adapter->nr = id;
  1572				return __i2c_add_numbered_adapter(adapter);
  1573			}
  1574		}
  1575	
  1576		mutex_lock(&core_lock);
  1577		id = idr_alloc(&i2c_adapter_idr, adapter,
  1578			       __i2c_first_dynamic_bus_num, 0, GFP_KERNEL);
  1579		mutex_unlock(&core_lock);
  1580		if (WARN(id < 0, "couldn't get idr"))
  1581			return id;
  1582	
  1583		adapter->nr = id;
  1584	
  1585		return i2c_register_adapter(adapter);
  1586	}
  1587	EXPORT_SYMBOL(i2c_add_adapter);
  1588	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 1/5] i2c: core: Pick i2c bus number from ACPI if present

[kernel test robot]

Hi Binbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: hexagon-randconfig-r041-20220922 (https://download.01.org/0day-ci/archive/20220923/202209230228.LIiHRmuw-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/749fc796eb66dc42c209c6a5808c6b2a5e47fbb6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
        git checkout 749fc796eb66dc42c209c6a5808c6b2a5e47fbb6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/i2c/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/i2c/i2c-core-base.c:1568:12: error: call to undeclared function 'acpi_evaluate_integer'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                   status = acpi_evaluate_integer(ACPI_HANDLE(dev->parent),
                            ^
   drivers/i2c/i2c-core-base.c:1568:12: note: did you mean 'acpi_evaluate_object'?
   include/acpi/acpixf.h:550:8: note: 'acpi_evaluate_object' declared here
                               acpi_evaluate_object(acpi_handle object,
                               ^
   include/acpi/platform/aclinux.h:93:21: note: expanded from macro 'ACPI_EXTERNAL_RETURN_STATUS'
           static ACPI_INLINE prototype {return(AE_NOT_CONFIGURED);}
                              ^
   1 error generated.


vim +/acpi_evaluate_integer +1568 drivers/i2c/i2c-core-base.c

  1540	
  1541	/**
  1542	 * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  1543	 * @adapter: the adapter to add
  1544	 * Context: can sleep
  1545	 *
  1546	 * This routine is used to declare an I2C adapter when its bus number
  1547	 * doesn't matter or when its bus number is specified by an dt alias.
  1548	 * Examples of bases when the bus number doesn't matter: I2C adapters
  1549	 * dynamically added by USB links or PCI plugin cards.
  1550	 *
  1551	 * When this returns zero, a new bus number was allocated and stored
  1552	 * in adap->nr, and the specified adapter became available for clients.
  1553	 * Otherwise, a negative errno value is returned.
  1554	 */
  1555	int i2c_add_adapter(struct i2c_adapter *adapter)
  1556	{
  1557		struct device *dev = &adapter->dev;
  1558		acpi_status status;
  1559		unsigned long long id;
  1560	
  1561		if (dev->of_node) {
  1562			id = of_alias_get_id(dev->of_node, "i2c");
  1563			if (id >= 0) {
  1564				adapter->nr = id;
  1565				return __i2c_add_numbered_adapter(adapter);
  1566			}
  1567		} else if (dev->parent->fwnode) {
> 1568			status = acpi_evaluate_integer(ACPI_HANDLE(dev->parent),
  1569							"_UID", NULL, &id);
  1570			if (ACPI_SUCCESS(status) && (id >= 0)) {
  1571				adapter->nr = id;
  1572				return __i2c_add_numbered_adapter(adapter);
  1573			}
  1574		}
  1575	
  1576		mutex_lock(&core_lock);
  1577		id = idr_alloc(&i2c_adapter_idr, adapter,
  1578			       __i2c_first_dynamic_bus_num, 0, GFP_KERNEL);
  1579		mutex_unlock(&core_lock);
  1580		if (WARN(id < 0, "couldn't get idr"))
  1581			return id;
  1582	
  1583		adapter->nr = id;
  1584	
  1585		return i2c_register_adapter(adapter);
  1586	}
  1587	EXPORT_SYMBOL(i2c_add_adapter);
  1588	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[Binbin Zhou]

Add support for the ACPI-based device registration so that the driver
can be also enabled through ACPI table.

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/i2c/busses/i2c-gpio.c | 41 ++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index b1985c1667e1..ccea37e755e6 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/acpi.h>
 #include <linux/of.h>
 #include <linux/platform_data/i2c-gpio.h>
 #include <linux/platform_device.h>
@@ -318,6 +319,24 @@ static void of_i2c_gpio_get_props(struct device_node *np,
 		of_property_read_bool(np, "i2c-gpio,scl-output-only");
 }
 
+static void acpi_i2c_gpio_get_props(struct device *dev,
+				  struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	device_property_read_u32(dev, "delay-us", &pdata->udelay);
+
+	if (!device_property_read_u32(dev, "timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		device_property_read_bool(dev, "sda-open-drain");
+	pdata->scl_is_open_drain =
+		device_property_read_bool(dev, "scl-open-drain");
+	pdata->scl_is_output_only =
+		device_property_read_bool(dev, "scl-output-only");
+}
+
 static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
 					   const char *con_id,
 					   unsigned int index,
@@ -363,6 +382,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	enum gpiod_flags gflags;
+	acpi_status status;
+	unsigned long long id;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -375,6 +396,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 
 	if (np) {
 		of_i2c_gpio_get_props(np, pdata);
+	} else if (ACPI_COMPANION(dev)) {
+		acpi_i2c_gpio_get_props(dev, pdata);
 	} else {
 		/*
 		 * If all platform data settings are zero it is OK
@@ -445,7 +468,14 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	adap->dev.parent = dev;
 	adap->dev.of_node = np;
 
-	adap->nr = pdev->id;
+	if (ACPI_COMPANION(dev)) {
+		status = acpi_evaluate_integer(ACPI_HANDLE(dev),
+						"_UID", NULL, &id);
+		if (ACPI_SUCCESS(status) && (id >= 0))
+			adap->nr = id;
+	} else
+		adap->nr = pdev->id;
+
 	ret = i2c_bit_add_numbered_bus(adap);
 	if (ret)
 		return ret;
@@ -491,10 +521,19 @@ static const struct of_device_id i2c_gpio_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);
 #endif
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id i2c_gpio_acpi_match[] = {
+	{"LOON0005"},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, i2c_gpio_acpi_match);
+#endif
+
 static struct platform_driver i2c_gpio_driver = {
 	.driver		= {
 		.name	= "i2c-gpio",
 		.of_match_table	= of_match_ptr(i2c_gpio_dt_ids),
+		.acpi_match_table = ACPI_PTR(i2c_gpio_acpi_match),
 	},
 	.probe		= i2c_gpio_probe,
 	.remove		= i2c_gpio_remove,
-- 
2.31.1

Re: [PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[Mika Westerberg]

Hi,

On Thu, Sep 22, 2022 at 07:39:55PM +0800, Binbin Zhou wrote:
> Add support for the ACPI-based device registration so that the driver
> can be also enabled through ACPI table.
> 
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/i2c/busses/i2c-gpio.c | 41 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> index b1985c1667e1..ccea37e755e6 100644
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> +#include <linux/acpi.h>
>  #include <linux/of.h>
>  #include <linux/platform_data/i2c-gpio.h>
>  #include <linux/platform_device.h>
> @@ -318,6 +319,24 @@ static void of_i2c_gpio_get_props(struct device_node *np,
>  		of_property_read_bool(np, "i2c-gpio,scl-output-only");
>  }
>  
> +static void acpi_i2c_gpio_get_props(struct device *dev,
> +				  struct i2c_gpio_platform_data *pdata)
> +{
> +	u32 reg;
> +
> +	device_property_read_u32(dev, "delay-us", &pdata->udelay);
> +
> +	if (!device_property_read_u32(dev, "timeout-ms", &reg))
> +		pdata->timeout = msecs_to_jiffies(reg);
> +
> +	pdata->sda_is_open_drain =
> +		device_property_read_bool(dev, "sda-open-drain");
> +	pdata->scl_is_open_drain =
> +		device_property_read_bool(dev, "scl-open-drain");
> +	pdata->scl_is_output_only =
> +		device_property_read_bool(dev, "scl-output-only");
> +}

I think this would work with the DT description too as it is using
device_property_xxx() so I wonder if you can just do:

	i2c_gpio_get_props(dev, pdata);

instead of

 	if (np) {
 		of_i2c_gpio_get_props(np, pdata);
	} else if (ACPI_COMPANION(dev)) {
		acpi_i2c_gpio_get_props(dev, pdata);

> +
>  static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
>  					   const char *con_id,
>  					   unsigned int index,
> @@ -363,6 +382,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
>  	enum gpiod_flags gflags;
> +	acpi_status status;
> +	unsigned long long id;
>  	int ret;
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -375,6 +396,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>  
>  	if (np) {
>  		of_i2c_gpio_get_props(np, pdata);
> +	} else if (ACPI_COMPANION(dev)) {
> +		acpi_i2c_gpio_get_props(dev, pdata);
>  	} else {
>  		/*
>  		 * If all platform data settings are zero it is OK
> @@ -445,7 +468,14 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>  	adap->dev.parent = dev;
>  	adap->dev.of_node = np;
>  
> -	adap->nr = pdev->id;
> +	if (ACPI_COMPANION(dev)) {
> +		status = acpi_evaluate_integer(ACPI_HANDLE(dev),
> +						"_UID", NULL, &id);
> +		if (ACPI_SUCCESS(status) && (id >= 0))
> +			adap->nr = id;

Unrelated change? And if not then same comment about why you need the
static number in the first place ;-)

Re: [PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[Binbin Zhou]

Hi Mika：

在 2022/9/22 20:26, Mika Westerberg 写道:
> Hi,
> 
> On Thu, Sep 22, 2022 at 07:39:55PM +0800, Binbin Zhou wrote:
>> Add support for the ACPI-based device registration so that the driver
>> can be also enabled through ACPI table.
>>
>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>> ---
>>   drivers/i2c/busses/i2c-gpio.c | 41 ++++++++++++++++++++++++++++++++++-
>>   1 file changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
>> index b1985c1667e1..ccea37e755e6 100644
>> --- a/drivers/i2c/busses/i2c-gpio.c
>> +++ b/drivers/i2c/busses/i2c-gpio.c
>> @@ -13,6 +13,7 @@
>>   #include <linux/init.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/module.h>
>> +#include <linux/acpi.h>
>>   #include <linux/of.h>
>>   #include <linux/platform_data/i2c-gpio.h>
>>   #include <linux/platform_device.h>
>> @@ -318,6 +319,24 @@ static void of_i2c_gpio_get_props(struct device_node *np,
>>   		of_property_read_bool(np, "i2c-gpio,scl-output-only");
>>   }
>>   
>> +static void acpi_i2c_gpio_get_props(struct device *dev,
>> +				  struct i2c_gpio_platform_data *pdata)
>> +{
>> +	u32 reg;
>> +
>> +	device_property_read_u32(dev, "delay-us", &pdata->udelay);
>> +
>> +	if (!device_property_read_u32(dev, "timeout-ms", &reg))
>> +		pdata->timeout = msecs_to_jiffies(reg);
>> +
>> +	pdata->sda_is_open_drain =
>> +		device_property_read_bool(dev, "sda-open-drain");
>> +	pdata->scl_is_open_drain =
>> +		device_property_read_bool(dev, "scl-open-drain");
>> +	pdata->scl_is_output_only =
>> +		device_property_read_bool(dev, "scl-output-only");
>> +}
> 
> I think this would work with the DT description too as it is using
> device_property_xxx() so I wonder if you can just do:
> 
> 	i2c_gpio_get_props(dev, pdata);
> 
> instead of
> 
>   	if (np) {
>   		of_i2c_gpio_get_props(np, pdata);
> 	} else if (ACPI_COMPANION(dev)) {
> 		acpi_i2c_gpio_get_props(dev, pdata);
> 
Sorry, I don't quite understand how to do a unified api.

We get the corresponding value by matching the propname, but obviously 
the propnames related in the two ways are different.

e.g. "delay-us"(ACPI) vs "i2c-gpio, delay-us"(FDT)

I think the judgment of "if..else.." is indispensable.

thanks.

Binbin

>> +
>>   static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
>>   					   const char *con_id,
>>   					   unsigned int index,
>> @@ -363,6 +382,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>>   	struct device *dev = &pdev->dev;
>>   	struct device_node *np = dev->of_node;
>>   	enum gpiod_flags gflags;
>> +	acpi_status status;
>> +	unsigned long long id;
>>   	int ret;
>>   
>>   	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> @@ -375,6 +396,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>>   
>>   	if (np) {
>>   		of_i2c_gpio_get_props(np, pdata);
>> +	} else if (ACPI_COMPANION(dev)) {
>> +		acpi_i2c_gpio_get_props(dev, pdata);
>>   	} else {
>>   		/*
>>   		 * If all platform data settings are zero it is OK
>> @@ -445,7 +468,14 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>>   	adap->dev.parent = dev;
>>   	adap->dev.of_node = np;
>>   
>> -	adap->nr = pdev->id;
>> +	if (ACPI_COMPANION(dev)) {
>> +		status = acpi_evaluate_integer(ACPI_HANDLE(dev),
>> +						"_UID", NULL, &id);
>> +		if (ACPI_SUCCESS(status) && (id >= 0))
>> +			adap->nr = id;
> 
> Unrelated change? And if not then same comment about why you need the
> static number in the first place ;-)

Re: [PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[Mika Westerberg]

On Fri, Sep 23, 2022 at 06:01:30PM +0800, Binbin Zhou wrote:
> Hi Mika：
> 
> 在 2022/9/22 20:26, Mika Westerberg 写道:
> > Hi,
> > 
> > On Thu, Sep 22, 2022 at 07:39:55PM +0800, Binbin Zhou wrote:
> > > Add support for the ACPI-based device registration so that the driver
> > > can be also enabled through ACPI table.
> > > 
> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > >   drivers/i2c/busses/i2c-gpio.c | 41 ++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 40 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> > > index b1985c1667e1..ccea37e755e6 100644
> > > --- a/drivers/i2c/busses/i2c-gpio.c
> > > +++ b/drivers/i2c/busses/i2c-gpio.c
> > > @@ -13,6 +13,7 @@
> > >   #include <linux/init.h>
> > >   #include <linux/interrupt.h>
> > >   #include <linux/module.h>
> > > +#include <linux/acpi.h>
> > >   #include <linux/of.h>
> > >   #include <linux/platform_data/i2c-gpio.h>
> > >   #include <linux/platform_device.h>
> > > @@ -318,6 +319,24 @@ static void of_i2c_gpio_get_props(struct device_node *np,
> > >   		of_property_read_bool(np, "i2c-gpio,scl-output-only");
> > >   }
> > > +static void acpi_i2c_gpio_get_props(struct device *dev,
> > > +				  struct i2c_gpio_platform_data *pdata)
> > > +{
> > > +	u32 reg;
> > > +
> > > +	device_property_read_u32(dev, "delay-us", &pdata->udelay);
> > > +
> > > +	if (!device_property_read_u32(dev, "timeout-ms", &reg))
> > > +		pdata->timeout = msecs_to_jiffies(reg);
> > > +
> > > +	pdata->sda_is_open_drain =
> > > +		device_property_read_bool(dev, "sda-open-drain");
> > > +	pdata->scl_is_open_drain =
> > > +		device_property_read_bool(dev, "scl-open-drain");
> > > +	pdata->scl_is_output_only =
> > > +		device_property_read_bool(dev, "scl-output-only");
> > > +}
> > 
> > I think this would work with the DT description too as it is using
> > device_property_xxx() so I wonder if you can just do:
> > 
> > 	i2c_gpio_get_props(dev, pdata);
> > 
> > instead of
> > 
> >   	if (np) {
> >   		of_i2c_gpio_get_props(np, pdata);
> > 	} else if (ACPI_COMPANION(dev)) {
> > 		acpi_i2c_gpio_get_props(dev, pdata);
> > 
> Sorry, I don't quite understand how to do a unified api.
> 
> We get the corresponding value by matching the propname, but obviously the
> propnames related in the two ways are different.
> 
> e.g. "delay-us"(ACPI) vs "i2c-gpio, delay-us"(FDT)

Oh, we have different bindings for these? :( That's unfortunate - they
should really have the same. That's the whole purpose of device
properties API in the first place.

Re: [PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[kernel test robot]

Hi Binbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: hexagon-randconfig-r045-20220922 (https://download.01.org/0day-ci/archive/20220923/202209230137.EmkAkBHm-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/90590b2a30c8afa5bb200812ffa52a3c5bb9da6a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
        git checkout 90590b2a30c8afa5bb200812ffa52a3c5bb9da6a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-gpio.c:472:12: error: call to undeclared function 'acpi_evaluate_integer'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                   status = acpi_evaluate_integer(ACPI_HANDLE(dev),
                            ^
   drivers/i2c/busses/i2c-gpio.c:472:12: note: did you mean 'acpi_evaluate_object'?
   include/acpi/acpixf.h:550:8: note: 'acpi_evaluate_object' declared here
                               acpi_evaluate_object(acpi_handle object,
                               ^
   include/acpi/platform/aclinux.h:93:21: note: expanded from macro 'ACPI_EXTERNAL_RETURN_STATUS'
           static ACPI_INLINE prototype {return(AE_NOT_CONFIGURED);}
                              ^
   1 error generated.


vim +/acpi_evaluate_integer +472 drivers/i2c/busses/i2c-gpio.c

   375	
   376	static int i2c_gpio_probe(struct platform_device *pdev)
   377	{
   378		struct i2c_gpio_private_data *priv;
   379		struct i2c_gpio_platform_data *pdata;
   380		struct i2c_algo_bit_data *bit_data;
   381		struct i2c_adapter *adap;
   382		struct device *dev = &pdev->dev;
   383		struct device_node *np = dev->of_node;
   384		enum gpiod_flags gflags;
   385		acpi_status status;
   386		unsigned long long id;
   387		int ret;
   388	
   389		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   390		if (!priv)
   391			return -ENOMEM;
   392	
   393		adap = &priv->adap;
   394		bit_data = &priv->bit_data;
   395		pdata = &priv->pdata;
   396	
   397		if (np) {
   398			of_i2c_gpio_get_props(np, pdata);
   399		} else if (ACPI_COMPANION(dev)) {
   400			acpi_i2c_gpio_get_props(dev, pdata);
   401		} else {
   402			/*
   403			 * If all platform data settings are zero it is OK
   404			 * to not provide any platform data from the board.
   405			 */
   406			if (dev_get_platdata(dev))
   407				memcpy(pdata, dev_get_platdata(dev), sizeof(*pdata));
   408		}
   409	
   410		/*
   411		 * First get the GPIO pins; if it fails, we'll defer the probe.
   412		 * If the SCL/SDA lines are marked "open drain" by platform data or
   413		 * device tree then this means that something outside of our control is
   414		 * marking these lines to be handled as open drain, and we should just
   415		 * handle them as we handle any other output. Else we enforce open
   416		 * drain as this is required for an I2C bus.
   417		 */
   418		if (pdata->sda_is_open_drain)
   419			gflags = GPIOD_OUT_HIGH;
   420		else
   421			gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
   422		priv->sda = i2c_gpio_get_desc(dev, "sda", 0, gflags);
   423		if (IS_ERR(priv->sda))
   424			return PTR_ERR(priv->sda);
   425	
   426		if (pdata->scl_is_open_drain)
   427			gflags = GPIOD_OUT_HIGH;
   428		else
   429			gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
   430		priv->scl = i2c_gpio_get_desc(dev, "scl", 1, gflags);
   431		if (IS_ERR(priv->scl))
   432			return PTR_ERR(priv->scl);
   433	
   434		if (gpiod_cansleep(priv->sda) || gpiod_cansleep(priv->scl))
   435			dev_warn(dev, "Slow GPIO pins might wreak havoc into I2C/SMBus bus timing");
   436		else
   437			bit_data->can_do_atomic = true;
   438	
   439		bit_data->setsda = i2c_gpio_setsda_val;
   440		bit_data->setscl = i2c_gpio_setscl_val;
   441	
   442		if (!pdata->scl_is_output_only)
   443			bit_data->getscl = i2c_gpio_getscl;
   444		bit_data->getsda = i2c_gpio_getsda;
   445	
   446		if (pdata->udelay)
   447			bit_data->udelay = pdata->udelay;
   448		else if (pdata->scl_is_output_only)
   449			bit_data->udelay = 50;			/* 10 kHz */
   450		else
   451			bit_data->udelay = 5;			/* 100 kHz */
   452	
   453		if (pdata->timeout)
   454			bit_data->timeout = pdata->timeout;
   455		else
   456			bit_data->timeout = HZ / 10;		/* 100 ms */
   457	
   458		bit_data->data = priv;
   459	
   460		adap->owner = THIS_MODULE;
   461		if (np)
   462			strscpy(adap->name, dev_name(dev), sizeof(adap->name));
   463		else
   464			snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);
   465	
   466		adap->algo_data = bit_data;
   467		adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
   468		adap->dev.parent = dev;
   469		adap->dev.of_node = np;
   470	
   471		if (ACPI_COMPANION(dev)) {
 > 472			status = acpi_evaluate_integer(ACPI_HANDLE(dev),
   473							"_UID", NULL, &id);
   474			if (ACPI_SUCCESS(status) && (id >= 0))
   475				adap->nr = id;
   476		} else
   477			adap->nr = pdev->id;
   478	
   479		ret = i2c_bit_add_numbered_bus(adap);
   480		if (ret)
   481			return ret;
   482	
   483		platform_set_drvdata(pdev, priv);
   484	
   485		/*
   486		 * FIXME: using global GPIO numbers is not helpful. If/when we
   487		 * get accessors to get the actual name of the GPIO line,
   488		 * from the descriptor, then provide that instead.
   489		 */
   490		dev_info(dev, "using lines %u (SDA) and %u (SCL%s)\n",
   491			 desc_to_gpio(priv->sda), desc_to_gpio(priv->scl),
   492			 pdata->scl_is_output_only
   493			 ? ", no clock stretching" : "");
   494	
   495		i2c_gpio_fault_injector_init(pdev);
   496	
   497		return 0;
   498	}
   499	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/5] i2c: gpio: Add support on ACPI-based system

[kernel test robot]

Hi Binbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: arc-randconfig-r043-20220922 (https://download.01.org/0day-ci/archive/20220923/202209230422.se6Sxqnq-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/90590b2a30c8afa5bb200812ffa52a3c5bb9da6a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
        git checkout 90590b2a30c8afa5bb200812ffa52a3c5bb9da6a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-gpio.c: In function 'i2c_gpio_probe':
>> drivers/i2c/busses/i2c-gpio.c:472:26: error: implicit declaration of function 'acpi_evaluate_integer'; did you mean 'acpi_evaluate_object'? [-Werror=implicit-function-declaration]
     472 |                 status = acpi_evaluate_integer(ACPI_HANDLE(dev),
         |                          ^~~~~~~~~~~~~~~~~~~~~
         |                          acpi_evaluate_object
   cc1: some warnings being treated as errors


vim +472 drivers/i2c/busses/i2c-gpio.c

   375	
   376	static int i2c_gpio_probe(struct platform_device *pdev)
   377	{
   378		struct i2c_gpio_private_data *priv;
   379		struct i2c_gpio_platform_data *pdata;
   380		struct i2c_algo_bit_data *bit_data;
   381		struct i2c_adapter *adap;
   382		struct device *dev = &pdev->dev;
   383		struct device_node *np = dev->of_node;
   384		enum gpiod_flags gflags;
   385		acpi_status status;
   386		unsigned long long id;
   387		int ret;
   388	
   389		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   390		if (!priv)
   391			return -ENOMEM;
   392	
   393		adap = &priv->adap;
   394		bit_data = &priv->bit_data;
   395		pdata = &priv->pdata;
   396	
   397		if (np) {
   398			of_i2c_gpio_get_props(np, pdata);
   399		} else if (ACPI_COMPANION(dev)) {
   400			acpi_i2c_gpio_get_props(dev, pdata);
   401		} else {
   402			/*
   403			 * If all platform data settings are zero it is OK
   404			 * to not provide any platform data from the board.
   405			 */
   406			if (dev_get_platdata(dev))
   407				memcpy(pdata, dev_get_platdata(dev), sizeof(*pdata));
   408		}
   409	
   410		/*
   411		 * First get the GPIO pins; if it fails, we'll defer the probe.
   412		 * If the SCL/SDA lines are marked "open drain" by platform data or
   413		 * device tree then this means that something outside of our control is
   414		 * marking these lines to be handled as open drain, and we should just
   415		 * handle them as we handle any other output. Else we enforce open
   416		 * drain as this is required for an I2C bus.
   417		 */
   418		if (pdata->sda_is_open_drain)
   419			gflags = GPIOD_OUT_HIGH;
   420		else
   421			gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
   422		priv->sda = i2c_gpio_get_desc(dev, "sda", 0, gflags);
   423		if (IS_ERR(priv->sda))
   424			return PTR_ERR(priv->sda);
   425	
   426		if (pdata->scl_is_open_drain)
   427			gflags = GPIOD_OUT_HIGH;
   428		else
   429			gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
   430		priv->scl = i2c_gpio_get_desc(dev, "scl", 1, gflags);
   431		if (IS_ERR(priv->scl))
   432			return PTR_ERR(priv->scl);
   433	
   434		if (gpiod_cansleep(priv->sda) || gpiod_cansleep(priv->scl))
   435			dev_warn(dev, "Slow GPIO pins might wreak havoc into I2C/SMBus bus timing");
   436		else
   437			bit_data->can_do_atomic = true;
   438	
   439		bit_data->setsda = i2c_gpio_setsda_val;
   440		bit_data->setscl = i2c_gpio_setscl_val;
   441	
   442		if (!pdata->scl_is_output_only)
   443			bit_data->getscl = i2c_gpio_getscl;
   444		bit_data->getsda = i2c_gpio_getsda;
   445	
   446		if (pdata->udelay)
   447			bit_data->udelay = pdata->udelay;
   448		else if (pdata->scl_is_output_only)
   449			bit_data->udelay = 50;			/* 10 kHz */
   450		else
   451			bit_data->udelay = 5;			/* 100 kHz */
   452	
   453		if (pdata->timeout)
   454			bit_data->timeout = pdata->timeout;
   455		else
   456			bit_data->timeout = HZ / 10;		/* 100 ms */
   457	
   458		bit_data->data = priv;
   459	
   460		adap->owner = THIS_MODULE;
   461		if (np)
   462			strscpy(adap->name, dev_name(dev), sizeof(adap->name));
   463		else
   464			snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);
   465	
   466		adap->algo_data = bit_data;
   467		adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
   468		adap->dev.parent = dev;
   469		adap->dev.of_node = np;
   470	
   471		if (ACPI_COMPANION(dev)) {
 > 472			status = acpi_evaluate_integer(ACPI_HANDLE(dev),
   473							"_UID", NULL, &id);
   474			if (ACPI_SUCCESS(status) && (id >= 0))
   475				adap->nr = id;
   476		} else
   477			adap->nr = pdev->id;
   478	
   479		ret = i2c_bit_add_numbered_bus(adap);
   480		if (ret)
   481			return ret;
   482	
   483		platform_set_drvdata(pdev, priv);
   484	
   485		/*
   486		 * FIXME: using global GPIO numbers is not helpful. If/when we
   487		 * get accessors to get the actual name of the GPIO line,
   488		 * from the descriptor, then provide that instead.
   489		 */
   490		dev_info(dev, "using lines %u (SDA) and %u (SCL%s)\n",
   491			 desc_to_gpio(priv->sda), desc_to_gpio(priv->scl),
   492			 pdata->scl_is_output_only
   493			 ? ", no clock stretching" : "");
   494	
   495		i2c_gpio_fault_injector_init(pdev);
   496	
   497		return 0;
   498	}
   499	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[PATCH 3/5] dt-bindings: i2c: add bindings for Loongson LS2X I2C

[Binbin Zhou]

Add device tree bindings for the i2c controller on the Loongson-2K Soc
or Loongosn LS7A bridge chip.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../bindings/i2c/loongson,ls2x-i2c.yaml       | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
new file mode 100644
index 000000000000..8c785f329d2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/loongson,ls2x-i2c.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Loongson LS2X I2C Controller
+
+maintainers:
+  - Binbin Zhou <zhoubinbin@loongson.cn>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - loongson,ls2k-i2c # Loongson-2K SoCs
+      - loongson,ls7a-i2c # Loongson LS7A Bridge
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c@1fe21000 {
+        compatible = "loongson,ls2k-i2c";
+        reg = <0 0x1fe21000 0 0x8>;
+        interrupts = <22>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        eeprom@57{
+            compatible = "atmel,24c16";
+            reg = <0x57>;
+            pagesize = <16>;
+        };
+    };
-- 
2.31.1

[PATCH 4/5] i2c: Add driver for Loongson-2K/LS7A I2C controller

[Binbin Zhou]

This I2C module is integrated into the Loongson-2K SoC and the Loongson
LS7A bridge chip.

Initialize the i2c controller early. This is required in order to ensure
that core system devices such as the display controller(DC) attached via
I2C are available early in boot.

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/i2c/busses/Kconfig    |   7 +
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-ls2x.c | 364 ++++++++++++++++++++++++++++++++++
 3 files changed, 372 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-ls2x.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7284206b278b..a4687a6abe75 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -887,6 +887,13 @@ config I2C_OWL
 	  Say Y here if you want to use the I2C bus controller on
 	  the Actions Semiconductor Owl SoC's.
 
+config I2C_LS2X
+	tristate "Loongson LS2X I2C adapter"
+	depends on MACH_LOONGSON64 || COMPILE_TEST
+	help
+	  If you say yes to this option, support will be included for the
+	  I2C interface on the Loongson's LS2K/LS7A Platform-Bridge.
+
 config I2C_PASEMI
 	tristate "PA Semi SMBus interface"
 	depends on PPC_PASEMI && PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index c5cac15f075c..721841361e34 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_I2C_MV64XXX)	+= i2c-mv64xxx.o
 obj-$(CONFIG_I2C_MXS)		+= i2c-mxs.o
 obj-$(CONFIG_I2C_NOMADIK)	+= i2c-nomadik.o
 obj-$(CONFIG_I2C_NPCM)		+= i2c-npcm7xx.o
+obj-$(CONFIG_I2C_LS2X)		+= i2c-ls2x.o
 obj-$(CONFIG_I2C_OCORES)	+= i2c-ocores.o
 obj-$(CONFIG_I2C_OMAP)		+= i2c-omap.o
 obj-$(CONFIG_I2C_OWL)		+= i2c-owl.o
diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
new file mode 100644
index 000000000000..80d8f1e12876
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ls2x.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loongson-2K/7A I2C master mode driver
+ *
+ * Copyright (C) 2013 Loongson Technology Corporation Limited
+ * Copyright (C) 2014-2017 Lemote, Inc.
+ *
+ * Originally written by liushaozong
+ */
+
+#include <linux/i2c.h>
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/completion.h>
+#include <linux/platform_device.h>
+
+#define LS2X_I2C_PRER_LO_REG	0x0
+#define LS2X_I2C_PRER_HI_REG	0x1
+#define LS2X_I2C_CTR_REG	0x2
+#define LS2X_I2C_TXR_REG	0x3
+#define LS2X_I2C_RXR_REG	0x3
+#define LS2X_I2C_CR_REG		0x4
+#define LS2X_I2C_SR_REG		0x4
+
+#define LS2X_I2C_CMD_START	BIT(7)
+#define LS2X_I2C_CMD_STOP	BIT(6)
+#define LS2X_I2C_CMD_READ	BIT(5)
+#define LS2X_I2C_CMD_WRITE	BIT(4)
+#define LS2X_I2C_CMD_ACK	BIT(3)
+#define LS2X_I2C_CMD_IACK	BIT(0)
+
+#define LS2X_I2C_SR_NOACK	BIT(7)
+#define LS2X_I2C_SR_BUSY	BIT(6)
+#define LS2X_I2C_SR_AL		BIT(5)
+#define LS2X_I2C_SR_TIP		BIT(1)
+#define LS2X_I2C_SR_IF		BIT(0)
+
+#define I2C_MAX_RETRIES		5
+
+/* I2C clock frequency 50M */
+#define I2C_CLK_RATE_50M	(50 * 1000000)
+
+#define i2c_readb(addr)		readb(dev->base + addr)
+#define i2c_writeb(val, addr)	writeb(val, dev->base + addr)
+
+struct ls2x_i2c_dev {
+	unsigned int		suspended:1;
+	struct device		*dev;
+	void __iomem		*base;
+	int			irq;
+	u32			bus_freq_hz;
+	struct completion	cmd_complete;
+	struct i2c_adapter	adapter;
+};
+
+static void i2c_stop(struct ls2x_i2c_dev *dev)
+{
+again:
+	i2c_writeb(LS2X_I2C_CMD_STOP, LS2X_I2C_CR_REG);
+	wait_for_completion(&dev->cmd_complete);
+
+	i2c_readb(LS2X_I2C_SR_REG);
+
+	while (i2c_readb(LS2X_I2C_SR_REG) & LS2X_I2C_SR_BUSY)
+		goto again;
+}
+
+static int ls2x_i2c_start(struct ls2x_i2c_dev *dev,
+		     int dev_addr, int flags)
+{
+	int retry = I2C_MAX_RETRIES;
+	unsigned char addr = (dev_addr & 0x7f) << 1;
+
+	addr |= (flags & I2C_M_RD) ? 1 : 0;
+start:
+	mdelay(1);
+	i2c_writeb(addr, LS2X_I2C_TXR_REG);
+	dev_dbg(dev->dev, "%s <line%d>: i2c device address: 0x%x\n",
+			__func__, __LINE__, addr);
+
+	i2c_writeb((LS2X_I2C_CMD_START | LS2X_I2C_CMD_WRITE),
+			LS2X_I2C_CR_REG);
+	wait_for_completion(&dev->cmd_complete);
+
+	if (i2c_readb(LS2X_I2C_SR_REG) & LS2X_I2C_SR_NOACK) {
+		i2c_stop(dev);
+		while (retry--)
+			goto start;
+		dev_info(dev->dev, "There is no i2c device ack\n");
+		return 0;
+	}
+
+	return 1;
+}
+
+static void ls2x_i2c_reginit(struct ls2x_i2c_dev *dev)
+{
+	u16 val = 0x12c;
+
+	if (dev->bus_freq_hz)
+		val = I2C_CLK_RATE_50M / (5 * dev->bus_freq_hz) - 1;
+
+	i2c_writeb(0, LS2X_I2C_CTR_REG);
+	i2c_writeb(val & 0xff, LS2X_I2C_PRER_LO_REG);
+	i2c_writeb((val & 0xff00) >> 8, LS2X_I2C_PRER_HI_REG);
+	i2c_writeb(0xc0, LS2X_I2C_CTR_REG);
+}
+
+static int ls2x_i2c_read(struct ls2x_i2c_dev *dev,
+			unsigned char *buf, int count)
+{
+	int i;
+	int cmd = LS2X_I2C_CMD_READ;
+
+	for (i = 0; i < count; i++) {
+		if (i == count - 1)
+			cmd |= LS2X_I2C_CMD_ACK;
+
+		i2c_writeb(cmd, LS2X_I2C_CR_REG);
+		wait_for_completion(&dev->cmd_complete);
+
+		buf[i] = i2c_readb(LS2X_I2C_RXR_REG);
+		dev_dbg(dev->dev, "%s <line%d>: read buf[%d] <= %02x\n",
+				__func__, __LINE__, i, buf[i]);
+	}
+
+	return i;
+}
+
+static int ls2x_i2c_write(struct ls2x_i2c_dev *dev,
+			unsigned char *buf, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		i2c_writeb(buf[i], LS2X_I2C_TXR_REG);
+		dev_dbg(dev->dev, "%s <line%d>: write buf[%d] => %02x\n",
+				__func__, __LINE__, i, buf[i]);
+
+		i2c_writeb(LS2X_I2C_CMD_WRITE, LS2X_I2C_CR_REG);
+		wait_for_completion(&dev->cmd_complete);
+
+		if (i2c_readb(LS2X_I2C_SR_REG) & LS2X_I2C_SR_NOACK) {
+			dev_dbg(dev->dev, "%s <line%d>: device no ack\n",
+					__func__, __LINE__);
+			i2c_stop(dev);
+			return 0;
+		}
+	}
+
+	return i;
+}
+
+static int ls2x_i2c_doxfer(struct ls2x_i2c_dev *dev,
+			struct i2c_msg *msgs, int num)
+{
+	int i;
+	struct i2c_msg *m = msgs;
+
+	for (i = 0; i < num; i++) {
+		reinit_completion(&dev->cmd_complete);
+		if (!ls2x_i2c_start(dev, m->addr, m->flags))
+			return 0;
+
+		if (m->flags & I2C_M_RD)
+			ls2x_i2c_read(dev, m->buf, m->len);
+		else
+			ls2x_i2c_write(dev, m->buf, m->len);
+		++m;
+	}
+
+	i2c_stop(dev);
+
+	return i;
+}
+
+static int ls2x_i2c_xfer(struct i2c_adapter *adap,
+			struct i2c_msg *msgs, int num)
+{
+	int ret, retry;
+	struct ls2x_i2c_dev *dev;
+
+	dev = i2c_get_adapdata(adap);
+	for (retry = 0; retry < adap->retries; retry++) {
+		ret = ls2x_i2c_doxfer(dev, msgs, num);
+		if (ret != -EAGAIN)
+			return ret;
+
+		udelay(100);
+	}
+
+	return -EREMOTEIO;
+}
+
+static unsigned int ls2x_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm ls2x_i2c_algo = {
+	.master_xfer	= ls2x_i2c_xfer,
+	.functionality	= ls2x_i2c_func,
+};
+
+/*
+ * Interrupt service routine. This gets called whenever an I2C interrupt
+ * occurs.
+ */
+static irqreturn_t ls2x_i2c_isr(int this_irq, void *dev_id)
+{
+	unsigned char iflag;
+	struct ls2x_i2c_dev *dev = dev_id;
+
+	iflag = i2c_readb(LS2X_I2C_SR_REG);
+
+	if (iflag & LS2X_I2C_SR_IF) {
+		i2c_writeb(LS2X_I2C_CMD_IACK, LS2X_I2C_CR_REG);
+		complete(&dev->cmd_complete);
+	} else
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static int ls2x_i2c_probe(struct platform_device *pdev)
+{
+	int r;
+	struct ls2x_i2c_dev *dev;
+	struct i2c_adapter *adap;
+
+	dev = devm_kzalloc(&pdev->dev, sizeof(struct ls2x_i2c_dev), GFP_KERNEL);
+	if (unlikely(!dev))
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, dev);
+	init_completion(&dev->cmd_complete);
+	dev->dev = &pdev->dev;
+
+	/* Map hardware registers */
+	dev->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dev->base))
+		return PTR_ERR(dev->base);
+
+	r = platform_get_irq(pdev, 0);
+	if (unlikely(r <= 0))
+		return -ENODEV;
+	dev->irq = r;
+
+	r = devm_request_irq(&pdev->dev, dev->irq, ls2x_i2c_isr,
+			      IRQF_SHARED, "ls2x-i2c", dev);
+	if (unlikely(r)) {
+		dev_err(dev->dev, "failure requesting irq %i\n", dev->irq);
+		return r;
+	}
+
+	dev->bus_freq_hz = i2c_acpi_find_bus_speed(&pdev->dev);
+	if (!dev->bus_freq_hz)
+		device_property_read_u32(&pdev->dev, "clock-frequency",
+					&dev->bus_freq_hz);
+
+	ls2x_i2c_reginit(dev);
+
+	/* Add the i2c adapter */
+	adap = &dev->adapter;
+	i2c_set_adapdata(adap, dev);
+	adap->nr = pdev->id;
+	strscpy(adap->name, pdev->name, sizeof(adap->name));
+	adap->owner = THIS_MODULE;
+	adap->class = I2C_CLASS_HWMON;
+	adap->retries = I2C_MAX_RETRIES;
+	adap->algo = &ls2x_i2c_algo;
+	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
+	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
+
+	/* i2c device drivers may be active on return from add_adapter() */
+	r = i2c_add_adapter(adap);
+	if (r) {
+		dev_err(dev->dev, "failure adding adapter\n");
+		return r;
+	}
+
+	return 0;
+}
+
+static int ls2x_i2c_remove(struct platform_device *pdev)
+{
+	struct ls2x_i2c_dev *dev = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&dev->adapter);
+	return 0;
+}
+
+static int __maybe_unused ls2x_i2c_suspend_noirq(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct ls2x_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+	i2c_dev->suspended = 1;
+
+	return 0;
+}
+
+static int __maybe_unused ls2x_i2c_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct ls2x_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+	i2c_dev->suspended = 0;
+	ls2x_i2c_reginit(i2c_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops ls2x_i2c_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ls2x_i2c_suspend_noirq, ls2x_i2c_resume)
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id ls2x_i2c_id_table[] = {
+	{.compatible = "loongson,ls2k-i2c"},
+	{.compatible = "loongson,ls7a-i2c"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ls2x_i2c_id_table);
+#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id ls2x_i2c_acpi_match[] = {
+	{"LOON0004"},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, ls2x_i2c_acpi_match);
+#endif
+
+static struct platform_driver ls2x_i2c_driver = {
+	.probe		= ls2x_i2c_probe,
+	.remove		= ls2x_i2c_remove,
+	.driver		= {
+		.name	= "ls2x-i2c",
+		.owner	= THIS_MODULE,
+		.pm	= &ls2x_i2c_dev_pm_ops,
+		.of_match_table = of_match_ptr(ls2x_i2c_id_table),
+		.acpi_match_table = ACPI_PTR(ls2x_i2c_acpi_match),
+	},
+};
+
+static int __init ls2x_i2c_init_driver(void)
+{
+	return platform_driver_register(&ls2x_i2c_driver);
+}
+subsys_initcall(ls2x_i2c_init_driver);
+
+static void __exit ls2x_i2c_exit_driver(void)
+{
+	platform_driver_unregister(&ls2x_i2c_driver);
+}
+module_exit(ls2x_i2c_exit_driver);
+
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
+MODULE_DESCRIPTION("Loongson LS2X I2C bus adapter");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ls2x-i2c");
-- 
2.31.1

Re: [PATCH 4/5] i2c: Add driver for Loongson-2K/LS7A I2C controller

[kernel test robot]

Hi Binbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20220923/202209231036.KnY4wKcL-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/df754cf9cc58fc815223d6126fa1c86717cd3465
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Binbin-Zhou/i2c-ls2x-Add-support-for-the-Loongson-2K-LS7A-I2C/20220922-194252
        git checkout df754cf9cc58fc815223d6126fa1c86717cd3465
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/i2c/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-ls2x.c: In function 'i2c_stop':
>> drivers/i2c/busses/i2c-ls2x.c:45:33: error: implicit declaration of function 'writeb' [-Werror=implicit-function-declaration]
      45 | #define i2c_writeb(val, addr)   writeb(val, dev->base + addr)
         |                                 ^~~~~~
   drivers/i2c/busses/i2c-ls2x.c:60:9: note: in expansion of macro 'i2c_writeb'
      60 |         i2c_writeb(LS2X_I2C_CMD_STOP, LS2X_I2C_CR_REG);
         |         ^~~~~~~~~~
>> drivers/i2c/busses/i2c-ls2x.c:44:33: error: implicit declaration of function 'readb' [-Werror=implicit-function-declaration]
      44 | #define i2c_readb(addr)         readb(dev->base + addr)
         |                                 ^~~~~
   drivers/i2c/busses/i2c-ls2x.c:63:9: note: in expansion of macro 'i2c_readb'
      63 |         i2c_readb(LS2X_I2C_SR_REG);
         |         ^~~~~~~~~
   cc1: some warnings being treated as errors


vim +/writeb +45 drivers/i2c/busses/i2c-ls2x.c

    43	
  > 44	#define i2c_readb(addr)		readb(dev->base + addr)
  > 45	#define i2c_writeb(val, addr)	writeb(val, dev->base + addr)
    46	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

[PATCH 5/5] LoongArch: Enable LS2X I2C in loongson3_defconfig

[Binbin Zhou]

This is now supported, enable for Loongson-3 systems. Other systems are
affected.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 arch/loongarch/configs/loongson3_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index 4083d3051109..7910adf20887 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -558,6 +558,7 @@ CONFIG_HW_RANDOM_VIRTIO=m
 CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_PIIX4=y
 CONFIG_I2C_GPIO=y
+CONFIG_I2C_LS2X=y
 CONFIG_SPI=y
 CONFIG_GPIO_SYSFS=y
 CONFIG_GPIO_LOONGSON=y
-- 
2.31.1