Re: [PATCH v2 4/7] usb: dwc3: core: Expose core driver as library

Krishna Kurapati
2024-08-19T17:48:49Z

On 8/12/2024 8:42 AM, Bjorn Andersson wrote:
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
>
> The DWC3 IP block is handled by three distinct device drivers: XHCI,
> DWC3 core and a platform specific (optional) DWC3 glue driver.
>
> This has resulted in, at least in the case of the Qualcomm glue, the
> presence of a number of layering violations, where the glue code either
> can't handle, or has to work around, the fact that core might not probe
> deterministically.
>
> An example of this is that the suspend path should operate slightly
> different depending on the device operating in host or peripheral mode,
> and the only way to determine the operating state is to peek into the
> core's drvdata.
>
> The Qualcomm glue driver is expected to make updates in the qscratch
> register region (the "glue" region) during role switch events, but with
> the glue and core split using the driver model, there is no reasonable
> way to introduce listeners for mode changes.
>
> Split the dwc3 core platform_driver callbacks and their implementation
> and export the implementation, to make it possible to deterministically
> instantiate the dwc3 core as part of the dwc3 glue drivers and to
> allow flattening of the DeviceTree representation.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>

...

> -static int dwc3_probe(struct platform_device *pdev)
> +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> + bool ignore_clocks_and_resets, void *glue)
> {
> struct device *dev = &pdev->dev;
> - struct resource *res, dwc_res;
> + struct resource dwc_res;
> unsigned int hw_mode;
> void __iomem *regs;
> struct dwc3 *dwc;
> @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
>
> dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
> if (!dwc)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> dwc->dev = dev;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!res) {
> - dev_err(dev, "missing memory resource\n");
> - return -ENODEV;
> - }

...

> +static int dwc3_plat_probe(struct platform_device *pdev)
> {
> - struct dwc3 *dwc = platform_get_drvdata(pdev);
> + struct resource *res;
> + struct dwc3 *dwc;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "missing memory resource\n");
> + return -ENODEV;
> + }
>
> - pm_runtime_get_sync(&pdev->dev);
> + dwc = dwc3_probe(pdev, res, false, NULL);
> + if (IS_ERR(dwc))
> + return PTR_ERR(dwc);
> +
> + platform_set_drvdata(pdev, dwc);

This setting of platform drvdata is redundant I believe. We already do
it in dwc3_probe.

> +
> + return 0;
> +}
> +