netdev - Re: [PATCH net-next 9/9] phy: fixed_phy: Set phy capabilities even when link is down

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: <55DA1387.9020009@gmail.com>
Date:	Sun, 23 Aug 2015 11:40:07 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
CC:	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 9/9] phy: fixed_phy: Set phy capabilities even
 when link is down

Le 08/23/15 02:47, Andrew Lunn a écrit :
> What features a phy supports is masked in genphy_config_init() by
> looking at the PHYs BMSR register.
> 
> If the link is down, fixed_phy_update_regs() will only set the auto-
> negotiation capable bit in BMSR. Thus genphy_config_init() comes to
> the conclusion the PHY can only perform 10/Half, and masks out the
> higher speed features. If however the link it up, BMSR is set to
> indicate the speed the PHY is capable of auto-negotiating, and
> genphy_config_init() does not mask out the high speed features.
> 
> To fix this, when the link is down, have fixed_phy_update_regs() leave
> the link status and auto-negotiation complete bit unset, but set all
> the other bits depending on the fixed phy speed.

This kinds of revert what Staas did in commit
868a4215be9a6d80548ccb74763b883dc99d32a2 ("net: phy: fixed_phy: handle
link-down case"). When the link is down, it does not seem to me like we
can rely on the previous speed and duplex parameters to be considered valid.

Your change does fix a valid use case though... humm.

> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
>  drivers/net/phy/fixed_phy.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index 14ab80899e77..ff4869222c27 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -57,9 +57,8 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>  	if (gpio_is_valid(fp->link_gpio))
>  		fp->status.link = !!gpio_get_value_cansleep(fp->link_gpio);
>  
> -	if (!fp->status.link)
> -		goto done;
> -	bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
> +	if (fp->status.link)
> +		bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>  
>  	if (fp->status.duplex) {
>  		bmcr |= BMCR_FULLDPLX;
> @@ -111,7 +110,6 @@ static int fixed_phy_update_regs(struct fixed_phy *fp)
>  	if (fp->status.asym_pause)
>  		lpa |= LPA_PAUSE_ASYM;
>  
> -done:
>  	fp->regs[MII_PHYSID1] = 0;
>  	fp->regs[MII_PHYSID2] = 0;
>  
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html