Thread (11 messages) 11 messages, 4 authors, 2014-03-13

Re: [PATCH v2 2/3] mfd: max8997: handle IRQs using regmap

From: Lee Jones <hidden>
Date: 2014-03-13 08:00:10
Also in: linux-leds, lkml

Guys, when replying please cut out all of the unnecessary text. Paging
down though unmodified/unreviewed code is tiresome.
quoted
quoted
This patch modifies mfd driver to use regmap for handling interrupts.
It allows to simplify irq handling process. This modifications needed
to make small changes in function drivers, which use interrupts.

Signed-off-by: Robert Baldyga <redacted>
---
 drivers/extcon/extcon-max8997.c     |   35 ++--
 drivers/mfd/Kconfig                 |    2 +-
 drivers/mfd/Makefile                |    2 +-
 drivers/mfd/max8997-irq.c           |  373 -----------------------------------
 drivers/mfd/max8997.c               |  113 ++++++++++-
 drivers/rtc/rtc-max8997.c           |    2 +-
 include/linux/mfd/max8997-private.h |   65 +++++-
 7 files changed, 183 insertions(+), 409 deletions(-)
 delete mode 100644 drivers/mfd/max8997-irq.c
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index f258c08..15fc5c0 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -46,15 +46,15 @@ struct max8997_muic_irq {
 };
 
 static struct max8997_muic_irq muic_irqs[] = {
-	{ MAX8997_MUICIRQ_ADCError,	"muic-ADCERROR" },
-	{ MAX8997_MUICIRQ_ADCLow,	"muic-ADCLOW" },
-	{ MAX8997_MUICIRQ_ADC,		"muic-ADC" },
-	{ MAX8997_MUICIRQ_VBVolt,	"muic-VBVOLT" },
-	{ MAX8997_MUICIRQ_DBChg,	"muic-DBCHG" },
-	{ MAX8997_MUICIRQ_DCDTmr,	"muic-DCDTMR" },
-	{ MAX8997_MUICIRQ_ChgDetRun,	"muic-CHGDETRUN" },
-	{ MAX8997_MUICIRQ_ChgTyp,	"muic-CHGTYP" },
-	{ MAX8997_MUICIRQ_OVP,		"muic-OVP" },
+	{ MAX8997_MUICIRQ_ADCERROR,	"MUIC-ADCERROR" },
+	{ MAX8997_MUICIRQ_ADCLOW,	"MUIC-ADCLOW" },
+	{ MAX8997_MUICIRQ_ADC,		"MUIC-ADC" },
+	{ MAX8997_MUICIRQ_VBVOLT,	"MUIC-VBVOLT" },
+	{ MAX8997_MUICIRQ_DBCHG,	"MUIC-DBCHG" },
+	{ MAX8997_MUICIRQ_DCDTMR,	"MUIC-DCDTMR" },
+	{ MAX8997_MUICIRQ_CHGDETRUN,	"MUIC-CHGDETRUN" },
+	{ MAX8997_MUICIRQ_CHGTYP,	"MUIC-CHGTYP" },
+	{ MAX8997_MUICIRQ_OVP,		"MUIC-OVP" },
 };

Why did you modify interrput name? Did you have some reason?
I think this modification don't need it.
I did it to have one naming convention in max8997-private.h file. Any
other interrupt names uses upper case, but MUIC iqr's from some reason
uses CamelCase. I think it's much better to have consistent style in
entire file.
I agree with the modification. CamelCase #defines/enums are hard to
read and look awful.

<snip>
quoted
quoted
+enum max8997_irq_muic {
 	MAX8997_MUICIRQ_ADC,
+	MAX8997_MUICIRQ_ADCLOW,
+	MAX8997_MUICIRQ_ADCERROR,
 
-	MAX8997_MUICIRQ_VBVolt,
-	MAX8997_MUICIRQ_DBChg,
-	MAX8997_MUICIRQ_DCDTmr,
-	MAX8997_MUICIRQ_ChgDetRun,
-	MAX8997_MUICIRQ_ChgTyp,
+	MAX8997_MUICIRQ_CHGTYP,
+	MAX8997_MUICIRQ_CHGDETRUN,
+	MAX8997_MUICIRQ_DCDTMR,
+	MAX8997_MUICIRQ_DBCHG,
+	MAX8997_MUICIRQ_VBVOLT,
ditto.
I don't understand why do you modify interrnut name/macro.
... because the old ones are ugly and hard to read.
quoted
quoted
 	MAX8997_MUICIRQ_OVP,
 
-	MAX8997_IRQ_NR,
+	MAX8997_MUCIRQ_NR,
ditto.
Here I have splitted enum into few enums, each for single interrupt
source, to make it easier to use with regmap irq handling. So I changed
MAX8997_IRQ_NR name to MAX8997_MUCIRQ_NR because now it's not number of
all interrupts but number of interrupts coming form MUIC.
The new name is more inline with the remaining enums, thus is better.

<snip>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help