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

[andy.shevchenko@gmail.com]

Wed, Apr 26, 2023 at 03:14:27PM +0800, Jiawen Wu kirjoitti:
> Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> with SFP.
> 
> Introduce the property "i2c-dw-flags" to match device data for software
> node case. Since IO resource was mapped on the ethernet driver, add a model
> quirk to get resource from platform info.
> 
> 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.

Thanks for an update, my comments below.

...

>  		goto done_nolock;
>  	}
>  
> +	if ((dev->flags & MODEL_MASK) == MODEL_WANGXUN_SP) {
> +		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> +		goto done_nolock;
> +	}

	switch (dev->flags & MODEL_MASK) {
	case AMD:
		...
	case WANGXUN:
		...
	default:
		break;
	}

...

> +static int txgbe_i2c_request_regs(struct dw_i2c_dev *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev->dev);
> +	struct resource *r;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r)
> +		return -ENODEV;
> +
> +	dev->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
> +
> +	return PTR_ERR_OR_ZERO(dev->base);
> +}

Redundant. See below.

...

>  	case MODEL_BAIKAL_BT1:
>  		ret = bt1_i2c_request_regs(dev);
>  		break;
> +	case MODEL_WANGXUN_SP:
> +		ret = txgbe_i2c_request_regs(dev);

How is it different to...

> +		break;
>  	default:
>  		dev->base = devm_platform_ioremap_resource(pdev, 0);

...this one?

...

>  	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);

> +	if (!dev->flags)

No need to check this. Just define priorities (I would go with above to be
higher priority).

> +		device_property_read_u32(&pdev->dev, "i2c-dw-flags", &dev->flags);

Needs to be added to the Device Tree bindings I believe.

But wait, don't we have other ways to detect your hardware at probe time and
initialize flags respectively?

-- 
With Best Regards,
Andy Shevchenko