On Wed, Jun 26, 2019 at 12:12 AM Ezequiel Garcia ezequiel@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@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.
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.