netdev - Re: [PATCH net-next 5/8] smsc95xx: use PAL framework read/write functions

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] [day] [month] [year] [list]

Message-ID: <20200630012641.GF597495@lunn.ch>
Date:   Tue, 30 Jun 2020 03:26:41 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Andre.Edich@...rochip.com
Cc:     netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com,
        steve.glendinning@...well.net, Parthiban.Veerasooran@...rochip.com
Subject: Re: [PATCH net-next 5/8] smsc95xx: use PAL framework read/write
 functions

On Mon, Jun 29, 2020 at 01:12:37PM +0000, Andre.Edich@...rochip.com wrote:
> Use functions phy_read and phy_write instead of smsc95xx_mdio_read and
> smsc95xx_mdio_write respectively.
> 
> Signed-off-by: Andre Edich <andre.edich@...rochip.com>
> ---
>  drivers/net/usb/smsc95xx.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 3b8f7e439f44..11af5d5ece60 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -574,7 +574,7 @@ static int smsc95xx_link_reset(struct usbnet *dev)
>  	int ret;
>  
>  	/* clear interrupt status */
> -	ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
> +	ret = phy_read(pdata->phydev, PHY_INT_SRC);
>  	if (ret < 0)
>  		return ret;

The PHY driver should be clearing the interrupt status before it
enables interrupts.

>  
> @@ -584,8 +584,8 @@ static int smsc95xx_link_reset(struct usbnet *dev)
>  
>  	mii_check_media(mii, 1, 1);
>  	mii_ethtool_gset(&dev->mii, &ecmd);
> -	lcladv = smsc95xx_mdio_read(dev->net, mii->phy_id,
> MII_ADVERTISE);
> -	rmtadv = smsc95xx_mdio_read(dev->net, mii->phy_id, MII_LPA);
> +	lcladv = phy_read(pdata->phydev, MII_ADVERTISE);
> +	rmtadv = phy_read(pdata->phydev, MII_LPA);
>  
>  	netif_dbg(dev, link, dev->net,
>  		  "speed: %u duplex: %d lcladv: %04x rmtadv: %04x\n",

One of your previous patches added a link change handler which prints
the current speed, duplex, etc. I think you can remove this code.

> @@ -753,10 +753,11 @@ static int smsc95xx_ethtool_set_wol(struct
> net_device *net,
>  static int get_mdix_status(struct net_device *net)
>  {

The PHY driver should be providing this information, not the MAC.

When using phylib, the MAC driver should never need to directly access
the PHY. The PHY driver does all the work. The MAC driver might need
to call into phylib to request the work is done.

   Andrew