usb: dwc3: core: Add shutdown callback for dwc3

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
 drivers/usb/dwc3/core.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.c +12 -7

index 94fdbe5..777b2b5 100644
@@ -1634,11 +1634,9 @@ static int dwc3_probe(struct platform_device *pdev)
        return ret;
 }
 
-static int dwc3_remove(struct platform_device *pdev)
+static void __dwc3_teardown(struct dwc3 *dwc)
 {
-       struct dwc3     *dwc = platform_get_drvdata(pdev);
-
-       pm_runtime_get_sync(&pdev->dev);
+       pm_runtime_get_sync(dwc->dev);
 
        dwc3_debugfs_exit(dwc);
        dwc3_core_exit_mode(dwc);

@@ -1646,19 +1644,32 @@ static int dwc3_remove(struct platform_device *pdev)
        dwc3_core_exit(dwc);
        dwc3_ulpi_exit(dwc);
 
-       pm_runtime_disable(&pdev->dev);
-       pm_runtime_put_noidle(&pdev->dev);
-       pm_runtime_set_suspended(&pdev->dev);
+       pm_runtime_disable(dwc->dev);
+       pm_runtime_put_noidle(dwc->dev);
+       pm_runtime_set_suspended(dwc->dev);
 
        dwc3_free_event_buffers(dwc);
        dwc3_free_scratch_buffers(dwc);
 
        if (dwc->usb_psy)
                power_supply_put(dwc->usb_psy);
+}
+
+static int dwc3_remove(struct platform_device *pdev)
+{
+       struct dwc3     *dwc = platform_get_drvdata(pdev);
+
+       __dwc3_teardown(dwc);

drivers/usb/dwc3/core.c +1 -0

 #ifdef CONFIG_PM
 static int dwc3_core_init_for_resume(struct dwc3 *dwc)
 {

@@ -1976,6 +1987,7 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
 static struct platform_driver dwc3_driver = {
        .probe          = dwc3_probe,
        .remove         = dwc3_remove,
+       .shutdown   = dwc3_shutdown,

Quoting Sandeep Maheswaram (2021-03-23 12:27:32)

Quoted 61 lines

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
  drivers/usb/dwc3/core.c | 26 +++++++++++++++++++-------
  1 file changed, 19 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.c +12 -7

index 94fdbe5..777b2b5 100644
@@ -1634,11 +1634,9 @@ static int dwc3_probe(struct platform_device *pdev)
         return ret;
  }
 
-static int dwc3_remove(struct platform_device *pdev)
+static void __dwc3_teardown(struct dwc3 *dwc)
  {
-       struct dwc3     *dwc = platform_get_drvdata(pdev);
-
-       pm_runtime_get_sync(&pdev->dev);
+       pm_runtime_get_sync(dwc->dev);
 
         dwc3_debugfs_exit(dwc);
         dwc3_core_exit_mode(dwc);

@@ -1646,19 +1644,32 @@ static int dwc3_remove(struct platform_device *pdev)
         dwc3_core_exit(dwc);
         dwc3_ulpi_exit(dwc);
 
-       pm_runtime_disable(&pdev->dev);
-       pm_runtime_put_noidle(&pdev->dev);
-       pm_runtime_set_suspended(&pdev->dev);
+       pm_runtime_disable(dwc->dev);
+       pm_runtime_put_noidle(dwc->dev);
+       pm_runtime_set_suspended(dwc->dev);
 
         dwc3_free_event_buffers(dwc);
         dwc3_free_scratch_buffers(dwc);
 
         if (dwc->usb_psy)
                 power_supply_put(dwc->usb_psy);
+}
+
+static int dwc3_remove(struct platform_device *pdev)
+{
+       struct dwc3     *dwc = platform_get_drvdata(pdev);
+
+       __dwc3_teardown(dwc);

drivers/usb/dwc3/core.c static int dwc3_remove(struct platform_device *pdev) · line 1672

Quoted 10 lines

drivers/usb/dwc3/core.c +6 -0

 
         return 0;
  }
 
+static void dwc3_shutdown(struct platform_device *pdev)
+{
+       struct dwc3     *dwc = platform_get_drvdata(pdev);
+
+       __dwc3_teardown(dwc);
+}

Can't this be

	static void dwc3_shutdown(struct platform_device *pdev)
	{
	       dwc3_remove(pdev);
	}

and then there's nothing else to change? Basically ignore return value of dwc3_remove() to make shutdown and remove harmonize. I also wonder if this is more common than we think and a struct driver flag could be set to say "call remove for shutdown" and then have driver core swizzle on that and save some duplicate functions.

Quoted 8 lines

drivers/usb/dwc3/core.c +1 -0

  #ifdef CONFIG_PM
  static int dwc3_core_init_for_resume(struct dwc3 *dwc)
  {

@@ -1976,6 +1987,7 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
  static struct platform_driver dwc3_driver = {
         .probe          = dwc3_probe,
         .remove         = dwc3_remove,
+       .shutdown   = dwc3_shutdown,

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address)

which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
  drivers/usb/dwc3/core.c | 26 +++++++++++++++++++-------
  1 file changed, 19 insertions(+), 7 deletions(-)

drivers/usb/dwc3/core.c +0 -0

index 94fdbe5..777b2b5 100644

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

thanks,

greg k-h

Quoted 18 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

Quoted 3 lines

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

Quoted 22 lines · Sandeep Maheswaram

Quoted 20 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

thanks,

Quoted 5 lines

Quoted 1 line

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

Quoted 30 lines · Greg Kroah-Hartman

Quoted 28 lines · Sandeep Maheswaram

Quoted 15 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Quoted 6 lines

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

Quoted 3 lines

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

What in-kernel user? Since when does shutdown callback need an in-kernel user? When you reboot or shutdown a system, it gets called. The reason why the shutdown callback is needed is provided in the commit text.

Quoted 35 lines · Sai Prakash Ranjan

Quoted 33 lines · Greg Kroah-Hartman

Quoted 20 lines · Sandeep Maheswaram

Quoted 18 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

Quoted 8 lines

Quoted 3 lines

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

Quoted 3 lines

What in-kernel user? Since when does shutdown callback need an in-kernel user? When you reboot or shutdown a system, it gets called. The reason why the shutdown callback is needed is provided in the commit text.

As I can't see the patch here, I have no idea...

Quoted 36 lines · Greg Kroah-Hartman

Quoted 35 lines · Sai Prakash Ranjan

Quoted 33 lines · Greg Kroah-Hartman

Quoted 20 lines · Sandeep Maheswaram

Quoted 18 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

Quoted 8 lines

Quoted 3 lines

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

Quoted 6 lines

Quoted 4 lines

What in-kernel user? Since when does shutdown callback need an in-kernel
user? When you reboot or shutdown a system, it gets called. The reason why the shutdown callback is needed is provided in the commit text.

As I can't see the patch here, I have no idea...

You are replying now to the same patch which adds this shutdown callback :) Anyways the qcom dwc3 driver patch which is abandoned which is also mentioned
in the commit text is here [1] and the new shutdown callback patch which we are both replying to is in here [2]

[1] https://lore.kernel.org/lkml/[email protected]/

[2] https://lore.kernel.org/lkml/[email protected]/

Quoted 50 lines · Sai Prakash Ranjan

Quoted 36 lines · Greg Kroah-Hartman

Quoted 35 lines · Sai Prakash Ranjan

Quoted 33 lines · Greg Kroah-Hartman

Quoted 20 lines · Sandeep Maheswaram

Quoted 18 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

Quoted 8 lines

Quoted 3 lines

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

Quoted 6 lines

Quoted 4 lines

What in-kernel user? Since when does shutdown callback need an in-kernel
user? When you reboot or shutdown a system, it gets called. The reason why the shutdown callback is needed is provided in the commit text.

As I can't see the patch here, I have no idea...

You are replying now to the same patch which adds this shutdown callback :) Anyways the qcom dwc3 driver patch which is abandoned which is also mentioned
in the commit text is here [1] and the new shutdown callback patch which we are both replying to is in here [2]

Quoted 3 lines

[1] https://lore.kernel.org/lkml/[email protected]/

[2] https://lore.kernel.org/lkml/[email protected]/

Thanks, so, what am I supposed to do here? The patch is long gone from my queue...

Quoted 44 lines · Sai Prakash Ranjan

Quoted 29 lines · Greg Kroah-Hartman

Quoted 28 lines · Sai Prakash Ranjan

Quoted 27 lines · Greg Kroah-Hartman

Quoted 26 lines · Sandeep Maheswaram

Quoted 15 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Quoted 6 lines

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

What in-kernel user? Since when does shutdown callback need an in-kernel
user? When you reboot or shutdown a system, it gets called. The reason why the shutdown callback is needed is provided in the commit text.

As I can't see the patch here, I have no idea...

You are replying now to the same patch which adds this shutdown callback :) Anyways the qcom dwc3 driver patch which is abandoned which is also mentioned
in the commit text is here [1] and the new shutdown callback patch which we are both replying to is in here [2]

