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.cdiff --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