linux-kernel - Re: [RFC PATCH net-next 1/4] net: phy: used the genphy_soft

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]

Date:   Wed, 2 Mar 2022 04:19:02 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Arun Ramadoss <arun.ramadoss@...rochip.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [RFC PATCH net-next 1/4] net: phy: used the genphy_soft_reset
 for phy reset in Lan87xx

On Mon, Feb 28, 2022 at 07:35:07PM +0530, Arun Ramadoss wrote:
> Replaced current code for soft resetting phy to genphy_soft_reset
> function. And added the macro for LAN87xx Phy ID.

Hi Arun

Please don't mix multiple things in one patch.

Looking at the actual path, you have:

> +#define LAN87XX_PHY_ID			0x0007c150
> +#define MICROCHIP_PHY_ID_MASK		0xfffffff0

Part of macros for PHY ID.

> +
>  /* External Register Control Register */
>  #define LAN87XX_EXT_REG_CTL                     (0x14)
>  #define LAN87XX_EXT_REG_CTL_RD_CTL              (0x1000)
> @@ -197,20 +200,10 @@ static int lan87xx_phy_init(struct phy_device *phydev)
>  	if (rc < 0)
>  		return rc;
>  
> -	/* Soft Reset the SMI block */
> -	rc = access_ereg_modify_changed(phydev, PHYACC_ATTR_BANK_SMI,
> -					0x00, 0x8000, 0x8000);
> -	if (rc < 0)
> -		return rc;
> -
> -	/* Check to see if the self-clearing bit is cleared */
> -	usleep_range(1000, 2000);
> -	rc = access_ereg(phydev, PHYACC_ATTR_MODE_READ,
> -			 PHYACC_ATTR_BANK_SMI, 0x00, 0);
> +	/* phy Soft reset */
> +	rc = genphy_soft_reset(phydev);
>  	if (rc < 0)
>  		return rc;
> -	if ((rc & 0x8000) != 0)
> -		return -ETIMEDOUT;

Soft reset.

>  
>  	/* PHY Initialization */
>  	for (i = 0; i < ARRAY_SIZE(init); i++) {
> @@ -273,6 +266,9 @@ static int lan87xx_config_init(struct phy_device *phydev)
>  {
>  	int rc = lan87xx_phy_init(phydev);
>  
> +	if (rc < 0)
> +		phydev_err(phydev, "failed to initialize phy\n");
> +

A new error message.

>  	return rc < 0 ? rc : 0;
>  }
>  
> @@ -506,18 +502,14 @@ static int lan87xx_cable_test_get_status(struct phy_device *phydev,
>  
>  static struct phy_driver microchip_t1_phy_driver[] = {
>  	{
> -		.phy_id         = 0x0007c150,
> -		.phy_id_mask    = 0xfffffff0,
> -		.name           = "Microchip LAN87xx T1",
> +		.phy_id         = LAN87XX_PHY_ID,
> +		.phy_id_mask    = MICROCHIP_PHY_ID_MASK,

2nd part of the PHY ID macros.

> +		.name           = "LAN87xx T1",

A change in name.

>  		.flags          = PHY_POLL_CABLE_TEST,
> -
>  		.features       = PHY_BASIC_T1_FEATURES,
> -
>  		.config_init	= lan87xx_config_init,
> -
>  		.config_intr    = lan87xx_phy_config_intr,
>  		.handle_interrupt = lan87xx_handle_interrupt,
> -

White space changes.

You can also use PHY_ID_MATCH_MODEL().

    Andrew