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

[Pawel Laszczak]

>
>It is for both cdns3 and cdnsp.
>
>Reply-To:
>In-Reply-To: <20240206104018.48272-1-pawell@xxxxxxxxxxx>
>
>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: <stable@xxxxxxxxxxxxxxx>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
>> USBSSP DRD Driver")
>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>> ---
>>  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