netdev - Re: [PATCH v7 03/11] net: pch_gbe: Probe PHY ID & initialize only once

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180627173106.s2lbolvz4x5mqr64@pburton-laptop>
Date:   Wed, 27 Jun 2018 10:31:06 -0700
From:   Paul Burton <paul.burton@...s.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH v7 03/11] net: pch_gbe: Probe PHY ID & initialize only
 once

Hi Andrew,

On Wed, Jun 27, 2018 at 07:21:31PM +0200, Andrew Lunn wrote:
> > [1] Please, someone patent PHY hotplugging & rigorously enforce said
> >     patent such that nobody can do it. At least not with an EG20T MAC.
> 
> Hi Paul
> 
> It is already possible, and probably patented. SFP cages are usually
> used for fibre optical modules. But it is also possible to have copper
> modules, which contain a standard PHY. And SFP modules are
> hot-plugable...

D'oh, but at least not relevant to the EG20T/pch_gbe :)

> > @@ -2577,6 +2579,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> >  	if (ret)
> >  		goto err_free_netdev;
> >  
> > +	pch_gbe_check_options(adapter);
> > +
> >  	/* Initialize PHY */
> >  	ret = pch_gbe_init_phy(adapter);
> >  	if (ret) {
> > @@ -2606,8 +2610,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> >  
> >  	INIT_WORK(&adapter->reset_task, pch_gbe_reset_task);
> >  
> > -	pch_gbe_check_options(adapter);
> > -
> >  	/* initialize the wol settings based on the eeprom settings */
> >  	adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
> >  	dev_info(&pdev->dev, "MAC address : %pM\n", netdev->dev_addr);
> 
> But these two changes seem unrelated. Should they be in a different
> patch?

This is actually needed because pch_gbe_check_options() sets up, amongst
other things, the autoneg_advertised field in struct pch_gbe_phy_info
and that needs to happen before pch_gbe_phy_init_setting() is called.

Thanks,
    Paul