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: <baa51dc7-3605-0001-386a-35e386b920da@ti.com>
Date:   Thu, 15 Sep 2022 14:06:46 +0530
From:   Siddharth Vadapalli <s-vadapalli@...com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
CC:     <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski@...aro.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <vladimir.oltean@....com>,
        <grygorii.strashko@...com>, <vigneshr@...com>, <nsekhar@...com>,
        <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <kishon@...com>,
        <s-vadapalli@...com>
Subject: Re: [PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES
 configuration

Hello Russell,

On 14/09/22 21:07, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:47PM +0530, Siddharth Vadapalli wrote:
>> @@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
>>  	struct net_device *ndev = port->ndev;
>>  	int tmo;
>>  
>> +	/* disable phy */
>> +	am65_cpsw_disable_phy(port->slave.ifphy);
>> +
> 
> This seems really strange. If you have a serdes interface which
> presumably supports SGMII, 1000base-X etc, then link status is sent
> across the serdes interface. If you power down the serdes, then you
> can't receive the link status, and so mac_link_up() won't be called.
> 
> Are you really sure you want to be enabling and disabling the PHY
> in mac_link_down()/mac_link_up() ?

Thank you for reviewing the patch. The PHY passed to the
"am65_cpsw_disable_phy()" and "am65_cpsw_disable_phy()" functions within
the "am65_cpsw_nuss_mac_link_down()" and "am65_cpsw_nuss_mac_link_up()"
functions respectively, is the CPSW ethernet MAC's PHY and not the
SERDES PHY. The SERDES PHY is powered on through the function call to
the "am65_cpsw_init_phy()" function.

The calls to the functions "am65_cpsw_enable_phy()" and
"am65_cpsw_disable_phy()" within the "am65_cpsw_nuss_mac_link_up()" and
"am65_cpsw_nuss_mac_link_down()" functions respectively, try to power on
and power off the CPSW ethernet MAC's phy. Looking at it again,they do
nothing, since the driver corresponding to the ethernet MAC's PHY which
happens to be drivers/phy/ti/phy-gmii-sel.c, does not provide any
methods to power on and power off the ethernet MAC's PHY. I have just
realized that this is stale code and will remove it in the v2 series.

Also, I realize now that I did not invoke "am65_cpsw_disable_phy()" on
the SERDES PHY in the driver's remove function. I will fix this in the
v2 series.

Regards,
Siddharth.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