usb: typec: ucsi_ccg: use dev_info for timeout error

PATCH In review Ajay Gupta 3 participants 5 messages Sep 21, 2020 – Sep 22, 2020

Subsystems (2)

USB USB Type-C class

lore

Share & tools

lore

Ajay Gupta

author

2020-09-21 17:42 UTC #0 lore raw

PATCH 1/1 usb: typec: ucsi_ccg: use dev_info for timeout error

From: Ajay Gupta <[email protected]>

Timeout error may be seen due to missing USB Type-C UCSI controller on some NVIDIA GPU card. Currently we don't have a correct way to identify these cards.

Tools like Plymouth (splashscreen) doesn't like dev_err so changing timeout status log to dev_info.

Bug information:
https://bugzilla.kernel.org/show_bug.cgi?id=206653

Signed-off-by Ajay Gupta <[email protected]>

---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/usb/typec/ucsi/ucsi_ccg.c +3 -3

index bff96d64dddf..d69432df866e 100644
@@ -252,7 +252,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 		put_unaligned_le16(rab, buf);
 		status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
 		if (status < 0) {
-			dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+			dev_info(uc->dev, "i2c_transfer failed %d\n", status);
 			pm_runtime_put_sync(uc->dev);
 			return status;
 		}

@@ -289,7 +289,7 @@ static int ccg_write(struct ucsi_ccg *uc, u16 rab, const u8 *data, u32 len)
 	pm_runtime_get_sync(uc->dev);
 	status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
 	if (status < 0) {
-		dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+		dev_info(uc->dev, "i2c_transfer failed %d\n", status);
 		pm_runtime_put_sync(uc->dev);
 		kfree(buf);
 		return status;

@@ -1345,7 +1345,7 @@ static int ucsi_ccg_probe(struct i2c_client *client,
 	/* reset ccg device and initialize ucsi */
 	status = ucsi_ccg_init(uc);
 	if (status < 0) {
-		dev_err(uc->dev, "ucsi_ccg_init failed - %d\n", status);
+		dev_info(uc->dev, "ucsi_ccg_init failed - %d\n", status);
 		return status;
 	}

Signature · 1 line

2.17.1

Greg KH

maintainer

2020-09-21 18:54 UTC #1 lore raw

Quoted 5 lines · Ajay Gupta

From: Ajay Gupta <[email protected]>
Timeout error may be seen due to missing USB Type-C UCSI controller on some NVIDIA GPU card. Currently we don't have a correct way to identify these cards.

No unique id anywhere??? That feels like someone really messed up :(

Quoted 17 lines

Tools like Plymouth (splashscreen) doesn't like dev_err so changing timeout status log to dev_info.
Bug information:
https://bugzilla.kernel.org/show_bug.cgi?id=206653
Signed-off-by Ajay Gupta <[email protected]>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +0 -0
index bff96d64dddf..d69432df866e 100644

drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) · line 255

Quoted 5 lines

drivers/usb/typec/ucsi/ucsi_ccg.c +1 -1

@@ -252,7 +252,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 		put_unaligned_le16(rab, buf);
 		status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
 		if (status < 0) {
-			dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+			dev_info(uc->dev, "i2c_transfer failed %d\n", status);

It's an error, leave it an error.

If it's not an error, don't log it.

Or better yet, fix the hardware :)

thanks,

greg k-h

Ajay Gupta

2020-09-21 20:55 UTC #2 lore raw

Hi Greg

Quoted 11 lines

-----Original Message-----
From: Greg KH <[email protected]>
Sent: Monday, September 21, 2020 11:55 AM
To: Ajay Gupta <[email protected]>
Cc: [email protected]; [email protected]; Ajay Gupta <[email protected]>
Subject: Re: [PATCH] usb: typec: ucsi_ccg: use dev_info for timeout error
External email: Use caution opening links or attachments

Quoted 8 lines

Quoted 5 lines · Ajay Gupta
From: Ajay Gupta <[email protected]>
Timeout error may be seen due to missing USB Type-C UCSI controller on some NVIDIA GPU card. Currently we don't have a correct way to identify these cards.
No unique id anywhere??? That feels like someone really messed up :(

Yes, there is none.

Quoted 27 lines

drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8
Quoted 19 lines
Tools like Plymouth (splashscreen) doesn't like dev_err so changing timeout status log to dev_info.
Bug information:
https://bugzilla.kernel.org/show_bug.cgi?id=206653
Signed-off-by Ajay Gupta <[email protected]>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +0 -0
index bff96d64dddf..d69432df866e 100644
@@ -252,7 +252,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8
*data, u32 len)
drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
Quoted 6 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +2 -1
              put_unaligned_le16(rab, buf);
              status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
              if (status < 0) {
-                     dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+                     dev_info(uc->dev, "i2c_transfer failed %d\n",
+ status);

Quoted 3 lines

It's an error, leave it an error.
If it's not an error, don't log it.

Looks like this is the best option we have.

Thanks

Quoted 7 lines

nvpublic
Or better yet, fix the hardware :)
thanks,
greg k-h

Greg KH

maintainer

2020-09-22 05:43 UTC #3 lore raw

Quoted 14 lines · Ajay Gupta

Hi Greg
Quoted 12 lines
-----Original Message-----
From: Greg KH <[email protected]>
Sent: Monday, September 21, 2020 11:55 AM
To: Ajay Gupta <[email protected]>
Cc: [email protected]; [email protected]; Ajay Gupta <[email protected]>
Subject: Re: [PATCH] usb: typec: ucsi_ccg: use dev_info for timeout error
External email: Use caution opening links or attachments
On Mon, Sep 21, 2020 at 10:42:16AM -0700, Ajay Gupta wrote:

Quoted 8 lines

Quoted 7 lines
Quoted 5 lines
From: Ajay Gupta <[email protected]>
Timeout error may be seen due to missing USB Type-C UCSI controller on some NVIDIA GPU card. Currently we don't have a correct way to identify these cards.
No unique id anywhere??? That feels like someone really messed up :(
Yes, there is none.

Can you identify them by the failure of an i2c transfer? Feels like that would work :)

Quoted 25 lines

Quoted 25 lines
drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8
Quoted 17 lines
Tools like Plymouth (splashscreen) doesn't like dev_err so changing timeout status log to dev_info.
Bug information:
https://bugzilla.kernel.org/show_bug.cgi?id=206653
Signed-off-by Ajay Gupta <[email protected]>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +0 -0
index bff96d64dddf..d69432df866e 100644
@@ -252,7 +252,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8
*data, u32 len)
Quoted 6 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +2 -1
              put_unaligned_le16(rab, buf);
              status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
              if (status < 0) {
-                     dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+                     dev_info(uc->dev, "i2c_transfer failed %d\n",
+ status);

Quoted 4 lines

Quoted 3 lines
It's an error, leave it an error.
If it's not an error, don't log it.
Looks like this is the best option we have.

It is an error, so we log it, that's fine. We are correctly reporting the fact that the hardware is broken and not working properly, don't paper over that please.

thanks,

greg k-h

Ajay Gupta

2020-09-22 23:25 UTC #4 lore raw

Hi Greg

Quoted 33 lines

-----Original Message-----
From: Greg KH <[email protected]>
Sent: Monday, September 21, 2020 10:44 PM
To: Ajay Gupta <[email protected]>
Cc: Ajay Gupta <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH] usb: typec: ucsi_ccg: use dev_info for timeout error
External email: Use caution opening links or attachments
Quoted 21 lines · Ajay Gupta
Hi Greg
Quoted 19 lines
-----Original Message-----
From: Greg KH <[email protected]>
Sent: Monday, September 21, 2020 11:55 AM
To: Ajay Gupta <[email protected]>
Cc: [email protected]; [email protected]; Ajay Gupta <[email protected]>
Subject: Re: [PATCH] usb: typec: ucsi_ccg: use dev_info for timeout error
External email: Use caution opening links or attachments
Quoted 5 lines · Ajay Gupta
From: Ajay Gupta <[email protected]>
Timeout error may be seen due to missing USB Type-C UCSI controller on some NVIDIA GPU card. Currently we don't have a correct way to identify these cards.

Quoted 6 lines

Quoted 3 lines
Quoted 2 lines
No unique id anywhere??? That feels like someone really messed up :(
Yes, there is none.
Can you identify them by the failure of an i2c transfer? Feels like that would work :)

Let me test it out and comeback.

Quoted 27 lines

Quoted 27 lines
Quoted 27 lines
Quoted 18 lines
Tools like Plymouth (splashscreen) doesn't like dev_err so changing timeout status log to dev_info.
Bug information:
https://bugzilla.kernel.org/show_bug.cgi?id=206653
Signed-off-by Ajay Gupta <[email protected]>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +0 -0
index bff96d64dddf..d69432df866e 100644
@@ -252,7 +252,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16
Quoted 1 line
rab, u8
*data, u32 len)
Quoted 6 lines
              put_unaligned_le16(rab, buf);
              status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
              if (status < 0) {
drivers/usb/typec/ucsi/ucsi_ccg.c +2 -1
-                     dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+                     dev_info(uc->dev, "i2c_transfer failed
+ %d\n", status);

Quoted 8 lines

Quoted 4 lines
Quoted 3 lines
It's an error, leave it an error.
If it's not an error, don't log it.
Looks like this is the best option we have.
It is an error, so we log it, that's fine. We are correctly reporting the fact that the hardware is broken and not working properly, don't paper over that please.

Ok.

Thanks

Quoted 5 lines

nvpublic
thanks,
greg k-h