linux-kernel - Re: [PATCH v5 00/12] Fix USB suspend on TI J7200 (cdns3-ti, cdns3, xhci)

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: <2bf38b29-6aac-4c08-837f-a43683a54b56@kernel.org>
Date: Mon, 5 Aug 2024 17:01:09 +0300
From: Roger Quadros <rogerq@...nel.org>
To: Théo Lebrun <theo.lebrun@...tlin.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Peter Chen <peter.chen@...nel.org>,
 Pawel Laszczak <pawell@...ence.com>, Mathias Nyman
 <mathias.nyman@...el.com>, Nishanth Menon <nm@...com>,
 Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
 "Vardhan, Vibhore" <vibhore@...com>, Vishal Mahaveer <vishalm@...com>
Cc: linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 Kevin Hilman <khilman@...nel.org>,
 Grégory Clement <gregory.clement@...tlin.com>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Conor Dooley <conor.dooley@...rochip.com>
Subject: Re: [PATCH v5 00/12] Fix USB suspend on TI J7200 (cdns3-ti, cdns3,
 xhci)

+Vibhore & Vishal

On 05/08/2024 11:58, Théo Lebrun wrote:
> Hello Roger,
> 
> On Sat Aug 3, 2024 at 5:14 PM CEST, Roger Quadros wrote:
>> On 26/07/2024 21:17, Théo Lebrun wrote:
>>> Currently, system-wide suspend is broken on J7200 because of a
>>> controller reset. The TI wrapper does not get re-initialised at resume
>>> and the first register access from cdns core fails.
>>>
>>> We address that in a few ways:
>>>  - In cdns3-ti, if a reset has occured at resume, we reconfigure the HW.
>>>  - We pass the XHCI_RESET_ON_RESUME quirk, meaning the XHCI core expects
>>>    a resume.
>>
>> OK.
>>>  - We add a xhci->lost_power flag.
>>
>> Why?
>>
>>>
>>> The previous revision had one big issue: we had to know if
>>> reset-on-resume was true, at probe-time. This is where the main
>>
>> Don't we already know this at probe-time? why not just rely on the existing
>> XHCI_RESET_ON_RESUME qurik, than add a new mechanism?
> 
> Some TI platforms cannot tell, before going to suspend, if their USB
> controller will reset. Suspend behavior is defined by (at least) two
> features:
> 
>  - Power domains. See arch/arm64/boot/dts/ti/k3-j7200-main.dtsi:
> 
>    usbss0: cdns-usb@...4000 {
>       compatible = "ti,j7200-usb", "ti,j721e-usb";
>       // ...
>       power-domains = <&k3_pds 288 TI_SCI_PD_EXCLUSIVE>;
>       // ...
>    };
> 
>    This `power-domains` property implies that even s2idle will reset
>    the controller.

I'm not so sure. All K3 platforms have the power-domains property for
the USB wrapper nodes.

> 
>  - Deep suspend. The type of suspend defines what will happen to various
>    controllers. Currently deep suspend is suspend-to-RAM, with the SoC
>    being turned off.
> 
>    This might evolve over time with more complex rules: the chosen
>    suspend behavior could depend on wakeup source and/or wakeup target
>    latencies. That information might not be available to Linux, being
>    decided upon by firmwares. We need to be able to resume successfully
>    without being surprised by a reset.
> 

Got it. Might be worth to mention this in the patch description.

> I am sorry the precise usecase wasn't clear from the get-go.
> 
> Thanks,
> 
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

-- 
cheers,
-roger