On Wed, May 17, 2023 at 11:49 AM Jarkko Nikula
<jarkko.nikula@xxxxxxxxxxxxxxx> wrote:
> On 5/15/23 12:24, andy.shevchenko@xxxxxxxxx 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
