Page MenuHomeFreeBSD

Summary: acpi/ec: Fix regression caused by r340644
ClosedPublic

Authored by bwidawsk on Nov 23 2018, 6:01 PM.
Tags
None
Referenced Files
F105903541: D18311.diff
Sun, Dec 22, 9:02 AM
Unknown Object (File)
Sat, Dec 7, 2:11 AM
Unknown Object (File)
Wed, Dec 4, 5:26 PM
Unknown Object (File)
Wed, Dec 4, 5:26 PM
Unknown Object (File)
Wed, Dec 4, 5:26 PM
Unknown Object (File)
Wed, Dec 4, 5:06 PM
Unknown Object (File)
Tue, Dec 3, 11:28 PM
Unknown Object (File)
Tue, Dec 3, 9:38 PM

Details

Summary

After r340644 there were two things wrong in cases where there is both
an ECDT, and an EC device exposed via acpica. The first is a rather
trivial situation where the device desc would say ECDT even when it was
not implicitly created via ECDT (not really sure why the compiler
doesn't seem to warn about this).

The other more pervasive issue is that the code is designed to
essentially not do anything for EC probe when its uid was already
created an EC based on the ECDT's uid. The issue was that probe would
still return 0 in this case, and so we'd end up with some weird
duplication. Now to be honest, I'm not actually sure what exactly broke,
but it was definitely not working as intended. To fix this, all that is
really needed is to make sure we return ENXIO when we're probing the
device already added for the ECDT entry. While here though, move the
check for this earlier to avoid wasted cycles when we know after
obtaining the uid that it's duplicative.

There remains one questionable bit here which I don't want to touch -
when doing probe for PNP0C09, if acquiring _UID for the device fails, 0
is assumed, which is a valid UID used by the implicit ECDT.

References: <CAFigVTNZku6B_H3aSENFNUvfGGNjw4TghoDdEoApDZ_me2fhGw@mail.gmail.com>

Test Plan

Posted to reporters on mailing list

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

So the device_disable() should have made the probe return of 0 vs ENXIO harmless since device_attach shouldn't have been called either way.

Ah, device_probe() only checks DF_ENABLED at the start. I can't decide if this is a bug or a feature. The result was that if probe returned 0 then the device_disable() was effectively ignored as the device was still attached.

sys/dev/acpica/acpi_ec.c
391 ↗(On Diff #51013)

peer != NULL is the preferred style (I realize the old code used this version I think)

This revision is now accepted and ready to land.Nov 26 2018, 6:57 PM
This revision was automatically updated to reflect the committed changes.