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

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

[Jiawen Wu]

mplement 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.

v8 -> v9:
- rename swnode property for specific I2C platform device
- add ".fast_io = true" for I2C regmap
- use raw_spinlock_t for GPIO reg lock and adjust its position
- remove redundant txgbe->mdiodev
- keep reverse x-mass tree order
- other minor style changes

v7 -> v8:
- use macro defined I2C FIFO depth instead of magic number
- fix return code of clock create failure
- add spinlock for writing GPIO registers
- implement triggering GPIO interrupts for both-edge type
- remove the condition that enables interrupts
- add mii bus check for PCS device
- other minor style changes

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      |   4 +
 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  |   4 +
 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    | 678 ++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_phy.h    |  10 +
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  89 +++
 drivers/net/pcs/pcs-xpcs.c                    |  30 +
 include/linux/pcs/pcs-xpcs.h                  |   1 +
 15 files changed, 995 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 v9 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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 cbe7f184b50e..f59066d7375c 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..be4b5ad74a3c
--- /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("wx,i2c-snps-model");
+	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 v9 1/9] net: txgbe: Add software nodes to support phylink

[Andy Shevchenko]

+Cc Hans (see below)

On Wed, May 24, 2023 at 05:17:14PM +0800, Jiawen Wu wrote:
> Register software nodes for GPIO, I2C, SFP and PHYLINK. Define the
> device properties.

...

> +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;

These 4 lines can be as simple as

	return ret;

> +}

...

> +#define NODE_PROP(_NAME, _PROP)			\
> +	(const struct software_node) {		\
> +		.name = _NAME,			\
> +		.properties = _PROP,		\
> +	}

Looking at the amount of drivers that want this, I would declare it in the
property.h with SOFTWARE_NODE_PROPERTY name. I'll Ack that.

Hans, what do you think?

-- 
With Best Regards,
Andy Shevchenko

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

[Jiawen Wu]

On Saturday, May 27, 2023 4:44 PM, Andy Shevchenko wrote:
> +Cc Hans (see below)
> 
> On Wed, May 24, 2023 at 05:17:14PM +0800, Jiawen Wu wrote:
> > Register software nodes for GPIO, I2C, SFP and PHYLINK. Define the
> > device properties.
> 
> ...
> 
> > +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;
> 
> These 4 lines can be as simple as
> 
> 	return ret;

This function is going to be extended with later patches, is it necessary to
simply it here?

> 
> > +}
> 
> ...
> 
> > +#define NODE_PROP(_NAME, _PROP)			\
> > +	(const struct software_node) {		\
> > +		.name = _NAME,			\
> > +		.properties = _PROP,		\
> > +	}
> 
> Looking at the amount of drivers that want this, I would declare it in the
> property.h with SOFTWARE_NODE_PROPERTY name. I'll Ack that.
> 
> Hans, what do you think?
 

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

[andy.shevchenko]

Tue, May 30, 2023 at 02:11:08PM +0800, Jiawen Wu kirjoitti:
> On Saturday, May 27, 2023 4:44 PM, Andy Shevchenko wrote:
> > On Wed, May 24, 2023 at 05:17:14PM +0800, Jiawen Wu wrote:

...

> > > +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;
> > 
> > These 4 lines can be as simple as
> > 
> > 	return ret;
> 
> This function is going to be extended with later patches, is it necessary to
> simply it here?

Nope, thank you for elaboration.

> > > +}

-- 
With Best Regards,
Andy Shevchenko

[PATCH net-next v9 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 "wx,i2c-snps-model" 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>
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>
---
 drivers/i2c/busses/i2c-designware-common.c  |  8 ++
 drivers/i2c/busses/i2c-designware-core.h    |  4 +
 drivers/i2c/busses/i2c-designware-master.c  | 89 +++++++++++++++++++--
 drivers/i2c/busses/i2c-designware-platdrv.c | 15 ++++
 4 files changed, 111 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 0dc6b1ce663f..cdd8c67d9129 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 = TXGBE_TX_FIFO_DEPTH;
+		dev->rx_fifo_depth = TXGBE_RX_FIFO_DEPTH;
+
+		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..782532c20bd1 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)
 
 /*
@@ -312,6 +313,9 @@ struct dw_i2c_dev {
 #define AMD_UCSI_INTR_REG			0x474
 #define AMD_UCSI_INTR_EN			0xd
 
+#define TXGBE_TX_FIFO_DEPTH			4
+#define TXGBE_RX_FIFO_DEPTH			0
+
 struct i2c_dw_semaphore_callbacks {
 	int	(*probe)(struct dw_i2c_dev *dev);
 	void	(*remove)(struct dw_i2c_dev *dev);
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..f5751d608048 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, "wx,i2c-snps-model"))
+		dev->flags |= MODEL_WANGXUN_SP;
+
 	dev->dev = &pdev->dev;
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
-- 
2.27.0

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

[Andy Shevchenko]

On Wed, May 24, 2023 at 05:17:15PM +0800, Jiawen Wu wrote:
> Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> with SFP.
> 
> Introduce the property "wx,i2c-snps-model" 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.

Looks better, thank you!
My comments below.

...

> +	if (device_property_present(&pdev->dev, "wx,i2c-snps-model"))

Assuming people are fine with this, I have no objection on the name.

> +		dev->flags |= MODEL_WANGXUN_SP;

You probably has to clear the model in dev_flags, but here still a question
which one should have a priority.

-- 
With Best Regards,
Andy Shevchenko

[PATCH net-next v9 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 be4b5ad74a3c..06506cfb8d06 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/clk-provider.h>
+#include <linux/clkdev.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 -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;
+
+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 v9 3/9] net: txgbe: Register fixed rate clock

[Andrew Lunn]

On Wed, May 24, 2023 at 05:17:16PM +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>

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

    Andrew

[PATCH net-next v9 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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |  2 +
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 70 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  4 ++
 3 files changed, 76 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 06506cfb8d06..6ea33e753df4 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -1,9 +1,11 @@
 // 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/clk-provider.h>
 #include <linux/clkdev.h>
+#include <linux/regmap.h>
 #include <linux/i2c.h>
 #include <linux/pci.h>
 
@@ -98,6 +100,64 @@ 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,
+	.fast_io = true,
+};
+
+static int txgbe_i2c_register(struct txgbe *txgbe)
+{
+	struct platform_device_info info = {};
+	struct platform_device *i2c_dev;
+	struct regmap *i2c_regmap;
+	struct pci_dev *pdev;
+	struct wx *wx;
+
+	wx = txgbe->wx;
+	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 I2C 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;
+
+	info.res = &DEFINE_RES_IRQ(pdev->irq);
+	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 +174,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 +193,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 v9 4/9] net: txgbe: Register I2C platform device

[kernel test robot]

Hi Jiawen,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiawen-Wu/net-txgbe-Add-software-nodes-to-support-phylink/20230524-173221
base:   net-next/main
patch link:    https://lore.kernel.org/r/20230524091722.522118-5-jiawenwu%40trustnetic.com
patch subject: [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device
config: csky-randconfig-r003-20230525 (https://download.01.org/0day-ci/archive/20230526/202305261703.MVtcjtyn-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c1ecc61d2d3c17aeaa4992b8f30a2ddca4eebe83
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jiawen-Wu/net-txgbe-Add-software-nodes-to-support-phylink/20230524-173221
        git checkout c1ecc61d2d3c17aeaa4992b8f30a2ddca4eebe83
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/i2c/busses/ drivers/net/phy/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305261703.MVtcjtyn-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-designware-platdrv.c: In function 'dw_i2c_plat_probe':
>> drivers/i2c/busses/i2c-designware-platdrv.c:310:9: error: implicit declaration of function 'i2c_parse_fw_timings' [-Werror=implicit-function-declaration]
     310 |         i2c_parse_fw_timings(&pdev->dev, t, false);
         |         ^~~~~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-designware-platdrv.c: In function 'dw_i2c_plat_remove':
>> drivers/i2c/busses/i2c-designware-platdrv.c:408:9: error: implicit declaration of function 'i2c_del_adapter'; did you mean 'i2c_verify_adapter'? [-Werror=implicit-function-declaration]
     408 |         i2c_del_adapter(&dev->adapter);
         |         ^~~~~~~~~~~~~~~
         |         i2c_verify_adapter
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
   Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
   Selected by [y]:
   - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]


vim +/i2c_parse_fw_timings +310 drivers/i2c/busses/i2c-designware-platdrv.c

78d5e9e299e31b Jan Dabros         2022-02-08  275  
6ad6fde3970c98 Jarkko Nikula      2015-08-31  276  static int dw_i2c_plat_probe(struct platform_device *pdev)
2373f6b9744d53 Dirk Brandewie     2011-10-29  277  {
2373f6b9744d53 Dirk Brandewie     2011-10-29  278  	struct i2c_adapter *adap;
e393f674c5fedc Luis Oliveira      2017-06-14  279  	struct dw_i2c_dev *dev;
e3ea52b578be22 Andy Shevchenko    2018-07-25  280  	struct i2c_timings *t;
f9288fcc5c6154 Andy Shevchenko    2020-05-19  281  	int irq, ret;
2373f6b9744d53 Dirk Brandewie     2011-10-29  282  
2373f6b9744d53 Dirk Brandewie     2011-10-29  283  	irq = platform_get_irq(pdev, 0);
b20d386485e259 Alexey Brodkin     2015-03-09  284  	if (irq < 0)
b20d386485e259 Alexey Brodkin     2015-03-09  285  		return irq;
2373f6b9744d53 Dirk Brandewie     2011-10-29  286  
1cb715ca46946b Andy Shevchenko    2013-04-10  287  	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
1cb715ca46946b Andy Shevchenko    2013-04-10  288  	if (!dev)
1cb715ca46946b Andy Shevchenko    2013-04-10  289  		return -ENOMEM;
2373f6b9744d53 Dirk Brandewie     2011-10-29  290  
fac25d7aaa03c4 Serge Semin        2020-05-28  291  	dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
b91dbec563d413 Jiawen Wu          2023-05-24  292  	if (device_property_present(&pdev->dev, "wx,i2c-snps-model"))
b91dbec563d413 Jiawen Wu          2023-05-24  293  		dev->flags |= MODEL_WANGXUN_SP;
b91dbec563d413 Jiawen Wu          2023-05-24  294  
1cb715ca46946b Andy Shevchenko    2013-04-10  295  	dev->dev = &pdev->dev;
2373f6b9744d53 Dirk Brandewie     2011-10-29  296  	dev->irq = irq;
2373f6b9744d53 Dirk Brandewie     2011-10-29  297  	platform_set_drvdata(pdev, dev);
2373f6b9744d53 Dirk Brandewie     2011-10-29  298  
b7c3d0777808cd Serge Semin        2020-05-28  299  	ret = dw_i2c_plat_request_regs(dev);
b7c3d0777808cd Serge Semin        2020-05-28  300  	if (ret)
b7c3d0777808cd Serge Semin        2020-05-28  301  		return ret;
b7c3d0777808cd Serge Semin        2020-05-28  302  
ab809fd81fde3d Zhangfei Gao       2016-12-27  303  	dev->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
a6af48ec0712a0 Andy Shevchenko    2019-08-19  304  	if (IS_ERR(dev->rst))
a6af48ec0712a0 Andy Shevchenko    2019-08-19  305  		return PTR_ERR(dev->rst);
a6af48ec0712a0 Andy Shevchenko    2019-08-19  306  
ab809fd81fde3d Zhangfei Gao       2016-12-27  307  	reset_control_deassert(dev->rst);
ab809fd81fde3d Zhangfei Gao       2016-12-27  308  
e3ea52b578be22 Andy Shevchenko    2018-07-25  309  	t = &dev->timings;
e3ea52b578be22 Andy Shevchenko    2018-07-25 @310  	i2c_parse_fw_timings(&pdev->dev, t, false);
8e5f6b2a289c43 Romain Baeriswyl   2014-08-20  311  
852f71942ce71f Andy Shevchenko    2020-06-23  312  	i2c_dw_adjust_bus_speed(dev);
1732c22abca8f4 Alexandre Belloni  2018-08-31  313  
1bb39959623b43 Alexandre Belloni  2018-08-31  314  	if (pdev->dev.of_node)
1bb39959623b43 Alexandre Belloni  2018-08-31  315  		dw_i2c_of_configure(pdev);
1bb39959623b43 Alexandre Belloni  2018-08-31  316  
4c5301abbf81f4 Mika Westerberg    2015-11-30  317  	if (has_acpi_companion(&pdev->dev))
f9288fcc5c6154 Andy Shevchenko    2020-05-19  318  		i2c_dw_acpi_configure(&pdev->dev);
4c5301abbf81f4 Mika Westerberg    2015-11-30  319  
20ee1d9020c923 Andy Shevchenko    2020-05-19  320  	ret = i2c_dw_validate_speed(dev);
20ee1d9020c923 Andy Shevchenko    2020-05-19  321  	if (ret)
ab809fd81fde3d Zhangfei Gao       2016-12-27  322  		goto exit_reset;
9803f868944e87 Christian Ruppert  2013-06-26  323  
e393f674c5fedc Luis Oliveira      2017-06-14  324  	ret = i2c_dw_probe_lock_support(dev);
e393f674c5fedc Luis Oliveira      2017-06-14  325  	if (ret)
ab809fd81fde3d Zhangfei Gao       2016-12-27  326  		goto exit_reset;
894acb2f823b13 David Box          2015-01-15  327  
3ebe40ed1c3901 Andy Shevchenko    2020-04-25  328  	i2c_dw_configure(dev);
2fa8326b4b1e5f Dirk Brandewie     2011-10-06  329  
c62ebb3d5f0d0e Phil Edworthy      2019-02-28  330  	/* Optional interface clock */
c62ebb3d5f0d0e Phil Edworthy      2019-02-28  331  	dev->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
71dc297ca9ab63 Andy Shevchenko    2019-08-19  332  	if (IS_ERR(dev->pclk)) {
71dc297ca9ab63 Andy Shevchenko    2019-08-19  333  		ret = PTR_ERR(dev->pclk);
71dc297ca9ab63 Andy Shevchenko    2019-08-19  334  		goto exit_reset;
71dc297ca9ab63 Andy Shevchenko    2019-08-19  335  	}
c62ebb3d5f0d0e Phil Edworthy      2019-02-28  336  
27071b5cbca59d Serge Semin        2022-06-10  337  	dev->clk = devm_clk_get_optional(&pdev->dev, NULL);
27071b5cbca59d Serge Semin        2022-06-10  338  	if (IS_ERR(dev->clk)) {
27071b5cbca59d Serge Semin        2022-06-10  339  		ret = PTR_ERR(dev->clk);
27071b5cbca59d Serge Semin        2022-06-10  340  		goto exit_reset;
27071b5cbca59d Serge Semin        2022-06-10  341  	}
27071b5cbca59d Serge Semin        2022-06-10  342  
27071b5cbca59d Serge Semin        2022-06-10  343  	ret = i2c_dw_prepare_clk(dev, true);
27071b5cbca59d Serge Semin        2022-06-10  344  	if (ret)
27071b5cbca59d Serge Semin        2022-06-10  345  		goto exit_reset;
27071b5cbca59d Serge Semin        2022-06-10  346  
27071b5cbca59d Serge Semin        2022-06-10  347  	if (dev->clk) {
e3ea52b578be22 Andy Shevchenko    2018-07-25  348  		u64 clk_khz;
e3ea52b578be22 Andy Shevchenko    2018-07-25  349  
925ddb240d6c76 Mika Westerberg    2014-09-30  350  		dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
e3ea52b578be22 Andy Shevchenko    2018-07-25  351  		clk_khz = dev->get_clk_rate_khz(dev);
925ddb240d6c76 Mika Westerberg    2014-09-30  352  
e3ea52b578be22 Andy Shevchenko    2018-07-25  353  		if (!dev->sda_hold_time && t->sda_hold_ns)
e3ea52b578be22 Andy Shevchenko    2018-07-25  354  			dev->sda_hold_time =
c045214a0f31dd Andy Shevchenko    2021-07-12  355  				DIV_S64_ROUND_CLOSEST(clk_khz * t->sda_hold_ns, MICRO);
925ddb240d6c76 Mika Westerberg    2014-09-30  356  	}
925ddb240d6c76 Mika Westerberg    2014-09-30  357  
2373f6b9744d53 Dirk Brandewie     2011-10-29  358  	adap = &dev->adapter;
2373f6b9744d53 Dirk Brandewie     2011-10-29  359  	adap->owner = THIS_MODULE;
db2a8b6f1df93d Ricardo Ribalda    2020-07-02  360  	adap->class = dmi_check_system(dw_i2c_hwmon_class_dmi) ?
db2a8b6f1df93d Ricardo Ribalda    2020-07-02  361  					I2C_CLASS_HWMON : I2C_CLASS_DEPRECATED;
8eb5c87a92c065 Dustin Byford      2015-10-23  362  	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
af71100c7acf3c Rob Herring        2011-11-08  363  	adap->dev.of_node = pdev->dev.of_node;
77f3381a83c2f6 Hans de Goede      2019-03-12  364  	adap->nr = -1;
2373f6b9744d53 Dirk Brandewie     2011-10-29  365  
d79294d0de12dd Hans de Goede      2020-04-07  366  	if (dev->flags & ACCESS_NO_IRQ_SUSPEND) {
d79294d0de12dd Hans de Goede      2020-04-07  367  		dev_pm_set_driver_flags(&pdev->dev,
75507a319876ab Richard Fitzgerald 2022-12-19  368  					DPM_FLAG_SMART_PREPARE);
d79294d0de12dd Hans de Goede      2020-04-07  369  	} else {
02e45646d53bdb Rafael J. Wysocki  2018-01-03  370  		dev_pm_set_driver_flags(&pdev->dev,
02e45646d53bdb Rafael J. Wysocki  2018-01-03  371  					DPM_FLAG_SMART_PREPARE |
75507a319876ab Richard Fitzgerald 2022-12-19  372  					DPM_FLAG_SMART_SUSPEND);
d79294d0de12dd Hans de Goede      2020-04-07  373  	}
422cb781e0d0f8 Rafael J. Wysocki  2018-01-03  374  
7c5b3c158b38dc Rajat Jain         2021-10-25  375  	device_enable_async_suspend(&pdev->dev);
7c5b3c158b38dc Rajat Jain         2021-10-25  376  
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  377  	/* The code below assumes runtime PM to be disabled. */
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  378  	WARN_ON(pm_runtime_enabled(&pdev->dev));
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  379  
43452335224bc0 Mika Westerberg    2013-04-10  380  	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
43452335224bc0 Mika Westerberg    2013-04-10  381  	pm_runtime_use_autosuspend(&pdev->dev);
7272194ed391f9 Mika Westerberg    2013-01-17  382  	pm_runtime_set_active(&pdev->dev);
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  383  
9cbeeca05049b1 Hans de Goede      2018-09-05  384  	if (dev->shared_with_punit)
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  385  		pm_runtime_get_noresume(&pdev->dev);
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  386  
7272194ed391f9 Mika Westerberg    2013-01-17  387  	pm_runtime_enable(&pdev->dev);
7272194ed391f9 Mika Westerberg    2013-01-17  388  
e393f674c5fedc Luis Oliveira      2017-06-14  389  	ret = i2c_dw_probe(dev);
e393f674c5fedc Luis Oliveira      2017-06-14  390  	if (ret)
ab809fd81fde3d Zhangfei Gao       2016-12-27  391  		goto exit_probe;
ab809fd81fde3d Zhangfei Gao       2016-12-27  392  
e393f674c5fedc Luis Oliveira      2017-06-14  393  	return ret;
36d48fb5766aee Wolfram Sang       2015-10-09  394  
ab809fd81fde3d Zhangfei Gao       2016-12-27  395  exit_probe:
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  396  	dw_i2c_plat_pm_cleanup(dev);
ab809fd81fde3d Zhangfei Gao       2016-12-27  397  exit_reset:
ab809fd81fde3d Zhangfei Gao       2016-12-27  398  	reset_control_assert(dev->rst);
e393f674c5fedc Luis Oliveira      2017-06-14  399  	return ret;
2373f6b9744d53 Dirk Brandewie     2011-10-29  400  }
2373f6b9744d53 Dirk Brandewie     2011-10-29  401  
6ad6fde3970c98 Jarkko Nikula      2015-08-31  402  static int dw_i2c_plat_remove(struct platform_device *pdev)
2373f6b9744d53 Dirk Brandewie     2011-10-29  403  {
2373f6b9744d53 Dirk Brandewie     2011-10-29  404  	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
2373f6b9744d53 Dirk Brandewie     2011-10-29  405  
7272194ed391f9 Mika Westerberg    2013-01-17  406  	pm_runtime_get_sync(&pdev->dev);
7272194ed391f9 Mika Westerberg    2013-01-17  407  
2373f6b9744d53 Dirk Brandewie     2011-10-29 @408  	i2c_del_adapter(&dev->adapter);
2373f6b9744d53 Dirk Brandewie     2011-10-29  409  
90312351fd1e47 Luis Oliveira      2017-06-14  410  	dev->disable(dev);
2373f6b9744d53 Dirk Brandewie     2011-10-29  411  
edfc39012364a6 Mika Westerberg    2015-06-17  412  	pm_runtime_dont_use_autosuspend(&pdev->dev);
edfc39012364a6 Mika Westerberg    2015-06-17  413  	pm_runtime_put_sync(&pdev->dev);
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  414  	dw_i2c_plat_pm_cleanup(dev);
126dbc6b49c867 Rafael J. Wysocki  2017-09-25  415  
78d5e9e299e31b Jan Dabros         2022-02-08  416  	i2c_dw_remove_lock_support(dev);
78d5e9e299e31b Jan Dabros         2022-02-08  417  
ab809fd81fde3d Zhangfei Gao       2016-12-27  418  	reset_control_assert(dev->rst);
7272194ed391f9 Mika Westerberg    2013-01-17  419  
2373f6b9744d53 Dirk Brandewie     2011-10-29  420  	return 0;
2373f6b9744d53 Dirk Brandewie     2011-10-29  421  }
2373f6b9744d53 Dirk Brandewie     2011-10-29  422  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Andy Shevchenko]

On Wed, May 24, 2023 at 05:17:17PM +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.

...

> +#include <linux/platform_device.h>

Can this be ordered (to some extent), please?

>  #include <linux/gpio/property.h>
>  #include <linux/clk-provider.h>
>  #include <linux/clkdev.h>
> +#include <linux/regmap.h>

This too.

>  #include <linux/i2c.h>
>  #include <linux/pci.h>

Somewhere here...

...

Otherwise looks good, thank you.

-- 
With Best Regards,
Andy Shevchenko

[PATCH net-next v9 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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.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 6ea33e753df4..3da5f5538f34 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -158,6 +158,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;
@@ -180,8 +199,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);
@@ -193,6 +220,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 v9 5/9] net: txgbe: Add SFP module identify

[kernel test robot]

Hi Jiawen,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiawen-Wu/net-txgbe-Add-software-nodes-to-support-phylink/20230524-173221
base:   net-next/main
patch link:    https://lore.kernel.org/r/20230524091722.522118-6-jiawenwu%40trustnetic.com
patch subject: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
config: csky-randconfig-r003-20230525 (https://download.01.org/0day-ci/archive/20230526/202305261959.mnGUW17n-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c382745a6443e8ff9b3fab9b10c90b216b2ca59b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jiawen-Wu/net-txgbe-Add-software-nodes-to-support-phylink/20230524-173221
        git checkout c382745a6443e8ff9b3fab9b10c90b216b2ca59b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/net/phy/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305261959.mnGUW17n-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/net/phy/sfp.c: In function 'sfp_i2c_read':
>> drivers/net/phy/sfp.c:609:23: error: implicit declaration of function 'i2c_transfer' [-Werror=implicit-function-declaration]
     609 |                 ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
         |                       ^~~~~~~~~~~~
   drivers/net/phy/sfp.c: In function 'sfp_i2c_configure':
>> drivers/net/phy/sfp.c:653:14: error: implicit declaration of function 'i2c_check_functionality' [-Werror=implicit-function-declaration]
     653 |         if (!i2c_check_functionality(i2c, I2C_FUNC_I2C))
         |              ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/sfp.c: In function 'sfp_cleanup':
>> drivers/net/phy/sfp.c:2919:17: error: implicit declaration of function 'i2c_put_adapter' [-Werror=implicit-function-declaration]
    2919 |                 i2c_put_adapter(sfp->i2c);
         |                 ^~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
   Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
   Selected by [y]:
   - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
   WARNING: unmet direct dependencies detected for SFP
   Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
   Selected by [y]:
   - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]


vim +/i2c_transfer +609 drivers/net/phy/sfp.c

73970055450eeb Russell King  2017-07-25  583  
3bb35261c74e39 Jon Nettleton 2018-02-27  584  static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
3bb35261c74e39 Jon Nettleton 2018-02-27  585  			size_t len)
73970055450eeb Russell King  2017-07-25  586  {
73970055450eeb Russell King  2017-07-25  587  	struct i2c_msg msgs[2];
426c6cbc409cbd Pali Rohár    2021-01-25  588  	u8 bus_addr = a2 ? 0x51 : 0x50;
426c6cbc409cbd Pali Rohár    2021-01-25  589  	size_t block_size = sfp->i2c_block_size;
28e74a7cfd6403 Russell King  2019-06-02  590  	size_t this_len;
73970055450eeb Russell King  2017-07-25  591  	int ret;
73970055450eeb Russell King  2017-07-25  592  
73970055450eeb Russell King  2017-07-25  593  	msgs[0].addr = bus_addr;
73970055450eeb Russell King  2017-07-25  594  	msgs[0].flags = 0;
73970055450eeb Russell King  2017-07-25  595  	msgs[0].len = 1;
73970055450eeb Russell King  2017-07-25  596  	msgs[0].buf = &dev_addr;
73970055450eeb Russell King  2017-07-25  597  	msgs[1].addr = bus_addr;
73970055450eeb Russell King  2017-07-25  598  	msgs[1].flags = I2C_M_RD;
73970055450eeb Russell King  2017-07-25  599  	msgs[1].len = len;
73970055450eeb Russell King  2017-07-25  600  	msgs[1].buf = buf;
73970055450eeb Russell King  2017-07-25  601  
28e74a7cfd6403 Russell King  2019-06-02  602  	while (len) {
28e74a7cfd6403 Russell King  2019-06-02  603  		this_len = len;
0d035bed2a4a6c Russell King  2020-12-09  604  		if (this_len > block_size)
0d035bed2a4a6c Russell King  2020-12-09  605  			this_len = block_size;
28e74a7cfd6403 Russell King  2019-06-02  606  
28e74a7cfd6403 Russell King  2019-06-02  607  		msgs[1].len = this_len;
28e74a7cfd6403 Russell King  2019-06-02  608  
3bb35261c74e39 Jon Nettleton 2018-02-27 @609  		ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
73970055450eeb Russell King  2017-07-25  610  		if (ret < 0)
73970055450eeb Russell King  2017-07-25  611  			return ret;
73970055450eeb Russell King  2017-07-25  612  
28e74a7cfd6403 Russell King  2019-06-02  613  		if (ret != ARRAY_SIZE(msgs))
28e74a7cfd6403 Russell King  2019-06-02  614  			break;
28e74a7cfd6403 Russell King  2019-06-02  615  
28e74a7cfd6403 Russell King  2019-06-02  616  		msgs[1].buf += this_len;
28e74a7cfd6403 Russell King  2019-06-02  617  		dev_addr += this_len;
28e74a7cfd6403 Russell King  2019-06-02  618  		len -= this_len;
28e74a7cfd6403 Russell King  2019-06-02  619  	}
28e74a7cfd6403 Russell King  2019-06-02  620  
28e74a7cfd6403 Russell King  2019-06-02  621  	return msgs[1].buf - (u8 *)buf;
73970055450eeb Russell King  2017-07-25  622  }
73970055450eeb Russell King  2017-07-25  623  
3bb35261c74e39 Jon Nettleton 2018-02-27  624  static int sfp_i2c_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
73970055450eeb Russell King  2017-07-25  625  	size_t len)
73970055450eeb Russell King  2017-07-25  626  {
3bb35261c74e39 Jon Nettleton 2018-02-27  627  	struct i2c_msg msgs[1];
3bb35261c74e39 Jon Nettleton 2018-02-27  628  	u8 bus_addr = a2 ? 0x51 : 0x50;
3bb35261c74e39 Jon Nettleton 2018-02-27  629  	int ret;
3bb35261c74e39 Jon Nettleton 2018-02-27  630  
3bb35261c74e39 Jon Nettleton 2018-02-27  631  	msgs[0].addr = bus_addr;
3bb35261c74e39 Jon Nettleton 2018-02-27  632  	msgs[0].flags = 0;
3bb35261c74e39 Jon Nettleton 2018-02-27  633  	msgs[0].len = 1 + len;
3bb35261c74e39 Jon Nettleton 2018-02-27  634  	msgs[0].buf = kmalloc(1 + len, GFP_KERNEL);
3bb35261c74e39 Jon Nettleton 2018-02-27  635  	if (!msgs[0].buf)
3bb35261c74e39 Jon Nettleton 2018-02-27  636  		return -ENOMEM;
3bb35261c74e39 Jon Nettleton 2018-02-27  637  
3bb35261c74e39 Jon Nettleton 2018-02-27  638  	msgs[0].buf[0] = dev_addr;
3bb35261c74e39 Jon Nettleton 2018-02-27  639  	memcpy(&msgs[0].buf[1], buf, len);
3bb35261c74e39 Jon Nettleton 2018-02-27  640  
3bb35261c74e39 Jon Nettleton 2018-02-27  641  	ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
3bb35261c74e39 Jon Nettleton 2018-02-27  642  
3bb35261c74e39 Jon Nettleton 2018-02-27  643  	kfree(msgs[0].buf);
3bb35261c74e39 Jon Nettleton 2018-02-27  644  
3bb35261c74e39 Jon Nettleton 2018-02-27  645  	if (ret < 0)
3bb35261c74e39 Jon Nettleton 2018-02-27  646  		return ret;
3bb35261c74e39 Jon Nettleton 2018-02-27  647  
3bb35261c74e39 Jon Nettleton 2018-02-27  648  	return ret == ARRAY_SIZE(msgs) ? len : 0;
73970055450eeb Russell King  2017-07-25  649  }
73970055450eeb Russell King  2017-07-25  650  
73970055450eeb Russell King  2017-07-25  651  static int sfp_i2c_configure(struct sfp *sfp, struct i2c_adapter *i2c)
73970055450eeb Russell King  2017-07-25  652  {
73970055450eeb Russell King  2017-07-25 @653  	if (!i2c_check_functionality(i2c, I2C_FUNC_I2C))
73970055450eeb Russell King  2017-07-25  654  		return -EINVAL;
73970055450eeb Russell King  2017-07-25  655  
73970055450eeb Russell King  2017-07-25  656  	sfp->i2c = i2c;
73970055450eeb Russell King  2017-07-25  657  	sfp->read = sfp_i2c_read;
3bb35261c74e39 Jon Nettleton 2018-02-27  658  	sfp->write = sfp_i2c_write;
73970055450eeb Russell King  2017-07-25  659  
e85b1347ace677 Marek Behún   2022-09-30  660  	return 0;
e85b1347ace677 Marek Behún   2022-09-30  661  }
e85b1347ace677 Marek Behún   2022-09-30  662  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Russell King (Oracle)]

On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote:
> Kconfig warnings: (for reference only)
>    WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
>    Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
>    Selected by [y]:
>    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
>    WARNING: unmet direct dependencies detected for SFP
>    Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
>    Selected by [y]:
>    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]

... and is basically caused by "select SFP". No. Do not do this unless
you look at the dependencies for SFP and ensure that those are also
satisfied - because if you don't you create messes like the above
build errors.

-- 
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 v9 5/9] net: txgbe: Add SFP module identify

[Jiawen Wu]

On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote:
> On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote:
> > Kconfig warnings: (for reference only)
> >    WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
> >    Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
> >    Selected by [y]:
> >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> >    WARNING: unmet direct dependencies detected for SFP
> >    Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
> >    Selected by [y]:
> >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> 
> ... and is basically caused by "select SFP". No. Do not do this unless
> you look at the dependencies for SFP and ensure that those are also
> satisfied - because if you don't you create messes like the above
> build errors.

So how do I make sure that the module I need compiles and loads correctly,
rely on the user to manually select it?

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

[Jiawen Wu]

On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote:
> On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote:
> > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote:
> > > Kconfig warnings: (for reference only)
> > >    WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
> > >    Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
> > >    Selected by [y]:
> > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > >    WARNING: unmet direct dependencies detected for SFP
> > >    Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
> > >    Selected by [y]:
> > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> >
> > ... and is basically caused by "select SFP". No. Do not do this unless
> > you look at the dependencies for SFP and ensure that those are also
> > satisfied - because if you don't you create messes like the above
> > build errors.
> 
> So how do I make sure that the module I need compiles and loads correctly,
> rely on the user to manually select it?

When I changed the TXGBE config to:
...
	depends on SFP
	select PCS_XPCS
...
the compilation gave an error:

drivers/net/phy/Kconfig:16:error: recursive dependency detected!
drivers/net/phy/Kconfig:16:     symbol PHYLIB is selected by PHYLINK
drivers/net/phy/Kconfig:6:      symbol PHYLINK is selected by PCS_XPCS
drivers/net/pcs/Kconfig:8:      symbol PCS_XPCS is selected by TXGBE
drivers/net/ethernet/wangxun/Kconfig:40:        symbol TXGBE depends on SFP
drivers/net/phy/Kconfig:63:     symbol SFP depends on PHYLIB
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"

Seems deleting "depends on SFP" is the correct way. But is this normal?
How do we ensure the dependency between TXGBE and SFP?

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

[Jiawen Wu]

On Tuesday, May 30, 2023 4:41 PM, Jiawen Wu wrote:
> On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote:
> > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote:
> > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote:
> > > > Kconfig warnings: (for reference only)
> > > >    WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
> > > >    Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
> > > >    Selected by [y]:
> > > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > > >    WARNING: unmet direct dependencies detected for SFP
> > > >    Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
> > > >    Selected by [y]:
> > > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > >
> > > ... and is basically caused by "select SFP". No. Do not do this unless
> > > you look at the dependencies for SFP and ensure that those are also
> > > satisfied - because if you don't you create messes like the above
> > > build errors.
> >
> > So how do I make sure that the module I need compiles and loads correctly,
> > rely on the user to manually select it?
> 
> When I changed the TXGBE config to:
> ...
> 	depends on SFP
> 	select PCS_XPCS
> ...
> the compilation gave an error:
> 
> drivers/net/phy/Kconfig:16:error: recursive dependency detected!
> drivers/net/phy/Kconfig:16:     symbol PHYLIB is selected by PHYLINK
> drivers/net/phy/Kconfig:6:      symbol PHYLINK is selected by PCS_XPCS
> drivers/net/pcs/Kconfig:8:      symbol PCS_XPCS is selected by TXGBE
> drivers/net/ethernet/wangxun/Kconfig:40:        symbol TXGBE depends on SFP
> drivers/net/phy/Kconfig:63:     symbol SFP depends on PHYLIB
> For a resolution refer to Documentation/kbuild/kconfig-language.rst
> subsection "Kconfig recursive dependency limitations"
> 
> Seems deleting "depends on SFP" is the correct way. But is this normal?
> How do we ensure the dependency between TXGBE and SFP?

Hi Russell,

