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

["Pali Rohár" <pali@kernel.org>]

On Saturday 22 June 2024 16:20:15 Pali Rohár wrote:
> > >> +	{
> > >> +		.matches = {
> > >> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > >> +			DMI_MATCH(DMI_PRODUCT_NAME, "XPS 15 7590"),
> > >> +		},
> > >> +		.driver_data = (void *)0x29L,
> > > 
> > > At least for me, casting i2c address to LONG and then to pointer looks
> > > very strange. If I look at this code without knowing what the number
> > > 0x29 means I would not figure out that expression "(void *)0x29L" is i2c
> > > address.
> > > 
> > > Is not there a better way to write i2c address? E.g. ".i2c_addr = 0x29"
> > > instead of ".something = (void *)0x29L" to make it readable?
> > 
> > struct dmi_system_id is an existing structure and we cannot just go adding
> > fields to it. driver_data is intended to tie driver specific data to
> > each DMI match, often pointing to some struct, so it is a void *, but
> 
> Yes, I know it.
> 
> > in this case we only need a single integer, so we store that in the
> > pointer. That is is the address becomes obvious when looking at the code
> > which consumes the data.
> 
> Ok, this makes sense. Anyway, is explicit void* cast and L suffix
> required?

I have checked compilers and L suffix is not needed. No error or warning
is generated without L.

Explicit cast is needed as without it compiler generates warning.