Quoted 3 lines

[1] https://lore.kernel.org/lkml/[email protected]/

[2] https://lore.kernel.org/lkml/[email protected]/

Thanks, so, what am I supposed to do here? The patch is long gone from my queue...

greg k-h

Quoted 45 lines · Greg Kroah-Hartman

Quoted 44 lines · Sai Prakash Ranjan

Quoted 29 lines · Greg Kroah-Hartman

Quoted 28 lines · Sai Prakash Ranjan

Quoted 27 lines · Greg Kroah-Hartman

Quoted 26 lines · Sandeep Maheswaram

Quoted 15 lines · Greg Kroah-Hartman

Quoted 9 lines · Sandeep Maheswaram

This patch adds a shutdown callback to USB DWC core driver to ensure that it is properly shutdown in reboot/shutdown path. This is required where SMMU address translation is enabled like on SC7180
SoC and few others. If the hardware is still accessing memory after SMMU translation is disabled as part of SMMU shutdown callback in system reboot or shutdown path, then IOVAs(I/O virtual address) which it was using will go on the bus as the physical addresses which might result in unknown crashes (NoC/interconnect errors).

Quoted 5 lines

Previously this was added in dwc3 qcom glue driver.
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=382449
But observed kernel panic as glue driver shutdown getting called after iommu shutdown. As we are adding iommu nodes in dwc core node in device tree adding shutdown callback in core driver seems correct.

Quoted 6 lines

So shouldn't you also remove this from the qcom glue driver at the same time? Please submit both as a patch series.

thanks,

greg k-h

Hi Greg,

The qcom glue driver patch is not merged yet. I have just mentioned for it for reference.

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

You know that we can not add callbacks for no in-kernel user, so what good is this patch for now?

What in-kernel user? Since when does shutdown callback need an in-kernel
user? When you reboot or shutdown a system, it gets called. The reason why the shutdown callback is needed is provided in the commit text.

As I can't see the patch here, I have no idea...

You are replying now to the same patch which adds this shutdown callback :) Anyways the qcom dwc3 driver patch which is abandoned which is also mentioned
in the commit text is here [1] and the new shutdown callback patch which we are both replying to is in here [2]

Quoted 7 lines

Quoted 3 lines

[1] https://lore.kernel.org/lkml/[email protected]/

[2] https://lore.kernel.org/lkml/[email protected]/

Thanks, so, what am I supposed to do here? The patch is long gone from my queue...

greg k-h

Hi Greg,

Should I resend this patch ? If so let me know your about opinion about Stephen's comment on just calling dwc3_remove in

dwc3_shutdown and ignoring return value.

https://lore.kernel.org/patchwork/patch/1401242/#1599316