linux-kernel - Re: [PATCH] extcon: sm5502: Clear pending interrupts during initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 10 Oct 2019 12:15:09 +0200
From:   Stephan Gerhold <stephan@...hold.net>
To:     Chanwoo Choi <cw00.choi@...sung.com>
Cc:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] extcon: sm5502: Clear pending interrupts during
 initialization

Hi Chanwoo,

thank you for your suggestions!

On Thu, Oct 10, 2019 at 04:46:56PM +0900, Chanwoo Choi wrote:
> On 19. 10. 10. 오후 4:33, Chanwoo Choi wrote:
> > It is not proper. Instead, initialize the SM5502_RET_INT1/2 with zero.

Sorry about that. I don't have a datasheet, so I wasn't sure what's the
best way to fix the problem.

> > 
> > The reset value of SM5502_RET_INT1/2 are zero (0x00) as following:
> > If you can test it with h/w, please try to test it and then
> > send the modified patch. 
> > 
> > diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
> > index c897f1aa4bf5..e168f77a18ba 100644
> > --- a/drivers/extcon/extcon-sm5502.c
> > +++ b/drivers/extcon/extcon-sm5502.c
> > @@ -68,6 +68,14 @@ static struct reg_data sm5502_reg_data[] = {
> >                 .reg = SM5502_REG_CONTROL,
> >                 .val = SM5502_REG_CONTROL_MASK_INT_MASK,
> >                 .invert = false,
> > +       }, {
> > +               .reg = SM5502_REG_INT1,
> > +               .val = SM5502_RET_INT1_MASK,
> > +               .invert = true,
> > +       }, {
> > +               .reg = SM5502_REG_INT2,
> > +               .val = SM5502_RET_INT1_MASK,
> > +               .invert = true,
> >         }, {
> >                 .reg = SM5502_REG_INTMASK1,
> >                 .val = SM5502_REG_INTM1_KP_MASK
> > diff --git a/drivers/extcon/extcon-sm5502.h b/drivers/extcon/extcon-sm5502.h
> > index 9dbb634d213b..5c4edb3e7fce 100644
> > --- a/drivers/extcon/extcon-sm5502.h
> > +++ b/drivers/extcon/extcon-sm5502.h
> > @@ -93,6 +93,8 @@ enum sm5502_reg {
> >  #define SM5502_REG_CONTROL_RAW_DATA_MASK       (0x1 << SM5502_REG_CONTROL_RAW_DATA_SHIFT)
> >  #define SM5502_REG_CONTROL_SW_OPEN_MASK                (0x1 << SM5502_REG_CONTROL_SW_OPEN_SHIFT)
> >  
> > +#define SM5502_RET_INT1_MASK                   (0xff)
> > +
> >  #define SM5502_REG_INTM1_ATTACH_SHIFT          0
> >  #define SM5502_REG_INTM1_DETACH_SHIFT          1
> >  #define SM5502_REG_INTM1_KP_SHIFT              2
> > 
> >>  }
> >>  
> >>  static int sm5022_muic_i2c_probe(struct i2c_client *i2c,
> >>
> > 
> > 

This patch (i.e. writing to SM5502_REG_INT1 and SM5502_REG_INT2)
does not result in any difference.
There are still no interrupts being sent.

On the other hand, your other suggestion fixes the problem:

> 
> When write 0x1 to SM5502_REG_RESET, reset the device.
> So, you can reset the all registers by writing 1 to SM5502_REG_RESET as following:
> 

Writing 0x1 to SM5502_REG_RESET seems to make interrupts work, so writing
to SM5502_REG_INT1 and SM5502_REG_INT2 is not even necessary in this case.

Would you still prefer initializing SM5502_REG_INT1/2 or is the patch
below enough?

diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
index dc43847ad2b0..b3d93baf4fc5 100644
--- a/drivers/extcon/extcon-sm5502.c
+++ b/drivers/extcon/extcon-sm5502.c
@@ -65,6 +65,10 @@ struct sm5502_muic_info {
 /* Default value of SM5502 register to bring up MUIC device. */
 static struct reg_data sm5502_reg_data[] = {
 	{
+		.reg = SM5502_REG_RESET,
+		.val = SM5502_REG_RESET_MASK,
+		.invert = true,
+	}, {
 		.reg = SM5502_REG_CONTROL,
 		.val = SM5502_REG_CONTROL_MASK_INT_MASK,
 		.invert = false,
diff --git a/drivers/extcon/extcon-sm5502.h b/drivers/extcon/extcon-sm5502.h
index 9dbb634d213b..2ea1bc01be0a 100644
--- a/drivers/extcon/extcon-sm5502.h
+++ b/drivers/extcon/extcon-sm5502.h
@@ -237,6 +237,8 @@ enum sm5502_reg {
 #define DM_DP_SWITCH_UART			((DM_DP_CON_SWITCH_UART <<SM5502_REG_MANUAL_SW1_DP_SHIFT) \
 						| (DM_DP_CON_SWITCH_UART <<SM5502_REG_MANUAL_SW1_DM_SHIFT))
 
+#define SM5502_REG_RESET_MASK		    (0x1)
+
 /* SM5502 Interrupts */
 enum sm5502_irq {
 	/* INT1 */