linux-kernel - RE: [PATCH v7 0/4] Extend device_get_match_data() to struct bus

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-next>] [day] [month] [year] [list]

Message-ID: <OS0PR01MB59221A1ADB67E96E9E39D0198613A@OS0PR01MB5922.jpnprd01.prod.outlook.com>
Date:   Thu, 10 Aug 2023 09:05:10 +0000
From:   Biju Das <biju.das.jz@...renesas.com>
To:     Jonathan Cameron <jic23@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Peter Rosin <peda@...ntia.se>
CC:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Daniel Scally <djrscally@...il.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        Andi Shyti <andi.shyti@...nel.org>,
        Wolfram Sang <wsa@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH v7 0/4] Extend device_get_match_data() to struct bus_type

Hi all,

> Subject: Re: [PATCH v7 0/4] Extend device_get_match_data() to struct
> bus_type
> 
> On Tue, 8 Aug 2023 15:18:52 +0300
> Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> 
> > On Mon, Aug 07, 2023 at 01:37:12PM -0700, Dmitry Torokhov wrote:
> > > On Mon, Aug 07, 2023 at 05:54:07PM +0300, Andy Shevchenko wrote:
> > > > On Sun, Aug 06, 2023 at 02:29:50PM +0100, Jonathan Cameron wrote:
> > > > > On Sat, 5 Aug 2023 17:42:21 +0000 Biju Das
> > > > > <biju.das.jz@...renesas.com> wrote:
> > > > > > > On Fri,  4 Aug 2023 17:17:24 +0100 Biju Das
> > > > > > > <biju.das.jz@...renesas.com> wrote:
> >
> > ...
> >
> > > > > > + * Besides the fact that some drivers abuse the device ID
> > > > > > + driver_data type
> > > > > > + * and claim it to be integer, for the bus specific ID tables
> > > > > > + the driver_data
> > > > > > + * may be defined as kernel_ulong_t. For these tables 0 is a
> > > > > > + valid response,
> > > > > > + * but not for this function. It's recommended to convert
> > > > > > + those either to avoid
> > > > > > + * 0 or use a real pointer to the predefined driver data.
> > > >
> > > > > We still need to maintain consistency across the two tables,
> > > > > which is a stronger requirement than avoiding 0.
> > > >
> > > > True. Any suggestion how to amend the above comment? Because the
> > > > documentation makes sense on its own (may be split from the
> series?).
> > > >
> > > > > Some drivers already do that by forcing the enum used to start
> > > > > at 1 which doesn't solver the different data types issue.
> > > >
> > > > And some maintainers do not want to see non-enum values in i2c ID
> table.
> > > > *Shrug*.
> > >
> > > So in legacy ID lookup path we can safely assume that values below
> > > 4096 are scalars and return NULL from the new
> > > device_get_match_data(). This way current drivers using the values
> > > as indices or doing direct comparisons against them can continue
> > > doing manual look up and using them as they see fit. And we can
> convert the drivers at our leisure.
> >
> > It's a good idea, but I believe will be received as hack.
> > But why not to try? We indeed have an error pointers for the last page
> > and NULL (which is only up to 16 IIRC) and reserved space in the first
> > page. To be more robust I would check all enums that are being used in
> > I2C ID tables for maximum value and if that is less than 16, use
> > ZERO_OR_NULL_PTR() instead of custom stuff.
> >
> See iio/adc/max1363 example that has 37ish.
> 
> Could tidy that one up first and hopefully not find any others that are in
> subsystems not keen on the move away from enums.

If there is no objection, I can fix this using i2c_get_match_data() for iio/adc/max1363

and 

device_match_data() will return ZERO_OR_NULL_PTR() if max enum ID in the ID lookup table is less than 16.

and the drivers that use legacy ID's will fallback to ID lookup.

So that there won't be any regression.

Cheers,
Biju