All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chia-Lin Kao \(AceLan\) via Intel-wired-lan" <intel-wired-lan@osuosl.org>
To: "Ruinskiy, Dima" <dima.ruinskiy@intel.com>
Cc: "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>,
	 "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
	 Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>,
	 "intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH 1/2] igc: Wait for MAC passthrough after reset
Date: Wed, 15 Jul 2026 13:46:09 +0800	[thread overview]
Message-ID: <alcd_FnUdrQug5cm@acelan-Precision-5480> (raw)
In-Reply-To: <1d6c3aae-c9b1-42da-b5fa-6f8ee9ebcae1@intel.com>

On Mon, Jun 29, 2026 at 07:42:43PM +0300, Ruinskiy, Dima wrote:
> Hi AceLan,
>
> Some more comments below.
>
> On 22/06/2026 4:57, Chia-Lin Kao (AceLan) wrote:
>
> > >
> > > Because 100 iterations of 100msec each - this translates to up-to 10
> > > seconds, no?
> > Yes, just in case it takes longer.
> > I think 5 seconds should be enough if you feel this is feasible.
> 5 seconds also sounds excessive, assuming the majority of the systems do not
> support / enable MAC passthrough via the FW feature (and I have a hunch it
> is the case). 2 seconds... maybe, but then you say that in some rare cases
> it exceeds 2 seconds as well. What if in some even rare cases it exceeds
> 5/10 seconds? Perhaps because of some glitch it will not come up at all on
> this cycle. Should the driver always wait?
>
> > I wish we can detect if the MAC passthrough is enabled, so that we
> > know if we need to poll for the MAC address.
> Unfortunately, like you I am not aware of any way for the driver to know
> whether MAC passthrough via FW is enabled. Because of this we have been
> exploring a simpler way to support this feature via ACPI objects (which are
> set by the BIOS when MAC passthrough is enabled and are easy for the driver
> to query). I know some vendors have already implemented it, and I am
> currently drafting a patch to send.
Any good news about this patch?

