diff mbox series

[v4,5/5] raw/ifpga_rawdev/base: fix retimer link status issue

Message ID 20190621084017.6763-5-tianfei.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v4,1/5] raw/ifpga_rawdev: fix use of untrusted scalar value |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Zhang, Tianfei June 21, 2019, 8:40 a.m. UTC 
  Fix the readout retimer link status incorrectly when we
remove the linux intel-fpga-driver and run the DPDK application.
The linux driver will stop the retimer when remove the kernel
modules.

Fixes: 8a256bef ("raw/ifpga/base: add eth group driver")
Cc: stable@dpdk.org

Reported-by: Sampath Amrutha <amrutha.sampath@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 .../raw/ifpga_rawdev/base/opae_eth_group.c    | 172 ++++++++++++++++++
 .../raw/ifpga_rawdev/base/opae_eth_group.h    |   6 +
 .../raw/ifpga_rawdev/base/opae_intel_max10.c  |   7 -
 3 files changed, 178 insertions(+), 7 deletions(-)

Comments

Xu, Rosen July 1, 2019, 10:31 a.m. UTC | #1 
> -----Original Message-----
> From: Zhang, Tianfei
> Sent: Friday, June 21, 2019 16:40
> To: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Xu, Rosen <rosen.xu@intel.com>; stable@dpdk.org; Zhang, Tianfei
> <tianfei.zhang@intel.com>
> Subject: [PATCH v4 5/5] raw/ifpga_rawdev/base: fix retimer link status issue
> 
> Fix the readout retimer link status incorrectly when we remove the linux
> intel-fpga-driver and run the DPDK application.
> The linux driver will stop the retimer when remove the kernel modules.
> 
> Fixes: 8a256bef ("raw/ifpga/base: add eth group driver")
> Cc: stable@dpdk.org
> 
> Reported-by: Sampath Amrutha <amrutha.sampath@intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
>  .../raw/ifpga_rawdev/base/opae_eth_group.c    | 172 ++++++++++++++++++
>  .../raw/ifpga_rawdev/base/opae_eth_group.h    |   6 +
>  .../raw/ifpga_rawdev/base/opae_intel_max10.c  |   7 -
>  3 files changed, 178 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
> b/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
> index 8db6693b1..d189dd578 100644
> --- a/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
> +++ b/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
> @@ -113,6 +113,171 @@ int eth_group_read_reg(struct eth_group_device
> *dev,
>  	return 0;
>  }
> 
> +static int eth_group_reset_mac(struct eth_group_device *dev, u8 index,
> +			       bool enable)
> +{
> +	u32 val;
> +	int ret;
> +
> +	/*
> +	 * only support 25G & 40G mac reset for now. It uses internal reset.
> +	 * as PHY and MAC are integrated together, below action will trigger
> +	 * PHY reset too.
> +	 */
> +	if (dev->speed != 25 && dev->speed != 40)
> +		return 0;
> +
> +	ret = eth_group_read_reg(dev, ETH_GROUP_MAC, index,
> MAC_CONFIG,
> +				 &val);
> +	if (ret) {
> +		dev_err(dev, "fail to read PHY_CONFIG: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* skip if mac is in expected state already */
> +	if ((((val & MAC_RESET_MASK) == MAC_RESET_MASK) && enable) ||
> +	    (((val & MAC_RESET_MASK) == 0) && !enable))
> +		return 0;
> +
> +	if (enable)
> +		val |= MAC_RESET_MASK;
> +	else
> +		val &= ~MAC_RESET_MASK;
> +
> +	ret = eth_group_write_reg(dev, ETH_GROUP_MAC, index,
> MAC_CONFIG,
> +				  val);
> +	if (ret)
> +		dev_err(dev, "fail to write PHY_CONFIG: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static void eth_group_mac_uinit(struct eth_group_device *dev) {
> +	u8 i;
> +
> +	for (i = 0; i < dev->mac_num; i++) {
> +		if (eth_group_reset_mac(dev, i, true))
> +			dev_err(dev, "fail to disable mac %d\n", i);
> +	}
> +}
> +
> +static int eth_group_mac_init(struct eth_group_device *dev) {
> +	int ret;
> +	u8 i;
> +
> +	for (i = 0; i < dev->mac_num; i++) {
> +		ret = eth_group_reset_mac(dev, i, false);
> +		if (ret) {
> +			dev_err(dev, "fail to enable mac %d\n", i);
> +			goto exit;
> +		}
> +	}
> +
> +	return 0;
> +
> +exit:
> +	while (i--)
> +		eth_group_reset_mac(dev, i, true);
> +
> +	return ret;
> +}
> +
> +static int eth_group_reset_phy(struct eth_group_device *dev, u8 index,
> +		bool enable)
> +{
> +	u32 val;
> +	int ret;
> +
> +	/* only support 10G PHY reset for now. It uses external reset. */
> +	if (dev->speed != 10)
> +		return 0;
> +
> +	ret = eth_group_read_reg(dev, ETH_GROUP_PHY, index,
> +			ADD_PHY_CTRL, &val);
> +	if (ret) {
> +		dev_err(dev, "fail to read ADD_PHY_CTRL reg: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* return if PHY is already in expected state */
> +	if ((val & PHY_RESET && enable) || (!(val & PHY_RESET) && !enable))
> +		return 0;
> +
> +	if (enable)
> +		val |= PHY_RESET;
> +	else
> +		val &= ~PHY_RESET;
> +
> +	ret = eth_group_write_reg(dev, ETH_GROUP_PHY, index,
> +			ADD_PHY_CTRL, val);
> +	if (ret)
> +		dev_err(dev, "fail to write ADD_PHY_CTRL reg: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int eth_group_phy_init(struct eth_group_device *dev) {
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < dev->phy_num; i++) {
> +		ret = eth_group_reset_phy(dev, i, false);
> +		if (ret) {
> +			dev_err(dev, "fail to enable phy %d\n", i);
> +			goto exit;
> +		}
> +	}
> +
> +	return 0;
> +exit:
> +	while (i--)
> +		eth_group_reset_phy(dev, i, true);
> +
> +	return ret;
> +}
> +
> +static void eth_group_phy_uinit(struct eth_group_device *dev) {
> +	int i;
> +
> +	for (i = 0; i < dev->phy_num; i++) {
> +		if (eth_group_reset_phy(dev, i, true))
> +			dev_err(dev, "fail to disable phy %d\n", i);
> +	}
> +}
> +
> +static int eth_group_hw_init(struct eth_group_device *dev) {
> +	int ret;
> +
> +	ret = eth_group_phy_init(dev);
> +	if (ret) {
> +		dev_err(dev, "fail to init eth group phys\n");
> +		return ret;
> +	}
> +
> +	ret = eth_group_mac_init(dev);
> +	if (ret) {
> +		dev_err(priv->dev, "fail to init eth group macs\n");
> +		goto phy_exit;
> +	}
> +
> +	return 0;
> +
> +phy_exit:
> +	eth_group_phy_uinit(dev);
> +	return ret;
> +}
> +
> +static void eth_group_hw_uinit(struct eth_group_device *dev) {
> +	eth_group_mac_uinit(dev);
> +	eth_group_phy_uinit(dev);
> +}
> +
>  struct eth_group_device *eth_group_probe(void *base)  {
>  	struct eth_group_device *dev;
> @@ -130,6 +295,11 @@ struct eth_group_device *eth_group_probe(void
> *base)
> 
>  	dev->status = ETH_GROUP_DEV_ATTACHED;
> 
> +	if (eth_group_hw_init(dev)) {
> +		dev_err(dev, "eth group hw init fail\n");
> +		return NULL;
> +	}
> +
>  	dev_info(dev, "eth group device %d probe done:
> phy_num=mac_num:%d, speed=%d\n",
>  			dev->group_id, dev->phy_num, dev->speed);
> 
> @@ -138,6 +308,8 @@ struct eth_group_device *eth_group_probe(void
> *base)
> 
>  void eth_group_release(struct eth_group_device *dev)  {
> +	eth_group_hw_uinit(dev);
> +
>  	if (dev) {
>  		dev->status = ETH_GROUP_DEV_NOUSED;
>  		opae_free(dev);
> diff --git a/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
> b/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
> index a66d77e27..4868bd0e1 100644
> --- a/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
> +++ b/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
> @@ -44,6 +44,12 @@
>  #define STAT_DATA_VAL		BIT_ULL(32)
>  #define STAT_RD_DATA		GENMASK_ULL(31, 0)
> 
> +/* Additional Feature Register */
> +#define ADD_PHY_CTRL            0x0
> +#define PHY_RESET               BIT(0)
> +#define MAC_CONFIG      0x310
> +#define MAC_RESET_MASK  GENMASK(2, 0)
> +
>  struct opae_eth_group_info {
>  	u8 group_id;
>  	u8 speed;
> diff --git a/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
> b/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
> index 3ff6575d7..9ed10e282 100644
> --- a/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
> +++ b/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
> @@ -57,13 +57,6 @@ intel_max10_device_probe(struct altera_spi_device
> *spi,
>  	}
>  	dev_info(dev, "FPGA loaded from %s Image\n", val ? "User" :
> "Factory");
> 
> -	/* set PKVL Polling manually in BBS */
> -	ret = max10_reg_write(PKVL_POLLING_CTRL, 0x3);
> -	if (ret != 0) {
> -		dev_err(dev, "%s set PKVL polling fail\n", __func__);
> -		goto spi_tran_fail;
> -	}
> -
>  	return dev;
> 
>  spi_tran_fail:
> --
> 2.17.1

Acked-by: Rosen Xu <rosen.xu@intel.com>
diff mbox series

Patch

diff --git a/drivers/raw/ifpga_rawdev/base/opae_eth_group.c b/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
index 8db6693b1..d189dd578 100644
--- a/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
+++ b/drivers/raw/ifpga_rawdev/base/opae_eth_group.c
@@ -113,6 +113,171 @@  int eth_group_read_reg(struct eth_group_device *dev,
 	return 0;
 }
 
+static int eth_group_reset_mac(struct eth_group_device *dev, u8 index,
+			       bool enable)
+{
+	u32 val;
+	int ret;
+
+	/*
+	 * only support 25G & 40G mac reset for now. It uses internal reset.
+	 * as PHY and MAC are integrated together, below action will trigger
+	 * PHY reset too.
+	 */
+	if (dev->speed != 25 && dev->speed != 40)
+		return 0;
+
+	ret = eth_group_read_reg(dev, ETH_GROUP_MAC, index, MAC_CONFIG,
+				 &val);
+	if (ret) {
+		dev_err(dev, "fail to read PHY_CONFIG: %d\n", ret);
+		return ret;
+	}
+
+	/* skip if mac is in expected state already */
+	if ((((val & MAC_RESET_MASK) == MAC_RESET_MASK) && enable) ||
+	    (((val & MAC_RESET_MASK) == 0) && !enable))
+		return 0;
+
+	if (enable)
+		val |= MAC_RESET_MASK;
+	else
+		val &= ~MAC_RESET_MASK;
+
+	ret = eth_group_write_reg(dev, ETH_GROUP_MAC, index, MAC_CONFIG,
+				  val);
+	if (ret)
+		dev_err(dev, "fail to write PHY_CONFIG: %d\n", ret);
+
+	return ret;
+}
+
+static void eth_group_mac_uinit(struct eth_group_device *dev)
+{
+	u8 i;
+
+	for (i = 0; i < dev->mac_num; i++) {
+		if (eth_group_reset_mac(dev, i, true))
+			dev_err(dev, "fail to disable mac %d\n", i);
+	}
+}
+
+static int eth_group_mac_init(struct eth_group_device *dev)
+{
+	int ret;
+	u8 i;
+
+	for (i = 0; i < dev->mac_num; i++) {
+		ret = eth_group_reset_mac(dev, i, false);
+		if (ret) {
+			dev_err(dev, "fail to enable mac %d\n", i);
+			goto exit;
+		}
+	}
+
+	return 0;
+
+exit:
+	while (i--)
+		eth_group_reset_mac(dev, i, true);
+
+	return ret;
+}
+
+static int eth_group_reset_phy(struct eth_group_device *dev, u8 index,
+		bool enable)
+{
+	u32 val;
+	int ret;
+
+	/* only support 10G PHY reset for now. It uses external reset. */
+	if (dev->speed != 10)
+		return 0;
+
+	ret = eth_group_read_reg(dev, ETH_GROUP_PHY, index,
+			ADD_PHY_CTRL, &val);
+	if (ret) {
+		dev_err(dev, "fail to read ADD_PHY_CTRL reg: %d\n", ret);
+		return ret;
+	}
+
+	/* return if PHY is already in expected state */
+	if ((val & PHY_RESET && enable) || (!(val & PHY_RESET) && !enable))
+		return 0;
+
+	if (enable)
+		val |= PHY_RESET;
+	else
+		val &= ~PHY_RESET;
+
+	ret = eth_group_write_reg(dev, ETH_GROUP_PHY, index,
+			ADD_PHY_CTRL, val);
+	if (ret)
+		dev_err(dev, "fail to write ADD_PHY_CTRL reg: %d\n", ret);
+
+	return ret;
+}
+
+static int eth_group_phy_init(struct eth_group_device *dev)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < dev->phy_num; i++) {
+		ret = eth_group_reset_phy(dev, i, false);
+		if (ret) {
+			dev_err(dev, "fail to enable phy %d\n", i);
+			goto exit;
+		}
+	}
+
+	return 0;
+exit:
+	while (i--)
+		eth_group_reset_phy(dev, i, true);
+
+	return ret;
+}
+
+static void eth_group_phy_uinit(struct eth_group_device *dev)
+{
+	int i;
+
+	for (i = 0; i < dev->phy_num; i++) {
+		if (eth_group_reset_phy(dev, i, true))
+			dev_err(dev, "fail to disable phy %d\n", i);
+	}
+}
+
+static int eth_group_hw_init(struct eth_group_device *dev)
+{
+	int ret;
+
+	ret = eth_group_phy_init(dev);
+	if (ret) {
+		dev_err(dev, "fail to init eth group phys\n");
+		return ret;
+	}
+
+	ret = eth_group_mac_init(dev);
+	if (ret) {
+		dev_err(priv->dev, "fail to init eth group macs\n");
+		goto phy_exit;
+	}
+
+	return 0;
+
+phy_exit:
+	eth_group_phy_uinit(dev);
+	return ret;
+}
+
+static void eth_group_hw_uinit(struct eth_group_device *dev)
+{
+	eth_group_mac_uinit(dev);
+	eth_group_phy_uinit(dev);
+}
+
 struct eth_group_device *eth_group_probe(void *base)
 {
 	struct eth_group_device *dev;
@@ -130,6 +295,11 @@  struct eth_group_device *eth_group_probe(void *base)
 
 	dev->status = ETH_GROUP_DEV_ATTACHED;
 
+	if (eth_group_hw_init(dev)) {
+		dev_err(dev, "eth group hw init fail\n");
+		return NULL;
+	}
+
 	dev_info(dev, "eth group device %d probe done: phy_num=mac_num:%d, speed=%d\n",
 			dev->group_id, dev->phy_num, dev->speed);
 
@@ -138,6 +308,8 @@  struct eth_group_device *eth_group_probe(void *base)
 
 void eth_group_release(struct eth_group_device *dev)
 {
+	eth_group_hw_uinit(dev);
+
 	if (dev) {
 		dev->status = ETH_GROUP_DEV_NOUSED;
 		opae_free(dev);
diff --git a/drivers/raw/ifpga_rawdev/base/opae_eth_group.h b/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
index a66d77e27..4868bd0e1 100644
--- a/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
+++ b/drivers/raw/ifpga_rawdev/base/opae_eth_group.h
@@ -44,6 +44,12 @@ 
 #define STAT_DATA_VAL		BIT_ULL(32)
 #define STAT_RD_DATA		GENMASK_ULL(31, 0)
 
+/* Additional Feature Register */
+#define ADD_PHY_CTRL            0x0
+#define PHY_RESET               BIT(0)
+#define MAC_CONFIG      0x310
+#define MAC_RESET_MASK  GENMASK(2, 0)
+
 struct opae_eth_group_info {
 	u8 group_id;
 	u8 speed;
diff --git a/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c b/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
index 3ff6575d7..9ed10e282 100644
--- a/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
+++ b/drivers/raw/ifpga_rawdev/base/opae_intel_max10.c
@@ -57,13 +57,6 @@  intel_max10_device_probe(struct altera_spi_device *spi,
 	}
 	dev_info(dev, "FPGA loaded from %s Image\n", val ? "User" : "Factory");
 
-	/* set PKVL Polling manually in BBS */
-	ret = max10_reg_write(PKVL_POLLING_CTRL, 0x3);
-	if (ret != 0) {
-		dev_err(dev, "%s set PKVL polling fail\n", __func__);
-		goto spi_tran_fail;
-	}
-
 	return dev;
 
 spi_tran_fail: