* [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-27 8:44 ` Andy Shevchenko
2023-05-24 9:17 ` [PATCH net-next v9 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC Jiawen Wu
` (7 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, Jiawen Wu, Piotr Raczynski
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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink
2023-05-24 9:17 ` [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink Jiawen Wu
@ 2023-05-27 8:44 ` Andy Shevchenko
2023-05-30 6:11 ` Jiawen Wu
0 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2023-05-27 8:44 UTC (permalink / raw)
To: Jiawen Wu, Hans de Goede
Cc: netdev, jarkko.nikula, mika.westerberg, jsd, Jose.Abreu, andrew,
hkallweit1, linux, linux-i2c, linux-gpio, mengyuanlou,
Piotr Raczynski
+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
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink
2023-05-27 8:44 ` Andy Shevchenko
@ 2023-05-30 6:11 ` Jiawen Wu
2023-05-30 10:45 ` andy.shevchenko
0 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-30 6:11 UTC (permalink / raw)
To: 'Andy Shevchenko', 'Hans de Goede'
Cc: netdev, jarkko.nikula, mika.westerberg, jsd, Jose.Abreu, andrew,
hkallweit1, linux, linux-i2c, linux-gpio, mengyuanlou,
'Piotr Raczynski'
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?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink
2023-05-30 6:11 ` Jiawen Wu
@ 2023-05-30 10:45 ` andy.shevchenko
0 siblings, 0 replies; 36+ messages in thread
From: andy.shevchenko @ 2023-05-30 10:45 UTC (permalink / raw)
To: Jiawen Wu
Cc: 'Andy Shevchenko', 'Hans de Goede',
netdev, jarkko.nikula, mika.westerberg, jsd, Jose.Abreu, andrew,
hkallweit1, linux, linux-i2c, linux-gpio, mengyuanlou,
'Piotr Raczynski'
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
2023-05-24 9:17 ` [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-27 8:36 ` Andy Shevchenko
2023-05-24 9:17 ` [PATCH net-next v9 3/9] net: txgbe: Register fixed rate clock Jiawen Wu
` (6 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, Jiawen Wu, Piotr Raczynski
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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC
2023-05-24 9:17 ` [PATCH net-next v9 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC Jiawen Wu
@ 2023-05-27 8:36 ` Andy Shevchenko
0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2023-05-27 8:36 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, jarkko.nikula, mika.westerberg, jsd, Jose.Abreu, andrew,
hkallweit1, linux, linux-i2c, linux-gpio, mengyuanlou,
Piotr Raczynski
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 3/9] net: txgbe: Register fixed rate clock
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
2023-05-24 9:17 ` [PATCH net-next v9 1/9] net: txgbe: Add software nodes to support phylink Jiawen Wu
2023-05-24 9:17 ` [PATCH net-next v9 2/9] i2c: designware: Add driver support for Wangxun 10Gb NIC Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-24 12:17 ` Andrew Lunn
2023-05-24 9:17 ` [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device Jiawen Wu
` (5 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, 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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 3/9] net: txgbe: Register fixed rate clock
2023-05-24 9:17 ` [PATCH net-next v9 3/9] net: txgbe: Register fixed rate clock Jiawen Wu
@ 2023-05-24 12:17 ` Andrew Lunn
0 siblings, 0 replies; 36+ messages in thread
From: Andrew Lunn @ 2023-05-24 12:17 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, hkallweit1, linux, linux-i2c, linux-gpio,
mengyuanlou
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
` (2 preceding siblings ...)
2023-05-24 9:17 ` [PATCH net-next v9 3/9] net: txgbe: Register fixed rate clock Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-26 9:35 ` kernel test robot
2023-05-27 8:38 ` Andy Shevchenko
2023-05-24 9:17 ` [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify Jiawen Wu
` (4 subsequent siblings)
8 siblings, 2 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, Jiawen Wu, Piotr Raczynski
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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device
2023-05-24 9:17 ` [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device Jiawen Wu
@ 2023-05-26 9:35 ` kernel test robot
2023-05-27 8:38 ` Andy Shevchenko
1 sibling, 0 replies; 36+ messages in thread
From: kernel test robot @ 2023-05-26 9:35 UTC (permalink / raw)
To: Jiawen Wu, netdev, jarkko.nikula, andriy.shevchenko,
mika.westerberg, jsd, Jose.Abreu, andrew, hkallweit1, linux
Cc: oe-kbuild-all, linux-i2c, linux-gpio, mengyuanlou, Jiawen Wu,
Piotr Raczynski
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device
2023-05-24 9:17 ` [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device Jiawen Wu
2023-05-26 9:35 ` kernel test robot
@ 2023-05-27 8:38 ` Andy Shevchenko
1 sibling, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2023-05-27 8:38 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, jarkko.nikula, mika.westerberg, jsd, Jose.Abreu, andrew,
hkallweit1, linux, linux-i2c, linux-gpio, mengyuanlou,
Piotr Raczynski
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
` (3 preceding siblings ...)
2023-05-24 9:17 ` [PATCH net-next v9 4/9] net: txgbe: Register I2C platform device Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-26 11:30 ` kernel test robot
2023-05-24 9:17 ` [PATCH net-next v9 6/9] net: txgbe: Support GPIO to SFP socket Jiawen Wu
` (3 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, Jiawen Wu, Piotr Raczynski
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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-24 9:17 ` [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify Jiawen Wu
@ 2023-05-26 11:30 ` kernel test robot
2023-05-26 11:36 ` Russell King (Oracle)
0 siblings, 1 reply; 36+ messages in thread
From: kernel test robot @ 2023-05-26 11:30 UTC (permalink / raw)
To: Jiawen Wu, netdev, jarkko.nikula, andriy.shevchenko,
mika.westerberg, jsd, Jose.Abreu, andrew, hkallweit1, linux
Cc: oe-kbuild-all, linux-i2c, linux-gpio, mengyuanlou, Jiawen Wu,
Piotr Raczynski
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-26 11:30 ` kernel test robot
@ 2023-05-26 11:36 ` Russell King (Oracle)
2023-05-29 2:06 ` Jiawen Wu
0 siblings, 1 reply; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-26 11:36 UTC (permalink / raw)
To: kernel test robot
Cc: Jiawen Wu, netdev, jarkko.nikula, andriy.shevchenko,
mika.westerberg, jsd, Jose.Abreu, andrew, hkallweit1,
oe-kbuild-all, linux-i2c, linux-gpio, mengyuanlou,
Piotr Raczynski
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-26 11:36 ` Russell King (Oracle)
@ 2023-05-29 2:06 ` Jiawen Wu
2023-05-30 8:40 ` Jiawen Wu
0 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-29 2:06 UTC (permalink / raw)
To: 'Russell King (Oracle)', 'kernel test robot'
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, oe-kbuild-all, linux-i2c,
linux-gpio, mengyuanlou, 'Piotr Raczynski'
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?
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-29 2:06 ` Jiawen Wu
@ 2023-05-30 8:40 ` Jiawen Wu
2023-05-31 9:19 ` Jiawen Wu
2023-05-31 9:47 ` Russell King (Oracle)
0 siblings, 2 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-30 8:40 UTC (permalink / raw)
To: 'Russell King (Oracle)', 'kernel test robot'
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, oe-kbuild-all, linux-i2c,
linux-gpio, mengyuanlou, 'Piotr Raczynski'
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?
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-30 8:40 ` Jiawen Wu
@ 2023-05-31 9:19 ` Jiawen Wu
2023-05-31 9:47 ` Russell King (Oracle)
1 sibling, 0 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-31 9:19 UTC (permalink / raw)
To: 'Russell King (Oracle)'
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, oe-kbuild-all, linux-i2c,
linux-gpio, mengyuanlou, 'Piotr Raczynski'
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?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-30 8:40 ` Jiawen Wu
2023-05-31 9:19 ` Jiawen Wu
@ 2023-05-31 9:47 ` Russell King (Oracle)
2023-05-31 10:11 ` Jiawen Wu
1 sibling, 1 reply; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-31 9:47 UTC (permalink / raw)
To: Jiawen Wu
Cc: 'kernel test robot',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, oe-kbuild-all, linux-i2c,
linux-gpio, mengyuanlou, 'Piotr Raczynski'
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify
2023-05-31 9:47 ` Russell King (Oracle)
@ 2023-05-31 10:11 ` Jiawen Wu
0 siblings, 0 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-31 10:11 UTC (permalink / raw)
To: 'Russell King (Oracle)'
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, oe-kbuild-all, linux-i2c,
linux-gpio, mengyuanlou, 'Piotr Raczynski'
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 6/9] net: txgbe: Support GPIO to SFP socket
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
` (4 preceding siblings ...)
2023-05-24 9:17 ` [PATCH net-next v9 5/9] net: txgbe: Add SFP module identify Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-24 12:51 ` Andrew Lunn
2023-05-24 9:17 ` [PATCH net-next v9 7/9] net: pcs: Add 10GBASE-R mode for Synopsys Designware XPCS Jiawen Wu
` (2 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, 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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 6/9] net: txgbe: Support GPIO to SFP socket
2023-05-24 9:17 ` [PATCH net-next v9 6/9] net: txgbe: Support GPIO to SFP socket Jiawen Wu
@ 2023-05-24 12:51 ` Andrew Lunn
2023-05-24 13:07 ` Russell King (Oracle)
0 siblings, 1 reply; 36+ messages in thread
From: Andrew Lunn @ 2023-05-24 12:51 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, hkallweit1, linux, linux-i2c, linux-gpio,
mengyuanlou
> +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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 6/9] net: txgbe: Support GPIO to SFP socket
2023-05-24 12:51 ` Andrew Lunn
@ 2023-05-24 13:07 ` Russell King (Oracle)
0 siblings, 0 replies; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-24 13:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jiawen Wu, netdev, jarkko.nikula, andriy.shevchenko,
mika.westerberg, jsd, Jose.Abreu, hkallweit1, linux-i2c,
linux-gpio, mengyuanlou
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 7/9] net: pcs: Add 10GBASE-R mode for Synopsys Designware XPCS
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
` (5 preceding siblings ...)
2023-05-24 9:17 ` [PATCH net-next v9 6/9] net: txgbe: Support GPIO to SFP socket Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-24 9:17 ` [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs Jiawen Wu
2023-05-24 9:17 ` [PATCH net-next v9 9/9] net: txgbe: Support phylink MAC layer Jiawen Wu
8 siblings, 0 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, 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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
` (6 preceding siblings ...)
2023-05-24 9:17 ` [PATCH net-next v9 7/9] net: pcs: Add 10GBASE-R mode for Synopsys Designware XPCS Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
2023-05-24 12:53 ` Andrew Lunn
2023-05-26 4:14 ` Jakub Kicinski
2023-05-24 9:17 ` [PATCH net-next v9 9/9] net: txgbe: Support phylink MAC layer Jiawen Wu
8 siblings, 2 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, 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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-24 9:17 ` [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs Jiawen Wu
@ 2023-05-24 12:53 ` Andrew Lunn
2023-05-26 4:14 ` Jakub Kicinski
1 sibling, 0 replies; 36+ messages in thread
From: Andrew Lunn @ 2023-05-24 12:53 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, hkallweit1, linux, linux-i2c, linux-gpio,
mengyuanlou
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-24 9:17 ` [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs Jiawen Wu
2023-05-24 12:53 ` Andrew Lunn
@ 2023-05-26 4:14 ` Jakub Kicinski
2023-05-26 6:21 ` Jiawen Wu
1 sibling, 1 reply; 36+ messages in thread
From: Jakub Kicinski @ 2023-05-26 4:14 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux, linux-i2c, linux-gpio,
mengyuanlou
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?
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 4:14 ` Jakub Kicinski
@ 2023-05-26 6:21 ` Jiawen Wu
2023-05-26 8:43 ` Russell King (Oracle)
0 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-26 6:21 UTC (permalink / raw)
To: 'Jakub Kicinski'
Cc: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux, linux-i2c, linux-gpio,
mengyuanlou
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 6:21 ` Jiawen Wu
@ 2023-05-26 8:43 ` Russell King (Oracle)
2023-05-26 9:01 ` Jiawen Wu
0 siblings, 1 reply; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-26 8:43 UTC (permalink / raw)
To: Jiawen Wu
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 8:43 ` Russell King (Oracle)
@ 2023-05-26 9:01 ` Jiawen Wu
2023-05-26 9:07 ` Russell King (Oracle)
0 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-26 9:01 UTC (permalink / raw)
To: 'Russell King (Oracle)'
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 9:01 ` Jiawen Wu
@ 2023-05-26 9:07 ` Russell King (Oracle)
2023-05-26 9:22 ` Jiawen Wu
0 siblings, 1 reply; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-26 9:07 UTC (permalink / raw)
To: Jiawen Wu
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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!
^ permalink raw reply related [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 9:07 ` Russell King (Oracle)
@ 2023-05-26 9:22 ` Jiawen Wu
2023-05-26 9:37 ` Russell King (Oracle)
0 siblings, 1 reply; 36+ messages in thread
From: Jiawen Wu @ 2023-05-26 9:22 UTC (permalink / raw)
To: 'Russell King (Oracle)'
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 9:22 ` Jiawen Wu
@ 2023-05-26 9:37 ` Russell King (Oracle)
2023-05-26 10:42 ` Russell King (Oracle)
0 siblings, 1 reply; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-26 9:37 UTC (permalink / raw)
To: Jiawen Wu
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 9:37 ` Russell King (Oracle)
@ 2023-05-26 10:42 ` Russell King (Oracle)
2023-05-29 1:57 ` Jiawen Wu
0 siblings, 1 reply; 36+ messages in thread
From: Russell King (Oracle) @ 2023-05-26 10:42 UTC (permalink / raw)
To: Jiawen Wu
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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!
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs
2023-05-26 10:42 ` Russell King (Oracle)
@ 2023-05-29 1:57 ` Jiawen Wu
0 siblings, 0 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-29 1:57 UTC (permalink / raw)
To: 'Russell King (Oracle)'
Cc: 'Jakub Kicinski',
netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux-i2c, linux-gpio,
mengyuanlou
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH net-next v9 9/9] net: txgbe: Support phylink MAC layer
2023-05-24 9:17 [PATCH net-next v9 0/9] TXGBE PHYLINK support Jiawen Wu
` (7 preceding siblings ...)
2023-05-24 9:17 ` [PATCH net-next v9 8/9] net: txgbe: Implement phylink pcs Jiawen Wu
@ 2023-05-24 9:17 ` Jiawen Wu
8 siblings, 0 replies; 36+ messages in thread
From: Jiawen Wu @ 2023-05-24 9:17 UTC (permalink / raw)
To: netdev, jarkko.nikula, andriy.shevchenko, mika.westerberg, jsd,
Jose.Abreu, andrew, hkallweit1, linux
Cc: linux-i2c, linux-gpio, mengyuanlou, 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
^ permalink raw reply related [flat|nested] 36+ messages in thread