On Sat, 2022-10-29 at 07:04 -0700, Guenter Roeck wrote: > On 10/29/22 03:30, Frank Crawford wrote: > > Changes in this patch set: > > > > * Define the DMI matching table for board specific settings during > > the > > chip initialisation and move the only current board specific > > setting > > to this new table. > > > > * Export the table for use by udev. > > > > v2: updates following comments: > > > > * Converted to use callback function. > > > > * Moved call to callback funtion to sio_data into it87_find in line > > with other settings for sio_data. This requires dmi_data also > > passed > > to access additional data. > > > > That is really not what I meant when I asked to use a callback > function. > As written, the code might as well call that function directly from > the > init code, and there would be no reason to have a callback function > pointer. > > A callback function would only make sense to me if it is added > to struct dmi_system_id, and called via dmi_check_system(). > See other callers of dmi_check_system() for examples. Oh, investigating other kernel code I see what you mean, and it does simplify one possible future update, but looking through the other modules in hwmon, I can't see any using a DMI callback. The primary use of dmi_check_system() is just as a count of successful matches. Also, just going back to a previous comment about creating a static version of sio_data and updating this in the callback, this does worry me going forward as in future I hope to add code to handle the case of multiple chips. Updating the static version for one chip may cause issues with the other chips. If I go this way, I'll would probably still use the DMI driver_data, save it, and apply that where required, rather than doing it directly in the callback function. This would be safer as, for example, in the current patch, setting the sio_data->skip_pwm is safest done after all the other settings in it87_find(), not prior to all the other setups done in that function. > > Thanks, > Guenter Regards Frank
