linux-kernel - Re: [PATCH] usb: typec: tpcm: Fix issues with power being removed during reset

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: <CAPTae5LOresaK3Epd4R_ifpx2kw21tubQDENg4OMrhJWsar-oA@mail.gmail.com>
Date: Tue, 13 Feb 2024 02:09:16 -0800
From: Badhri Jagan Sridharan <badhri@...gle.com>
To: Mark Brown <broonie@...nel.org>
Cc: Guenter Roeck <linux@...ck-us.net>, Heikki Krogerus <heikki.krogerus@...ux.intel.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Gábor Stefanik <netrolller.3d@...il.com>, 
	rdbabiera@...gle.com, amitsd@...gle.com, linux-usb@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: typec: tpcm: Fix issues with power being removed
 during reset

Hi Mark,

While HI-Zing CC pins disrupts power for batteryless devices, not
Hi-Zing CC pins would prevent clean error recovery for self powered
devices which is why "usb: typec: tcpm: fix cc role at port reset" was reverted.
Please note that the breakage in error recovery behavior is a
regression as well.
Hi-Zing CC pins would make the port partner recognize it as disconnect
and will result in bringup the connection back cleanly.

How about leveraging "self-powered" device tree property and Hi-Zing
CC pins only when using "self-powered" ?
This should help devices which don't have batteries while NOT regressing
the error recovery behavior for the self powered devices.

--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4897,7 +4897,11 @@ static void run_state_machine(struct tcpm_port *port)
                break;
        case PORT_RESET:
                tcpm_reset_port(port);
-               tcpm_set_cc(port, TYPEC_CC_OPEN);
+               if (port->self_powered)
+                       tcpm_set_cc(port, TYPEC_CC_OPEN);
+               else
+                       tcpm_set_cc(port, tcpm_default_state(port) ==
SNK_UNATTACHED ?
+                               TYPEC_CC_RD : tcpm_rp_cc(port));
                tcpm_set_state(port, PORT_RESET_WAIT_OFF,
                               PD_T_ERROR_RECOVERY);

Thanks,
Badhri


On Mon, Feb 12, 2024 at 10:42 AM Mark Brown <broonie@...nel.org> wrote:
>
> Since the merge of b717dfbf73e8 ("Revert "usb: typec: tcpm: fix
> cc role at port reset"") into mainline the LibreTech Renegade
> Elite/Firefly has died during boot, the main symptom observed in testing
> is a sudden stop in console output.  Gábor Stefanik identified in review
> that the patch would cause power to be removed from devices without
> batteries (like this board), observing that while the patch is correct
> according to the spec this appears to be an oversight in the spec.
>
> Given that the change makes previously working systems unusable let's
> revert it, there was some discussion of identifying systems that have
> alternative power and implementing the standards conforming behaviour in
> only that case.
>
> Fixes: b717dfbf73e8 ("Revert "usb: typec: tcpm: fix cc role at port reset"")
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index f7d7daa60c8d..a0978ed1a257 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4876,7 +4876,8 @@ static void run_state_machine(struct tcpm_port *port)
>                 break;
>         case PORT_RESET:
>                 tcpm_reset_port(port);
> -               tcpm_set_cc(port, TYPEC_CC_OPEN);
> +               tcpm_set_cc(port, tcpm_default_state(port) == SNK_UNATTACHED ?
> +                           TYPEC_CC_RD : tcpm_rp_cc(port));
>                 tcpm_set_state(port, PORT_RESET_WAIT_OFF,
>                                PD_T_ERROR_RECOVERY);
>                 break;
>
> ---
> base-commit: 841c35169323cd833294798e58b9bf63fa4fa1de
> change-id: 20240212-usb-fix-renegade-837d35cfc0c2
>
> Best regards,
> --
> Mark Brown <broonie@...nel.org>
>