On Tue, Apr 18, 2023 at 05:48:35PM +0530, Udipto Goswami wrote:
First, the subject of this patch looks wrong.
> When the dwc3 device is runtime suspended, various required clocks would
> get disabled and it is not guaranteed that access to any registers would
> work. Depending on the SoC glue, a register read could be as benign as
> returning 0 or be fatal enough to hang the system.
>
> In order to prevent such scenarios of fatal errors, make sure to resume
> dwc3 then allow the function to proceed.
>
> Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
> Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
I did not sign-off on this patch (and neither did Oliver I presume).
You must never add a SoB on behalf on someone else without their
permission. Please read Documentation/process/submitting-patches.rst and
make sure you understand what SoB means before sending any further
patches.
> Signed-off-by: Udipto Goswami <quic_ugoswami@xxxxxxxxxxx>
> ---
> v5: Reworked the patch to resume dwc3 while accessing the registers.
Please include the full changelog for v6 (and future patches).
> drivers/usb/dwc3/debugfs.c | 123 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 123 insertions(+)
>
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index e4a2560b9dc0..d622b0dfeb76 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -332,6 +332,13 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
> unsigned int current_mode;
> unsigned long flags;
> u32 reg;
> + int ret;
> +
> + ret = pm_runtime_get_sync(dwc->dev);
> + if (!ret || ret < 0) {
This is broken as you would not print anything on success (ret == 0).
Did you test this patch at all?
> + pm_runtime_put(dwc->dev);
> + return 0;
Why do you return 'success' on errors?
> + }
Johan
