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: <CAHp75Vdnm1bykoX5Dh9nen7jB5bGfLELw0PvXBcqs1PXTf31rA@mail.gmail.com>
Date:   Thu, 27 Apr 2023 08:57:04 +0300
From:   Andy Shevchenko <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, 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

On Thu, Apr 27, 2023 at 5:15 AM Jiawen Wu <jiawenwu@...stnetic.com> wrote:
> On Wednesday, April 26, 2023 11:45 PM, andy.shevchenko@...il.com wrote:
> > Wed, Apr 26, 2023 at 03:14:27PM +0800, Jiawen Wu kirjoitti:

...

> > > +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?
>
> devm_platform_ioremap_resource() has one more devm_request_mem_region()
> operation than devm_ioremap(). By my test, this memory cannot be re-requested,
> only re-mapped.

Yeah, which makes a point that the mother driver requests a region
that doesn't belong to it. You need to split that properly in the
mother driver and avoid requesting it there. Is it feasible? If not,
why?

...

> > >     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?
>
> I2C is connected to our NIC chip with no PCI ID, so I register a platform device for it.
> Please see the 4/9 patch. Software nodes are used to pass the device structure but
> no DT and ACPI. I haven't found another way to initialize flags yet, other than the
> platform data used in the previous patch (it seems to be an obsolete way).

You can share a common data structure between the mother driver and
her children. In that case you may access it via
`dev_get_drvdata(pdev.dev->parent)` call.

OTOH, the property, if only Linux (kernel) specific for internal
usage, should be named accordingly, or be prepared to have one in
Device Tree / ACPI / etc. Examples: USB dwc3 driver (see "linux,"
ones), or intel-lpss-pci.c/intel-lpss-acpi.c (see the SPI type).

-- 
With Best Regards,
Andy Shevchenko