Zhao Yakui wrote:
Couple of concerns -- I think, I already mentioned one, but still...
1. By parsing DSDT unconditionally, we loose all performance (if any) benefits of
having ECDT table. If we just don't look into ECDT, we will save some efforts.
Agree with what you said. It is not appropriate to punish the system
that already works well. It will take more time to execute the function
of acpi_ec_ecdt_probe if ECDT table exists. OS can't confirm whether the
ECDT table is correct before this check. Only after the check is added,
we can know whether the info defined in ECDT table is reliable. So in my
patch it is assumed that the info defined in ECDT table is not
reliable.
So, either ECDT parse should go the way of do-do,
or DSDT parse in presence of ECDT should not happen.
I prefer second way -- as it is closer to the specification :)
2. We are speaking about broken (hopefully rare) BIOS -- it might not be wise to punish everyone (see #1).
How about adding check in acpi_ec_start(), that with the same GPE we found difference in addresses?
It is a good point to add the check in the acpi_ec_start. But in such
case there exists two issues:
a. OS will scan the ACPI namespace to build the ACPI device tree
before the ACPI ec driver is registered. As the EC device is already
initialized, maybe the EC will be accessed in the scanning of ACPI
device tree.(For example: in the _STA object of battery, AC Adapter). In
such case some warning message will be reported.
>ACPI: EC: input buffer is not empty, aborting transaction
>[18.125459] ACPI Exception (evregion-0420): AE_TIME, Returned by Handler
for [EmbeddedControl] [20070126]
Remember, we are speaking about broken hardware, it is OK to spill some
errors, while
we provide solution to user in the end. In this case only user of broken
hardware is punished, not everybody.
After the acpi ec driver is registered, the above message won't be reported again.
But it seems very confusing.
b. On the laptops of bug 11880 all the info in ECDT table is incorrect.
The EC GPE number and EC namepath are incorrect besides the EC Cmd/Status/Data I/O port.
As there exists the ECDT table, the EC device will be initialized and the incorrect
EC GPE is enabled before building ACPI device tree. In such case although the EC address
info can be corrected by calling the ec_parse_device in acpi_ec_start, the EC GPE can't be
corrected(Still the incorrect GPE number is enabled for EC).
You could insert some check like if ("errors in boot_ec" ||
"acpi_ec_start does not find boot ec"), print same advice...
And if this check triggers, print "Your ECDT does not conform to DSDT. Add acpi_ec.skip_ecdt=1 kernel option."
Add acpi_ec.skip_ecdt option and skip ECDT parse if it is set...
3. This patch seems to imply that DSDT contains single EC. As this is first place with such assumption,
Yes. What you said is right. One EC device is assumed in my patch.
In theory there exists more than one EC device. But it seems that now no
laptops have more than one EC devices. Can this case be ignored?
I have notebook with 2 EC on my desk at the moment, made by FIC/VIA.
I don't like to break working hardware to enable broken.
Regards,
Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
