netdev - RE: [PATCH net-next v8 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 for Android: free password hash cracker in your pocket

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

Date: Wed, 17 May 2023 17:25:19 +0800
From: Jiawen Wu <jiawenwu@...stnetic.com>
To: "'Jarkko Nikula'" <jarkko.nikula@...ux.intel.com>,
	<andy.shevchenko@...il.com>
Cc: <netdev@...r.kernel.org>,
	<andriy.shevchenko@...ux.intel.com>,
	<mika.westerberg@...ux.intel.com>,
	<jsd@...ihalf.com>,
	<Jose.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>,
	"'Piotr Raczynski'" <piotr.raczynski@...el.com>
Subject: RE: [PATCH net-next v8 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC

On Wednesday, May 17, 2023 4:49 PM, Jarkko Nikula wrote:
> On 5/15/23 12:24, andy.shevchenko@...il.com wrote:
> > Mon, May 15, 2023 at 02:31:53PM +0800, Jiawen Wu kirjoitti:
> >> Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> >> with SFP.
> >>
> >> Introduce the property "snps,i2c-platform" to match device data for Wangxun
> >> in software node case. Since IO resource was mapped on the ethernet driver,
> >> add a model quirk to get regmap from parent device.
> >>
> >> 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.
> >
> > ...
> >
> >>   	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
> >> +	if (device_property_present(&pdev->dev, "snps,i2c-platform"))
> >> +		dev->flags |= MODEL_WANGXUN_SP;
> >
> > What I meant here is to use device_property_present() _iff_ you have decided to
> > go with the _vendor-specific_ property name.
> >
> > Otherwise it should be handled differently, i.e. with reading the actual value
> > of that property. Hence it should correspond the model enum, which you need to
> > declare in the Device Tree bindings before use.
> >
> > So, either
> >
> > 	if (device_property_present(&pdev->dev, "wx,..."))
> > 		dev->flags |= MODEL_WANGXUN_SP;
> >
> > or
> >
> > 	if ((dev->flags & MODEL_MASK) == MODEL_NONE) {
> > 	// you now have to distinguish that there is no model set in driver data
> > 		u32 model;
> >
> > 		ret = device_property_read_u32(dev, "snps,i2c-platform");
> > 		if (ret) {
> > 			...handle error...
> > 		}
> > 		dev->flags |= model
> >
> I'm not a device tree expert but I wonder would it be possible somehow
> combine this and compatible properties in dw_i2c_of_match[]? They set
> model flag for MODEL_MSCC_OCELOT and MODEL_BAIKAL_BT1.

Maybe the table could be changed to match device property, instead of relying
on DT only. Or device_get_match_data() could be also implemented in
software node case?

> 
> Then I'm thinking is "snps,i2c-platform" descriptive enough name for a
> model and does it confuse with "snps,designware-i2c" compatible property?

I'd like to change the name back to "wx,i2c-snps-model" for the specific one.