2021-11-08 01:46:28

by Ondřej Jirman

[permalink] [raw]
Subject: [PATCH 06/15] usb: typec: fusb302: Fix masking of comparator and bc_lvl interrupts

The masks are swapped (interrupts are enabled when the mask is 0).

This caused inability of the driver to recognize cable unplug events
in host mode (where only comparator interrupt is generated, and VBUS
interrupt is not).

Signed-off-by: Ondrej Jirman <[email protected]>
---
drivers/usb/typec/tcpm/fusb302.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 7a2a17866a823..72f9001b07921 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -669,25 +669,27 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
FUSB_REG_MASK_BC_LVL |
FUSB_REG_MASK_COMP_CHNG,
- FUSB_REG_MASK_COMP_CHNG);
+ FUSB_REG_MASK_BC_LVL);
if (ret < 0) {
fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
ret);
goto done;
}
chip->intr_comp_chng = true;
+ chip->intr_bc_lvl = false;
break;
case TYPEC_CC_RD:
ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
FUSB_REG_MASK_BC_LVL |
FUSB_REG_MASK_COMP_CHNG,
- FUSB_REG_MASK_BC_LVL);
+ FUSB_REG_MASK_COMP_CHNG);
if (ret < 0) {
fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
ret);
goto done;
}
chip->intr_bc_lvl = true;
+ chip->intr_comp_chng = false;
break;
default:
break;
--
2.33.1


2021-11-08 10:30:14

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH 06/15] usb: typec: fusb302: Fix masking of comparator and bc_lvl interrupts

On Mon, Nov 08, 2021 at 09:10:31AM +0100, Greg Kroah-Hartman wrote:
> On Sun, Nov 07, 2021 at 07:57:24PM +0100, Ondřej Jirman wrote:
> > Hi,
> >
> > On Sun, Nov 07, 2021 at 07:54:33PM +0100, megous hlavni wrote:
> > > The masks are swapped (interrupts are enabled when the mask is 0).
> >
> > Please ignore the 06/15 in the subject. This is just a single patch
> > from my local series and I forgot to edit the subject.
>
> But I see 2 patches sent in this series?

The other one is unrelated. This patch is a fix for a real bug.

The other seemed like an independent correctness issue that I noticed from code
review I had to do to figure out the reason for failure to handle disconnect
detection properly. It's unrelated to this patch other than touching the same driver.

regards,
o.

> Can you just fix this up and resend them correctly?
>
> thanks,
>
> greg k-h

2021-11-08 13:38:32

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 06/15] usb: typec: fusb302: Fix masking of comparator and bc_lvl interrupts

Hi,

On Mon, Nov 08, 2021 at 10:25:23AM +0100, Ondřej Jirman wrote:
> On Mon, Nov 08, 2021 at 09:10:31AM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Nov 07, 2021 at 07:57:24PM +0100, Ondřej Jirman wrote:
> > > Hi,
> > >
> > > On Sun, Nov 07, 2021 at 07:54:33PM +0100, megous hlavni wrote:
> > > > The masks are swapped (interrupts are enabled when the mask is 0).
> > >
> > > Please ignore the 06/15 in the subject. This is just a single patch
> > > from my local series and I forgot to edit the subject.
> >
> > But I see 2 patches sent in this series?
>
> The other one is unrelated. This patch is a fix for a real bug.

If this fixes a bug, then please include the appropriate Fixes tag and
CC [email protected]. I'm guessing this is what the Fixes tag
should look like:

Fixes: 48242e30532b ("usb: typec: fusb302: Revert "Resolve fixed power role contract setup"")

You probable also want to CC Hans de Goede <[email protected]> since
you are patching his code.

> The other seemed like an independent correctness issue that I noticed from code
> review I had to do to figure out the reason for failure to handle disconnect
> detection properly. It's unrelated to this patch other than touching the same driver.

It still says "PATCH 07/15..." which means it's a part of a series,
no? So..

> > Can you just fix this up and resend them correctly?

thanks,

--
heikki

2021-11-08 15:44:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 06/15] usb: typec: fusb302: Fix masking of comparator and bc_lvl interrupts

On Mon, Nov 08, 2021 at 10:25:23AM +0100, Ondřej Jirman wrote:
> On Mon, Nov 08, 2021 at 09:10:31AM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Nov 07, 2021 at 07:57:24PM +0100, Ondřej Jirman wrote:
> > > Hi,
> > >
> > > On Sun, Nov 07, 2021 at 07:54:33PM +0100, megous hlavni wrote:
> > > > The masks are swapped (interrupts are enabled when the mask is 0).
> > >
> > > Please ignore the 06/15 in the subject. This is just a single patch
> > > from my local series and I forgot to edit the subject.
> >
> > But I see 2 patches sent in this series?
>
> The other one is unrelated. This patch is a fix for a real bug.
>
> The other seemed like an independent correctness issue that I noticed from code
> review I had to do to figure out the reason for failure to handle disconnect
> detection properly. It's unrelated to this patch other than touching the same driver.

Please resend anything you wish to have reviewed and applied properly,
with the correct subject lines, so we can do so and do not get confused.

Remember, we deal with thousands of patches a week...

thanks,

greg k-h