Re: [PATCH v7 0/4] Extend device_get_match_data() to struct bus_type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andy,

On Mon, Aug 7, 2023 at 4:54 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> wrote:
> > > > On Fri,  4 Aug 2023 17:17:24 +0100
> > > > Biju Das <biju.das.jz@xxxxxxxxxxxxxx> 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?).

I do have an idea how to handle that (inspired by the macro-heavy
R-Car pin control drivers ;-)

    #define DEVICES(fn) \
        fn("vendor1", "device1", &driver_data1) \
        fn("vendor3", "device2", &driver_data2) \
        fn("vendor3", "device2", &driver_data3)

    OF_MATCH_TABLE(driver_of_match, DEVICES);
    I2C_MATCH_TABLE(driver_i2c_ids, DEVICES);

and in the of resp. i2c headers:

    #define EMIT_OF_ENTRY(_vendor, _device, _data) \
            { .name = _vendor ## "," ## _device, .data = _data) },
    #define EMIT_OF_ENTRIES(_name, _devs) \
            static const struct of_device_id _name[] = { \
                    _devs(EMIT_OF_ENTRY) \
                    { } \
            } \

    #define EMIT_I2C_ENTRY(_vendor, _device, _data) \
            { .name = _device, .driver_data = (void *)_data) },
    #define EMIT_I2C_ENTRIES(_name, _devs) \
            static const struct i2c_device_id _name[] = { \
                    _devs(EMIT_I2C_ENTRY) \
                    { } \
            } \

I didn't try to compile this, so I may have missed something (e.g.
a required intermediate macro to ensure proper expansion).
Unfortunately this would break grep'ability for compatible values...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux