diff mbox series

[net-next,v4,2/8] i2c: designware: Add driver support for Wangxun 10Gb NIC

Message ID 20230422045621.360918-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/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 2 maintainers not CCed: jsd@semihalf.com mika.westerberg@linux.intel.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 82 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiawen Wu April 22, 2023, 4:56 a.m. UTC 
Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
with SFP.

Add platform data to pass IOMEM base address, board flag since resource
address was mapped on ethernet driver. Since there is no device tree to
get the clock, the parameters hcnt/lcnt are also set by platform data.

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.

Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/i2c/busses/i2c-designware-common.c  |  8 ++
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-master.c  | 84 ++++++++++++++++++++-
 drivers/i2c/busses/i2c-designware-platdrv.c | 36 ++++++++-
 include/linux/platform_data/i2c-dw.h        | 15 ++++
 5 files changed, 140 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/platform_data/i2c-dw.h

Comments

Andy Shevchenko April 22, 2023, 4:25 p.m. UTC | #1 
On Sat, Apr 22, 2023 at 12:56:15PM +0800, Jiawen Wu wrote:
> Wangxun 10Gb ethernet chip is connected to Designware I2C, to communicate
> with SFP.
> 
> Add platform data to pass IOMEM base address, board flag since resource
> address was mapped on ethernet driver. Since there is no device tree to
> get the clock, the parameters hcnt/lcnt are also set by platform data.
> 
> 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.
> 
> Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Please, use --cc parameter to `git format-patch ...`.

Also for tag block we do not use blank lines.

...

>  #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)

Yeah, maybe next one will need to transform this from bitfield to plain number.

...

> -static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
> +static int poll_i2c_adap_quirk(struct dw_i2c_dev *dev)

i2c_dw_poll_adap_quirk()

...

> +static bool i2c_is_model_poll(struct dw_i2c_dev *dev)

i2c_dw_is_...

...

> +++ b/include/linux/platform_data/i2c-dw.h

No way we need this in a new code.

> +struct dw_i2c_platform_data {
> +	void __iomem *base;

You should use regmap.

> +	unsigned int flags;
> +	unsigned int ss_hcnt;
> +	unsigned int ss_lcnt;
> +	unsigned int fs_hcnt;
> +	unsigned int fs_lcnt;

No, use device properties.

> +};
Jiawen Wu April 23, 2023, 2:31 a.m. UTC | #2 
> > +++ b/include/linux/platform_data/i2c-dw.h
> 
> No way we need this in a new code.

Do I have to rely on OF or ACPI if I need these parameters?

> 
> > +struct dw_i2c_platform_data {
> > +	void __iomem *base;
> 
> You should use regmap.

The resource was mapped on the ethernet driver. How do I map it again
with I2C offset?

> 
> > +	unsigned int flags;
> > +	unsigned int ss_hcnt;
> > +	unsigned int ss_lcnt;
> > +	unsigned int fs_hcnt;
> > +	unsigned int fs_lcnt;
> 
> No, use device properties.
> 
> > +};
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 
>
Jiawen Wu April 24, 2023, 2:39 a.m. UTC | #3 
> > +++ b/include/linux/platform_data/i2c-dw.h
> 
> No way we need this in a new code.
> 
> > +struct dw_i2c_platform_data {
> > +	void __iomem *base;
> 
> You should use regmap.
> 
> > +	unsigned int flags;
> > +	unsigned int ss_hcnt;
> > +	unsigned int ss_lcnt;
> > +	unsigned int fs_hcnt;
> > +	unsigned int fs_lcnt;
> 
> No, use device properties.
> 
> > +};
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Is it acceptable to add such a function into dw_i2c_plat_probe()?
Otherwise I really can't find a way to get these parameters without DT and ACPI.

+static void i2c_dw_parse_property(struct dw_i2c_dev *dev)
+{
+       if (!is_software_node(dev_fwnode(dev->dev)))
+               return;
+
+       if (!dev->flags)
+               device_property_read_u32(dev->dev, "dw-i2c-flags", &dev->flags);
+
+       device_property_read_u16(dev->dev, "i2c-ss-scl-hcnt", &dev->ss_hcnt);
+       device_property_read_u16(dev->dev, "i2c-ss-scl-lcnt", &dev->ss_lcnt);
+       device_property_read_u16(dev->dev, "i2c-fs-scl-hcnt", &dev->fs_hcnt);
+       device_property_read_u16(dev->dev, "i2c-fs-scl-lcnt", &dev->fs_lcnt);
+
+       if (!dev->ss_hcnt || !dev->ss_lcnt) {
+               dev->ss_hcnt = 6;
+               dev->ss_lcnt = 8;
+       }
+       if (!dev->fs_hcnt || !dev->fs_lcnt) {
+               dev->fs_hcnt = 6;
+               dev->fs_lcnt = 8;
+       }
+}
Andy Shevchenko April 24, 2023, 1:28 p.m. UTC | #4 
On Sun, Apr 23, 2023 at 10:31:09AM +0800, Jiawen Wu wrote:
> > > +++ b/include/linux/platform_data/i2c-dw.h
> > 
> > No way we need this in a new code.
> 
> Do I have to rely on OF or ACPI if I need these parameters?
> 
> > 
> > > +struct dw_i2c_platform_data {
> > > +	void __iomem *base;
> > 
> > You should use regmap.
> 
> The resource was mapped on the ethernet driver. How do I map it again
> with I2C offset?

Create a regmap MMIO and pass the pointer to the child driver via existing
private members. See how MFD drivers do that, e.g. intel_soc_pmic_*.c.

> > > +	unsigned int flags;
> > > +	unsigned int ss_hcnt;
> > > +	unsigned int ss_lcnt;
> > > +	unsigned int fs_hcnt;
> > > +	unsigned int fs_lcnt;
> > 
> > No, use device properties.
> > 
> > > +};
Andi Shyti April 25, 2023, 2:08 p.m. UTC | #5 
Hi Andy,

[...]

> >  #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)
> 
> Yeah, maybe next one will need to transform this from bitfield to plain number.

You mean this?

-#define ACCESS_INTR_MASK                       BIT(0)
-#define ACCESS_NO_IRQ_SUSPEND                  BIT(1)
-#define ARBITRATION_SEMAPHORE                  BIT(2)
-
-#define MODEL_MSCC_OCELOT                      BIT(8)
-#define MODEL_BAIKAL_BT1                       BIT(9)
-#define MODEL_AMD_NAVI_GPU                     BIT(10)
-#define MODEL_MASK                             GENMASK(11, 8)
+#define ACCESS_INTR_MASK                       0x00
+#define ACCESS_NO_IRQ_SUSPEND                  0x01
+#define ARBITRATION_SEMAPHORE                  0x02
+
+#define MODEL_MSCC_OCELOT                      0x08
+#define MODEL_BAIKAL_BT1                       0x09
+#define MODEL_AMD_NAVI_GPU                     0x0a
+#define MODEL_MASK                             0x78

I actually like more bitfield to plain numbers.

Andi
Andy Shevchenko May 1, 2023, 6:28 p.m. UTC | #6 
On Tue, Apr 25, 2023 at 04:08:59PM +0200, Andi Shyti wrote:
> > >  #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)
> > 
> > Yeah, maybe next one will need to transform this from bitfield to plain number.
> 
> You mean this?

No, only MODEL_XXX bits.

> -#define ACCESS_INTR_MASK                       BIT(0)
> -#define ACCESS_NO_IRQ_SUSPEND                  BIT(1)
> -#define ARBITRATION_SEMAPHORE                  BIT(2)
> -
> -#define MODEL_MSCC_OCELOT                      BIT(8)
> -#define MODEL_BAIKAL_BT1                       BIT(9)
> -#define MODEL_AMD_NAVI_GPU                     BIT(10)
> -#define MODEL_MASK                             GENMASK(11, 8)
> +#define ACCESS_INTR_MASK                       0x00
> +#define ACCESS_NO_IRQ_SUSPEND                  0x01
> +#define ARBITRATION_SEMAPHORE                  0x02
> +
> +#define MODEL_MSCC_OCELOT                      0x08
> +#define MODEL_BAIKAL_BT1                       0x09
> +#define MODEL_AMD_NAVI_GPU                     0x0a
> +#define MODEL_MASK                             0x78
> 
> I actually like more bitfield to plain numbers.

Too limited. For model we get 16 out of 4 bits, which is much better and as you
see we have a trend.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 0dc6b1ce663f..a7c2e67ccbf6 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -575,6 +575,14 @@  int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 	unsigned int param;
 	int ret;
 
+	/* DW_IC_COMP_PARAM_1 not implement for IP issue */
+	if ((dev->flags & MODEL_MASK) == MODEL_WANGXUN_SP) {
+		dev->tx_fifo_depth = 4;
+		dev->rx_fifo_depth = 0;
+
+		return 0;
+	}
+
 	/*
 	 * Try to detect the FIFO depth if not set by interface driver,
 	 * the depth could be from 2 to 256 from HW spec.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 050d8c63ad3c..c686198e12d2 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -303,6 +303,7 @@  struct dw_i2c_dev {
 #define MODEL_MSCC_OCELOT			BIT(8)
 #define MODEL_BAIKAL_BT1			BIT(9)
 #define MODEL_AMD_NAVI_GPU			BIT(10)
+#define MODEL_WANGXUN_SP			BIT(11)
 #define MODEL_MASK				GENMASK(11, 8)
 
 /*
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 55ea91a63382..962b791dae39 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
@@ -568,6 +630,11 @@  i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		goto done_nolock;
 	}
 
+	if ((dev->flags & MODEL_MASK) == MODEL_WANGXUN_SP) {
+		ret = txgbe_i2c_dw_xfer_quirk(adap, msgs, num);
+		goto done_nolock;
+	}
+
 	reinit_completion(&dev->cmd_complete);
 	dev->msgs = msgs;
 	dev->msgs_num = num;
@@ -848,7 +915,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 poll_i2c_adap_quirk(struct dw_i2c_dev *dev)
 {
 	struct i2c_adapter *adap = &dev->adapter;
 	int ret;
@@ -862,6 +929,17 @@  static int amd_i2c_adap_quirk(struct dw_i2c_dev *dev)
 	return ret;
 }
 
+static bool i2c_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 +995,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_is_model_poll(dev))
+		return poll_i2c_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 74182db03a88..10b2c61b279f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -23,6 +23,7 @@ 
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/i2c-dw.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
@@ -179,12 +180,14 @@  static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev->dev);
-	int ret;
+	int ret = 0;
 
 	switch (dev->flags & MODEL_MASK) {
 	case MODEL_BAIKAL_BT1:
 		ret = bt1_i2c_request_regs(dev);
 		break;
+	case MODEL_WANGXUN_SP:
+		break;
 	default:
 		dev->base = devm_platform_ioremap_resource(pdev, 0);
 		ret = PTR_ERR_OR_ZERO(dev->base);
@@ -194,6 +197,35 @@  static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
 	return ret;
 }
 
+static void dw_i2c_get_plat_data(struct dw_i2c_dev *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev->dev);
+	struct dw_i2c_platform_data *pdata;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata)
+		return;
+
+	dev->flags |= pdata->flags;
+	dev->base = pdata->base;
+
+	if (pdata->ss_hcnt && pdata->ss_lcnt) {
+		dev->ss_hcnt = pdata->ss_hcnt;
+		dev->ss_lcnt = pdata->ss_lcnt;
+	} else {
+		dev->ss_hcnt = 6;
+		dev->ss_lcnt = 8;
+	}
+
+	if (pdata->fs_hcnt && pdata->fs_lcnt) {
+		dev->fs_hcnt = pdata->fs_hcnt;
+		dev->fs_lcnt = pdata->fs_lcnt;
+	} else {
+		dev->fs_hcnt = 6;
+		dev->fs_lcnt = 8;
+	}
+}
+
 static const struct dmi_system_id dw_i2c_hwmon_class_dmi[] = {
 	{
 		.ident = "Qtechnology QT5222",
@@ -282,6 +314,8 @@  static int dw_i2c_plat_probe(struct platform_device *pdev)
 	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
 
+	dw_i2c_get_plat_data(dev);
+
 	ret = dw_i2c_plat_request_regs(dev);
 	if (ret)
 		return ret;
diff --git a/include/linux/platform_data/i2c-dw.h b/include/linux/platform_data/i2c-dw.h
new file mode 100644
index 000000000000..f4552df08084
--- /dev/null
+++ b/include/linux/platform_data/i2c-dw.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_I2C_DW_H
+#define _LINUX_I2C_DW_H
+
+struct dw_i2c_platform_data {
+	void __iomem *base;
+	unsigned int flags;
+	unsigned int ss_hcnt;
+	unsigned int ss_lcnt;
+	unsigned int fs_hcnt;
+	unsigned int fs_lcnt;
+};
+
+#endif /* _LINUX_I2C_DW_H */