Page MenuHomeFreeBSD

Adding WMI embedded Binary MOF object interface driver.
ClosedPublic

Authored by takawata on Thu, Sep 5, 3:45 AM.

Details

Summary

This patch extracts Managed Object Format object embedded to ACPI WMI devices, which appears some laptops like ThinkPad.

It is compiled binary form and needs extra tools to obtain human readable output. (http://github.com/pali/bmfdec)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

takawata created this revision.Thu, Sep 5, 3:45 AM
avg added a comment.Thu, Sep 5, 6:25 AM

If this driver just provides a single sysctl, then I think that it would be simpler to add that sysctl to acpi_wmi.
Do you plan to have any other functionality in acpi_wmi_bmof?

takawata updated this revision to Diff 61671.Thu, Sep 5, 9:00 AM

Add the feature to acpi_wmi driver, instead of creating new driver.

takawata updated this revision to Diff 61672.Thu, Sep 5, 9:10 AM

Free MOF buf only when successfilly detach.

takawata updated this revision to Diff 61693.Thu, Sep 5, 3:37 PM

Some nit, pointed out by hrs@

takawata added a reviewer: hrs.EditedFri, Sep 6, 5:24 AM

No features other than obtaining MOF is expected, so I add the feature in parent driver, instead of having new driver.

avg added inline comments.Fri, Sep 6, 5:38 AM
share/man/man4/acpi_wmi.4
61 ↗(On Diff #61693)

Are you sure about '-width indent' ?
I am not an expert on this, it just looks unusual.

63 ↗(On Diff #61693)

This line looks too long. Please break it up.
Also, I am not sure if any special mark up is needed for URLs.

94 ↗(On Diff #61693)

I would use something else rather than "blah blah" here.
Maybe "..." or "etc".

116 ↗(On Diff #61693)

No need for the space before the final '.'.

sys/dev/acpi_support/acpi_wmi.c
281 ↗(On Diff #61693)

Please use the standard FreeBSD indentation for continuation lines (+4 spaces) here and in other new code.

282 ↗(On Diff #61693)

Missing space before '}'.

286 ↗(On Diff #61693)

Continuation

287 ↗(On Diff #61693)

missing space

289 ↗(On Diff #61693)

missing space

291 ↗(On Diff #61693)

'(' should be on the original line.
Also, wrong indentation.

takawata updated this revision to Diff 61717.Fri, Sep 6, 7:57 AM

Style nit.

avg added a comment.Fri, Sep 6, 8:08 AM

A couple more nits.

share/man/man4/acpi_wmi.4
63 ↗(On Diff #61717)

I think that it is preferred that a new sentence is started on a new line.
That is to say that there should be a line break after each period that ends a sentence.

sys/dev/acpi_support/acpi_wmi.c
281 ↗(On Diff #61717)

Extra space before ';'

282 ↗(On Diff #61717)

A blank line is needed after a variable definition block.

291 ↗(On Diff #61717)

I think that we do not do a nested continuation, so this line should probably be indented the same as the previous one. Though, that would be less readable, so I don't insist on such change.

takawata updated this revision to Diff 61719.Fri, Sep 6, 8:41 AM

style nit.

avg accepted this revision.Fri, Sep 6, 9:56 AM

Looks good to me.

This revision is now accepted and ready to land.Fri, Sep 6, 9:56 AM
This revision was automatically updated to reflect the committed changes.