Re: [PATCH 2/5] platform: arm64: add Huawei Matebook E Go (sc8280xp) EC driver

On Tue, Dec 31, 2024 at 03:44:36PM +0800, Pengyu Luo wrote:

On Tue, Dec 31, 2024 at 1:00 PM Aiqun(Maria) Yu [off-list ref] wrote:

quoted

On 12/30/2024 6:44 PM, Pengyu Luo wrote:

quoted

On Mon, Dec 30, 2024 at 5:04 PM Aiqun(Maria) Yu [off-list ref] wrote:

quoted

On 12/28/2024 1:13 AM, Pengyu Luo wrote:

[...]

quoted

quoted

quoted

+     i2c_transfer(client->adapter, msgs, 2);

ARRAY_SIZE(msgs) is suggested instead of pure 2.

Agree

quoted

quoted

+     usleep_range(2000, 2500);

Why is a sleep needed here? Is this information specified in any datasheet?

Have a break between 2 transaction. This sleep happens in acpi code, also
inside a critical region. I rearranged it.

Local7 = Acquire (\_SB.IC16.MUEC, 0x03E8)
...
write ops
...
Sleep (0x02)
...
read ops
...
Release (\_SB.IC16.MUEC)

Could you please share the exact code snippet that is being referenced?
I'm a bit confused because it doesn't seem to align with the current
logic, which doesn't have read operations within the same mutex lock. I
also want to understand the background and necessity of the sleep function.

I mentioned I rearranged it to optimize it. In a EC transaction,
write sleep read => write read sleep, in this way, we sleep once a
transaction.

Sleeping between write and read is logical: it provides EC some time to
respond. Sleeping after read is complete doesn't seem to have any
reason.

Please search
'device name + acpi table' on the internet, someone dumped it and uploaded
it, in SSDT, check ECCD. I am not sure if huawei allows users to dump it.
So I don't provide it here.

-- 
With best wishes
Dmitry