U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3 06/13] rockchip: rk3399: Add Orangepi RK3399 support
Date: Fri, 26 Apr 2019 20:02:51 +0200	[thread overview]
Message-ID: <a834fa57c21ad6a7e2d9b468e9029a763ec0c447.camel@bootlin.com> (raw)
In-Reply-To: <CAMty3ZAts9NCmCNr5p0yqrX3Lnp_uSOf1RKxGRjVPy2Kr7rVSQ@mail.gmail.com>

Hi,

Le vendredi 26 avril 2019 à 23:12 +0530, Jagan Teki a écrit :
> On Fri, Apr 26, 2019 at 11:07 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > Le vendredi 26 avril 2019 à 22:51 +0530, Jagan Teki a écrit :
> > > On Fri, Apr 26, 2019 at 10:38 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > Le vendredi 26 avril 2019 à 22:23 +0530, Jagan Teki a écrit :
> > > > > > > > > First series, I get introduced rk3399-u-boot.dtsi, and only the new
> > > > > > > > > boards are using this file and next series rest of the boards are
> > > > > > > > > using which indeed a valid step. what is inconsistent here, I don't
> > > > > > > > > understand.
> > > > > > > > 
> > > > > > > > Yes, what you are describing is exactly the issue. It does not make
> > > > > > > > sense to introduce a new common u-boot dtsi for rk3399 and add new
> > > > > > > > devices that use this file *before* switching existing devices to the
> > > > > > > > new common u-boot dtsi for rk3399 in the same series.
> > > > > > > 
> > > > > > > How come? existing boards doesn't use rk3399-u-boot.dtsi at all? as
> > > > > > > patch says it is an initial one and it is bit hard to add all at once.
> > > > > > > ie what I did for i.MX6. having said that rk3399-u-boot.dtsi is
> > > > > > > unaffected to any of the existing dts files.
> > > > > > 
> > > > > > Again, it's not a technical issue. Your proposal works and has no
> > > > > > technical issue. The issue is in how the commits are grouped together.
> > > > > > Patch series need to make logical sense. One patch series should
> > > > > > accomplish one change, and each patch represents a step of that change.
> > > > > 
> > > > > It's about how you think. say if I wouldn't send the binman, I'm sure
> > > > > this kind of discussion may not happen. In first mail you said
> > > > > "something broken and how it can repair next" that indeed doesn't make
> > > > > any sense of without understanding the whole series of changes.
> > > > 
> > > > One part to that is that I had misunderstood a few things, but I really
> > > > should not have to look at each patch before I can have an idea of what
> > > > the series does. Your series is called "rockchip: Add new rk3399
> > > > boards" and with the v3.1 change you made, the title is no longer true.
> > > > You are doing that + introducing a new rk3399 u-boot dtsi without
> > > > switching existing boards to it before adding new ones.
> > > > 
> > > > From that moment, you needed to split your changes into two series.
> > > > Instead of that, you made another series with mostly unrelated changes
> > > > where you included an unrelated commit adding the pre-reloc entries to
> > > > the dtsi created in the previous series.
> > > > 
> > > > That's an issue for communication with the community.
> > > > 
> > > > > Any new changes would come up with initial version, that what we
> > > > > usually supported, ie what we did -u-boot.dtsi changes to i.MX6 and
> > > > > developers are using the same is adding one after another.
> > > > 
> > > > But you already have boards in the tree that needs the tree. You can
> > > > make individual commits for switching boards to the new dtsi, but you
> > > > need to do that in the same series as introducing the dtsi if you are
> > > > submitting both things at the same time. If you want to have them
> > > > merged separately, then you can send device dtsi patches individually
> > > > for that, but not as part of another unrelated series.
> > > > 
> > > > > > This is how upstream contributions work, and it's a powerful way to
> > > > > > allow both efficient code review and good code quality. We want to keep
> > > > > > things as simple, explicit and well-described as possible, so that
> > > > > > things are easy for reviewers and as many people as possible can
> > > > > > understand the issue and share their thoughts about it.
> > > > > 
> > > > > Yes, we adopt these principles and that what we are really working.
> > > > > 
> > > > > > It is all part of communication with others as part of a community.
> > > > > > It is definitely an implicit rule that is not written down somewhere
> > > > > > precisely, but that's the social contract between developers that work
> > > > > > on a common software project.
> > > > > > 
> > > > > > In that, contributing to upstream is different than baseline tech
> > > > > > company standards, because you have to take extra steps to describe
> > > > > > your work and explain it to others. You must make sure you give them
> > > > > > all the comfort they need to painlessly understand what you did.
> > > > > 
> > > > > I hope all my communication was relevant to the topics, I can even
> > > > > given the example how things were moved.
> > > > 
> > > > Clearly there was a major communication issue between us. You only
> > > > answered on technical topics and never about how your patch series is
> > > > organized and what logical changes you are making.
> > > > 
> > > > To be honest with you, I feel like we have a general communication
> > > > issue where you only seem to focus on technical aspects, when the
> > > > issues are about the meta-data of the changes such as commit messages,
> > > > code comments and how changes are organized together.
> > > 
> > > No one is better in all aspects, and things were improving no one cam
> > > make global statements like this though he can find something gaps on
> > > on previous one. You would better suggest on the commit where it
> > > confused can make me better understanding.
> > 
> > I am making such a global statement because I've had this issue with
> > you a few times before already, and from what I could see about sunxi-
> > related discussions, it seems that others have had similar issues
> > before too. Each time, it feels like you are not discussing the issue
> > and answering on mostly-unrelated technical topics, exactly like it's
> > happening now.
> > 
> > Now this is really nothing personal against you, I have no interest in
> > pointing out things that need to be worked on in the work you submit in
> > order to make you feel bad or inferior. But at this point, I feel like
> > you are not understanding the global issue so I am bringing the
> > discussion to more general conclusions, since we are talking about
> > communication issues more generally.
> > 
> > Everyone is trying to improve and it's perfectly fine to have flaws.
> > For that matter, the situation was already reversed a few times when
> > you reviewed some of my sunxi patches saying that the commit log was
> > unclear, and I gladly accepted the criticism to make a better next
> > revision. I also want to add that you make significant important
> > contributions to lots of projects I really care about. This thread is a
> > perfect example of that too, since I plan on using a RK3399 boards you
> > are introducing for my personal use. So I am more than happy to see you
> > do all that hard work and I truly value your contributions.
> > 
> > Really, I'm mostly trying to help here and collaborate towards
> > resolving the situation.
> 
> Thanks.
> 
> > So could you please send 3 distinct series that deal with each one of
> > the aspects from the list that follows?
> 
> Just now I sent v5, which would resolve what you are thinking of,
> please have a look.

