ucsi_ccg: Check DEV_INT bit only when starting CCG4

PATCH Superseded Wayne Chang 3 participants 6 messages Jan 12, 2022

Subsystems (2)

USB USB Type-C class

lore

Share & tools

lore

Wayne Chang

author

2022-01-12 05:49 UTC #0 lore raw

PATCH v4 1/1 ucsi_ccg: Check DEV_INT bit only when starting CCG4

From: Sing-Han Chen <[email protected]>

after driver sending the UCSI_START cmd, CCGx would clear Bit 0:Device Interrupt in the INTR_REG if CCGX reset successfully.

however, there might be a chance that other bits in INTR_REG are not cleared due to internal data queued in PPM and cause the driver thinks CCGx reset failed.

the commit checks bit 0 in INTR_REG and ignore other bits. ucsi driver would reset PPM later.

Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")

Signed-off-by Sing-Han Chen <[email protected]>

Signed-off-by Wayne Chang <[email protected]>

---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index bff96d64dddf..6db7c8ddd51c 100644
@@ -325,7 +325,7 @@ static int ucsi_ccg_init(struct ucsi_ccg *uc)
 		if (status < 0)
 			return status;
 
-		if (!data)
+		if (!(data & DEV_INT))
 			return 0;
 
 		status = ccg_write(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));

Signature · 1 line

2.25.1

Greg KH

maintainer

2022-01-12 07:07 UTC #1 lore raw

Quoted 36 lines · Wayne Chang

From: Sing-Han Chen <[email protected]>
after driver sending the UCSI_START cmd, CCGx would clear Bit 0:Device Interrupt in the INTR_REG if CCGX reset successfully.
however, there might be a chance that other bits in INTR_REG are not cleared due to internal data queued in PPM and cause the driver thinks CCGx reset failed.
the commit checks bit 0 in INTR_REG and ignore other bits. ucsi driver would reset PPM later.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
Signed-off-by Sing-Han Chen <[email protected]>
Signed-off-by Wayne Chang <[email protected]>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +1 -1
index bff96d64dddf..6db7c8ddd51c 100644
@@ -325,7 +325,7 @@ static int ucsi_ccg_init(struct ucsi_ccg *uc)
 		if (status < 0)
 			return status;
 
-		if (!data)
+		if (!(data & DEV_INT))
 			return 0;
 
 		status = ccg_write(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
Signature · 1 line
2.25.1

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree.

You are receiving this message because of the following common error(s) as indicated below:

- This looks like a new version of a previously submitted patch, but you did not list below the --- line any changes from the previous version. Please read the section entitled "The canonical patch format" in the kernel file, Documentation/SubmittingPatches for what needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers.

thanks,

greg k-h's patch email bot

Heikki Krogerus

maintainer

2022-01-12 07:50 UTC #2 lore raw

In reply to Wayne Chang

Hi,

Quoted 6 lines · Wayne Chang

From: Sing-Han Chen <[email protected]>
after driver sending the UCSI_START cmd, CCGx would clear Bit 0:Device Interrupt in the INTR_REG if CCGX reset successfully.

Quoted 8 lines

however, there might be a chance that other bits in INTR_REG are not cleared due to internal data queued in PPM and cause the driver thinks CCGx reset failed.
the commit checks bit 0 in INTR_REG and ignore other bits. ucsi driver would reset PPM later.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")

So... no need to take this to the stable kernel releases?

Quoted 3 lines

Signed-off-by Sing-Han Chen <[email protected]>
Signed-off-by Wayne Chang <[email protected]>
---

What has changed since v3 (and v2) - there is no patch changelog here?

In case this is new to you, you list those changes under that '---' line so they don't become part of the permanent changelog:

https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format

Quoted 8 lines

 drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index bff96d64dddf..6db7c8ddd51c 100644

drivers/usb/typec/ucsi/ucsi_ccg.c static int ucsi_ccg_init(struct ucsi_ccg *uc) · line 331

Quoted 8 lines

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

@@ -325,7 +325,7 @@ static int ucsi_ccg_init(struct ucsi_ccg *uc)
 		if (status < 0)
 			return status;
 
-		if (!data)
+		if (!(data & DEV_INT))
 			return 0;
 
 		status = ccg_write(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));

thanks,

Signature · 1 line

heikki

Wayne Chang

author

2022-01-12 08:41 UTC #3 lore raw

Hi Heikki

Thanks for the review.

Quoted 15 lines · Heikki Krogerus

Hi,
Quoted 10 lines · Wayne Chang
From: Sing-Han Chen <[email protected]>
after driver sending the UCSI_START cmd, CCGx would clear Bit 0:Device Interrupt in the INTR_REG if CCGX reset successfully.
however, there might be a chance that other bits in INTR_REG are not cleared due to internal data queued in PPM and cause the driver thinks CCGx reset failed.

Quoted 7 lines

Quoted 4 lines
the commit checks bit 0 in INTR_REG and ignore other bits. ucsi driver would reset PPM later.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
So... no need to take this to the stable kernel releases?

Should I put cc tag here?

Quoted 11 lines

Quoted 3 lines
Signed-off-by Sing-Han Chen <[email protected]>
Signed-off-by Wayne Chang <[email protected]>
---
What has changed since v3 (and v2) - there is no patch changelog here?
In case this is new to you, you list those changes under that '---' line so they don't become part of the permanent changelog:
https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format

The changes are resent for updating the commit messages.
Added the Fixes tag which Greg asked for.
Thanks for information. I went through the documents.
Should I put the following messages in the patch after "---" or should I use resend in subject for the commit messages updated?
v3 -> v4 commit messages updated
v2 -> v3 commit messages updated
v1 -> v2 commit messages updated

Quoted 15 lines

  drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index bff96d64dddf..6db7c8ddd51c 100644
@@ -325,7 +325,7 @@ static int ucsi_ccg_init(struct ucsi_ccg *uc)
               if (status < 0)
                       return status;
 
-             if (!data)
+             if (!(data & DEV_INT))
                       return 0;

Quoted 7 lines

drivers/usb/typec/ucsi/ucsi_ccg.c static int ucsi_ccg_init(struct ucsi_ccg *uc) · line 331
Quoted 1 line
drivers/usb/typec/ucsi/ucsi_ccg.c +0 -0
               status = ccg_write(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
thanks,
--
heikki

thanks,
Wayne.

Heikki Krogerus

maintainer

2022-01-12 09:09 UTC #4 lore raw

Quoted 20 lines · Wayne Chang

Hi Heikki
Thanks for the review.
Quoted 15 lines · Heikki Krogerus
Hi,
Quoted 10 lines · Wayne Chang
From: Sing-Han Chen <[email protected]>
after driver sending the UCSI_START cmd, CCGx would clear Bit 0:Device Interrupt in the INTR_REG if CCGX reset successfully.
however, there might be a chance that other bits in INTR_REG are not cleared due to internal data queued in PPM and cause the driver thinks CCGx reset failed.

Quoted 8 lines

Quoted 7 lines
Quoted 4 lines
the commit checks bit 0 in INTR_REG and ignore other bits. ucsi driver would reset PPM later.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
So... no need to take this to the stable kernel releases?
Should I put cc tag here?

Yes, Cc: [email protected], unless you have some good reason not to. I'm guessing you don't. This stuff is also explained here:

https://docs.kernel.org/process/submitting-patches.html#select-the-recipients-for-your-patch

Quoted 9 lines

Quoted 9 lines
Quoted 3 lines
Signed-off-by Sing-Han Chen <[email protected]>
Signed-off-by Wayne Chang <[email protected]>
---
What has changed since v3 (and v2) - there is no patch changelog here?
In case this is new to you, you list those changes under that '---' line so they don't become part of the permanent changelog:

Quoted 8 lines

Quoted 2 lines
https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
The changes are resent for updating the commit messages.
Added the Fixes tag which Greg asked for.
Thanks for information. I went through the documents.
Should I put the following messages in the patch after "---" or should I use resend in subject for the commit messages updated?
v3 -> v4 commit messages updated

I think you can say exactly what you did, like "Added the Fixes tag" or whatever you did.

Quoted 1 line

v2 -> v3 commit messages updated

Ditto.

Quoted 1 line

v1 -> v2 commit messages updated

Ditto.

Since you are going to do one more version of this, could you also start the sentences with upper-case letters in the commit message? Normally I'm not very picky about the grammar stuff (I make plenty of mistakes myself), but this time it's driving me crazy for some reason.

thanks,

Signature · 1 line

heikki

Wayne Chang

author

2022-01-12 09:36 UTC #5 lore raw

Quoted 28 lines · Heikki Krogerus

Quoted 25 lines · Wayne Chang
Hi Heikki
Thanks for the review.
Quoted 20 lines · Heikki Krogerus
Hi,
Quoted 14 lines · Wayne Chang
From: Sing-Han Chen <[email protected]>
after driver sending the UCSI_START cmd, CCGx would clear Bit 0:Device Interrupt in the INTR_REG if CCGX reset successfully.
however, there might be a chance that other bits in INTR_REG are not cleared due to internal data queued in PPM and cause the driver thinks CCGx reset failed.
the commit checks bit 0 in INTR_REG and ignore other bits. ucsi driver would reset PPM later.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")

Quoted 9 lines

Quoted 3 lines
Quoted 2 lines
So... no need to take this to the stable kernel releases?
Should I put cc tag here?
Yes, Cc: [email protected], unless you have some good reason not to. I'm guessing you don't. This stuff is also explained here:
https://docs.kernel.org/process/submitting-patches.html#select-the-recipients-for-your-patch

Thanks for the information. I'll add the Cc tag in the next version.

Quoted 25 lines

Quoted 9 lines
Quoted 9 lines
Quoted 3 lines
Signed-off-by Sing-Han Chen <[email protected]>
Signed-off-by Wayne Chang <[email protected]>
---
What has changed since v3 (and v2) - there is no patch changelog here?
In case this is new to you, you list those changes under that '---' line so they don't become part of the permanent changelog:
Quoted 8 lines
Quoted 2 lines
https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
The changes are resent for updating the commit messages.
Added the Fixes tag which Greg asked for.
Thanks for information. I went through the documents.
Should I put the following messages in the patch after "---" or should I use resend in subject for the commit messages updated?
v3 -> v4 commit messages updated
I think you can say exactly what you did, like "Added the Fixes tag" or whatever you did.
Quoted 1 line
v2 -> v3 commit messages updated
Ditto.

Quoted 9 lines

Quoted 1 line
v1 -> v2 commit messages updated
Ditto.
Since you are going to do one more version of this, could you also start the sentences with upper-case letters in the commit message? Normally I'm not very picky about the grammar stuff (I make plenty of mistakes myself), but this time it's driving me crazy for some reason.

Sure, I'll also minimize the words in a sentence and make it readable. Thanks for the advice.

Quoted 5 lines

thanks,
--
heikki

thanks,
Wayne.