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

[Wolfram Sang <wsa@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

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.

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

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]