[U-Boot] [PATCH v2 00/99] ram: rk3399: Add LPDDR4 support

[Kever Yang]

Jagan,


On 06/26/2019 06:22 PM, Jagan Teki wrote:
> On Wed, Jun 26, 2019 at 12:12 AM Ezequiel Garcia
> <ezequiel at vanguardiasur.com.ar> wrote:
>> Hi Jagan,
>>
>> Thanks for your hard work. I'm sure everyone in the Rockchip community
>> is excited about finally having this support in U-Boot.
>>
>> On Tue, 25 Jun 2019 at 12:46, Jagan Teki <jagan at amarulasolutions.com> wrote:
>> [..]
>>>> Was it absolutely necessary to split these changes into 99 commits? I
>>>> believe at least some of them can be squashed. Reviewing 99 patches
>>>> isn't feasible.
>>> Squashed, I'm not sure because the patches were created to satisfy the
>>> bisectability and travis-ci, if you find any please feel to comment.
>>> About the commit count, I have mentioned in v1, the idea of having
>>> many commits in one series to have all lpddr4(-related) changes in one
>>> place and also all the commit has incremental approach of supporting
>>> rank detection and lpddr4. If require I'm open to sent next versions
>>> as multiple series, no problem on that.
>>>
>> I strongly agree with Vasily, and I don't think multiple series makes it any
>> better.
>>
>> What's the reason for having two commits for:
>>
>> "ram: rk3399: Set lpddr4 MR3" and "ram: rk3399: Set lpddr4 MR12" ?
> These are individual lpddr4 set rate registers to support, each one is
> independent on it' own initialization and more over on the whole, it
> is critical to review.
>
>> Or splitting all the "ram: rk3399: Add ... macro" ?
> You mean the patches 13 to 20 same like above each one has it's own
> meaning. It is not meaningful to squash them all.

99 patches is really too much, but I'm not sure how smaller it can be.
Reference to kernel document, it suggest not more than 15 at one time:
    NO!!!! No more huge patch bombs to linux-kernel at vger.kernel.org
people! <https://lkml.org/lkml/2005/7/11/336> But please note that I
don't think split this into different series make any sense. But maybe
you can try to squash as much as you can. eg. the update for
dram_all_config may able to squash into one patch,
and some new MACRO and its reference code may be able to squash.
So it depends on how you define about _logical change_.
I'm not sure if this have happen in the history of U-Boot mailing list, but
I think this big patch set will be complained by many people if this is
send to
kernel.

I would need to review all the code one by one, and merge them all at
one time,
so it does not affect much about my review, but for U-Boot patch submit
requirement, I'm not sure if any suggestion from other maintainers.

Thanks,
- Kever
>> What do you loose if you merge the patches into one?
>>
>> I must confess I am very surprised, and don't really understand what do you
>> gain with this excessive split. Normally, when we are adding a new feature,
>> we normally don't need many patches, so it's the other way around, really.
>>
>> Bisectability is about not breaking existing support, but because the feature
>> is new, normally this is easy.
> Look, like the whole confusion seems to be because of more patches in
> one series and the cover-letter states that it support lpddr4. I
> understand it now, will send the relevant changes in next version
> accordingly, if require I will squash if any.
>
> Jagan.
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>