Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] usb: cdns3: fixes for 5.4-rc
@ 2019-10-07 12:15 Roger Quadros
  2019-10-07 12:16 ` [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role() Roger Quadros
  2019-10-07 12:16 ` [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode Roger Quadros
  0 siblings, 2 replies; 9+ messages in thread
From: Roger Quadros @ 2019-10-07 12:15 UTC (permalink / raw)
  To: felipe.balbi, gregkh, pawell
  Cc: peter.chen, nsekhar, kurahul, linux-usb, linux-kernel, Roger Quadros

Hi,

Here are 2 fixes we found while testing the cdns3 driver
on our platform.

The first one fixes a corner case where super-speed in host mode 
doesn't work if device was plugged before the cdns3 driver probes.

The second one fixes the case when gadget driver is
limited to full-speed.

cheers,
-roger

Roger Quadros (2):
  usb: cdns3: fix cdns3_core_init_role()
  usb: cdns3: gadget: Fix full-speed mode

 drivers/usb/cdns3/core.c   | 20 +++++++++++++++++++-
 drivers/usb/cdns3/gadget.c |  1 +
 2 files changed, 20 insertions(+), 1 deletion(-)

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
  2019-10-07 12:15 [PATCH 0/2] usb: cdns3: fixes for 5.4-rc Roger Quadros
@ 2019-10-07 12:16 ` Roger Quadros
  2019-10-16  4:32   ` Pawel Laszczak
  2019-10-07 12:16 ` [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode Roger Quadros
  1 sibling, 1 reply; 9+ messages in thread
From: Roger Quadros @ 2019-10-07 12:16 UTC (permalink / raw)
  To: felipe.balbi, gregkh, pawell
  Cc: peter.chen, nsekhar, kurahul, linux-usb, linux-kernel, Roger Quadros

At startup we should trigger the HW state machine
only if it is OTG mode. Otherwise we should just
start the respective role.

Initialize idle role by default. If we don't do this then
cdns3_idle_role_stop() is not called when switching to
host/device role and so lane switch mechanism
doesn't work. This results to super-speed device not working
in one orientation if it was plugged before driver probe.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/usb/cdns3/core.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 06f1e105be4e..1109dc5a4c39 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -160,10 +160,28 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
 	if (ret)
 		goto err;
 
-	if (cdns->dr_mode != USB_DR_MODE_OTG) {
+	/* Initialize idle role to start with */
+	ret = cdns3_role_start(cdns, USB_ROLE_NONE);
+	if (ret)
+		goto err;
+
+	switch (cdns->dr_mode) {
+	case USB_DR_MODE_UNKNOWN:
+	case USB_DR_MODE_OTG:
 		ret = cdns3_hw_role_switch(cdns);
 		if (ret)
 			goto err;
+		break;
+	case USB_DR_MODE_PERIPHERAL:
+		ret = cdns3_role_start(cdns, USB_ROLE_DEVICE);
+		if (ret)
+			goto err;
+		break;
+	case USB_DR_MODE_HOST:
+		ret = cdns3_role_start(cdns, USB_ROLE_HOST);
+		if (ret)
+			goto err;
+		break;
 	}
 
 	return ret;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode
  2019-10-07 12:15 [PATCH 0/2] usb: cdns3: fixes for 5.4-rc Roger Quadros
  2019-10-07 12:16 ` [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role() Roger Quadros
@ 2019-10-07 12:16 ` Roger Quadros
  2019-10-08  8:07   ` Peter Chen
  1 sibling, 1 reply; 9+ messages in thread
From: Roger Quadros @ 2019-10-07 12:16 UTC (permalink / raw)
  To: felipe.balbi, gregkh, pawell
  Cc: peter.chen, nsekhar, kurahul, linux-usb, linux-kernel, Roger Quadros

We need to disable USB3 PHY for full-speed mode else
gadget mode is broken.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/usb/cdns3/gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index 228cdc4ab886..157536753b8c 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -2571,6 +2571,7 @@ static int cdns3_gadget_start(struct cdns3 *cdns)
 	switch (max_speed) {
 	case USB_SPEED_FULL:
 		writel(USB_CONF_SFORCE_FS, &priv_dev->regs->usb_conf);
+		writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf);
 		break;
 	case USB_SPEED_HIGH:
 		writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf);
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode
  2019-10-07 12:16 ` [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode Roger Quadros
@ 2019-10-08  8:07   ` Peter Chen
  2019-10-16  4:35     ` Pawel Laszczak
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Chen @ 2019-10-08  8:07 UTC (permalink / raw)
  To: Roger Quadros
  Cc: felipe.balbi, gregkh, pawell, nsekhar, kurahul, linux-usb, linux-kernel

On 19-10-07 15:16:01, Roger Quadros wrote:
> We need to disable USB3 PHY for full-speed mode else
> gadget mode is broken.
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  drivers/usb/cdns3/gadget.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 228cdc4ab886..157536753b8c 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -2571,6 +2571,7 @@ static int cdns3_gadget_start(struct cdns3 *cdns)
>  	switch (max_speed) {
>  	case USB_SPEED_FULL:
>  		writel(USB_CONF_SFORCE_FS, &priv_dev->regs->usb_conf);
> +		writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf);
>  		break;
>  	case USB_SPEED_HIGH:
>  		writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf);

Reviewed-by: Peter Chen <peter.chen@nxp.com>

-- 

Thanks,
Peter Chen

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
  2019-10-07 12:16 ` [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role() Roger Quadros
@ 2019-10-16  4:32   ` Pawel Laszczak
  2019-10-16  9:20     ` Roger Quadros
  0 siblings, 1 reply; 9+ messages in thread
From: Pawel Laszczak @ 2019-10-16  4:32 UTC (permalink / raw)
  To: Roger Quadros, felipe.balbi, gregkh
  Cc: peter.chen, nsekhar, Rahul Kumar, linux-usb, linux-kernel

Hi Roger

>
>At startup we should trigger the HW state machine
>only if it is OTG mode. Otherwise we should just
>start the respective role.
>
>Initialize idle role by default. If we don't do this then
>cdns3_idle_role_stop() is not called when switching to
>host/device role and so lane switch mechanism
>doesn't work. This results to super-speed device not working
>in one orientation if it was plugged before driver probe.
>
>Signed-off-by: Roger Quadros <rogerq@ti.com>
>Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>---
> drivers/usb/cdns3/core.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>index 06f1e105be4e..1109dc5a4c39 100644
>--- a/drivers/usb/cdns3/core.c
>+++ b/drivers/usb/cdns3/core.c
>@@ -160,10 +160,28 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
> 	if (ret)
> 		goto err;
>
>-	if (cdns->dr_mode != USB_DR_MODE_OTG) {
>+	/* Initialize idle role to start with */
>+	ret = cdns3_role_start(cdns, USB_ROLE_NONE);
>+	if (ret)
>+		goto err;
>+
>+	switch (cdns->dr_mode) {
>+	case USB_DR_MODE_UNKNOWN:

One note in this place. USB_DR_MODE_UNKNOWN is not possible in this place.
If cdns->dr_mode will be USB_DR_MODE_UNKNOWN then driver returns -EINVAL
some line before after returning form cdns3_drd_update_mode and in consequence 
it jump to err label. 

Maybe for better readability it this condition should be treated here also as error.

>+	case USB_DR_MODE_OTG:
> 		ret = cdns3_hw_role_switch(cdns);
> 		if (ret)
> 			goto err;
>+		break;
>+	case USB_DR_MODE_PERIPHERAL:
>+		ret = cdns3_role_start(cdns, USB_ROLE_DEVICE);
>+		if (ret)
>+			goto err;
>+		break;
>+	case USB_DR_MODE_HOST:
>+		ret = cdns3_role_start(cdns, USB_ROLE_HOST);
>+		if (ret)
>+			goto err;
>+		break;
> 	}
>
> 	return ret;

Reviewed-by: Pawel Laszczak <pawell@cadence.com>
Tested-by: Pawel Laszczak <pawell@cadence.com>

>--
>Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
Regards,
Pawel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode
  2019-10-08  8:07   ` Peter Chen
@ 2019-10-16  4:35     ` Pawel Laszczak
  0 siblings, 0 replies; 9+ messages in thread
From: Pawel Laszczak @ 2019-10-16  4:35 UTC (permalink / raw)
  To: Peter Chen, Roger Quadros
  Cc: felipe.balbi, gregkh, nsekhar, Rahul Kumar, linux-usb, linux-kernel

>On 19-10-07 15:16:01, Roger Quadros wrote:
>> We need to disable USB3 PHY for full-speed mode else
>> gadget mode is broken.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>>  drivers/usb/cdns3/gadget.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index 228cdc4ab886..157536753b8c 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -2571,6 +2571,7 @@ static int cdns3_gadget_start(struct cdns3 *cdns)
>>  	switch (max_speed) {
>>  	case USB_SPEED_FULL:
>>  		writel(USB_CONF_SFORCE_FS, &priv_dev->regs->usb_conf);
>> +		writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf);
>>  		break;
>>  	case USB_SPEED_HIGH:
>>  		writel(USB_CONF_USB3DIS, &priv_dev->regs->usb_conf);
>
>Reviewed-by: Peter Chen <peter.chen@nxp.com>

Reviewed-by: Pawel Laszczak <pawell@cadence.com>
>
>--
>
>Thanks,
>Peter Chen

Regards,
Pawell

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
  2019-10-16  4:32   ` Pawel Laszczak
@ 2019-10-16  9:20     ` Roger Quadros
  2019-10-16  9:38       ` Pawel Laszczak
  0 siblings, 1 reply; 9+ messages in thread
From: Roger Quadros @ 2019-10-16  9:20 UTC (permalink / raw)
  To: Pawel Laszczak, felipe.balbi, gregkh
  Cc: peter.chen, nsekhar, Rahul Kumar, linux-usb, linux-kernel

Hi Pawel,

On 16/10/2019 07:32, Pawel Laszczak wrote:
> Hi Roger
> 
>>
>> At startup we should trigger the HW state machine
>> only if it is OTG mode. Otherwise we should just
>> start the respective role.
>>
>> Initialize idle role by default. If we don't do this then
>> cdns3_idle_role_stop() is not called when switching to
>> host/device role and so lane switch mechanism
>> doesn't work. This results to super-speed device not working
>> in one orientation if it was plugged before driver probe.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>> drivers/usb/cdns3/core.c | 20 +++++++++++++++++++-
>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index 06f1e105be4e..1109dc5a4c39 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -160,10 +160,28 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>> 	if (ret)
>> 		goto err;
>>
>> -	if (cdns->dr_mode != USB_DR_MODE_OTG) {
>> +	/* Initialize idle role to start with */
>> +	ret = cdns3_role_start(cdns, USB_ROLE_NONE);
>> +	if (ret)
>> +		goto err;
>> +
>> +	switch (cdns->dr_mode) {
>> +	case USB_DR_MODE_UNKNOWN:
> 
> One note in this place. USB_DR_MODE_UNKNOWN is not possible in this place.
> If cdns->dr_mode will be USB_DR_MODE_UNKNOWN then driver returns -EINVAL

At which place? I could not find.

> some line before after returning form cdns3_drd_update_mode and in consequence
> it jump to err label.
> 
> Maybe for better readability it this condition should be treated here also as error.
> 
>> +	case USB_DR_MODE_OTG:
>> 		ret = cdns3_hw_role_switch(cdns);
>> 		if (ret)
>> 			goto err;
>> +		break;
>> +	case USB_DR_MODE_PERIPHERAL:
>> +		ret = cdns3_role_start(cdns, USB_ROLE_DEVICE);
>> +		if (ret)
>> +			goto err;
>> +		break;
>> +	case USB_DR_MODE_HOST:
>> +		ret = cdns3_role_start(cdns, USB_ROLE_HOST);
>> +		if (ret)
>> +			goto err;
>> +		break;
>> 	}
>>
>> 	return ret;
> 
> Reviewed-by: Pawel Laszczak <pawell@cadence.com>
> Tested-by: Pawel Laszczak <pawell@cadence.com>
> 
> --
> Regards,
> Pawel
> 

-- 
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
  2019-10-16  9:20     ` Roger Quadros
@ 2019-10-16  9:38       ` Pawel Laszczak
  2019-10-16  9:54         ` Roger Quadros
  0 siblings, 1 reply; 9+ messages in thread
From: Pawel Laszczak @ 2019-10-16  9:38 UTC (permalink / raw)
  To: Roger Quadros, felipe.balbi, gregkh
  Cc: peter.chen, nsekhar, Rahul Kumar, linux-usb, linux-kernel

>
>
>Hi Pawel,
>
>On 16/10/2019 07:32, Pawel Laszczak wrote:
>> Hi Roger
>>
>>>
>>> At startup we should trigger the HW state machine
>>> only if it is OTG mode. Otherwise we should just
>>> start the respective role.
>>>
>>> Initialize idle role by default. If we don't do this then
>>> cdns3_idle_role_stop() is not called when switching to
>>> host/device role and so lane switch mechanism
>>> doesn't work. This results to super-speed device not working
>>> in one orientation if it was plugged before driver probe.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>>> ---
>>> drivers/usb/cdns3/core.c | 20 +++++++++++++++++++-
>>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>> index 06f1e105be4e..1109dc5a4c39 100644
>>> --- a/drivers/usb/cdns3/core.c
>>> +++ b/drivers/usb/cdns3/core.c
>>> @@ -160,10 +160,28 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>>> 	if (ret)
>>> 		goto err;
>>>
>>> -	if (cdns->dr_mode != USB_DR_MODE_OTG) {
>>> +	/* Initialize idle role to start with */
>>> +	ret = cdns3_role_start(cdns, USB_ROLE_NONE);
>>> +	if (ret)
>>> +		goto err;
>>> +
>>> +	switch (cdns->dr_mode) {
>>> +	case USB_DR_MODE_UNKNOWN:
>>
>> One note in this place. USB_DR_MODE_UNKNOWN is not possible in this place.
>> If cdns->dr_mode will be USB_DR_MODE_UNKNOWN then driver returns -EINVAL
>
>At which place? I could not find.

In this patch we can't see this line:
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/usb/cdns3/core.c#L159 
There is called  cdns3_drd_update_mode(cdns);

int cdns3_drd_update_mode(struct cdns3 *cdns)
{
	int ret = 0;

	switch (cdns->dr_mode) {
	case USB_DR_MODE_PERIPHERAL:
		ret = cdns3_set_mode(cdns, USB_DR_MODE_PERIPHERAL);
		break;
	case USB_DR_MODE_HOST:
		ret = cdns3_set_mode(cdns, USB_DR_MODE_HOST);
		break;
	case USB_DR_MODE_OTG:
		ret = cdns3_init_otg_mode(cdns);
		break;
	default:
		dev_err(cdns->dev, "Unsupported mode of operation %d\n",
			cdns->dr_mode);
		return -EINVAL;
	}

	return ret;
}

After calling cdns3_drd_update_mode  we have 2 first lines from this patch 
	if (ret)
		goto err;

Pawel
>
>> some line before after returning form cdns3_drd_update_mode and in consequence
>> it jump to err label.
>>
>> Maybe for better readability it this condition should be treated here also as error.
>>
>>> +	case USB_DR_MODE_OTG:
>>> 		ret = cdns3_hw_role_switch(cdns);
>>> 		if (ret)
>>> 			goto err;
>>> +		break;
>>> +	case USB_DR_MODE_PERIPHERAL:
>>> +		ret = cdns3_role_start(cdns, USB_ROLE_DEVICE);
>>> +		if (ret)
>>> +			goto err;
>>> +		break;
>>> +	case USB_DR_MODE_HOST:
>>> +		ret = cdns3_role_start(cdns, USB_ROLE_HOST);
>>> +		if (ret)
>>> +			goto err;
>>> +		break;
>>> 	}
>>>
>>> 	return ret;
>>
>> Reviewed-by: Pawel Laszczak <pawell@cadence.com>
>> Tested-by: Pawel Laszczak <pawell@cadence.com>
>>
>> --
>> Regards,
>> Pawel
>>
>
>--
>cheers,
>-roger
>Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
  2019-10-16  9:38       ` Pawel Laszczak
@ 2019-10-16  9:54         ` Roger Quadros
  0 siblings, 0 replies; 9+ messages in thread
From: Roger Quadros @ 2019-10-16  9:54 UTC (permalink / raw)
  To: Pawel Laszczak, felipe.balbi, gregkh
  Cc: peter.chen, nsekhar, Rahul Kumar, linux-usb, linux-kernel



On 16/10/2019 12:38, Pawel Laszczak wrote:
>>
>>
>> Hi Pawel,
>>
>> On 16/10/2019 07:32, Pawel Laszczak wrote:
>>> Hi Roger
>>>
>>>>
>>>> At startup we should trigger the HW state machine
>>>> only if it is OTG mode. Otherwise we should just
>>>> start the respective role.
>>>>
>>>> Initialize idle role by default. If we don't do this then
>>>> cdns3_idle_role_stop() is not called when switching to
>>>> host/device role and so lane switch mechanism
>>>> doesn't work. This results to super-speed device not working
>>>> in one orientation if it was plugged before driver probe.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>>>> ---
>>>> drivers/usb/cdns3/core.c | 20 +++++++++++++++++++-
>>>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>> index 06f1e105be4e..1109dc5a4c39 100644
>>>> --- a/drivers/usb/cdns3/core.c
>>>> +++ b/drivers/usb/cdns3/core.c
>>>> @@ -160,10 +160,28 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>>>> 	if (ret)
>>>> 		goto err;
>>>>
>>>> -	if (cdns->dr_mode != USB_DR_MODE_OTG) {
>>>> +	/* Initialize idle role to start with */
>>>> +	ret = cdns3_role_start(cdns, USB_ROLE_NONE);
>>>> +	if (ret)
>>>> +		goto err;
>>>> +
>>>> +	switch (cdns->dr_mode) {
>>>> +	case USB_DR_MODE_UNKNOWN:
>>>
>>> One note in this place. USB_DR_MODE_UNKNOWN is not possible in this place.
>>> If cdns->dr_mode will be USB_DR_MODE_UNKNOWN then driver returns -EINVAL
>>
>> At which place? I could not find.
> 
> In this patch we can't see this line:
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/usb/cdns3/core.c#L159
> There is called  cdns3_drd_update_mode(cdns);
> 
> int cdns3_drd_update_mode(struct cdns3 *cdns)
> {
> 	int ret = 0;
> 
> 	switch (cdns->dr_mode) {
> 	case USB_DR_MODE_PERIPHERAL:
> 		ret = cdns3_set_mode(cdns, USB_DR_MODE_PERIPHERAL);
> 		break;
> 	case USB_DR_MODE_HOST:
> 		ret = cdns3_set_mode(cdns, USB_DR_MODE_HOST);
> 		break;
> 	case USB_DR_MODE_OTG:
> 		ret = cdns3_init_otg_mode(cdns);
> 		break;
> 	default:
> 		dev_err(cdns->dev, "Unsupported mode of operation %d\n",
> 			cdns->dr_mode);
> 		return -EINVAL;
> 	}
> 
> 	return ret;
> }
> 
> After calling cdns3_drd_update_mode  we have 2 first lines from this patch
> 	if (ret)
> 		goto err;

I see now. I will update this patch to error out if dr_mode is USB_DR_MODE_UNKNOWN.
Thanks.

cheers,
-roger

> 
> Pawel
>>
>>> some line before after returning form cdns3_drd_update_mode and in consequence
>>> it jump to err label.
>>>
>>> Maybe for better readability it this condition should be treated here also as error.
>>>
>>>> +	case USB_DR_MODE_OTG:
>>>> 		ret = cdns3_hw_role_switch(cdns);
>>>> 		if (ret)
>>>> 			goto err;
>>>> +		break;
>>>> +	case USB_DR_MODE_PERIPHERAL:
>>>> +		ret = cdns3_role_start(cdns, USB_ROLE_DEVICE);
>>>> +		if (ret)
>>>> +			goto err;
>>>> +		break;
>>>> +	case USB_DR_MODE_HOST:
>>>> +		ret = cdns3_role_start(cdns, USB_ROLE_HOST);
>>>> +		if (ret)
>>>> +			goto err;
>>>> +		break;
>>>> 	}
>>>>
>>>> 	return ret;
>>>
>>> Reviewed-by: Pawel Laszczak <pawell@cadence.com>
>>> Tested-by: Pawel Laszczak <pawell@cadence.com>
>>>
>>> --
>>> Regards,
>>> Pawel
>>>
>>
>> --
>> cheers,
>> -roger
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 12:15 [PATCH 0/2] usb: cdns3: fixes for 5.4-rc Roger Quadros
2019-10-07 12:16 ` [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role() Roger Quadros
2019-10-16  4:32   ` Pawel Laszczak
2019-10-16  9:20     ` Roger Quadros
2019-10-16  9:38       ` Pawel Laszczak
2019-10-16  9:54         ` Roger Quadros
2019-10-07 12:16 ` [PATCH 2/2] usb: cdns3: gadget: Fix full-speed mode Roger Quadros
2019-10-08  8:07   ` Peter Chen
2019-10-16  4:35     ` Pawel Laszczak

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git