From: Jakub Kicinski <kuba@kernel.org>
To: "Lifshits, Vitaly" <vitaly.lifshits@intel.com>
Cc: netdev@vger.kernel.org, Hui Wang <hui.wang@canonical.com>,
anthony.l.nguyen@intel.com, intel-wired-lan@lists.osuosl.org,
dima.ruinskiy@intel.com
Subject: Re: [Intel-wired-lan] [PATCH] e1000e: move force SMBUS near the end of enable_ulp function
Date: Tue, 16 Apr 2024 08:18:42 -0700 [thread overview]
Message-ID: <20240416081842.35995b10@kernel.org> (raw)
In-Reply-To: <f4bd1573-400b-4b45-940a-e1dc5e19df45@intel.com>
On Mon, 15 Apr 2024 13:15:41 +0300 Lifshits, Vitaly wrote:
> Thank you for this patch and this observation.
> I think that you found a real misbehaviour in the original patch.
> However, I still think that forcing SMBUS functionality shouldn't be
> part of the ULP enabling flow, since they are two independent
> configurations.
>
> I will soon submit a patch where I wrap forcing SMBUS in e1000_shutdown
> with an if that checks if the FWSM_FW_VALID bit it set.
Why are you submitting a patch instead of asking the author to change
theirs? This is not how code reviews work.
WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: "Lifshits, Vitaly" <vitaly.lifshits@intel.com>
Cc: Hui Wang <hui.wang@canonical.com>,
<intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
<jesse.brandeburg@intel.com>, <anthony.l.nguyen@intel.com>,
<dima.ruinskiy@intel.com>
Subject: Re: [PATCH] e1000e: move force SMBUS near the end of enable_ulp function
Date: Tue, 16 Apr 2024 08:18:42 -0700 [thread overview]
Message-ID: <20240416081842.35995b10@kernel.org> (raw)
In-Reply-To: <f4bd1573-400b-4b45-940a-e1dc5e19df45@intel.com>
On Mon, 15 Apr 2024 13:15:41 +0300 Lifshits, Vitaly wrote:
> Thank you for this patch and this observation.
> I think that you found a real misbehaviour in the original patch.
> However, I still think that forcing SMBUS functionality shouldn't be
> part of the ULP enabling flow, since they are two independent
> configurations.
>
> I will soon submit a patch where I wrap forcing SMBUS in e1000_shutdown
> with an if that checks if the FWSM_FW_VALID bit it set.
Why are you submitting a patch instead of asking the author to change
theirs? This is not how code reviews work.
next prev parent reply other threads:[~2024-04-16 15:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-13 9:27 [Intel-wired-lan] [PATCH] e1000e: move force SMBUS near the end of enable_ulp function Hui Wang
2024-04-13 9:27 ` Hui Wang
2024-04-15 10:15 ` [Intel-wired-lan] " Lifshits, Vitaly
2024-04-15 10:15 ` Lifshits, Vitaly
2024-04-16 15:18 ` Jakub Kicinski [this message]
2024-04-16 15:18 ` Jakub Kicinski
2024-04-17 18:38 ` [Intel-wired-lan] " Lifshits, Vitaly
2024-04-17 18:38 ` Lifshits, Vitaly
2024-05-02 7:00 ` [Intel-wired-lan] " naamax.meir
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=20240416081842.35995b10@kernel.org \
--to=kuba@kernel.org \
--cc=anthony.l.nguyen@intel.com \
--cc=dima.ruinskiy@intel.com \
--cc=hui.wang@canonical.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=netdev@vger.kernel.org \
--cc=vitaly.lifshits@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.