Re: [PATCH v4] i2c: add support for Zhaoxin I2C controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/06/2023 05:16, Hans Hu wrote:
> Add Zhaoxin I2C controller driver. It provides the access to the i2c
> busses, which connects to the touchpad, eeprom, etc.
> 
> Zhaoxin I2C controller has two separate busses, so may accommodate up
> to two I2C adapters. Those adapters are listed in the ACPI namespace
> with the "IIC1D17" HID, and probed by a platform driver.
> 
> The driver works with IRQ mode, and supports basic I2C features. Flags
> I2C_AQ_NO_ZERO_LEN and I2C_AQ_COMB_WRITE_THEN_READ are used to limit
> the unsupported access.
> 


> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cb932c6f8959..bb61e19eef72 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9761,6 +9761,13 @@ L:	linux-i2c@vger.kernel.org
>  F:	Documentation/i2c/busses/i2c-ismt.rst
>  F:	drivers/i2c/busses/i2c-ismt.c
>  
> +ZHAOXIN I2C CONTROLLER DRIVER
> +M:	Hans Hu <hanshu@zhaoxin.com>
> +L:	linux-i2c@vger.kernel.org
> +S:	Maintained
> +W:	https://www.zhaoxin.com
> +F:	drivers/i2c/busses/i2c-zhaoxin.c
> +
>  I2C/SMBUS STUB DRIVER
>  M:	Jean Delvare <jdelvare@suse.com>
>  L:	linux-i2c@vger.kernel.org
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 87600b4aacb3..1f181757ce2a 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -333,6 +333,16 @@ config I2C_VIAPRO
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-viapro.
>  
> +config I2C_ZHAOXIN
> +	tristate "ZHAOXIN I2C Interface"
> +	depends on (PCI && ACPI) || COMPILE_TEST
> +	help
> +	  If you say yes to this option, support will be included for the
> +	  ZHAOXIN I2C interface
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-zhaoxin.
> +
>  if ACPI
>  
>  comment "ACPI drivers"

Shouldn't you driver be in ACPI drivers?

...

> +
> +static int zxi2c_probe(struct platform_device *pdev)
> +{
> +	int err = 0;
> +	struct zxi2c *i2c;
> +	struct pci_dev *pci;
> +	struct device *dev;
> +
> +	/* make sure this is zhaoxin platform */

Why? You didn't provid explanation last time for this.

> +	dev = pdev->dev.parent;
> +	if (dev && dev_is_pci(dev)) {
> +		pci = to_pci_dev(dev);
> +		if (pci->vendor != PCI_VENDOR_ID_ZHAOXIN ||
> +		    pci->device != ZXI2C_PARENT_PCI_DID)
> +			return -ENODEV;
> +	} else {
> +		return -ENODEV;
> +	}

Drop it.

> +


Best regards,
Krzysztof