[v2,13/25] net/axgbe: remove use of comm owned field

Message ID 20240507124305.2318-13-venkatkumar.ande@amd.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v2,01/25] net/axgbe: fix mdio access for non-zero ports and CL45 PHYs |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Venkat Kumar Ande May 7, 2024, 12:42 p.m. UTC
The comm_owned field can hide logic where double locking is attempted
and prevent multiple threads for the same device from accessing the
mutex properly.  Remove the comm_owned field and use the mutex API
exclusively for gaining ownership.

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 16 ----------------
 1 file changed, 16 deletions(-)
  

Comments

Sebastian, Selwin May 20, 2024, 10:42 a.m. UTC | #1
[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

-----Original Message-----
From: Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Sent: Tuesday, May 7, 2024 6:13 PM
To: dev@dpdk.org
Cc: Sebastian, Selwin <Selwin.Sebastian@amd.com>; Ande, Venkat Kumar <VenkatKumar.Ande@amd.com>
Subject: [PATCH v2 13/25] net/axgbe: remove use of comm owned field

The comm_owned field can hide logic where double locking is attempted and prevent multiple threads for the same device from accessing the mutex properly.  Remove the comm_owned field and use the mutex API exclusively for gaining ownership.

Signed-off-by: Venkat Kumar Ande <venkatkumar.ande@amd.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 0ad5b4ba85..d2a2b124be 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -204,8 +204,6 @@ struct axgbe_phy_data {

        unsigned int mdio_addr;

-       unsigned int comm_owned;
-
        /* SFP Support */
        enum axgbe_sfp_comm sfp_comm;
        unsigned int sfp_mux_address;
@@ -253,12 +251,6 @@ static enum axgbe_an_mode axgbe_phy_an_mode(struct axgbe_port *pdata);  static int axgbe_phy_i2c_xfer(struct axgbe_port *pdata,
                              struct axgbe_i2c_op *i2c_op)
 {
-       struct axgbe_phy_data *phy_data = pdata->phy_data;
-
-       /* Be sure we own the bus */
-       if (!phy_data->comm_owned)
-               return -EIO;
-
        return pdata->i2c_if.i2c_xfer(pdata, i2c_op);  }

@@ -399,10 +391,6 @@ static int axgbe_phy_sfp_get_mux(struct axgbe_port *pdata)

 static void axgbe_phy_put_comm_ownership(struct axgbe_port *pdata)  {
-       struct axgbe_phy_data *phy_data = pdata->phy_data;
-
-       phy_data->comm_owned = 0;
-
        pthread_mutex_unlock(&pdata->phy_mutex);
 }

@@ -418,9 +406,6 @@ static int axgbe_phy_get_comm_ownership(struct axgbe_port *pdata)
         */
        pthread_mutex_lock(&pdata->phy_mutex);

-       if (phy_data->comm_owned)
-               return 0;
-
        /* Clear the mutexes */
        XP_IOWRITE(pdata, XP_I2C_MUTEX, AXGBE_MUTEX_RELEASE);
        XP_IOWRITE(pdata, XP_MDIO_MUTEX, AXGBE_MUTEX_RELEASE); @@ -443,7 +428,6 @@ static int axgbe_phy_get_comm_ownership(struct axgbe_port *pdata)
                XP_IOWRITE(pdata, XP_I2C_MUTEX, mutex_id);
                XP_IOWRITE(pdata, XP_MDIO_MUTEX, mutex_id);

-               phy_data->comm_owned = 1;
                return 0;
        }

--
2.34.1
  

Patch

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 0ad5b4ba85..d2a2b124be 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -204,8 +204,6 @@  struct axgbe_phy_data {
 
 	unsigned int mdio_addr;
 
-	unsigned int comm_owned;
-
 	/* SFP Support */
 	enum axgbe_sfp_comm sfp_comm;
 	unsigned int sfp_mux_address;
@@ -253,12 +251,6 @@  static enum axgbe_an_mode axgbe_phy_an_mode(struct axgbe_port *pdata);
 static int axgbe_phy_i2c_xfer(struct axgbe_port *pdata,
 			      struct axgbe_i2c_op *i2c_op)
 {
-	struct axgbe_phy_data *phy_data = pdata->phy_data;
-
-	/* Be sure we own the bus */
-	if (!phy_data->comm_owned)
-		return -EIO;
-
 	return pdata->i2c_if.i2c_xfer(pdata, i2c_op);
 }
 
@@ -399,10 +391,6 @@  static int axgbe_phy_sfp_get_mux(struct axgbe_port *pdata)
 
 static void axgbe_phy_put_comm_ownership(struct axgbe_port *pdata)
 {
-	struct axgbe_phy_data *phy_data = pdata->phy_data;
-
-	phy_data->comm_owned = 0;
-
 	pthread_mutex_unlock(&pdata->phy_mutex);
 }
 
@@ -418,9 +406,6 @@  static int axgbe_phy_get_comm_ownership(struct axgbe_port *pdata)
 	 */
 	pthread_mutex_lock(&pdata->phy_mutex);
 
-	if (phy_data->comm_owned)
-		return 0;
-
 	/* Clear the mutexes */
 	XP_IOWRITE(pdata, XP_I2C_MUTEX, AXGBE_MUTEX_RELEASE);
 	XP_IOWRITE(pdata, XP_MDIO_MUTEX, AXGBE_MUTEX_RELEASE);
@@ -443,7 +428,6 @@  static int axgbe_phy_get_comm_ownership(struct axgbe_port *pdata)
 		XP_IOWRITE(pdata, XP_I2C_MUTEX, mutex_id);
 		XP_IOWRITE(pdata, XP_MDIO_MUTEX, mutex_id);
 
-		phy_data->comm_owned = 1;
 		return 0;
 	}