2024-02-06 11:17:19

by Pawel Laszczak

[permalink] [raw]
Subject: [PATCH] usb: cdnsp: blocked some cdns3 specific code

host.c file has some parts of code that were introduced for CDNS3 driver
and should not be used with CDNSP driver.
This patch blocks using these parts of codes by CDNSP driver.
These elements include:
- xhci_plat_cdns3_xhci object
- cdns3 specific XECP_PORT_CAP_REG register
- cdns3 specific XECP_AUX_CTRL_REG1 register

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/host.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index 6164fc4c96a4..ceca4d839dfd 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -18,6 +18,11 @@
#include "../host/xhci.h"
#include "../host/xhci-plat.h"

+/*
+ * The XECP_PORT_CAP_REG and XECP_AUX_CTRL_REG1 exist only
+ * in Cadence USB3 dual-role controller, so it can't be used
+ * with Cadence CDNSP dual-role controller.
+ */
#define XECP_PORT_CAP_REG 0x8000
#define XECP_AUX_CTRL_REG1 0x8120

@@ -57,6 +62,8 @@ static const struct xhci_plat_priv xhci_plat_cdns3_xhci = {
.resume_quirk = xhci_cdns3_resume_quirk,
};

+static const struct xhci_plat_priv xhci_plat_cdnsp_xhci;
+
static int __cdns_host_init(struct cdns *cdns)
{
struct platform_device *xhci;
@@ -81,8 +88,13 @@ static int __cdns_host_init(struct cdns *cdns)
goto err1;
}

