From: Peter Chen <peter.chen@cixtech.com>
To: Pawel Laszczak <pawell@cadence.com>
Cc: "robh@kernel.org" <robh@kernel.org>,
"krzk+dt@kernel.org" <krzk+dt@kernel.org>,
"conor+dt@kernel.org" <conor+dt@kernel.org>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"rogerq@kernel.org" <rogerq@kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"cix-kernel-upstream@cixtech.com"
<cix-kernel-upstream@cixtech.com>
Subject: Re: [PATCH v2 2/2] usb: cdns3: Add USBSSP platform driver support
Date: Wed, 25 Mar 2026 15:35:00 +0800 [thread overview]
Message-ID: <acOQJMLwyT7QtCIQ@nchen-desktop> (raw)
In-Reply-To: <PH7PR07MB9538B73EB7746CED9A094447DD48A@PH7PR07MB9538.namprd07.prod.outlook.com>
On 26-03-24 12:10:43, Pawel Laszczak wrote:
> >
> >+static int cdns3_plat_gadget_init(struct cdns *cdns)
> >+{
> >+ if (cdns->version < CDNSP_CONTROLLER_V2)
> >+ return cdns3_gadget_init(cdns);
> >+ else
> >+ return cdnsp_gadget_init(cdns);
> >+}
> >+
> >@@ -143,12 +153,15 @@ static int cdns3_plat_probe(struct platform_device
> >*pdev)
> > if (ret)
> > goto err_phy_power_on;
> >
> >- cdns->gadget_init = cdns3_gadget_init;
> >-
> > ret = cdns_init(cdns);
> > if (ret)
> > goto err_cdns_init;
> >
> >+ cdns->gadget_init = cdns3_plat_gadget_init;
>
> The call to the following function could have been placed in cdns_init.
> Then assigned of gadget_init should be above of cdns_init.
No. It needs to know cdns->version before assigning gadget_init to know correct
.gadget_init for USBSS or USBSSP. And cdns->version is decided at cdns_init,
and former cdns_init API includes cdns_core_init_role which calls cdns->gadget_init.
Peter
>
> Besides,
> Acked-by: Pawel Laszczak <pawell@cadence.com>
>
> Pawel,
>
> >+ ret = cdns_core_init_role(cdns);
> >+ if (ret)
> >+ goto err_cdns_init;
> >+
> > device_set_wakeup_capable(dev, true);
> > pm_runtime_set_active(dev);
> > pm_runtime_enable(dev);
> >diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-
> >gadget.c
> >index 6b3815f8a6e5..8db7eee528a1 100644
> >--- a/drivers/usb/cdns3/cdnsp-gadget.c
> >+++ b/drivers/usb/cdns3/cdnsp-gadget.c
> >@@ -2075,3 +2075,7 @@ int cdnsp_gadget_init(struct cdns *cdns)
> >
> > return 0;
> > }
> >+EXPORT_SYMBOL_GPL(cdnsp_gadget_init);
> >+
> >+MODULE_LICENSE("GPL");
> >+MODULE_DESCRIPTION("Cadence CDNSP DRD Driver - gadget");
> >diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
> >index 566d94e49102..432007cfe695 100644
> >--- a/drivers/usb/cdns3/cdnsp-pci.c
> >+++ b/drivers/usb/cdns3/cdnsp-pci.c
> >@@ -1,6 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /*
> >- * Cadence PCI Glue driver.
> >+ * Cadence USBSSP PCI Glue driver.
> > *
> > * Copyright (C) 2019 Cadence.
> > *
> >@@ -16,7 +16,19 @@
> > #include <linux/pci.h>
> >
> > #include "core.h"
> >-#include "gadget-export.h"
> >+
> >+struct cdnsp_wrap {
> >+ struct platform_device *plat_dev;
> >+ struct resource dev_res[6];
> >+ int devfn;
> >+};
> >+
> >+#define RES_IRQ_HOST_ID 0
> >+#define RES_IRQ_PERIPHERAL_ID 1
> >+#define RES_IRQ_OTG_ID 2
> >+#define RES_HOST_ID 3
> >+#define RES_DEV_ID 4
> >+#define RES_DRD_ID 5
> >
> > #define PCI_BAR_HOST 0
> > #define PCI_BAR_OTG 0
> >@@ -26,16 +38,16 @@
> > #define PCI_DEV_FN_OTG 1
> >
> > #define PCI_DRIVER_NAME "cdns-pci-usbssp"
> >-#define PLAT_DRIVER_NAME "cdns-usbssp"
> >+#define PLAT_DRIVER_NAME "cdns-usb3"
> >
> >-#define CHICKEN_APB_TIMEOUT_VALUE 0x1C20
> >+#define CHICKEN_APB_TIMEOUT_VALUE 0x1C20
> >
> > static struct pci_dev *cdnsp_get_second_fun(struct pci_dev *pdev)
> > {
> > /*
> > * Gets the second function.
> >- * Platform has two function. The fist keeps resources for
> >- * Host/Device while the secon keeps resources for DRD/OTG.
> >+ * Platform has two function. The first keeps resources for
> >+ * Host/Device while the second keeps resources for DRD/OTG.
> > */
> > if (pdev->device == PCI_DEVICE_ID_CDNS_USBSSP)
> > return pci_get_device(pdev->vendor,
> >PCI_DEVICE_ID_CDNS_USBSS, NULL);
> >@@ -48,11 +60,12 @@ static struct pci_dev *cdnsp_get_second_fun(struct
> >pci_dev *pdev)
> > static int cdnsp_pci_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> > {
> >- struct device *dev = &pdev->dev;
> >- struct pci_dev *func;
> >+ struct platform_device_info plat_info;
> >+ static struct cdns3_platform_data pdata;
> >+ struct cdnsp_wrap *wrap;
> > struct resource *res;
> >- struct cdns *cdnsp;
> >- int ret;
> >+ struct pci_dev *func;
> >+ int ret = 0;
> >
> > /*
> > * For GADGET/HOST PCI (devfn) function number is 0,
> >@@ -79,146 +92,105 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
> > }
> >
> > pci_set_master(pdev);
> >+
> > if (pci_is_enabled(func)) {
> >- cdnsp = pci_get_drvdata(func);
> >+ wrap = pci_get_drvdata(func);
> > } else {
> >- cdnsp = kzalloc_obj(*cdnsp);
> >- if (!cdnsp) {
> >+ wrap = kzalloc_obj(*wrap);
> >+ if (!wrap) {
> > ret = -ENOMEM;
> > goto put_pci;
> > }
> > }
> >
> >- /* For GADGET device function number is 0. */
> >- if (pdev->devfn == 0) {
> >- resource_size_t rsrc_start, rsrc_len;
> >-
> >- /* Function 0: host(BAR_0) + device(BAR_1).*/
> >- dev_dbg(dev, "Initialize resources\n");
> >- rsrc_start = pci_resource_start(pdev, PCI_BAR_DEV);
> >- rsrc_len = pci_resource_len(pdev, PCI_BAR_DEV);
> >- res = devm_request_mem_region(dev, rsrc_start, rsrc_len,
> >"dev");
> >- if (!res) {
> >- dev_dbg(dev, "controller already in use\n");
> >- ret = -EBUSY;
> >- goto free_cdnsp;
> >- }
> >-
> >- cdnsp->dev_regs = devm_ioremap(dev, rsrc_start, rsrc_len);
> >- if (!cdnsp->dev_regs) {
> >- dev_dbg(dev, "error mapping memory\n");
> >- ret = -EFAULT;
> >- goto free_cdnsp;
> >- }
> >-
> >- cdnsp->dev_irq = pdev->irq;
> >- dev_dbg(dev, "USBSS-DEV physical base addr: %pa\n",
> >- &rsrc_start);
> >-
> >- res = &cdnsp->xhci_res[0];
> >- res->start = pci_resource_start(pdev, PCI_BAR_HOST);
> >- res->end = pci_resource_end(pdev, PCI_BAR_HOST);
> >- res->name = "xhci";
> >- res->flags = IORESOURCE_MEM;
> >- dev_dbg(dev, "USBSS-XHCI physical base addr: %pa\n",
> >- &res->start);
> >-
> >- /* Interrupt for XHCI, */
> >- res = &cdnsp->xhci_res[1];
> >- res->start = pdev->irq;
> >- res->name = "host";
> >- res->flags = IORESOURCE_IRQ;
> >+ res = wrap->dev_res;
> >+
> >+ if (pdev->devfn == PCI_DEV_FN_HOST_DEVICE) {
> >+ /* Function 0: host(BAR_0) + device(BAR_2). */
> >+ dev_dbg(&pdev->dev, "Initialize Device resources\n");
> >+ res[RES_DEV_ID].start = pci_resource_start(pdev,
> >PCI_BAR_DEV);
> >+ res[RES_DEV_ID].end = pci_resource_end(pdev,
> >PCI_BAR_DEV);
> >+ res[RES_DEV_ID].name = "dev";
> >+ res[RES_DEV_ID].flags = IORESOURCE_MEM;
> >+ dev_dbg(&pdev->dev, "USBSSP-DEV physical base addr:
> >%pa\n",
> >+ &res[RES_DEV_ID].start);
> >+
> >+ res[RES_HOST_ID].start = pci_resource_start(pdev,
> >PCI_BAR_HOST);
> >+ res[RES_HOST_ID].end = pci_resource_end(pdev,
> >PCI_BAR_HOST);
> >+ res[RES_HOST_ID].name = "xhci";
> >+ res[RES_HOST_ID].flags = IORESOURCE_MEM;
> >+ dev_dbg(&pdev->dev, "USBSSP-XHCI physical base addr:
> >%pa\n",
> >+ &res[RES_HOST_ID].start);
> >+
> >+ /* Interrupt for XHCI */
> >+ wrap->dev_res[RES_IRQ_HOST_ID].start = pdev->irq;
> >+ wrap->dev_res[RES_IRQ_HOST_ID].name = "host";
> >+ wrap->dev_res[RES_IRQ_HOST_ID].flags = IORESOURCE_IRQ;
> >+
> >+ /* Interrupt for device. It's the same as for HOST. */
> >+ wrap->dev_res[RES_IRQ_PERIPHERAL_ID].start = pdev->irq;
> >+ wrap->dev_res[RES_IRQ_PERIPHERAL_ID].name =
> >"peripheral";
> >+ wrap->dev_res[RES_IRQ_PERIPHERAL_ID].flags =
> >IORESOURCE_IRQ;
> > } else {
> >- res = &cdnsp->otg_res;
> >- res->start = pci_resource_start(pdev, PCI_BAR_OTG);
> >- res->end = pci_resource_end(pdev, PCI_BAR_OTG);
> >- res->name = "otg";
> >- res->flags = IORESOURCE_MEM;
> >- dev_dbg(dev, "CDNSP-DRD physical base addr: %pa\n",
> >- &res->start);
> >+ res[RES_DRD_ID].start = pci_resource_start(pdev,
> >PCI_BAR_OTG);
> >+ res[RES_DRD_ID].end = pci_resource_end(pdev,
> >PCI_BAR_OTG);
> >+ res[RES_DRD_ID].name = "otg";
> >+ res[RES_DRD_ID].flags = IORESOURCE_MEM;
> >+ dev_dbg(&pdev->dev, "CDNSP-DRD physical base addr:
> >%pa\n",
> >+ &res[RES_DRD_ID].start);
> >
> > /* Interrupt for OTG/DRD. */
> >- cdnsp->otg_irq = pdev->irq;
> >+ wrap->dev_res[RES_IRQ_OTG_ID].start = pdev->irq;
> >+ wrap->dev_res[RES_IRQ_OTG_ID].name = "otg";
> >+ wrap->dev_res[RES_IRQ_OTG_ID].flags = IORESOURCE_IRQ;
> > }
> >
> >- /*
> >- * Cadence PCI based platform require some longer timeout for APB
> >- * to fixes domain clock synchronization issue after resuming
> >- * controller from L1 state.
> >- */
> >- cdnsp->override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
> >- pci_set_drvdata(pdev, cdnsp);
> >-
> > if (pci_is_enabled(func)) {
> >- cdnsp->dev = dev;
> >- cdnsp->gadget_init = cdnsp_gadget_init;
> >-
> >- ret = cdns_init(cdnsp);
> >- if (ret)
> >- goto free_cdnsp;
> >+ /* set up platform device info */
> >+ pdata.override_apb_timeout =
> >CHICKEN_APB_TIMEOUT_VALUE;
> >+ memset(&plat_info, 0, sizeof(plat_info));
> >+ plat_info.parent = &pdev->dev;
> >+ plat_info.fwnode = pdev->dev.fwnode;
> >+ plat_info.name = PLAT_DRIVER_NAME;
> >+ plat_info.id = pdev->devfn;
> >+ plat_info.res = wrap->dev_res;
> >+ plat_info.num_res = ARRAY_SIZE(wrap->dev_res);
> >+ plat_info.dma_mask = pdev->dma_mask;
> >+ plat_info.data = &pdata;
> >+ plat_info.size_data = sizeof(pdata);
> >+ wrap->devfn = pdev->devfn;
> >+ /* register platform device */
> >+ wrap->plat_dev = platform_device_register_full(&plat_info);
> >+ if (IS_ERR(wrap->plat_dev)) {
> >+ ret = PTR_ERR(wrap->plat_dev);
> >+ kfree(wrap);
> >+ goto put_pci;
> >+ }
> > }
> >
> >- device_wakeup_enable(&pdev->dev);
> >- if (pci_dev_run_wake(pdev))
> >- pm_runtime_put_noidle(&pdev->dev);
> >-
> >- return 0;
> >-
> >-free_cdnsp:
> >- if (!pci_is_enabled(func))
> >- kfree(cdnsp);
> >-
> >+ pci_set_drvdata(pdev, wrap);
> > put_pci:
> > pci_dev_put(func);
> >-
> > return ret;
> > }
> >
> > static void cdnsp_pci_remove(struct pci_dev *pdev)
> > {
> >- struct cdns *cdnsp;
> >+ struct cdnsp_wrap *wrap;
> > struct pci_dev *func;
> >
> > func = cdnsp_get_second_fun(pdev);
> >- cdnsp = (struct cdns *)pci_get_drvdata(pdev);
> >+ wrap = pci_get_drvdata(pdev);
> >
> >- if (pci_dev_run_wake(pdev))
> >- pm_runtime_get_noresume(&pdev->dev);
> >+ if (wrap->devfn == pdev->devfn)
> >+ platform_device_unregister(wrap->plat_dev);
> >
> >- if (pci_is_enabled(func)) {
> >- cdns_remove(cdnsp);
> >- } else {
> >- kfree(cdnsp);
> >- }
> >+ if (!pci_is_enabled(func))
> >+ kfree(wrap);
> >
> > pci_dev_put(func);
> > }
> >
> >-static int __maybe_unused cdnsp_pci_suspend(struct device *dev)
> >-{
> >- struct cdns *cdns = dev_get_drvdata(dev);
> >-
> >- return cdns_suspend(cdns);
> >-}
> >-
> >-static int __maybe_unused cdnsp_pci_resume(struct device *dev)
> >-{
> >- struct cdns *cdns = dev_get_drvdata(dev);
> >- unsigned long flags;
> >- int ret;
> >-
> >- spin_lock_irqsave(&cdns->lock, flags);
> >- ret = cdns_resume(cdns);
> >- spin_unlock_irqrestore(&cdns->lock, flags);
> >- cdns_set_active(cdns, 1);
> >-
> >- return ret;
> >-}
> >-
> >-static const struct dev_pm_ops cdnsp_pci_pm_ops = {
> >- SET_SYSTEM_SLEEP_PM_OPS(cdnsp_pci_suspend,
> >cdnsp_pci_resume)
> >-};
> >-
> > static const struct pci_device_id cdnsp_pci_ids[] = {
> > { PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
> > .class = PCI_CLASS_SERIAL_USB_DEVICE },
> >@@ -230,13 +202,10 @@ static const struct pci_device_id cdnsp_pci_ids[] = {
> > };
> >
> > static struct pci_driver cdnsp_pci_driver = {
> >- .name = "cdnsp-pci",
> >+ .name = PCI_DRIVER_NAME,
> > .id_table = cdnsp_pci_ids,
> > .probe = cdnsp_pci_probe,
> > .remove = cdnsp_pci_remove,
> >- .driver = {
> >- .pm = &cdnsp_pci_pm_ops,
> >- }
> > };
> >
> > module_pci_driver(cdnsp_pci_driver);
> >@@ -245,4 +214,4 @@ MODULE_DEVICE_TABLE(pci, cdnsp_pci_ids);
> > MODULE_ALIAS("pci:cdnsp");
> > MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
> > MODULE_LICENSE("GPL v2");
> >-MODULE_DESCRIPTION("Cadence CDNSP PCI driver");
> >+MODULE_DESCRIPTION("Cadence CDNSP PCI wrapper");
> >diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> >index f0e32227c0b7..10f00b6c3c83 100644
> >--- a/drivers/usb/cdns3/core.c
> >+++ b/drivers/usb/cdns3/core.c
> >@@ -80,7 +80,7 @@ static void cdns_exit_roles(struct cdns *cdns)
> > *
> > * Returns 0 on success otherwise negative errno
> > */
> >-static int cdns_core_init_role(struct cdns *cdns)
> >+int cdns_core_init_role(struct cdns *cdns)
> > {
> > struct device *dev = cdns->dev;
> > enum usb_dr_mode best_dr_mode;
> >@@ -197,11 +197,14 @@ static int cdns_core_init_role(struct cdns *cdns)
> > goto err;
> > }
> >
> >+ dev_dbg(dev, "Cadence USB3 core: probe succeed\n");
> >+
> > return 0;
> > err:
> > cdns_exit_roles(cdns);
> > return ret;
> > }
> >+EXPORT_SYMBOL_GPL(cdns_core_init_role);
> >
> > /**
> > * cdns_hw_role_state_machine - role switch state machine based on hw
> >events.
> >@@ -469,14 +472,8 @@ int cdns_init(struct cdns *cdns)
> > if (ret)
> > goto init_failed;
> >
> >- ret = cdns_core_init_role(cdns);
> >- if (ret)
> >- goto init_failed;
> >-
> > spin_lock_init(&cdns->lock);
> >
> >- dev_dbg(dev, "Cadence USB3 core: probe succeed\n");
> >-
> > return 0;
> > init_failed:
> > cdns_drd_exit(cdns);
> >diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> >index 801be9e61340..dc8c4137de15 100644
> >--- a/drivers/usb/cdns3/core.h
> >+++ b/drivers/usb/cdns3/core.h
> >@@ -45,6 +45,7 @@ struct cdns3_platform_data {
> > unsigned long quirks;
> > #define CDNS3_DEFAULT_PM_RUNTIME_ALLOW BIT(0)
> > #define CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE BIT(1)
> >+ u32 override_apb_timeout; /* 0 = use default (e.g. for PCI) */
> > };
> >
> > /**
> >@@ -119,14 +120,14 @@ struct cdns {
> > struct cdns3_platform_data *pdata;
> > spinlock_t lock;
> > struct xhci_plat_priv *xhci_plat_data;
> >- u32 override_apb_timeout;
> >-
> > int (*gadget_init)(struct cdns *cdns);
> >+ u32 override_apb_timeout;
> > };
> >
> > int cdns_hw_role_switch(struct cdns *cdns);
> > int cdns_init(struct cdns *cdns);
> > int cdns_remove(struct cdns *cdns);
> >+int cdns_core_init_role(struct cdns *cdns);
> >
> > #ifdef CONFIG_PM_SLEEP
> > int cdns_resume(struct cdns *cdns);
> >diff --git a/drivers/usb/cdns3/gadget-export.h b/drivers/usb/cdns3/gadget-
> >export.h
> >index c37b6269b001..0cb600e2b5d2 100644
> >--- a/drivers/usb/cdns3/gadget-export.h
> >+++ b/drivers/usb/cdns3/gadget-export.h
> >@@ -10,7 +10,7 @@
> > #ifndef __LINUX_CDNS3_GADGET_EXPORT
> > #define __LINUX_CDNS3_GADGET_EXPORT
> >
> >-#if IS_ENABLED(CONFIG_USB_CDNSP_GADGET)
> >+#if defined(CONFIG_USB_CDNSP_GADGET) &&
> >IS_REACHABLE(CONFIG_USB_CDNSP)
> >
> > int cdnsp_gadget_init(struct cdns *cdns);
> > #else
> >@@ -22,7 +22,7 @@ static inline int cdnsp_gadget_init(struct cdns *cdns)
> >
> > #endif /* CONFIG_USB_CDNSP_GADGET */
> >
> >-#if IS_ENABLED(CONFIG_USB_CDNS3_GADGET)
> >+#if defined(CONFIG_USB_CDNS3_GADGET) &&
> >IS_REACHABLE(CONFIG_USB_CDNS3)
> >
> > int cdns3_gadget_init(struct cdns *cdns);
> > #else
> >--
> >2.50.1
>
--
Best regards,
Peter
prev parent reply other threads:[~2026-03-25 7:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-16 6:48 [PATCH v2 0/2] usb: cdns3: USBSSP platform driver support Peter Chen
2026-03-16 6:48 ` [PATCH v2 1/2] dt-bindings: usb: cdns,usb3: document USBSSP controller support Peter Chen
2026-03-24 1:06 ` Rob Herring (Arm)
2026-03-16 6:48 ` [PATCH v2 2/2] usb: cdns3: Add USBSSP platform driver support Peter Chen
2026-03-24 12:10 ` Pawel Laszczak
2026-03-25 7:35 ` Peter Chen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acOQJMLwyT7QtCIQ@nchen-desktop \
--to=peter.chen@cixtech.com \
--cc=cix-kernel-upstream@cixtech.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=krzk+dt@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=pawell@cadence.com \
--cc=robh@kernel.org \
--cc=rogerq@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox