diff mbox series

[net-next,2/6] net: txgbe: Implement I2C bus master driver

Message ID 20230403064528.343866-3-jiawenwu@trustnetic.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series TXGBE PHYLINK support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/apply fail Patch does not apply to net-next

Commit Message

Jiawen Wu April 3, 2023, 6:45 a.m. UTC
I2C bus is integrated in Wangxun 10Gb ethernet chip. Implement I2C bus
driver to receive I2C messages.

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

Comments

Andrew Lunn April 3, 2023, 12:52 p.m. UTC | #1
On Mon, Apr 03, 2023 at 02:45:24PM +0800, Jiawen Wu wrote:
> I2C bus is integrated in Wangxun 10Gb ethernet chip. Implement I2C bus
> driver to receive I2C messages.

Please Cc: the i2c mailing list for comments. They know more about I2C
than the netdev people.

Is the I2C bus master your own IP, or have you licensed a core? Or
using the open cores i2C bus master? I just want to make sure there is
not already a linux driver for this.
 
> +static void txgbe_i2c_start(struct wx *wx, u16 dev_addr)
> +{
> +	wr32(wx, TXGBE_I2C_ENABLE, 0);
> +
> +	wr32(wx, TXGBE_I2C_CON,
> +	     (TXGBE_I2C_CON_MASTER_MODE |
> +	      TXGBE_I2C_CON_SPEED(1) |
> +	      TXGBE_I2C_CON_RESTART_EN |
> +	      TXGBE_I2C_CON_SLAVE_DISABLE));
> +	/* Default addr is 0xA0 ,bit 0 is configure for read/write! */

A generic I2C bus master should not care about that address is being
read/write. For the SFP use case, 0xa0 will be used most of the time,
plus 0xa2 for diagnostics. But when the SFP contains a copper PHY,
other addresses will be used as well.

> +static int txgbe_i2c_xfer(struct i2c_adapter *i2c_adap,
> +			  struct i2c_msg *msg, int num_msgs)
> +{
> +	struct wx *wx = i2c_get_adapdata(i2c_adap);
> +	u8 *dev_addr = msg[0].buf;
> +	bool read = false;
> +	int i, ret;
> +	u8 *buf;
> +	u16 len;
> +
> +	txgbe_i2c_start(wx, msg[0].addr);
> +
> +	for (i = 0; i < num_msgs; i++) {
> +		if (msg[i].flags & I2C_M_RD) {
> +			read = true;
> +			len = msg[i].len;
> +			buf = msg[i].buf;
> +		}
> +	}
> +
> +	if (!read) {
> +		wx_err(wx, "I2C write not supported\n");
> +		return num_msgs;
> +	}

Write is not supported at all? Is this a hardware limitation?  I think
-EOPNOTSUPP is required here, and you need to ensure the code using
the I2C bus master has quirks to not try to write.

> +#define TXGBE_I2C_SLAVE_ADDR                    (0xA0 >> 1)
> +#define TXGBE_I2C_EEPROM_DEV_ADDR               0xA0

These two do not appear to be used? I guess you took your hard coded
SFP i2c bus master and generalised it? Please clean up dead code like
this.

	Andrew
Jiawen Wu April 4, 2023, 2:47 a.m. UTC | #2
On Monday, April 3, 2023 8:53 PM, Andrew Lunn wrote:
> On Mon, Apr 03, 2023 at 02:45:24PM +0800, Jiawen Wu wrote:
> > I2C bus is integrated in Wangxun 10Gb ethernet chip. Implement I2C bus
> > driver to receive I2C messages.
> 
> Please Cc: the i2c mailing list for comments. They know more about I2C than
the
> netdev people.
> 
> Is the I2C bus master your own IP, or have you licensed a core? Or using the
open
> cores i2C bus master? I just want to make sure there is not already a linux
driver for
> this.
> 

I use the I2C core driver, and implement my own i2c_algorithm. I think it needs
to configure my registers to realize the function.

> > +static void txgbe_i2c_start(struct wx *wx, u16 dev_addr) {
> > +	wr32(wx, TXGBE_I2C_ENABLE, 0);
> > +
> > +	wr32(wx, TXGBE_I2C_CON,
> > +	     (TXGBE_I2C_CON_MASTER_MODE |
> > +	      TXGBE_I2C_CON_SPEED(1) |
> > +	      TXGBE_I2C_CON_RESTART_EN |
> > +	      TXGBE_I2C_CON_SLAVE_DISABLE));
> > +	/* Default addr is 0xA0 ,bit 0 is configure for read/write! */
> 
> A generic I2C bus master should not care about that address is being
read/write.
> For the SFP use case, 0xa0 will be used most of the time, plus 0xa2 for
diagnostics.
> But when the SFP contains a copper PHY, other addresses will be used as well.
> 

Yes, this comment is redundant. The address will be set to 'msg[0].addr'.

> > +static int txgbe_i2c_xfer(struct i2c_adapter *i2c_adap,
> > +			  struct i2c_msg *msg, int num_msgs) {
> > +	struct wx *wx = i2c_get_adapdata(i2c_adap);
> > +	u8 *dev_addr = msg[0].buf;
> > +	bool read = false;
> > +	int i, ret;
> > +	u8 *buf;
> > +	u16 len;
> > +
> > +	txgbe_i2c_start(wx, msg[0].addr);
> > +
> > +	for (i = 0; i < num_msgs; i++) {
> > +		if (msg[i].flags & I2C_M_RD) {
> > +			read = true;
> > +			len = msg[i].len;
> > +			buf = msg[i].buf;
> > +		}
> > +	}
> > +
> > +	if (!read) {
> > +		wx_err(wx, "I2C write not supported\n");
> > +		return num_msgs;
> > +	}
> 
> Write is not supported at all? Is this a hardware limitation?  I think
-EOPNOTSUPP
> is required here, and you need to ensure the code using the I2C bus master has
> quirks to not try to write.

It is supported. False testing leads to false perceptions, I'll fix it.

> 
> > +#define TXGBE_I2C_SLAVE_ADDR                    (0xA0 >> 1)
> > +#define TXGBE_I2C_EEPROM_DEV_ADDR               0xA0
> 
> These two do not appear to be used? I guess you took your hard coded SFP i2c
bus
> master and generalised it? Please clean up dead code like this.
> 
> 	Andrew
>
Andrew Lunn April 4, 2023, 2:18 p.m. UTC | #3
On Tue, Apr 04, 2023 at 10:47:28AM +0800, Jiawen Wu wrote:
> On Monday, April 3, 2023 8:53 PM, Andrew Lunn wrote:
> > On Mon, Apr 03, 2023 at 02:45:24PM +0800, Jiawen Wu wrote:
> > > I2C bus is integrated in Wangxun 10Gb ethernet chip. Implement I2C bus
> > > driver to receive I2C messages.
> > 
> > Please Cc: the i2c mailing list for comments. They know more about I2C than
> the
> > netdev people.
> > 
> > Is the I2C bus master your own IP, or have you licensed a core? Or using the
> open
> > cores i2C bus master? I just want to make sure there is not already a linux
> driver for
> > this.
> > 
> 
> I use the I2C core driver, and implement my own i2c_algorithm. I think it needs
> to configure my registers to realize the function.

I had a quick look, and it appears the hardware is not an open-cores
derived I2C bus master.

As i tried to say, sometimes you just license an I2C bus master,
rather than develop one from scratch. And if it was a licensed IP
core, there is likely to be an existing driver.

> > > +	if (!read) {
> > > +		wx_err(wx, "I2C write not supported\n");
> > > +		return num_msgs;
> > > +	}
> > 
> > Write is not supported at all? Is this a hardware limitation?  I think
> -EOPNOTSUPP
> > is required here, and you need to ensure the code using the I2C bus master has
> > quirks to not try to write.
> 
> It is supported. False testing leads to false perceptions, I'll fix it.

Great. It would be odd not having write support.

	Andrew
Jiawen Wu April 6, 2023, 6:59 a.m. UTC | #4
> > > Is the I2C bus master your own IP, or have you licensed a core?
> > > Or using the open cores i2C bus master? I just want to make sure
> > > there is not already a linux driver for this.
> > >
> >
> > I use the I2C core driver, and implement my own i2c_algorithm.
> > I think it needs to configure my registers to realize the function.
> 
> I had a quick look, and it appears the hardware is not an open-cores
> derived I2C bus master.
> 
> As i tried to say, sometimes you just license an I2C bus master,
> rather than develop one from scratch. And if it was a licensed IP
> core, there is likely to be an existing driver.
> 

By consulting with hardware designers, our I2C licensed the IP of
Synopsys.
But I can't use the I2C_DESIGNWARE_CORE directly, because IRQ is not
supported in our I2C.
Andrew Lunn April 7, 2023, 2:03 a.m. UTC | #5
> By consulting with hardware designers, our I2C licensed the IP of
> Synopsys.
> But I can't use the I2C_DESIGNWARE_CORE directly, because IRQ is not
> supported in our I2C.

When you repost this patch and Cc: the i2c list, please point this
out. Please also Cc: the Designware I2C maintainers. They might have
some ideas how the driver can be used in polled mode.

Ideally, you want to use the existing driver, since it has hardware
work around, optimisations, and probably less bugs because it is used
and tested a lot.

And it is not too unusual for interrupt support to be missing. The
ocores driver can be used without interrupts. And there is sometimes a
polled mode supported so you can for example use the I2C bus to talk
to the power controller to turn the power at the end of shutdown when
nearly everything has stop working.

	Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index c9d88673d306..8cbf0dd48a2c 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -41,6 +41,7 @@  config TXGBE
 	tristate "Wangxun(R) 10GbE PCI Express adapters support"
 	depends on PCI
 	select LIBWX
+	select I2C
 	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 163acb7e515e..f8a4b211f4e8 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -2,9 +2,12 @@ 
 /* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
 
 #include <linux/gpio/property.h>
+#include <linux/iopoll.h>
+#include <linux/i2c.h>
 #include <linux/pci.h>
 
 #include "../libwx/wx_type.h"
+#include "../libwx/wx_hw.h"
 #include "txgbe_type.h"
 #include "txgbe_phy.h"
 
@@ -60,6 +63,130 @@  static int txgbe_swnodes_register(struct txgbe *txgbe)
 	return software_node_register_node_group(nodes->group);
 }
 
+static void txgbe_i2c_start(struct wx *wx, u16 dev_addr)
+{
+	wr32(wx, TXGBE_I2C_ENABLE, 0);
+
+	wr32(wx, TXGBE_I2C_CON,
+	     (TXGBE_I2C_CON_MASTER_MODE |
+	      TXGBE_I2C_CON_SPEED(1) |
+	      TXGBE_I2C_CON_RESTART_EN |
+	      TXGBE_I2C_CON_SLAVE_DISABLE));
+	/* Default addr is 0xA0 ,bit 0 is configure for read/write! */
+	wr32(wx, TXGBE_I2C_TAR, dev_addr);
+	wr32(wx, TXGBE_I2C_SS_SCL_HCNT, 600);
+	wr32(wx, TXGBE_I2C_SS_SCL_LCNT, 600);
+	wr32(wx, TXGBE_I2C_RX_TL, 0); /* 1byte for rx full signal */
+	wr32(wx, TXGBE_I2C_TX_TL, 4);
+	wr32(wx, TXGBE_I2C_SCL_STUCK_TIMEOUT, 0xFFFFFF);
+	wr32(wx, TXGBE_I2C_SDA_STUCK_TIMEOUT, 0xFFFFFF);
+
+	wr32(wx, TXGBE_I2C_INTR_MASK, 0);
+	wr32(wx, TXGBE_I2C_ENABLE, 1);
+}
+
+static int txgbe_read_i2c_bytes(struct wx *wx, u8 addr, u16 len, u8 *buf)
+{
+	int err, i;
+	u16 val;
+
+	for (i = 0; i < len; i++) {
+		/* wait tx empty */
+		err = read_poll_timeout(rd32, val,
+					(val & TXGBE_I2C_INTR_STAT_TEMP) ==
+					TXGBE_I2C_INTR_STAT_TEMP,
+					100, 1000, false, wx,
+					TXGBE_I2C_RAW_INTR_STAT);
+		if (err != 0)
+			return err;
+
+		/* read data */
+		wr32(wx, TXGBE_I2C_DATA_CMD, (addr + i) | TXGBE_I2C_DATA_CMD_STOP);
+		wr32(wx, TXGBE_I2C_DATA_CMD, TXGBE_I2C_DATA_CMD_READ);
+
+		/* wait for read complete */
+		err = read_poll_timeout(rd32, val,
+					(val & TXGBE_I2C_INTR_STAT_RFUL) ==
+					TXGBE_I2C_INTR_STAT_RFUL,
+					100, 1000, false, wx,
+					TXGBE_I2C_RAW_INTR_STAT);
+		if (err != 0)
+			return err;
+
+		buf[i] = 0xFF & rd32(wx, TXGBE_I2C_DATA_CMD);
+	}
+
+	return 0;
+}
+
+static int txgbe_i2c_xfer(struct i2c_adapter *i2c_adap,
+			  struct i2c_msg *msg, int num_msgs)
+{
+	struct wx *wx = i2c_get_adapdata(i2c_adap);
+	u8 *dev_addr = msg[0].buf;
+	bool read = false;
+	int i, ret;
+	u8 *buf;
+	u16 len;
+
+	txgbe_i2c_start(wx, msg[0].addr);
+
+	for (i = 0; i < num_msgs; i++) {
+		if (msg[i].flags & I2C_M_RD) {
+			read = true;
+			len = msg[i].len;
+			buf = msg[i].buf;
+		}
+	}
+
+	if (!read) {
+		wx_err(wx, "I2C write not supported\n");
+		return num_msgs;
+	}
+
+	ret = txgbe_read_i2c_bytes(wx, *dev_addr, len, buf);
+	if (!ret)
+		ret = num_msgs;
+
+	return ret;
+}
+
+static u32 txgbe_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C;
+}
+
+static const struct i2c_algorithm txgbe_i2c_algo = {
+	.master_xfer = txgbe_i2c_xfer,
+	.functionality = txgbe_i2c_func,
+};
+
+static int txgbe_i2c_adapter_add(struct txgbe *txgbe)
+{
+	struct pci_dev *pdev = txgbe->wx->pdev;
+	struct i2c_adapter *i2c_adap;
+	int ret;
+
+	i2c_adap = devm_kzalloc(&pdev->dev, sizeof(*i2c_adap), GFP_KERNEL);
+	if (!i2c_adap)
+		return -ENOMEM;
+
+	i2c_adap->owner = THIS_MODULE;
+	i2c_adap->algo = &txgbe_i2c_algo;
+	i2c_adap->dev.parent = &pdev->dev;
+	i2c_adap->dev.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_I2C]);
+	strscpy(i2c_adap->name, "txgbe_i2c", sizeof(i2c_adap->name));
+
+	i2c_set_adapdata(i2c_adap, txgbe->wx);
+	ret = i2c_add_adapter(i2c_adap);
+	if (ret)
+		return ret;
+
+	txgbe->i2c_adap = i2c_adap;
+
+	return 0;
+}
+
 int txgbe_init_phy(struct txgbe *txgbe)
 {
 	int ret;
@@ -70,10 +197,24 @@  int txgbe_init_phy(struct txgbe *txgbe)
 		return ret;
 	}
 
