[2/3] i2c: thunderx: Add support for High speed mode
Commit Message
From: Suneel Garapati <sgarapati@marvell.com>
Support High speed mode clock setup for OcteonTX2 platforms.
Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
---
drivers/i2c/busses/i2c-octeon-core.c | 78 ++++++++++++++++--------
drivers/i2c/busses/i2c-octeon-core.h | 3 +
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 3 +-
3 files changed, 57 insertions(+), 27 deletions(-)
Comments
Hi Piyush,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.3-rc4 next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Piyush-Malgujar/i2c-thunderx-Clock-divisor-logic-changes/20230330-214626
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230330133953.21074-3-pmalgujar%40marvell.com
patch subject: [PATCH 2/3] i2c: thunderx: Add support for High speed mode
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20230330/202303302309.SSHrlrqN-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
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/c7866465c9142bf77cc1bc651704bfbfc9b0b411
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Piyush-Malgujar/i2c-thunderx-Clock-divisor-logic-changes/20230330-214626
git checkout c7866465c9142bf77cc1bc651704bfbfc9b0b411
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/i2c/busses/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303302309.SSHrlrqN-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-octeon-core.c: In function 'octeon_i2c_wait':
>> drivers/i2c/busses/i2c-octeon-core.c:74:33: error: implicit declaration of function '__udelay'; did you mean '__delay'? [-Werror=implicit-function-declaration]
74 | __udelay(1);
| ^~~~~~~~
| __delay
cc1: some warnings being treated as errors
vim +74 drivers/i2c/busses/i2c-octeon-core.c
39
40 /**
41 * octeon_i2c_wait - wait for the IFLG to be set
42 * @i2c: The struct octeon_i2c
43 *
44 * Returns 0 on success, otherwise a negative errno.
45 */
46 static int octeon_i2c_wait(struct octeon_i2c *i2c)
47 {
48 long time_left;
49
50 /*
51 * Some chip revisions don't assert the irq in the interrupt
52 * controller. So we must poll for the IFLG change.
53 */
54 if (i2c->broken_irq_mode) {
55 u64 end = get_jiffies_64() + i2c->adap.timeout;
56
57 while (!octeon_i2c_test_iflg(i2c) &&
58 time_before64(get_jiffies_64(), end))
59 usleep_range(I2C_OCTEON_EVENT_WAIT / 2, I2C_OCTEON_EVENT_WAIT);
60
61 return octeon_i2c_test_iflg(i2c) ? 0 : -ETIMEDOUT;
62 }
63
64 if (IS_LS_FREQ(i2c->twsi_freq)) {
65 i2c->int_enable(i2c);
66 time_left = wait_event_timeout(i2c->queue,
67 octeon_i2c_test_iflg(i2c),
68 i2c->adap.timeout);
69 i2c->int_disable(i2c);
70 } else {
71 time_left = 1000; /* 1ms */
72 do {
73 if (time_left--)
> 74 __udelay(1);
75 } while (!octeon_i2c_test_iflg(i2c) && time_left);
76 }
77
78 if (i2c->broken_irq_check && !time_left &&
79 octeon_i2c_test_iflg(i2c)) {
80 dev_err(i2c->dev, "broken irq connection detected, switching to polling mode.\n");
81 i2c->broken_irq_mode = true;
82 return 0;
83 }
84
85 if (!time_left)
86 return -ETIMEDOUT;
87
88 return 0;
89 }
90
Hi Piyush,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.3-rc4 next-20230330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Piyush-Malgujar/i2c-thunderx-Clock-divisor-logic-changes/20230330-214626
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230330133953.21074-3-pmalgujar%40marvell.com
patch subject: [PATCH 2/3] i2c: thunderx: Add support for High speed mode
config: riscv-randconfig-r042-20230329 (https://download.01.org/0day-ci/archive/20230331/202303310153.yx2xXH8s-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/c7866465c9142bf77cc1bc651704bfbfc9b0b411
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Piyush-Malgujar/i2c-thunderx-Clock-divisor-logic-changes/20230330-214626
git checkout c7866465c9142bf77cc1bc651704bfbfc9b0b411
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/i2c/busses/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303310153.yx2xXH8s-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-octeon-core.c:74:5: error: call to undeclared function '__udelay'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
__udelay(1);
^
drivers/i2c/busses/i2c-octeon-core.c:74:5: note: did you mean '__delay'?
arch/riscv/include/asm/delay.h:18:13: note: '__delay' declared here
extern void __delay(unsigned long cycles);
^
1 error generated.
vim +/__udelay +74 drivers/i2c/busses/i2c-octeon-core.c
39
40 /**
41 * octeon_i2c_wait - wait for the IFLG to be set
42 * @i2c: The struct octeon_i2c
43 *
44 * Returns 0 on success, otherwise a negative errno.
45 */
46 static int octeon_i2c_wait(struct octeon_i2c *i2c)
47 {
48 long time_left;
49
50 /*
51 * Some chip revisions don't assert the irq in the interrupt
52 * controller. So we must poll for the IFLG change.
53 */
54 if (i2c->broken_irq_mode) {
55 u64 end = get_jiffies_64() + i2c->adap.timeout;
56
57 while (!octeon_i2c_test_iflg(i2c) &&
58 time_before64(get_jiffies_64(), end))
59 usleep_range(I2C_OCTEON_EVENT_WAIT / 2, I2C_OCTEON_EVENT_WAIT);
60
61 return octeon_i2c_test_iflg(i2c) ? 0 : -ETIMEDOUT;
62 }
63
64 if (IS_LS_FREQ(i2c->twsi_freq)) {
65 i2c->int_enable(i2c);
66 time_left = wait_event_timeout(i2c->queue,
67 octeon_i2c_test_iflg(i2c),
68 i2c->adap.timeout);
69 i2c->int_disable(i2c);
70 } else {
71 time_left = 1000; /* 1ms */
72 do {
73 if (time_left--)
> 74 __udelay(1);
75 } while (!octeon_i2c_test_iflg(i2c) && time_left);
76 }
77
78 if (i2c->broken_irq_check && !time_left &&
79 octeon_i2c_test_iflg(i2c)) {
80 dev_err(i2c->dev, "broken irq connection detected, switching to polling mode.\n");
81 i2c->broken_irq_mode = true;
82 return 0;
83 }
84
85 if (!time_left)
86 return -ETIMEDOUT;
87
88 return 0;
89 }
90
Hi Suneel and Piyush,
[...]
> @@ -61,10 +61,19 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
> return octeon_i2c_test_iflg(i2c) ? 0 : -ETIMEDOUT;
> }
>
> - i2c->int_enable(i2c);
> - time_left = wait_event_timeout(i2c->queue, octeon_i2c_test_iflg(i2c),
> - i2c->adap.timeout);
> - i2c->int_disable(i2c);
> + if (IS_LS_FREQ(i2c->twsi_freq)) {
> + i2c->int_enable(i2c);
> + time_left = wait_event_timeout(i2c->queue,
> + octeon_i2c_test_iflg(i2c),
> + i2c->adap.timeout);
> + i2c->int_disable(i2c);
> + } else {
> + time_left = 1000; /* 1ms */
> + do {
> + if (time_left--)
> + __udelay(1);
Are you sure you want to wait 1ms with __udelay(). This is a bit
dsruptive, can we use a more relaxed waiting method?
> + } while (!octeon_i2c_test_iflg(i2c) && time_left);
> + }
[...]
> * Find divisors to produce target frequency, start with large delta
> * to cover wider range of divisors, note thp = TCLK half period.
> */
> - int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
> + int ds = 10, thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
unsigned?
[...]
> + if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
> + u64 mode;
> +
> + mode = __raw_readq(i2c->twsi_base + MODE(i2c));
> + /* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
> + if (!IS_LS_FREQ(i2c->twsi_freq))
> + mode |= BIT(4) | BIT(0);
> + else
> + mode &= ~(BIT(4) | BIT(0));
would be nice to have this defined and with some meaning as
comment.
> + octeon_i2c_writeq_flush(mode, i2c->twsi_base + MODE(i2c));
> + }
Robert, any comment here?
Thanks,
Andi
@@ -61,10 +61,19 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)
return octeon_i2c_test_iflg(i2c) ? 0 : -ETIMEDOUT;
}
- i2c->int_enable(i2c);
- time_left = wait_event_timeout(i2c->queue, octeon_i2c_test_iflg(i2c),
- i2c->adap.timeout);
- i2c->int_disable(i2c);
+ if (IS_LS_FREQ(i2c->twsi_freq)) {
+ i2c->int_enable(i2c);
+ time_left = wait_event_timeout(i2c->queue,
+ octeon_i2c_test_iflg(i2c),
+ i2c->adap.timeout);
+ i2c->int_disable(i2c);
+ } else {
+ time_left = 1000; /* 1ms */
+ do {
+ if (time_left--)
+ __udelay(1);
+ } while (!octeon_i2c_test_iflg(i2c) && time_left);
+ }
if (i2c->broken_irq_check && !time_left &&
octeon_i2c_test_iflg(i2c)) {
@@ -608,25 +617,27 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
struct octeon_i2c *i2c = i2c_get_adapdata(adap);
int i, ret = 0;
- if (num == 1) {
- if (msgs[0].len > 0 && msgs[0].len <= 8) {
- if (msgs[0].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_write(i2c, msgs);
- goto out;
- }
- } else if (num == 2) {
- if ((msgs[0].flags & I2C_M_RD) == 0 &&
- (msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
- msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
- msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (IS_LS_FREQ(i2c->twsi_freq)) {
+ if (num == 1) {
+ if (msgs[0].len > 0 && msgs[0].len <= 8) {
+ if (msgs[0].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_write(i2c, msgs);
+ goto out;
+ }
+ } else if (num == 2) {
+ if ((msgs[0].flags & I2C_M_RD) == 0 &&
+ (msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
+ msgs[0].len > 0 && msgs[0].len <= 2 &&
+ msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[0].addr == msgs[1].addr) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
@@ -666,11 +677,13 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
* Find divisors to produce target frequency, start with large delta
* to cover wider range of divisors, note thp = TCLK half period.
*/
- int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
+ int ds = 10, thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
thp = 0x3;
mdiv_min = 0;
+ if (!IS_LS_FREQ(i2c->twsi_freq))
+ ds = 15;
}
for (ndiv_idx = 0; ndiv_idx < 8 && delta_hz != 0; ndiv_idx++) {
@@ -683,7 +696,7 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
* For given ndiv and mdiv values check the
* two closest thp values.
*/
- tclk = i2c->twsi_freq * (mdiv_idx + 1) * 10;
+ tclk = i2c->twsi_freq * (mdiv_idx + 1) * ds;
tclk *= (1 << ndiv_idx);
if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev)))
thp_base = (i2c->sys_freq / tclk) - 2;
@@ -701,7 +714,9 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
foscl = i2c->sys_freq /
(2 * (thp_idx + 1));
foscl = foscl / (1 << ndiv_idx);
- foscl = foscl / (mdiv_idx + 1) / 10;
+ foscl = foscl / (mdiv_idx + 1) / ds;
+ if (foscl > i2c->twsi_freq)
+ continue;
diff = abs(foscl - i2c->twsi_freq);
/* Use it if smaller diff from target */
if (diff < delta_hz) {
@@ -715,6 +730,17 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
}
octeon_i2c_reg_write(i2c, SW_TWSI_OP_TWSI_CLK, thp);
octeon_i2c_reg_write(i2c, SW_TWSI_EOP_TWSI_CLKCTL, (mdiv << 3) | ndiv);
+ if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
+ u64 mode;
+
+ mode = __raw_readq(i2c->twsi_base + MODE(i2c));
+ /* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
+ if (!IS_LS_FREQ(i2c->twsi_freq))
+ mode |= BIT(4) | BIT(0);
+ else
+ mode &= ~(BIT(4) | BIT(0));
+ octeon_i2c_writeq_flush(mode, i2c->twsi_base + MODE(i2c));
+ }
}
int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
@@ -93,11 +93,13 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int mode;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define MODE(x) (x->roff.mode)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -212,6 +214,7 @@ static inline void octeon_i2c_write_int(struct octeon_i2c *i2c, u64 data)
octeon_i2c_writeq_flush(data, i2c->twsi_base + TWSI_INT(i2c));
}
+#define IS_LS_FREQ(twsi_freq) ((twsi_freq) <= 400000)
#define PCI_SUBSYS_DEVID_9XXX 0xB
/**
* octeon_i2c_is_otx2 - check for chip ID
@@ -165,6 +165,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.mode = 0x1038;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
@@ -209,7 +210,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
* For OcteonTX2 chips, set reference frequency to 100MHz
* as refclk_src in TWSI_MODE register defaults to 100MHz.
*/
- if (octeon_i2c_is_otx2(pdev))
+ if (octeon_i2c_is_otx2(pdev) && IS_LS_FREQ(i2c->twsi_freq))
i2c->sys_freq = 100000000;
octeon_i2c_set_clock(i2c);