2024-02-06 10:45:05

by Pawel Laszczak

[permalink] [raw]
Subject: [PATCH] usb: cdnsp: fixed issue with incorrect detecting CDNSP family controllers

Cadence have several controllers from 0x000403xx family but current
driver suuport detecting only one with DID equal 0x0004034E.
It causes that if someone use different CDNSP controller then driver
will use incorrect version and register space.
Patch fix this issue.

cc: <[email protected]>
Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
Signed-off-by: Pawel Laszczak <[email protected]>
---
drivers/usb/cdns3/core.c | 1 -
drivers/usb/cdns3/drd.c | 13 +++++++++----
drivers/usb/cdns3/drd.h | 6 +++++-
3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 33548771a0d3..465e9267b49c 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -395,7 +395,6 @@ static int cdns_role_set(struct usb_role_switch *sw, enum usb_role role)
return ret;
}

-
/**
* cdns_wakeup_irq - interrupt handler for wakeup events
* @irq: irq number for cdns3/cdnsp core device
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 04b6d12f2b9a..ee917f1b091c 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -156,7 +156,8 @@ bool cdns_is_device(struct cdns *cdns)
*/
static void cdns_otg_disable_irq(struct cdns *cdns)
{
- writel(0, &cdns->otg_irq_regs->ien);
+ if (cdns->version)
+ writel(0, &cdns->otg_irq_regs->ien);
}

/**
@@ -422,15 +423,20 @@ int cdns_drd_init(struct cdns *cdns)

cdns->otg_regs = (void __iomem *)&cdns->otg_v1_regs->cmd;

- if (readl(&cdns->otg_cdnsp_regs->did) == OTG_CDNSP_DID) {
+ state = readl(&cdns->otg_cdnsp_regs->did);
+
+ if (OTG_CDNSP_CHECK_DID(state)) {
cdns->otg_irq_regs = (struct cdns_otg_irq_regs __iomem *)
&cdns->otg_cdnsp_regs->ien;
cdns->version = CDNSP_CONTROLLER_V2;
- } else {
+ } else if (OTG_CDNS3_CHECK_DID(state)) {
cdns->otg_irq_regs = (struct cdns_otg_irq_regs __iomem *)
&cdns->otg_v1_regs->ien;
writel(1, &cdns->otg_v1_regs->simulate);
cdns->version = CDNS3_CONTROLLER_V1;
+ } else {
+ dev_err(cdns->dev, "not supporte DID=0x%08x\n", state);
+ return -EINVAL;
}

dev_dbg(cdns->dev, "DRD version v1 (ID: %08x, rev: %08x)\n",
@@ -483,7 +489,6 @@ int cdns_drd_exit(struct cdns *cdns)
return 0;
}

-
/* Indicate the cdns3 core was power lost before */
bool cdns_power_is_lost(struct cdns *cdns)
{
diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
index cbdf94f73ed9..d72370c321d3 100644
--- a/drivers/usb/cdns3/drd.h
+++ b/drivers/usb/cdns3/drd.h
@@ -79,7 +79,11 @@ struct cdnsp_otg_regs {
__le32 susp_timing_ctrl;
};

-#define OTG_CDNSP_DID 0x0004034E
+/* CDNSP driver supports 0x000403xx Cadence USB controller family. */
+#define OTG_CDNSP_CHECK_DID(did) (((did) & GENMASK(31, 8)) == 0x00040300)
+
+/* CDNS3 driver supports 0x000402xx Cadence USB controller family. */
+#define OTG_CDNS3_CHECK_DID(did) (((did) & GENMASK(31, 8)) == 0x00040200)

/*
* Common registers interface for both CDNS3 and CDNSP version of DRD.
--
2.37.2



2024-02-15 07:18:42

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH] usb: cdnsp: fixed issue with incorrect detecting CDNSP family controllers

On 24-02-06 11:43:25, Pawel Laszczak wrote:
> Cadence have several controllers from 0x000403xx family but current
> driver suuport detecting only one with DID equal 0x0004034E.
> It causes that if someone use different CDNSP controller then driver

%s/use/uses

> will use incorrect version and register space.
> Patch fix this issue.
>
> cc: <[email protected]>
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> Signed-off-by: Pawel Laszczak <[email protected]>
> ---
> drivers/usb/cdns3/core.c | 1 -
> drivers/usb/cdns3/drd.c | 13 +++++++++----
> drivers/usb/cdns3/drd.h | 6 +++++-
> 3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 33548771a0d3..465e9267b49c 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -395,7 +395,6 @@ static int cdns_role_set(struct usb_role_switch *sw, enum usb_role role)
> return ret;
> }
>
> -
> /**
> * cdns_wakeup_irq - interrupt handler for wakeup events
> * @irq: irq number for cdns3/cdnsp core device
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 04b6d12f2b9a..ee917f1b091c 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -156,7 +156,8 @@ bool cdns_is_device(struct cdns *cdns)
> */
> static void cdns_otg_disable_irq(struct cdns *cdns)
> {
> - writel(0, &cdns->otg_irq_regs->ien);
> + if (cdns->version)
> + writel(0, &cdns->otg_irq_regs->ien);
> }
>
> /**
> @@ -422,15 +423,20 @@ int cdns_drd_init(struct cdns *cdns)
>
> cdns->otg_regs = (void __iomem *)&cdns->otg_v1_regs->cmd;
>
> - if (readl(&cdns->otg_cdnsp_regs->did) == OTG_CDNSP_DID) {
> + state = readl(&cdns->otg_cdnsp_regs->did);

Use a meaningful variable.

> +
> + if (OTG_CDNSP_CHECK_DID(state)) {
> cdns->otg_irq_regs = (struct cdns_otg_irq_regs __iomem *)
> &cdns->otg_cdnsp_regs->ien;
> cdns->version = CDNSP_CONTROLLER_V2;
> - } else {
> + } else if (OTG_CDNS3_CHECK_DID(state)) {
> cdns->otg_irq_regs = (struct cdns_otg_irq_regs __iomem *)
> &cdns->otg_v1_regs->ien;
> writel(1, &cdns->otg_v1_regs->simulate);
> cdns->version = CDNS3_CONTROLLER_V1;
> + } else {
> + dev_err(cdns->dev, "not supporte DID=0x%08x\n", state);
> + return -EINVAL;
> }
>
> dev_dbg(cdns->dev, "DRD version v1 (ID: %08x, rev: %08x)\n",
> @@ -483,7 +489,6 @@ int cdns_drd_exit(struct cdns *cdns)
> return 0;
> }
>
> -
> /* Indicate the cdns3 core was power lost before */
> bool cdns_power_is_lost(struct cdns *cdns)
> {
> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> index cbdf94f73ed9..d72370c321d3 100644
> --- a/drivers/usb/cdns3/drd.h
> +++ b/drivers/usb/cdns3/drd.h
> @@ -79,7 +79,11 @@ struct cdnsp_otg_regs {
> __le32 susp_timing_ctrl;
> };
>
> -#define OTG_CDNSP_DID 0x0004034E
> +/* CDNSP driver supports 0x000403xx Cadence USB controller family. */
> +#define OTG_CDNSP_CHECK_DID(did) (((did) & GENMASK(31, 8)) == 0x00040300)

GENMASK(19, 8)?

> +
> +/* CDNS3 driver supports 0x000402xx Cadence USB controller family. */
> +#define OTG_CDNS3_CHECK_DID(did) (((did) & GENMASK(31, 8)) == 0x00040200)
>
> /*
> * Common registers interface for both CDNS3 and CDNSP version of DRD.
> --
> 2.37.2
>

--

Thanks,
Peter Chen