diff mbox series

[v4,1/3] usb: renesas_usbhs: simplify usbhs_status_get_device_state()

Message ID 1568043974-1236-1-git-send-email-external.veeraiyan.c@de.adit-jv.com (mailing list archive)
State Superseded
Headers show
Series [v4,1/3] usb: renesas_usbhs: simplify usbhs_status_get_device_state() | expand

Commit Message

veeraiyan chidambaram Sept. 9, 2019, 3:46 p.m. UTC 
From: Eugeniu Rosca <erosca@de.adit-jv.com>

Similar to usbhs_status_get_ctrl_stage(), *_get_device_state() is not
supposed to return any error code since its return value is the DVSQ
bitfield of the INTSTS0 register. According to SoC HW manual rev1.00,
every single value of DVSQ[2:0] is valid and none is an error:

----8<----
Device State
000: Powered state
001: Default state
010: Address state
011: Configuration state
1xx: Suspended state
----8<----

Hence, simplify the function body. The motivation behind dropping the
switch/case construct is being able to implement reading the suspended
state. The latter (based on the above DVSQ[2:0] description) doesn't
have a unique value, but is rather a list of states (which makes
switch/case less suitable for reading/validating it):

100: (Suspended) Powered state
101: (Suspended) Default state
110: (Suspended) Address state
111: (Suspended) Configuration state

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Signed-off-by: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>
---
v4: patch sequence change
v3: https://patchwork.kernel.org/patch/11137697/
v2: https://patchwork.kernel.org/patch/11135111/
v1: https://patchwork.kernel.org/patch/10581479/

 drivers/usb/renesas_usbhs/mod.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

Comments

Yoshihiro Shimoda Sept. 10, 2019, 4:41 a.m. UTC | #1 
Hi Veeraiyan,

> From: Veeraiyan Chidambaram, Sent: Tuesday, September 10, 2019 12:46 AM
<snip>
> 
> Similar to usbhs_status_get_ctrl_stage(), *_get_device_state() is not
> supposed to return any error code since its return value is the DVSQ
> bitfield of the INTSTS0 register. According to SoC HW manual rev1.00,
> every single value of DVSQ[2:0] is valid and none is an error:
> 
> ----8<----
> Device State
> 000: Powered state
> 001: Default state
> 010: Address state
> 011: Configuration state
> 1xx: Suspended state
> ----8<----
> 
> Hence, simplify the function body. The motivation behind dropping the
> switch/case construct is being able to implement reading the suspended
> state. The latter (based on the above DVSQ[2:0] description) doesn't
> have a unique value, but is rather a list of states (which makes
> switch/case less suitable for reading/validating it):
> 
> 100: (Suspended) Powered state
> 101: (Suspended) Default state
> 110: (Suspended) Address state
> 111: (Suspended) Configuration state
> 
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> Signed-off-by: Veeraiyan Chidambaram <veeraiyan.chidambaram@in.bosch.com>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda
diff mbox series

Patch

diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
index 7475c4f64724..4fbb1d538b82 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -170,17 +170,7 @@  void usbhs_mod_remove(struct usbhs_priv *priv)
  */
 int usbhs_status_get_device_state(struct usbhs_irq_state *irq_state)
 {
-	int state = irq_state->intsts0 & DVSQ_MASK;
-
-	switch (state) {
-	case POWER_STATE:
-	case DEFAULT_STATE:
-	case ADDRESS_STATE:
-	case CONFIGURATION_STATE:
-		return state;
-	}
-
-	return -EIO;
+	return (int)irq_state->intsts0 & DVSQ_MASK;
 }
 
 int usbhs_status_get_ctrl_stage(struct usbhs_irq_state *irq_state)