* [PATCH net 1/3] net: txgbe: add IO address in I2C platform device data
[not found] <20240823030242.3083528-1-jiawenwu@trustnetic.com>
@ 2024-08-23 3:02 ` Jiawen Wu
2024-08-23 14:14 ` Andy Shevchenko
2024-08-23 3:02 ` [PATCH net 2/3] i2c: designware: add device private data passing to lock functions Jiawen Wu
2024-08-23 3:02 ` [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC Jiawen Wu
2 siblings, 1 reply; 10+ messages in thread
From: Jiawen Wu @ 2024-08-23 3:02 UTC (permalink / raw)
To: andi.shyti, jarkko.nikula, andriy.shevchenko, mika.westerberg,
jsd, davem, edumazet, kuba, pabeni, rmk+kernel, piotr.raczynski,
andrew, linux-i2c, netdev
Cc: mengyuanlou, duanqiangwen, Jiawen Wu, stable
Consider the necessity of reading/writing the IO address to acquire and
release the lock between software and firmware, add the IO address as
the platform data to register I2C platform device.
Cc: stable@vger.kernel.org
Fixes: c625e72561f6 ("net: txgbe: Register I2C platform device")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 5 +++++
include/linux/platform_data/i2c-wx.h | 11 +++++++++++
2 files changed, 16 insertions(+)
create mode 100644 include/linux/platform_data/i2c-wx.h
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 5f502265f0a6..781a3a34aa4c 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -9,6 +9,7 @@
#include <linux/i2c.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/platform_data/i2c-wx.h>
#include <linux/regmap.h>
#include <linux/pcs/pcs-xpcs.h>
#include <linux/phylink.h>
@@ -618,6 +619,7 @@ static const struct regmap_config i2c_regmap_config = {
static int txgbe_i2c_register(struct txgbe *txgbe)
{
+ struct txgbe_i2c_platform_data pdata = {};
struct platform_device_info info = {};
struct platform_device *i2c_dev;
struct regmap *i2c_regmap;
@@ -636,6 +638,9 @@ static int txgbe_i2c_register(struct txgbe *txgbe)
info.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_I2C]);
info.name = "i2c_designware";
info.id = pci_dev_id(pdev);
+ pdata.hw_addr = wx->hw_addr;
+ info.data = &pdata;
+ info.size_data = sizeof(pdata);
info.res = &DEFINE_RES_IRQ(pdev->irq);
info.num_res = 1;
diff --git a/include/linux/platform_data/i2c-wx.h b/include/linux/platform_data/i2c-wx.h
new file mode 100644
index 000000000000..b46777fa1d85
--- /dev/null
+++ b/include/linux/platform_data/i2c-wx.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */
+
+#ifndef _I2C_WX_H_
+#define _I2C_WX_H_
+
+struct txgbe_i2c_platform_data {
+ void __iomem *hw_addr;
+};
+
+#endif /* _I2C_WX_H_ */
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 1/3] net: txgbe: add IO address in I2C platform device data
2024-08-23 3:02 ` [PATCH net 1/3] net: txgbe: add IO address in I2C platform device data Jiawen Wu
@ 2024-08-23 14:14 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-08-23 14:14 UTC (permalink / raw)
To: Jiawen Wu
Cc: andi.shyti, jarkko.nikula, mika.westerberg, jsd, davem, edumazet,
kuba, pabeni, rmk+kernel, piotr.raczynski, andrew, linux-i2c,
netdev, mengyuanlou, duanqiangwen, stable
On Fri, Aug 23, 2024 at 11:02:40AM +0800, Jiawen Wu wrote:
> Consider the necessity of reading/writing the IO address to acquire and
> release the lock between software and firmware, add the IO address as
> the platform data to register I2C platform device.
...
> +#include <linux/platform_data/i2c-wx.h>
I don't like this. Can you provide a property for that or so?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 2/3] i2c: designware: add device private data passing to lock functions
[not found] <20240823030242.3083528-1-jiawenwu@trustnetic.com>
2024-08-23 3:02 ` [PATCH net 1/3] net: txgbe: add IO address in I2C platform device data Jiawen Wu
@ 2024-08-23 3:02 ` Jiawen Wu
2024-08-23 14:06 ` Andy Shevchenko
` (2 more replies)
2024-08-23 3:02 ` [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC Jiawen Wu
2 siblings, 3 replies; 10+ messages in thread
From: Jiawen Wu @ 2024-08-23 3:02 UTC (permalink / raw)
To: andi.shyti, jarkko.nikula, andriy.shevchenko, mika.westerberg,
jsd, davem, edumazet, kuba, pabeni, rmk+kernel, piotr.raczynski,
andrew, linux-i2c, netdev
Cc: mengyuanlou, duanqiangwen, Jiawen Wu, stable
In order to add the hardware lock for Wangxun devices with minimal
modification, pass struct dw_i2c_dev to the acquire and release lock
functions.
Cc: stable@vger.kernel.org
Fixes: 2f8d1ed79345 ("i2c: designware: Add driver support for Wangxun 10Gb NIC")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
drivers/i2c/busses/i2c-designware-amdpsp.c | 4 ++--
drivers/i2c/busses/i2c-designware-baytrail.c | 14 ++++++++++++--
drivers/i2c/busses/i2c-designware-common.c | 4 ++--
drivers/i2c/busses/i2c-designware-core.h | 4 ++--
4 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c
index 63454b06e5da..ee7cc4b33f4b 100644
--- a/drivers/i2c/busses/i2c-designware-amdpsp.c
+++ b/drivers/i2c/busses/i2c-designware-amdpsp.c
@@ -167,7 +167,7 @@ static void psp_release_i2c_bus_deferred(struct work_struct *work)
}
static DECLARE_DELAYED_WORK(release_queue, psp_release_i2c_bus_deferred);
-static int psp_acquire_i2c_bus(void)
+static int psp_acquire_i2c_bus(struct dw_i2c_dev *dev)
{
int status;
@@ -206,7 +206,7 @@ static int psp_acquire_i2c_bus(void)
return 0;
}
-static void psp_release_i2c_bus(void)
+static void psp_release_i2c_bus(struct dw_i2c_dev *dev)
{
mutex_lock(&psp_i2c_access_mutex);
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 45774aa47c28..9dde796e0fcc 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -12,6 +12,16 @@
#include "i2c-designware-core.h"
+static int iosf_mbi_block_punit_i2c_access_dev(struct dw_i2c_dev *dev)
+{
+ return iosf_mbi_block_punit_i2c_access();
+}
+
+static void iosf_mbi_unblock_punit_i2c_access_dev(struct dw_i2c_dev *dev)
+{
+ return iosf_mbi_unblock_punit_i2c_access();
+}
+
int i2c_dw_baytrail_probe_lock_support(struct dw_i2c_dev *dev)
{
acpi_status status;
@@ -36,8 +46,8 @@ int i2c_dw_baytrail_probe_lock_support(struct dw_i2c_dev *dev)
return -EPROBE_DEFER;
dev_info(dev->dev, "I2C bus managed by PUNIT\n");
- dev->acquire_lock = iosf_mbi_block_punit_i2c_access;
- dev->release_lock = iosf_mbi_unblock_punit_i2c_access;
+ dev->acquire_lock = iosf_mbi_block_punit_i2c_access_dev;
+ dev->release_lock = iosf_mbi_unblock_punit_i2c_access_dev;
dev->shared_with_punit = true;
return 0;
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e8a688d04aee..743875090356 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -524,7 +524,7 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev)
if (!dev->acquire_lock)
return 0;
- ret = dev->acquire_lock();
+ ret = dev->acquire_lock(dev);
if (!ret)
return 0;
@@ -536,7 +536,7 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev)
void i2c_dw_release_lock(struct dw_i2c_dev *dev)
{
if (dev->release_lock)
- dev->release_lock();
+ dev->release_lock(dev);
}
/*
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index e9606c00b8d1..12b77f464fb5 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -291,8 +291,8 @@ struct dw_i2c_dev {
u16 fp_lcnt;
u16 hs_hcnt;
u16 hs_lcnt;
- int (*acquire_lock)(void);
- void (*release_lock)(void);
+ int (*acquire_lock)(struct dw_i2c_dev *dev);
+ void (*release_lock)(struct dw_i2c_dev *dev);
int semaphore_idx;
bool shared_with_punit;
void (*disable)(struct dw_i2c_dev *dev);
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 2/3] i2c: designware: add device private data passing to lock functions
2024-08-23 3:02 ` [PATCH net 2/3] i2c: designware: add device private data passing to lock functions Jiawen Wu
@ 2024-08-23 14:06 ` Andy Shevchenko
2024-08-27 13:24 ` Paolo Abeni
2024-08-29 5:16 ` kernel test robot
2 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-08-23 14:06 UTC (permalink / raw)
To: Jiawen Wu
Cc: andi.shyti, jarkko.nikula, mika.westerberg, jsd, davem, edumazet,
kuba, pabeni, rmk+kernel, piotr.raczynski, andrew, linux-i2c,
netdev, mengyuanlou, duanqiangwen, stable
On Fri, Aug 23, 2024 at 11:02:41AM +0800, Jiawen Wu wrote:
> In order to add the hardware lock for Wangxun devices with minimal
> modification, pass struct dw_i2c_dev to the acquire and release lock
> functions.
...
> +static int iosf_mbi_block_punit_i2c_access_dev(struct dw_i2c_dev *dev)
> +static void iosf_mbi_unblock_punit_i2c_access_dev(struct dw_i2c_dev *dev)
Rather name them in accordance with the namespace of this module.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 2/3] i2c: designware: add device private data passing to lock functions
2024-08-23 3:02 ` [PATCH net 2/3] i2c: designware: add device private data passing to lock functions Jiawen Wu
2024-08-23 14:06 ` Andy Shevchenko
@ 2024-08-27 13:24 ` Paolo Abeni
2024-08-29 5:16 ` kernel test robot
2 siblings, 0 replies; 10+ messages in thread
From: Paolo Abeni @ 2024-08-27 13:24 UTC (permalink / raw)
To: Jiawen Wu, andi.shyti, jarkko.nikula, andriy.shevchenko,
mika.westerberg, jsd, davem, edumazet, kuba, rmk+kernel,
piotr.raczynski, andrew, linux-i2c, netdev
Cc: mengyuanlou, duanqiangwen, stable
On 8/23/24 05:02, Jiawen Wu wrote:
> In order to add the hardware lock for Wangxun devices with minimal
> modification, pass struct dw_i2c_dev to the acquire and release lock
> functions.
>
> Cc: stable@vger.kernel.org
> Fixes: 2f8d1ed79345 ("i2c: designware: Add driver support for Wangxun 10Gb NIC")
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
> drivers/i2c/busses/i2c-designware-amdpsp.c | 4 ++--
> drivers/i2c/busses/i2c-designware-baytrail.c | 14 ++++++++++++--
> drivers/i2c/busses/i2c-designware-common.c | 4 ++--
> drivers/i2c/busses/i2c-designware-core.h | 4 ++--
> 4 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c
> index 63454b06e5da..ee7cc4b33f4b 100644
> --- a/drivers/i2c/busses/i2c-designware-amdpsp.c
> +++ b/drivers/i2c/busses/i2c-designware-amdpsp.c
> @@ -167,7 +167,7 @@ static void psp_release_i2c_bus_deferred(struct work_struct *work)
> }
> static DECLARE_DELAYED_WORK(release_queue, psp_release_i2c_bus_deferred);
>
> -static int psp_acquire_i2c_bus(void)
> +static int psp_acquire_i2c_bus(struct dw_i2c_dev *dev)
> {
> int status;
>
This function is used in a few other places in this compilation unit.
You need to update all the users accordingly.
> @@ -206,7 +206,7 @@ static int psp_acquire_i2c_bus(void)
> return 0;
> }
>
> -static void psp_release_i2c_bus(void)
> +static void psp_release_i2c_bus(struct dw_i2c_dev *dev)
> {
> mutex_lock(&psp_i2c_access_mutex);
>
The same here.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net 2/3] i2c: designware: add device private data passing to lock functions
2024-08-23 3:02 ` [PATCH net 2/3] i2c: designware: add device private data passing to lock functions Jiawen Wu
2024-08-23 14:06 ` Andy Shevchenko
2024-08-27 13:24 ` Paolo Abeni
@ 2024-08-29 5:16 ` kernel test robot
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-08-29 5:16 UTC (permalink / raw)
To: Jiawen Wu, andi.shyti, jarkko.nikula, andriy.shevchenko,
mika.westerberg, jsd, davem, edumazet, kuba, pabeni, rmk+kernel,
piotr.raczynski, andrew, linux-i2c, netdev
Cc: llvm, oe-kbuild-all, mengyuanlou, duanqiangwen, Jiawen Wu, stable
Hi Jiawen,
kernel test robot noticed the following build errors:
[auto build test ERROR on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Jiawen-Wu/net-txgbe-add-IO-address-in-I2C-platform-device-data/20240826-122232
base: net/main
patch link: https://lore.kernel.org/r/20240823030242.3083528-3-jiawenwu%40trustnetic.com
patch subject: [PATCH net 2/3] i2c: designware: add device private data passing to lock functions
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240829/202408291212.L5DejDpz-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240829/202408291212.L5DejDpz-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408291212.L5DejDpz-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-designware-amdpsp.c:246:22: error: too few arguments to function call, single argument 'dev' was not specified
246 | psp_acquire_i2c_bus();
| ~~~~~~~~~~~~~~~~~~~ ^
drivers/i2c/busses/i2c-designware-amdpsp.c:170:12: note: 'psp_acquire_i2c_bus' declared here
170 | static int psp_acquire_i2c_bus(struct dw_i2c_dev *dev)
| ^ ~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-designware-amdpsp.c:259:22: error: too few arguments to function call, single argument 'dev' was not specified
259 | psp_acquire_i2c_bus();
| ~~~~~~~~~~~~~~~~~~~ ^
drivers/i2c/busses/i2c-designware-amdpsp.c:170:12: note: 'psp_acquire_i2c_bus' declared here
170 | static int psp_acquire_i2c_bus(struct dw_i2c_dev *dev)
| ^ ~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-designware-amdpsp.c:267:22: error: too few arguments to function call, single argument 'dev' was not specified
267 | psp_release_i2c_bus();
| ~~~~~~~~~~~~~~~~~~~ ^
drivers/i2c/busses/i2c-designware-amdpsp.c:209:13: note: 'psp_release_i2c_bus' declared here
209 | static void psp_release_i2c_bus(struct dw_i2c_dev *dev)
| ^ ~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
vim +/dev +246 drivers/i2c/busses/i2c-designware-amdpsp.c
78d5e9e299e31bc Jan Dabros 2022-02-08 235
78d5e9e299e31bc Jan Dabros 2022-02-08 236 /*
78d5e9e299e31bc Jan Dabros 2022-02-08 237 * Locking methods are based on the default implementation from
78d5e9e299e31bc Jan Dabros 2022-02-08 238 * drivers/i2c/i2c-core-base.c, but with psp acquire and release operations
78d5e9e299e31bc Jan Dabros 2022-02-08 239 * added. With this in place we can ensure that i2c clients on the bus shared
78d5e9e299e31bc Jan Dabros 2022-02-08 240 * with psp are able to lock HW access to the bus for arbitrary number of
78d5e9e299e31bc Jan Dabros 2022-02-08 241 * operations - that is e.g. write-wait-read.
78d5e9e299e31bc Jan Dabros 2022-02-08 242 */
78d5e9e299e31bc Jan Dabros 2022-02-08 243 static void i2c_adapter_dw_psp_lock_bus(struct i2c_adapter *adapter,
78d5e9e299e31bc Jan Dabros 2022-02-08 244 unsigned int flags)
78d5e9e299e31bc Jan Dabros 2022-02-08 245 {
78d5e9e299e31bc Jan Dabros 2022-02-08 @246 psp_acquire_i2c_bus();
78d5e9e299e31bc Jan Dabros 2022-02-08 247 rt_mutex_lock_nested(&adapter->bus_lock, i2c_adapter_depth(adapter));
78d5e9e299e31bc Jan Dabros 2022-02-08 248 }
78d5e9e299e31bc Jan Dabros 2022-02-08 249
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC
[not found] <20240823030242.3083528-1-jiawenwu@trustnetic.com>
2024-08-23 3:02 ` [PATCH net 1/3] net: txgbe: add IO address in I2C platform device data Jiawen Wu
2024-08-23 3:02 ` [PATCH net 2/3] i2c: designware: add device private data passing to lock functions Jiawen Wu
@ 2024-08-23 3:02 ` Jiawen Wu
2024-08-23 14:13 ` Andy Shevchenko
2 siblings, 1 reply; 10+ messages in thread
From: Jiawen Wu @ 2024-08-23 3:02 UTC (permalink / raw)
To: andi.shyti, jarkko.nikula, andriy.shevchenko, mika.westerberg,
jsd, davem, edumazet, kuba, pabeni, rmk+kernel, piotr.raczynski,
andrew, linux-i2c, netdev
Cc: mengyuanlou, duanqiangwen, Jiawen Wu, stable
Support acquire_lock() and release_lock() for Wangxun 10Gb NIC. Since the
firmware needs to access I2C all the time for some features, the semaphore
is used between software and firmware. The driver should set software
semaphore before accessing I2C bus and release it when it is finished.
Otherwise, there is probability that the correct information on I2C bus
will not be obtained.
Cc: stable@vger.kernel.org
Fixes: 2f8d1ed79345 ("i2c: designware: Add driver support for Wangxun 10Gb NIC")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-designware-core.h | 2 +
drivers/i2c/busses/i2c-designware-platdrv.c | 3 +
drivers/i2c/busses/i2c-designware-wx.c | 65 +++++++++++++++++++++
4 files changed, 71 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-designware-wx.c
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 78d0561339e5..f0ad9ebaacaa 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -58,6 +58,7 @@ i2c-designware-core-y += i2c-designware-master.o
i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-slave.o
obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o
i2c-designware-platform-y := i2c-designware-platdrv.o
+i2c-designware-platform-y += i2c-designware-wx.o
i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_AMDPSP) += i2c-designware-amdpsp.o
i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-baytrail.o
obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 12b77f464fb5..eae2c4cdc851 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -414,6 +414,8 @@ int i2c_dw_baytrail_probe_lock_support(struct dw_i2c_dev *dev);
int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev);
#endif
+int i2c_dw_txgbe_probe_lock_support(struct dw_i2c_dev *dev);
+
int i2c_dw_validate_speed(struct dw_i2c_dev *dev);
void i2c_dw_adjust_bus_speed(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index df3dc1e8093e..49c71ae5b6c1 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -229,6 +229,9 @@ static const struct i2c_dw_semaphore_callbacks i2c_dw_semaphore_cb_table[] = {
.probe = i2c_dw_amdpsp_probe_lock_support,
},
#endif
+ {
+ .probe = i2c_dw_txgbe_probe_lock_support,
+ },
{}
};
diff --git a/drivers/i2c/busses/i2c-designware-wx.c b/drivers/i2c/busses/i2c-designware-wx.c
new file mode 100644
index 000000000000..0f98160b956d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-wx.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */
+
+#include <linux/platform_data/i2c-wx.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+#include <linux/pci.h>
+
+#include "i2c-designware-core.h"
+
+#define I2C_DW_TXGBE_REQ_RETRY_CNT 4000
+#define I2C_DW_TXGBE_MNG_SW 0x1E004
+#define I2C_DW_TXGBE_MNG_SW_SM BIT(0)
+#define I2C_DW_TXGBE_FLUSH 0x10000
+
+static int i2c_dw_txgbe_acquire_lock(struct dw_i2c_dev *dev)
+{
+ void __iomem *req_addr;
+ u32 swsm;
+ int i;
+
+ req_addr = dev->ext + I2C_DW_TXGBE_MNG_SW;
+
+ for (i = 0; i < I2C_DW_TXGBE_REQ_RETRY_CNT; i++) {
+ writel(I2C_DW_TXGBE_MNG_SW_SM, req_addr);
+
+ /* If we set the bit successfully then we got semaphore. */
+ swsm = readl(req_addr);
+ if (swsm & I2C_DW_TXGBE_MNG_SW_SM)
+ break;
+
+ udelay(50);
+ }
+
+ if (i == I2C_DW_TXGBE_REQ_RETRY_CNT)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+static void i2c_dw_txgbe_release_lock(struct dw_i2c_dev *dev)
+{
+ writel(0, dev->ext + I2C_DW_TXGBE_MNG_SW);
+ /* flush register status */
+ readl(dev->ext + I2C_DW_TXGBE_FLUSH);
+}
+
+int i2c_dw_txgbe_probe_lock_support(struct dw_i2c_dev *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev->dev);
+ struct txgbe_i2c_platform_data *pdata;
+
+ pdata = dev_get_platdata(&pdev->dev);
+ if (!pdata)
+ return -ENXIO;
+
+ dev->ext = pdata->hw_addr;
+ if (!dev->ext)
+ return -ENXIO;
+
+ dev->acquire_lock = i2c_dw_txgbe_acquire_lock;
+ dev->release_lock = i2c_dw_txgbe_release_lock;
+
+ return 0;
+}
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC
2024-08-23 3:02 ` [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC Jiawen Wu
@ 2024-08-23 14:13 ` Andy Shevchenko
2024-08-29 9:15 ` Jiawen Wu
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2024-08-23 14:13 UTC (permalink / raw)
To: Jiawen Wu
Cc: andi.shyti, jarkko.nikula, mika.westerberg, jsd, davem, edumazet,
kuba, pabeni, rmk+kernel, piotr.raczynski, andrew, linux-i2c,
netdev, mengyuanlou, duanqiangwen, stable
On Fri, Aug 23, 2024 at 11:02:42AM +0800, Jiawen Wu wrote:
> Support acquire_lock() and release_lock() for Wangxun 10Gb NIC. Since the
> firmware needs to access I2C all the time for some features, the semaphore
> is used between software and firmware. The driver should set software
> semaphore before accessing I2C bus and release it when it is finished.
> Otherwise, there is probability that the correct information on I2C bus
> will not be obtained.
...
> i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-slave.o
> i2c-designware-platform-y := i2c-designware-platdrv.o
> +i2c-designware-platform-y += i2c-designware-wx.o
These lines have TABs/spaces mixture. Please fix at least your entry to avoid
this from happening.
...
> int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev);
> #endif
^^^
> +int i2c_dw_txgbe_probe_lock_support(struct dw_i2c_dev *dev);
See below.
...
> .probe = i2c_dw_amdpsp_probe_lock_support,
> },
> #endif
^^^
> + {
> + .probe = i2c_dw_txgbe_probe_lock_support,
> + },
Do we all need this support? Even if the driver is not compiled? Why?
...
> +#include <linux/platform_data/i2c-wx.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/pci.h>
This is a semi-random list. Please, take your time to understand the core you
wrote. Follow IWYU principle.
...
> +static int i2c_dw_txgbe_acquire_lock(struct dw_i2c_dev *dev)
> +{
> + void __iomem *req_addr;
> + u32 swsm;
> + int i;
> +
> + req_addr = dev->ext + I2C_DW_TXGBE_MNG_SW;
> +
> + for (i = 0; i < I2C_DW_TXGBE_REQ_RETRY_CNT; i++) {
Retry loops much better in a form of
unsigned int retries = ...;
...
do {
...
} while (--retries);
BUT... see below.
> + writel(I2C_DW_TXGBE_MNG_SW_SM, req_addr);
> +
> + /* If we set the bit successfully then we got semaphore. */
> + swsm = readl(req_addr);
> + if (swsm & I2C_DW_TXGBE_MNG_SW_SM)
> + break;
> +
> + udelay(50);
So, can a macro from iopoll.h be utilised here? Why not?
> + }
> +
> + if (i == I2C_DW_TXGBE_REQ_RETRY_CNT)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +int i2c_dw_txgbe_probe_lock_support(struct dw_i2c_dev *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev->dev);
Why do you need this dance? I.o.w. how pdev is being used here?
> + struct txgbe_i2c_platform_data *pdata;
> +
> + pdata = dev_get_platdata(&pdev->dev);
> + if (!pdata)
> + return -ENXIO;
> +
> + dev->ext = pdata->hw_addr;
> + if (!dev->ext)
> + return -ENXIO;
> +
> + dev->acquire_lock = i2c_dw_txgbe_acquire_lock;
> + dev->release_lock = i2c_dw_txgbe_release_lock;
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC
2024-08-23 14:13 ` Andy Shevchenko
@ 2024-08-29 9:15 ` Jiawen Wu
2024-08-29 10:59 ` 'Andy Shevchenko'
0 siblings, 1 reply; 10+ messages in thread
From: Jiawen Wu @ 2024-08-29 9:15 UTC (permalink / raw)
To: 'Andy Shevchenko'
Cc: andi.shyti, jarkko.nikula, mika.westerberg, jsd, davem, edumazet,
kuba, pabeni, rmk+kernel, andrew, linux-i2c, netdev, mengyuanlou,
duanqiangwen, stable
On Fri, Aug 23, 2024 10:13 PM, Andy Shevchenko wrote:
> On Fri, Aug 23, 2024 at 11:02:42AM +0800, Jiawen Wu wrote:
> > Support acquire_lock() and release_lock() for Wangxun 10Gb NIC. Since the
> > firmware needs to access I2C all the time for some features, the semaphore
> > is used between software and firmware. The driver should set software
> > semaphore before accessing I2C bus and release it when it is finished.
> > Otherwise, there is probability that the correct information on I2C bus
> > will not be obtained.
>
> ...
>
> > i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-slave.o
>
> > i2c-designware-platform-y := i2c-designware-platdrv.o
> > +i2c-designware-platform-y += i2c-designware-wx.o
>
> These lines have TABs/spaces mixture. Please fix at least your entry to avoid
> this from happening.
>
>
> ...
>
> > int i2c_dw_amdpsp_probe_lock_support(struct dw_i2c_dev *dev);
> > #endif
>
> ^^^
>
> > +int i2c_dw_txgbe_probe_lock_support(struct dw_i2c_dev *dev);
>
> See below.
>
> ...
>
> > .probe = i2c_dw_amdpsp_probe_lock_support,
> > },
> > #endif
>
> ^^^
>
> > + {
> > + .probe = i2c_dw_txgbe_probe_lock_support,
> > + },
>
> Do we all need this support? Even if the driver is not compiled? Why?
I'll add the macro CONFIG_I2C_DESIGNWARE_WX to control it.
> ...
>
> > +#include <linux/platform_data/i2c-wx.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/i2c.h>
> > +#include <linux/pci.h>
>
> This is a semi-random list. Please, take your time to understand the core you
> wrote. Follow IWYU principle.
>
> ...
>
> > +static int i2c_dw_txgbe_acquire_lock(struct dw_i2c_dev *dev)
> > +{
> > + void __iomem *req_addr;
> > + u32 swsm;
> > + int i;
> > +
> > + req_addr = dev->ext + I2C_DW_TXGBE_MNG_SW;
> > +
> > + for (i = 0; i < I2C_DW_TXGBE_REQ_RETRY_CNT; i++) {
>
> Retry loops much better in a form of
>
> unsigned int retries = ...;
> ...
> do {
> ...
> } while (--retries);
>
> BUT... see below.
>
> > + writel(I2C_DW_TXGBE_MNG_SW_SM, req_addr);
> > +
> > + /* If we set the bit successfully then we got semaphore. */
> > + swsm = readl(req_addr);
> > + if (swsm & I2C_DW_TXGBE_MNG_SW_SM)
> > + break;
> > +
> > + udelay(50);
>
> So, can a macro from iopoll.h be utilised here? Why not?
I need to write the register first and then read it in this loop.
It does not seem to apply to the macros in iopoll.h.
> > + }
> > +
> > + if (i == I2C_DW_TXGBE_REQ_RETRY_CNT)
> > + return -ETIMEDOUT;
> > +
> > + return 0;
> > +}
>
> > +int i2c_dw_txgbe_probe_lock_support(struct dw_i2c_dev *dev)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev->dev);
>
> Why do you need this dance? I.o.w. how pdev is being used here?
I'll change to add the data in node property.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net 3/3] i2c: designware: support hardware lock for Wangxun 10Gb NIC
2024-08-29 9:15 ` Jiawen Wu
@ 2024-08-29 10:59 ` 'Andy Shevchenko'
0 siblings, 0 replies; 10+ messages in thread
From: 'Andy Shevchenko' @ 2024-08-29 10:59 UTC (permalink / raw)
To: Jiawen Wu
Cc: andi.shyti, jarkko.nikula, mika.westerberg, jsd, davem, edumazet,
kuba, pabeni, rmk+kernel, andrew, linux-i2c, netdev, mengyuanlou,
duanqiangwen, stable
On Thu, Aug 29, 2024 at 05:15:42PM +0800, Jiawen Wu wrote:
> On Fri, Aug 23, 2024 10:13 PM, Andy Shevchenko wrote:
> > On Fri, Aug 23, 2024 at 11:02:42AM +0800, Jiawen Wu wrote:
...
> > > +static int i2c_dw_txgbe_acquire_lock(struct dw_i2c_dev *dev)
> > > +{
> > > + void __iomem *req_addr;
> > > + u32 swsm;
> > > + int i;
> > > +
> > > + req_addr = dev->ext + I2C_DW_TXGBE_MNG_SW;
> > > +
> > > + for (i = 0; i < I2C_DW_TXGBE_REQ_RETRY_CNT; i++) {
> >
> > Retry loops much better in a form of
> >
> > unsigned int retries = ...;
> > ...
> > do {
> > ...
> > } while (--retries);
> >
> > BUT... see below.
> >
> > > + writel(I2C_DW_TXGBE_MNG_SW_SM, req_addr);
> > > +
> > > + /* If we set the bit successfully then we got semaphore. */
> > > + swsm = readl(req_addr);
> > > + if (swsm & I2C_DW_TXGBE_MNG_SW_SM)
> > > + break;
> > > +
> > > + udelay(50);
> >
> > So, can a macro from iopoll.h be utilised here? Why not?
>
> I need to write the register first and then read it in this loop.
> It does not seem to apply to the macros in iopoll.h.
I don't see how does it prevent from using read_poll_timeout() macro.
You need to implement a body of the loop as a helper function that you supply
into macro as a parameter.
> > > + }
> > > +
> > > + if (i == I2C_DW_TXGBE_REQ_RETRY_CNT)
> > > + return -ETIMEDOUT;
> > > +
> > > + return 0;
> > > +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread