[PATCH net-next v7 0/9] TXGBE PHYLINK support

[PATCH net-next v7 0/9] TXGBE PHYLINK support

[Jiawen Wu]

Implement I2C, SFP, GPIO and PHYLINK to setup TXGBE link.

Because our I2C and PCS are based on Synopsys Designware IP-core, extend
the i2c-designware and pcs-xpcs driver to realize our functions.

v6 -> v7:
- change swnode property of I2C platform to be boolean
- use device_property_present() to match I2C device data

v5 -> v6:
- fix to set error code if pointer of txgbe is NULL
- change "if" to "switch" for *_i2c_dw_xfer_quirk()
- rename property for I2C device flag
- use regmap to access I2C mem region
- use DEFINE_RES_IRQ()
- use phylink_mii_c45_pcs_get_state() for DW_XPCS_10GBASER

v4 -> v5:
- add clock register
- delete i2c-dw.h with platform data
- introduce property "i2c-dw-flags" to match device flags
- get resource from platform info to do ioremap
- rename quirk functions in i2c-designware-*.c
- fix calling txgbe_phylink_init()

v3 -> v4:
- modify I2C transfer to be generic implementation
- avoid to read DW_IC_COMP_PARAM_1
- remove redundant "if" statement
- add specific labels to handle error in txgbe_init_phy(), and remove
  "if" conditions in txgbe_remove_phy()

v2 -> v3:
- delete own I2C bus master driver, support it in i2c-designware
- delete own PCS functions, remove pma configuration and 1000BASE-X mode
- add basic function for 10GBASE-R interface in pcs-xpcs
- add helper to get txgbe pointer from netdev

v1 -> v2:
- add comments to indicate GPIO lines
- add I2C write operation support
- modify GPIO direction functions
- rename functions related to PHY interface
- add condition on interface changing to re-config PCS
- add to set advertise and fix to get status for 1000BASE-X mode
- other redundant codes remove

Jiawen Wu (9):
  net: txgbe: Add software nodes to support phylink
  i2c: designware: Add driver support for Wangxun 10Gb NIC
  net: txgbe: Register fixed rate clock
  net: txgbe: Register I2C platform device
  net: txgbe: Add SFP module identify
  net: txgbe: Support GPIO to SFP socket
  net: pcs: Add 10GBASE-R mode for Synopsys Designware XPCS
  net: txgbe: Implement phylink pcs
  net: txgbe: Support phylink MAC layer

 drivers/i2c/busses/i2c-designware-common.c    |   8 +
 drivers/i2c/busses/i2c-designware-core.h      |   1 +
 drivers/i2c/busses/i2c-designware-master.c    |  89 ++-
 drivers/i2c/busses/i2c-designware-platdrv.c   |  15 +
 drivers/net/ethernet/wangxun/Kconfig          |   8 +
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   |   3 +-
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |   3 +
 drivers/net/ethernet/wangxun/txgbe/Makefile   |   1 +
 .../ethernet/wangxun/txgbe/txgbe_ethtool.c    |  28 +
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  65 +-
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 651 ++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_phy.h    |  10 +
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  94 +++
 drivers/net/pcs/pcs-xpcs.c                    |  30 +
 include/linux/pcs/pcs-xpcs.h                  |   1 +
 15 files changed, 969 insertions(+), 38 deletions(-)
 create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
 create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h

-- 
2.27.0

[PATCH net-next v7 1/9] net: txgbe: Add software nodes to support phylink

[Jiawen Wu]

