asmc: introduce the concept of generic models

[ngie]

Summary

Having to enter in each of the models for Apple hardware, recompiling,
etc, is tedious. Provide generic models so end-users can leverage some
of the capabilities provided by the driver, i.e., common features like
minimal fans and lights (if present on the generic model) support.

The generic models are as follows:

• Macmini
• MacBookAir
• MacBookPro
• MacPro

This sort of follows the pattern established by the applesmc driver in
Linux.

MFC after: 2 weeks

Test Plan

I've been using a diff similar to this with my MacBookPro13,2 (Retina MBP) and MacBookPro16,1 (Gen2 TouchBar-based MBP) for some time in order to introduce initial support for both of the models.

[ngie]

Fix syntax in previous revision

[ngie]

Use terse messages to make @kib happier 😛.

[ngie]

Revert nitems(model->smc_temps) change

This part isn't ready for prime time: I have another larger change that
greatly refactors the temp sensor code and the existing code won't play
well with this part of the change.

[kib]

I would add a local like const struct asmc_model *m;, and set it to the found 'model'. Then goto to the end of the function, which would do

free(model);
return (m);

This way you have free() only in one place, and all returns would be guaranteed to free the memory.

sys/dev/asmc/asmc.c

634

There should be blank line before this.

639

This is anything but terse. All this stuff belongs to either comment above, or to the man page.
Kernel _must not_ output that.

[ngie]

In D55395#1267017, @kib wrote:

I would add a local like const struct asmc_model *m;, and set it to the found 'model'. Then goto to the end of the function, which would do

free(model);
return (m);

This way you have free() only in one place, and all returns would be guaranteed to free the memory.

Yup -- agreed.

sys/dev/asmc/asmc.c

639

This is anything but terse. All this stuff belongs to either comment above, or to the man page.
Kernel _must not_ output that.

Yeah. I have mixed feelings about outputting information like this, so I agree: your comment is valid.

Would you propose saying something like this to the manpage?

DIAGNOSTICS
.Pp
.Bl -diag
.It "asmc0: generic model detected"
Model specific quirks not applied for a given model.
Partial SMC support is enabled and some features might not function properly.

[ngie]

sys/dev/asmc/asmc.c

639

PS This is a classic example of me trying to be helpful to end-users and encourage data gathering to improve the driver.
This only supports x86* today, but the Apple SMC controller was carried forward to their ARM chips, so there's no reason why this shouldn't [eventually] support arm64 platforms.

[ngie]

Address @kib's comments.

[ngie]

sys/dev/asmc/asmc.c

639

@ziaee : do you have any thoughts on the manpage suggestion in the thread?
This seems a bit more helpful BTW:

DIAGNOSTICS
.Pp
.Bl -diag
.It "asmc0: generic model detected"
Model specific quirks not applied.
Partial SMC support is enabled.
Some features, e.g., sensors, fans, lights, etc, might not function properly.

[ngie]

Fix obvious compilation errors

[guest-seuros]

This is awesome, this is the similar concept i was planning on proposing,

I was trying to gather enough hardware samples from the community to better understand the patterns across different models before suggesting a generic approach. (probed from machines from 2009 to 2017)

For example, on some laptops I have worked with, I had to upgrade them to the latest macOS version supported by the hardware in order to get the SMC firmware updated.
Only after that would FreeBSD behave properly. That suggests there are firmware-level differences that may influence how far a generic model can safely go. (We can't downgrade the versions, so i can't retry)

For example, some Mac mini and laptop models expose WOL support (see D54439), while others do not. It is not totally clear to me whether capabilities are consistent enough across all models within a given family string (ex. Macmini) to safely assume the same feature set.

[kib]

sys/dev/asmc/asmc.c

612–616

This might be goto out; as well.

[ngie]

sys/dev/asmc/asmc.c

612–616

It's not strictly required since the only resource needing to be free'd (model_name) hasn't been properly allocated, but I'll do it for the sake of consistency.

[ngie]

Update fast exit path in asmc_match(..) to use goto out.

[ngie]

Add newline in device_printf call for aesthetics

[ngie]

Yank device_printf added to asmc_match in this change.

Per @imp, probe is called 2 in each driver's lifecycle. Ergo, the message
would be printed out 3 times by asmc_match since the function is called
twice from probe and once from attach. I'm looking at refactoring how this
works given that the model description component after this change is a bit
messy due to tight coupling with asmc_match and its callers, but for now it's
better to have this support than not have it.

[ngie]

Yank device_printf added to asmc_match in this change.

Per @imp, probe is called 2 in each driver's lifecycle. Ergo, the message
would be printed out 3 times by asmc_match since the function is called
twice from probe and once from attach.

I'm looking at refactoring how this works given that setting the device
description after this change is a bit messy due to tight coupling with
asmc_match and its callers, but for now it's better to have this support
than not have it.