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 <d...@danielbertalan.dev>
>> 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@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
