On 10/29/22 19:06, Frank Crawford wrote:
On Sat, 2022-10-29 at 18:39 -0700, Guenter Roeck wrote:
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:
...
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.
For present cases, yes, but consider the current setting, which
disables pmw2 for the FN68PT board, if there was a second chip on that
board, you would not want the same setting for both chips.
Quite obviously it _is_ known that this board only has a single chip,
and it will never magically have a second chip.
Chip specific code should be implemented in the probe function (which
is chip specific), not in the init function (which isn't).
I haven't yet worked out how it would be distinguish at the time, but
also it hasn't been strictly necessary.
A simple case I have coming up for future patch is to use the DMI table
to ignore ACPI conflicts when we know it is safe, but that should be
done on each chip separately, not necessarily globally for all chips on
that board. Again, in practice it isn't important, and I haven't
worked out how to specify it separately yet.
Also, I have been looking at the difference in the use of
dmi_check_system() and what the use of dmi_first_match() does and it
really is just a case of the callback being used at the time of
matching vs deferring the actions to be performed at later and possibly
more appropriate stages.
It doesn't need to do anything but set values or parameters. Any action
should be done in the probe function, not in the init function.
Guenter
