[PATCH] extcon: max77843: Clear IRQ bits state before request IRQ

[PATCH] extcon: max77843: Clear IRQ bits state before request IRQ

[Jaewon Kim]

IRQ signal before driver probe is needless because driver sends
current state after platform booting done.
So, this patch clears MUIC IRQ bits before request IRQ.

Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
 drivers/extcon/extcon-max77843.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index d78a64d..11e09d1 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -781,6 +781,15 @@ static int max77843_muic_probe(struct platform_device *pdev)
 	/* Support virtual irq domain for max77843 MUIC device */
 	INIT_WORK(&info->irq_work, max77843_muic_irq_work);
 
+	/* Clear IRQ bits before request IRQs */
+	ret = regmap_bulk_read(max77843->regmap_muic,
+			MAX77843_MUIC_REG_INT1, info->status,
+			MAX77843_MUIC_IRQ_NUM);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to Clear IRQ bits\n");
+		goto err_muic_irq;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(max77843_muic_irqs); i++) {
 		struct max77843_muic_irq *muic_irq = &max77843_muic_irqs[i];
 		unsigned int virq = 0;
-- 
1.7.9.5

Re: [PATCH] extcon: max77843: Clear IRQ bits state before request IRQ

[Chanwoo Choi]

Hi Jaewon,

On 06/05/2015 01:32 PM, Jaewon Kim wrote:
> IRQ signal before driver probe is needless because driver sends
> current state after platform booting done.
> So, this patch clears MUIC IRQ bits before request IRQ.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> ---
>  drivers/extcon/extcon-max77843.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index d78a64d..11e09d1 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -781,6 +781,15 @@ static int max77843_muic_probe(struct platform_device *pdev)
>  	/* Support virtual irq domain for max77843 MUIC device */
>  	INIT_WORK(&info->irq_work, max77843_muic_irq_work);
>  
> +	/* Clear IRQ bits before request IRQs */
> +	ret = regmap_bulk_read(max77843->regmap_muic,
> +			MAX77843_MUIC_REG_INT1, info->status,
> +			MAX77843_MUIC_IRQ_NUM);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to Clear IRQ bits\n");
> +		goto err_muic_irq;
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(max77843_muic_irqs); i++) {
>  		struct max77843_muic_irq *muic_irq = &max77843_muic_irqs[i];
>  		unsigned int virq = 0;
> 

Applied it.

Thanks,
Chanwoo Choi

Re: [PATCH] extcon: max77843: Clear IRQ bits state before request IRQ

[MyungJoo Ham]

>   
>  IRQ signal before driver probe is needless because driver sends
> current state after platform booting done.
> So, this patch clears MUIC IRQ bits before request IRQ.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> ---
>  drivers/extcon/extcon-max77843.c |    9 +++++++++
>  1 file changed, 9 insertions(+)

Q1. Is this because the pending bits are USELESS?
or because the pendeing bits incurs INCORRECT behaviors?

Q2. Does clearing (by reading) INT1 do everything you need?
What about INT2 and INT3?

Q3. I presume that "driver sends current state after..." is
coming from the invokation of "queue_delayed_work()" at the end 
of the probe function. It appears that you are only serving
the pending status of "cable detection" with it while INT1
seems to have more functionalities. Does that delayed work
do everything that are pending, really?


Cheers,
MyungJoo

> 
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index d78a64d..11e09d1 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -781,6 +781,15 @@ static int max77843_muic_probe(struct platform_device *pdev)
>  	/* Support virtual irq domain for max77843 MUIC device */
>  	INIT_WORK(&info->irq_work, max77843_muic_irq_work);
>  
> +	/* Clear IRQ bits before request IRQs */
> +	ret = regmap_bulk_read(max77843->regmap_muic,
> +			MAX77843_MUIC_REG_INT1, info->status,
> +			MAX77843_MUIC_IRQ_NUM);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to Clear IRQ bits\n");
> +		goto err_muic_irq;
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(max77843_muic_irqs); i++) {
>  		struct max77843_muic_irq *muic_irq = &max77843_muic_irqs[i];
>  		unsigned int virq = 0;
> -- 
> 1.7.9.5
> 

Re: [PATCH] extcon: max77843: Clear IRQ bits state before request IRQ

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1898 bytes --]

>   
>  IRQ signal before driver probe is needless because driver sends
> current state after platform booting done.
> So, this patch clears MUIC IRQ bits before request IRQ.
> 
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> ---
>  drivers/extcon/extcon-max77843.c |    9 +++++++++
>  1 file changed, 9 insertions(+)

Q1. Is this because the pending bits are USELESS?
or because the pendeing bits incurs INCORRECT behaviors?

Q2. Does clearing (by reading) INT1 do everything you need?
What about INT2 and INT3?

Q3. I presume that "driver sends current state after..." is
coming from the invokation of "queue_delayed_work()" at the end 
of the probe function. It appears that you are only serving
the pending status of "cable detection" with it while INT1
seems to have more functionalities. Does that delayed work
do everything that are pending, really?


Cheers,
MyungJoo

> 
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index d78a64d..11e09d1 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -781,6 +781,15 @@ static int max77843_muic_probe(struct platform_device *pdev)
>  	/* Support virtual irq domain for max77843 MUIC device */
>  	INIT_WORK(&info->irq_work, max77843_muic_irq_work);
>  
> +	/* Clear IRQ bits before request IRQs */
> +	ret = regmap_bulk_read(max77843->regmap_muic,
> +			MAX77843_MUIC_REG_INT1, info->status,
> +			MAX77843_MUIC_IRQ_NUM);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to Clear IRQ bits\n");
> +		goto err_muic_irq;
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(max77843_muic_irqs); i++) {
>  		struct max77843_muic_irq *muic_irq = &max77843_muic_irqs[i];
>  		unsigned int virq = 0;
> -- 
> 1.7.9.5
> 
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] extcon: max77843: Clear IRQ bits state before request IRQ

[Chanwoo Choi]

On 06/05/2015 01:54 PM, MyungJoo Ham wrote:
>>   
>>  IRQ signal before driver probe is needless because driver sends
>> current state after platform booting done.
>> So, this patch clears MUIC IRQ bits before request IRQ.
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> ---
>>  drivers/extcon/extcon-max77843.c |    9 +++++++++
>>  1 file changed, 9 insertions(+)
> 
> Q1. Is this because the pending bits are USELESS?
> or because the pendeing bits incurs INCORRECT behaviors?

The max77843 datasheet includes following sentence:
- "All bits are cleared after a read" about INT1/INT2/INT3 register.
There are no problem about interrupt handling.

> 
> Q2. Does clearing (by reading) INT1 do everything you need?
> What about INT2 and INT3?

The MAXIM MAX77843 MUIC support the one more interrupts (e.g., ADC1K, VBVolt, ChgTyp ...).
The each interrupt is included in the one register among INT1/2/3.

This patch clear the all interrupts of MAX77843 before requesting the interrupts.

> 
> Q3. I presume that "driver sends current state after..." is
> coming from the invokation of "queue_delayed_work()" at the end 
> of the probe function. It appears that you are only serving
> the pending status of "cable detection" with it while INT1
> seems to have more functionalities. Does that delayed work
> do everything that are pending, really?

When completed kernel booting, the delayed work of extcon-max77843.c driver
use the MAX77843_MUIC_STATUSx register to detect the type of connected
external connectors. So, there are no problme about clearing all bits of INT1/2/3 interrupt register.

If user-space platform don't finish the initialization of all user-process daemons
and extcon driver send the uevent during only kernel booting, the uevent is not handled
on user-space daemons.

Thanks,
Chanwoo Choi