Register software nodes for GPIO, I2C, SFP and PHYLINK. Define the
device properties.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  1 +
 drivers/net/ethernet/wangxun/txgbe/Makefile   |  1 +
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 ++++-
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 89 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_phy.h    | 10 +++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   | 49 ++++++++++
 6 files changed, 171 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
 create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 32f952d93009..97bce855bc60 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -611,6 +611,7 @@ enum wx_isb_idx {
 
 struct wx {
 	u8 __iomem *hw_addr;
+	void *priv;
 	struct pci_dev *pdev;
 	struct net_device *netdev;
 	struct wx_bus_info bus;
diff --git a/drivers/net/ethernet/wangxun/txgbe/Makefile b/drivers/net/ethernet/wangxun/txgbe/Makefile
index 6db14a2cb2d0..7507f762edfe 100644
--- a/drivers/net/ethernet/wangxun/txgbe/Makefile
+++ b/drivers/net/ethernet/wangxun/txgbe/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_TXGBE) += txgbe.o
 
 txgbe-objs := txgbe_main.o \
               txgbe_hw.o \
+              txgbe_phy.o \
               txgbe_ethtool.o
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index 5b8a121fb496..e10296abf5b4 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -15,6 +15,7 @@
 #include "../libwx/wx_hw.h"
 #include "txgbe_type.h"
 #include "txgbe_hw.h"
+#include "txgbe_phy.h"
 #include "txgbe_ethtool.h"
 
 char txgbe_driver_name[] = "txgbe";
@@ -513,6 +514,7 @@ static int txgbe_probe(struct pci_dev *pdev,
 	struct net_device *netdev;
 	int err, expected_gts;
 	struct wx *wx = NULL;
+	struct txgbe *txgbe;
 
 	u16 eeprom_verh = 0, eeprom_verl = 0, offset = 0;
 	u16 eeprom_cfg_blkh = 0, eeprom_cfg_blkl = 0;
@@ -663,10 +665,23 @@ static int txgbe_probe(struct pci_dev *pdev,
 			 "0x%08x", etrack_id);
 	}
 
-	err = register_netdev(netdev);
+	txgbe = devm_kzalloc(&pdev->dev, sizeof(*txgbe), GFP_KERNEL);
+	if (!txgbe) {
+		err = -ENOMEM;
+		goto err_release_hw;
+	}
+
+	txgbe->wx = wx;
+	wx->priv = txgbe;
+
+	err = txgbe_init_phy(txgbe);
 	if (err)
 		goto err_release_hw;
 
+	err = register_netdev(netdev);
+	if (err)
+		goto err_remove_phy;
+
 	pci_set_drvdata(pdev, wx);
 
 	netif_tx_stop_all_queues(netdev);
@@ -694,6 +709,8 @@ static int txgbe_probe(struct pci_dev *pdev,
 
 	return 0;
 
+err_remove_phy:
+	txgbe_remove_phy(txgbe);
 err_release_hw:
 	wx_clear_interrupt_scheme(wx);
 	wx_control_hw(wx, false);
@@ -719,11 +736,14 @@ static int txgbe_probe(struct pci_dev *pdev,
 static void txgbe_remove(struct pci_dev *pdev)
 {
 	struct wx *wx = pci_get_drvdata(pdev);
+	struct txgbe *txgbe = wx->priv;
 	struct net_device *netdev;
 
 	netdev = wx->netdev;
 	unregister_netdev(netdev);
 
+	txgbe_remove_phy(txgbe);
+
 	pci_release_selected_regions(pdev,
 				     pci_select_bars(pdev, IORESOURCE_MEM));
 
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
new file mode 100644
index 000000000000..3476074869cb
--- /dev/null
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
+
+#include <linux/gpio/property.h>
+#include <linux/i2c.h>
+#include <linux/pci.h>
+
+#include "../libwx/wx_type.h"
+#include "txgbe_type.h"
+#include "txgbe_phy.h"
+
+static int txgbe_swnodes_register(struct txgbe *txgbe)
+{
+	struct txgbe_nodes *nodes = &txgbe->nodes;
+	struct pci_dev *pdev = txgbe->wx->pdev;
+	struct software_node *swnodes;
+	u32 id;
+
+	id = (pdev->bus->number << 8) | pdev->devfn;
+
+	snprintf(nodes->gpio_name, sizeof(nodes->gpio_name), "txgbe_gpio-%x", id);
+	snprintf(nodes->i2c_name, sizeof(nodes->i2c_name), "txgbe_i2c-%x", id);
+	snprintf(nodes->sfp_name, sizeof(nodes->sfp_name), "txgbe_sfp-%x", id);
+	snprintf(nodes->phylink_name, sizeof(nodes->phylink_name), "txgbe_phylink-%x", id);
+
+	swnodes = nodes->swnodes;
+
+	/* GPIO 0: tx fault
+	 * GPIO 1: tx disable
+	 * GPIO 2: sfp module absent
+	 * GPIO 3: rx signal lost
+	 * GPIO 4: rate select, 1G(0) 10G(1)
+	 * GPIO 5: rate select, 1G(0) 10G(1)
+	 */
+	nodes->gpio_props[0] = PROPERTY_ENTRY_STRING("pinctrl-names", "default");
+	swnodes[SWNODE_GPIO] = NODE_PROP(nodes->gpio_name, nodes->gpio_props);
+	nodes->gpio0_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 0, GPIO_ACTIVE_HIGH);
+	nodes->gpio1_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 1, GPIO_ACTIVE_HIGH);
+	nodes->gpio2_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 2, GPIO_ACTIVE_LOW);
+	nodes->gpio3_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 3, GPIO_ACTIVE_HIGH);
+	nodes->gpio4_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 4, GPIO_ACTIVE_HIGH);
+	nodes->gpio5_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 5, GPIO_ACTIVE_HIGH);
+
+	nodes->i2c_props[0] = PROPERTY_ENTRY_STRING("compatible", "snps,designware-i2c");
+	nodes->i2c_props[1] = PROPERTY_ENTRY_BOOL("snps,i2c-platform");
+	nodes->i2c_props[2] = PROPERTY_ENTRY_U32("clock-frequency", I2C_MAX_STANDARD_MODE_FREQ);
+	swnodes[SWNODE_I2C] = NODE_PROP(nodes->i2c_name, nodes->i2c_props);
+	nodes->i2c_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_I2C]);
+
+	nodes->sfp_props[0] = PROPERTY_ENTRY_STRING("compatible", "sff,sfp");
+	nodes->sfp_props[1] = PROPERTY_ENTRY_REF_ARRAY("i2c-bus", nodes->i2c_ref);
+	nodes->sfp_props[2] = PROPERTY_ENTRY_REF_ARRAY("tx-fault-gpios", nodes->gpio0_ref);
+	nodes->sfp_props[3] = PROPERTY_ENTRY_REF_ARRAY("tx-disable-gpios", nodes->gpio1_ref);
+	nodes->sfp_props[4] = PROPERTY_ENTRY_REF_ARRAY("mod-def0-gpios", nodes->gpio2_ref);
+	nodes->sfp_props[5] = PROPERTY_ENTRY_REF_ARRAY("los-gpios", nodes->gpio3_ref);
+	nodes->sfp_props[6] = PROPERTY_ENTRY_REF_ARRAY("rate-select1-gpios", nodes->gpio4_ref);
+	nodes->sfp_props[7] = PROPERTY_ENTRY_REF_ARRAY("rate-select0-gpios", nodes->gpio5_ref);
+	swnodes[SWNODE_SFP] = NODE_PROP(nodes->sfp_name, nodes->sfp_props);
+	nodes->sfp_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_SFP]);
+
+	nodes->phylink_props[0] = PROPERTY_ENTRY_STRING("managed", "in-band-status");
+	nodes->phylink_props[1] = PROPERTY_ENTRY_REF_ARRAY("sfp", nodes->sfp_ref);
+	swnodes[SWNODE_PHYLINK] = NODE_PROP(nodes->phylink_name, nodes->phylink_props);
+
+	nodes->group[SWNODE_GPIO] = &swnodes[SWNODE_GPIO];
+	nodes->group[SWNODE_I2C] = &swnodes[SWNODE_I2C];
+	nodes->group[SWNODE_SFP] = &swnodes[SWNODE_SFP];
+	nodes->group[SWNODE_PHYLINK] = &swnodes[SWNODE_PHYLINK];
+
+	return software_node_register_node_group(nodes->group);
+}
+
+int txgbe_init_phy(struct txgbe *txgbe)
+{
+	int ret;
+
+	ret = txgbe_swnodes_register(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to register software nodes\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+void txgbe_remove_phy(struct txgbe *txgbe)
+{
+	software_node_unregister_node_group(txgbe->nodes.group);
+}
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h
new file mode 100644
index 000000000000..1ab592124986
--- /dev/null
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
+
+#ifndef _TXGBE_PHY_H_
+#define _TXGBE_PHY_H_
+
+int txgbe_init_phy(struct txgbe *txgbe);
+void txgbe_remove_phy(struct txgbe *txgbe);
+
+#endif /* _TXGBE_NODE_H_ */
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 63a1c733718d..5bef0f9df523 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -4,6 +4,8 @@
 #ifndef _TXGBE_TYPE_H_
 #define _TXGBE_TYPE_H_
 
+#include <linux/property.h>
+
 /* Device IDs */
 #define TXGBE_DEV_ID_SP1000                     0x1001
 #define TXGBE_DEV_ID_WX1820                     0x2001
@@ -99,4 +101,51 @@
 
 extern char txgbe_driver_name[];
 
+static inline struct txgbe *netdev_to_txgbe(struct net_device *netdev)
+{
+	struct wx *wx = netdev_priv(netdev);
+
+	return wx->priv;
+}
+
+#define NODE_PROP(_NAME, _PROP)			\
+	(const struct software_node) {		\
+		.name = _NAME,			\
+		.properties = _PROP,		\
+	}
+
+enum txgbe_swnodes {
+	SWNODE_GPIO = 0,
+	SWNODE_I2C,
+	SWNODE_SFP,
+	SWNODE_PHYLINK,
+	SWNODE_MAX
+};
+
+struct txgbe_nodes {
+	char gpio_name[32];
+	char i2c_name[32];
+	char sfp_name[32];
+	char phylink_name[32];
+	struct property_entry gpio_props[1];
+	struct property_entry i2c_props[3];
+	struct property_entry sfp_props[8];
+	struct property_entry phylink_props[2];
+	struct software_node_ref_args i2c_ref[1];
+	struct software_node_ref_args gpio0_ref[1];
+	struct software_node_ref_args gpio1_ref[1];
+	struct software_node_ref_args gpio2_ref[1];
+	struct software_node_ref_args gpio3_ref[1];
+	struct software_node_ref_args gpio4_ref[1];
+	struct software_node_ref_args gpio5_ref[1];
+	struct software_node_ref_args sfp_ref[1];
+	struct software_node swnodes[SWNODE_MAX];
+	const struct software_node *group[SWNODE_MAX + 1];
+};
+
+struct txgbe {
+	struct wx *wx;
+	struct txgbe_nodes nodes;
+};
+
 #endif /* _TXGBE_TYPE_H_ */
-- 
2.27.0

Re: [PATCH net-next v7 1/9] net: txgbe: Add software nodes to support phylink

[Piotr Raczynski]

On Tue, May 09, 2023 at 10:27:26AM +0800, Jiawen Wu wrote:
> Register software nodes for GPIO, I2C, SFP and PHYLINK. Define the
> device properties.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
LGTM, thanks.
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_type.h  |  1 +
>  drivers/net/ethernet/wangxun/txgbe/Makefile   |  1 +
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 ++++-
>  .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 89 +++++++++++++++++++
>  .../net/ethernet/wangxun/txgbe/txgbe_phy.h    | 10 +++
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   | 49 ++++++++++
>  6 files changed, 171 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
>  create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index 32f952d93009..97bce855bc60 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -611,6 +611,7 @@ enum wx_isb_idx {
>  
>  struct wx {
>  	u8 __iomem *hw_addr;
> +	void *priv;
>  	struct pci_dev *pdev;
>  	struct net_device *netdev;
>  	struct wx_bus_info bus;
> diff --git a/drivers/net/ethernet/wangxun/txgbe/Makefile b/drivers/net/ethernet/wangxun/txgbe/Makefile
> index 6db14a2cb2d0..7507f762edfe 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/Makefile
> +++ b/drivers/net/ethernet/wangxun/txgbe/Makefile
> @@ -8,4 +8,5 @@ obj-$(CONFIG_TXGBE) += txgbe.o
>  
>  txgbe-objs := txgbe_main.o \
>                txgbe_hw.o \
> +              txgbe_phy.o \
>                txgbe_ethtool.o
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index 5b8a121fb496..e10296abf5b4 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -15,6 +15,7 @@
>  #include "../libwx/wx_hw.h"
>  #include "txgbe_type.h"
>  #include "txgbe_hw.h"
> +#include "txgbe_phy.h"
>  #include "txgbe_ethtool.h"
>  
>  char txgbe_driver_name[] = "txgbe";
> @@ -513,6 +514,7 @@ static int txgbe_probe(struct pci_dev *pdev,
>  	struct net_device *netdev;
>  	int err, expected_gts;
>  	struct wx *wx = NULL;
> +	struct txgbe *txgbe;
>  
>  	u16 eeprom_verh = 0, eeprom_verl = 0, offset = 0;
>  	u16 eeprom_cfg_blkh = 0, eeprom_cfg_blkl = 0;
> @@ -663,10 +665,23 @@ static int txgbe_probe(struct pci_dev *pdev,
>  			 "0x%08x", etrack_id);
>  	}
>  
> -	err = register_netdev(netdev);
> +	txgbe = devm_kzalloc(&pdev->dev, sizeof(*txgbe), GFP_KERNEL);
> +	if (!txgbe) {
> +		err = -ENOMEM;
> +		goto err_release_hw;
> +	}
> +
> +	txgbe->wx = wx;
> +	wx->priv = txgbe;
> +
> +	err = txgbe_init_phy(txgbe);
>  	if (err)
>  		goto err_release_hw;
>  
> +	err = register_netdev(netdev);
> +	if (err)
> +		goto err_remove_phy;
> +
>  	pci_set_drvdata(pdev, wx);
>  
>  	netif_tx_stop_all_queues(netdev);
> @@ -694,6 +709,8 @@ static int txgbe_probe(struct pci_dev *pdev,
>  
>  	return 0;
>  
> +err_remove_phy:
> +	txgbe_remove_phy(txgbe);
>  err_release_hw:
>  	wx_clear_interrupt_scheme(wx);
>  	wx_control_hw(wx, false);
> @@ -719,11 +736,14 @@ static int txgbe_probe(struct pci_dev *pdev,
>  static void txgbe_remove(struct pci_dev *pdev)
>  {
>  	struct wx *wx = pci_get_drvdata(pdev);
> +	struct txgbe *txgbe = wx->priv;
>  	struct net_device *netdev;
>  
>  	netdev = wx->netdev;
>  	unregister_netdev(netdev);
>  
> +	txgbe_remove_phy(txgbe);
> +
>  	pci_release_selected_regions(pdev,
>  				     pci_select_bars(pdev, IORESOURCE_MEM));
>  
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> new file mode 100644
> index 000000000000..3476074869cb
> --- /dev/null
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
> +
> +#include <linux/gpio/property.h>
> +#include <linux/i2c.h>
> +#include <linux/pci.h>
> +
> +#include "../libwx/wx_type.h"
> +#include "txgbe_type.h"
> +#include "txgbe_phy.h"
> +
> +static int txgbe_swnodes_register(struct txgbe *txgbe)
> +{
> +	struct txgbe_nodes *nodes = &txgbe->nodes;
> +	struct pci_dev *pdev = txgbe->wx->pdev;
> +	struct software_node *swnodes;
> +	u32 id;
> +
> +	id = (pdev->bus->number << 8) | pdev->devfn;
> +
> +	snprintf(nodes->gpio_name, sizeof(nodes->gpio_name), "txgbe_gpio-%x", id);
> +	snprintf(nodes->i2c_name, sizeof(nodes->i2c_name), "txgbe_i2c-%x", id);
> +	snprintf(nodes->sfp_name, sizeof(nodes->sfp_name), "txgbe_sfp-%x", id);
> +	snprintf(nodes->phylink_name, sizeof(nodes->phylink_name), "txgbe_phylink-%x", id);
> +
> +	swnodes = nodes->swnodes;
> +
> +	/* GPIO 0: tx fault
> +	 * GPIO 1: tx disable
> +	 * GPIO 2: sfp module absent
> +	 * GPIO 3: rx signal lost
> +	 * GPIO 4: rate select, 1G(0) 10G(1)
> +	 * GPIO 5: rate select, 1G(0) 10G(1)
> +	 */
> +	nodes->gpio_props[0] = PROPERTY_ENTRY_STRING("pinctrl-names", "default");
> +	swnodes[SWNODE_GPIO] = NODE_PROP(nodes->gpio_name, nodes->gpio_props);
> +	nodes->gpio0_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 0, GPIO_ACTIVE_HIGH);
> +	nodes->gpio1_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 1, GPIO_ACTIVE_HIGH);
> +	nodes->gpio2_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 2, GPIO_ACTIVE_LOW);
> +	nodes->gpio3_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 3, GPIO_ACTIVE_HIGH);
> +	nodes->gpio4_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 4, GPIO_ACTIVE_HIGH);
> +	nodes->gpio5_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_GPIO], 5, GPIO_ACTIVE_HIGH);
> +
> +	nodes->i2c_props[0] = PROPERTY_ENTRY_STRING("compatible", "snps,designware-i2c");
> +	nodes->i2c_props[1] = PROPERTY_ENTRY_BOOL("snps,i2c-platform");
> +	nodes->i2c_props[2] = PROPERTY_ENTRY_U32("clock-frequency", I2C_MAX_STANDARD_MODE_FREQ);
> +	swnodes[SWNODE_I2C] = NODE_PROP(nodes->i2c_name, nodes->i2c_props);
> +	nodes->i2c_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_I2C]);
> +
> +	nodes->sfp_props[0] = PROPERTY_ENTRY_STRING("compatible", "sff,sfp");
> +	nodes->sfp_props[1] = PROPERTY_ENTRY_REF_ARRAY("i2c-bus", nodes->i2c_ref);
> +	nodes->sfp_props[2] = PROPERTY_ENTRY_REF_ARRAY("tx-fault-gpios", nodes->gpio0_ref);
> +	nodes->sfp_props[3] = PROPERTY_ENTRY_REF_ARRAY("tx-disable-gpios", nodes->gpio1_ref);
> +	nodes->sfp_props[4] = PROPERTY_ENTRY_REF_ARRAY("mod-def0-gpios", nodes->gpio2_ref);
> +	nodes->sfp_props[5] = PROPERTY_ENTRY_REF_ARRAY("los-gpios", nodes->gpio3_ref);
> +	nodes->sfp_props[6] = PROPERTY_ENTRY_REF_ARRAY("rate-select1-gpios", nodes->gpio4_ref);
> +	nodes->sfp_props[7] = PROPERTY_ENTRY_REF_ARRAY("rate-select0-gpios", nodes->gpio5_ref);
> +	swnodes[SWNODE_SFP] = NODE_PROP(nodes->sfp_name, nodes->sfp_props);
> +	nodes->sfp_ref[0] = SOFTWARE_NODE_REFERENCE(&swnodes[SWNODE_SFP]);
> +
> +	nodes->phylink_props[0] = PROPERTY_ENTRY_STRING("managed", "in-band-status");
> +	nodes->phylink_props[1] = PROPERTY_ENTRY_REF_ARRAY("sfp", nodes->sfp_ref);
> +	swnodes[SWNODE_PHYLINK] = NODE_PROP(nodes->phylink_name, nodes->phylink_props);
> +
> +	nodes->group[SWNODE_GPIO] = &swnodes[SWNODE_GPIO];
> +	nodes->group[SWNODE_I2C] = &swnodes[SWNODE_I2C];
> +	nodes->group[SWNODE_SFP] = &swnodes[SWNODE_SFP];
> +	nodes->group[SWNODE_PHYLINK] = &swnodes[SWNODE_PHYLINK];
> +
> +	return software_node_register_node_group(nodes->group);
> +}
> +
> +int txgbe_init_phy(struct txgbe *txgbe)
> +{
> +	int ret;
> +
> +	ret = txgbe_swnodes_register(txgbe);
> +	if (ret) {
> +		wx_err(txgbe->wx, "failed to register software nodes\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void txgbe_remove_phy(struct txgbe *txgbe)
> +{
> +	software_node_unregister_node_group(txgbe->nodes.group);
> +}
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h
> new file mode 100644
> index 000000000000..1ab592124986
> --- /dev/null
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
> +
> +#ifndef _TXGBE_PHY_H_
> +#define _TXGBE_PHY_H_
> +
> +int txgbe_init_phy(struct txgbe *txgbe);
> +void txgbe_remove_phy(struct txgbe *txgbe);
> +
> +#endif /* _TXGBE_NODE_H_ */
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> index 63a1c733718d..5bef0f9df523 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> @@ -4,6 +4,8 @@
>  #ifndef _TXGBE_TYPE_H_
>  #define _TXGBE_TYPE_H_
>  
> +#include <linux/property.h>
> +
>  /* Device IDs */
>  #define TXGBE_DEV_ID_SP1000                     0x1001
>  #define TXGBE_DEV_ID_WX1820                     0x2001
> @@ -99,4 +101,51 @@
>  
>  extern char txgbe_driver_name[];
>  
> +static inline struct txgbe *netdev_to_txgbe(struct net_device *netdev)
> +{
> +	struct wx *wx = netdev_priv(netdev);
> +
> +	return wx->priv;
> +}
> +
> +#define NODE_PROP(_NAME, _PROP)			\
> +	(const struct software_node) {		\
> +		.name = _NAME,			\
> +		.properties = _PROP,		\
> +	}
> +
> +enum txgbe_swnodes {
> +	SWNODE_GPIO = 0,
> +	SWNODE_I2C,
> +	SWNODE_SFP,
> +	SWNODE_PHYLINK,
> +	SWNODE_MAX
> +};
> +
> +struct txgbe_nodes {
> +	char gpio_name[32];
> +	char i2c_name[32];
> +	char sfp_name[32];
> +	char phylink_name[32];
> +	struct property_entry gpio_props[1];
> +	struct property_entry i2c_props[3];
> +	struct property_entry sfp_props[8];
> +	struct property_entry phylink_props[2];
> +	struct software_node_ref_args i2c_ref[1];
> +	struct software_node_ref_args gpio0_ref[1];
> +	struct software_node_ref_args gpio1_ref[1];
> +	struct software_node_ref_args gpio2_ref[1];
> +	struct software_node_ref_args gpio3_ref[1];
> +	struct software_node_ref_args gpio4_ref[1];
> +	struct software_node_ref_args gpio5_ref[1];
> +	struct software_node_ref_args sfp_ref[1];
> +	struct software_node swnodes[SWNODE_MAX];
> +	const struct software_node *group[SWNODE_MAX + 1];
> +};
> +
> +struct txgbe {
> +	struct wx *wx;
> +	struct txgbe_nodes nodes;
> +};
> +
>  #endif /* _TXGBE_TYPE_H_ */
> -- 
> 2.27.0
> 
> 

[PATCH net-next v7 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC

[Jiawen Wu]

Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
with SFP.

Introduce the property "snps,i2c-platform" to match device data for Wangxun
in software node case. Since IO resource was mapped on the ethernet driver,
add a model quirk to get regmap from parent device.

The exists IP limitations are dealt as workarounds:
- IP does not support interrupt mode, it works on polling mode.
- Additionally set FIFO depth address the chip issue.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/i2c/busses/i2c-designware-common.c  |  8 ++
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-master.c  | 89 +++++++++++++++++++--
 drivers/i2c/busses/i2c-designware-platdrv.c | 15 ++++
 4 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 0dc6b1ce663f..a7c2e67ccbf6 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -575,6 +575,14 @@ int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 	unsigned int param;
 	int ret;
 
+	/* DW_IC_COMP_PARAM_1 not implement for IP issue */
+	if ((dev->flags & MODEL_MASK) == MODEL_WANGXUN_SP) {
+		dev->tx_fifo_depth = 4;
+		dev->rx_fifo_depth = 0;
+
+		return 0;
+	}
+
 	/*
 	 * Try to detect the FIFO depth if not set by interface driver,
 	 * the depth could be from 2 to 256 from HW spec.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index c5d87aae39c6..e2213b08d724 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -303,6 +303,7 @@ struct dw_i2c_dev {
 #define MODEL_MSCC_OCELOT			BIT(8)
 #define MODEL_BAIKAL_BT1			BIT(9)
 #define MODEL_AMD_NAVI_GPU			BIT(10)
+#define MODEL_WANGXUN_SP			BIT(11)
 #define MODEL_MASK				GENMASK(11, 8)
 
 /*
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 55ea91a63382..3bfd7a2232db 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -354,6 +354,68 @@ static int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	return 0;
 }
 
+static int i2c_dw_poll_tx_empty(struct dw_i2c_dev *dev)
+{
+	u32 val;
+
+	return regmap_read_poll_timeout(dev->map, DW_IC_RAW_INTR_STAT, val,
+					val & DW_IC_INTR_TX_EMPTY,
+					100, 1000);
+}
+
+static int i2c_dw_poll_rx_full(struct dw_i2c_dev *dev)
+{
+	u32 val;
+
+	return regmap_read_poll_timeout(dev->map, DW_IC_RAW_INTR_STAT, val,
+					val & DW_IC_INTR_RX_FULL,
+					100, 1000);
+}
+
+static int txgbe_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
+				   int num_msgs)
+{
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+	int msg_idx, buf_len, data_idx, ret;
+	unsigned int val, stop = 0;
+	u8 *buf;
+
+	dev->msgs = msgs;
+	dev->msgs_num = num_msgs;
+	i2c_dw_xfer_init(dev);
+	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
+
+	for (msg_idx = 0; msg_idx < num_msgs; msg_idx++) {
+		buf = msgs[msg_idx].buf;
+		buf_len = msgs[msg_idx].len;
+
+		for (data_idx = 0; data_idx < buf_len; data_idx++) {
+			if (msg_idx == num_msgs - 1 && data_idx == buf_len - 1)
+				stop |= BIT(9);
+
+			if (msgs[msg_idx].flags & I2C_M_RD) {
+				regmap_write(dev->map, DW_IC_DATA_CMD, 0x100 | stop);
+
+				ret = i2c_dw_poll_rx_full(dev);
+				if (ret)
+					return ret;
+
+				regmap_read(dev->map, DW_IC_DATA_CMD, &val);
+				buf[data_idx] = val;
+			} else {
+				ret = i2c_dw_poll_tx_empty(dev);
+				if (ret)
+					return ret;
+
+				regmap_write(dev->map, DW_IC_DATA_CMD,
+					     buf[data_idx] | stop);
+			}
+		}
+	}
+
+	return num_msgs;
+}
+
 /*
  * Initiate (and continue) low level master read/write transaction.
  * This function is only called from i2c_dw_isr, and pumping i2c_msg
@@ -559,13 +621,19 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	pm_runtime_get_sync(dev->dev);
 
 	/*
-	 * Initiate I2C message transfer when AMD NAVI GPU card is enabled,
+	 * Initiate I2C message transfer when polling mode is enabled,
 	 * As it is polling based transfer mechanism, which does not support
 	 * interrupt based functionalities of existing DesignWare driver.
 	 */
-	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
+	switch (dev->flags & MODEL_MASK) {
+	case MODEL_AMD_NAVI_GPU:
 		ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
 		goto done_nolock;
+	case MODEL_WANGXUN_SP:
+		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
+		goto done_nolock;
+	default:
+		break;
 	}
 
 	reinit_completion(&dev->cmd_complete);
@@ -848,7 +916,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
 	return 0;
 }
 
-static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
+static int i2c_dw_poll_adap_quirk(struct dw_i2c_dev *dev)
 {
 	struct i2c_adapter *adap = &dev->adapter;
 	int ret;
@@ -862,6 +930,17 @@ static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
 	return ret;
 }
 
+static bool i2c_dw_is_model_poll(struct dw_i2c_dev *dev)
+{
+	switch (dev->flags & MODEL_MASK) {
+	case MODEL_AMD_NAVI_GPU:
+	case MODEL_WANGXUN_SP:
+		return true;
+	default:
+		return false;
+	}
+}
+
 int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 {
 	struct i2c_adapter *adap = &dev->adapter;
@@ -917,8 +996,8 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 	adap->dev.parent = dev->dev;
 	i2c_set_adapdata(adap, dev);
 
-	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
-		return amd_i2c_adap_quirk(dev);
+	if (i2c_dw_is_model_poll(dev))
+		return i2c_dw_poll_adap_quirk(dev);
 
 	if (dev->flags & ACCESS_NO_IRQ_SUSPEND) {
 		irq_flags = IRQF_NO_SUSPEND;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 89ad88c54754..1bf50150386d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -168,6 +168,15 @@ static inline int dw_i2c_of_configure(struct platform_device *pdev)
 }
 #endif
 
+static int txgbe_i2c_request_regs(struct dw_i2c_dev *dev)
+{
+	dev->map = dev_get_regmap(dev->dev->parent, NULL);
+	if (!dev->map)
+		return -ENODEV;
+
+	return 0;
+}
+
 static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 {
 	pm_runtime_disable(dev->dev);
@@ -185,6 +194,9 @@ static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
 	case MODEL_BAIKAL_BT1:
 		ret = bt1_i2c_request_regs(dev);
 		break;
+	case MODEL_WANGXUN_SP:
+		ret = txgbe_i2c_request_regs(dev);
+		break;
 	default:
 		dev->base = devm_platform_ioremap_resource(pdev, 0);
 		ret = PTR_ERR_OR_ZERO(dev->base);
@@ -277,6 +289,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
+	if (device_property_present(&pdev->dev, "snps,i2c-platform"))
+		dev->flags |= MODEL_WANGXUN_SP;
+
 	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
-- 
2.27.0

Re: [PATCH net-next v7 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC

[Piotr Raczynski]

On Tue, May 09, 2023 at 10:27:27AM +0800, Jiawen Wu wrote:
> Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> with SFP.
> 
> Introduce the property "snps,i2c-platform" to match device data for Wangxun
> in software node case. Since IO resource was mapped on the ethernet driver,
> add a model quirk to get regmap from parent device.
> 
> The exists IP limitations are dealt as workarounds:
> - IP does not support interrupt mode, it works on polling mode.
> - Additionally set FIFO depth address the chip issue.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

I'm definitely not an i2c expert, a couple of nit picks below, thanks.
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>

> ---
>  drivers/i2c/busses/i2c-designware-common.c  |  8 ++
>  drivers/i2c/busses/i2c-designware-core.h    |  1 +
>  drivers/i2c/busses/i2c-designware-master.c  | 89 +++++++++++++++++++--
>  drivers/i2c/busses/i2c-designware-platdrv.c | 15 ++++
>  4 files changed, 108 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index 0dc6b1ce663f..a7c2e67ccbf6 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -575,6 +575,14 @@ int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
>  	unsigned int param;
>  	int ret;
>  
> +	/* DW_IC_COMP_PARAM_1 not implement for IP issue */
> +	if ((dev->flags & MODEL_MASK) == MODEL_WANGXUN_SP) {
> +		dev->tx_fifo_depth = 4;
I understand this is some kind of workaround but is the number chosen
empirically? Maybe a defined value would be clearer instead of magic
number.
> +		dev->rx_fifo_depth = 0;
> +
> +		return 0;
> +	}
> +
>  	/*
>  	 * Try to detect the FIFO depth if not set by interface driver,
>  	 * the depth could be from 2 to 256 from HW spec.
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index c5d87aae39c6..e2213b08d724 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -303,6 +303,7 @@ struct dw_i2c_dev {
>  #define MODEL_MSCC_OCELOT			BIT(8)
>  #define MODEL_BAIKAL_BT1			BIT(9)
>  #define MODEL_AMD_NAVI_GPU			BIT(10)
> +#define MODEL_WANGXUN_SP			BIT(11)
>  #define MODEL_MASK				GENMASK(11, 8)
>  
>  /*
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index 55ea91a63382..3bfd7a2232db 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -354,6 +354,68 @@ static int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  	return 0;
>  }
>  
> +static int i2c_dw_poll_tx_empty(struct dw_i2c_dev *dev)
> +{
> +	u32 val;
> +
> +	return regmap_read_poll_timeout(dev->map, DW_IC_RAW_INTR_STAT, val,
> +					val & DW_IC_INTR_TX_EMPTY,
> +					100, 1000);
> +}
> +
> +static int i2c_dw_poll_rx_full(struct dw_i2c_dev *dev)
> +{
> +	u32 val;
> +
> +	return regmap_read_poll_timeout(dev->map, DW_IC_RAW_INTR_STAT, val,
> +					val & DW_IC_INTR_RX_FULL,
> +					100, 1000);
> +}
> +
> +static int txgbe_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +				   int num_msgs)
> +{
> +	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
> +	int msg_idx, buf_len, data_idx, ret;
> +	unsigned int val, stop = 0;
> +	u8 *buf;
> +
> +	dev->msgs = msgs;
> +	dev->msgs_num = num_msgs;
> +	i2c_dw_xfer_init(dev);
> +	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
> +
> +	for (msg_idx = 0; msg_idx < num_msgs; msg_idx++) {
> +		buf = msgs[msg_idx].buf;
> +		buf_len = msgs[msg_idx].len;
> +
> +		for (data_idx = 0; data_idx < buf_len; data_idx++) {
> +			if (msg_idx == num_msgs - 1 && data_idx == buf_len - 1)
> +				stop |= BIT(9);
> +
> +			if (msgs[msg_idx].flags & I2C_M_RD) {
> +				regmap_write(dev->map, DW_IC_DATA_CMD, 0x100 | stop);
> +
> +				ret = i2c_dw_poll_rx_full(dev);
> +				if (ret)
> +					return ret;
> +
> +				regmap_read(dev->map, DW_IC_DATA_CMD, &val);
> +				buf[data_idx] = val;
> +			} else {
> +				ret = i2c_dw_poll_tx_empty(dev);
> +				if (ret)
> +					return ret;
> +
> +				regmap_write(dev->map, DW_IC_DATA_CMD,
> +					     buf[data_idx] | stop);
> +			}
> +		}
> +	}
> +
> +	return num_msgs;
> +}
> +
>  /*
>   * Initiate (and continue) low level master read/write transaction.
>   * This function is only called from i2c_dw_isr, and pumping i2c_msg
> @@ -559,13 +621,19 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  	pm_runtime_get_sync(dev->dev);
>  
>  	/*
> -	 * Initiate I2C message transfer when AMD NAVI GPU card is enabled,
> +	 * Initiate I2C message transfer when polling mode is enabled,
>  	 * As it is polling based transfer mechanism, which does not support
>  	 * interrupt based functionalities of existing DesignWare driver.
>  	 */
> -	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
> +	switch (dev->flags & MODEL_MASK) {
> +	case MODEL_AMD_NAVI_GPU:
>  		ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
>  		goto done_nolock;
> +	case MODEL_WANGXUN_SP:
> +		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> +		goto done_nolock;
> +	default:
> +		break;
>  	}
Nit pick, when I first saw above code it looked a little weird, maybe it would be a
little clearer with:

	if (i2c_dw_is_model_poll(dev)) {
		switch (dev->flags & MODEL_MASK) {
		case MODEL_AMD_NAVI_GPU:
			ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
			break;
		case MODEL_WANGXUN_SP:
			ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
			break;
		default:
			break;
		}
		goto done_nolock;
	}

I do not insist though.

>  
>  	reinit_completion(&dev->cmd_complete);
> @@ -848,7 +916,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
>  	return 0;
>  }
>  
> -static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
> +static int i2c_dw_poll_adap_quirk(struct dw_i2c_dev *dev)
>  {
>  	struct i2c_adapter *adap = &dev->adapter;
>  	int ret;
> @@ -862,6 +930,17 @@ static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
>  	return ret;
>  }
>  
> +static bool i2c_dw_is_model_poll(struct dw_i2c_dev *dev)
> +{
> +	switch (dev->flags & MODEL_MASK) {
> +	case MODEL_AMD_NAVI_GPU:
> +	case MODEL_WANGXUN_SP:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  int i2c_dw_probe_master(struct dw_i2c_dev *dev)
>  {
>  	struct i2c_adapter *adap = &dev->adapter;
> @@ -917,8 +996,8 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
>  	adap->dev.parent = dev->dev;
>  	i2c_set_adapdata(adap, dev);
>  
> -	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
> -		return amd_i2c_adap_quirk(dev);
> +	if (i2c_dw_is_model_poll(dev))
> +		return i2c_dw_poll_adap_quirk(dev);
>  
>  	if (dev->flags & ACCESS_NO_IRQ_SUSPEND) {
>  		irq_flags = IRQF_NO_SUSPEND;
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 89ad88c54754..1bf50150386d 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -168,6 +168,15 @@ static inline int dw_i2c_of_configure(struct platform_device *pdev)
>  }
>  #endif
>  
> +static int txgbe_i2c_request_regs(struct dw_i2c_dev *dev)
> +{
> +	dev->map = dev_get_regmap(dev->dev->parent, NULL);
> +	if (!dev->map)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
>  static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
>  {
>  	pm_runtime_disable(dev->dev);
> @@ -185,6 +194,9 @@ static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
>  	case MODEL_BAIKAL_BT1:
>  		ret = bt1_i2c_request_regs(dev);
>  		break;
> +	case MODEL_WANGXUN_SP:
> +		ret = txgbe_i2c_request_regs(dev);
> +		break;
>  	default:
>  		dev->base = devm_platform_ioremap_resource(pdev, 0);
>  		ret = PTR_ERR_OR_ZERO(dev->base);
> @@ -277,6 +289,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
> +	if (device_property_present(&pdev->dev, "snps,i2c-platform"))
> +		dev->flags |= MODEL_WANGXUN_SP;
> +
>  	dev->dev = &pdev->dev;
>  	dev->irq = irq;
>  	platform_set_drvdata(pdev, dev);
> -- 
> 2.27.0
> 
> 

RE: [PATCH net-next v7 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC

[Jiawen Wu]

On Tuesday, May 9, 2023 9:52 PM, Piotr Raczynski wrote:
> On Tue, May 09, 2023 at 10:27:27AM +0800, Jiawen Wu wrote:
> > Wangxun 10Gb ethernet chip is connected to Designware I2C, to
> > communicate with SFP.
> >
> > Introduce the property "snps,i2c-platform" to match device data for
> > Wangxun in software node case. Since IO resource was mapped on the
> > ethernet driver, add a model quirk to get regmap from parent device.
> >
> > The exists IP limitations are dealt as workarounds:
> > - IP does not support interrupt mode, it works on polling mode.
> > - Additionally set FIFO depth address the chip issue.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 
> I'm definitely not an i2c expert, a couple of nit picks below, thanks.
> Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>
> 
> > ---
> >  drivers/i2c/busses/i2c-designware-common.c  |  8 ++
> >  drivers/i2c/busses/i2c-designware-core.h    |  1 +
> >  drivers/i2c/busses/i2c-designware-master.c  | 89
> > +++++++++++++++++++--  drivers/i2c/busses/i2c-designware-platdrv.c |
> > 15 ++++
> >  4 files changed, 108 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-designware-common.c
> > b/drivers/i2c/busses/i2c-designware-common.c
> > index 0dc6b1ce663f..a7c2e67ccbf6 100644
> > --- a/drivers/i2c/busses/i2c-designware-common.c
> > +++ b/drivers/i2c/busses/i2c-designware-common.c
> > @@ -575,6 +575,14 @@ int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
> >  	unsigned int param;
> >  	int ret;
> >
> > +	/* DW_IC_COMP_PARAM_1 not implement for IP issue */
> > +	if ((dev->flags & MODEL_MASK) == MODEL_WANGXUN_SP) {
> > +		dev->tx_fifo_depth = 4;
> I understand this is some kind of workaround but is the number chosen
> empirically? Maybe a defined value would be clearer instead of magic
> number.

Yes, this value setting worked and passed test.

> > @@ -559,13 +621,19 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> >  	pm_runtime_get_sync(dev->dev);
> >
> >  	/*
> > -	 * Initiate I2C message transfer when AMD NAVI GPU card is enabled,
> > +	 * Initiate I2C message transfer when polling mode is enabled,
> >  	 * As it is polling based transfer mechanism, which does not support
> >  	 * interrupt based functionalities of existing DesignWare driver.
> >  	 */
> > -	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
> > +	switch (dev->flags & MODEL_MASK) {
> > +	case MODEL_AMD_NAVI_GPU:
> >  		ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
> >  		goto done_nolock;
> > +	case MODEL_WANGXUN_SP:
> > +		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> > +		goto done_nolock;
> > +	default:
> > +		break;
> >  	}
> Nit pick, when I first saw above code it looked a little weird,
> maybe it would be a little clearer with:
> 
> 	if (i2c_dw_is_model_poll(dev)) {
> 		switch (dev->flags & MODEL_MASK) {
> 		case MODEL_AMD_NAVI_GPU:
> 			ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
> 			break;
> 		case MODEL_WANGXUN_SP:
> 			ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> 			break;
> 		default:
> 			break;
> 		}
> 		goto done_nolock;
> 	}
> 
> I do not insist though.

Sure, it looks more obvious as polling mode quirk.
 

Re: [PATCH net-next v7 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC

[andy.shevchenko]

Wed, May 10, 2023 at 02:43:50PM +0800, Jiawen Wu kirjoitti:
> On Tuesday, May 9, 2023 9:52 PM, Piotr Raczynski wrote:
> > On Tue, May 09, 2023 at 10:27:27AM +0800, Jiawen Wu wrote:

...

> > >  	/*
> > > -	 * Initiate I2C message transfer when AMD NAVI GPU card is enabled,
> > > +	 * Initiate I2C message transfer when polling mode is enabled,
> > >  	 * As it is polling based transfer mechanism, which does not support
> > >  	 * interrupt based functionalities of existing DesignWare driver.
> > >  	 */
> > > -	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
> > > +	switch (dev->flags & MODEL_MASK) {
> > > +	case MODEL_AMD_NAVI_GPU:
> > >  		ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
> > >  		goto done_nolock;
> > > +	case MODEL_WANGXUN_SP:
> > > +		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> > > +		goto done_nolock;
> > > +	default:
> > > +		break;
> > >  	}
> > Nit pick, when I first saw above code it looked a little weird,
> > maybe it would be a little clearer with:
> > 
> > 	if (i2c_dw_is_model_poll(dev)) {
> > 		switch (dev->flags & MODEL_MASK) {
> > 		case MODEL_AMD_NAVI_GPU:
> > 			ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
> > 			break;
> > 		case MODEL_WANGXUN_SP:
> > 			ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> > 			break;
> > 		default:
> > 			break;
> > 		}
> > 		goto done_nolock;
> > 	}
> > 
> > I do not insist though.
> 
> Sure, it looks more obvious as polling mode quirk.

I don't think we need a double checks. The i2c_dw_is_model_poll() will repeat
the switch. Please, leave it as is in your current version.


-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH net-next v7 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC

[Jiawen Wu]

On Wednesday, May 10, 2023 3:48 PM, andy.shevchenko@gmail.com wrote:
> Wed, May 10, 2023 at 02:43:50PM +0800, Jiawen Wu kirjoitti:
> > On Tuesday, May 9, 2023 9:52 PM, Piotr Raczynski wrote:
> > > On Tue, May 09, 2023 at 10:27:27AM +0800, Jiawen Wu wrote:
> 
> ...
> 
> > > >  	/*
> > > > -	 * Initiate I2C message transfer when AMD NAVI GPU card is enabled,
> > > > +  * Initiate I2C message transfer when polling mode is enabled,
> > > >  	 * As it is polling based transfer mechanism, which does not support
> > > >  	 * interrupt based functionalities of existing DesignWare driver.
> > > >  	 */
> > > > -	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
> > > > +	switch (dev->flags & MODEL_MASK) {
> > > > +	case MODEL_AMD_NAVI_GPU:
> > > >  		ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
> > > >  		goto done_nolock;
> > > > +	case MODEL_WANGXUN_SP:
> > > > +		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> > > > +		goto done_nolock;
> > > > +	default:
> > > > +		break;
> > > >  	}
> > > Nit pick, when I first saw above code it looked a little weird,
> > > maybe it would be a little clearer with:
> > >
> > > 	if (i2c_dw_is_model_poll(dev)) {
> > > 		switch (dev->flags & MODEL_MASK) {
> > > 		case MODEL_AMD_NAVI_GPU:
> > > 			ret = amd_i2c_dw_xfer_quirk(adap, msgs, num);
> > > 			break;
> > > 		case MODEL_WANGXUN_SP:
> > > 			ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
> > > 			break;
> > > 		default:
> > > 			break;
> > > 		}
> > > 		goto done_nolock;
> > > 	}
> > >
> > > I do not insist though.
> >
> > Sure, it looks more obvious as polling mode quirk.
> 
> I don't think we need a double checks. The i2c_dw_is_model_poll() will repeat
> the switch. Please, leave it as is in your current version.

Okay, thanks for all your comments.

[PATCH net-next v7 3/9] net: txgbe: Register fixed rate clock

[Jiawen Wu]

In order for I2C to be able to work in standard mode, register a fixed
rate clock for each I2C device.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |  1 +
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 41 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  2 +
 3 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index c9d88673d306..a62614eeed2e 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -40,6 +40,7 @@ config NGBE
 config TXGBE
 	tristate "Wangxun(R) 10GbE PCI Express adapters support"
 	depends on PCI
+	select COMMON_CLK
 	select LIBWX
 	help
 	  This driver supports Wangxun(R) 10GbE PCI Express family of
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 3476074869cb..052354b650a5 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -2,6 +2,8 @@
 /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
 
 #include <linux/gpio/property.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
 #include <linux/i2c.h>
 #include <linux/pci.h>
 
@@ -70,6 +72,32 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
 	return software_node_register_node_group(nodes->group);
 }
 
+static int txgbe_clock_register(struct txgbe *txgbe)
+{
+	struct pci_dev *pdev = txgbe->wx->pdev;
+	struct clk_lookup *clock;
+	char clk_name[32];
+	struct clk *clk;
+
+	snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
+		 (pdev->bus->number << 8) | pdev->devfn);
+
+	clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	clock = clkdev_create(clk, NULL, clk_name);
+	if (!clock) {
+		clk_unregister(clk);
+		return PTR_ERR(clock);
+	}
+
+	txgbe->clk = clk;
+	txgbe->clock = clock;
+
+	return 0;
+}
+
 int txgbe_init_phy(struct txgbe *txgbe)
 {
 	int ret;
@@ -80,10 +108,23 @@ int txgbe_init_phy(struct txgbe *txgbe)
 		return ret;
 	}
 
+	ret = txgbe_clock_register(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
+		goto err_unregister_swnode;
+	}
+
 	return 0;
+
+err_unregister_swnode:
+	software_node_unregister_node_group(txgbe->nodes.group);
+
+	return ret;
 }
 
 void txgbe_remove_phy(struct txgbe *txgbe)
 {
+	clkdev_drop(txgbe->clock);
+	clk_unregister(txgbe->clk);
 	software_node_unregister_node_group(txgbe->nodes.group);
 }
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 5bef0f9df523..cdbc4b37f832 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -146,6 +146,8 @@ struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	struct clk_lookup *clock;
+	struct clk *clk;
 };
 
 #endif /* _TXGBE_TYPE_H_ */
-- 
2.27.0

Re: [PATCH net-next v7 3/9] net: txgbe: Register fixed rate clock

[Simon Horman]

On Tue, May 09, 2023 at 10:27:28AM +0800, Jiawen Wu wrote:
> In order for I2C to be able to work in standard mode, register a fixed
> rate clock for each I2C device.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

...

> @@ -70,6 +72,32 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
>  	return software_node_register_node_group(nodes->group);
>  }
>  
> +static int txgbe_clock_register(struct txgbe *txgbe)
> +{
> +	struct pci_dev *pdev = txgbe->wx->pdev;
> +	struct clk_lookup *clock;
> +	char clk_name[32];
> +	struct clk *clk;
> +
> +	snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
> +		 (pdev->bus->number << 8) | pdev->devfn);
> +
> +	clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	clock = clkdev_create(clk, NULL, clk_name);
> +	if (!clock) {
> +		clk_unregister(clk);
> +		return PTR_ERR(clock);

Hi Jiawen,

Sorry for missing this earlier, but the above error handling doesn't seem
right.

   * This error condition is met if clock == NULL
   * So the above is returning PTR_ERR(NULL), which is a yellow flag to me.
     In any case, PTR_ERR(NULL) => 0 is returned on error.
   * The caller treats a 0 return value as success.

   Perhaps this should be: return -ENOMEM?

> +	}
> +
> +	txgbe->clk = clk;
> +	txgbe->clock = clock;
> +
> +	return 0;
> +}
> +
>  int txgbe_init_phy(struct txgbe *txgbe)
>  {
>  	int ret;
> @@ -80,10 +108,23 @@ int txgbe_init_phy(struct txgbe *txgbe)
>  		return ret;
>  	}
>  
> +	ret = txgbe_clock_register(txgbe);
> +	if (ret) {
> +		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
> +		goto err_unregister_swnode;
> +	}
> +
>  	return 0;

...

RE: [PATCH net-next v7 3/9] net: txgbe: Register fixed rate clock

[Jiawen Wu]

On Tuesday, May 9, 2023 11:32 PM, Simon Horman wrote:
> On Tue, May 09, 2023 at 10:27:28AM +0800, Jiawen Wu wrote:
> > In order for I2C to be able to work in standard mode, register a fixed
> > rate clock for each I2C device.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 
> ...
> 
> > @@ -70,6 +72,32 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
> >  	return software_node_register_node_group(nodes->group);
> >  }
> >
> > +static int txgbe_clock_register(struct txgbe *txgbe) {
> > +	struct pci_dev *pdev = txgbe->wx->pdev;
> > +	struct clk_lookup *clock;
> > +	char clk_name[32];
> > +	struct clk *clk;
> > +
> > +	snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
> > +		 (pdev->bus->number << 8) | pdev->devfn);
> > +
> > +	clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	clock = clkdev_create(clk, NULL, clk_name);
> > +	if (!clock) {
> > +		clk_unregister(clk);
> > +		return PTR_ERR(clock);
> 
> Hi Jiawen,
> 
> Sorry for missing this earlier, but the above error handling doesn't seem right.
> 
>    * This error condition is met if clock == NULL
>    * So the above is returning PTR_ERR(NULL), which is a yellow flag to me.
>      In any case, PTR_ERR(NULL) => 0 is returned on error.
>    * The caller treats a 0 return value as success.
> 
>    Perhaps this should be: return -ENOMEM?

No problem, I will fix it in patch v8.

> 
> > +	}
> > +
> > +	txgbe->clk = clk;
> > +	txgbe->clock = clock;
> > +
> > +	return 0;
> > +}
> > +
> >  int txgbe_init_phy(struct txgbe *txgbe)  {
> >  	int ret;
> > @@ -80,10 +108,23 @@ int txgbe_init_phy(struct txgbe *txgbe)
> >  		return ret;
> >  	}
> >
> > +	ret = txgbe_clock_register(txgbe);
> > +	if (ret) {
> > +		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
> > +		goto err_unregister_swnode;
> > +	}
> > +
> >  	return 0;
> 
> ...
> 

[PATCH net-next v7 4/9] net: txgbe: Register I2C platform device

[Jiawen Wu]

Register the platform device to use Designware I2C bus master driver.
Use regmap to read/write I2C device region from given base offset.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |  2 +
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 71 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  4 ++
 3 files changed, 77 insertions(+)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index a62614eeed2e..ec058a72afb6 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -40,6 +40,8 @@ config NGBE
 config TXGBE
 	tristate "Wangxun(R) 10GbE PCI Express adapters support"
 	depends on PCI
+	select I2C_DESIGNWARE_PLATFORM
+	select REGMAP
 	select COMMON_CLK
 	select LIBWX
 	help
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 052354b650a5..7da497ebc4a8 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -1,7 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
 
+#include <linux/platform_device.h>
 #include <linux/gpio/property.h>
+#include <linux/regmap.h>
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/i2c.h>
@@ -98,6 +100,65 @@ static int txgbe_clock_register(struct txgbe *txgbe)
 	return 0;
 }
 
+static int txgbe_i2c_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct wx *wx = context;
+
+	*val = rd32(wx, reg + TXGBE_I2C_BASE);
+
+	return 0;
+}
+
+static int txgbe_i2c_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct wx *wx = context;
+
+	wr32(wx, reg + TXGBE_I2C_BASE, val);
+
+	return 0;
+}
+
+static const struct regmap_config i2c_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_read = txgbe_i2c_read,
+	.reg_write = txgbe_i2c_write,
+};
+
+static int txgbe_i2c_register(struct txgbe *txgbe)
+{
+	struct platform_device_info info = {};
+	struct platform_device *i2c_dev;
+	struct wx *wx = txgbe->wx;
+	struct regmap *i2c_regmap;
+	struct resource res = {};
+	struct pci_dev *pdev;
+
+	pdev = wx->pdev;
+	i2c_regmap = devm_regmap_init(&pdev->dev, NULL, wx,
+				      &i2c_regmap_config);
+	if (IS_ERR(i2c_regmap)) {
+		wx_err(wx, "failed to init regmap\n");
+		return PTR_ERR(i2c_regmap);
+	}
+
+	info.parent = &pdev->dev;
+	info.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_I2C]);
+	info.name = "i2c_designware";
+	info.id = (pdev->bus->number << 8) | pdev->devfn;
+
+	res = DEFINE_RES_IRQ(pdev->irq);
+	info.res = &res;
+	info.num_res = 1;
+	i2c_dev = platform_device_register_full(&info);
+	if (IS_ERR(i2c_dev))
+		return PTR_ERR(i2c_dev);
+
+	txgbe->i2c_dev = i2c_dev;
+
+	return 0;
+}
+
 int txgbe_init_phy(struct txgbe *txgbe)
 {
 	int ret;
@@ -114,8 +175,17 @@ int txgbe_init_phy(struct txgbe *txgbe)
 		goto err_unregister_swnode;
 	}
 
+	ret = txgbe_i2c_register(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to init i2c interface: %d\n", ret);
+		goto err_unregister_clk;
+	}
+
 	return 0;
 
+err_unregister_clk:
+	clkdev_drop(txgbe->clock);
+	clk_unregister(txgbe->clk);
 err_unregister_swnode:
 	software_node_unregister_node_group(txgbe->nodes.group);
 
@@ -124,6 +194,7 @@ int txgbe_init_phy(struct txgbe *txgbe)
 
 void txgbe_remove_phy(struct txgbe *txgbe)
 {
+	platform_device_unregister(txgbe->i2c_dev);
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
 	software_node_unregister_node_group(txgbe->nodes.group);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index cdbc4b37f832..55979abf01f2 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -55,6 +55,9 @@
 #define TXGBE_TS_CTL                            0x10300
 #define TXGBE_TS_CTL_EVAL_MD                    BIT(31)
 
+/* I2C registers */
+#define TXGBE_I2C_BASE                          0x14900
+
 /* Part Number String Length */
 #define TXGBE_PBANUM_LENGTH                     32
 
@@ -146,6 +149,7 @@ struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
 	struct clk *clk;
 };
-- 
2.27.0

Re: [PATCH net-next v7 4/9] net: txgbe: Register I2C platform device

[Andrew Lunn]

On Tue, May 09, 2023 at 10:27:29AM +0800, Jiawen Wu wrote:
> Register the platform device to use Designware I2C bus master driver.
> Use regmap to read/write I2C device region from given base offset.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v7 4/9] net: txgbe: Register I2C platform device

[Piotr Raczynski]

On Tue, May 09, 2023 at 10:27:29AM +0800, Jiawen Wu wrote:
> Register the platform device to use Designware I2C bus master driver.
> Use regmap to read/write I2C device region from given base offset.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
LGTM, thanks.
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>

[PATCH net-next v7 5/9] net: txgbe: Add SFP module identify

[Jiawen Wu]

Register SFP platform device to get modules information.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |  1 +
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 28 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  1 +
 3 files changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index ec058a72afb6..90812d76181d 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -44,6 +44,7 @@ config TXGBE
 	select REGMAP
 	select COMMON_CLK
 	select LIBWX
+	select SFP
 	help
 	  This driver supports Wangxun(R) 10GbE PCI Express family of
 	  adapters.
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 7da497ebc4a8..23fa7311db45 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -159,6 +159,25 @@ static int txgbe_i2c_register(struct txgbe *txgbe)
 	return 0;
 }
 
+static int txgbe_sfp_register(struct txgbe *txgbe)
+{
+	struct pci_dev *pdev = txgbe->wx->pdev;
+	struct platform_device_info info = {};
+	struct platform_device *sfp_dev;
+
+	info.parent = &pdev->dev;
+	info.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_SFP]);
+	info.name = "sfp";
+	info.id = (pdev->bus->number << 8) | pdev->devfn;
+	sfp_dev = platform_device_register_full(&info);
+	if (IS_ERR(sfp_dev))
+		return PTR_ERR(sfp_dev);
+
+	txgbe->sfp_dev = sfp_dev;
+
+	return 0;
+}
+
 int txgbe_init_phy(struct txgbe *txgbe)
 {
 	int ret;
@@ -181,8 +200,16 @@ int txgbe_init_phy(struct txgbe *txgbe)
 		goto err_unregister_clk;
 	}
 
+	ret = txgbe_sfp_register(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to register sfp\n");
+		goto err_unregister_i2c;
+	}
+
 	return 0;
 
+err_unregister_i2c:
+	platform_device_unregister(txgbe->i2c_dev);
 err_unregister_clk:
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
@@ -194,6 +221,7 @@ int txgbe_init_phy(struct txgbe *txgbe)
 
 void txgbe_remove_phy(struct txgbe *txgbe)
 {
+	platform_device_unregister(txgbe->sfp_dev);
 	platform_device_unregister(txgbe->i2c_dev);
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 55979abf01f2..fc91e0fc37a6 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -149,6 +149,7 @@ struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	struct platform_device *sfp_dev;
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
 	struct clk *clk;
-- 
2.27.0

Re: [PATCH net-next v7 5/9] net: txgbe: Add SFP module identify

[Andrew Lunn]

On Tue, May 09, 2023 at 10:27:30AM +0800, Jiawen Wu wrote:
> Register SFP platform device to get modules information.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v7 5/9] net: txgbe: Add SFP module identify

[Piotr Raczynski]

On Tue, May 09, 2023 at 10:27:30AM +0800, Jiawen Wu wrote:
> Register SFP platform device to get modules information.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
Looks fine, thanks.
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>

[PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Jiawen Wu]

Register GPIO chip and handle GPIO IRQ for SFP socket.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |   2 +
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   |   3 +-
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |   2 +
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  20 +-
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 228 ++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  27 +++
 6 files changed, 263 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index 90812d76181d..73f4492928c0 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -41,6 +41,8 @@ config TXGBE
 	tristate "Wangxun(R) 10GbE PCI Express adapters support"
 	depends on PCI
 	select I2C_DESIGNWARE_PLATFORM
+	select GPIOLIB_IRQCHIP
+	select GPIOLIB
 	select REGMAP
 	select COMMON_CLK
 	select LIBWX
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index 1e8d8b7b0c62..590215303d45 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -1348,7 +1348,8 @@ void wx_free_irq(struct wx *wx)
 		free_irq(entry->vector, q_vector);
 	}
 
-	free_irq(wx->msix_entries[vector].vector, wx);
+	if (wx->mac.type == wx_mac_em)
+		free_irq(wx->msix_entries[vector].vector, wx);
 }
 EXPORT_SYMBOL(wx_free_irq);
 
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 97bce855bc60..d151d6f79022 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -79,7 +79,9 @@
 #define WX_GPIO_INTMASK              0x14834
 #define WX_GPIO_INTTYPE_LEVEL        0x14838
 #define WX_GPIO_POLARITY             0x1483C
+#define WX_GPIO_INTSTATUS            0x14844
 #define WX_GPIO_EOI                  0x1484C