Could you please give me some suggestions?

I checked "kconfig-language" doc, the practical solution is that swap all
"select FOO" to "depends on FOO" or swap all "depends on FOO" to
"select FOO". Config PCS_XPCS has to be selected in order to load modules
properly, so how should I fix the warning?

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

[Russell King (Oracle)]

On Tue, May 30, 2023 at 04:40:36PM +0800, Jiawen Wu wrote:
> On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote:
> > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote:
> > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote:
> > > > Kconfig warnings: (for reference only)
> > > >    WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
> > > >    Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
> > > >    Selected by [y]:
> > > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > > >    WARNING: unmet direct dependencies detected for SFP
> > > >    Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
> > > >    Selected by [y]:
> > > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > >
> > > ... and is basically caused by "select SFP". No. Do not do this unless
> > > you look at the dependencies for SFP and ensure that those are also
> > > satisfied - because if you don't you create messes like the above
> > > build errors.
> > 
> > So how do I make sure that the module I need compiles and loads correctly,
> > rely on the user to manually select it?
> 
> When I changed the TXGBE config to:
> ...
> 	depends on SFP
> 	select PCS_XPCS
> ...
> the compilation gave an error:
> 
> drivers/net/phy/Kconfig:16:error: recursive dependency detected!
> drivers/net/phy/Kconfig:16:     symbol PHYLIB is selected by PHYLINK
> drivers/net/phy/Kconfig:6:      symbol PHYLINK is selected by PCS_XPCS
> drivers/net/pcs/Kconfig:8:      symbol PCS_XPCS is selected by TXGBE
> drivers/net/ethernet/wangxun/Kconfig:40:        symbol TXGBE depends on SFP
> drivers/net/phy/Kconfig:63:     symbol SFP depends on PHYLIB
> For a resolution refer to Documentation/kbuild/kconfig-language.rst
> subsection "Kconfig recursive dependency limitations"
> 
> Seems deleting "depends on SFP" is the correct way. But is this normal?
> How do we ensure the dependency between TXGBE and SFP?

First, I would do this:

	select PHYLINK
	select PCS_XPCS

but then I'm principled, and I don't agree that PCS_XPCS should be
selecting PHYLINK.

The second thing I don't particularly like is selecting user visible
symbols, but as I understand it, with TXGBE, the SFP slot is not an
optional feature, so there's little option.

So, because SFP requires I2C:

	select I2C
	select SFP

That is basically what I meant by "you look at the dependencies for
SFP and ensure that those are also satisfied".

Adding that "select I2C" also solves the unmet dependencies for
I2C_DESIGNWARE_PLATFORM.

However, even with that, we're not done with the evilness of select,
because there's one more permitted configuration combination that
will break.

If you build TXGBE into the kernel, that will force SFP=y, I2C=y,
PHYLINK=y, PHYLIB=y. So far so good. However, if HWMON=m, then things
will again break. So I would also suggest:

	select HWMON if TXGBE=y

even though you don't require it, it solves the build fallout from
where HWMON=m but you force SFP=y.

Maybe someone else has better ideas how to do this, but the above is
the best I can come up with.


IMHO, select is nothing but pure evil, and should be used with utmost
care and a full understanding of its ramifications, and a realisation
that it *totally* and *utterly* blows away any "depends on" on the
target of the select statement.

An option that states that it depends on something else generally does
because... oddly enough, it _depends_ on that other option. So, if
select forces an option on without its dependencies, then it's not
surprising that stuff fails to build.

Whenever a select statement is added, one must _always_ look at the
target symbol and consider any "depends on" there, and how to ensure
that those dependencies are guaranteed to always be satisfied.

-- 
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 v9 5/9] net: txgbe: Add SFP module identify

[Jiawen Wu]

On Wednesday, May 31, 2023 5:48 PM, Russell King (Oracle) wrote:
> On Tue, May 30, 2023 at 04:40:36PM +0800, Jiawen Wu wrote:
> > On Monday, May 29, 2023 10:06 AM, Jiawen Wu wrote:
> > > On Friday, May 26, 2023 7:37 PM, Russell King (Oracle) wrote:
> > > > On Fri, May 26, 2023 at 07:30:45PM +0800, kernel test robot wrote:
> > > > > Kconfig warnings: (for reference only)
> > > > >    WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
> > > > >    Depends on [n]: I2C [=n] && HAS_IOMEM [=y] && (ACPI && COMMON_CLK [=y] || !ACPI)
> > > > >    Selected by [y]:
> > > > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > > > >    WARNING: unmet direct dependencies detected for SFP
> > > > >    Depends on [n]: NETDEVICES [=y] && PHYLIB [=y] && I2C [=n] && PHYLINK [=y] && (HWMON [=n] || HWMON [=n]=n)
> > > > >    Selected by [y]:
> > > > >    - TXGBE [=y] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_WANGXUN [=y] && PCI [=y]
> > > >
> > > > ... and is basically caused by "select SFP". No. Do not do this unless
> > > > you look at the dependencies for SFP and ensure that those are also
> > > > satisfied - because if you don't you create messes like the above
> > > > build errors.
> > >
> > > So how do I make sure that the module I need compiles and loads correctly,
> > > rely on the user to manually select it?
> >
> > When I changed the TXGBE config to:
> > ...
> > 	depends on SFP
> > 	select PCS_XPCS
> > ...
> > the compilation gave an error:
> >
> > drivers/net/phy/Kconfig:16:error: recursive dependency detected!
> > drivers/net/phy/Kconfig:16:     symbol PHYLIB is selected by PHYLINK
> > drivers/net/phy/Kconfig:6:      symbol PHYLINK is selected by PCS_XPCS
> > drivers/net/pcs/Kconfig:8:      symbol PCS_XPCS is selected by TXGBE
> > drivers/net/ethernet/wangxun/Kconfig:40:        symbol TXGBE depends on SFP
> > drivers/net/phy/Kconfig:63:     symbol SFP depends on PHYLIB
> > For a resolution refer to Documentation/kbuild/kconfig-language.rst
> > subsection "Kconfig recursive dependency limitations"
> >
> > Seems deleting "depends on SFP" is the correct way. But is this normal?
> > How do we ensure the dependency between TXGBE and SFP?
> 
> First, I would do this:
> 
> 	select PHYLINK
> 	select PCS_XPCS
> 
> but then I'm principled, and I don't agree that PCS_XPCS should be
> selecting PHYLINK.
> 
> The second thing I don't particularly like is selecting user visible
> symbols, but as I understand it, with TXGBE, the SFP slot is not an
> optional feature, so there's little option.
> 
> So, because SFP requires I2C:
> 
> 	select I2C
> 	select SFP
> 
> That is basically what I meant by "you look at the dependencies for
> SFP and ensure that those are also satisfied".
> 
> Adding that "select I2C" also solves the unmet dependencies for
> I2C_DESIGNWARE_PLATFORM.
> 
> However, even with that, we're not done with the evilness of select,
> because there's one more permitted configuration combination that
> will break.
> 
> If you build TXGBE into the kernel, that will force SFP=y, I2C=y,
> PHYLINK=y, PHYLIB=y. So far so good. However, if HWMON=m, then things
> will again break. So I would also suggest:
> 
> 	select HWMON if TXGBE=y
> 
> even though you don't require it, it solves the build fallout from
> where HWMON=m but you force SFP=y.
> 
> Maybe someone else has better ideas how to do this, but the above is
> the best I can come up with.
> 
> 
> IMHO, select is nothing but pure evil, and should be used with utmost
> care and a full understanding of its ramifications, and a realisation
> that it *totally* and *utterly* blows away any "depends on" on the
> target of the select statement.
> 
> An option that states that it depends on something else generally does
> because... oddly enough, it _depends_ on that other option. So, if
> select forces an option on without its dependencies, then it's not
> surprising that stuff fails to build.
> 
> Whenever a select statement is added, one must _always_ look at the
> target symbol and consider any "depends on" there, and how to ensure
> that those dependencies are guaranteed to always be satisfied.

Thanks for the detailed explanation. I'll check each of the required options,
and use "depends on" whenever possible.

