Hi Guenter Roeck, Thanks for the feedback. > Subject: Re: [PATCH 0/2] Convert enum->pointer for data in the tmp51x match > tables > > On Sun, Aug 20, 2023 at 01:49:08PM +0100, Biju Das wrote: > > Convert enum->pointer for data in the match tables, so that > > device_get_match_data() can do match against OF/ACPI/I2C tables, once > > i2c bus type match support added to it. > > > > I don't see why this would be necessary. You don't explain why the current > implementation would no longer work. Various other drivers implement the > same mechanism as this driver, i.e., type cast the return value of > device_get_match_data() to a non-pointer. I'd argue that changing the > functionality of device_get_match_data() such that this is no longer > possible would be inherently flawed and would introduce unnecessary > complexity to drivers using that mechanism today. If > device_get_match_data() is enhanced to include the functionality of > i2c_match_id(), it should be done in a way that doesn't mandate such an API > change. Currently nothing is broken. There is a helper function to do OF/ACPI/ID match(i2c_get_match_data). But Dmitry proposed to extend device_get_match_data() to buses so that we can get rid of i2c_get_match_data and future it can be extended to SPI aswell. see [1]. While doing this Jonathan found an issue where it won't work if OF table is using pointer and ID is using enum in the match data. Moreover,the proposed API returns NULL for non-match. So Andy proposed to convert the valid enums to non-zero or use a pointer. In this case, pointer makes sense as the hardware differences between the chips are compared against type rather than using feature and driver data. In the second patch, I just used a driver data to avoid one such case. [1] https://lore.kernel.org/all/20230804161728.394920-1-biju.das.jz@xxxxxxxxxxxxxx/ > > > This patch series is only compile tested. > > I assume that means you don't have access to the chip. Is this correct ? > Just asking, because it would be great to have a register dump which would > enable me to write unit test code. That is correct, I don't have access to the chip. Cheers, Biju > > > > > Biju Das (2): > > hwmon: tmp513: Convert enum->pointer for data in the match tables > > hwmon: tmp513: Add temp_config to struct tmp51x_info > > > > drivers/hwmon/tmp513.c | 51 > > ++++++++++++++++++++++-------------------- > > 1 file changed, 27 insertions(+), 24 deletions(-) > > > > -- > > 2.25.1 > >
