From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753643AbcARE1i (ORCPT ); Sun, 17 Jan 2016 23:27:38 -0500 Received: from mailout3.w1.samsung.com ([210.118.77.13]:10393 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753198AbcARE10 (ORCPT ); Sun, 17 Jan 2016 23:27:26 -0500 X-AuditID: cbfec7f4-f79026d00000418a-9f-569c69abfaf2 Subject: Re: [PATCH V2 5/6] rtc: max77xxx: add RTC driver for Maxim MAX77xxx series RTC IP To: Javier Martinez Canillas , Laxman Dewangan 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> <5695D268.3060705@samsung.com> <5695D1AF.9000806@nvidia.com> <5696665A.5030508@nvidia.com> <5696F0C4.1060904@samsung.com> <5696F7F8.108@samsung.com> <56978BA1.9050409@nvidia.com> Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Linus Walleij , Alexandre Courbot , Lee Jones , Mark Brown , Alessandro Zummo , Alexandre Belloni , Liam Girdwood , "devicetree@vger.kernel.org" , Linux Kernel , Linux GPIO List , rtc-linux@googlegroups.com, Stephen Warren , Thierry Reding From: Krzysztof Kozlowski Message-id: <569C699F.10407@samsung.com> Date: Mon, 18 Jan 2016 13:27:11 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrBIsWRmVeSWpSXmKPExsVy+t/xK7qrM+eEGUz/I2Wx5OJVdouOa4uZ LKY+fMJmMf/IOVaL/jcLWS3OvXrEAiRWMlpc+z2DzeL1C0OLpftWs1jc/3qU0eLblQ4miyl/ ljNZbJ7/h9Hi8q45bBZLr19kspgwfS2LReveI+wW+zs7GC1uTG9htbj9m89B1GPNvDWMHpf7 epk8/s5uZfZ4sukio8fOWXfZPfZMPMnmsXL5FzaPTas62TzuXNvD5tHb/I7No2/LKkaP6fN+ Mnl83iQXwBvFZZOSmpNZllqkb5fAlTFn03uWgtdSFf8Xr2RuYFwm2sXIySEhYCKxfcs+Nghb TOLCvfVANheHkMBSRonuXe1QzlNGidN7vjODVAkLxEi8WD2XBcQWAbL/TpjKAlG0j1ni7K3r rCAOs8ALVokVjcuYQKrYBIwlNi9fAraDV0BD4n/nQrA4i4CqxKr/ixhBbFGBCInDnV3sEDWC Ej8m3wPbwCkQLHHiZidQLwfQUHWJKVNyQcLMAvISm9e8ZZ7AKDALSccshKpZSKoWMDKvYhRN LU0uKE5KzzXUK07MLS7NS9dLzs/dxAiJ3y87GBcfszrEKMDBqMTDO+P87DAh1sSy4srcQ4wS HMxKIrzSKXPChHhTEiurUovy44tKc1KLDzFKc7AoifPO3fU+REggPbEkNTs1tSC1CCbLxMEp 1cBYn2bqU2bG925pbdLkUi3O9cbvmnomOWm7b41Jt5TfMNnWKV46+eqGc2zbz6/flOb5Yd2U nLmP1Ys2B56sO22m8pnrBMfDfzvr9xettSv2TQ5u6YixEf0gzP/CnkP14y7p3SWbT6UzquZN fx/KPu2HQZBRRP7bzTGWyzuX1UX/la/d9PDLzx4lluKMREMt5qLiRADsyXOT2wIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15.01.2016 10:50, Javier Martinez Canillas wrote: > Hello, > > On Thu, Jan 14, 2016 at 8:50 AM, Laxman Dewangan wrote: >> On Thursday 14 January 2016 06:50 AM, Krzysztof Kozlowski wrote: >>> On 14.01.2016 09:50, Krzysztof Kozlowski wrote: > > [snip] > >>>> >>>> The max77802 does exactly the same (BTW, these should be merged as >>>> well... I'll add this to the TODO list) so I think this is necessary. >>> >>> How about merging max77802 to max77686 first? The only differences I >>> found are: >>> 1. It uses main MFD/PMIC regmap. >>> This can be solved as part of decoupling code. The driver will get >>> MFD's regmap and set up its own (only on max77686). The max77802 will >>> only use parent's regmap. >>> >>> 2. It has different register address. >>> We need a register-layout/configuration structure. The logic is the >>> same except few differences (e.g. presence of MAX77802_RTC_AE1). >>> >>> It may be easier to merge them now, before adding support for max77620? > > Agreed. > > When I originally posted the max77802 support, I had separate MFD, > RTC, regulator and clock drivers and the feedback (IIRC) was that the > MFD and clock blocks were too similar to the max77686 so I extended > those drivers instead of adding new ones. But that the RTC and > regulator blocks were different so a separate driver was justified... > but it's true that are not that different and rtc-max77686 could be > extended and rtc-max77802 removed. Great! > > In fact, the ChromiumOS vendor tree has a single RTC driver for both > max77686 and max77802: > > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/rtc/rtc-max77xxx.c > >>> I could handle this probably next week or in the following week >>> (assuming someone would test max77802 because I don't have the hardware). >>> > > I could also work on this next week if you want. After all I feel > guilty for the code duplication :-) So feel free to take that job from me. :) I will happily do the testing and provide complains (I mean, comments). > >>> Anyway I think we should develop these RTC patches having this in mind: >>> merge all of them. >>> >> >> I think we can do the merging of max77802 and max77686 at second step. >> >> At first, lets decouple the rtc-max77686.c with its mfd driver. This will >> give better picture how will it be done. > > I don't quite understand what you mean by decoupling here, the > max77686 MFD driver today is not highly coupled to their cells devices > drivers since it supports both max77802 and max77686 already. Yes, > some changes will be needed but I think those should be small. The decoupling needed is to move RTC-related stuff (i2c_new_dummy and regmap) entirely to RTC driver. This work is independent of which driver will be merged to rtc-max77xxx first. > >> Once this is there then max77620 can use this and in parallel, can be work >> for max77802 to use the same driver. >> >> >> Let's first conclude and get accpepted for max77686 and max77620 as both of >> us have related HW to verify the changes. >> > > I agree with Krzysztof that merging first before extending makes more > sense but I don't have a strong opinion on this and can be done as a > followup as well. AFAIR, Javier have max77802 on Chromebook, right? Best regards, Krzysztof