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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 6/22/24 6:26 PM, Pali Rohár wrote:
> 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.

The L definitely is necessary to avoid a warning about casting an
integer to a pointer of different size. Without the L the integer
constant will alway be 32 bits which triggers the different size
warning on architectures with 64 bit pointers.

Regards,

Hans