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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZElCHGho-szyySGC@surfacebook>
Date:   Wed, 26 Apr 2023 18:45:14 +0300
From:   andy.shevchenko@...il.com
To:     Jiawen Wu <jiawenwu@...stnetic.com>
Cc:     netdev@...r.kernel.org, jarkko.nikula@...ux.intel.com,
        andriy.shevchenko@...ux.intel.com, mika.westerberg@...ux.intel.com,
        jsd@...ihalf.com, ose.Abreu@...opsys.com, andrew@...n.ch,
        hkallweit1@...il.com, linux@...linux.org.uk,
        linux-i2c@...r.kernel.org, linux-gpio@...r.kernel.org,
        mengyuanlou@...-swift.com
Subject: Re: [RFC PATCH net-next v5 2/9] i2c: designware: Add driver support
 for Wangxun 10Gb NIC

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