Created attachment 235042 [details] asmc driver patch file Add support for MacBookPro6,2 (Mid 2010, 15") to asmc driver. Ignores interrupt from ambient light sensor (referred to as ALSL from what I could find). Adds the number of an unknown interrupt to the log output. First patch submission. Please let me know if I should change/fix anything. P.S. The reason I choose to ignore the interrupt is that it triggers so often it fills the logs/console with messages; I am not aware of how to determine if the light levels are going up or down to make it useful; if this interrupt exists on other models, I didn't see any code for it; and this interrupt does not trigger on a MacBookPro9,2 that I tested the original asmc driver against. I'm pretty sure it is specific to this model.
Thank you for your report and patch Ed
I suspect that we should, at least, add some comment explaining why we don't print anything for the light sensor interrupt, and perhaps make that behaviour conditional on the model. And even if we silence the message, the interrupt handler is still running and sends notifications to devd. So, it'd be good to figure out exactly what this interrupt is for and whether we can mask it instead of ignoring it.
(In reply to Mark Johnston from comment #2) I agree with Mark's suggestions. Other than that, the patch looks fine to me.
Thank you both for the feed back. I will submit a few new options. I can confidently say the interrupt only triggers on a change of ambient light - up or down. The reason it triggers so often (for me at least) is that in a low to mildly lighted room, the laptop's own display brightness will trigger the sensor. eg running top on what was a sparse white text on a black background console, or switching between a light background and a dark background window in an X11 session. I'm not sure how/if i can get much detail on this specific Apple product. I did not want to look at any Linux(GPL) code that might exist for this driver. If you have any references on interrupt handling/masking and where that is handled, that would be helpful. I'll take a look at devd documentation. I'm getting re-antiquated with freeBSD (I spent many days using 2.0-4.x. My day jobs unfortunately took me elsewhere in Unix land). So please forgive my ignorance on freeBSD specifics. I really appreciate anyone even considering a patch for a 12 year old piece of hardware.
Hello, I'm attaching a new patch that added a comment, a check for the model, and a new error message if the interrupt triggers on a different model than expected. I'm still looking into how drivers work in general, asmc in particular, and how to mask it so as not to trigger devd. This will take some time for me.
Created attachment 235397 [details] asmc driver patch for MacBookPro6,2 Updated with some of the recommendations.
(In reply to ed crowe from comment #5) Thanks! With respect to interrupts, what happens in asmc(4) is that an interrupt triggers a call to asmc_sms_intrfast(), which determines the interrupt type, latches it in sc->sc_sms_intrtype, and potentially prints a message. Then, it schedules a thread to deliver the devd notification, by calling taskqueue_enqueue(). That causes asmc_sms_task() to be called. So, the question is whether we do in fact want to hide these interrupts from devd. Per comment 4 it seems like the interrupts aren't spurious, i.e., the driver is behaving more or less as intended. So I think the real question is whether the light sensor interrupt is generating "too many" notifications: when running top(1), do you see devd consuming a significant amount of CPU time? If not, then I wouldn't worry much about it and can simply commit the latest patch.
(In reply to Mark Johnston from comment #7) Thanks for the driver info. I think it is performing as expected. Neither devd's CPU nor WCPU went over 0.15% while madly waving a flash light in front of the machine in a dark room. I'm fine with committing it as is. If I get some time, I'll give suppressing the interrupt a shot and submit a new patch. Thanks again!
(In reply to ed crowe from comment #8) Ok. I will just go ahead and commit then, I think the patch is good.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=3416f5cde7a7e01b25b5f5fd240ae8aa12fd70bf commit 3416f5cde7a7e01b25b5f5fd240ae8aa12fd70bf Author: ed crowe <fbsdbug@edcrowe.com> AuthorDate: 2022-07-25 15:35:46 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2022-07-25 15:43:56 +0000 asmc: Add support for MacBookPro6,2 Modify asmc_sms_printintr() to be silent when the ambient light sensor interrupt fires on this model, since the messages can otherwise fill up the dmesg. PR: 265005 Reviewed by: markj MFC after: 2 weeks sys/dev/asmc/asmc.c | 18 +++++++++++++++++- sys/dev/asmc/asmcvar.h | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-)
I made some small changes, mostly to fix whitespace and to fetch the model string out of the softc instead of using kern_getenv(). Thanks for the patch.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=00a59bf122d9ceb0d5847be9b465a9e684624e77 commit 00a59bf122d9ceb0d5847be9b465a9e684624e77 Author: ed crowe <fbsdbug@edcrowe.com> AuthorDate: 2022-07-25 15:35:46 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2022-08-08 13:52:13 +0000 asmc: Add support for MacBookPro6,2 Modify asmc_sms_printintr() to be silent when the ambient light sensor interrupt fires on this model, since the messages can otherwise fill up the dmesg. PR: 265005 Reviewed by: markj (cherry picked from commit 3416f5cde7a7e01b25b5f5fd240ae8aa12fd70bf) sys/dev/asmc/asmc.c | 18 +++++++++++++++++- sys/dev/asmc/asmcvar.h | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-)