| Message ID | 20230605025211.743823-3-jiawenwu@trustnetic.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | TXGBE PHYLINK support | expand |
Hi, On Mon, Jun 05, 2023 at 10:52:04AM +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 Does this not need some binding documentation somewhere? > in software node case. Since IO resource was mapped on the ethernet driver, > add a model quirk to get regmap from parent device. All the best, Wolfram
On Monday, June 5, 2023 3:02 PM, Wolfram Sang wrote: > Hi, > > On Mon, Jun 05, 2023 at 10:52:04AM +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 > > Does this not need some binding documentation somewhere? Do you mean the device tree binding? This property in only used in case of software node, for wangxun Soc, which has no device tree structure. > > > in software node case. Since IO resource was mapped on the ethernet driver, > > add a model quirk to get regmap from parent device. > > All the best, > > Wolfram
> Do you mean the device tree binding? This property in only used in case of software > node, for wangxun Soc, which has no device tree structure. I see, thanks. How is the dependency of these patches? I'd like to take this patch via the i2c tree if possible. I guess the other patches will build even if this patch is not in the net-tree? Or do we need an immutable branch? Or is it really better if all goes in via net? We might get merge conflicts then, though. There are other designware patches pending.
On 6/5/23 05:52, 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. > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> > Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.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(-) > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
On Monday, June 5, 2023 3:53 PM, Wolfram Sang wrote: > > Do you mean the device tree binding? This property in only used in case of software > > node, for wangxun Soc, which has no device tree structure. > > I see, thanks. > > How is the dependency of these patches? I'd like to take this patch via > the i2c tree if possible. I guess the other patches will build even if > this patch is not in the net-tree? Or do we need an immutable branch? Or > is it really better if all goes in via net? We might get merge conflicts > then, though. There are other designware patches pending. Yes, other patches will build even without this patch. But SFP will not work. This patch series implement I2C, GPIO, SFP and PHYLINK. The support of SFP is dependent on I2C and GPIO. If these patches will be end up merging in the same upstream version, it's not a problem to merge them in different trees, I think.
> Yes, other patches will build even without this patch. But SFP will not work. > This patch series implement I2C, GPIO, SFP and PHYLINK. The support of SFP > is dependent on I2C and GPIO. If these patches will be end up merging in the > same upstream version, it's not a problem to merge them in different trees, > I think. That's how I saw it as well. Applied to for-next, thanks!
On Mon, Jun 05, 2023 at 12:05:35PM +0200, 'Wolfram Sang' wrote: > > > Yes, other patches will build even without this patch. But SFP will not work. > > This patch series implement I2C, GPIO, SFP and PHYLINK. The support of SFP > > is dependent on I2C and GPIO. If these patches will be end up merging in the > > same upstream version, it's not a problem to merge them in different trees, > > I think. > > That's how I saw it as well. > > Applied to for-next, thanks! Be careful... net-next uses patchwork, and I suspect as this is posted as a series which the subject line states as being destined by the author for the "net-next" tree, the entire series will end up being slurped into the net-next tree. https://www.kernel.org/doc/html/v5.10/networking/netdev-FAQ.html#q-how-do-i-indicate-which-tree-net-vs-net-next-my-patch-should-be-in This patch is still marked as "new" in patchwork: https://patchwork.kernel.org/project/netdevbpf/patch/20230605025211.743823-3-jiawenwu@trustnetic.com/
> Be careful... net-next uses patchwork, and I suspect as this is posted > as a series which the subject line states as being destined by the > author for the "net-next" tree, the entire series will end up being > slurped into the net-next tree. Thanks for the pointer. Jiawen Wu, would you kindly send a v12 of the series (without the I2C patch)?
On Mon, Jun 05, 2023 at 02:57:36PM +0200, 'Wolfram Sang' wrote: > > > Be careful... net-next uses patchwork, and I suspect as this is posted > > as a series which the subject line states as being destined by the > > author for the "net-next" tree, the entire series will end up being > > slurped into the net-next tree. > > Thanks for the pointer. Jiawen Wu, would you kindly send a v12 of the > series (without the I2C patch)? I'm wondering if it would be easier just mark it in patchwork as applied elsewhere (don't remember exact variant, but meaning is the same).
On Mon, 5 Jun 2023 16:03:23 +0300 Andy Shevchenko wrote: > I'm wondering if it would be easier just mark it in patchwork as applied > elsewhere (don't remember exact variant, but meaning is the same). It would not. PW doesn't support sparse series well and it's just confusing to everyone. We don't do manual handpicking in netdev. Reposting the remaining patches is the right thing to do.
On Monday, June 5, 2023 8:58 PM, Wolfram Sang wrote: > > Be careful... net-next uses patchwork, and I suspect as this is posted > > as a series which the subject line states as being destined by the > > author for the "net-next" tree, the entire series will end up being > > slurped into the net-next tree. > > Thanks for the pointer. Jiawen Wu, would you kindly send a v12 of the > series (without the I2C patch)? Okay, v12 series will be other 8 patches for net-next. Do I need to send this independent patch to I2C tree?
> Okay, v12 series will be other 8 patches for net-next. > Do I need to send this independent patch to I2C tree? No, it is already applied.
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..5a476a38b52f 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);