Re: [PATCH net-next v4 2/8] i2c: designware: Add driver support for Wangxun 10Gb NIC

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Sat, Apr 22, 2023 at 12:56:15PM +0800, Jiawen Wu wrote:
> Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> with SFP.
> 
> Add platform data to pass IOMEM base address, board flag since resource
> address was mapped on ethernet driver. Since there is no device tree to
> get the clock, the parameters hcnt/lcnt are also set by platform data.
> 
> The exists IP limitations are dealt as workarounds:
> - IP does not support interrupt mode, it works on polling mode.
> - Additionally set FIFO depth address the chip issue.
> 
> Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Please, use --cc parameter to `git format-patch ...`.

Also for tag block we do not use blank lines.

...

>  #define MODEL_MSCC_OCELOT			BIT(8)
>  #define MODEL_BAIKAL_BT1			BIT(9)
>  #define MODEL_AMD_NAVI_GPU			BIT(10)
> +#define MODEL_WANGXUN_SP			BIT(11)
>  #define MODEL_MASK				GENMASK(11, 8)

Yeah, maybe next one will need to transform this from bitfield to plain number.

...

> -static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
> +static int poll_i2c_adap_quirk(struct dw_i2c_dev *dev)

i2c_dw_poll_adap_quirk()

...

> +static bool i2c_is_model_poll(struct dw_i2c_dev *dev)

i2c_dw_is_...

...

> +++ b/include/linux/platform_data/i2c-dw.h

No way we need this in a new code.

> +struct dw_i2c_platform_data {
> +	void __iomem *base;

You should use regmap.

> +	unsigned int flags;
> +	unsigned int ss_hcnt;
> +	unsigned int ss_lcnt;
> +	unsigned int fs_hcnt;
> +	unsigned int fs_lcnt;

No, use device properties.

> +};

-- 
With Best Regards,
Andy Shevchenko