[PATCH net-next v9 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  |   3 +
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  20 +-
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 248 ++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  23 ++
 6 files changed, 280 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 f59066d7375c..297c389fa890 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 */
@@ -643,6 +645,7 @@ struct wx {
 	bool wol_enabled;
 	bool ncsi_enabled;
 	bool gpio_ctrl;
+	raw_spinlock_t gpio_lock;
 
 	/* Tx fast path data */
 	int num_tx_queues;
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 3da5f5538f34..6aba22a4fc5e 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/platform_device.h>
+#include <linux/gpio/machine.h>
+#include <linux/gpio/driver.h>
 #include <linux/gpio/property.h>
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
@@ -10,6 +12,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 +77,245 @@ 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);
+	int val;
+
+	val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
+
+	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);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+	wr32m(wx, WX_GPIO_DDR, BIT(offset), 0);
+	raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+
+	return 0;
+}
+
+static int txgbe_gpio_direction_out(struct gpio_chip *chip, unsigned int offset,
+				    int val)
+{
+	struct wx *wx = gpiochip_get_data(chip);
+	unsigned long flags;
+	u32 set;
+
+	set = val ? BIT(offset) : 0;
+
+	raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+	wr32m(wx, WX_GPIO_DR, BIT(offset), set);
+	wr32m(wx, WX_GPIO_DDR, BIT(offset), BIT(offset));
+	raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+
+	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);
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+	wr32(wx, WX_GPIO_EOI, BIT(hwirq));
+	raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+}
+
+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);
+	unsigned long flags;
+
+	gpiochip_disable_irq(gc, hwirq);
+
+	raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+	wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), BIT(hwirq));
+	raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+}
+
+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);
+	unsigned long flags;
+
+	gpiochip_enable_irq(gc, hwirq);
+
+	raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+	wr32m(wx, WX_GPIO_INTMASK, BIT(hwirq), 0);
+	raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+}
+
+static void txgbe_toggle_trigger(struct gpio_chip *gc, unsigned int offset)
+{
+	struct wx *wx = gpiochip_get_data(gc);
+	u32 pol, val;
+
+	pol = rd32(wx, WX_GPIO_POLARITY);
+	val = rd32(wx, WX_GPIO_EXT);
+
+	if (val & BIT(offset))
+		pol &= ~BIT(offset);
+	else
+		pol |= BIT(offset);
+
+	wr32(wx, WX_GPIO_POLARITY, pol);
+}
+
+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, mask;
+	unsigned long flags;
+
+	mask = BIT(hwirq);
+
+	if (type & IRQ_TYPE_LEVEL_MASK) {
+		level = 0;
+		irq_set_handler_locked(d, handle_level_irq);
+	} else {
+		level = mask;
+		irq_set_handler_locked(d, handle_edge_irq);
+	}
+
+	if (type == IRQ_TYPE_EDGE_RISING || type == IRQ_TYPE_LEVEL_HIGH)
+		polarity = mask;
+	else
+		polarity = 0;
+
+	raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+
+	wr32m(wx, WX_GPIO_INTEN, mask, mask);
+	wr32m(wx, WX_GPIO_INTTYPE_LEVEL, mask, level);
+	if (type == IRQ_TYPE_EDGE_BOTH)
+		txgbe_toggle_trigger(gc, hwirq);
+	else
+		wr32m(wx, WX_GPIO_POLARITY, mask, polarity);
+
+	raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+
+	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;
+	unsigned long flags;
+
+	chained_irq_enter(chip, desc);
+
+	gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
+
+	gc = txgbe->gpio;
+	for_each_set_bit(hwirq, &gpioirq, gc->ngpio) {
+		int gpio = irq_find_mapping(gc->irq.domain, hwirq);
+		u32 irq_type = irq_get_trigger_type(gpio);
+
+		generic_handle_domain_irq(gc->irq.domain, hwirq);
+
+		if ((irq_type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH) {
+			raw_spin_lock_irqsave(&wx->gpio_lock, flags);
+			txgbe_toggle_trigger(gc, hwirq);
+			raw_spin_unlock_irqrestore(&wx->gpio_lock, flags);
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+
+	/* unmask interrupt */
+	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
+}
+
+static int txgbe_gpio_init(struct txgbe *txgbe)
+{
+	struct gpio_irq_chip *girq;
+	struct gpio_chip *gc;
+	struct device *dev;
+	struct wx *wx;
+	int ret;
+
+	wx = txgbe->wx;
+	dev = &wx->pdev->dev;
+
+	raw_spin_lock_init(&wx->gpio_lock);
+
+	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
+	if (!gc)
+		return -ENOMEM;
+
+	gc->label = devm_kasprintf(dev, GFP_KERNEL, "txgbe_gpio-%x",
+				   (wx->pdev->bus->number << 8) | wx->pdev->devfn);
+	gc->base = -1;
+	gc->ngpio = 6;
+	gc->owner = THIS_MODULE;
+	gc->parent = 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;
+
+	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(dev, girq->num_parents,
+				     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(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;
@@ -187,6 +429,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..6c903e4517c7 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -55,6 +55,28 @@
 #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 +175,7 @@ struct txgbe {
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
 	struct clk *clk;
+	struct gpio_chip *gpio;
 };
 
 #endif /* _TXGBE_TYPE_H_ */
-- 
2.27.0

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

[Andrew Lunn]

> +static void txgbe_toggle_trigger(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct wx *wx = gpiochip_get_data(gc);
> +	u32 pol, val;
> +
> +	pol = rd32(wx, WX_GPIO_POLARITY);
> +	val = rd32(wx, WX_GPIO_EXT);
> +
> +	if (val & BIT(offset))
> +		pol &= ~BIT(offset);
> +	else
> +		pol |= BIT(offset);
> +
> +	wr32(wx, WX_GPIO_POLARITY, pol);
> +}

So you look at the current state of the GPIO and set the polarity to
trigger an interrupt when it changes.

This is not race free. And if it does race, at best you loose an
interrupt. The worst is your hardware locks up because that interrupt
was missed and it cannot continue until some action is taken.

Is there any other GPIO driver doing this?

I think you would be better indicating you don't support
IRQ_TYPE_EDGE_BOTH.

	Andrew

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

[Russell King (Oracle)]

On Wed, May 24, 2023 at 02:51:25PM +0200, Andrew Lunn wrote:
> > +static void txgbe_toggle_trigger(struct gpio_chip *gc, unsigned int offset)
> > +{
> > +	struct wx *wx = gpiochip_get_data(gc);
> > +	u32 pol, val;
> > +
> > +	pol = rd32(wx, WX_GPIO_POLARITY);
> > +	val = rd32(wx, WX_GPIO_EXT);
> > +
> > +	if (val & BIT(offset))
> > +		pol &= ~BIT(offset);
> > +	else
> > +		pol |= BIT(offset);
> > +
> > +	wr32(wx, WX_GPIO_POLARITY, pol);
> > +}
> 
> So you look at the current state of the GPIO and set the polarity to
> trigger an interrupt when it changes.
> 
> This is not race free. And if it does race, at best you loose an
> interrupt. The worst is your hardware locks up because that interrupt
> was missed and it cannot continue until some action is taken.
> 
> Is there any other GPIO driver doing this?
> 
> I think you would be better indicating you don't support
> IRQ_TYPE_EDGE_BOTH.

... which will make the whole point of having interrupt support for this
driver moot, and the SFP code will then poll the GPIOs which will
probably more reliably detect changes.

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

[PATCH net-next v9 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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 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 72f25e778840..c9924d7ad783 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 (test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising)) {
 			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

[PATCH net-next v9 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    | 97 ++++++++++++++++++-
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  5 +
 3 files changed, 101 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 6aba22a4fc5e..d2a6f8ca78e7 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -8,6 +8,8 @@
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
 #include <linux/regmap.h>
+#include <linux/pcs/pcs-xpcs.h>
+#include <linux/mdio.h>
 #include <linux/i2c.h>
 #include <linux/pci.h>
 
@@ -77,6 +79,88 @@ 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;
+
+	if (addr)
+		return -EOPNOTSUPP;
+
+	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;
+
+	if (addr)
+		return -EOPNOTSUPP;
+
+	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 mii_bus *mii_bus;
+	struct dw_xpcs *xpcs;
+	struct pci_dev *pdev;
+	struct wx *wx;
+	int ret = 0;
+
+	wx = txgbe->wx;
+	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(xpcs)) {
+		mdio_device_free(mdiodev);
+		return PTR_ERR(xpcs);
+	}
+
+	txgbe->xpcs = xpcs;
+
+	return 0;
+}
+
 static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int offset)
 {
 	struct wx *wx = gpiochip_get_data(chip);
@@ -429,16 +513,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);
@@ -460,6 +550,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);
 
@@ -472,5 +564,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 6c903e4517c7..d1f62f62c28c 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -80,6 +80,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
 
@@ -171,6 +175,7 @@ struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	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 v9 8/9] net: txgbe: Implement phylink pcs

[Andrew Lunn]

On Wed, May 24, 2023 at 05:17:21PM +0800, Jiawen Wu wrote:
> 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>

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

    Andrew

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

[Jakub Kicinski]

On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> +	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(xpcs)) {
> +		mdio_device_free(mdiodev);
> +		return PTR_ERR(xpcs);
> +	}

How does the mdiodev get destroyed in case of success?
Seems like either freeing it in case of xpcs error is unnecessary 
or it needs to also be freed when xpcs is destroyed?

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

[Jiawen Wu]

On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote:
> On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> > +	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(xpcs)) {
> > +		mdio_device_free(mdiodev);
> > +		return PTR_ERR(xpcs);
> > +	}
> 
> How does the mdiodev get destroyed in case of success?
> Seems like either freeing it in case of xpcs error is unnecessary
> or it needs to also be freed when xpcs is destroyed?

When xpcs is destroyed, that means mdiodev is no longer needed.
I think there is no need to free mdiodev in case of xpcs error,
since devm_* function leads to free it.

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

[Russell King (Oracle)]

On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote:
> On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote:
> > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> > > +	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(xpcs)) {
> > > +		mdio_device_free(mdiodev);
> > > +		return PTR_ERR(xpcs);
> > > +	}
> > 
> > How does the mdiodev get destroyed in case of success?
> > Seems like either freeing it in case of xpcs error is unnecessary
> > or it needs to also be freed when xpcs is destroyed?
> 
> When xpcs is destroyed, that means mdiodev is no longer needed.
> I think there is no need to free mdiodev in case of xpcs error,
> since devm_* function leads to free it.

If you are relying on the devm-ness of devm_mdiobus_register() then
it won't. Although mdiobus_unregister() walks bus->mdio_map[], I
think you are assuming that the mdio device you've created in
mdio_device_create() will be in that array. MDIO devices only get
added to that array when mdiobus_register_device() has been called,
which must only be called from mdio_device_register().

Please arrange to call mdio_device_free() prior to destroying the
XPCS in every case.

-- 
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 v9 8/9] net: txgbe: Implement phylink pcs

[Jiawen Wu]

On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote:
> On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote:
> > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote:
> > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> > > > +	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(xpcs)) {
> > > > +		mdio_device_free(mdiodev);
> > > > +		return PTR_ERR(xpcs);
> > > > +	}
> > >
> > > How does the mdiodev get destroyed in case of success?
> > > Seems like either freeing it in case of xpcs error is unnecessary
> > > or it needs to also be freed when xpcs is destroyed?
> >
> > When xpcs is destroyed, that means mdiodev is no longer needed.
> > I think there is no need to free mdiodev in case of xpcs error,
> > since devm_* function leads to free it.
> 
> If you are relying on the devm-ness of devm_mdiobus_register() then
> it won't. Although mdiobus_unregister() walks bus->mdio_map[], I
> think you are assuming that the mdio device you've created in
> mdio_device_create() will be in that array. MDIO devices only get
> added to that array when mdiobus_register_device() has been called,
> which must only be called from mdio_device_register().
> 
> Please arrange to call mdio_device_free() prior to destroying the
> XPCS in every case.

Get it.

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

[Russell King (Oracle)]

On Fri, May 26, 2023 at 05:01:49PM +0800, Jiawen Wu wrote:
> On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote:
> > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote:
> > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote:
> > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> > > > > +	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(xpcs)) {
> > > > > +		mdio_device_free(mdiodev);
> > > > > +		return PTR_ERR(xpcs);
> > > > > +	}
> > > >
> > > > How does the mdiodev get destroyed in case of success?
> > > > Seems like either freeing it in case of xpcs error is unnecessary
> > > > or it needs to also be freed when xpcs is destroyed?
> > >
> > > When xpcs is destroyed, that means mdiodev is no longer needed.
> > > I think there is no need to free mdiodev in case of xpcs error,
> > > since devm_* function leads to free it.
> > 
> > If you are relying on the devm-ness of devm_mdiobus_register() then
> > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I
> > think you are assuming that the mdio device you've created in
> > mdio_device_create() will be in that array. MDIO devices only get
> > added to that array when mdiobus_register_device() has been called,
> > which must only be called from mdio_device_register().
> > 
> > Please arrange to call mdio_device_free() prior to destroying the
> > XPCS in every case.
> 
> Get it.

It seems this is becoming a pattern, so I think we need to solve it
differently. How about something like this, which means you only have
to care about calling xpcs_create_mdiodev() and xpcs_destroy() ?

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index b87c69c4cdd7..802222581feb 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
 	if (!xpcs)
 		return ERR_PTR(-ENOMEM);
 
+	mdio_device_get(mdiodev);
 	xpcs->mdiodev = mdiodev;
 
 	xpcs_id = xpcs_get_id(xpcs);
@@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
 	ret = -ENODEV;
 
 out:
+	mdio_device_put(mdiodev);
 	kfree(xpcs);
 
 	return ERR_PTR(ret);
@@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create);
 
 void xpcs_destroy(struct dw_xpcs *xpcs)
 {
+	mdio_device_put(mdiodev);
 	kfree(xpcs);
 }
 EXPORT_SYMBOL_GPL(xpcs_destroy);
 
+struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
+				    phy_interface_t interface)
+{
+	struct mdio_device *mdiodev;
+	struct dw_xpcs *xpcs;
+
+	mdiodev = mdio_device_create(bus, addr);
+	if (IS_ERR(mdiodev))
+		return ERR_CAST(mdiodev);
+
+	xpcs = xpcs_create(mdiodev, interface);
+
+	/* xpcs_create() has taken a refcount on the mdiodev if it was
+	 * successful. If xpcs_create() fails, this will free the mdio
+	 * device here. In any case, we don't need to hold our reference
+	 * anymore, and putting it here will allow mdio_device_put() in
+	 * xpcs_destroy() to automatically free the mdio device.
+	 */
+	mdio_device_put(mdiodev);
+
+	return xpcs;
+}
+EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
+
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 1d7d550bbf1a..537b62330c90 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv);
 void mdio_driver_unregister(struct mdio_driver *drv);
 int mdio_device_bus_match(struct device *dev, struct device_driver *drv);
 
+static inline void mdio_device_get(struct mdio_device *mdiodev)
+{
+	get_device(&mdiodev->dev);
+}
+
+static inline void mdio_device_put(struct mdio_device *mdiodev)
+{
+	mdio_device_free(mdiodev);
+}
+
 static inline bool mdio_phy_id_is_c45(int phy_id)
 {
 	return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK);

-- 
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 v9 8/9] net: txgbe: Implement phylink pcs

[Jiawen Wu]

On Friday, May 26, 2023 5:07 PM, Russell King (Oracle) wrote:
> On Fri, May 26, 2023 at 05:01:49PM +0800, Jiawen Wu wrote:
> > On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote:
> > > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote:
> > > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote:
> > > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> > > > > > +	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(xpcs)) {
> > > > > > +		mdio_device_free(mdiodev);
> > > > > > +		return PTR_ERR(xpcs);
> > > > > > +	}
> > > > >
> > > > > How does the mdiodev get destroyed in case of success?
> > > > > Seems like either freeing it in case of xpcs error is unnecessary
> > > > > or it needs to also be freed when xpcs is destroyed?
> > > >
> > > > When xpcs is destroyed, that means mdiodev is no longer needed.
> > > > I think there is no need to free mdiodev in case of xpcs error,
> > > > since devm_* function leads to free it.
> > >
> > > If you are relying on the devm-ness of devm_mdiobus_register() then
> > > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I
> > > think you are assuming that the mdio device you've created in
> > > mdio_device_create() will be in that array. MDIO devices only get
> > > added to that array when mdiobus_register_device() has been called,
> > > which must only be called from mdio_device_register().
> > >
> > > Please arrange to call mdio_device_free() prior to destroying the
> > > XPCS in every case.
> >
> > Get it.
> 
> It seems this is becoming a pattern, so I think we need to solve it
> differently. How about something like this, which means you only have
> to care about calling xpcs_create_mdiodev() and xpcs_destroy() ?
> 
> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index b87c69c4cdd7..802222581feb 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
> @@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
>  	if (!xpcs)
>  		return ERR_PTR(-ENOMEM);
> 
> +	mdio_device_get(mdiodev);
>  	xpcs->mdiodev = mdiodev;
> 
>  	xpcs_id = xpcs_get_id(xpcs);
> @@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
>  	ret = -ENODEV;
> 
>  out:
> +	mdio_device_put(mdiodev);
>  	kfree(xpcs);
> 
>  	return ERR_PTR(ret);
> @@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create);
> 
>  void xpcs_destroy(struct dw_xpcs *xpcs)
>  {
> +	mdio_device_put(mdiodev);
>  	kfree(xpcs);
>  }
>  EXPORT_SYMBOL_GPL(xpcs_destroy);
> 
> +struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
> +				    phy_interface_t interface)
> +{
> +	struct mdio_device *mdiodev;
> +	struct dw_xpcs *xpcs;
> +
> +	mdiodev = mdio_device_create(bus, addr);
> +	if (IS_ERR(mdiodev))
> +		return ERR_CAST(mdiodev);
> +
> +	xpcs = xpcs_create(mdiodev, interface);
> +
> +	/* xpcs_create() has taken a refcount on the mdiodev if it was
> +	 * successful. If xpcs_create() fails, this will free the mdio
> +	 * device here. In any case, we don't need to hold our reference
> +	 * anymore, and putting it here will allow mdio_device_put() in
> +	 * xpcs_destroy() to automatically free the mdio device.
> +	 */
> +	mdio_device_put(mdiodev);
> +
> +	return xpcs;
> +}
> +EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
> +
>  MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> index 1d7d550bbf1a..537b62330c90 100644
> --- a/include/linux/mdio.h
> +++ b/include/linux/mdio.h
> @@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv);
>  void mdio_driver_unregister(struct mdio_driver *drv);
>  int mdio_device_bus_match(struct device *dev, struct device_driver *drv);
> 
> +static inline void mdio_device_get(struct mdio_device *mdiodev)
> +{
> +	get_device(&mdiodev->dev);
> +}
> +
> +static inline void mdio_device_put(struct mdio_device *mdiodev)
> +{
> +	mdio_device_free(mdiodev);
> +}
> +
>  static inline bool mdio_phy_id_is_c45(int phy_id)
>  {
>  	return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK);

