mfd: sec-irq: fix support for devices without irq specified

PATCH In review Bartlomiej Zolnierkiewicz 2 participants 5 messages Aug 7, 2014 – Aug 8, 2014

Subsystems (3)

Arm Samsung Exynos Multifunction devices (MFD) Samsung multifunction PMIC device drivers

lore

Share & tools

lore

Bartlomiej Zolnierkiewicz

author

2014-08-07 16:42 UTC #0 lore raw

PATCH 1/1 mfd: sec-irq: fix support for devices without irq specified

Add missing check for the case of device without irq specified in sec_irq_exit() (please note that sec_irq_init() already correctly handles such devices).

This is needed for Insignal's Exynos4412 based Origen board.

Cc Krzysztof Kozlowski <[email protected]>

Cc Sangbeom Kim <[email protected]>

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

Acked-by Kyungmin Park <[email protected]>

---
patch is against next-20140804 branch of linux-next kernel

 drivers/mfd/sec-irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mfd/sec-irq.c +2 -1

index f9a5786..b65a7f0 100644
@@ -478,5 +478,6 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 
 void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
 {
-	regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
+	if (sec_pmic->irq)
+		regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
 }

Signature · 1 line

1.8.2.3

Bartlomiej Zolnierkiewicz

author

2014-08-07 16:48 UTC #1 lore raw

[ added missing linux-samsung-soc ML, sorry for the noise ]

Quoted 28 lines · Bartlomiej Zolnierkiewicz

Add missing check for the case of device without irq specified in sec_irq_exit() (please note that sec_irq_init() already correctly handles such devices).
This is needed for Insignal's Exynos4412 based Origen board.
Cc Krzysztof Kozlowski <[email protected]>
Cc Sangbeom Kim <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by Kyungmin Park <[email protected]>
---
patch is against next-20140804 branch of linux-next kernel
 drivers/mfd/sec-irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mfd/sec-irq.c +2 -1
index f9a5786..b65a7f0 100644
@@ -478,5 +478,6 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 
 void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
 {
-	regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
+	if (sec_pmic->irq)
+		regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
 }

Krzysztof Kozlowski

Reviewed-by Tested-by

2014-08-08 07:34 UTC #2 lore raw

Quoted 18 lines · Bartlomiej Zolnierkiewicz

[ added missing linux-samsung-soc ML, sorry for the noise ]
Quoted 15 lines · Bartlomiej Zolnierkiewicz
Add missing check for the case of device without irq specified in sec_irq_exit() (please note that sec_irq_init() already correctly handles such devices).
This is needed for Insignal's Exynos4412 based Origen board.
Cc Krzysztof Kozlowski <[email protected]>
Cc Sangbeom Kim <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by Kyungmin Park <[email protected]>
---
patch is against next-20140804 branch of linux-next kernel
 drivers/mfd/sec-irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Looks and works good (tested on board with S2MPS14).

Reviewed-by Krzysztof Kozlowski <[email protected]>

Tested-by Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

Quoted 13 lines

drivers/mfd/sec-irq.c +2 -1

index f9a5786..b65a7f0 100644
@@ -478,5 +478,6 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 
 void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
 {
-	regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
+	if (sec_pmic->irq)
+		regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
 }

Krzysztof Kozlowski

2014-08-08 07:48 UTC #3 lore raw

Quoted 34 lines · Krzysztof Kozlowski

Quoted 18 lines · Bartlomiej Zolnierkiewicz
[ added missing linux-samsung-soc ML, sorry for the noise ]
Quoted 15 lines · Bartlomiej Zolnierkiewicz
Add missing check for the case of device without irq specified in sec_irq_exit() (please note that sec_irq_init() already correctly handles such devices).
This is needed for Insignal's Exynos4412 based Origen board.
Cc Krzysztof Kozlowski <[email protected]>
Cc Sangbeom Kim <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by Kyungmin Park <[email protected]>
---
patch is against next-20140804 branch of linux-next kernel
 drivers/mfd/sec-irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Looks and works good (tested on board with S2MPS14).
Reviewed-by Krzysztof Kozlowski <[email protected]>
Tested-by Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Quoted 7 lines
Quoted 7 lines
drivers/mfd/sec-irq.c +0 -0
index f9a5786..b65a7f0 100644
@@ -478,5 +478,6 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 

Quoted 6 lines

Quoted 6 lines
drivers/mfd/sec-irq.c int sec_irq_init(struct sec_pmic_dev *sec_pmic) · line 483
Quoted 6 lines
drivers/mfd/sec-irq.c +2 -1
 void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
 {
-	regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
+	if (sec_pmic->irq)
+		regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
 }

Seems I jumped too far with this one. Patch looks OK and works fine but is it really needed? If (!sec_pmic->irq) then sec_pmic->irq_data will be NULL and regmap_del_irq_chip() will handle it correctly.

Your change adds some sense of precautions (the sec_pmic->irq_data may be set by some other module by mistake) but still it does not look like "needed" for Origen.

Best regards,
Krzysztof

Bartlomiej Zolnierkiewicz

author

2014-08-08 12:44 UTC #4 lore raw

Quoted 46 lines · Krzysztof Kozlowski

Quoted 34 lines · Krzysztof Kozlowski
Quoted 18 lines · Bartlomiej Zolnierkiewicz
[ added missing linux-samsung-soc ML, sorry for the noise ]
Quoted 15 lines · Bartlomiej Zolnierkiewicz
Add missing check for the case of device without irq specified in sec_irq_exit() (please note that sec_irq_init() already correctly handles such devices).
This is needed for Insignal's Exynos4412 based Origen board.
Cc Krzysztof Kozlowski <[email protected]>
Cc Sangbeom Kim <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by Kyungmin Park <[email protected]>
---
patch is against next-20140804 branch of linux-next kernel
 drivers/mfd/sec-irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Looks and works good (tested on board with S2MPS14).
Reviewed-by Krzysztof Kozlowski <[email protected]>
Tested-by Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Quoted 7 lines
Quoted 7 lines
drivers/mfd/sec-irq.c +0 -0
index f9a5786..b65a7f0 100644
@@ -478,5 +478,6 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 
Quoted 6 lines
Quoted 6 lines
drivers/mfd/sec-irq.c int sec_irq_init(struct sec_pmic_dev *sec_pmic) · line 483
Quoted 6 lines
drivers/mfd/sec-irq.c +2 -1
 void sec_irq_exit(struct sec_pmic_dev *sec_pmic)
 {
-	regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
+	if (sec_pmic->irq)
+		regmap_del_irq_chip(sec_pmic->irq, sec_pmic->irq_data);
 }
Seems I jumped too far with this one. Patch looks OK and works fine but is it really needed? If (!sec_pmic->irq) then sec_pmic->irq_data will be NULL and regmap_del_irq_chip() will handle it correctly.

Quoted 3 lines

Your change adds some sense of precautions (the sec_pmic->irq_data may be set by some other module by mistake) but still it does not look like "needed" for Origen.

Indeed, I somehow assumed that regmap_del_irq_chip() doesn't check for NULL 'struct regmap_irq_chip_data *d' argument. Since such check is actually present in regmap_del_irq_chip() the patch is not needed and can be dropped. Thanks for noticing this.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics