LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: AceLan Kao <acelan.kao@canonical.com>
Cc: Jay Cliburn <jcliburn@gmail.com>,
	Chris Snook <chris.snook@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Rakesh Pandit <rakesh@tuxera.com>,
	netdev@vger.kernel.org, Emily Chien <emily.chien@canonical.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	Johannes Stezenbach <js@sig21.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Revert "alx: remove WoL support"
Date: Wed, 30 May 2018 15:58:59 +0200	[thread overview]
Message-ID: <20180530135859.GB27537@lunn.ch> (raw)
In-Reply-To: <20180530021008.15080-1-acelan.kao@canonical.com>

On Wed, May 30, 2018 at 10:10:08AM +0800, AceLan Kao wrote:
> This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.
> 
> The WoL feature is a must to pass Energy Star 6.1 and above,
> the power consumption will be measured during S3 with WoL is enabled.
> 
> Reverting "alx: remove WoL support", and will try to fix the unintentional
> wake up issue when WoL is enabled.

Hi AceLan

I find this change log entry rather odd.

If i remember correctly, you first argued that you did not want to
have to distribute out of tree patches.

It was suggested that you might be able to justify the revert using
the argument that the cure is worse than the decease. You ignored
that, and when with this Energy Star argument. That got shot down by
DaveM, and told to actually try to find the problem.

So you then come back and said you think the problem is fixed, but
don't know exactly what fixed it. So DaveM said try again.

Now you are back to Energy Star.

I don't get this. It was the fact you said it was probably fixed that
made DaveM reconsider. That is the argument you should be using in the
change log. We want to know what testing you have done. See a
tested-by: from somebody who had the issue which caused the revert,
and now says the issue is fixed.

Ideally we would like to know which change actually fixed the issue,
so it can be added to stable. But that requires somebody to do a long
git bisect.

    Andrew

  reply	other threads:[~2018-05-30 13:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30  2:10 AceLan Kao
2018-05-30 13:58 ` Andrew Lunn [this message]
2018-05-31  2:13   ` AceLan Kao
  -- strict thread matches above, loose matches on Subject: below --
2018-05-14  3:28 AceLan Kao
2018-05-14 13:35 ` David Miller
2018-05-21  3:14   ` AceLan Kao
2018-05-21  3:18     ` David Miller
2018-05-28  5:06       ` AceLan Kao
2018-05-29 14:57         ` David Miller

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=20180530135859.GB27537@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=acelan.kao@canonical.com \
    --cc=chris.snook@gmail.com \
    --cc=davem@davemloft.net \
    --cc=emily.chien@canonical.com \
    --cc=jcliburn@gmail.com \
    --cc=johannes@sipsolutions.net \
    --cc=js@sig21.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rakesh@tuxera.com \
    --subject='Re: [PATCH v2] Revert "alx: remove WoL support"' \
    /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

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).