linux-kernel - Re: [PATCH] platform/chrome: cros_ec_typec: deferred probe when typec count mismatch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20221221070122.4167-1-zhouruihai@huaqin.corp-partner.google.com>
Date:   Wed, 21 Dec 2022 15:01:22 +0800
From:   Ruihai Zhou <zhouruihai@...qin.corp-partner.google.com>
To:     pmalani@...omium.org, bleung@...omium.org, groeck@...omium.org,
        knoxchiou@...omium.org, weishunc@...omium.org
Cc:     chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] platform/chrome: cros_ec_typec: deferred probe when typec count mismatch

>>> Is this really seen in the field ? The EC should never report a wrong
>>> (random) number of ports. If it is not ready, there should be _some_
>>> indication that it isn't ready. Does it really report a more or less
>>> random number in this case ?
>>
>>  Yes, I saw this on corsola board. The EC report a wrong number(not random), because corsola emulates HDMI MUX over the current
>>  type-c mux stack. The ec has to fake a type-c port to pass the MUX info. But the task are not initiated on starting up, and increase the
>>  type-c port counts after the tasks finished. In this case, I saw the typec->num_ports = 1, but the nports = 2, which will be probe failed and
>>  block the HDMI mux function.
>>
>>  I will send v2 patch, if nports > EC_USB_PD_MAX_PORTS, then return -EINVAL, but if nports > typec->num_ports, we consider wait a second
>>  to ec task increase the type-c port counts if there're a HDMI DB attach, then return -EPROBE_DEFER
>
> The current code just reduces nports if it is larger than
> EC_USB_PD_MAX_PORTS. Again, I don't see a reason to change that.
>
> Anyway, I am not sure if the above will work reliably. I am not sure
> what "HDMI DB" refers to, but if it is an external connector its
> status could change anytime. In that situation, no amount of waiting
> would help. Either case, the EC on corsola should really not change
> the number of ports it supports. Either it is a constant that should
> not change, or it is dynamic and the EC would need to tell the host if
> the number of ports changes (up or down). Trying to fix this in
> cros_ec_typec without well defined protocol exchange with the EC is at
> best a kludge, but not a real solution.

Thank you for the reply. The "HDMI DB" refer to the daughter board
attached to the mainboard, which don't change anytime. In fact,
the corsola EC increase the port count dynamically with some delay
(no see a standard yet) during bootup. There is the EC code to
increase the port count [1]. If the cros-ec-typec get the type-c
port counts from EC before the EC increase the port counts, then
will probe failed if return -EINVAL. I think the cros-ec-typec
is replying on the fragile delay in EC, cros-ec-typec module will
not get the fake(HDMI) type-c port counts once the EC deferred
call is later than the driver probe. That is why I make this
change. For the verification, the driver always probe failed
when EC reboot without the patch. But the driver probe passed with
the patch after a few times deferred probe.

Link:
[1] https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/program/corsola/src/usbc.c;l=47