Re: [PATCH v3 3/6] platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800

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

 



On Sun, Jun 23, 2024 at 12:50 AM Pali Rohár <pali@xxxxxxxxxx> wrote:
> On Sunday 23 June 2024 00:43:17 Andy Shevchenko wrote:
> > On Sat, Jun 22, 2024 at 6:43 PM Pali Rohár <pali@xxxxxxxxxx> wrote:
> > > On Saturday 22 June 2024 16:20:15 Pali Rohár wrote:

...

> > > Definition of the table can be simplified by defining a macro which
> > > expand to those verbose parts which are being repeating, without need to
> > > introduce something "new". E.g.:
> > >
> > > #define DELL_LIS3LV02D_DMI_ENTRY(product_name, i2c_addr) \
> > >         { \
> > >                 .matches = {
> > >                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), \
> > >                         DMI_MATCH(DMI_PRODUCT_NAME, product_name), \
> > >                 }, \
> > >                 .driver_data = (void *)(i2c_addr), \
> >
> > I'm not against this as we have a lot of different examples similar to
> > this (with maybe other types of ID tables). But what makes me worry is
> > the use of (void *) here. Shouldn't it be (const void *) so we exclude
> > the (potential) cases of dropping const qualifier?
>
> I do not know what is the best way here for casting short int to void*.
> For me it looks strange if such casting is needed. Anyway I think that
> in any case casting 16-bit short integer to const void* does not produce
> different result as casting to plain (non-const) void*. It is not about
> const qualifier but about integer-to-pointer cast, where is dropped
> everything to that integer type.

You missed the long-term issue with macros like this. If we ever go
away from an integer to a real pointer, it will be easier to make such
a mistake. using proper casting will prevent you from doing that.

>
> > >         }
> > >
> > > static const struct dmi_system_id smo8800_lis3lv02d_devices[] = {
> > >         DELL_LIS3LV02D_DMI_ENTRY("Latitude E5250", 0x29),
> > >         DELL_LIS3LV02D_DMI_ENTRY("Latitude E5450", 0x29),
> > >         ...
> > >         { }
> > > };
> > >
> > > Any opinion about this?

-- 
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux