[PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Peter Chen]

After that, the role switch device (eg, Type-C device) could call
cdns3_role_set to finish the role switch.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 4aafba20f450..704c679a0c5d 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -528,6 +528,8 @@ static int cdns3_probe(struct platform_device *pdev)
 	sw_desc.get = cdns3_role_get;
 	sw_desc.allow_userspace_control = true;
 	sw_desc.driver_data = cdns;
+	if (device_property_read_bool(dev, "usb-role-switch"))
+		sw_desc.fwnode = dev->fwnode;
 
 	cdns->role_sw = usb_role_switch_register(dev, &sw_desc);
 	if (IS_ERR(cdns->role_sw)) {
-- 
2.17.1

[PATCH 2/4] usb: cdns3: delete role_override

[Peter Chen]

In short, we have three kinds of role switches:
- Based on SoC: ID and VBUS
- Based on external connnctor, eg, Type-C or GPIO Connector
- Based on user choices through sysfs

Since HW handling and usb-role-switch handling are at
different places, we do not need role_override any more,
and this flag could not judge external connector case well.

With role_override deleted, We use cdns3_hw_role_switch for
the 1st use case, and usb-role-switch for the 2nd and 3rd cases.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/core.c | 37 -------------------------------------
 drivers/usb/cdns3/core.h |  2 --
 2 files changed, 39 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 704c679a0c5d..f57c66a9f87c 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -291,10 +291,6 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
 	enum usb_role real_role, current_role;
 	int ret = 0;
 
-	/* Do nothing if role based on syfs. */
-	if (cdns->role_override)
-		return 0;
-
 	pm_runtime_get_sync(cdns->dev);
 
 	current_role = cdns->role;
@@ -353,39 +349,6 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
 
 	pm_runtime_get_sync(cdns->dev);
 
-	/*
-	 * FIXME: switch role framework should be extended to meet
-	 * requirements. Driver assumes that role can be controlled
-	 * by SW or HW. Temporary workaround is to use USB_ROLE_NONE to
-	 * switch from SW to HW control.
-	 *
-	 * For dr_mode == USB_DR_MODE_OTG:
-	 *	if user sets USB_ROLE_HOST or USB_ROLE_DEVICE then driver
-	 *	sets role_override flag and forces that role.
-	 *	if user sets USB_ROLE_NONE, driver clears role_override and lets
-	 *	HW state machine take over.
-	 *
-	 * For dr_mode != USB_DR_MODE_OTG:
-	 *	Assumptions:
-	 *	1. Restricted user control between NONE and dr_mode.
-	 *	2. Driver doesn't need to rely on role_override flag.
-	 *	3. Driver needs to ensure that HW state machine is never called
-	 *	   if dr_mode != USB_DR_MODE_OTG.
-	 */
-	if (role == USB_ROLE_NONE)
-		cdns->role_override = 0;
-	else
-		cdns->role_override = 1;
-
-	/*
-	 * HW state might have changed so driver need to trigger
-	 * HW state machine if dr_mode == USB_DR_MODE_OTG.
-	 */
-	if (!cdns->role_override && cdns->dr_mode == USB_DR_MODE_OTG) {
-		cdns3_hw_role_switch(cdns);
-		goto pm_put;
-	}
-
 	if (cdns->role == role)
 		goto pm_put;
 
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 969eb94de204..1ad1f1fe61e9 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -62,7 +62,6 @@ struct cdns3_role_driver {
  *           This field based on firmware setting, kernel configuration
  *           and hardware configuration.
  * @role_sw: pointer to role switch object.
- * @role_override: set 1 if role rely on SW.
  */
 struct cdns3 {
 	struct device			*dev;
@@ -90,7 +89,6 @@ struct cdns3 {
 	struct mutex			mutex;
 	enum usb_dr_mode		dr_mode;
 	struct usb_role_switch		*role_sw;
-	int				role_override;
 };
 
 int cdns3_hw_role_switch(struct cdns3 *cdns);
-- 
2.17.1

[PATCH 3/4] usb: cdns3: change "cdsn3" to"cdns3"

[Peter Chen]

And delete cdsn3_hw_role_state_machine declare which doesn't
be needed.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/core.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index f57c66a9f87c..19bbb5b7e6b6 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -82,8 +82,6 @@ static void cdns3_exit_roles(struct cdns3 *cdns)
 	cdns3_drd_exit(cdns);
 }
 
-static enum usb_role cdsn3_hw_role_state_machine(struct cdns3 *cdns);
-
 /**
  * cdns3_core_init_role - initialize role of operation
  * @cdns: Pointer to cdns3 structure
@@ -193,12 +191,12 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
 }
 
 /**
- * cdsn3_hw_role_state_machine  - role switch state machine based on hw events.
+ * cdns3_hw_role_state_machine  - role switch state machine based on hw events.
  * @cdns: Pointer to controller structure.
  *
  * Returns next role to be entered based on hw events.
  */
-static enum usb_role cdsn3_hw_role_state_machine(struct cdns3 *cdns)
+static enum usb_role cdns3_hw_role_state_machine(struct cdns3 *cdns)
 {
 	enum usb_role role;
 	int id, vbus;
@@ -294,7 +292,7 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
 	pm_runtime_get_sync(cdns->dev);
 
 	current_role = cdns->role;
-	real_role = cdsn3_hw_role_state_machine(cdns);
+	real_role = cdns3_hw_role_state_machine(cdns);
 
 	/* Do nothing if nothing changed */
 	if (current_role == real_role)
-- 
2.17.1

[PATCH 4/4] usb: cdns3: change dev_info to dev_dbg for debug message

[Peter Chen]

During device mode initialization, lots of device information
are printed to console, see below. Change them as debug message.

cdns-usb3 5b130000.cdns3: Initialized  ep0 support:
cdns-usb3 5b130000.cdns3: Initialized  ep1out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep2out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep3out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep4out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep5out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep6out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep7out support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep1in support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep2in support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep3in support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep4in support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep5in support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep6in support: BULK, INT ISO
cdns-usb3 5b130000.cdns3: Initialized  ep7in support: BULK, INT ISO

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/drd.c    | 4 ++--
 drivers/usb/cdns3/gadget.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 16ad485f0b69..58089841ed52 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -329,7 +329,7 @@ int cdns3_drd_init(struct cdns3 *cdns)
 		cdns->otg_v1_regs = NULL;
 		cdns->otg_regs = regs;
 		writel(1, &cdns->otg_v0_regs->simulate);
-		dev_info(cdns->dev, "DRD version v0 (%08x)\n",
+		dev_dbg(cdns->dev, "DRD version v0 (%08x)\n",
 			 readl(&cdns->otg_v0_regs->version));
 	} else {
 		cdns->otg_v0_regs = NULL;
@@ -337,7 +337,7 @@ int cdns3_drd_init(struct cdns3 *cdns)
 		cdns->otg_regs = (void *)&cdns->otg_v1_regs->cmd;
 		cdns->version  = CDNS3_CONTROLLER_V1;
 		writel(1, &cdns->otg_v1_regs->simulate);
-		dev_info(cdns->dev, "DRD version v1 (ID: %08x, rev: %08x)\n",
+		dev_dbg(cdns->dev, "DRD version v1 (ID: %08x, rev: %08x)\n",
 			 readl(&cdns->otg_v1_regs->did),
 			 readl(&cdns->otg_v1_regs->rid));
 	}
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index 3c05080a9ad5..2dab1da4a2bc 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -2954,7 +2954,7 @@ static int cdns3_init_eps(struct cdns3_device *priv_dev)
 
 		priv_ep->flags = 0;
 
-		dev_info(priv_dev->dev, "Initialized  %s support: %s %s\n",
+		dev_dbg(priv_dev->dev, "Initialized  %s support: %s %s\n",
 			 priv_ep->name,
 			 priv_ep->endpoint.caps.type_bulk ? "BULK, INT" : "",
 			 priv_ep->endpoint.caps.type_iso ? "ISO" : "");
-- 
2.17.1

Re: [PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Roger Quadros]

Hi Peter,

On 31/03/2020 11:10, Peter Chen wrote:
> After that, the role switch device (eg, Type-C device) could call
> cdns3_role_set to finish the role switch.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>

This does not apply on v5.6. What branch did you test this on?

cheers,
-roger

> ---
>   drivers/usb/cdns3/core.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 4aafba20f450..704c679a0c5d 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -528,6 +528,8 @@ static int cdns3_probe(struct platform_device *pdev)
>   	sw_desc.get = cdns3_role_get;
>   	sw_desc.allow_userspace_control = true;
>   	sw_desc.driver_data = cdns;
> +	if (device_property_read_bool(dev, "usb-role-switch"))
> +		sw_desc.fwnode = dev->fwnode;
>   
>   	cdns->role_sw = usb_role_switch_register(dev, &sw_desc);
>   	if (IS_ERR(cdns->role_sw)) {
> 

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

RE: [PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Peter Chen]

 
> 
> On 31/03/2020 11:10, Peter Chen wrote:
> > After that, the role switch device (eg, Type-C device) could call
> > cdns3_role_set to finish the role switch.
> >
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> 
> This does not apply on v5.6. What branch did you test this on?
> 

Hi Roger,
 
I use 5.6-rc4.  Which conflict you have met? Do you have below patch?

8708a9fa2938 usb: roles: Provide the switch drivers handle to the switch in the API

Peter

Re: [PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Roger Quadros]

On 02/04/2020 12:24, Peter Chen wrote:
> 
>   
>>
>> On 31/03/2020 11:10, Peter Chen wrote:
>>> After that, the role switch device (eg, Type-C device) could call
>>> cdns3_role_set to finish the role switch.
>>>
>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>
>> This does not apply on v5.6. What branch did you test this on?
>>
> 
> Hi Roger,
>   
> I use 5.6-rc4.  Which conflict you have met? Do you have below patch?
> 
> 8708a9fa2938 usb: roles: Provide the switch drivers handle to the switch in the API

Applying: usb: cdns3: core: get role switch node from firmware
error: patch failed: drivers/usb/cdns3/core.c:528
error: drivers/usb/cdns3/core.c: patch does not apply
Patch failed at 0001 usb: cdns3: core: get role switch node from firmware
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

RE: [PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Peter Chen]

> >>
> >
> > Hi Roger,
> >
> > I use 5.6-rc4.  Which conflict you have met? Do you have below patch?
> >
> > 8708a9fa2938 usb: roles: Provide the switch drivers handle to the
> > switch in the API
> 
> Applying: usb: cdns3: core: get role switch node from firmware
> error: patch failed: drivers/usb/cdns3/core.c:528
> error: drivers/usb/cdns3/core.c: patch does not apply Patch failed at 0001 usb:
> cdns3: core: get role switch node from firmware Use 'git am --show-current-patch'
> to see the failed patch When you have resolved this problem, run "git am --
> continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
 
I just tried the latest Greg's usb-next, it could apply. Do you have the patch I mentioned?
What's the conflict in source file?

Peter 

Re: [PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Roger Quadros]

On 02/04/2020 12:37, Peter Chen wrote:
>>>>
>>>
>>> Hi Roger,
>>>
>>> I use 5.6-rc4.  Which conflict you have met? Do you have below patch?
>>>
>>> 8708a9fa2938 usb: roles: Provide the switch drivers handle to the
>>> switch in the API
>>
>> Applying: usb: cdns3: core: get role switch node from firmware
>> error: patch failed: drivers/usb/cdns3/core.c:528
>> error: drivers/usb/cdns3/core.c: patch does not apply Patch failed at 0001 usb:
>> cdns3: core: get role switch node from firmware Use 'git am --show-current-patch'
>> to see the failed patch When you have resolved this problem, run "git am --
>> continue".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>>
>   
> I just tried the latest Greg's usb-next, it could apply. Do you have the patch I mentioned?
> What's the conflict in source file?
> 

OK, it applies on usb-next. Thanks.

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

Re: [PATCH 1/4] usb: cdns3: core: get role switch node from firmware

[Roger Quadros]

On 31/03/2020 11:10, Peter Chen wrote:
> After that, the role switch device (eg, Type-C device) could call
> cdns3_role_set to finish the role switch.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>

For this patch and all in the series

Reviewed-by: Roger Quadros <rogerq@ti.com>

> ---
>   drivers/usb/cdns3/core.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 4aafba20f450..704c679a0c5d 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -528,6 +528,8 @@ static int cdns3_probe(struct platform_device *pdev)
>   	sw_desc.get = cdns3_role_get;
>   	sw_desc.allow_userspace_control = true;
>   	sw_desc.driver_data = cdns;
> +	if (device_property_read_bool(dev, "usb-role-switch"))
> +		sw_desc.fwnode = dev->fwnode;
>   
>   	cdns->role_sw = usb_role_switch_register(dev, &sw_desc);
>   	if (IS_ERR(cdns->role_sw)) {
> 

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