linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: shawnguo@kernel.org (Shawn Guo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
Date: Thu, 4 May 2017 20:44:48 +0800	[thread overview]
Message-ID: <20170504124446.GO18578@dragon> (raw)
In-Reply-To: <6dbf67a0-42ea-ee84-d144-41422a21c1c5@denx.de>

On Thu, May 04, 2017 at 12:06:11PM +0200, Marek Vasut wrote:
> On 05/04/2017 11:42 AM, Leonard Crestez wrote:
> > I think there is a further misunderstanding here. I have a problem
> > where imx6sx-sdb rev C boards crash on boot with upstream (but are
> > reported to work fine with rev B). Removing the OPP overrides fixes
> > this specific issue.
> > 
> > I don't object to the second part of your patch, setting correct supply links is a good thing for various reasons. It is just not necessary for fixing the concrete crash mentioned above (and I tested this). It should probably go in a separate patch.
> 
> Mind you, my patch is not fixing any crash, it's correcting the
> regulator binding and removing the OPP override which is at that
> point useless.

Heh, that's the primary reason why I prefer Leonard's patch, as his
patch is fixing a critical crash issue, while yours is just removing
some useless stuff and correcting something that doesn't show any real
problem.

So Leonard's patch will need to be applied for v4.12-rc and copy stable
kernel, while yours will only be applied for next merge window as a
cleanup/improving thing.

Patches are similar, but they can be handled very differently, because
commit log tells completely different stories.  Do you see how commit
log matters now?

> > It might seem a pedantic difference but it's good to accurately describe the effect of patches in commit messages. For example it might help somebody looking to backport various fixes.

@Leonard, no, it's not pedantic at all.  I really appreciate your commit
message and all the comments you added in the discussion, which is
extremely helpful for us to understand the changes.

> Which part of the following commit message is unclear?
> 
> "
> The imx6sx-sdb has one power supply that drives both VDDARM_IN
> and VDDSOC_IN, which is the sw1a regulator on the PFUZE PMIC.
> Connect both inputs to the sw1a regulator on the PMIC and drop
> the OPP hackery which is no longer needed as the power framework
> will take care of the regulator configuration as needed.
> "

Something unclear in my opinion:

 - The OPP hackery was never needed, as it's only needed for LDO bypass
   mode which hasn't been supported by mainline kernel.  It's not 'no
   longer needed as the power framework ...'.

 - What are the related changes in power framework?  It will be more
   clear if we can have the particular commit mentioned here.

> btw if sending obvious bugfix upstream is met with this sort of

Leonard's patch is an obvious bugfix, but yours is not.

> resistance and pointless discussion, it is extremely demotivating.

As I said to Leonard above, the discussion is extremely helpful, IMO.

> Waiting for a maintainer reply for 2-4 weeks, only to get a kurt

This is not a paid job for maintainer.  It's expected that he doesn't
always reply in a timely manner, especially when the tree is kinda
'frozen' for merge window.

> reply like "I don't like the commit message" doesn't help either.

What really annoys me is your attitude to commit message.

Shawn

  reply	other threads:[~2017-05-04 12:44 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-25 16:57 [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override Leonard Crestez
2017-04-25 17:02 ` Fabio Estevam
2017-04-25 17:02   ` Fabio Estevam
2017-04-25 17:23     ` Leonard Crestez
2017-04-25 17:26       ` Fabio Estevam
2017-04-25 17:28       ` Marek Vasut
2017-05-03 13:57         ` Shawn Guo
2017-05-03 14:26           ` Marek Vasut
2017-05-03 14:32             ` Marek Vasut
2017-05-03 14:41               ` Shawn Guo
2017-05-03 14:51                 ` Marek Vasut
2017-05-03 14:58             ` Leonard Crestez
2017-05-03 15:59               ` Marek Vasut
2017-05-03 17:58                 ` Leonard Crestez
2017-05-03 19:33                   ` Marek Vasut
2017-05-04  9:42                     ` Leonard Crestez
2017-05-04 10:06                       ` Marek Vasut
2017-05-04 12:44                         ` Shawn Guo [this message]
2017-05-04 13:08                           ` Marek Vasut
2017-05-04 13:41                             ` Shawn Guo
2017-05-04 14:34                               ` Marek Vasut
2017-05-05  1:18                                 ` Shawn Guo
2017-05-05 10:11                                   ` Leonard Crestez
2017-04-27  1:17 ` Peter Chen
2017-05-04 11:43   ` Shawn Guo
2017-05-04 11:46     ` Fabio Estevam
2017-05-04 12:50 ` Shawn Guo

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=20170504124446.GO18578@dragon \
    --to=shawnguo@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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;
as well as URLs for NNTP newsgroup(s).