+#define WX_GPIO_EXT                  0x14850
 
 /*********************** Transmit DMA registers **************************/
 /* transmit global control */
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index e10296abf5b4..ded04e9e136f 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -82,6 +82,8 @@ static int txgbe_enumerate_functions(struct wx *wx)
  **/
 static void txgbe_irq_enable(struct wx *wx, bool queues)
 {
+	wr32(wx, WX_PX_MISC_IEN, TXGBE_PX_MISC_IEN_MASK);
+
 	/* unmask interrupt */
 	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
 	if (queues)
@@ -129,17 +131,6 @@ static irqreturn_t txgbe_intr(int __always_unused irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t txgbe_msix_other(int __always_unused irq, void *data)
-{
-	struct wx *wx = data;
-
-	/* re-enable the original interrupt state */
-	if (netif_running(wx->netdev))
-		txgbe_irq_enable(wx, false);
-
-	return IRQ_HANDLED;
-}
-
 /**
  * txgbe_request_msix_irqs - Initialize MSI-X interrupts
  * @wx: board private structure
@@ -171,13 +162,6 @@ static int txgbe_request_msix_irqs(struct wx *wx)
 		}
 	}
 
-	err = request_irq(wx->msix_entries[vector].vector,
-			  txgbe_msix_other, 0, netdev->name, wx);
-	if (err) {
-		wx_err(wx, "request_irq for msix_other failed: %d\n", err);
-		goto free_queue_irqs;
-	}
-
 	return 0;
 
 free_queue_irqs:
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 23fa7311db45..8085616a9146 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -2,6 +2,9 @@
 /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
 
 #include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
+#include <linux/gpio/driver.h>
 #include <linux/gpio/property.h>
 #include <linux/regmap.h>
 #include <linux/clkdev.h>
@@ -10,6 +13,7 @@
 #include <linux/pci.h>
 
 #include "../libwx/wx_type.h"
+#include "../libwx/wx_hw.h"
 #include "txgbe_type.h"
 #include "txgbe_phy.h"
 
@@ -74,6 +78,224 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
 	return software_node_register_node_group(nodes->group);
 }
 
+static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct wx *wx = gpiochip_get_data(chip);
+	struct txgbe *txgbe = wx->priv;
+	int val;
+
+	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
+
+	txgbe->gpio_orig &= ~BIT(offset);
+	txgbe->gpio_orig |= val;
+
+	return !!(val & BIT(offset));
+}
+
+static int txgbe_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct wx *wx = gpiochip_get_data(chip);
+	u32 val;
+
+	val = rd32(wx, WX_GPIO_DDR);
+	if (BIT(offset) & val)
+		return GPIO_LINE_DIRECTION_OUT;
+
+	return GPIO_LINE_DIRECTION_IN;
+}
+
+static int txgbe_gpio_direction_in(struct gpio_chip *chip, unsigned int offset)
+{
+	struct wx *wx = gpiochip_get_data(chip);
+
+	wr32m(wx, WX_GPIO_DDR, BIT(offset), 0);
+
+	return 0;
+}
+
+static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
+				    int val)
+{
+	struct wx *wx = gpiochip_get_data(chip);
+	u32 mask;
+
+	mask = BIT(offset) | BIT(offset - 1);
+	if (val)
+		wr32m(wx, WX_GPIO_DR, mask, mask);
+	else
+		wr32m(wx, WX_GPIO_DR, mask, 0);
+
+	wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));
+
+	return 0;
+}
+
+static void txgbe_gpio_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	struct wx *wx = gpiochip_get_data(gc);
+
+	wr32(wx, WX_GPIO_EOI, BIT(hwirq));
+}
+
+static void txgbe_gpio_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	struct wx *wx = gpiochip_get_data(gc);
+
+	gpiochip_disable_irq(gc, hwirq);
+
+	wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), BIT(hwirq));
+}
+
+static void txgbe_gpio_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	struct wx *wx = gpiochip_get_data(gc);
+
+	gpiochip_enable_irq(gc, hwirq);
+
+	wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), 0);
+}
+
+static int txgbe_gpio_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	struct wx *wx = gpiochip_get_data(gc);
+	u32 level, polarity;
+
+	level = rd32(wx, WX_GPIO_INTTYPE_LEVEL);
+	polarity = rd32(wx, WX_GPIO_POLARITY);
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_BOTH:
+		level |= BIT(hwirq);
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		level |= BIT(hwirq);
+		polarity |= BIT(hwirq);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		level |= BIT(hwirq);
+		polarity &= ~BIT(hwirq);
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		level &= ~BIT(hwirq);
+		polarity |= BIT(hwirq);
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		level &= ~BIT(hwirq);
+		polarity &= ~BIT(hwirq);
+		break;
+	}
+
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(d, handle_level_irq);
+	else if (type & IRQ_TYPE_EDGE_BOTH)
+		irq_set_handler_locked(d, handle_edge_irq);
+
+	wr32m(wx, WX_GPIO_INTEN, BIT(hwirq), BIT(hwirq));
+	wr32(wx, WX_GPIO_INTTYPE_LEVEL, level);
+	if (type != IRQ_TYPE_EDGE_BOTH)
+		wr32(wx, WX_GPIO_POLARITY, polarity);
+
+	return 0;
+}
+
+static const struct irq_chip txgbe_gpio_irq_chip = {
+	.name = "txgbe_gpio_irq",
+	.irq_ack = txgbe_gpio_irq_ack,
+	.irq_mask = txgbe_gpio_irq_mask,
+	.irq_unmask = txgbe_gpio_irq_unmask,
+	.irq_set_type = txgbe_gpio_set_type,
+	.flags = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static void txgbe_irq_handler(struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct wx *wx = irq_desc_get_handler_data(desc);
+	struct txgbe *txgbe = wx->priv;
+	irq_hw_number_t hwirq;
+	unsigned long gpioirq;
+	struct gpio_chip *gc;
+	u32 gpio;
+
+	chained_irq_enter(chip, desc);
+
+	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
+
+	/* workaround for hysteretic gpio interrupts */
+	gpio = rd32(wx, WX_GPIO_EXT);
+	if (!gpioirq)
+		gpioirq = txgbe->gpio_orig ^ gpio;
+
+	gc = txgbe->gpio;
+	for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
+		generic_handle_domain_irq(gc->irq.domain, hwirq);
+
+	chained_irq_exit(chip, desc);
+
+	/* unmask interrupt */
+	if (netif_running(wx->netdev))
+		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
+}
+
+static int txgbe_gpio_init(struct txgbe *txgbe)
+{
+	struct gpio_irq_chip *girq;
+	struct wx *wx = txgbe->wx;
+	struct pci_dev *pdev;
+	struct gpio_chip *gc;
+	int ret;
+
+	pdev = wx->pdev;
+	txgbe->gpio_orig = 0;
+
+	gc = devm_kzalloc(&pdev->dev, sizeof(*gc), GFP_KERNEL);
+	if (!gc)
+		return -ENOMEM;
+
+	gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "txgbe_gpio-%x",
+				   (pdev->bus->number << 8) | pdev->devfn);
+	gc->base = -1;
+	gc->ngpio = 6;
+	gc->owner = THIS_MODULE;
+	gc->parent = &pdev->dev;
+	gc->fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_GPIO]);
+	gc->get = txgbe_gpio_get;
+	gc->get_direction = txgbe_gpio_get_direction;
+	gc->direction_input = txgbe_gpio_direction_in;
+	gc->direction_output = txgbe_gpio_direction_out;
+	gc->can_sleep = false;
+
+	girq = &gc->irq;
+	gpio_irq_chip_set_chip(girq, &txgbe_gpio_irq_chip);
+	girq->parent_handler = txgbe_irq_handler;
+	girq->parent_handler_data = wx;
+	girq->num_parents = 1;
+	girq->parents = devm_kcalloc(&pdev->dev, 1, sizeof(*girq->parents),
+				     GFP_KERNEL);
+	if (!girq->parents)
+		return -ENOMEM;
+	girq->parents[0] = wx->msix_entries[wx->num_q_vectors].vector;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_bad_irq;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, gc, wx);
+	if (ret)
+		return ret;
+
+	txgbe->gpio = gc;
+
+	return 0;
+}
+
 static int txgbe_clock_register(struct txgbe *txgbe)
 {
 	struct pci_dev *pdev = txgbe->wx->pdev;
@@ -188,6 +410,12 @@ int txgbe_init_phy(struct txgbe *txgbe)
 		return ret;
 	}
 
+	ret = txgbe_gpio_init(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to init gpio\n");
+		goto err_unregister_swnode;
+	}
+
 	ret = txgbe_clock_register(txgbe);
 	if (ret) {
 		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index fc91e0fc37a6..796f33fe3016 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -55,6 +55,31 @@
 #define TXGBE_TS_CTL                            0x10300
 #define TXGBE_TS_CTL_EVAL_MD                    BIT(31)
 
+/* GPIO register bit */
+#define TXGBE_GPIOBIT_0                         BIT(0) /* I:tx fault */
+#define TXGBE_GPIOBIT_1                         BIT(1) /* O:tx disabled */
+#define TXGBE_GPIOBIT_2                         BIT(2) /* I:sfp module absent */
+#define TXGBE_GPIOBIT_3                         BIT(3) /* I:rx signal lost */
+#define TXGBE_GPIOBIT_4                         BIT(4) /* O:rate select, 1G(0) 10G(1) */
+#define TXGBE_GPIOBIT_5                         BIT(5) /* O:rate select, 1G(0) 10G(1) */
+
+/* Extended Interrupt Enable Set */
+#define TXGBE_PX_MISC_ETH_LKDN                  BIT(8)
+#define TXGBE_PX_MISC_DEV_RST                   BIT(10)
+#define TXGBE_PX_MISC_ETH_EVENT                 BIT(17)
+#define TXGBE_PX_MISC_ETH_LK                    BIT(18)
+#define TXGBE_PX_MISC_ETH_AN                    BIT(19)
+#define TXGBE_PX_MISC_INT_ERR                   BIT(20)
+#define TXGBE_PX_MISC_GPIO                      BIT(26)
+#define TXGBE_PX_MISC_IEN_MASK ( \
+				TXGBE_PX_MISC_ETH_LKDN | \
+				TXGBE_PX_MISC_DEV_RST | \
+				TXGBE_PX_MISC_ETH_EVENT | \
+				TXGBE_PX_MISC_ETH_LK | \
+				TXGBE_PX_MISC_ETH_AN | \
+				TXGBE_PX_MISC_INT_ERR | \
+				TXGBE_PX_MISC_GPIO)
+
 /* I2C registers */
 #define TXGBE_I2C_BASE                          0x14900
 
@@ -153,6 +178,8 @@ struct txgbe {
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
 	struct clk *clk;
+	struct gpio_chip *gpio;
+	u32 gpio_orig;
 };
 
 #endif /* _TXGBE_TYPE_H_ */
-- 
2.27.0

Re: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Andrew Lunn]

> +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	struct txgbe *txgbe = wx->priv;
> +	int val;
> +
> +	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> +
> +	txgbe->gpio_orig &= ~BIT(offset);
> +	txgbe->gpio_orig |= val;
> +
> +	return !!(val & BIT(offset));
> +}

> +static void txgbe_irq_handler(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct wx *wx = irq_desc_get_handler_data(desc);
> +	struct txgbe *txgbe = wx->priv;
> +	irq_hw_number_t hwirq;
> +	unsigned long gpioirq;
> +	struct gpio_chip *gc;
> +	u32 gpio;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
> +
> +	/* workaround for hysteretic gpio interrupts */
> +	gpio = rd32(wx, WX_GPIO_EXT);
> +	if (!gpioirq)
> +		gpioirq = txgbe->gpio_orig ^ gpio;

Please could you expand on the comment. Are you saying that
WX_GPIO_INTSTATUS sometimes does not contain the GPIO which caused the
interrupt? If so, you then compare the last gpio_get with the current
value and assume that is what caused the interrupt?

> +
> +	gc = txgbe->gpio;
> +	for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
> +		generic_handle_domain_irq(gc->irq.domain, hwirq);
> +
> +	chained_irq_exit(chip, desc);
> +
> +	/* unmask interrupt */
> +	if (netif_running(wx->netdev))
> +		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));

Is that a hardware requirement, that interrupts only work when the
interface is running? Interrupts are not normally conditional like
this, at least when the SoC provides the GPIO controller. 

Andrew

---
pw-bot: cr

RE: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Jiawen Wu]

On Thursday, May 11, 2023 8:32 PM, Andrew Lunn wrote:
> > +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int
> > +offset) {
> > +	struct wx *wx = gpiochip_get_data(chip);
> > +	struct txgbe *txgbe = wx->priv;
> > +	int val;
> > +
> > +	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> > +
> > +	txgbe->gpio_orig &= ~BIT(offset);
> > +	txgbe->gpio_orig |= val;
> > +
> > +	return !!(val & BIT(offset));
> > +}
> 
> > +static void txgbe_irq_handler(struct irq_desc *desc) {
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	struct wx *wx = irq_desc_get_handler_data(desc);
> > +	struct txgbe *txgbe = wx->priv;
> > +	irq_hw_number_t hwirq;
> > +	unsigned long gpioirq;
> > +	struct gpio_chip *gc;
> > +	u32 gpio;
> > +
> > +	chained_irq_enter(chip, desc);
> > +
> > +	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
> > +
> > +	/* workaround for hysteretic gpio interrupts */
> > +	gpio = rd32(wx, WX_GPIO_EXT);
> > +	if (!gpioirq)
> > +		gpioirq = txgbe->gpio_orig ^ gpio;
> 
> Please could you expand on the comment. Are you saying that
> WX_GPIO_INTSTATUS sometimes does not contain the GPIO which caused the
> interrupt? If so, you then compare the last gpio_get with the current value and
> assume that is what caused the interrupt?

Yes. Sometime there is a lag in WX_GPIO_INTSTATUS. When the GPIO interrupt
cause,  the GPIO state has been back to its previous state. So I added this
workaround to save some...but only if there are other interrupts at the same
time, i.e. txgbe_irq_handler() called.

But I will remove it in the next version, because I find a more accurate solution.

> 
> > +
> > +	gc = txgbe->gpio;
> > +	for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
> > +		generic_handle_domain_irq(gc->irq.domain, hwirq);
> > +
> > +	chained_irq_exit(chip, desc);
> > +
> > +	/* unmask interrupt */
> > +	if (netif_running(wx->netdev))
> > +		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
> 
> Is that a hardware requirement, that interrupts only work when the interface is
> running? Interrupts are not normally conditional like this, at least when the SoC
> provides the GPIO controller.

Should we handle the interrupts when interface is not running?

Re: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Andrew Lunn]

> > Is that a hardware requirement, that interrupts only work when the interface is
> > running? Interrupts are not normally conditional like this, at least when the SoC
> > provides the GPIO controller.
> 
> Should we handle the interrupts when interface is not running?

You are adding a generic GPIO controller. It could in theory be used
for anything. GPIO controlled LEDs, buttons, bit-banging MDIO/SPI/I2C,
etc. I would not artificially limit interrupts to just when the
interface is up, unless you need to.

You should also take a look at the SFP code. When does it enable
interrupts? When the SFP device is created, or when phylink_start() is
called? SoC GPIO controllers work all the time, so it could be the SFP
code just assumes interrupts work as soon as the GPIO is
available. With the interface down, try disconnecting/connecting the
fibre and see if /sys/kernel/debug/sfpX/state shows it sees LOS
change.

     Andrew

Re: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Andrew Lunn]

> +static int txgbe_gpio_set_type(struct irq_data *d, unsigned int type)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	struct wx *wx = gpiochip_get_data(gc);
> +	u32 level, polarity;
> +
> +	level = rd32(wx, WX_GPIO_INTTYPE_LEVEL);
> +	polarity = rd32(wx, WX_GPIO_POLARITY);
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_BOTH:
> +		level |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_EDGE_RISING:
> +		level |= BIT(hwirq);
> +		polarity |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		level |= BIT(hwirq);
> +		polarity &= ~BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		level &= ~BIT(hwirq);
> +		polarity |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		level &= ~BIT(hwirq);
> +		polarity &= ~BIT(hwirq);
> +		break;
> +	}

You have two configuration bits, level and polarity, yet handle 5
different types?

> +	wr32m(wx, WX_GPIO_INTEN, BIT(hwirq), BIT(hwirq));
> +	wr32(wx, WX_GPIO_INTTYPE_LEVEL, level);
> +	if (type != IRQ_TYPE_EDGE_BOTH)
> +		wr32(wx, WX_GPIO_POLARITY, polarity);

If we are interested in both edges, then polarity is meaningless. So i
can understand not writing it. But how does the hardware know polarity
should not be used?

       Andrew

RE: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Jiawen Wu]

On Thursday, May 11, 2023 8:39 PM, Andrew Lunn wrote:
> > +static int txgbe_gpio_set_type(struct irq_data *d, unsigned int type)
> > +{
> > +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> > +	struct wx *wx = gpiochip_get_data(gc);
> > +	u32 level, polarity;
> > +
> > +	level = rd32(wx, WX_GPIO_INTTYPE_LEVEL);
> > +	polarity = rd32(wx, WX_GPIO_POLARITY);
> > +
> > +	switch (type) {
> > +	case IRQ_TYPE_EDGE_BOTH:
> > +		level |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_EDGE_RISING:
> > +		level |= BIT(hwirq);
> > +		polarity |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_EDGE_FALLING:
> > +		level |= BIT(hwirq);
> > +		polarity &= ~BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_HIGH:
> > +		level &= ~BIT(hwirq);
> > +		polarity |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_LOW:
> > +		level &= ~BIT(hwirq);
> > +		polarity &= ~BIT(hwirq);
> > +		break;
> > +	}
> 
> You have two configuration bits, level and polarity, yet handle 5 different types?
> 
> > +	wr32m(wx, WX_GPIO_INTEN, BIT(hwirq), BIT(hwirq));
> > +	wr32(wx, WX_GPIO_INTTYPE_LEVEL, level);
> > +	if (type != IRQ_TYPE_EDGE_BOTH)
> > +		wr32(wx, WX_GPIO_POLARITY, polarity);
> 
> If we are interested in both edges, then polarity is meaningless. So i can
> understand not writing it. But how does the hardware know polarity should not
> be used?

I will add toggle trigger to set polarity in both edges, to solve the hysteretic
interrupts problem that has been bothering me.

Re: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[andy.shevchenko]

Tue, May 09, 2023 at 10:27:31AM +0800, Jiawen Wu kirjoitti:
> Register GPIO chip and handle GPIO IRQ for SFP socket.

...

> +#include <linux/gpio/consumer.h>

What for?

> +#include <linux/gpio/machine.h>
> +#include <linux/gpio/driver.h>

...

> +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	struct txgbe *txgbe = wx->priv;
> +	int val;
> +
> +	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> +
> +	txgbe->gpio_orig &= ~BIT(offset);
> +	txgbe->gpio_orig |= val;

This can use standard pattern in conjunction with simple rd32() call:

	txgbe->gpio_orig = (txgbe->gpio_orig & ~BIT(offset)) | (val & BIT(offset));

otherwise it's not immediately obvious that val can have only one bit set.

> +	return !!(val & BIT(offset));
> +}

...

> +static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
> +				    int val)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	u32 mask;
> +
> +	mask = BIT(offset) | BIT(offset - 1);
> +	if (val)
> +		wr32m(wx, WX_GPIO_DR, mask, mask);
> +	else
> +		wr32m(wx, WX_GPIO_DR, mask, 0);
> +
> +	wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));

Can you explain, what prevents to have this flow to be interleaved by other API
calls, like ->direction_in()? Didn't you missed proper locking schema?

> +	return 0;
> +}

...

> +	switch (type) {
> +	case IRQ_TYPE_EDGE_BOTH:
> +		level |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_EDGE_RISING:
> +		level |= BIT(hwirq);
> +		polarity |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		level |= BIT(hwirq);

> +		polarity &= ~BIT(hwirq);

This...

> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		level &= ~BIT(hwirq);

...and this can be done outside of the switch-case. Then you simply set certain
bits where it's needed.

> +		polarity |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		level &= ~BIT(hwirq);
> +		polarity &= ~BIT(hwirq);
> +		break;

default?

> +	}

...

> +	/* workaround for hysteretic gpio interrupts */

GPIO

...

> +	gc->can_sleep = false;

Useless, kzalloc() already sets this to 0.

...

> +	girq->num_parents = 1;
> +	girq->parents = devm_kcalloc(&pdev->dev, 1, sizeof(*girq->parents),

Use girq->num_parents instead of explicit 1 in this call.

> +				     GFP_KERNEL);

Also with 

	struct device *dev = &pdev->dev;

this (and others) can be modified as

	girq->parents = devm_kcalloc(dev, girq->num_parents, sizeof(*girq->parents),

> +	if (!girq->parents)
> +		return -ENOMEM;


...

> +#define TXGBE_PX_MISC_IEN_MASK ( \
> +				TXGBE_PX_MISC_ETH_LKDN | \
> +				TXGBE_PX_MISC_DEV_RST | \
> +				TXGBE_PX_MISC_ETH_EVENT | \
> +				TXGBE_PX_MISC_ETH_LK | \
> +				TXGBE_PX_MISC_ETH_AN | \
> +				TXGBE_PX_MISC_INT_ERR | \
> +				TXGBE_PX_MISC_GPIO)

Wouldn't be better

#define TXGBE_PX_MISC_IEN_MASK				  \
	(TXGBE_PX_MISC_ETH_LKDN | TXGBE_PX_MISC_ETH_LK |  \
	 TXGBE_PX_MISC_ETH_EVENT | TXGBE_PX_MISC_ETH_AN | \
	 TXGBE_PX_MISC_DEV_RST | TXGBE_PX_MISC_INT_ERR |  \
	 TXGBE_PX_MISC_GPIO)

?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Jiawen Wu]

> > +static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
> > +				    int val)
> > +{
> > +	struct wx *wx = gpiochip_get_data(chip);
> > +	u32 mask;
> > +
> > +	mask = BIT(offset) | BIT(offset - 1);
> > +	if (val)
> > +		wr32m(wx, WX_GPIO_DR, mask, mask);
> > +	else
> > +		wr32m(wx, WX_GPIO_DR, mask, 0);
> > +
> > +	wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));
> 
> Can you explain, what prevents to have this flow to be interleaved by other API
> calls, like ->direction_in()? Didn't you missed proper locking schema?

It's true, I should add spinlock for writing GPIO registers.

> 
> > +	return 0;
> > +}
> 
> ...
> 
> > +	switch (type) {
> > +	case IRQ_TYPE_EDGE_BOTH:
> > +		level |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_EDGE_RISING:
> > +		level |= BIT(hwirq);
> > +		polarity |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_EDGE_FALLING:
> > +		level |= BIT(hwirq);
> 
> > +		polarity &= ~BIT(hwirq);
> 
> This...
> 
> > +		break;
> > +	case IRQ_TYPE_LEVEL_HIGH:
> > +		level &= ~BIT(hwirq);
> 
> ...and this can be done outside of the switch-case. Then you simply set certain
> bits where it's needed.
> 
> > +		polarity |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_LOW:
> > +		level &= ~BIT(hwirq);
> > +		polarity &= ~BIT(hwirq);
> > +		break;
> 
> default?

Do you mean that treat IRQ_TYPE_LEVEL_LOW as default case, clear level and
polarity firstly, then set the bits in other needed case? 

Re: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Andy Shevchenko]

On Fri, May 12, 2023 at 11:58 AM Jiawen Wu <jiawenwu@trustnetic.com> wrote:

...

> > > +   switch (type) {
> > > +   case IRQ_TYPE_EDGE_BOTH:
> > > +           level |= BIT(hwirq);
> > > +           break;
> > > +   case IRQ_TYPE_EDGE_RISING:
> > > +           level |= BIT(hwirq);
> > > +           polarity |= BIT(hwirq);
> > > +           break;
> > > +   case IRQ_TYPE_EDGE_FALLING:
> > > +           level |= BIT(hwirq);
> >
> > > +           polarity &= ~BIT(hwirq);
> >
> > This...
> >
> > > +           break;
> > > +   case IRQ_TYPE_LEVEL_HIGH:
> > > +           level &= ~BIT(hwirq);
> >
> > ...and this can be done outside of the switch-case. Then you simply set certain
> > bits where it's needed.
> >
> > > +           polarity |= BIT(hwirq);
> > > +           break;
> > > +   case IRQ_TYPE_LEVEL_LOW:
> > > +           level &= ~BIT(hwirq);
> > > +           polarity &= ~BIT(hwirq);
> > > +           break;
> >
> > default?
>
> Do you mean that treat IRQ_TYPE_LEVEL_LOW as default case, clear level and
> polarity firstly, then set the bits in other needed case?

level &= ...
polarity &= ...

switch () {
case X:
  level |= ...
  break;
case Y:
  polarity |= ...
  break;
case Z:
  ...
  break;
default:
  ...handle error...
}


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Russell King (Oracle)]

On Tue, May 09, 2023 at 10:27:31AM +0800, Jiawen Wu wrote:
> Register GPIO chip and handle GPIO IRQ for SFP socket.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  drivers/net/ethernet/wangxun/Kconfig          |   2 +
>  drivers/net/ethernet/wangxun/libwx/wx_lib.c   |   3 +-
>  drivers/net/ethernet/wangxun/libwx/wx_type.h  |   2 +
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  20 +-
>  .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 228 ++++++++++++++++++
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  27 +++
>  6 files changed, 263 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
> index 90812d76181d..73f4492928c0 100644
> --- a/drivers/net/ethernet/wangxun/Kconfig
> +++ b/drivers/net/ethernet/wangxun/Kconfig
> @@ -41,6 +41,8 @@ config TXGBE
>  	tristate "Wangxun(R) 10GbE PCI Express adapters support"
>  	depends on PCI
>  	select I2C_DESIGNWARE_PLATFORM
> +	select GPIOLIB_IRQCHIP
> +	select GPIOLIB
>  	select REGMAP
>  	select COMMON_CLK
>  	select LIBWX
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> index 1e8d8b7b0c62..590215303d45 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> @@ -1348,7 +1348,8 @@ void wx_free_irq(struct wx *wx)
>  		free_irq(entry->vector, q_vector);
>  	}
>  
> -	free_irq(wx->msix_entries[vector].vector, wx);
> +	if (wx->mac.type == wx_mac_em)
> +		free_irq(wx->msix_entries[vector].vector, wx);
>  }
>  EXPORT_SYMBOL(wx_free_irq);
>  
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index 97bce855bc60..d151d6f79022 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -79,7 +79,9 @@
>  #define WX_GPIO_INTMASK              0x14834
>  #define WX_GPIO_INTTYPE_LEVEL        0x14838
>  #define WX_GPIO_POLARITY             0x1483C
> +#define WX_GPIO_INTSTATUS            0x14844
>  #define WX_GPIO_EOI                  0x1484C
> +#define WX_GPIO_EXT                  0x14850
>  
>  /*********************** Transmit DMA registers **************************/
>  /* transmit global control */
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index e10296abf5b4..ded04e9e136f 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -82,6 +82,8 @@ static int txgbe_enumerate_functions(struct wx *wx)
>   **/
>  static void txgbe_irq_enable(struct wx *wx, bool queues)
>  {
> +	wr32(wx, WX_PX_MISC_IEN, TXGBE_PX_MISC_IEN_MASK);
> +
>  	/* unmask interrupt */
>  	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
>  	if (queues)
> @@ -129,17 +131,6 @@ static irqreturn_t txgbe_intr(int __always_unused irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -static irqreturn_t txgbe_msix_other(int __always_unused irq, void *data)
> -{
> -	struct wx *wx = data;
> -
> -	/* re-enable the original interrupt state */
> -	if (netif_running(wx->netdev))
> -		txgbe_irq_enable(wx, false);
> -
> -	return IRQ_HANDLED;
> -}
> -
>  /**
>   * txgbe_request_msix_irqs - Initialize MSI-X interrupts
>   * @wx: board private structure
> @@ -171,13 +162,6 @@ static int txgbe_request_msix_irqs(struct wx *wx)
>  		}
>  	}
>  
> -	err = request_irq(wx->msix_entries[vector].vector,
> -			  txgbe_msix_other, 0, netdev->name, wx);
> -	if (err) {
> -		wx_err(wx, "request_irq for msix_other failed: %d\n", err);
> -		goto free_queue_irqs;
> -	}
> -
>  	return 0;
>  
>  free_queue_irqs:
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> index 23fa7311db45..8085616a9146 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -2,6 +2,9 @@
>  /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
>  
>  #include <linux/platform_device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/gpio/property.h>
>  #include <linux/regmap.h>
>  #include <linux/clkdev.h>
> @@ -10,6 +13,7 @@
>  #include <linux/pci.h>
>  
>  #include "../libwx/wx_type.h"
> +#include "../libwx/wx_hw.h"
>  #include "txgbe_type.h"
>  #include "txgbe_phy.h"
>  
> @@ -74,6 +78,224 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
>  	return software_node_register_node_group(nodes->group);
>  }
>  
> +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	struct txgbe *txgbe = wx->priv;
> +	int val;
> +
> +	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> +
> +	txgbe->gpio_orig &= ~BIT(offset);
> +	txgbe->gpio_orig |= val;

You seem to be using this as a way to implement triggering interrupts on
both levels. Reading the GPIO value using the GPIO functions should not
change the interrupt state, so this is wrong.

> +
> +	return !!(val & BIT(offset));
> +}
> +
> +static int txgbe_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	u32 val;
> +
> +	val = rd32(wx, WX_GPIO_DDR);
> +	if (BIT(offset) & val)
> +		return GPIO_LINE_DIRECTION_OUT;
> +
> +	return GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int txgbe_gpio_direction_in(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +
> +	wr32m(wx, WX_GPIO_DDR, BIT(offset), 0);
> +
> +	return 0;
> +}
> +
> +static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
> +				    int val)
> +{
> +	struct wx *wx = gpiochip_get_data(chip);
> +	u32 mask;
> +
> +	mask = BIT(offset) | BIT(offset - 1);
> +	if (val)
> +		wr32m(wx, WX_GPIO_DR, mask, mask);
> +	else
> +		wr32m(wx, WX_GPIO_DR, mask, 0);

Why are you writing two neighbouring bits here? If GPIO 0 is requested
to change, offset will be zero, and BIT(-1) is probably not what you
want.

Moreover, if requesting a change to GPIO 3, BIT(offset - 1) will also
hit GPIO 2.

Maybe there's a "* 2" missing here?

If this code is in fact correct, it needs a comment to explain what's
going on here.

> +
> +	wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));
> +
> +	return 0;
> +}
> +
> +static void txgbe_gpio_irq_ack(struct irq_data *d)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	struct wx *wx = gpiochip_get_data(gc);
> +
> +	wr32(wx, WX_GPIO_EOI, BIT(hwirq));
> +}
> +
> +static void txgbe_gpio_irq_mask(struct irq_data *d)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	struct wx *wx = gpiochip_get_data(gc);
> +
> +	gpiochip_disable_irq(gc, hwirq);
> +
> +	wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), BIT(hwirq));
> +}
> +
> +static void txgbe_gpio_irq_unmask(struct irq_data *d)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	struct wx *wx = gpiochip_get_data(gc);
> +
> +	gpiochip_enable_irq(gc, hwirq);
> +
> +	wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), 0);
> +}
> +
> +static int txgbe_gpio_set_type(struct irq_data *d, unsigned int type)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	struct wx *wx = gpiochip_get_data(gc);
> +	u32 level, polarity;
> +
> +	level = rd32(wx, WX_GPIO_INTTYPE_LEVEL);
> +	polarity = rd32(wx, WX_GPIO_POLARITY);
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_BOTH:
> +		level |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_EDGE_RISING:
> +		level |= BIT(hwirq);
> +		polarity |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		level |= BIT(hwirq);

Are you sure you've got this correct. "level" gets set when edge cases
are requested and cleared when level cases are requested. It seems that
the register really selects edge-mode if the bit is set? Is it described
in the documentation as "not level" ?

> +		polarity &= ~BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		level &= ~BIT(hwirq);
> +		polarity |= BIT(hwirq);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		level &= ~BIT(hwirq);
> +		polarity &= ~BIT(hwirq);
> +		break;
> +	}
> +
> +	if (type & IRQ_TYPE_LEVEL_MASK)
> +		irq_set_handler_locked(d, handle_level_irq);
> +	else if (type & IRQ_TYPE_EDGE_BOTH)
> +		irq_set_handler_locked(d, handle_edge_irq);

So what handler do we end up with if a GPIO is initially requested for
a level IRQ, released, and then requested for an edge IRQ?

Secondly, a more general comment. You are using the masks here, and we
can see from the above that there is a pattern to the setting of level
and polarity. Also, IMHO there's a simpler way to do this:

	u32 level, polarity, mask, val;

	mask = BIT(hwirq);

	if (type & IRQ_TYPE_LEVEL_MASK) {
		level = 0;
		irq_set_handler_locked(d, handle_level_irq);
	} else {
		level = mask;
		/* fixme: irq_set_handler_locked(handle_edge_irq); ? */
	}

	if (type == IRQ_TYPE_EDGE_RISING || type == IRQ_TYPE_LEVEL_HIGH)
		polarity = mask;
	else
		polarity = 0;

	/* fixme: what does this register do, and why is it configured
	 * here?
	 */
	wr32m(wx, WX_GPIO_INTEN, mask, mask);

	wr32m(wx, WX_GPIO_INTTYPE_LEVEL, mask, level);
	if (type != IRQ_TYPE_EDGE_BOTH)
		wr32m(wx, WX_GPIO_POLARITY, mask, polarity);

Now, as for both-edge interrupts, this needs further thought. As I say
above, using the _get method to update the internal interrupt state
won't be reliable.

If the hardware has no way to trigger on both edges, then it's going to
take some additional complexity to make this work. Firstly, you need to
record which interrupts are using both-edges in the driver, so you know
which need extra work in the interrupt handler.

Secondly, you still need to configure the polarity as best you can to
pick up the first change in state here. That means reading the current
GPIO state, and configuring the GPIO polarity correctly here. It's
going to be racy with the hardware, so the closer together you can get
the GPIO state-to-polarity-set the better in terms of the size of the
race window.

> +static void txgbe_irq_handler(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct wx *wx = irq_desc_get_handler_data(desc);
> +	struct txgbe *txgbe = wx->priv;
> +	irq_hw_number_t hwirq;
> +	unsigned long gpioirq;
> +	struct gpio_chip *gc;
> +	u32 gpio;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);

Does reading INTSTATUS clear down any of the pending status bits?

> +
> +	/* workaround for hysteretic gpio interrupts */
> +	gpio = rd32(wx, WX_GPIO_EXT);
> +	if (!gpioirq)
> +		gpioirq = txgbe->gpio_orig ^ gpio;

This doesn't solve the both-edge case, because this will only get
evaluated if some other GPIO also happens to raise an interrupt.

For any GPIOs should have both-edge applied, you need to read the
current state of the GPIO and program the polarity appropriately,
then re-read the GPIO to see if it changed state during that race
and handle that as best that can be.

The problem is that making both-edge work reliably on hardware that
doesn't support both-edge will always be rather racy.

> +
> +	gc = txgbe->gpio;
> +	for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
> +		generic_handle_domain_irq(gc->irq.domain, hwirq);
> +
> +	chained_irq_exit(chip, desc);
> +
> +	/* unmask interrupt */
> +	if (netif_running(wx->netdev))
> +		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));

Why does this depend on whether the network interface is running, and
why is it done at the end of the interrupt handler? Maybe this needs a
better comment in the code explaining what it's actually doing?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next v7 6/9] net: txgbe: Support GPIO to SFP socket

[Jiawen Wu]