Looks great, it can eliminate to create mdiodev in the ethernet driver, this device
only be used in xpcs.

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

[Russell King (Oracle)]

On Fri, May 26, 2023 at 05:22:29PM +0800, Jiawen Wu wrote:
> On Friday, May 26, 2023 5:07 PM, Russell King (Oracle) wrote:
> > On Fri, May 26, 2023 at 05:01:49PM +0800, Jiawen Wu wrote:
> > > On Friday, May 26, 2023 4:43 PM, Russell King (Oracle) wrote:
> > > > On Fri, May 26, 2023 at 02:21:23PM +0800, Jiawen Wu wrote:
> > > > > On Friday, May 26, 2023 12:14 PM, Jakub Kicinski wrote:
> > > > > > On Wed, 24 May 2023 17:17:21 +0800 Jiawen Wu wrote:
> > > > > > > +	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(xpcs)) {
> > > > > > > +		mdio_device_free(mdiodev);
> > > > > > > +		return PTR_ERR(xpcs);
> > > > > > > +	}
> > > > > >
> > > > > > How does the mdiodev get destroyed in case of success?
> > > > > > Seems like either freeing it in case of xpcs error is unnecessary
> > > > > > or it needs to also be freed when xpcs is destroyed?
> > > > >
> > > > > When xpcs is destroyed, that means mdiodev is no longer needed.
> > > > > I think there is no need to free mdiodev in case of xpcs error,
> > > > > since devm_* function leads to free it.
> > > >
> > > > If you are relying on the devm-ness of devm_mdiobus_register() then
> > > > it won't. Although mdiobus_unregister() walks bus->mdio_map[], I
> > > > think you are assuming that the mdio device you've created in
> > > > mdio_device_create() will be in that array. MDIO devices only get
> > > > added to that array when mdiobus_register_device() has been called,
> > > > which must only be called from mdio_device_register().
> > > >
> > > > Please arrange to call mdio_device_free() prior to destroying the
> > > > XPCS in every case.
> > >
> > > Get it.
> > 
> > It seems this is becoming a pattern, so I think we need to solve it
> > differently. How about something like this, which means you only have
> > to care about calling xpcs_create_mdiodev() and xpcs_destroy() ?
> > 
> > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > index b87c69c4cdd7..802222581feb 100644
> > --- a/drivers/net/pcs/pcs-xpcs.c
> > +++ b/drivers/net/pcs/pcs-xpcs.c
> > @@ -1240,6 +1240,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
> >  	if (!xpcs)
> >  		return ERR_PTR(-ENOMEM);
> > 
> > +	mdio_device_get(mdiodev);
> >  	xpcs->mdiodev = mdiodev;
> > 
> >  	xpcs_id = xpcs_get_id(xpcs);
> > @@ -1272,6 +1273,7 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
> >  	ret = -ENODEV;
> > 
> >  out:
> > +	mdio_device_put(mdiodev);
> >  	kfree(xpcs);
> > 
> >  	return ERR_PTR(ret);
> > @@ -1280,8 +1282,33 @@ EXPORT_SYMBOL_GPL(xpcs_create);
> > 
> >  void xpcs_destroy(struct dw_xpcs *xpcs)
> >  {
> > +	mdio_device_put(mdiodev);
> >  	kfree(xpcs);
> >  }
> >  EXPORT_SYMBOL_GPL(xpcs_destroy);
> > 
> > +struct dw_xpcs *xpcs_create_mdiodev(struct mii_bus *bus, int addr,
> > +				    phy_interface_t interface)
> > +{
> > +	struct mdio_device *mdiodev;
> > +	struct dw_xpcs *xpcs;
> > +
> > +	mdiodev = mdio_device_create(bus, addr);
> > +	if (IS_ERR(mdiodev))
> > +		return ERR_CAST(mdiodev);
> > +
> > +	xpcs = xpcs_create(mdiodev, interface);
> > +
> > +	/* xpcs_create() has taken a refcount on the mdiodev if it was
> > +	 * successful. If xpcs_create() fails, this will free the mdio
> > +	 * device here. In any case, we don't need to hold our reference
> > +	 * anymore, and putting it here will allow mdio_device_put() in
> > +	 * xpcs_destroy() to automatically free the mdio device.
> > +	 */
> > +	mdio_device_put(mdiodev);
> > +
> > +	return xpcs;
> > +}
> > +EXPORT_SYMBOL_GPL(xpcs_create_mdiodev);
> > +
> >  MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> > index 1d7d550bbf1a..537b62330c90 100644
> > --- a/include/linux/mdio.h
> > +++ b/include/linux/mdio.h
> > @@ -108,6 +108,16 @@ int mdio_driver_register(struct mdio_driver *drv);
> >  void mdio_driver_unregister(struct mdio_driver *drv);
> >  int mdio_device_bus_match(struct device *dev, struct device_driver *drv);
> > 
> > +static inline void mdio_device_get(struct mdio_device *mdiodev)
> > +{
> > +	get_device(&mdiodev->dev);
> > +}
> > +
> > +static inline void mdio_device_put(struct mdio_device *mdiodev)
> > +{
> > +	mdio_device_free(mdiodev);
> > +}
> > +
> >  static inline bool mdio_phy_id_is_c45(int phy_id)
> >  {
> >  	return (phy_id & MDIO_PHY_ID_C45) && !(phy_id & ~MDIO_PHY_ID_C45_MASK);
> 
> Looks great, it can eliminate to create mdiodev in the ethernet driver, this device
> only be used in xpcs.

I'm just creating a patch series for both xpcs and lynx, which this
morning have had patches identifying similar problems with creation
and destruction.

-- 
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 v9 8/9] net: txgbe: Implement phylink pcs

[Russell King (Oracle)]

On Fri, May 26, 2023 at 10:37:04AM +0100, Russell King (Oracle) wrote:
> I'm just creating a patch series for both xpcs and lynx, which this
> morning have had patches identifying similar problems with creation
> and destruction.

https://lore.kernel.org/all/ZHCGZ8IgAAwr8bla@shell.armlinux.org.uk/

-- 
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 v9 8/9] net: txgbe: Implement phylink pcs

[Jiawen Wu]

On Friday, May 26, 2023 6:42 PM, Russell King (Oracle) wrote:
> On Fri, May 26, 2023 at 10:37:04AM +0100, Russell King (Oracle) wrote:
> > I'm just creating a patch series for both xpcs and lynx, which this
> > morning have had patches identifying similar problems with creation
> > and destruction.
> 
> https://lore.kernel.org/all/ZHCGZ8IgAAwr8bla@shell.armlinux.org.uk/

OK, I will send the updated patches after this patch series merged.

[PATCH net-next v9 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, 155 insertions(+), 15 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..73764555af1a 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;
 
 	wx_control_hw(wx, true);
 	wx_configure_vectors(wx);
@@ -213,24 +215,17 @@ static void txgbe_up_complete(struct wx *wx)
 	smp_mb__before_atomic();
 	wx_napi_enable_all(wx);
 
+	txgbe = netdev_to_txgbe(netdev);
+	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 +259,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 +289,11 @@ static void txgbe_disable_device(struct wx *wx)
 
 static void txgbe_down(struct wx *wx)
 {
+	struct txgbe *txgbe = netdev_to_txgbe(wx->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 d2a6f8ca78e7..a4c02a5d128b 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -9,11 +9,13 @@
 #include <linux/clkdev.h>
 #include <linux/regmap.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"
@@ -161,6 +163,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);
@@ -324,6 +415,9 @@ static void txgbe_irq_handler(struct irq_desc *desc)
 	unsigned long gpioirq;
 	struct gpio_chip *gc;
 	unsigned long flags;
+	u32 eicr;
+
+	eicr = wx_misc_isb(wx, WX_ISB_MISC);
 
 	chained_irq_enter(chip, desc);
 
@@ -345,6 +439,12 @@ 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)) {
+		u32 reg = rd32(wx, TXGBE_CFG_PORT_ST);
+
+		phylink_mac_change(txgbe->phylink, !!(reg & TXGBE_CFG_PORT_ST_LINK_UP));
+	}
+
 	/* unmask interrupt */
 	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
 }
@@ -519,16 +619,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);
@@ -550,6 +656,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:
@@ -564,6 +672,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 d1f62f62c28c..e9e5c5186f74 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -77,6 +77,10 @@
 	 TXGBE_PX_MISC_ETH_AN | 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
 
@@ -176,6 +180,7 @@ struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
 	struct dw_xpcs *xpcs;
+	struct phylink *phylink;
 	struct platform_device *sfp_dev;
 	struct platform_device *i2c_dev;
 	struct clk_lookup *clock;
-- 
2.27.0