| 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
| ||
|
Date: Thu, 10 Oct 2019 16:46:56 +0900
From: Chanwoo Choi <cw00.choi@...sung.com>
To: Stephan Gerhold <stephan@...hold.net>,
MyungJoo Ham <myungjoo.ham@...sung.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] extcon: sm5502: Clear pending interrupts during
initialization
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@...hold.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, ®_data);
>> + regmap_read(info->regmap, SM5502_REG_INT2, ®_data);
>> + regmap_read(info->regmap, SM5502_REG_INT1, ®_data);
>> + regmap_read(info->regmap, SM5502_REG_INT2, ®_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
Powered by blists - more mailing lists