Re: [PATCH 0/2 v3] Add pl031 RTC support for Hi6220

[Guodong Xu]

On 7 July 2016 at 08:58, John Stultz <john.stultz@xxxxxxxxxx> wrote:
> On Wed, Jul 6, 2016 at 12:38 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> On Wednesday, July 6, 2016 12:20:15 AM CEST John Stultz wrote:
>>> On Wed, Jul 6, 2016 at 12:04 AM, Olof Johansson <olof@xxxxxxxxx> wrote:
>>> > On Tue, Jul 5, 2016 at 11:55 PM, John Stultz <john.stultz@xxxxxxxxxx> wrote:
>>> >> On Tue, Jul 5, 2016 at 10:22 PM, Olof Johansson <olof@xxxxxxxxx> wrote:
>>> >>> On Wed, Jun 29, 2016 at 05:48:43PM -0700, John Stultz wrote:
>>> >>>> This patchset enables the pl031 RTC on the Hi6220 SoC.
>>> >>>>
>>> >>>> I'd like to submit it to be merged.
>>> >>>>
>>> >>>> Wei has acked the second patch (modulo a whitespace fix which
>>> >>>> I've included in this v3), so it seems like both could go
>>> >>>> through the clk tree.
>>> >>>>
>>> >>>> But Wei also seemed open to pulling in a clk tree branch
>>> >>>> as it goes through arm-soc.
>>> >>>>
>>> >>>> Michael/Stephen: If there's no other objections, could you
>>> >>>> queue the first patch and make it avilable via the branch for
>>> >>>> Wei, or just take both patches?
>>> >>>
>>> >>> I happen to dread these kind of patchsets these days. There's added
>>> >>> dependencies across trees just because a defined name for the clock
>>> >>> number is added to a header file.
>>> >>>
>>> >>> I much prefer to use numerical clocks for one release, and then once
>>> >>> everything is in, switch over to the defines in the DTS.
>>> >>>
>>> >>> That way there are no dependencies, no need to setup a shared branch
>>> >>> for a simple 3-line patch, etc.
>>> >>>
>>> >>> So, mind respinning the DTS piece?
>>> >>
>>> >> Huh..
>>> >
>>> > Sorry if it appeared random, I've complained about it for a while to
>>> > submaintainers.
>>>
>>> No.. I get it, the cross-maintainer shared branch is complex enough to
>>> want to avoid. I figured it would be easier to just take a maintainer
>>> acked patch in via the clk tree, but its not my tree, so I'll leave it
>>> to you maintainers to resolve.
>>
>> The question this raises is why that clock was missed the first time
>> around. I'd suggest whoever owns the clock driver can go through the
>> documentation again and look for others that may have been missed,
>> then send a patch to the driver to add *all* the missing ones for the
>> merge window, and one release later we add the driver depending on
>> previously unknown clocks.
>
> Though this seemingly goes against the otherwise widely recommended
> approach of breaking up patches into small obvious chunks.
>
> And personally, and I don't mean to criticize, but the suggestions
> here (use numerical values, then later rename to macros; add
> everything in one go, then make dts changes a release later) all seem
> like non-optimal workarounds for the fact that adding almost any
> functionality requires cross subsystem-maintainer negotiations (or two
> release steps to get one bit of functionality merged).
>
> It seems like it might even just be clearer to make the
> two-release-steps method the widely broadcast rule (ie: no
> dependencies on in-flight patches for dts changes), so this doesn't
> confuse/dismay new developers.
>
> Anyway... In this case, I don't have the clk documentation, so I'll
> ping Zhangfei to check if there is any other clock values that should

Arnd, John

After checking documentation, I didn't see other clock values that
need to be added.

-Guodong

> be added in the future, but at least for HiKey, while there are still
> a few clk patches remaining in the tree, I don't have any more
> additions to the clk list.
>
> thanks
> -john