[gPXE-devel] [PATCH] Re-enable management mode checks on e1000e

[Geoff Lywood]

I am a little bit worried about removing the management mode checks based on the comments in the code. From what I see, there are two places where management mode comes into play:

1. The driver avoids powering off the PHY if management mode is enabled. Presumably, powering off the PHY has ill effects on the other (IAMT) driver using the PHY. I'm not sure if we ever call into this path, though, so we may be able to remove it.

2. e1000e_copper_link_setup_gg82563_80003es2lan checks if management mode is enabled before writing some PHY registers. I think that this check might really be needed, because it implies that the PHY registers could have different values if they were set up by the IAMT side of the interface, and we don't want to clobber what is there.

(If we're really looking to save bytes, we could rewrite the call site so that it doesn't use a function pointer.)

Thanks,
Geoff


> -----Original Message-----
> From: oremanj at gmail.com [mailto:oremanj at gmail.com] On Behalf Of Joshua
> Oreman
> Sent: Thursday, May 27, 2010 5:04 PM
> To: Geoff Lywood
> Cc: gpxe-devel at etherboot.org; Marty Connor
> Subject: Re: [gPXE-devel] [PATCH] Re-enable management mode checks on
> e1000e
> 
> On Thu, May 27, 2010 at 4:29 PM, Geoff Lywood <glywood at vmware.com> wrote:
> > Currently the e1000e driver will crash when the 80003es2lan code
> attempts to
> > call hw->mac.ops.check_mng_mode(), because that function pointer is
> never
> > initialized. This patch makes sure that check_mng_mode is set for every
> type
> > of PHY.
> >
> > I am open to suggestions for other ways to solve this problem. I do not
> > really understand why the management mode checks were commented out in
> the
> > first place.
> 
> Adding Marty on CC, as he's the one who ported the driver. I suspect
> this was simply a case of overzealous feature removal (as most of the
> e1000e's features are unnecessary in gPXE); might it be possible
> instead to remove the need to check for management mode?
> 
> -- Josh