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

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

Hi Wolfram,


On 2023/6/23 16:47, Wolfram Sang wrote:
> Hi,
>
> thank you for submitting this driver!
>
>> diff --git a/drivers/i2c/busses/i2c-zhaoxin.c b/drivers/i2c/busses/i2c-zhaoxin.c
>> new file mode 100644
>> index 000000000000..bd5432bec7be
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-zhaoxin.c
>> @@ -0,0 +1,599 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + *  i2c-zhaoxin-i2c.c - Zhaoxin I2C controller driver
> This doesn't match the real file name anymore. I suggest to leave out
> the filename completely.
>

will leave out the filename completely.


>> + *
>> + *  Copyright(c) 2023 Shanghai Zhaoxin Semiconductor Corporation.
>> + *                    All rights reserved.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/i2c.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
> Some includes are missing here. For example, module.h as you are using
> some of its functionality directly. Probably some more includes are
> needed.


include files that are directly referenced will be added.


>> + * registers
>> + */
>> +/* I2C MMIO Address Constants */
>> +#define ZXI2C_CR_L                     0x00
>> +#define   ZXI2C_CPU_RDY          BIT(3)
>> +#define   ZXI2C_TX_END           BIT(2)
>> +#define   ZXI2C_RX_ACK           BIT(1)
> The register layout looks similar to the i2c-wmt driver. Could this
> driver maybe be reused for your hardware? I'll stop reviewing here
> because we need to have that clear first.
>
> Thank you and happy hacking,
>
>     Wolfram
>

I just learned that they are indeed derived from the same VIA I2C IP.
The relationship between WonderMedia/VIA/Zhaoxin looks like this:
* The i2c-wmt.c driver is used on the WonderMedia/VT8500 platform,
   an ARM-based SOC from VIA. This SOC hasn't been updated in years.
   Its MAINTAINERS are also Orphan.
* Lots of VIA IP is now owned by Zhaoxin, including I2C, SMBus, etc.
   Zhaoxin adds some new features on the basis of the original I2C IP,
   such as FIFO mode. And there are plans to continue adding new
   features.

Based on this information, we thought it might be more maintainable
to create a new driver file.

Thank you,
Hans