It seems that we're not quite there yet and you still need to split the
series. Looking forward to that!

Cheers,

Paul

  reply	other threads:[~2019-04-26 18:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 17:34 [U-Boot] [PATCH v3 00/13] rockchip: Add new rk3399 boards Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 01/13] rockchip: dts: rk3399: Sync rk3399-opp from Linux Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 02/13] rockchip: dts: rk3399: Sync pwm2_pin_pull_down from Linux 5.1-rc2 Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 03/13] rockchip: dts: rk3399: Create initial rk3399-u-boot.dtsi Jagan Teki
2019-04-26 13:21   ` Jagan Teki
2019-04-26 13:33     ` Paul Kocialkowski
2019-04-25 17:34 ` [U-Boot] [PATCH v3 04/13] Kconfig: Add default SPL_FIT_GENERATOR for rockchip Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 05/13] arm: rockchip: rk3399: Move common configs in Kconfig Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 06/13] rockchip: rk3399: Add Orangepi RK3399 support Jagan Teki
2019-04-26 14:17   ` Paul Kocialkowski
2019-04-26 14:27     ` Jagan Teki
2019-04-26 14:34       ` Paul Kocialkowski
2019-04-26 14:45         ` Jagan Teki
2019-04-26 14:54           ` Paul Kocialkowski
2019-04-26 15:01             ` Jagan Teki
2019-04-26 15:12               ` Paul Kocialkowski
2019-04-26 15:18                 ` Jagan Teki
2019-04-26 15:38                   ` Paul Kocialkowski
2019-04-26 15:48                     ` Jagan Teki
2019-04-26 16:08                       ` Paul Kocialkowski
2019-04-26 16:09                         ` Philipp Tomsich
2019-04-26 16:16                         ` Jagan Teki
2019-04-26 16:40                           ` Paul Kocialkowski
2019-04-26 16:53                             ` Jagan Teki
2019-04-26 17:08                               ` Paul Kocialkowski
2019-04-26 17:21                                 ` Jagan Teki
2019-04-26 17:37                                   ` Paul Kocialkowski
2019-04-26 17:42                                     ` Jagan Teki
2019-04-26 18:02                                       ` Paul Kocialkowski [this message]
2019-04-25 17:34 ` [U-Boot] [PATCH v3 07/13] rockchip: dts: rk3399: Sync rk3399-nanopi4.dtsi from Linux Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 08/13] rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1 Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 09/13] rockchip: rk3399: Add Nanopi M4 board support Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 10/13] rockchip: rk3399: Add Nanopi NEO4 " Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 11/13] rockchip: rk3399: Add Rockpro64 " Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 12/13] rockchip: rk3399: Add Rock PI 4 support Jagan Teki
2019-04-25 17:34 ` [U-Boot] [PATCH v3 13/13] doc: rockchip: Add global doc for rk3399 build/flash Jagan Teki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a834fa57c21ad6a7e2d9b468e9029a763ec0c447.camel@bootlin.com \
    --to=paul.kocialkowski@bootlin.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox