linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] extcon: sm5502: Clear pending interrupts during initialization
@ 2019-10-08 10:54 ` Stephan Gerhold
  2019-10-10  7:33   ` Chanwoo Choi
  0 siblings, 1 reply; 5+ messages in thread
From: Stephan Gerhold @ 2019-10-08 10:54 UTC (permalink / raw)
  To: Chanwoo Choi, MyungJoo Ham; +Cc: linux-kernel, Stephan Gerhold

On some devices (e.g. Samsung Galaxy A5 (2015)), the bootloader
seems to keep interrupts enabled for SM5502 when booting Linux.
Changing the cable state (i.e. plugging in a cable) - until the driver
is loaded - will therefore produce an interrupt that is never read.

In this situation, the cable state will be stuck forever on the
initial state because SM5502 stops sending interrupts.
This can be avoided by clearing those pending interrupts after
the driver has been loaded.

Reading the interrupt status registers twice seems to be sufficient
to make interrupts work in this situation.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
This makes interrupts work on the Samsung Galaxy A5 (2015), which
has recently gained mainline support [1].

I was not able to find a datasheet for SM5502, so this patch is
merely based on testing and comparison with the downstream driver [2].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1329c1ab0730b521e6cd3051c56a2ff3d55f21e6
[2]: https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/SM-A500FU/drivers/misc/sm5502.c#L1566-L1578
---
 drivers/extcon/extcon-sm5502.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
index dc43847ad2b0..c897f1aa4bf5 100644
--- a/drivers/extcon/extcon-sm5502.c
+++ b/drivers/extcon/extcon-sm5502.c
@@ -539,6 +539,12 @@ static void sm5502_init_dev_type(struct sm5502_muic_info *info)
 			val = info->reg_data[i].val;
 		regmap_write(info->regmap, info->reg_data[i].reg, val);
 	}
+
+	/* Clear pending interrupts */
+	regmap_read(info->regmap, SM5502_REG_INT1, &reg_data);
+	regmap_read(info->regmap, SM5502_REG_INT2, &reg_data);
+	regmap_read(info->regmap, SM5502_REG_INT1, &reg_data);
+	regmap_read(info->regmap, SM5502_REG_INT2, &reg_data);
 }
 
 static int sm5022_muic_i2c_probe(struct i2c_client *i2c,
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: sm5502: Clear pending interrupts during initialization
  2019-10-08 10:54 ` [PATCH] extcon: sm5502: Clear pending interrupts during initialization Stephan Gerhold
@ 2019-10-10  7:33   ` Chanwoo Choi
  2019-10-10  7:46     ` Chanwoo Choi
  0 siblings, 1 reply; 5+ messages in thread
From: Chanwoo Choi @ 2019-10-10  7:33 UTC (permalink / raw)
  To: Stephan Gerhold, MyungJoo Ham; +Cc: linux-kernel

On 19. 10. 8. 오후 7:54, Stephan Gerhold wrote:
> On some devices (e.g. Samsung Galaxy A5 (2015)), the bootloader
> seems to keep interrupts enabled for SM5502 when booting Linux.
> Changing the cable state (i.e. plugging in a cable) - until the driver
> is loaded - will therefore produce an interrupt that is never read.
> 
> In this situation, the cable state will be stuck forever on the
> initial state because SM5502 stops sending interrupts.
> This can be avoided by clearing those pending interrupts after
> the driver has been loaded.
> 
> Reading the interrupt status registers twice seems to be sufficient
> to make interrupts work in this situation.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> This makes interrupts work on the Samsung Galaxy A5 (2015), which
> has recently gained mainline support [1].
> 
> I was not able to find a datasheet for SM5502, so this patch is
> merely based on testing and comparison with the downstream driver [2].
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1329c1ab0730b521e6cd3051c56a2ff3d55f21e6
> [2]: https://protect2.fireeye.com/url?k=029d0042-5ffa4464-029c8b0d-0cc47a31384a-14ac0bce09798d1f&u=https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/SM-A500FU/drivers/misc/sm5502.c#L1566-L1578
> ---
>  drivers/extcon/extcon-sm5502.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
> index dc43847ad2b0..c897f1aa4bf5 100644
> --- a/drivers/extcon/extcon-sm5502.c
> +++ b/drivers/extcon/extcon-sm5502.c
> @@ -539,6 +539,12 @@ static void sm5502_init_dev_type(struct sm5502_muic_info *info)
>  			val = info->reg_data[i].val;
>  		regmap_write(info->regmap, info->reg_data[i].reg, val);
>  	}
> +
> +	/* Clear pending interrupts */
> +	regmap_read(info->regmap, SM5502_REG_INT1, &reg_data);
> +	regmap_read(info->regmap, SM5502_REG_INT2, &reg_data);
> +	regmap_read(info->regmap, SM5502_REG_INT1, &reg_data);
> +	regmap_read(info->regmap, SM5502_REG_INT2, &reg_data);

It is not proper. Instead, initialize the SM5502_RET_INT1/2 with zero.

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,
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: sm5502: Clear pending interrupts during initialization
  2019-10-10  7:33   ` Chanwoo Choi
@ 2019-10-10  7:46     ` Chanwoo Choi
  2019-10-10 10:15       ` Stephan Gerhold
  0 siblings, 1 reply; 5+ messages in thread
From: Chanwoo Choi @ 2019-10-10  7:46 UTC (permalink / raw)
  To: Stephan Gerhold, MyungJoo Ham; +Cc: linux-kernel

On 19. 10. 10. 오후 4:33, Chanwoo Choi wrote:
> On 19. 10. 8. 오후 7:54, Stephan Gerhold wrote:
>> On some devices (e.g. Samsung Galaxy A5 (2015)), the bootloader
>> seems to keep interrupts enabled for SM5502 when booting Linux.
>> Changing the cable state (i.e. plugging in a cable) - until the driver
>> is loaded - will therefore produce an interrupt that is never read.
>>
>> In this situation, the cable state will be stuck forever on the
>> initial state because SM5502 stops sending interrupts.
>> This can be avoided by clearing those pending interrupts after
>> the driver has been loaded.
>>
>> Reading the interrupt status registers twice seems to be sufficient
>> to make interrupts work in this situation.
>>
>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>> ---
>> This makes interrupts work on the Samsung Galaxy A5 (2015), which
>> has recently gained mainline support [1].
>>
>> I was not able to find a datasheet for SM5502, so this patch is
>> merely based on testing and comparison with the downstream driver [2].
>>
>> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1329c1ab0730b521e6cd3051c56a2ff3d55f21e6
>> [2]: https://protect2.fireeye.com/url?k=029d0042-5ffa4464-029c8b0d-0cc47a31384a-14ac0bce09798d1f&u=https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/SM-A500FU/drivers/misc/sm5502.c#L1566-L1578
>> ---
>>  drivers/extcon/extcon-sm5502.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
>> index dc43847ad2b0..c897f1aa4bf5 100644
>> --- a/drivers/extcon/extcon-sm5502.c
>> +++ b/drivers/extcon/extcon-sm5502.c
>> @@ -539,6 +539,12 @@ static void sm5502_init_dev_type(struct sm5502_muic_info *info)
>>  			val = info->reg_data[i].val;
>>  		regmap_write(info->regmap, info->reg_data[i].reg, val);
>>  	}
>> +
>> +	/* Clear pending interrupts */
>> +	regmap_read(info->regmap, SM5502_REG_INT1, &reg_data);
>> +	regmap_read(info->regmap, SM5502_REG_INT2, &reg_data);
>> +	regmap_read(info->regmap, SM5502_REG_INT1, &reg_data);
>> +	regmap_read(info->regmap, SM5502_REG_INT2, &reg_data);
> 
> It is not proper. Instead, initialize the SM5502_RET_INT1/2 with zero.
> 
> 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,
>>
> 
> 

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:


diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
index c897f1aa4bf5..96a578d2533a 100644
--- a/drivers/extcon/extcon-sm5502.c
+++ b/drivers/extcon/extcon-sm5502.c
@@ -65,9 +65,22 @@ 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,
+       }, {
+               .reg = SM5502_REG_INT1,
+               .val = SM5502_REG_INT1_MASK,
+               .invert = true,
+       }, {
+               .reg = SM5502_REG_INT2,
+               .val = SM5502_REG_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..2db62e093ef3 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_REG_INT1_MASK                   (0xff)
+
 #define SM5502_REG_INTM1_ATTACH_SHIFT          0
 #define SM5502_REG_INTM1_DETACH_SHIFT          1
 #define SM5502_REG_INTM1_KP_SHIFT              2
@@ -237,6 +239,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_RER_RESET_MASK                  (0x1)
+
 /* SM5502 Interrupts */
 enum sm5502_irq {
        /* INT1 */





-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: sm5502: Clear pending interrupts during initialization
  2019-10-10  7:46     ` Chanwoo Choi
@ 2019-10-10 10:15       ` Stephan Gerhold
  2019-10-10 10:29         ` Chanwoo Choi
  0 siblings, 1 reply; 5+ messages in thread
From: Stephan Gerhold @ 2019-10-10 10:15 UTC (permalink / raw)
  To: Chanwoo Choi; +Cc: MyungJoo Ham, linux-kernel

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 */

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] extcon: sm5502: Clear pending interrupts during initialization
  2019-10-10 10:15       ` Stephan Gerhold
@ 2019-10-10 10:29         ` Chanwoo Choi
  0 siblings, 0 replies; 5+ messages in thread
From: Chanwoo Choi @ 2019-10-10 10:29 UTC (permalink / raw)
  To: Stephan Gerhold; +Cc: MyungJoo Ham, linux-kernel

Hi Stephan,

On 19. 10. 10. 오후 7:15, Stephan Gerhold wrote:
> 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.

OK.

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

No. If SM5502_REG_RESET is enough to fix this issue, I'm OK.
Thanks.

> 
> 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 */
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-10 10:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20191008105726epcas3p178e8421ce5062b6955475199efa130e1@epcas3p1.samsung.com>
2019-10-08 10:54 ` [PATCH] extcon: sm5502: Clear pending interrupts during initialization Stephan Gerhold
2019-10-10  7:33   ` Chanwoo Choi
2019-10-10  7:46     ` Chanwoo Choi
2019-10-10 10:15       ` Stephan Gerhold
2019-10-10 10:29         ` Chanwoo Choi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).