2023-03-30 13:45:19

by Piyush Malgujar

[permalink] [raw]
Subject: [PATCH 0/3] i2c: thunderx: Marvell thunderx i2c changes

The changes are for Marvell OcteonTX2 SOC family:

- Handling clock divisor logic using subsytem ID
- Support for high speed mode
- Handle watchdog timeout

Suneel Garapati (3):
i2c: thunderx: Clock divisor logic changes
i2c: thunderx: Add support for High speed mode
i2c: octeon: Handle watchdog timeout

drivers/i2c/busses/i2c-octeon-core.c | 113 +++++++++++++++++------
drivers/i2c/busses/i2c-octeon-core.h | 19 ++++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 7 ++
3 files changed, 110 insertions(+), 29 deletions(-)

--
2.17.1


2023-03-30 13:45:21

by Piyush Malgujar

[permalink] [raw]
Subject: [PATCH 2/3] i2c: thunderx: Add support for High speed mode

From: Suneel Garapati <[email protected]>

Support High speed mode clock setup for OcteonTX2 platforms.

Signed-off-by: Suneel Garapati <[email protected]>
Signed-off-by: Piyush Malgujar <[email protected]>
---
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(-)

diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index dfd58bbec47b1f0554ae0c100c680b6ba9be61ec..7c49dc8ccbd2ef05fec675d282193b98f2b69835 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -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)
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 8a0033c94a8a291fb255b0da03858274035c46f4..89d7d3bb8e30bd5787978d17d5a9b20ab0d41e22 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -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
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index eecd27f9f1730e522dcccafc9f12ea891a3b59ef..3dd5a4d798f99e9b5282360cf9d5840042fc8dcc 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -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);

--
2.17.1

2023-03-30 15:50:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/3] i2c: thunderx: Add support for High speed mode

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-03-30 17:55:59

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/3] i2c: thunderx: Add support for High speed mode

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-06-10 12:58:42

by Andi Shyti

[permalink] [raw]
Subject: Re: [PATCH 2/3] i2c: thunderx: Add support for High speed mode

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