- cdns->xhci_plat_data = kmemdup(&xhci_plat_cdns3_xhci,
- sizeof(struct xhci_plat_priv), GFP_KERNEL);
+ if (cdns->version < CDNSP_CONTROLLER_V2)
+ cdns->xhci_plat_data = kmemdup(&xhci_plat_cdns3_xhci,
+ sizeof(struct xhci_plat_priv), GFP_KERNEL);
+ else
+ cdns->xhci_plat_data = kmemdup(&xhci_plat_cdnsp_xhci,
+ sizeof(struct xhci_plat_priv), GFP_KERNEL);
+
if (!cdns->xhci_plat_data) {
ret = -ENOMEM;
goto err1;
--
2.37.2



2024-02-15 07:23:35

by Peter Chen

[permalink] [raw]
Subject: Re: [PATCH] usb: cdnsp: blocked some cdns3 specific code

It is for both cdns3 and cdnsp.

Reply-To:
In-Reply-To: <[email protected]>

On 24-02-06 11:40:18, Pawel Laszczak wrote:
> host.c file has some parts of code that were introduced for CDNS3 driver
> and should not be used with CDNSP driver.
> This patch blocks using these parts of codes by CDNSP driver.
> These elements include:
> - xhci_plat_cdns3_xhci object
> - cdns3 specific XECP_PORT_CAP_REG register
> - cdns3 specific XECP_AUX_CTRL_REG1 register
>
> 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/host.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
> index 6164fc4c96a4..ceca4d839dfd 100644
> --- a/drivers/usb/cdns3/host.c
> +++ b/drivers/usb/cdns3/host.c
> @@ -18,6 +18,11 @@
> #include "../host/xhci.h"
> #include "../host/xhci-plat.h"
>
> +/*
> + * The XECP_PORT_CAP_REG and XECP_AUX_CTRL_REG1 exist only
> + * in Cadence USB3 dual-role controller, so it can't be used
> + * with Cadence CDNSP dual-role controller.
> + */
> #define XECP_PORT_CAP_REG 0x8000
> #define XECP_AUX_CTRL_REG1 0x8120
>
> @@ -57,6 +62,8 @@ static const struct xhci_plat_priv xhci_plat_cdns3_xhci = {
> .resume_quirk = xhci_cdns3_resume_quirk,
> };
>
> +static const struct xhci_plat_priv xhci_plat_cdnsp_xhci;
> +
> static int __cdns_host_init(struct cdns *cdns)
> {
> struct platform_device *xhci;
> @@ -81,8 +88,13 @@ static int __cdns_host_init(struct cdns *cdns)
> goto err1;
> }
>
> - cdns->xhci_plat_data = kmemdup(&xhci_plat_cdns3_xhci,
> - sizeof(struct xhci_plat_priv), GFP_KERNEL);
> + if (cdns->version < CDNSP_CONTROLLER_V2)
> + cdns->xhci_plat_data = kmemdup(&xhci_plat_cdns3_xhci,
> + sizeof(struct xhci_plat_priv), GFP_KERNEL);
> + else
> + cdns->xhci_plat_data = kmemdup(&xhci_plat_cdnsp_xhci,
> + sizeof(struct xhci_plat_priv), GFP_KERNEL);
> +

You may explain why you duplicate another structure for cdnsp at commit
log.

> if (!cdns->xhci_plat_data) {
> ret = -ENOMEM;
> goto err1;
>

Others are okay for me.

--

Thanks,
Peter Chen

2024-02-15 08:36:56

by Pawel Laszczak

[permalink] [raw]
Subject: RE: [PATCH] usb: cdnsp: blocked some cdns3 specific code

>
>It is for both cdns3 and cdnsp.
>
>Reply-To:
>In-Reply-To: <[email protected]>
>
>On 24-02-06 11:40:18, Pawel Laszczak wrote:
>> host.c file has some parts of code that were introduced for CDNS3
>> driver and should not be used with CDNSP driver.
>> This patch blocks using these parts of codes by CDNSP driver.
>> These elements include:
>> - xhci_plat_cdns3_xhci object
>> - cdns3 specific XECP_PORT_CAP_REG register
>> - cdns3 specific XECP_AUX_CTRL_REG1 register
>>
>> 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/host.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c index
>> 6164fc4c96a4..ceca4d839dfd 100644
>> --- a/drivers/usb/cdns3/host.c
>> +++ b/drivers/usb/cdns3/host.c
>> @@ -18,6 +18,11 @@
>> #include "../host/xhci.h"
>> #include "../host/xhci-plat.h"
>>
>> +/*
>> + * The XECP_PORT_CAP_REG and XECP_AUX_CTRL_REG1 exist only
>> + * in Cadence USB3 dual-role controller, so it can't be used
>> + * with Cadence CDNSP dual-role controller.
>> + */
>> #define XECP_PORT_CAP_REG 0x8000
>> #define XECP_AUX_CTRL_REG1 0x8120
>>
>> @@ -57,6 +62,8 @@ static const struct xhci_plat_priv xhci_plat_cdns3_xhci =
>{
>> .resume_quirk = xhci_cdns3_resume_quirk, };
>>
>> +static const struct xhci_plat_priv xhci_plat_cdnsp_xhci;
>> +
>> static int __cdns_host_init(struct cdns *cdns) {
>> struct platform_device *xhci;
>> @@ -81,8 +88,13 @@ static int __cdns_host_init(struct cdns *cdns)
>> goto err1;
>> }
>>
>> - cdns->xhci_plat_data = kmemdup(&xhci_plat_cdns3_xhci,
>> - sizeof(struct xhci_plat_priv), GFP_KERNEL);
>> + if (cdns->version < CDNSP_CONTROLLER_V2)
>> + cdns->xhci_plat_data = kmemdup(&xhci_plat_cdns3_xhci,
>> + sizeof(struct xhci_plat_priv), GFP_KERNEL);
>> + else
>> + cdns->xhci_plat_data = kmemdup(&xhci_plat_cdnsp_xhci,
>> + sizeof(struct xhci_plat_priv), GFP_KERNEL);
>> +
>
>You may explain why you duplicate another structure for cdnsp at commit log.

In my opinion it is more readable. If someone will review the code then easier to notice the difference between these driver.

Thanks
Pawel

>
>> if (!cdns->xhci_plat_data) {
>> ret = -ENOMEM;
>> goto err1;
>>
>
>Others are okay for me.
>
>--
>
>Thanks,
>Peter Chen