From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754088AbcAME2c (ORCPT ); Tue, 12 Jan 2016 23:28:32 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:20897 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752541AbcAME2a (ORCPT ); Tue, 12 Jan 2016 23:28:30 -0500 X-AuditID: cbfec7f5-f79b16d000005389-d2-5695d26ad811 Subject: Re: [PATCH V2 5/6] rtc: max77xxx: add RTC driver for Maxim MAX77xxx series RTC IP To: Laxman Dewangan , robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, linus.walleij@linaro.org, gnurou@gmail.com, lee.jones@linaro.org, broonie@kernel.org, a.zummo@towertech.it, alexandre.belloni@free-electrons.com References: <1452590273-16421-1-git-send-email-ldewangan@nvidia.com> <1452590273-16421-6-git-send-email-ldewangan@nvidia.com> <569594F0.4020007@samsung.com> <5695CD73.5010709@nvidia.com> Cc: lgirdwood@gmail.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, rtc-linux@googlegroups.com, swarren@nvidia.com, treding@nvidia.com From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <5695D268.3060705@samsung.com> Date: Wed, 13 Jan 2016 13:28:24 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-version: 1.0 In-reply-to: <5695CD73.5010709@nvidia.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrFIsWRmVeSWpSXmKPExsVy+t/xy7pZl6aGGTTM17FYcvEqu0XHtcVM FlMfPmGzmH/kHKtF/5uFrBbnXj1iARIrGS1evzC0WLpvNYvF/a9HGS2+XelgspjyZzmTxeb5 fxgtLu+aw2ax9PpFJosJ09eyWLTuPcJusb+zg9HixvQWVovbv/kcRDzWzFvD6HG5r5fJ48mm i4weO2fdZffYM/Ekm8fK5V/YPDat6mTzuHNtD5tHb/M7No++LasYPabP+8nk8XmTXABPFJdN SmpOZllqkb5dAlfGpnWTmAoWKFYsbDzL1sA4Q6qLkZNDQsBEYtmNd0wQtpjEhXvr2boYuTiE BJYySjxuew/lPGWUeLTuBjNIlbBAjMSL1XNZQBIiAtuZJDbdeMYIUbWPUeLF5umsIA6zwFZG ie9bP7KAtLAJGEtsXr6EDWKJnERv9ySwOK+AlsTRL8cZQWwWAVWJJV17gWo4OEQFIiQW7ciE KBGU+DH5Hlg5J1D56knzmEFKmAX0JO5f1AIJMwvIS2xe85Z5AqPgLCQdsxCqZiGpWsDIvIpR NLU0uaA4KT3XSK84Mbe4NC9dLzk/dxMjJIa/7mBceszqEKMAB6MSD6/BjKlhQqyJZcWVuYcY JTiYlUR4C08BhXhTEiurUovy44tKc1KLDzFKc7AoifPO3PU+REggPbEkNTs1tSC1CCbLxMEp 1cAYuNHWLaI3WIZ3El/8GpYPhha+t1RMPzTWTmpR6v9iu/jllbwKna/aU4wkjrze2rykLv+G vIiHmW1pffXdaTPaJe+YKu46ocGSsOTL7IwHJrsv88w8+Mzguklc43IeleSa2GXmqQcWu69/ sldY/Fe+j3Xv/xAdnbuqV5cszv7/e34KW/imnZZKLMUZiYZazEXFiQByWcAC3QIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13.01.2016 13:07, Laxman Dewangan wrote: > > On Wednesday 13 January 2016 05:36 AM, Krzysztof Kozlowski wrote: >> On 12.01.2016 18:17, Laxman Dewangan wrote: >>> Maxim Semiconductor's PMIC MAX77620, MAX77686, MAX20024 have >>> same RTC IP on these PMICs. >>> >>> Add generic MAX77xxxx series RTC driver which can be used as >>> RTC driver for these PMIC and avoids duplication of RTC driver >>> for each PMICs. Their MFD driver can be different here. >>> >>> Signed-off-by: Laxman Dewangan >>> --- >>> Changes from V1: >>> - Rename the file to rtc-max77xxx.c and make the generic implementation. >>> - Direct regmap apis are used for the register access. >>> - Decouped from max77620 driver. >>> - Taken care of cleanup comments form V1 version. >>> >>> drivers/rtc/Kconfig | 10 + >>> drivers/rtc/Makefile | 1 + >>> drivers/rtc/rtc-max77xxx.c | 500 >>> +++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 511 insertions(+) >>> create mode 100644 drivers/rtc/rtc-max77xxx.c >>> >>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig >>> index 376322f..4972dd5 100644 >>> --- a/drivers/rtc/Kconfig >>> +++ b/drivers/rtc/Kconfig >>> @@ -315,6 +315,16 @@ config RTC_DRV_MAX8997 >>> This driver can also be built as a module. If so, the module >>> will be called rtc-max8997. >>> +config RTC_DRV_MAX77XXX >>> + tristate "Maxim MAX77XXX series generic RTC driver" >>> + help >>> + If you say yes here you will get support for the generic RTC >>> driver >>> + for Maxim Semiconductor MAX77XXX series of PMIC like MAX77620. >>> + This also supports the RTC driver for Maxim PMIC MaX20024 which >>> + is almost same as MAX77620. >>> + This driver can also be built as a module. If so, the module >>> + will be called rtc-max77xxx. >>> + >> That was not the consensus... You still added a new driver - but now >> with different name. >> >> That is useless duplication >> >> Please work with existing code. Use existing maxim RTC drivers: either >> max77686 or max77802. >> >> There is no need for new one. > > If we modify the existing one then that work will be outside of this > series to make it independent. And that is the problem? The series evolve. The ultimate goal is to support max77686, max77802, max77620 and max20024. > > However, the file name does not suggest common in older file. This is not a sensible argument. The name does not matter. But if really needed we can rename it... > Also this > will require mfd and rtc driver changes to decouple it. Yes, decouple everything! I like it! :) Make it robust, generic, readable, fix bugs etc. :) > > Here is my approach: > - Let's have common driver in implementation and file name. This will be > independent of the mfd driver on all sense. > - Once it is merged, move the max77686 and max77802 to use this driver, > this will need the modification on the mfd driver, related defconfig > file and if specific stuff needed in the rtc then addition of that. Nope, because *the second part won't happen*. Never. After merging you will be happy and another duplicated stuff ends in the kernel. Fix things before merging. Not after. > > Per your approach: > - Modify rtc max77686 and mfd driver max77686 to decouple and proper > registration. > - Add support of max77620 on the max77686 driver if any specific is > required. That is the way we usually extend the drivers for new devices. > > That is also fine to me but still I am not comfortable with the config > name and driver file name as this does not suggest the common. The name does not matter. Really. We have a lot of drivers with a specific device-like name and supporting different devices. To point that your argument is invalid - your initial name of driver "rtc-max77620.c" supported totally different "names": the max77620 and max20024. It also wasn't suggesting something "common"... With my approach we are not developing common think neither. We just want to extend/re-use existing max77686 (or max77802) driver for new devices. Just like everywhere else. Best regards, Krzysztof