> > +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
> > +{
> > +	struct wx *wx = gpiochip_get_data(chip);
> > +	struct txgbe *txgbe = wx->priv;
> > +	int val;
> > +
> > +	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> > +
> > +	txgbe->gpio_orig &= ~BIT(offset);
> > +	txgbe->gpio_orig |= val;
> 
> You seem to be using this as a way to implement triggering interrupts on
> both levels. Reading the GPIO value using the GPIO functions should not
> change the interrupt state, so this is wrong.

Yes, I just found the correct way to deal with it.

> > +static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
> > +				    int val)
> > +{
> > +	struct wx *wx = gpiochip_get_data(chip);
> > +	u32 mask;
> > +
> > +	mask = BIT(offset) | BIT(offset - 1);
> > +	if (val)
> > +		wr32m(wx, WX_GPIO_DR, mask, mask);
> > +	else
> > +		wr32m(wx, WX_GPIO_DR, mask, 0);
> 
> Why are you writing two neighbouring bits here? If GPIO 0 is requested
> to change, offset will be zero, and BIT(-1) is probably not what you
> want.
> 
> Moreover, if requesting a change to GPIO 3, BIT(offset - 1) will also
> hit GPIO 2.
> 
> Maybe there's a "* 2" missing here?
> 
> If this code is in fact correct, it needs a comment to explain what's
> going on here.

GPIO lines description:
	/* GPIO 0: tx fault
	 * GPIO 1: tx disable
	 * GPIO 2: sfp module absent
	 * GPIO 3: rx signal lost
	 * GPIO 4: rate select 1, 1G(0) 10G(1)
	 * GPIO 5: rate select 0, 1G(0) 10G(1)
	 */
The previous consideration was processing GPIO 0&1, 4&5. The output
lines are 1/4/5. Under the persistent misconfiguration of flash, GPIO 0 is
treated as the output signal to enable/disable TX laser, together with
GPIO 1. And GPIO 4 seems not be used by SFP driver, to change module
rate, also this driver does not implement rate switching either.

In my understanding, the input GPIO does not call this function, so I put
no  condition there. But in general, with all GPIO being used correctly,
removing these odd codes should work as well. I'll fix it and test it again.

> > +static int txgbe_gpio_set_type(struct irq_data *d, unsigned int type)
> > +{
> > +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> > +	struct wx *wx = gpiochip_get_data(gc);
> > +	u32 level, polarity;
> > +
> > +	level = rd32(wx, WX_GPIO_INTTYPE_LEVEL);
> > +	polarity = rd32(wx, WX_GPIO_POLARITY);
> > +
> > +	switch (type) {
> > +	case IRQ_TYPE_EDGE_BOTH:
> > +		level |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_EDGE_RISING:
> > +		level |= BIT(hwirq);
> > +		polarity |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_EDGE_FALLING:
> > +		level |= BIT(hwirq);
> 
> Are you sure you've got this correct. "level" gets set when edge cases
> are requested and cleared when level cases are requested. It seems that
> the register really selects edge-mode if the bit is set? Is it described
> in the documentation as "not level" ?

Yes, the WX_GPIO_INTTYPE_LEVEL register shows that 0 for level-sensitive,
1 for edge-sensitive.

> 
> > +		polarity &= ~BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_HIGH:
> > +		level &= ~BIT(hwirq);
> > +		polarity |= BIT(hwirq);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_LOW:
> > +		level &= ~BIT(hwirq);
> > +		polarity &= ~BIT(hwirq);
> > +		break;
> > +	}
> > +
> > +	if (type & IRQ_TYPE_LEVEL_MASK)
> > +		irq_set_handler_locked(d, handle_level_irq);
> > +	else if (type & IRQ_TYPE_EDGE_BOTH)
> > +		irq_set_handler_locked(d, handle_edge_irq);
> 
> So what handler do we end up with if a GPIO is initially requested for
> a level IRQ, released, and then requested for an edge IRQ?

Sorry I don't know much about it, who controls the IRQ type? In fact,
edge IRQ always be requested in my test, and the type is EDGE_BOTH.

> 
> Secondly, a more general comment. You are using the masks here, and we
> can see from the above that there is a pattern to the setting of level
> and polarity. Also, IMHO there's a simpler way to do this:
> 
> 	u32 level, polarity, mask, val;
> 
> 	mask = BIT(hwirq);
> 
> 	if (type & IRQ_TYPE_LEVEL_MASK) {
> 		level = 0;
> 		irq_set_handler_locked(d, handle_level_irq);
> 	} else {
> 		level = mask;
> 		/* fixme: irq_set_handler_locked(handle_edge_irq); ? */
> 	}
> 
> 	if (type == IRQ_TYPE_EDGE_RISING || type == IRQ_TYPE_LEVEL_HIGH)
> 		polarity = mask;
> 	else
> 		polarity = 0;
> 
> 	/* fixme: what does this register do, and why is it configured
> 	 * here?
> 	 */
> 	wr32m(wx, WX_GPIO_INTEN, mask, mask);

It enables corresponding GPIO interrupt.

> 
> 	wr32m(wx, WX_GPIO_INTTYPE_LEVEL, mask, level);
> 	if (type != IRQ_TYPE_EDGE_BOTH)
> 		wr32m(wx, WX_GPIO_POLARITY, mask, polarity);
> 
> Now, as for both-edge interrupts, this needs further thought. As I say
> above, using the _get method to update the internal interrupt state
> won't be reliable.
> 
> If the hardware has no way to trigger on both edges, then it's going to
> take some additional complexity to make this work. Firstly, you need to
> record which interrupts are using both-edges in the driver, so you know
> which need extra work in the interrupt handler.
> 
> Secondly, you still need to configure the polarity as best you can to
> pick up the first change in state here. That means reading the current
> GPIO state, and configuring the GPIO polarity correctly here. It's
> going to be racy with the hardware, so the closer together you can get
> the GPIO state-to-polarity-set the better in terms of the size of the
> race window.

Thanks for the detailed advice. Hardware can trigger the interrupts on both
edges, just set polarity.

> 
> > +static void txgbe_irq_handler(struct irq_desc *desc)
> > +{
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	struct wx *wx = irq_desc_get_handler_data(desc);
> > +	struct txgbe *txgbe = wx->priv;
> > +	irq_hw_number_t hwirq;
> > +	unsigned long gpioirq;
> > +	struct gpio_chip *gc;
> > +	u32 gpio;
> > +
> > +	chained_irq_enter(chip, desc);
> > +
> > +	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
> 
> Does reading INTSTATUS clear down any of the pending status bits?

No, it's read only. Interrupt status bits will be cleared in txgbe_gpio_irq_ack().

> 
> > +
> > +	/* workaround for hysteretic gpio interrupts */
> > +	gpio = rd32(wx, WX_GPIO_EXT);
> > +	if (!gpioirq)
> > +		gpioirq = txgbe->gpio_orig ^ gpio;
> 
> This doesn't solve the both-edge case, because this will only get
> evaluated if some other GPIO also happens to raise an interrupt.
> 
> For any GPIOs should have both-edge applied, you need to read the
> current state of the GPIO and program the polarity appropriately,
> then re-read the GPIO to see if it changed state during that race
> and handle that as best that can be.
> 
> The problem is that making both-edge work reliably on hardware that
> doesn't support both-edge will always be rather racy.

Thanks again, I learned a lot.

> 
> > +
> > +	gc = txgbe->gpio;
> > +	for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
> > +		generic_handle_domain_irq(gc->irq.domain, hwirq);
> > +
> > +	chained_irq_exit(chip, desc);
> > +
> > +	/* unmask interrupt */
> > +	if (netif_running(wx->netdev))
> > +		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
> 
> Why does this depend on whether the network interface is running, and
> why is it done at the end of the interrupt handler? Maybe this needs a
> better comment in the code explaining what it's actually doing?

This register should be written after handling interrupts, otherwise the next
Interrupt may not come, by hardware design. And should we handle the
interrupts when interface is not running? I didn't think it should do, so I made
it conditional.

[PATCH net-next v7 7/9] net: pcs: Add 10GBASE-R mode for Synopsys Designware XPCS

[Jiawen Wu]

Add basic support for XPCS using 10GBASE-R interface. This mode will
be extended to use interrupt, so set pcs.poll false. And avoid soft
reset so that the device using this mode is in the default configuration.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/pcs/pcs-xpcs.c   | 30 ++++++++++++++++++++++++++++++
 include/linux/pcs/pcs-xpcs.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 539cd43eae8d..c7c8b8d1311f 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -64,6 +64,16 @@ static const int xpcs_xlgmii_features[] = {
 	__ETHTOOL_LINK_MODE_MASK_NBITS,
 };
 
+static const int xpcs_10gbaser_features[] = {
+	ETHTOOL_LINK_MODE_Pause_BIT,
+	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+	ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
+	ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
+	ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+	ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+	__ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
 static const int xpcs_sgmii_features[] = {
 	ETHTOOL_LINK_MODE_Pause_BIT,
 	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
@@ -106,6 +116,10 @@ static const phy_interface_t xpcs_xlgmii_interfaces[] = {
 	PHY_INTERFACE_MODE_XLGMII,
 };
 
+static const phy_interface_t xpcs_10gbaser_interfaces[] = {
+	PHY_INTERFACE_MODE_10GBASER,
+};
+
 static const phy_interface_t xpcs_sgmii_interfaces[] = {
 	PHY_INTERFACE_MODE_SGMII,
 };
@@ -123,6 +137,7 @@ enum {
 	DW_XPCS_USXGMII,
 	DW_XPCS_10GKR,
 	DW_XPCS_XLGMII,
+	DW_XPCS_10GBASER,
 	DW_XPCS_SGMII,
 	DW_XPCS_1000BASEX,
 	DW_XPCS_2500BASEX,
@@ -246,6 +261,7 @@ static int xpcs_soft_reset(struct dw_xpcs *xpcs,
 
 	switch (compat->an_mode) {
 	case DW_AN_C73:
+	case DW_10GBASER:
 		dev = MDIO_MMD_PCS;
 		break;
 	case DW_AN_C37_SGMII:
@@ -872,6 +888,8 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
 		return -ENODEV;
 
 	switch (compat->an_mode) {
+	case DW_10GBASER:
+		break;
 	case DW_AN_C73:
 		if (phylink_autoneg_inband(mode)) {
 			ret = xpcs_config_aneg_c73(xpcs, compat);
@@ -1033,6 +1051,9 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
 		return;
 
 	switch (compat->an_mode) {
+	case DW_10GBASER:
+		phylink_mii_c45_pcs_get_state(xpcs->mdiodev, state);
+		break;
 	case DW_AN_C73:
 		ret = xpcs_get_state_c73(xpcs, state, compat);
 		if (ret) {
@@ -1188,6 +1209,12 @@ static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
 		.num_interfaces = ARRAY_SIZE(xpcs_xlgmii_interfaces),
 		.an_mode = DW_AN_C73,
 	},
+	[DW_XPCS_10GBASER] = {
+		.supported = xpcs_10gbaser_features,
+		.interface = xpcs_10gbaser_interfaces,
+		.num_interfaces = ARRAY_SIZE(xpcs_10gbaser_interfaces),
+		.an_mode = DW_10GBASER,
+	},
 	[DW_XPCS_SGMII] = {
 		.supported = xpcs_sgmii_features,
 		.interface = xpcs_sgmii_interfaces,
@@ -1290,6 +1317,9 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
 		}
 
 		xpcs->pcs.ops = &xpcs_phylink_ops;
+		if (compat->an_mode == DW_10GBASER)
+			return xpcs;
+
 		xpcs->pcs.poll = true;
 
 		ret = xpcs_soft_reset(xpcs, compat);
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index d2da1e0b4a92..61df0c717a0e 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -18,6 +18,7 @@
 #define DW_AN_C37_SGMII			2
 #define DW_2500BASEX			3
 #define DW_AN_C37_1000BASEX		4
+#define DW_10GBASER			5
 
 struct xpcs_id;
 
-- 
2.27.0

Re: [PATCH net-next v7 7/9] net: pcs: Add 10GBASE-R mode for Synopsys Designware XPCS

[Andrew Lunn]

On Tue, May 09, 2023 at 10:27:32AM +0800, Jiawen Wu wrote:
> Add basic support for XPCS using 10GBASE-R interface. This mode will
> be extended to use interrupt, so set pcs.poll false. And avoid soft
> reset so that the device using this mode is in the default configuration.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v7 8/9] net: txgbe: Implement phylink pcs

[Jiawen Wu]

Register MDIO bus for PCS layer to use Synopsys designware XPCS, support
10GBASE-R interface to the controller.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |  1 +
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 91 ++++++++++++++++++-
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  6 ++
 3 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index 73f4492928c0..f3fb273e6fd0 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -45,6 +45,7 @@ config TXGBE
 	select GPIOLIB
 	select REGMAP
 	select COMMON_CLK
+	select PCS_XPCS
 	select LIBWX
 	select SFP
 	help
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 8085616a9146..0ab2898e764a 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -9,6 +9,8 @@
 #include <linux/regmap.h>
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
+#include <linux/pcs/pcs-xpcs.h>
+#include <linux/mdio.h>
 #include <linux/i2c.h>
 #include <linux/pci.h>
 
@@ -78,6 +80,82 @@ static int txgbe_swnodes_register(struct txgbe *txgbe)
 	return software_node_register_node_group(nodes->group);
 }
 
+static int txgbe_pcs_read(struct mii_bus *bus, int addr, int devnum, int regnum)
+{
+	struct wx *wx  = bus->priv;
+	u32 offset, val;
+
+	offset = devnum << 16 | regnum;
+
+	/* Set the LAN port indicator to IDA_ADDR */
+	wr32(wx, TXGBE_XPCS_IDA_ADDR, offset);
+
+	/* Read the data from IDA_DATA register */
+	val = rd32(wx, TXGBE_XPCS_IDA_DATA);
+
+	return (u16)val;
+}
+
+static int txgbe_pcs_write(struct mii_bus *bus, int addr, int devnum, int regnum, u16 val)
+{
+	struct wx *wx = bus->priv;
+	u32 offset;
+
+	offset = devnum << 16 | regnum;
+
+	/* Set the LAN port indicator to IDA_ADDR */
+	wr32(wx, TXGBE_XPCS_IDA_ADDR, offset);
+
+	/* Write the data to IDA_DATA register */
+	wr32(wx, TXGBE_XPCS_IDA_DATA, val);
+
+	return 0;
+}
+
+static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
+{
+	struct mdio_device *mdiodev;
+	struct wx *wx = txgbe->wx;
+	struct mii_bus *mii_bus;
+	struct dw_xpcs *xpcs;
+	struct pci_dev *pdev;
+	int ret = 0;
+
+	pdev = wx->pdev;
+
+	mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	if (!mii_bus)
+		return -ENOMEM;
+
+	mii_bus->name = "txgbe_pcs_mdio_bus";
+	mii_bus->read_c45 = &txgbe_pcs_read;
+	mii_bus->write_c45 = &txgbe_pcs_write;
+	mii_bus->parent = &pdev->dev;
+	mii_bus->phy_mask = ~0;
+	mii_bus->priv = wx;
+	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "txgbe_pcs-%x",
+		 (pdev->bus->number << 8) | pdev->devfn);
+
+	ret = devm_mdiobus_register(&pdev->dev, mii_bus);
+	if (ret)
+		return ret;
+
+	mdiodev = mdio_device_create(mii_bus, 0);
+	if (IS_ERR(mdiodev))
+		return PTR_ERR(mdiodev);
+
+	xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER);
+	if (IS_ERR_OR_NULL(xpcs)) {
+		mdio_device_free(mdiodev);
+		return PTR_ERR(xpcs);
+	}
+
+	txgbe->mdiodev = mdiodev;
+	txgbe->xpcs = xpcs;
+
+	return 0;
+}
+
 static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
 {
 	struct wx *wx = gpiochip_get_data(chip);
@@ -410,16 +488,22 @@ int txgbe_init_phy(struct txgbe *txgbe)
 		return ret;
 	}
 
+	ret = txgbe_mdio_pcs_init(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to init mdio pcs: %d\n", ret);
+		goto err_unregister_swnode;
+	}
+
 	ret = txgbe_gpio_init(txgbe);
 	if (ret) {
 		wx_err(txgbe->wx, "failed to init gpio\n");
-		goto err_unregister_swnode;
+		goto err_destroy_xpcs;
 	}
 
 	ret = txgbe_clock_register(txgbe);
 	if (ret) {
 		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
-		goto err_unregister_swnode;
+		goto err_destroy_xpcs;
 	}
 
 	ret = txgbe_i2c_register(txgbe);
@@ -441,6 +525,8 @@ int txgbe_init_phy(struct txgbe *txgbe)
 err_unregister_clk:
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
+err_destroy_xpcs:
+	xpcs_destroy(txgbe->xpcs);
 err_unregister_swnode:
 	software_node_unregister_node_group(txgbe->nodes.group);
 
@@ -453,5 +539,6 @@ void txgbe_remove_phy(struct txgbe *txgbe)
 	platform_device_unregister(txgbe->i2c_dev);
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
+	xpcs_destroy(txgbe->xpcs);
 	software_node_unregister_node_group(txgbe->nodes.group);
 }
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 796f33fe3016..75b9c7ae3c21 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -83,6 +83,10 @@
 /* I2C registers */
 #define TXGBE_I2C_BASE                          0x14900
 
+/************************************** ETH PHY ******************************/
+#define TXGBE_XPCS_IDA_ADDR                     0x13000
+#define TXGBE_XPCS_IDA_DATA                     0x13004
+
 /* Part Number String Length */
 #define TXGBE_PBANUM_LENGTH                     32
 
@@ -174,6 +178,8 @@ struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	struct mdio_device *mdiodev;
+	struct dw_xpcs *xpcs;
 	struct platform_device *sfp_dev;
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
-- 
2.27.0

Re: [PATCH net-next v7 8/9] net: txgbe: Implement phylink pcs

[Andrew Lunn]

> +static int txgbe_pcs_read(struct mii_bus *bus, int addr, int devnum, int regnum)
> +{
> +	struct wx *wx  = bus->priv;
> +	u32 offset, val;
> +
> +	offset = devnum << 16 | regnum;
> +
> +	/* Set the LAN port indicator to IDA_ADDR */
> +	wr32(wx, TXGBE_XPCS_IDA_ADDR, offset);
> +
> +	/* Read the data from IDA_DATA register */
> +	val = rd32(wx, TXGBE_XPCS_IDA_DATA);

addr is ignored here. So i assume the hardware only supports a single
address? Please add a check for address. If it is 0, do the read,
otherwise return either -EOPNOTSUPP, or 0xffff. What we don't want is
it to appear there are 32 PCS devices.

   Andrew

Re: [PATCH net-next v7 8/9] net: txgbe: Implement phylink pcs

[Piotr Raczynski]

> +static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
> +{
> +	struct mdio_device *mdiodev;
> +	struct wx *wx = txgbe->wx;
> +	struct mii_bus *mii_bus;
> +	struct dw_xpcs *xpcs;
> +	struct pci_dev *pdev;
> +	int ret = 0;
> +
> +	pdev = wx->pdev;
> +
> +	mii_bus = devm_mdiobus_alloc(&pdev->dev);
> +	if (!mii_bus)
> +		return -ENOMEM;
> +
> +	mii_bus->name = "txgbe_pcs_mdio_bus";
> +	mii_bus->read_c45 = &txgbe_pcs_read;
> +	mii_bus->write_c45 = &txgbe_pcs_write;
> +	mii_bus->parent = &pdev->dev;
> +	mii_bus->phy_mask = ~0;
> +	mii_bus->priv = wx;
> +	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "txgbe_pcs-%x",
> +		 (pdev->bus->number << 8) | pdev->devfn);
> +
> +	ret = devm_mdiobus_register(&pdev->dev, mii_bus);
> +	if (ret)
> +		return ret;
> +
> +	mdiodev = mdio_device_create(mii_bus, 0);
> +	if (IS_ERR(mdiodev))
> +		return PTR_ERR(mdiodev);
> +
> +	xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER);
> +	if (IS_ERR_OR_NULL(xpcs)) {
> +		mdio_device_free(mdiodev);
> +		return PTR_ERR(xpcs);
> +	}

xpcs_create does not seem to return NULL but if it would then you'd
return success here. Is this intentional?

> +
> +	txgbe->mdiodev = mdiodev;
> +	txgbe->xpcs = xpcs;
> +
> +	return 0;
> +}

RE: [PATCH net-next v7 8/9] net: txgbe: Implement phylink pcs

[Jiawen Wu]

On Friday, May 12, 2023 4:33 AM, Piotr Raczynski wrote:
> > +static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
> > +{
> > +	struct mdio_device *mdiodev;
> > +	struct wx *wx = txgbe->wx;
> > +	struct mii_bus *mii_bus;
> > +	struct dw_xpcs *xpcs;
> > +	struct pci_dev *pdev;
> > +	int ret = 0;
> > +
> > +	pdev = wx->pdev;
> > +
> > +	mii_bus = devm_mdiobus_alloc(&pdev->dev);
> > +	if (!mii_bus)
> > +		return -ENOMEM;
> > +
> > +	mii_bus->name = "txgbe_pcs_mdio_bus";
> > +	mii_bus->read_c45 = &txgbe_pcs_read;
> > +	mii_bus->write_c45 = &txgbe_pcs_write;
> > +	mii_bus->parent = &pdev->dev;
> > +	mii_bus->phy_mask = ~0;
> > +	mii_bus->priv = wx;
> > +	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "txgbe_pcs-%x",
> > +		 (pdev->bus->number << 8) | pdev->devfn);
> > +
> > +	ret = devm_mdiobus_register(&pdev->dev, mii_bus);
> > +	if (ret)
> > +		return ret;
> > +
> > +	mdiodev = mdio_device_create(mii_bus, 0);
> > +	if (IS_ERR(mdiodev))
> > +		return PTR_ERR(mdiodev);
> > +
> > +	xpcs = xpcs_create(mdiodev, PHY_INTERFACE_MODE_10GBASER);
> > +	if (IS_ERR_OR_NULL(xpcs)) {
> > +		mdio_device_free(mdiodev);
> > +		return PTR_ERR(xpcs);
> > +	}
> 
> xpcs_create does not seem to return NULL but if it would then you'd
> return success here. Is this intentional?

Should be if (IS_ERR(xpcs)) ...

> 
> > +
> > +	txgbe->mdiodev = mdiodev;
> > +	txgbe->xpcs = xpcs;
> > +
> > +	return 0;
> > +}
> 

[PATCH net-next v7 9/9] net: txgbe: Support phylink MAC layer

[Jiawen Wu]

Add phylink support to Wangxun 10Gb Ethernet controller for the 10GBASE-R
interface.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |   1 +
 .../ethernet/wangxun/txgbe/txgbe_ethtool.c    |  28 +++++
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  23 ++--
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 113 +++++++++++++++++-
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |   5 +
 5 files changed, 154 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index f3fb273e6fd0..2ca163f07359 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -46,6 +46,7 @@ config TXGBE
 	select REGMAP
 	select COMMON_CLK
 	select PCS_XPCS
+	select PHYLINK
 	select LIBWX
 	select SFP
 	help
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
index d914e9a05404..859da112586a 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
@@ -6,11 +6,39 @@
 #include <linux/netdevice.h>
 
 #include "../libwx/wx_ethtool.h"
+#include "../libwx/wx_type.h"
+#include "txgbe_type.h"
 #include "txgbe_ethtool.h"
 
+static int txgbe_nway_reset(struct net_device *netdev)
+{
+	struct txgbe *txgbe = netdev_to_txgbe(netdev);
+
+	return phylink_ethtool_nway_reset(txgbe->phylink);
+}
+
+static int txgbe_get_link_ksettings(struct net_device *netdev,
+				    struct ethtool_link_ksettings *cmd)
+{
+	struct txgbe *txgbe = netdev_to_txgbe(netdev);
+
+	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
+}
+
+static int txgbe_set_link_ksettings(struct net_device *netdev,
+				    const struct ethtool_link_ksettings *cmd)
+{
+	struct txgbe *txgbe = netdev_to_txgbe(netdev);
+
+	return phylink_ethtool_ksettings_set(txgbe->phylink, cmd);
+}
+
 static const struct ethtool_ops txgbe_ethtool_ops = {
 	.get_drvinfo		= wx_get_drvinfo,
+	.nway_reset		= txgbe_nway_reset,
 	.get_link		= ethtool_op_get_link,
+	.get_link_ksettings	= txgbe_get_link_ksettings,
+	.set_link_ksettings	= txgbe_set_link_ksettings,
 };
 
 void txgbe_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index ded04e9e136f..bdf735e863eb 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -7,6 +7,7 @@
 #include <linux/netdevice.h>
 #include <linux/string.h>
 #include <linux/etherdevice.h>
+#include <linux/phylink.h>
 #include <net/ip.h>
 #include <linux/if_vlan.h>
 
@@ -204,7 +205,8 @@ static int txgbe_request_irq(struct wx *wx)
 
 static void txgbe_up_complete(struct wx *wx)
 {
-	u32 reg;
+	struct net_device *netdev = wx->netdev;
+	struct txgbe *txgbe = netdev_to_txgbe(netdev);
 
 	wx_control_hw(wx, true);
 	wx_configure_vectors(wx);
@@ -213,24 +215,16 @@ static void txgbe_up_complete(struct wx *wx)
 	smp_mb__before_atomic();
 	wx_napi_enable_all(wx);
 
+	phylink_start(txgbe->phylink);
+
 	/* clear any pending interrupts, may auto mask */
 	rd32(wx, WX_PX_IC(0));
 	rd32(wx, WX_PX_IC(1));
 	rd32(wx, WX_PX_MISC_IC);
 	txgbe_irq_enable(wx, true);
 
-	/* Configure MAC Rx and Tx when link is up */
-	reg = rd32(wx, WX_MAC_RX_CFG);
-	wr32(wx, WX_MAC_RX_CFG, reg);
-	wr32(wx, WX_MAC_PKT_FLT, WX_MAC_PKT_FLT_PR);
-	reg = rd32(wx, WX_MAC_WDG_TIMEOUT);
-	wr32(wx, WX_MAC_WDG_TIMEOUT, reg);
-	reg = rd32(wx, WX_MAC_TX_CFG);
-	wr32(wx, WX_MAC_TX_CFG, (reg & ~WX_MAC_TX_CFG_SPEED_MASK) | WX_MAC_TX_CFG_SPEED_10G);
-
 	/* enable transmits */
-	netif_tx_start_all_queues(wx->netdev);
-	netif_carrier_on(wx->netdev);
+	netif_tx_start_all_queues(netdev);
 }
 
 static void txgbe_reset(struct wx *wx)
@@ -264,7 +258,6 @@ static void txgbe_disable_device(struct wx *wx)
 		wx_disable_rx_queue(wx, wx->rx_ring[i]);
 
 	netif_tx_stop_all_queues(netdev);
-	netif_carrier_off(netdev);
 	netif_tx_disable(netdev);
 
 	wx_irq_disable(wx);
@@ -295,8 +288,12 @@ static void txgbe_disable_device(struct wx *wx)
 
 static void txgbe_down(struct wx *wx)
 {
+	struct net_device *netdev = wx->netdev;
+	struct txgbe *txgbe = netdev_to_txgbe(netdev);
+
 	txgbe_disable_device(wx);
 	txgbe_reset(wx);
+	phylink_stop(txgbe->phylink);
 
 	wx_clean_all_tx_rings(wx);
 	wx_clean_all_rx_rings(wx);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 0ab2898e764a..4eedb60e93af 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -10,11 +10,13 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/pcs/pcs-xpcs.h>
+#include <linux/phylink.h>
 #include <linux/mdio.h>
 #include <linux/i2c.h>
 #include <linux/pci.h>
 
 #include "../libwx/wx_type.h"
+#include "../libwx/wx_lib.h"
 #include "../libwx/wx_hw.h"
 #include "txgbe_type.h"
 #include "txgbe_phy.h"
@@ -156,6 +158,95 @@ static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
 	return 0;
 }
 
+static struct phylink_pcs *txgbe_phylink_mac_select(struct phylink_config *config,
+						    phy_interface_t interface)
+{
+	struct txgbe *txgbe = netdev_to_txgbe(to_net_dev(config->dev));
+
+	return &txgbe->xpcs->pcs;
+}
+
+static void txgbe_mac_config(struct phylink_config *config, unsigned int mode,
+			     const struct phylink_link_state *state)
+{
+}
+
+static void txgbe_mac_link_down(struct phylink_config *config,
+				unsigned int mode, phy_interface_t interface)
+{
+	struct wx *wx = netdev_priv(to_net_dev(config->dev));
+
+	wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
+}
+
+static void txgbe_mac_link_up(struct phylink_config *config,
+			      struct phy_device *phy,
+			      unsigned int mode, phy_interface_t interface,
+			      int speed, int duplex,
+			      bool tx_pause, bool rx_pause)
+{
+	struct wx *wx = netdev_priv(to_net_dev(config->dev));
+	u32 txcfg, wdg;
+
+	txcfg = rd32(wx, WX_MAC_TX_CFG);
+	txcfg &= ~WX_MAC_TX_CFG_SPEED_MASK;
+
+	switch (speed) {
+	case SPEED_10000:
+		txcfg |= WX_MAC_TX_CFG_SPEED_10G;
+		break;
+	case SPEED_1000:
+	case SPEED_100:
+	case SPEED_10:
+		txcfg |= WX_MAC_TX_CFG_SPEED_1G;
+		break;
+	default:
+		break;
+	}
+
+	wr32(wx, WX_MAC_TX_CFG, txcfg | WX_MAC_TX_CFG_TE);
+
+	/* Re configure MAC Rx */
+	wr32m(wx, WX_MAC_RX_CFG, WX_MAC_RX_CFG_RE, WX_MAC_RX_CFG_RE);
+	wr32(wx, WX_MAC_PKT_FLT, WX_MAC_PKT_FLT_PR);
+	wdg = rd32(wx, WX_MAC_WDG_TIMEOUT);
+	wr32(wx, WX_MAC_WDG_TIMEOUT, wdg);
+}
+
+static const struct phylink_mac_ops txgbe_mac_ops = {
+	.mac_select_pcs = txgbe_phylink_mac_select,
+	.mac_config = txgbe_mac_config,
+	.mac_link_down = txgbe_mac_link_down,
+	.mac_link_up = txgbe_mac_link_up,
+};
+
+static int txgbe_phylink_init(struct txgbe *txgbe)
+{
+	struct phylink_config *config;
+	struct fwnode_handle *fwnode;
+	struct wx *wx = txgbe->wx;
+	phy_interface_t phy_mode;
+	struct phylink *phylink;
+
+	config = devm_kzalloc(&wx->pdev->dev, sizeof(*config), GFP_KERNEL);
+	if (!config)
+		return -ENOMEM;
+
+	config->dev = &wx->netdev->dev;
+	config->type = PHYLINK_NETDEV;
+	config->mac_capabilities = MAC_10000FD | MAC_1000FD | MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
+	phy_mode = PHY_INTERFACE_MODE_10GBASER;
+	__set_bit(PHY_INTERFACE_MODE_10GBASER, config->supported_interfaces);
+	fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_PHYLINK]);
+	phylink = phylink_create(config, fwnode, phy_mode, &txgbe_mac_ops);
+	if (IS_ERR(phylink))
+		return PTR_ERR(phylink);
+
+	txgbe->phylink = phylink;
+
+	return 0;
+}
+
 static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
 {
 	struct wx *wx = gpiochip_get_data(chip);
@@ -302,7 +393,9 @@ static void txgbe_irq_handler(struct irq_desc *desc)
 	irq_hw_number_t hwirq;
 	unsigned long gpioirq;
 	struct gpio_chip *gc;
-	u32 gpio;
+	u32 gpio, eicr, reg;
+
+	eicr = wx_misc_isb(wx, WX_ISB_MISC);
 
 	chained_irq_enter(chip, desc);
 
@@ -319,6 +412,11 @@ static void txgbe_irq_handler(struct irq_desc *desc)
 
 	chained_irq_exit(chip, desc);
 
+	if (eicr & (TXGBE_PX_MISC_ETH_LK | TXGBE_PX_MISC_ETH_LKDN)) {
+		reg = rd32(wx, TXGBE_CFG_PORT_ST);
+		phylink_mac_change(txgbe->phylink, !!(reg & TXGBE_CFG_PORT_ST_LINK_UP));
+	}
+
 	/* unmask interrupt */
 	if (netif_running(wx->netdev))
 		wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
@@ -494,16 +592,22 @@ int txgbe_init_phy(struct txgbe *txgbe)
 		goto err_unregister_swnode;
 	}
 
+	ret = txgbe_phylink_init(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to init phylink\n");
+		goto err_destroy_xpcs;
+	}
+
 	ret = txgbe_gpio_init(txgbe);
 	if (ret) {
 		wx_err(txgbe->wx, "failed to init gpio\n");
-		goto err_destroy_xpcs;
+		goto err_destroy_phylink;
 	}
 
 	ret = txgbe_clock_register(txgbe);
 	if (ret) {
 		wx_err(txgbe->wx, "failed to register clock: %d\n", ret);
-		goto err_destroy_xpcs;
+		goto err_destroy_phylink;
 	}
 
 	ret = txgbe_i2c_register(txgbe);
@@ -525,6 +629,8 @@ int txgbe_init_phy(struct txgbe *txgbe)
 err_unregister_clk:
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
+err_destroy_phylink:
+	phylink_destroy(txgbe->phylink);
 err_destroy_xpcs:
 	xpcs_destroy(txgbe->xpcs);
 err_unregister_swnode:
@@ -539,6 +645,7 @@ void txgbe_remove_phy(struct txgbe *txgbe)
 	platform_device_unregister(txgbe->i2c_dev);
 	clkdev_drop(txgbe->clock);
 	clk_unregister(txgbe->clk);
+	phylink_destroy(txgbe->phylink);
 	xpcs_destroy(txgbe->xpcs);
 	software_node_unregister_node_group(txgbe->nodes.group);
 }
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 75b9c7ae3c21..a889f340b14d 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -80,6 +80,10 @@
 				TXGBE_PX_MISC_INT_ERR | \
 				TXGBE_PX_MISC_GPIO)
 
+/* Port cfg registers */
+#define TXGBE_CFG_PORT_ST                       0x14404
+#define TXGBE_CFG_PORT_ST_LINK_UP               BIT(0)
+
 /* I2C registers */
 #define TXGBE_I2C_BASE                          0x14900
 
@@ -180,6 +184,7 @@ struct txgbe {
 	struct txgbe_nodes nodes;
 	struct mdio_device *mdiodev;
 	struct dw_xpcs *xpcs;
+	struct phylink *phylink;
 	struct platform_device *sfp_dev;
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
-- 
2.27.0