Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Esben Haabendal <esben@geanix.com>
To:     netdev@vger.kernel.org
Cc:     "David S. Miller" <davem@davemloft.net>,
        Michal Simek <michal.simek@xilinx.com>,
        YueHaibing <yuehaibing@huawei.com>,
        Yang Wei <yang.wei9@zte.com.cn>,
        Luis Chamberlain <mcgrof@kernel.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: [PATCH 07/12] net: ll_temac: Support indirect_mutex share within TEMAC IP
Date:   Fri, 26 Apr 2019 09:32:26 +0200
Message-Id: <20190426073231.4008-8-esben@geanix.com>
In-Reply-To: <20190426073231.4008-1-esben@geanix.com>
References: <20190426073231.4008-1-esben@geanix.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Indirect register access goes through a DCR bus bridge, which
allows only one outstanding transaction.  And to make matters
worse, each TEMAC IP block contains two Ethernet interfaces, and
although they seem to have separate registers for indirect access,
they actually share the registers.  Or to be more specific, MSW, LSW
and CTL registers are physically shared between Ethernet interfaces
in same TEMAC IP, with RDY register being (almost) specificic to
the Ethernet interface.  The 0x10000 bit in RDY reflects combined
bus ready state though.

So we need to take care to synchronize not only within a single
device, but also between devices in same TEMAC IP.

This commit allows to do that with legacy platform devices.

For OF devices, the xlnx,compound parent of the temac node should be
used to find siblings, and setup a shared indirect_mutex between them.
I will leave this work to somebody else, as I don't have hardware to
test that.  No regression is introduced by that, as before this commit
using two Ethernet interfaces in same TEMAC block is simply broken.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/net/ethernet/xilinx/ll_temac.h      |  5 ++++-
 drivers/net/ethernet/xilinx/ll_temac_main.c | 30 ++++++++++++++++++-----------
 drivers/net/ethernet/xilinx/ll_temac_mdio.c | 16 +++++++--------
 include/linux/xilinx_ll_temac.h             |  6 ++++++
 4 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/ll_temac.h b/drivers/net/ethernet/xilinx/ll_temac.h
index 23d8dd5..990f9ed 100644
--- a/drivers/net/ethernet/xilinx/ll_temac.h
+++ b/drivers/net/ethernet/xilinx/ll_temac.h
@@ -358,7 +358,10 @@ struct temac_local {
 
 	struct sk_buff **rx_skb;
 	spinlock_t rx_lock;
-	struct mutex indirect_mutex;
+	/* For synchronization of indirect register access.  Must be
+	 * shared mutex between interfaces in same TEMAC block.
+	 */
+	struct mutex *indirect_mutex;
 	u32 options;			/* Current options word */
 	int last_link;
 	unsigned int temac_features;
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 817bc57..309f149 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -346,7 +346,7 @@ static void temac_do_set_mac_address(struct net_device *ndev)
 	struct temac_local *lp = netdev_priv(ndev);
 
 	/* set up unicast MAC address filter set its mac address */
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	temac_indirect_out32(lp, XTE_UAW0_OFFSET,
 			     (ndev->dev_addr[0]) |
 			     (ndev->dev_addr[1] << 8) |
@@ -357,7 +357,7 @@ static void temac_do_set_mac_address(struct net_device *ndev)
 	temac_indirect_out32(lp, XTE_UAW1_OFFSET,
 			     (ndev->dev_addr[4] & 0x000000ff) |
 			     (ndev->dev_addr[5] << 8));
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 }
 
 static int temac_init_mac_address(struct net_device *ndev, const void *address)
@@ -386,7 +386,7 @@ static void temac_set_multicast_list(struct net_device *ndev)
 	u32 multi_addr_msw, multi_addr_lsw, val;
 	int i;
 
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	if (ndev->flags & (IFF_ALLMULTI | IFF_PROMISC) ||
 	    netdev_mc_count(ndev) > MULTICAST_CAM_TABLE_NUM) {
 		/*
@@ -425,7 +425,7 @@ static void temac_set_multicast_list(struct net_device *ndev)
 		temac_indirect_out32(lp, XTE_MAW1_OFFSET, 0);
 		dev_info(&ndev->dev, "Promiscuous mode disabled.\n");
 	}
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 }
 
 static struct temac_option {
@@ -517,7 +517,7 @@ static u32 temac_setoptions(struct net_device *ndev, u32 options)
 	struct temac_option *tp = &temac_options[0];
 	int reg;
 
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	while (tp->opt) {
 		reg = temac_indirect_in32(lp, tp->reg) & ~tp->m_or;
 		if (options & tp->opt)
@@ -526,7 +526,7 @@ static u32 temac_setoptions(struct net_device *ndev, u32 options)
 		tp++;
 	}
 	lp->options |= options;
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 
 	return 0;
 }
@@ -545,7 +545,7 @@ static void temac_device_reset(struct net_device *ndev)
 
 	dev_dbg(&ndev->dev, "%s()\n", __func__);
 
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	/* Reset the receiver and wait for it to finish reset */
 	temac_indirect_out32(lp, XTE_RXC1_OFFSET, XTE_RXC1_RXRST_MASK);
 	timeout = 1000;
@@ -597,7 +597,7 @@ static void temac_device_reset(struct net_device *ndev)
 	temac_indirect_out32(lp, XTE_TXC_OFFSET, 0);
 	temac_indirect_out32(lp, XTE_FCC_OFFSET, XTE_FCC_RXFLO_MASK);
 
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 
 	/* Sync default options with HW
 	 * but leave receiver and transmitter disabled.  */
@@ -625,7 +625,7 @@ static void temac_adjust_link(struct net_device *ndev)
 	/* hash together the state values to decide if something has changed */
 	link_state = phy->speed | (phy->duplex << 1) | phy->link;
 
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	if (lp->last_link != link_state) {
 		mii_speed = temac_indirect_in32(lp, XTE_EMCFG_OFFSET);
 		mii_speed &= ~XTE_EMCFG_LINKSPD_MASK;
@@ -641,7 +641,7 @@ static void temac_adjust_link(struct net_device *ndev)
 		lp->last_link = link_state;
 		phy_print_status(phy);
 	}
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 }
 
 #ifdef CONFIG_64BIT
@@ -1092,7 +1092,15 @@ static int temac_probe(struct platform_device *pdev)
 	lp->dev = &pdev->dev;
 	lp->options = XTE_OPTION_DEFAULTS;
 	spin_lock_init(&lp->rx_lock);
-	mutex_init(&lp->indirect_mutex);
+
+	/* Setup mutex for synchronization of indirect register access */
+	if (pdata && pdata->indirect_mutex) {
+		lp->indirect_mutex = pdata->indirect_mutex;
+	} else {
+		lp->indirect_mutex = devm_kmalloc(
+			&pdev->dev, sizeof(*lp->indirect_mutex), GFP_KERNEL);
+		mutex_init(lp->indirect_mutex);
+	}
 
 	/* map device registers */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/net/ethernet/xilinx/ll_temac_mdio.c b/drivers/net/ethernet/xilinx/ll_temac_mdio.c
index 61b832f..c8707f1 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_mdio.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_mdio.c
@@ -29,10 +29,10 @@ static int temac_mdio_read(struct mii_bus *bus, int phy_id, int reg)
 	/* Write the PHY address to the MIIM Access Initiator register.
 	 * When the transfer completes, the PHY register value will appear
 	 * in the LSW0 register */
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	temac_iow(lp, XTE_LSW0_OFFSET, (phy_id << 5) | reg);
 	rc = temac_indirect_in32(lp, XTE_MIIMAI_OFFSET);
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 
 	dev_dbg(lp->dev, "temac_mdio_read(phy_id=%i, reg=%x) == %x\n",
 		phy_id, reg, rc);
@@ -50,10 +50,10 @@ static int temac_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val)
 	/* First write the desired value into the write data register
 	 * and then write the address into the access initiator register
 	 */
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	temac_indirect_out32(lp, XTE_MGTDR_OFFSET, val);
 	temac_indirect_out32(lp, XTE_MIIMAI_OFFSET, (phy_id << 5) | reg);
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 
 	return 0;
 }
@@ -87,9 +87,9 @@ int temac_mdio_setup(struct temac_local *lp, struct platform_device *pdev)
 
 	/* Enable the MDIO bus by asserting the enable bit and writing
 	 * in the clock config */
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	temac_indirect_out32(lp, XTE_MC_OFFSET, 1 << 6 | clk_div);
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 
 	bus = devm_mdiobus_alloc(&pdev->dev);
 	if (!bus)
@@ -116,10 +116,10 @@ int temac_mdio_setup(struct temac_local *lp, struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	mutex_lock(&lp->indirect_mutex);
+	mutex_lock(lp->indirect_mutex);
 	dev_dbg(lp->dev, "MDIO bus registered;  MC:%x\n",
 		temac_indirect_in32(lp, XTE_MC_OFFSET));
-	mutex_unlock(&lp->indirect_mutex);
+	mutex_unlock(lp->indirect_mutex);
 	return 0;
 }
 
diff --git a/include/linux/xilinx_ll_temac.h b/include/linux/xilinx_ll_temac.h
index 53f9203..c4c66be 100644
--- a/include/linux/xilinx_ll_temac.h
+++ b/include/linux/xilinx_ll_temac.h
@@ -15,6 +15,12 @@ struct ll_temac_platform_data {
 	phy_interface_t phy_interface; /* PHY interface mode */
 	bool reg_little_endian;	/* Little endian TEMAC register access  */
 	bool dma_little_endian;	/* Little endian DMA register access  */
+	/* Pre-initialized mutex to use for synchronizing indirect
+	 * register access.  When using both interfaces of a single
+	 * TEMAC IP block, the same mutex should be passed here, as
+	 * they share the same DCR bus bridge.
+	 */
+	struct mutex *indirect_mutex;
 };
 
 #endif /* __LINUX_XILINX_LL_TEMAC_H */
-- 
2.4.11