Re: [PATCH v2 1/1] hwmon: (it87) Create DMI matching table for various board settings

[Guenter Roeck <linux@roeck-us.net>]

On Sun, Oct 30, 2022 at 10:43:59AM +1100, Frank Crawford wrote:
> 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.
> 
The value is set based on DMI data. I don't see how that would make
a difference even if there are multiple chips. The DMI data would
still be the same and is board specific, not chip specific.

Guenter