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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Wed, 17 May 2023 12:40:00 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Jarkko Nikula <jarkko.nikula@...ux.intel.com>
Cc: Jiawen Wu <jiawenwu@...stnetic.com>, 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 Wed, May 17, 2023 at 11:49 AM Jarkko Nikula
<jarkko.nikula@...ux.intel.com> 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

Me neither, that's why I replied earlier that this needs to be
reviewed by DT people.

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



-- 
With Best Regards,
Andy Shevchenko