+	ret = txgbe_i2c_adapter_add(txgbe);
+	if (ret) {
+		wx_err(txgbe->wx, "failed to init i2c interface: %d\n", ret);
+		goto err;
+	}
+
 	return 0;
+
+err:
+	txgbe_remove_phy(txgbe);
+
+	return ret;
 }
 
 void txgbe_remove_phy(struct txgbe *txgbe)
 {
+	if (txgbe->i2c_adap)
+		i2c_del_adapter(txgbe->i2c_adap);
+
 	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 d30684378f4e..de488609f713 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -55,6 +55,31 @@ 
 #define TXGBE_TS_CTL                            0x10300
 #define TXGBE_TS_CTL_EVAL_MD                    BIT(31)
 
+/* I2C registers */
+#define TXGBE_I2C_CON                           0x14900 /* I2C Control */
+#define TXGBE_I2C_CON_SLAVE_DISABLE             BIT(6)
+#define TXGBE_I2C_CON_RESTART_EN                BIT(5)
+#define TXGBE_I2C_CON_SPEED(_v)                 FIELD_PREP(GENMASK(2, 1), _v)
+#define TXGBE_I2C_CON_MASTER_MODE               BIT(0)
+#define TXGBE_I2C_TAR                           0x14904 /* I2C Target Address */
+#define TXGBE_I2C_DATA_CMD                      0x14910 /* I2C Rx/Tx Data Buf and Cmd */
+#define TXGBE_I2C_DATA_CMD_STOP                 BIT(9)
+#define TXGBE_I2C_DATA_CMD_READ                 (BIT(8) | TXGBE_I2C_DATA_CMD_STOP)
+#define TXGBE_I2C_SS_SCL_HCNT                   0x14914
+#define TXGBE_I2C_SS_SCL_LCNT                   0x14918
+#define TXGBE_I2C_INTR_MASK                     0x14930 /* I2C Interrupt Mask */
+#define TXGBE_I2C_RAW_INTR_STAT                 0x14934 /* I2C Raw Interrupt Status */
+#define TXGBE_I2C_INTR_STAT_RFUL                BIT(2)
+#define TXGBE_I2C_INTR_STAT_TEMP                BIT(4)
+#define TXGBE_I2C_RX_TL                         0x14938 /* I2C Receive FIFO Threshold */
+#define TXGBE_I2C_TX_TL                         0x1493C /* I2C TX FIFO Threshold */
+#define TXGBE_I2C_ENABLE                        0x1496C /* I2C Enable */
+#define TXGBE_I2C_SCL_STUCK_TIMEOUT             0x149AC
+#define TXGBE_I2C_SDA_STUCK_TIMEOUT             0x149B0
+
+#define TXGBE_I2C_SLAVE_ADDR                    (0xA0 >> 1)
+#define TXGBE_I2C_EEPROM_DEV_ADDR               0xA0
+
 /* Part Number String Length */
 #define TXGBE_PBANUM_LENGTH                     32
 
@@ -139,6 +164,7 @@  struct txgbe_nodes {
 struct txgbe {
 	struct wx *wx;
 	struct txgbe_nodes nodes;
+	struct i2c_adapter *i2c_adap;
 };
 
 #endif /* _TXGBE_TYPE_H_ */