linux-kernel - Re: [PATCH V2 5/6] rtc: max77xxx: add RTC driver for Maxim MAX77xxx series RTC IP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 18 Jan 2016 13:27:11 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Javier Martinez Canillas <javier@...hile0.org>,
	Laxman Dewangan <ldewangan@...dia.com>
Cc:	Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Lee Jones <lee.jones@...aro.org>,
	Mark Brown <broonie@...nel.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux GPIO List <linux-gpio@...r.kernel.org>,
	rtc-linux@...glegroups.com, Stephen Warren <swarren@...dia.com>,
	Thierry Reding <treding@...dia.com>
Subject: Re: [PATCH V2 5/6] rtc: max77xxx: add RTC driver for Maxim MAX77xxx
 series RTC IP

On 15.01.2016 10:50, Javier Martinez Canillas wrote:
> Hello,
> 
> On Thu, Jan 14, 2016 at 8:50 AM, Laxman Dewangan <ldewangan@...dia.com> 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