On Fri, May 6, 2022 at 8:31 AM Daehwan Jung <dh10.jung@xxxxxxxxxxx> wrote: > > This is for xHCI Host Controller driver on Exynos SOC. > It registers vendor ops before loading xhci platform driver. > > Signed-off-by: Daehwan Jung <dh10.jung@xxxxxxxxxxx> > --- > drivers/usb/dwc3/dwc3-exynos.c | 100 ++++++++++++++++++++++++++++++++- > 1 file changed, 99 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > index 0ecf20eeceee..c22ea5cd6ab0 100644 > --- a/drivers/usb/dwc3/dwc3-exynos.c > +++ b/drivers/usb/dwc3/dwc3-exynos.c > @@ -17,6 +17,12 @@ > #include <linux/of_platform.h> > #include <linux/regulator/consumer.h> > > +#include "core.h" > + > +#if IS_ENABLED(CONFIG_USB_XHCI_EXYNOS) > +int xhci_exynos_register_vendor_ops(void); > +#endif Function declarations should always be in a header file, and not guarded by an #ifdef. This particular one is probably not needed anyway if the driver is done correctly though, see below. > @@ -46,12 +53,81 @@ static int dwc3_exynos_remove_child(struct device *dev, void *unused) > return 0; > } > > +#if IS_ENABLED(CONFIG_USB_XHCI_EXYNOS) > +static int dwc3_exynos_host_init(struct dwc3_exynos *exynos) > +{ > + struct dwc3 *dwc = exynos->dwc; > + struct device *dev = exynos->dev; > + struct platform_device *xhci; > + struct resource *res; > + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); > + int ret = 0; > + > + /* Configuration xhci resources */ > + xhci_exynos_register_vendor_ops(); > + > + res = platform_get_resource(dwc3_pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(dev, "missing memory resource\n"); > + return -ENODEV; > + } > + dwc->xhci_resources[0].start = res->start; > + dwc->xhci_resources[0].end = dwc->xhci_resources[0].start + > + DWC3_XHCI_REGS_END; > + dwc->xhci_resources[0].flags = res->flags; > + dwc->xhci_resources[0].name = res->name; > + > + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, 0); > + if (!res) { > + dev_err(dev, "missing irq resource\n"); > + return -ENODEV; > + } > + > + dwc->xhci_resources[1].start = dwc->irq_gadget; > + dwc->xhci_resources[1].end = dwc->irq_gadget; > + dwc->xhci_resources[1].flags = res->flags; > + dwc->xhci_resources[1].name = res->name; > + > + xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO); > + if (!xhci) { > + dev_err(dwc->dev, "couldn't allocate xHCI device\n"); > + return -ENOMEM; > + } > + > + xhci->dev.parent = dwc->dev; > + ret = dma_set_mask_and_coherent(&xhci->dev, DMA_BIT_MASK(36)); > + if (ret) { > + pr_err("xhci dma set mask ret = %d\n", ret); > + return ret; > + } This looks like you have the abstraction backwards from what normal drivers do. If you need a specialization of a driver that already exists, create a new driver module with a platform_driver that matches the specialized of_device_id, and have it call into the more general driver, do avoid having the general driver know about the specializations. Allocating a platform_device and making it DMA capable doesn't generally work correctly, and misses the IOMMU setup, so make sure you have a device node for it instead and probe it from DT. Arnd