[U-Boot] [PATCH 0/2] rockchip: Initial RK3368 and GeekBox support

[Simon Glass]

Hi Tom, Andreas,

On 7 August 2016 at 14:07, Tom Rini <trini at konsulko.com> wrote:
> On Sun, Aug 07, 2016 at 06:46:37PM +0200, Andreas Färber wrote:
>> Am 07.08.2016 um 15:31 schrieb Tom Rini:
>> > On Sat, Aug 06, 2016 at 06:05:29PM +0200, Andreas Färber wrote:
>> >> Hi Simon,
>> >>
>> >> Am 06.08.2016 um 06:30 schrieb Simon Glass:
>> >>> Hi Andreas,
>> >>>
>> >>> On 17 July 2016 at 19:06, Andreas Färber <afaerber at suse.de> wrote:
>> >>>> Hi,
>> >>>>
>> >>>> This series adds initial support for RK3368 SoC and GeekBox.
>> >>>> For more details see the commit message.
>> >>>>
>> >>>> Will need to be rebased onto Heiko's cleanups and Kever's RK3399 series.
>> >>>>
>> >>>> Regards,
>> >>>> Andreas
>> >>>>
>> >>>> Cc: Simon Glass <sjg at chromium.org>
>> >>>> Cc: Kever Yang <kever.yang at rock-chips.com>
>> >>>> Cc: Heiko Stübner <heiko at sntech.de>
>> >>>>
>> >>>> Andreas Färber (2):
>> >>>>   dts: Import rk3368-geekbox.dts
>> >>>>   ARM64: rockchip: Add initial support for RK3368 based GeekBox
>> >>>>
>> >>>
>> >>> Are you planning to respin these patches?
>> >>
>> >> Eventually...?
>> >>
>> >>> I'd like to get them applied soon.
>> >>
>> >> And I'd like to get my work recognized! However, despite our previous
>> >> IRC chat, I had to find out _while_ replying to the rk3399 mails that
>> >> you had once again not just applied all patches (twenty minutes after
>> >> ack'ing them on a Saturday) but already sent a pull on Tuesday my
>> >> nighttime that I was not CC'ed on and that Tom has merged the night
>> >> after. So it feels like I'm wasting my time here and consequently I
>> >> stopped my review and rebase.
>> >
>> > In the U-Boot community, we are not in the habit of cc'ing everyone with
>> > a change in a given pull request.  Is there a tool the kernel folks use
>> > here that makes this easy?
>>
>> Not that I'm aware of.
>>
>> But that is besides the point, as my very complaint is that I'm not
>> credited in the patches that got merged, so no tool could've extracted
>> my name for CC'ing.
>
> Alright.
>
>> It's about Simon having mismerged those patches and having overlooked
>> unresolved review comments of mine for those patches before and me
>> specifically having complained to him about not waiting for my
>> Reviewed-by before applying them. So him seeing that I did not reply to
>> his Saturday mails, I feel it would've been fair in this particular case
>> a) to ping me again after the weekend and b) to let me know that he is
>> no longer waiting for my review comments or that I really need to hurry
>> up with an objection until X. He did not say so in a reply that reached
>> my inbox, and I was not CC'ed on the pull request itself, thus a pull
>> request behind my back.
>>
>> I'm not too deep into U-Boot, so maybe there was a reason for this
>> hush-hush workflow, but then at least the communication was fairly bad.
>>
>> Had I known that the pull is already on the list, I wouldn't have
>> replied with a Reviewed-by for 1/4 that same day (which surely Simon was
>> CC'ed on) or I could've asked Tom to hold off merging it until I'm done
>> reviewing the next day.
>>
>> > And the rule of thumb that I use, and I try and get everyone else to use
>> > as well is that a patch should be out for a week before it gets picked
>> > up and merged as that should give everyone time to review, comment and
>> > test.  Did that not happen with the patches Simon picked up?
>>
>> Slightly less than a week. For some other projects it's ~two weeks.
>> Also again note that this is not about some random patch but one where
>> Simon specifically said he would be away, that he would exchange the
>> patches on his branch where necessary and where he asked me to "sing".
>> It leaves a bad taste that Simon was absent himself the week the patches
>> were posted but apparently expects me to be available whenever he is. I
>> don't work on U-Boot as a job, and for rebasing rk3368 patches - which
>> many of my review comments resulted from - I need access to the hardware
>> for testing.
>>
>> Note that I was similarly surprised how quickly two patches of mine went
>> into his tree, with just one day in between and despite conflicts
>> between my rk3368 and Kever's rk3399 preparations. I can see that having
>> patches in a tree facilitates testing, but it also prevents serious peer
>> review when not just staging but also merging them.
>
> I want to treat all of the above at once.  First, sorry.  We don't have
> an intentionally "hush-hush" workflow, but every custodian does decide
> how many emails to send when moving a patch forward.  And unless I'm
> testing multiple PRs (or they come in while I'm already testing one) the
> time between getting a PR and applying it is usually the same (US, east
> coast) day if it passes my testing.  But we are trying to include more
> crediting, not less, so it is not intentional to have left things
> out[1].  So this was a mistake in our part, sorry.  Sometimes review
> comments are missed.  But this too is not usually intentional unless
> it's small things that can be addressed in a follow-up in order to get
> things otherwise in and unblocking other work.  In the end however,
> Simon, please slow down a bit.  It's OK if stuff misses a release,
> there's always the next one.  And it may make sense to follow what Hans
> has been doing with the sunxi tree and having a next branch (which can
> always be rebased and stuff replaced in!) so it's both clear that
> patches are being picked up, but not rushing to get things in before a
> window closes.  Thanks!

OK thanks Tom for looking at this. I'll slow down a bit on the
Rockchip stuff. I'll look at using a -next branch for that also.

Andreas let me know how things go. I'd really like to see a review for
new patches within a week if possible (absent holidays, etc.).

>
> --
> Tom
>
> [1]: Patchwork had an outage recently and some stuff didn't make it in
> there and thus was missed unless picked up manually.  Most tags come in
> via patchwork not manually adding to the commit message.

Yes that was a pain!

Regards,
SImon