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] [day] [month] [year] [list]

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

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