Hi,
On 4/15/23 15:24, Liav Albani wrote:
>
> On 4/15/23 13:12, Hans de Goede wrote:
>> Hi,
>>
>> On 4/14/23 20:02, Daniel Bertalan wrote:
>>> On the X380 Yoga, the `ECRD` and `ECWR` ACPI objects cannot be used for
>>> accessing the Embedded Controller: instead of a method that reads from
>>> the EC's memory, `ECRD` is the name of a location in high memory. This
>>> meant that trying to call them would fail with the following message:
>>>
>>> ACPI: \_SB.PCI0.LPCB.EC.ECRD: 1 arguments were passed to a non-method
>>> ACPI object (RegionField)
>>>
>>> With this commit, it is now possible to access the EC and read
>>> temperature and fan speed information. Note that while writes to the
>>> HFSP register do go through (as indicated by subsequent reads showing
>>> the new value), the fan does not actually change its speed.
>>>
>>> Signed-off-by: Daniel Bertalan <dani@xxxxxxxxxxxxxxxxxx>
>> Interestig, this looks like a pretty clean solution to me.
>
> Daniel and I have looked in the DSDT ASL code and found a bunch of registers in high physical memory location (which is an ACPI OpRegion),
> and one of the registers had a bit called ECRD.
> However, there were many other registers that might be interesting as well, the problem is the short names in the ASL code (so we only see abbreviations essentially).
>
>
> While I do agree that adding this code is indeed a clean solution, if you (people that are dealing with Thinkpad laptops) know about cleaner way to access the embedded controller, I think it's preferable, because this way Daniel might be able to trigger the fan on that laptop so it will actually spin and will dissipate-out heat from the system, without the relying on the embedded controller to get into some sort of thermal state and then to trigger the fan.
Have you tried falling back to the ec_read() and ec_write() helpers
exported by the ACPI EC code ?
The "first_ec" pointer used by these functions is exported too,
so you could try modifying thinkpad_acpi to use ec_read() and
ec_write() as fallback when first_ec is set (and the quirk
added by this patch triggers).
Regards,
Hans
>> Mark Pearson, do you have any remarks on this ?
>>
>> Regards,
>>
>> Hans
>>
>>
>>> ---
>>> drivers/platform/x86/thinkpad_acpi.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>> index 7191ff2625b1..6fe82f805ea8 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -11699,6 +11699,7 @@ static int __init thinkpad_acpi_module_init(void)
>>> {
>>> const struct dmi_system_id *dmi_id;
>>> int ret, i;
>>> + acpi_object_type obj_type;
>>>
>>> tpacpi_lifecycle = TPACPI_LIFE_INIT;
>>>
>>> @@ -11724,6 +11725,21 @@ static int __init thinkpad_acpi_module_init(void)
>>> TPACPI_ACPIHANDLE_INIT(ecrd);
>>> TPACPI_ACPIHANDLE_INIT(ecwr);
>>>
>>> + /*
>>> + * Quirk: in some models (e.g. X380 Yoga), an object named ECRD
>>> + * exists, but it is a register, not a method.
>>> + */
>>> + if (ecrd_handle) {
>>> + acpi_get_type(ecrd_handle, &obj_type);
>>> + if (obj_type != ACPI_TYPE_METHOD)
>>> + ecrd_handle = NULL;
>>> + }
>>> + if (ecwr_handle) {
>>> + acpi_get_type(ecwr_handle, &obj_type);
>>> + if (obj_type != ACPI_TYPE_METHOD)
>>> + ecwr_handle = NULL;
>>> + }
>>> +
>>> tpacpi_wq = create_singlethread_workqueue(TPACPI_WORKQUEUE_NAME);
>>> if (!tpacpi_wq) {
>>> thinkpad_acpi_module_exit();
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
