Re: [PATCH V7 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

From: Lee Jones
Date: Wed Feb 10 2016 - 11:08:38 EST


On Tue, 09 Feb 2016, Laxman Dewangan wrote:

> To make RTC block of MAX77686/MAX77802 as independent driver,
> move the registration of i2c device, regmap for register access
> and irq_chip for interrupt support inside the RTC driver.
> Removed the same initialisation from MFD driver.
>
> Having this change will allow to reuse this driver for different
> PMIC/devices from Maxim Semiconductor if they kept same RTC IP on
> different PMIC. Some of examples as PMIC MAX77620, MAX20024 where
> same RTC IP used and hence driver for these chips will use this
> driver only for RTC support.
>
> Suggested-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
> Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx>
> CC: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
> CC: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> Tested-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
>
> ---
> Changes from V1:
> - Remove changes from Kconfig.
> - Maintain all register definition in max77686 private header and remove
> the movement to rtc driver.
> - Taken care of all comments on V1 from Krzysztof and Javier.
>
> Changes from V2:
> - Taken care of missed sequence for removing the resource.
> - Fix the crash when doing unbind by using requested_threaded_irq()
> instead of demv_requested_threaded_irq().
>
> Changes from V3:
> - Fix the issue of suspend-resume with unbind/bind by unmapping the
> virq.
>
> Changes from V4:
> - Dispose mapped irq in error path in probe
>
> Changes from V5:
> - Remove the new APIs from regmap as functionality added to dispose the
> virq inside the regmap_del_irq_chip().
>
> Changes from V6:
> Make this as 5th of series as regmap change applied independently.
> Also rebase on linux-next 20160209
>
> drivers/mfd/max77686.c | 85 +-------------------
> drivers/rtc/rtc-max77686.c | 148 ++++++++++++++++++++++++++++++-----
> include/linux/mfd/max77686-private.h | 3 -
> 3 files changed, 130 insertions(+), 106 deletions(-)

Yes, yes, yes!

This is how it should have been done in the first place IMHO.

Acked-by: Lee Jones <lee.jones@xxxxxxxxxx>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog