diff mbox series

[v5,07/12] usb: cdns3-ti: add J7200 support with reset-on-resume behavior

Message ID 20240726-s2r-cdns-v5-7-8664bfb032ac@bootlin.com (mailing list archive)
State New
Headers show
Series Fix USB suspend on TI J7200 (cdns3-ti, cdns3, xhci) | expand

Commit Message

Théo Lebrun July 26, 2024, 6:17 p.m. UTC
Add ti,j7200-usb compatible. Match data indicates the controller resets
on resume which tells that to the cdns3 core. This in turn silences a
xHCI warning visible in cases of unexpected resets.

We also inherit the errata quirk CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE from
the default `cdns_ti_auxdata` configuration.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/usb/cdns3/cdns3-ti.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Roger Quadros Aug. 5, 2024, 1:54 p.m. UTC | #1
On 26/07/2024 21:17, Théo Lebrun wrote:
> Add ti,j7200-usb compatible. Match data indicates the controller resets
> on resume which tells that to the cdns3 core. This in turn silences a
> xHCI warning visible in cases of unexpected resets.
> 
> We also inherit the errata quirk CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE from
> the default `cdns_ti_auxdata` configuration.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/usb/cdns3/cdns3-ti.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/cdns3-ti.c b/drivers/usb/cdns3/cdns3-ti.c
> index 159814dfc856..65b8b6f4c654 100644
> --- a/drivers/usb/cdns3/cdns3-ti.c
> +++ b/drivers/usb/cdns3/cdns3-ti.c
> @@ -258,7 +258,21 @@ static const struct of_dev_auxdata cdns_ti_auxdata[] = {
>  	{},
>  };
>  
> +static struct cdns3_platform_data cdns_ti_j7200_pdata = {
> +	.quirks = CDNS3_RESET_ON_RESUME |

But you mentioned that the behavior can be different based on which
idle state the system went into.
Setting this flag will means Reset is required on every resume.


Instead, you just need to rely on the runtime check and set the xhci->lost_power flag at resume.


> +		  CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE,   /* Errata i2409 */
> +};
> +
> +static const struct of_dev_auxdata cdns_ti_j7200_auxdata[] = {
> +	{
> +		.compatible = "cdns,usb3",
> +		.platform_data = &cdns_ti_j7200_pdata,
> +	},
> +	{},
> +};
> +
>  static const struct of_device_id cdns_ti_of_match[] = {
> +	{ .compatible = "ti,j7200-usb", .data = cdns_ti_j7200_auxdata },
>  	{ .compatible = "ti,j721e-usb", .data = cdns_ti_auxdata },
>  	{ .compatible = "ti,am64-usb", .data = cdns_ti_auxdata },
>  	{},
>
Théo Lebrun Sept. 10, 2024, 1:57 p.m. UTC | #2
On Mon Aug 5, 2024 at 3:54 PM CEST, Roger Quadros wrote:
> On 26/07/2024 21:17, Théo Lebrun wrote:
> > Add ti,j7200-usb compatible. Match data indicates the controller resets
> > on resume which tells that to the cdns3 core. This in turn silences a
> > xHCI warning visible in cases of unexpected resets.
> > 
> > We also inherit the errata quirk CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE from
> > the default `cdns_ti_auxdata` configuration.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  drivers/usb/cdns3/cdns3-ti.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/usb/cdns3/cdns3-ti.c b/drivers/usb/cdns3/cdns3-ti.c
> > index 159814dfc856..65b8b6f4c654 100644
> > --- a/drivers/usb/cdns3/cdns3-ti.c
> > +++ b/drivers/usb/cdns3/cdns3-ti.c
> > @@ -258,7 +258,21 @@ static const struct of_dev_auxdata cdns_ti_auxdata[] = {
> >  	{},
> >  };
> >  
> > +static struct cdns3_platform_data cdns_ti_j7200_pdata = {
> > +	.quirks = CDNS3_RESET_ON_RESUME |
>
> But you mentioned that the behavior can be different based on which
> idle state the system went into.
> Setting this flag will means Reset is required on every resume.

No, this flag's only behavior is to enable XHCI_RESET_ON_RESUME in the
lower xHCI stack. Code is in __cdns_host_init():

	if (cdns->pdata && (cdns->pdata->quirks & CDNS3_RESET_ON_RESUME))
		cdns->xhci_plat_data->quirks |= XHCI_RESET_ON_RESUME;

> Instead, you just need to rely on the runtime check and set the xhci->lost_power flag at resume.

I argued about my view of the XHCI_RESET_ON_RESUME quirk under
[PATCH v5 09/12].
https://lore.kernel.org/lkml/D42NIH63EHZG.KKWZR2WZB68L@bootlin.com/

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
diff mbox series

Patch

diff --git a/drivers/usb/cdns3/cdns3-ti.c b/drivers/usb/cdns3/cdns3-ti.c
index 159814dfc856..65b8b6f4c654 100644
--- a/drivers/usb/cdns3/cdns3-ti.c
+++ b/drivers/usb/cdns3/cdns3-ti.c
@@ -258,7 +258,21 @@  static const struct of_dev_auxdata cdns_ti_auxdata[] = {
 	{},
 };
 
+static struct cdns3_platform_data cdns_ti_j7200_pdata = {
+	.quirks = CDNS3_RESET_ON_RESUME |
+		  CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE,   /* Errata i2409 */
+};
+
+static const struct of_dev_auxdata cdns_ti_j7200_auxdata[] = {
+	{
+		.compatible = "cdns,usb3",
+		.platform_data = &cdns_ti_j7200_pdata,
+	},
+	{},
+};
+
 static const struct of_device_id cdns_ti_of_match[] = {
+	{ .compatible = "ti,j7200-usb", .data = cdns_ti_j7200_auxdata },
 	{ .compatible = "ti,j721e-usb", .data = cdns_ti_auxdata },
 	{ .compatible = "ti,am64-usb", .data = cdns_ti_auxdata },
 	{},