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

[Hans Hu <HansHu-oc@zhaoxin.com>]

Hi Krzysztof,


On 2023/6/9 17:33, Krzysztof Kozlowski wrote:
> 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?


Ok, so should I add COMPILE_TEST after ACPI?
Like:
-if ACPI
+if ACPI || COMPILE_TEST
[...]
+    config I2C_ZHAOXIN
+        tristate "ZHAOXIN I2C Interface"
+        depends on PCI || COMPILE_TEST


> ...
>
>> +
>> +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.


I explained this earlier in my email.  Link:
https://lore.kernel.org/all/689522d4-b7b3-59ee-685d-fa88315dbde6@zhaoxin.com/
Since I have not received any further reply from you, I have not made 
any change.

I saw your latest discussion with Andi. I understand what you mean now,
so I will drop it in the patch v5.

Thank you,
Hans


>
>> +	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
>