| Message ID | 20231120-j7200-usb-suspend-v2-4-038c7e4a3df4@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: cdns: fix suspend on J7200 by assuming reset-on-resume | expand |
On 20/11/2023 18:06, Théo Lebrun wrote: > Hardware initialisation is only done at probe. The J7200 USB controller > is reset at resume because of power-domains toggling off & on. We > therefore reconfigure the hardware at resume. > > Reuse the newly extracted cdns_ti_init_hw() function that contains the > register write sequence. > > Only focus J7200 as other SoC are untested. If the controller does not > reset we do not want to redo reg writes. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > drivers/usb/cdns3/cdns3-ti.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/cdns3/cdns3-ti.c b/drivers/usb/cdns3/cdns3-ti.c > index d4232b440e4e..84f93c2fcd5c 100644 > --- a/drivers/usb/cdns3/cdns3-ti.c > +++ b/drivers/usb/cdns3/cdns3-ti.c > @@ -58,6 +58,7 @@ struct cdns_ti { > struct clk *usb2_refclk; > struct clk *lpm_clk; > int usb2_refclk_rate_code; > + bool reset_on_resume; > }; > > static const int cdns_ti_rate_table[] = { /* in KHZ */ > @@ -172,6 +173,7 @@ static int cdns_ti_probe(struct platform_device *pdev) > data->usb2_refclk_rate_code = i; > data->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); > data->usb2_only = device_property_read_bool(dev, "ti,usb2-only"); > + data->reset_on_resume = of_device_is_compatible(node, "ti,j7200-usb"); No, use driver data for this. Don't put compatibles in the code. It makes it unmanageable soon. I am repeating this issue from time to time :/ Best regards, Krzysztof
Hello Krzysztof, On Mon Nov 20, 2023 at 6:31 PM CET, Krzysztof Kozlowski wrote: > On 20/11/2023 18:06, Théo Lebrun wrote: [...] > > --- a/drivers/usb/cdns3/cdns3-ti.c > > +++ b/drivers/usb/cdns3/cdns3-ti.c > > static const int cdns_ti_rate_table[] = { /* in KHZ */ > > @@ -172,6 +173,7 @@ static int cdns_ti_probe(struct platform_device *pdev) > > data->usb2_refclk_rate_code = i; > > data->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); > > data->usb2_only = device_property_read_bool(dev, "ti,usb2-only"); > > + data->reset_on_resume = of_device_is_compatible(node, "ti,j7200-usb"); > > No, use driver data for this. Don't put compatibles in the code. It > makes it unmanageable soon. Fixed for next revision. I hesitated on this patch but I'll know for next time. Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/drivers/usb/cdns3/cdns3-ti.c b/drivers/usb/cdns3/cdns3-ti.c index d4232b440e4e..84f93c2fcd5c 100644 --- a/drivers/usb/cdns3/cdns3-ti.c +++ b/drivers/usb/cdns3/cdns3-ti.c @@ -58,6 +58,7 @@ struct cdns_ti { struct clk *usb2_refclk; struct clk *lpm_clk; int usb2_refclk_rate_code; + bool reset_on_resume; }; static const int cdns_ti_rate_table[] = { /* in KHZ */ @@ -172,6 +173,7 @@ static int cdns_ti_probe(struct platform_device *pdev) data->usb2_refclk_rate_code = i; data->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider"); data->usb2_only = device_property_read_bool(dev, "ti,usb2-only"); + data->reset_on_resume = of_device_is_compatible(node, "ti,j7200-usb"); cdns_ti_init_hw(data); @@ -202,7 +204,26 @@ static void cdns_ti_remove(struct platform_device *pdev) platform_set_drvdata(pdev, NULL); } +#ifdef CONFIG_PM + +static int cdns_ti_resume(struct device *dev) +{ + struct cdns_ti *data = dev_get_drvdata(dev); + + if (data->reset_on_resume) + cdns_ti_init_hw(data); + + return 0; +} + +static const struct dev_pm_ops cdns_ti_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(NULL, cdns_ti_resume) +}; + +#endif /* CONFIG_PM */ + static const struct of_device_id cdns_ti_of_match[] = { + { .compatible = "ti,j7200-usb", }, { .compatible = "ti,j721e-usb", }, { .compatible = "ti,am64-usb", }, {}, @@ -213,8 +234,9 @@ static struct platform_driver cdns_ti_driver = { .probe = cdns_ti_probe, .remove_new = cdns_ti_remove, .driver = { - .name = "cdns3-ti", + .name = "cdns3-ti", .of_match_table = cdns_ti_of_match, + .pm = pm_ptr(&cdns_ti_pm_ops), }, };
Hardware initialisation is only done at probe. The J7200 USB controller is reset at resume because of power-domains toggling off & on. We therefore reconfigure the hardware at resume. Reuse the newly extracted cdns_ti_init_hw() function that contains the register write sequence. Only focus J7200 as other SoC are untested. If the controller does not reset we do not want to redo reg writes. Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> --- drivers/usb/cdns3/cdns3-ti.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)