>
> > For the FW interrupt mechanism also needs BIOS support, and we don't
> > have the power to push this.
> The mechanism I have in mind does not require BIOS support - the I225/I226
> FW already supports the required interrupt, AFAIK - it merely needs to be
> enabled in the igc driver. With that said, there still remains the question
> of how to notify the network stack above us that the MAC address has changed
> post-probe - and whether it is even supported.
>
> --Dima
>
> >
> > >
> > > Thanks,
> > > Dima.
> > >
> > > >
> > > > > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > > > > ---
> > > > >    drivers/net/ethernet/intel/igc/igc_main.c | 48
> > > > > +++++++++++++++++++++++
> > > > >    1 file changed, 48 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > b/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > index 2c9e2dfd8499..fa9752ed8bc5 100644
> > > > > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > @@ -11,6 +11,7 @@
> > > > >    #include <net/pkt_sched.h>
> > > > >    #include <linux/bpf_trace.h>
> > > > >    #include <net/xdp_sock_drv.h>
> > > > > +#include <linux/etherdevice.h>
> > > > >    #include <linux/pci.h>
> > > > >    #include <linux/mdio.h>
> > > > >
> > > > > @@ -69,6 +70,52 @@ static const struct pci_device_id igc_pci_tbl[] = {
> > > > >
> > > > >    MODULE_DEVICE_TABLE(pci, igc_pci_tbl);
> > > > >
> > > > > +static void igc_read_rar0(struct igc_hw *hw, u8 *addr, u32 *ral, u32
> > > > > +*rah) {
> > > > > +	*ral = rd32(IGC_RAL(0));
> > > > > +	*rah = rd32(IGC_RAH(0));
> > > > > +
> > > > > +	addr[0] = *ral & 0xff;
> > > > > +	addr[1] = (*ral >> 8) & 0xff;
> > > > > +	addr[2] = (*ral >> 16) & 0xff;
> > > > > +	addr[3] = (*ral >> 24) & 0xff;
> > > > > +	addr[4] = *rah & 0xff;
> > > > > +	addr[5] = (*rah >> 8) & 0xff;
> > > > > +}
> > > > > +
> > > > > +static bool igc_is_lmvp_device(struct pci_dev *pdev) {
> > > > > +	switch (pdev->device) {
> > > > > +	case IGC_DEV_ID_I225_LMVP:
> > > > > +	case IGC_DEV_ID_I226_LMVP:
> > > > > +		return true;
> > > > > +	default:
> > > > > +		return false;
> > > > > +	}
> > > > > +}
> > > > > +
> > > > > +static void igc_wait_for_lmvp_mac_passthrough(struct pci_dev *pdev,
> > > > > +					      struct igc_hw *hw)
> > > > > +{
> > > > > +	u8 addr[ETH_ALEN] __aligned(2);
> > > > > +	u32 orig_ral, orig_rah;
> > > > > +	u32 ral, rah;
> > > > > +	int i;
> > > > > +
> > > > > +	if (!igc_is_lmvp_device(pdev))
> > > > > +		return;
> > > > > +
> > > > > +	igc_read_rar0(hw, addr, &orig_ral, &orig_rah);
> > > > > +
> > > > > +	for (i = 0; i < 100; i++) {
> > > > > +		msleep(100);
> > > > > +		igc_read_rar0(hw, addr, &ral, &rah);
> > > > > +		if ((ral != orig_ral || rah != orig_rah) &&
> > > > > +		    is_valid_ether_addr(addr))
> > > > > +			return;
> > > > > +	}
> > > > > +}
> > > > > +
> > > > >    enum latency_range {
> > > > >    	lowest_latency = 0,
> > > > >    	low_latency = 1,
> > > > > @@ -7259,6 +7306,7 @@ static int igc_probe(struct pci_dev *pdev,
> > > > >    	 * known good starting state
> > > > >    	 */
> > > > >    	hw->mac.ops.reset_hw(hw);
> > > > > +	igc_wait_for_lmvp_mac_passthrough(pdev, hw);
> > > > >
> > > > >    	if (igc_get_flash_presence_i225(hw)) {
> > > > >    		if (hw->nvm.ops.validate(hw) < 0) {
> > > > > --
> > > > > 2.53.0
> > > >
> > >
>

WARNING: multiple messages have this Message-ID (diff)
From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>
To: "Ruinskiy, Dima" <dima.ruinskiy@intel.com>
Cc: "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>,
	 "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
	 Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>,
	 "intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH 1/2] igc: Wait for MAC passthrough after reset
Date: Wed, 15 Jul 2026 13:46:09 +0800	[thread overview]
Message-ID: <alcd_FnUdrQug5cm@acelan-Precision-5480> (raw)
In-Reply-To: <1d6c3aae-c9b1-42da-b5fa-6f8ee9ebcae1@intel.com>

On Mon, Jun 29, 2026 at 07:42:43PM +0300, Ruinskiy, Dima wrote:
> Hi AceLan,
>
> Some more comments below.
>
> On 22/06/2026 4:57, Chia-Lin Kao (AceLan) wrote:
>
> > >
> > > Because 100 iterations of 100msec each - this translates to up-to 10
> > > seconds, no?
> > Yes, just in case it takes longer.
> > I think 5 seconds should be enough if you feel this is feasible.
> 5 seconds also sounds excessive, assuming the majority of the systems do not
> support / enable MAC passthrough via the FW feature (and I have a hunch it
> is the case). 2 seconds... maybe, but then you say that in some rare cases
> it exceeds 2 seconds as well. What if in some even rare cases it exceeds
> 5/10 seconds? Perhaps because of some glitch it will not come up at all on
> this cycle. Should the driver always wait?
>
> > I wish we can detect if the MAC passthrough is enabled, so that we
> > know if we need to poll for the MAC address.
> Unfortunately, like you I am not aware of any way for the driver to know
> whether MAC passthrough via FW is enabled. Because of this we have been
> exploring a simpler way to support this feature via ACPI objects (which are
> set by the BIOS when MAC passthrough is enabled and are easy for the driver
> to query). I know some vendors have already implemented it, and I am
> currently drafting a patch to send.
Any good news about this patch?

>
> > For the FW interrupt mechanism also needs BIOS support, and we don't
> > have the power to push this.
> The mechanism I have in mind does not require BIOS support - the I225/I226
> FW already supports the required interrupt, AFAIK - it merely needs to be
> enabled in the igc driver. With that said, there still remains the question
> of how to notify the network stack above us that the MAC address has changed
> post-probe - and whether it is even supported.
>
> --Dima
>
> >
> > >
> > > Thanks,
> > > Dima.
> > >
> > > >
> > > > > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > > > > ---
> > > > >    drivers/net/ethernet/intel/igc/igc_main.c | 48
> > > > > +++++++++++++++++++++++
> > > > >    1 file changed, 48 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > b/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > index 2c9e2dfd8499..fa9752ed8bc5 100644
> > > > > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > > > > @@ -11,6 +11,7 @@
> > > > >    #include <net/pkt_sched.h>
> > > > >    #include <linux/bpf_trace.h>
> > > > >    #include <net/xdp_sock_drv.h>
> > > > > +#include <linux/etherdevice.h>
> > > > >    #include <linux/pci.h>
> > > > >    #include <linux/mdio.h>
> > > > >
> > > > > @@ -69,6 +70,52 @@ static const struct pci_device_id igc_pci_tbl[] = {
> > > > >
> > > > >    MODULE_DEVICE_TABLE(pci, igc_pci_tbl);
> > > > >
> > > > > +static void igc_read_rar0(struct igc_hw *hw, u8 *addr, u32 *ral, u32
> > > > > +*rah) {
> > > > > +	*ral = rd32(IGC_RAL(0));
> > > > > +	*rah = rd32(IGC_RAH(0));
> > > > > +
> > > > > +	addr[0] = *ral & 0xff;
> > > > > +	addr[1] = (*ral >> 8) & 0xff;
> > > > > +	addr[2] = (*ral >> 16) & 0xff;
> > > > > +	addr[3] = (*ral >> 24) & 0xff;
> > > > > +	addr[4] = *rah & 0xff;
> > > > > +	addr[5] = (*rah >> 8) & 0xff;
> > > > > +}
> > > > > +
> > > > > +static bool igc_is_lmvp_device(struct pci_dev *pdev) {
> > > > > +	switch (pdev->device) {
> > > > > +	case IGC_DEV_ID_I225_LMVP:
> > > > > +	case IGC_DEV_ID_I226_LMVP:
> > > > > +		return true;
> > > > > +	default:
> > > > > +		return false;
> > > > > +	}
> > > > > +}
> > > > > +
> > > > > +static void igc_wait_for_lmvp_mac_passthrough(struct pci_dev *pdev,
> > > > > +					      struct igc_hw *hw)
> > > > > +{
> > > > > +	u8 addr[ETH_ALEN] __aligned(2);
> > > > > +	u32 orig_ral, orig_rah;
> > > > > +	u32 ral, rah;
> > > > > +	int i;
> > > > > +
> > > > > +	if (!igc_is_lmvp_device(pdev))
> > > > > +		return;
> > > > > +
> > > > > +	igc_read_rar0(hw, addr, &orig_ral, &orig_rah);
> > > > > +
> > > > > +	for (i = 0; i < 100; i++) {
> > > > > +		msleep(100);
> > > > > +		igc_read_rar0(hw, addr, &ral, &rah);
> > > > > +		if ((ral != orig_ral || rah != orig_rah) &&
> > > > > +		    is_valid_ether_addr(addr))
> > > > > +			return;
> > > > > +	}
> > > > > +}
> > > > > +
> > > > >    enum latency_range {
> > > > >    	lowest_latency = 0,
> > > > >    	low_latency = 1,
> > > > > @@ -7259,6 +7306,7 @@ static int igc_probe(struct pci_dev *pdev,
> > > > >    	 * known good starting state
> > > > >    	 */
> > > > >    	hw->mac.ops.reset_hw(hw);
> > > > > +	igc_wait_for_lmvp_mac_passthrough(pdev, hw);
> > > > >
> > > > >    	if (igc_get_flash_presence_i225(hw)) {
> > > > >    		if (hw->nvm.ops.validate(hw) < 0) {
> > > > > --
> > > > > 2.53.0
> > > >
> > >
>

  reply	other threads:[~2026-07-15  5:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18  7:33 [Intel-wired-lan] [PATCH 1/2] igc: Wait for MAC passthrough after reset Chia-Lin Kao (AceLan) via Intel-wired-lan
2026-06-18  7:33 ` Chia-Lin Kao (AceLan)
2026-06-18  7:33 ` [Intel-wired-lan] [PATCH 2/2] igc: Cache MAC passthrough address Chia-Lin Kao (AceLan) via Intel-wired-lan
2026-06-18  7:33   ` Chia-Lin Kao (AceLan)
2026-06-18  7:55 ` [Intel-wired-lan] [PATCH 1/2] igc: Wait for MAC passthrough after reset Loktionov, Aleksandr
2026-06-18  7:55   ` Loktionov, Aleksandr
2026-06-18  8:51   ` Ruinskiy, Dima
2026-06-22  1:57     ` Chia-Lin Kao (AceLan) via Intel-wired-lan
2026-06-22  1:57       ` Chia-Lin Kao (AceLan)
2026-06-22  8:54       ` Loktionov, Aleksandr
2026-06-22  8:54         ` Loktionov, Aleksandr
2026-06-29 16:42       ` Ruinskiy, Dima
2026-07-15  5:46         ` Chia-Lin Kao (AceLan) via Intel-wired-lan [this message]
2026-07-15  5:46           ` Chia-Lin Kao (AceLan)
2026-07-15  7:57           ` Ruinskiy, Dima
2026-06-18  8:08 ` Andrew Lunn
2026-06-18  8:08   ` Andrew Lunn
2026-06-18  8:49 ` [Intel-wired-lan] " Kwapulinski, Piotr
2026-06-18  8:49   ` Kwapulinski, Piotr
2026-06-18 10:11 ` Paul Menzel

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=alcd_FnUdrQug5cm@acelan-Precision-5480 \
    --to=intel-wired-lan@osuosl.org \
    --cc=acelan.kao@canonical.com \
    --cc=aleksandr.loktionov@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=dima.ruinskiy@intel.com \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@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.