i2c-octeon and i2c-thunderx drivers
Subsystems (3)
v6 cover i2c-octeon and i2c-thunderx drivers
Hi Wolfram,
in the meantime I've converted the octeon driver to use the i2c recovery framework. I thing this makes the code easier to follow. I've also cleaned up some other aspects, like the register access and fixed some bugs I found while going over the code again.
This series for the Octeon i2c driver is an attempt to upstream some bug fixes and features that accumulated for some time.
On top of the Octeon changes a i2c driver for the ThunderX SOC is added which uses the same functional block as the Octeon driver.
Patches are on top of 4.6-rc3 and were tested on OCTEON, OCTEON-78 and ThunderX.
Changes to v5:
- Switch to i2c recovery framework
- Clean-up register access, introduce new helper functions
- Fixed ready bit check in combined write
- Fixed IFLG clear in hlc_enable
- Removed complicated last phase logic, not needed when we send START for every message part
Changes to v4:
- Splitted the High-Level Controller patch into several patches
- Reworded some commit messages
Changes to v3:
- Added more functionality flags for SMBUS
- Removed both module parameters
- Make xfer return also other errors than EGAIN
- Return EPROTO on invalid SMBUS block length
- Use devm_ioremap_resource
- Added rename-only patch
- Removed kerneldoc patch from series
- Improved defines
Changes to v2:
- Split clenaup patch into several patches
- Strictly moved functional changes to later patches
- Fixed do-while checkpatch errors
- Moved defines to the patches that use them
- Use BIT_ULL macro
- Split ThunderX patch into 2 patches
Changes to v1:
- Fixed compile error on x86_64
- Disabled thunderx driver on MIPS
- Re-ordered some thunderx probe functions for readability
- Fix missing of_irq.h and i2c-smbus.h includes
- Use IS_ENABLED for CONFIG options
Jan
-------------------------------------------------
David Daney (3):
i2c: octeon: Enable High-Level Controller
i2c: octeon: Add support for cn78xx chips
i2c: octeon: Add workaround for broken irqs on CN3860
Jan Glauber (14):
i2c: octeon: Increase retry default and use fixed timeout value
i2c: octeon: Move set-clock and init-lowlevel upward
i2c: octeon: Rename [read|write]_sw to reg_[read|write]
i2c: octeon: Introduce helper functions for register access
i2c: octeon: Remove superfluous check in octeon_i2c_test_iflg
i2c: octeon: Improve error status checking
i2c: octeon: Use i2c recovery framework
dt-bindings: i2c: Add Octeon cn78xx TWSI
i2c: octeon: Move read function before write
i2c: octeon: Rename driver to prepare for split
i2c: octeon: Split the driver into two parts
i2c: thunderx: Add i2c driver for ThunderX SOC
i2c: octeon,thunderx: Move register offsets to struct
i2c: thunderx: Add smbus alert support
Peter Swain (2):
i2c: octeon: Flush TWSI writes with readback
i2c: octeon: Faster operation when IFLG signals late
.../devicetree/bindings/i2c/i2c-octeon.txt | 6 + drivers/i2c/busses/Kconfig | 10 + drivers/i2c/busses/Makefile | 3 + drivers/i2c/busses/i2c-cavium.c | 805 +++++++++++++++++++++ drivers/i2c/busses/i2c-cavium.h | 218 ++++++ drivers/i2c/busses/i2c-octeon-core.c | 288 ++++++++ drivers/i2c/busses/i2c-octeon.c | 600 --------------- drivers/i2c/busses/i2c-thunderx-core.c | 308 ++++++++ 8 files changed, 1638 insertions(+), 600 deletions(-) create mode 100644 drivers/i2c/busses/i2c-cavium.c create mode 100644 drivers/i2c/busses/i2c-cavium.h create mode 100644 drivers/i2c/busses/i2c-octeon-core.c delete mode 100644 drivers/i2c/busses/i2c-octeon.c create mode 100644 drivers/i2c/busses/i2c-thunderx-core.c
Signature · 1 line
1.9.1
PATCH v6 1/19 i2c: octeon: Increase retry default and use fixed timeout value
Convert the adapter timeout to 2 ms independently of depending on CONFIG_HZ. CONFIG_HZ is 100 for MIPS Cavium-Octeon so the timeout value is not changed.
Also set retries to 5 to improve robustness.
--- drivers/i2c/busses/i2c-octeon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/i2c/busses/i2c-octeon.c +2 -1
@@ -426,7 +426,6 @@ static struct i2c_adapter octeon_i2c_ops = { .owner = THIS_MODULE, .name = "OCTEON adapter", .algo = &octeon_i2c_algo, - .timeout = HZ / 50, };/* calculate and set clock divisors */
@@ -553,6 +552,8 @@ static int octeon_i2c_probe(struct platform_device *pdev)octeon_i2c_set_clock (i2c); i2c->adap = octeon_i2c_ops; + i2c->adap.timeout =msecs_to_jiffies (2 ); + i2c->adap.retries =5 ; i2c->adap.dev.parent = &pdev->dev; i2c->adap.dev.of_node = node;i2c_set_adapdata (&i2c->adap, i2c);
Signature · 1 line
1.9.1
Wolfram Sang
AppliedRe: [PATCH v6 01/19] i2c: octeon: Increase retry default and use fixed timeout value
Quoted 6 lines · Jan Glauber
Convert the adapter timeout to 2 ms independently of depending on
CONFIG_HZ.CONFIG_HZis 100 for MIPS Cavium-Octeon so the timeout value is not changed.Also set retries to 5 to improve robustness.
Signed-off-by Jan Glauber <[email protected]>
Applied to for-next, thanks!
PATCH v6 2/19 i2c: octeon: Move set-clock and init-lowlevel upward
In reply to Jan GlauberNo functional change, just moving the functions upward in preparation of improving the recovery.
--- drivers/i2c/busses/i2c-octeon.c | 126 ++++++++++++++++++++-------------------- 1 file changed, 63 insertions(+), 63 deletions(-)
drivers/i2c/busses/i2c-octeon.c +63 -63
@@ -214,6 +214,69 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)return 0 ; } +/* calculate and set clock divisors */ +static void octeon_i2c_set_clock (struct octeon_i2c *i2c) +{ +int tclk, thp_base, inc, thp_idx, mdiv_idx, ndiv_idx, foscl, diff; +int thp =0x18 , mdiv =2 , ndiv =0 , delta_hz =1000000 ; + +for (ndiv_idx =0 ; ndiv_idx <8 && delta_hz !=0 ; ndiv_idx++) { +/* +* An mdiv value of less than 2 seems to not work well +* with ds1337 RTCs, so we constrain it to larger values. +*/ +for (mdiv_idx =15 ; mdiv_idx >=2 && delta_hz !=0 ; mdiv_idx--) { +/* +* For given ndiv and mdiv values check the +* two closest thp values. +*/ + tclk = i2c->twsi_freq * (mdiv_idx +1 ) *10 ; + tclk *= (1 << ndiv_idx); + thp_base = (i2c->sys_freq / (tclk *2 )) -1 ; + +for (inc =0 ; inc <=1 ; inc++) { + thp_idx = thp_base + inc; +if (thp_idx <5 || thp_idx >0xff ) +continue ; + + foscl = i2c->sys_freq / (2 * (thp_idx +1 )); + foscl = foscl / (1 << ndiv_idx); + foscl = foscl / (mdiv_idx +1 ) /10 ; + diff =abs (foscl - i2c->twsi_freq); +if (diff < delta_hz) { + delta_hz = diff; + thp = thp_idx; + mdiv = mdiv_idx; + ndiv = ndiv_idx; + } + } + } + } +octeon_i2c_write_sw (i2c,SW_TWSI_OP_TWSI_CLK , thp); +octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CLKCTL , (mdiv <<3 ) | ndiv); +} + +static int octeon_i2c_init_lowlevel (struct octeon_i2c *i2c) +{ +u8 status; +int tries; + +/* disable high level controller, enable bus access */ +octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); + +/* reset controller */ +octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_RST ,0 ); + +for (tries =10 ; tries; tries--) { +udelay (1 ); + status =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ); +if (status ==STAT_IDLE ) +return 0 ; + } +dev_err (i2c->dev,"%s: TWSI_RST failed! (0x%x)\n" , __func__, status); +return -EIO ; +} +/** * octeon_i2c_start - send START to the bus * @i2c: The struct octeon_i2c
@@ -428,69 +491,6 @@ static struct i2c_adapter octeon_i2c_ops = { .algo = &octeon_i2c_algo, }; -/* calculate and set clock divisors */ -static void octeon_i2c_set_clock (struct octeon_i2c *i2c) -{ -int tclk, thp_base, inc, thp_idx, mdiv_idx, ndiv_idx, foscl, diff; -int thp =0x18 , mdiv =2 , ndiv =0 , delta_hz =1000000 ; - -for (ndiv_idx =0 ; ndiv_idx <8 && delta_hz !=0 ; ndiv_idx++) { -/* -* An mdiv value of less than 2 seems to not work well -* with ds1337 RTCs, so we constrain it to larger values. -*/ -for (mdiv_idx =15 ; mdiv_idx >=2 && delta_hz !=0 ; mdiv_idx--) { -/* -* For given ndiv and mdiv values check the -* two closest thp values. -*/ - tclk = i2c->twsi_freq * (mdiv_idx +1 ) *10 ; - tclk *= (1 << ndiv_idx); - thp_base = (i2c->sys_freq / (tclk *2 )) -1 ; - -for (inc =0 ; inc <=1 ; inc++) { - thp_idx = thp_base + inc; -if (thp_idx <5 || thp_idx >0xff ) -continue ; - - foscl = i2c->sys_freq / (2 * (thp_idx +1 )); - foscl = foscl / (1 << ndiv_idx); - foscl = foscl / (mdiv_idx +1 ) /10 ; - diff =abs (foscl - i2c->twsi_freq); -if (diff < delta_hz) { - delta_hz = diff; - thp = thp_idx; - mdiv = mdiv_idx; - ndiv = ndiv_idx; - } - } - } - } -octeon_i2c_write_sw (i2c,SW_TWSI_OP_TWSI_CLK , thp); -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CLKCTL , (mdiv <<3 ) | ndiv); -} - -static int octeon_i2c_init_lowlevel (struct octeon_i2c *i2c) -{ -u8 status; -int tries; - -/* disable high level controller, enable bus access */ -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); - -/* reset controller */ -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_RST ,0 ); - -for (tries =10 ; tries; tries--) { -udelay (1 ); - status =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ); -if (status ==STAT_IDLE ) -return 0 ; - } -dev_err (i2c->dev,"%s: TWSI_RST failed! (0x%x)\n" , __func__, status); -return -EIO ; -} -static int octeon_i2c_probe (struct platform_device *pdev) {struct device_node *node = pdev->dev.of_node;
Signature · 1 line
1.9.1
Wolfram Sang
AppliedRe: [PATCH v6 02/19] i2c: octeon: Move set-clock and init-lowlevel upward
Quoted 4 lines · Jan Glauber
No functional change, just moving the functions upward in preparation of improving the recovery.
Signed-off-by Jan Glauber <[email protected]>
Applied to for-next, thanks!
PATCH v6 3/19 i2c: octeon: Rename [read|write]_sw to reg_[read|write]
In reply to Jan Glauberocteon_i2c_read_sw -> octeon_i2c_reg_readocteon_i2c_write_sw -> octeon_i2c_reg_write
--- drivers/i2c/busses/i2c-octeon.c | 52 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
drivers/i2c/busses/i2c-octeon.c +26 -26
@@ -80,14 +80,14 @@ struct octeon_i2c { };/** -* octeon_i2c_write_sw - write an I2C core register +* octeon_i2c_reg_write - write an I2C core register * @i2c: The struct octeon_i2c * @eop_reg: Register selector * @data: Value to be written * * The I2C core registers are accessed indirectly via the SW_TWSI CSR. */ -static void octeon_i2c_write_sw (struct octeon_i2c *i2c,u64 eop_reg,u8 data) +static void octeon_i2c_reg_write (struct octeon_i2c *i2c,u64 eop_reg,u8 data) {u64 tmp;
@@ -98,7 +98,7 @@ static void octeon_i2c_write_sw(struct octeon_i2c *i2c, u64 eop_reg, u8 data) }/** -* octeon_i2c_read_sw - read lower bits of an I2C core register +* octeon_i2c_reg_read - read lower bits of an I2C core register * @i2c: The struct octeon_i2c * @eop_reg: Register selector *
@@ -106,7 +106,7 @@ static void octeon_i2c_write_sw(struct octeon_i2c *i2c, u64 eop_reg, u8 data)* * The I2C core registers are accessed indirectly via the SW_TWSI CSR. */ -static u8 octeon_i2c_read_sw (struct octeon_i2c *i2c,u64 eop_reg) +static u8 octeon_i2c_reg_read (struct octeon_i2c *i2c,u64 eop_reg) {u64 tmp;
@@ -189,7 +189,7 @@ static irqreturn_t octeon_i2c_isr(int irq, void *dev_id)static int octeon_i2c_test_iflg (struct octeon_i2c *i2c) { -return (octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_CTL ) &TWSI_CTL_IFLG ) !=0 ; +return (octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_CTL ) &TWSI_CTL_IFLG ) !=0 ; }/**
@@ -252,8 +252,8 @@ static void octeon_i2c_set_clock(struct octeon_i2c *i2c) } } } -octeon_i2c_write_sw (i2c,SW_TWSI_OP_TWSI_CLK , thp); -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CLKCTL , (mdiv <<3 ) | ndiv); +octeon_i2c_reg_write (i2c,SW_TWSI_OP_TWSI_CLK , thp); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CLKCTL , (mdiv <<3 ) | ndiv); }static int octeon_i2c_init_lowlevel (struct octeon_i2c *i2c)
@@ -262,14 +262,14 @@ static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)int tries;/* disable high level controller, enable bus access */ -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB );/* reset controller */ -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_RST ,0 ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_RST ,0 );for (tries =10 ; tries; tries--) {udelay (1 ); - status =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ); + status =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT );if (status ==STAT_IDLE )return 0 ; }
@@ -288,19 +288,19 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)int result;u8 data; -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL , +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB |TWSI_CTL_STA ); result =octeon_i2c_wait (i2c);if (result) { -if (octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ) ==STAT_IDLE ) { +if (octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT ) ==STAT_IDLE ) {/* * Controller refused to send start flag May * be a client is holding SDA low - let's try * to free it. */ octeon_i2c_unblock (i2c); -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL , +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB |TWSI_CTL_STA ); result =octeon_i2c_wait (i2c); }
@@ -308,7 +308,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)return result; } - data =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ); + data =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT );if ((data !=STAT_START ) && (data !=STAT_RSTART )) {dev_err (i2c->dev,"%s: bad status (0x%x)\n" , __func__, data);return -EIO ;
@@ -320,7 +320,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)/* send STOP to the bus */ static void octeon_i2c_stop (struct octeon_i2c *i2c) { -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL , +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB |TWSI_CTL_STP ); }
@@ -345,15 +345,15 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,if (result)return result; -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_DATA , target <<1 ); -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_DATA , target <<1 ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)return result;for (i =0 ; i < length; i++) { - tmp =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ); + tmp =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT );if ((tmp !=STAT_TXADDR_ACK ) && (tmp !=STAT_TXDATA_ACK )) { dev_err(i2c->dev,
@@ -362,8 +362,8 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,return -EIO ; } -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_DATA , data[i]); -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_DATA , data[i]); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)
@@ -398,15 +398,15 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,if (result)return result; -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_DATA , (target <<1 ) |1 ); -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_DATA , (target <<1 ) |1 ); +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)return result;for (i =0 ; i < length; i++) { - tmp =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_STAT ); + tmp =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT );if ((tmp !=STAT_RXDATA_ACK ) && (tmp !=STAT_RXADDR_ACK )) { dev_err(i2c->dev,
@@ -416,17 +416,17 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target, }if (i +1 < length) -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL , +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB |TWSI_CTL_AAK );else -octeon_i2c_write_sw (i2c,SW_TWSI_EOP_TWSI_CTL , +octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)return result; - data[i] =octeon_i2c_read_sw (i2c,SW_TWSI_EOP_TWSI_DATA ); + data[i] =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_DATA );if (recv_len && i ==0 ) {if (data[i] >I2C_SMBUS_BLOCK_MAX +1 ) { dev_err(i2c->dev,
Signature · 1 line
1.9.1
Re: [PATCH v6 03/19] i2c: octeon: Rename [read|write]_sw to reg_[read|write]
Quoted 4 lines · Jan Glauber
octeon_i2c_read_sw->octeon_i2c_reg_readocteon_i2c_write_sw->octeon_i2c_reg_writeSigned-off-by Jan Glauber <[email protected]>
Please add the motivation of the change (the reason). No need to resend the whole patch. Just reply with the updated commit message and I'll fix up locally.
Re: [PATCH v6 03/19] i2c: octeon: Rename [read|write]_sw to reg_[read|write]
Quoted 10 lines · Wolfram Sang
Quoted 4 lines · Jan Glauber
octeon_i2c_read_sw->octeon_i2c_reg_readocteon_i2c_write_sw->octeon_i2c_reg_writeSigned-off-by Jan Glauber <[email protected]>Please add the motivation of the change (the reason). No need to resend the whole patch. Just reply with the updated commit message and I'll fix up locally.
Sorry, forgot to expand the initial description a bit.
Update:
---
i2c: octeon: Rename [read|write]_sw to reg_[read|write]
Rename the [read|write]_sw functions to make it clearer they access the TWSI registers.
---
Thanks, Jan
Wolfram Sang
AppliedRe: [PATCH v6 03/19] i2c: octeon: Rename [read|write]_sw to reg_[read|write]
Quoted 5 lines · Jan Glauber
Quoted 4 lines · Wolfram Sang
Quoted 3 lines · Jan Glauber
octeon_i2c_read_sw->octeon_i2c_reg_readocteon_i2c_write_sw->octeon_i2c_reg_write
Quoted 9 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Jan Glauber <[email protected]>Please add the motivation of the change (the reason). No need to resend the whole patch. Just reply with the updated commit message and I'll fix up locally.
Sorry, forgot to expand the initial description a bit.
Applied to for-next, thanks!
PATCH v6 4/19 i2c: octeon: Introduce helper functions for register access
In reply to Jan GlauberAdd helper functions for control, data and status register access.
--- drivers/i2c/busses/i2c-octeon.c | 56 +++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 25 deletions(-)
drivers/i2c/busses/i2c-octeon.c +31 -25
@@ -97,6 +97,11 @@ static void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8 data) }while ((tmp &SW_TWSI_V ) !=0 ); } +#define octeon_i2c_ctl_write (i2c, val) \ + octeon_i2c_reg_write(i2c, SW_TWSI_EOP_TWSI_CTL, val) +#define octeon_i2c_data_write (i2c, val) \ + octeon_i2c_reg_write(i2c, SW_TWSI_EOP_TWSI_DATA, val) +/** * octeon_i2c_reg_read - read lower bits of an I2C core register * @i2c: The struct octeon_i2c
@@ -118,6 +123,13 @@ static u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)return tmp &0xFF ; } +#define octeon_i2c_ctl_read (i2c) \ + octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL) +#define octeon_i2c_data_read (i2c) \ + octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA) +#define octeon_i2c_stat_read (i2c) \ + octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT) +/** * octeon_i2c_write_int - write the TWSI_INT register * @i2c: The struct octeon_i2c
@@ -189,7 +201,7 @@ static irqreturn_t octeon_i2c_isr(int irq, void *dev_id)static int octeon_i2c_test_iflg (struct octeon_i2c *i2c) { -return (octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_CTL ) &TWSI_CTL_IFLG ) !=0 ; +return (octeon_i2c_ctl_read (i2c) &TWSI_CTL_IFLG ) !=0 ; }/**
@@ -262,14 +274,14 @@ static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)int tries;/* disable high level controller, enable bus access */ -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB );/* reset controller */ octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_RST ,0 );for (tries =10 ; tries; tries--) {udelay (1 ); - status =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT ); + status =octeon_i2c_stat_read (i2c);if (status ==STAT_IDLE )return 0 ; }
@@ -288,27 +300,25 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)int result;u8 data; -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL , -TWSI_CTL_ENAB |TWSI_CTL_STA ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STA ); result =octeon_i2c_wait (i2c);if (result) { -if (octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT ) ==STAT_IDLE ) { +if (octeon_i2c_stat_read (i2c) ==STAT_IDLE ) {/* * Controller refused to send start flag May * be a client is holding SDA low - let's try * to free it. */ octeon_i2c_unblock (i2c); -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL , -TWSI_CTL_ENAB |TWSI_CTL_STA ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STA ); result =octeon_i2c_wait (i2c); }if (result)return result; } - data =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT ); + data =octeon_i2c_stat_read (i2c);if ((data !=STAT_START ) && (data !=STAT_RSTART )) {dev_err (i2c->dev,"%s: bad status (0x%x)\n" , __func__, data);return -EIO ;
@@ -320,8 +330,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)/* send STOP to the bus */ static void octeon_i2c_stop (struct octeon_i2c *i2c) { -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL , -TWSI_CTL_ENAB |TWSI_CTL_STP ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STP ); }/**
@@ -345,15 +354,15 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,if (result)return result; -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_DATA , target <<1 ); -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_data_write (i2c, target <<1 ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)return result;for (i =0 ; i < length; i++) { - tmp =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT ); + tmp =octeon_i2c_stat_read (i2c);if ((tmp !=STAT_TXADDR_ACK ) && (tmp !=STAT_TXDATA_ACK )) { dev_err(i2c->dev,
@@ -362,8 +371,8 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,return -EIO ; } -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_DATA , data[i]); -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_data_write (i2c, data[i]); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)
@@ -398,16 +407,15 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,if (result)return result; -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_DATA , (target <<1 ) |1 ); -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL ,TWSI_CTL_ENAB ); +octeon_i2c_data_write (i2c, (target <<1 ) |1 ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)return result;for (i =0 ; i < length; i++) { - tmp =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_STAT ); - + tmp =octeon_i2c_stat_read (i2c);if ((tmp !=STAT_RXDATA_ACK ) && (tmp !=STAT_RXADDR_ACK )) { dev_err(i2c->dev, "%s: bad status before read (0x%x)\n",
@@ -416,17 +424,15 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target, }if (i +1 < length) -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL , -TWSI_CTL_ENAB |TWSI_CTL_AAK ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_AAK );else -octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_CTL , -TWSI_CTL_ENAB ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); result =octeon_i2c_wait (i2c);if (result)return result; - data[i] =octeon_i2c_reg_read (i2c,SW_TWSI_EOP_TWSI_DATA ); + data[i] =octeon_i2c_data_read (i2c);if (recv_len && i ==0 ) {if (data[i] >I2C_SMBUS_BLOCK_MAX +1 ) { dev_err(i2c->dev,
Signature · 1 line
1.9.1
Re: [PATCH v6 04/19] i2c: octeon: Introduce helper functions for register access
Quoted 3 lines · Jan Glauber
Add helper functions for control, data and status register access.
Signed-off-by Jan Glauber <[email protected]>
Same as patch 3.
Re: [PATCH v6 04/19] i2c: octeon: Introduce helper functions for register access
Quoted 7 lines · Wolfram Sang
Quoted 3 lines · Jan Glauber
Add helper functions for control, data and status register access.
Signed-off-by Jan Glauber <[email protected]>Same as patch 3.
Updated commit message:
i2c: octeon: Introduce helper functions for register access
Add helper functions for control, data and status register access. This simplifies the code and makes the purpose of the register access clearer.
Wolfram Sang
AppliedRe: [PATCH v6 04/19] i2c: octeon: Introduce helper functions for register access
Quoted 10 lines · Jan Glauber
Quoted 7 lines · Wolfram Sang
Quoted 3 lines · Jan Glauber
Add helper functions for control, data and status register access.
Signed-off-by Jan Glauber <[email protected]>Same as patch 3.
Updated commit message:
Applied to for-next, thanks!
PATCH v6 5/19 i2c: octeon: Remove superfluous check in octeon_i2c_test_iflg
In reply to Jan GlauberRemove superfluous check and stray newline.
--- drivers/i2c/busses/i2c-octeon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/i2c/busses/i2c-octeon.c +1 -2
@@ -198,10 +198,9 @@ static irqreturn_t octeon_i2c_isr(int irq, void *dev_id)return IRQ_HANDLED ; } -static int octeon_i2c_test_iflg (struct octeon_i2c *i2c) { -return (octeon_i2c_ctl_read (i2c) &TWSI_CTL_IFLG ) !=0 ; +return (octeon_i2c_ctl_read (i2c) &TWSI_CTL_IFLG ); }/**
Signature · 1 line
1.9.1
Wolfram Sang
AppliedRe: [PATCH v6 05/19] i2c: octeon: Remove superfluous check in octeon_i2c_test_iflg
Quoted 3 lines · Jan Glauber
Remove superfluous check and stray newline.
Signed-off-by Jan Glauber <[email protected]>
Applied to for-next, thanks!
PATCH v6 6/19 i2c: octeon: Improve error status checking
In reply to Jan GlauberIntroduce a function that checks for valid status codes depending on the phase of a transmit or receive. Also add all existing status codes and improve error handling for various states.
The Octeon TWSI has "assert acknowledge" bit (TWSI_CTL_AAK) that is required to be set in master receive mode until the last byte is requested. The state check needs to consider if this bit was set.
--- drivers/i2c/busses/i2c-octeon.c | 125 +++++++++++++++++++++++++++++++++------- 1 file changed, 103 insertions(+), 22 deletions(-)
drivers/i2c/busses/i2c-octeon.c +103 -22
@@ -55,13 +55,35 @@#define TWSI_CTL_IFLG 0x08/* HW event, SW writes 0 to ACK */ #define TWSI_CTL_AAK 0x04/* Assert ACK */ -/* Some status values */ +/* Status values */ +#define STAT_ERROR 0x00#define STAT_START 0x08 -#define STAT_RSTART 0x10 +#define STAT_REP_START 0x10#define STAT_TXADDR_ACK 0x18 +#define STAT_TXADDR_NAK 0x20#define STAT_TXDATA_ACK 0x28 +#define STAT_TXDATA_NAK 0x30 +#define STAT_LOST_ARB_38 0x38#define STAT_RXADDR_ACK 0x40 +#define STAT_RXADDR_NAK 0x48#define STAT_RXDATA_ACK 0x50 +#define STAT_RXDATA_NAK 0x58 +#define STAT_SLAVE_60 0x60 +#define STAT_LOST_ARB_68 0x68 +#define STAT_SLAVE_70 0x70 +#define STAT_LOST_ARB_78 0x78 +#define STAT_SLAVE_80 0x80 +#define STAT_SLAVE_88 0x88 +#define STAT_GENDATA_ACK 0x90 +#define STAT_GENDATA_NAK 0x98 +#define STAT_SLAVE_A0 0xA0 +#define STAT_SLAVE_A8 0xA8 +#define STAT_LOST_ARB_B0 0xB0 +#define STAT_SLAVE_LOST 0xB8 +#define STAT_SLAVE_NAK 0xC0 +#define STAT_SLAVE_ACK 0xC8 +#define STAT_AD2W_ACK 0xD0 +#define STAT_AD2W_NAK 0xD8#define STAT_IDLE 0xF8/* TWSI_INT values */
@@ -225,6 +247,65 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)return 0 ; } +static int octeon_i2c_check_status (struct octeon_i2c *i2c,int final_read) +{ +u8 stat =octeon_i2c_stat_read (i2c); + +switch (stat) { +/* Everything is fine */ +case STAT_AD2W_ACK : +case STAT_RXADDR_ACK : +case STAT_TXADDR_ACK : +case STAT_TXDATA_ACK : +return 0 ; + +/* ACK allowed on pre-terminal bytes only */ +case STAT_RXDATA_ACK : +if (!final_read) +return 0 ; +return -EAGAIN ; + +/* NAK allowed on terminal byte only */ +case STAT_RXDATA_NAK : +if (final_read) +return 0 ; +return -EAGAIN ; + +/* Arbitration lost */ +case STAT_LOST_ARB_38 : +case STAT_LOST_ARB_68 : +case STAT_LOST_ARB_78 : +case STAT_LOST_ARB_B0 : +return -EAGAIN ; + +/* Being addressed as slave, should back off & listen */ +case STAT_SLAVE_60 : +case STAT_SLAVE_70 : +case STAT_GENDATA_ACK : +case STAT_GENDATA_NAK : +return -EIO ; + +/* Core busy as slave */ +case STAT_SLAVE_80 : +case STAT_SLAVE_88 : +case STAT_SLAVE_A0 : +case STAT_SLAVE_A8 : +case STAT_SLAVE_LOST : +case STAT_SLAVE_NAK : +case STAT_SLAVE_ACK : +return -EIO ; + +case STAT_TXDATA_NAK : +case STAT_TXADDR_NAK : +case STAT_RXADDR_NAK : +case STAT_AD2W_NAK : +return -EAGAIN ; +default : +dev_err (i2c->dev,"unhandled state: %d\n" , stat); +return -EIO ; + } +} +/* calculate and set clock divisors */ static void octeon_i2c_set_clock (struct octeon_i2c *i2c) {
@@ -347,7 +428,6 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target, const u8 *data, int length) { int i, result; -u8 tmp; result =octeon_i2c_start (i2c);if (result)
@@ -361,14 +441,9 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target,return result;for (i =0 ; i < length; i++) { - tmp =octeon_i2c_stat_read (i2c); - -if ((tmp !=STAT_TXADDR_ACK ) && (tmp !=STAT_TXDATA_ACK )) { -dev_err (i2c->dev, -"%s: bad status before write (0x%x)\n" , - __func__, tmp); -return -EIO ; - } + result =octeon_i2c_check_status (i2c, false); +if (result) +return result;octeon_i2c_data_write (i2c, data[i]);octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB );
@@ -397,7 +472,7 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target, u8 *data, u16 *rlength, bool recv_len) {int i, result, length = *rlength; -u8 tmp; +bool final_read = false;if (length <1 )return -EINVAL ;
@@ -413,19 +488,21 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,if (result)return result; +/* address OK ? */ + result =octeon_i2c_check_status (i2c, false); +if (result) +return result; +for (i =0 ; i < length; i++) { - tmp =octeon_i2c_stat_read (i2c); -if ((tmp !=STAT_RXDATA_ACK ) && (tmp !=STAT_RXADDR_ACK )) { -dev_err (i2c->dev, -"%s: bad status before read (0x%x)\n" , - __func__, tmp); -return -EIO ; - } +/* for the last byte TWSI_CTL_AAK must not be set */ +if (i +1 == length) + final_read = true; -if (i +1 < length) -octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_AAK ); -else +/* clear iflg to allow next event */ +if (final_read)octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); +else +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_AAK ); result =octeon_i2c_wait (i2c);if (result)
@@ -441,6 +518,10 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target, } length += data[i]; } + + result =octeon_i2c_check_status (i2c, final_read); +if (result) +return result; } *rlength = length;return 0 ;
Signature · 1 line
1.9.1
Re: [PATCH v6 06/19] i2c: octeon: Improve error status checking
Please have a look at Documentation/i2c/fault-codes. -EAGAIN is only for arbitration loss.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 272
Quoted 11 lines
drivers/i2c/busses/i2c-octeon.c +11
-0+/* ACK allowed on pre-terminal bytes only */ +case STAT_RXDATA_ACK : +if (!final_read) +return 0 ; +return -EAGAIN ; + +/* NAK allowed on terminal byte only */ +case STAT_RXDATA_NAK : +if (final_read) +return 0 ; +return -EAGAIN ;
-EIO? Can this happen? It is the master who sends the NAK, so we are in control of that.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 279
Quoted 7 lines
drivers/i2c/busses/i2c-octeon.c +7
-0+ +/* Arbitration lost */ +case STAT_LOST_ARB_38 : +case STAT_LOST_ARB_68 : +case STAT_LOST_ARB_78 : +case STAT_LOST_ARB_B0 : +return -EAGAIN ;
OK.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 286
Quoted 7 lines
drivers/i2c/busses/i2c-octeon.c +7
-0+ +/* Being addressed as slave, should back off & listen */ +case STAT_SLAVE_60 : +case STAT_SLAVE_70 : +case STAT_GENDATA_ACK : +case STAT_GENDATA_NAK : +return -EIO ;
-EOPNOTSUPP?
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 296
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +10
-0+ +/* Core busy as slave */ +case STAT_SLAVE_80 : +case STAT_SLAVE_88 : +case STAT_SLAVE_A0 : +case STAT_SLAVE_A8 : +case STAT_SLAVE_LOST : +case STAT_SLAVE_NAK : +case STAT_SLAVE_ACK : +return -EIO ;
-EOPNOTSUPP?
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 298
Quoted 1 line
drivers/i2c/busses/i2c-octeon.c +1
-0+case STAT_TXDATA_NAK :
-EIO?
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 301
Quoted 3 lines
drivers/i2c/busses/i2c-octeon.c +3
-0+case STAT_TXADDR_NAK : +case STAT_RXADDR_NAK : +case STAT_AD2W_NAK :
-ENXIO?
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 305
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +4
-0+return -EAGAIN ; +default : +dev_err (i2c->dev,"unhandled state: %d\n" , stat); +return -EIO ;
OK.
Re: [PATCH v6 06/19] i2c: octeon: Improve error status checking
Quoted 4 lines · Wolfram Sang
Please have a look at
Documentation/i2c/fault-codes.-EAGAINis only for arbitration loss.
Quoted 14 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 272
Quoted 11 lines
drivers/i2c/busses/i2c-octeon.c +11
-0+/* ACK allowed on pre-terminal bytes only */ +case STAT_RXDATA_ACK : +if (!final_read) +return 0 ; +return -EAGAIN ; + +/* NAK allowed on terminal byte only */ +case STAT_RXDATA_NAK : +if (final_read) +return 0 ; +return -EAGAIN ;
-EIO? Can this happen? It is the master who sends the NAK, so we are in control of that.
If it happens not in the final_read part then it is an error, so -EIO would be better suited. Also for the other error codes I will follow your suggestion.
Should I resend the whole series or will you review the other patches before?
Thanks,
Jan
Quoted 50 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 279
Quoted 7 lines
drivers/i2c/busses/i2c-octeon.c +7
-0+ +/* Arbitration lost */ +case STAT_LOST_ARB_38 : +case STAT_LOST_ARB_68 : +case STAT_LOST_ARB_78 : +case STAT_LOST_ARB_B0 : +return -EAGAIN ;OK.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 286
Quoted 7 lines
drivers/i2c/busses/i2c-octeon.c +7
-0+ +/* Being addressed as slave, should back off & listen */ +case STAT_SLAVE_60 : +case STAT_SLAVE_70 : +case STAT_GENDATA_ACK : +case STAT_GENDATA_NAK : +return -EIO ;
-EOPNOTSUPP?drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 296
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +10
-0+ +/* Core busy as slave */ +case STAT_SLAVE_80 : +case STAT_SLAVE_88 : +case STAT_SLAVE_A0 : +case STAT_SLAVE_A8 : +case STAT_SLAVE_LOST : +case STAT_SLAVE_NAK : +case STAT_SLAVE_ACK : +return -EIO ;
-EOPNOTSUPP?drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 298
Quoted 1 line
drivers/i2c/busses/i2c-octeon.c +1
-0+case STAT_TXDATA_NAK :
-EIO?drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 301
Quoted 3 lines
drivers/i2c/busses/i2c-octeon.c +3
-0+case STAT_TXADDR_NAK : +case STAT_RXADDR_NAK : +case STAT_AD2W_NAK :
-ENXIO?drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_wait(struct octeon_i2c *i2c) · line 305
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +4
-0+return -EAGAIN ; +default : +dev_err (i2c->dev,"unhandled state: %d\n" , stat); +return -EIO ;OK.
Re: [PATCH v6 06/19] i2c: octeon: Improve error status checking
Quoted 3 lines
Should I resend the whole series or will you review the other patches before?
The next batch I want to review is up to patch 10. Please hold back until I am there.
Thanks,
Wolfram
PATCH v6 7/19 i2c: octeon: Use i2c recovery framework
In reply to Jan GlauberSwitch to the i2c bus recovery framework using generic SCL recovery. If this fails try to reset the hardware. The recovery is triggered during START on timeout of the interrupt or failure to reach the START / repeated-START condition.
The START function is moved to xfer and while at it:
- removed xfer debug message (i2c core already provides debugging)
- removed length is zero check
--- drivers/i2c/busses/i2c-octeon.c | 178 +++++++++++++++++++++++++--------------- 1 file changed, 111 insertions(+), 67 deletions(-)
drivers/i2c/busses/i2c-octeon.c +111 -67
@@ -90,6 +90,8 @@#define TWSI_INT_CORE_EN BIT_ULL(6)#define TWSI_INT_SDA_OVR BIT_ULL(8)#define TWSI_INT_SCL_OVR BIT_ULL(9) +#define TWSI_INT_SDA BIT_ULL(10) +#define TWSI_INT_SCL BIT_ULL(11)struct octeon_i2c {wait_queue_head_t queue;
@@ -152,6 +154,18 @@ static u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)#define octeon_i2c_stat_read (i2c) \ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT) + +/** +* octeon_i2c_write_int - read the TWSI_INT register +* @i2c: The struct octeon_i2c +* +* Returns the value of the register. +*/ +static u64 octeon_i2c_read_int (struct octeon_i2c *i2c) +{ +return __raw_readq (i2c->twsi_base +TWSI_INT ); +} +/** * octeon_i2c_write_int - write the TWSI_INT register * @i2c: The struct octeon_i2c
@@ -182,33 +196,6 @@ static void octeon_i2c_int_disable(struct octeon_i2c *i2c)octeon_i2c_write_int (i2c,0 ); } -/** -* octeon_i2c_unblock - unblock the bus -* @i2c: The struct octeon_i2c -* -* If there was a reset while a device was driving 0 to bus, bus is blocked. -* We toggle it free manually by some clock cycles and send a stop. -*/ -static void octeon_i2c_unblock (struct octeon_i2c *i2c) -{ -int i; - -dev_dbg (i2c->dev,"%s\n" , __func__); - -for (i =0 ; i <9 ; i++) { -octeon_i2c_write_int (i2c,0 ); -udelay (5 ); -octeon_i2c_write_int (i2c,TWSI_INT_SCL_OVR ); -udelay (5 ); - } -/* hand-crank a STOP */ -octeon_i2c_write_int (i2c,TWSI_INT_SDA_OVR |TWSI_INT_SCL_OVR ); -udelay (5 ); -octeon_i2c_write_int (i2c,TWSI_INT_SDA_OVR ); -udelay (5 ); -octeon_i2c_write_int (i2c,0 ); -} -/* interrupt service routine */ static irqreturn_t octeon_i2c_isr (int irq,void *dev_id) {
@@ -369,6 +356,17 @@ static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)return -EIO ; } +static int octeon_i2c_recovery (struct octeon_i2c *i2c) +{ +int ret; + + ret =i2c_recover_bus (&i2c->adap); +if (ret) +/* recover failed, try hardware re-init */ + ret =octeon_i2c_init_lowlevel (i2c); +return ret; +} +/** * octeon_i2c_start - send START to the bus * @i2c: The struct octeon_i2c
@@ -377,34 +375,34 @@ static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)*/ static int octeon_i2c_start (struct octeon_i2c *i2c) { -int result; -u8 data; - -octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STA ); - - result =octeon_i2c_wait (i2c); -if (result) { -if (octeon_i2c_stat_read (i2c) ==STAT_IDLE ) { -/* -* Controller refused to send start flag May -* be a client is holding SDA low - let's try -* to free it. -*/ -octeon_i2c_unblock (i2c); -octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STA ); - result =octeon_i2c_wait (i2c); +int ret, retries =2 ; +u8 stat; + +retry: +if (retries >0 ) { +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STA ); + ret =octeon_i2c_wait (i2c); +if (ret) { +/* timeout error, try to recover */ + ret =octeon_i2c_recovery (i2c); +if (ret) +return ret; + retries--; + goto retry; } -if (result) -return result; - } + }else +return -EAGAIN ; - data =octeon_i2c_stat_read (i2c); -if ((data !=STAT_START ) && (data !=STAT_RSTART )) { -dev_err (i2c->dev,"%s: bad status (0x%x)\n" , __func__, data); -return -EIO ; - } + stat =octeon_i2c_stat_read (i2c); +if (stat ==STAT_START || stat ==STAT_REP_START ) +/* START successful, bail out */ +return 0 ; -return 0 ; +/* START failed, try to recover */ + ret =octeon_i2c_recovery (i2c); +if (!ret && retries--) + goto retry; +return ret; }/* send STOP to the bus */
@@ -429,10 +427,6 @@ static int octeon_i2c_write(struct octeon_i2c *i2c, int target, {int i, result; - result =octeon_i2c_start (i2c); -if (result) -return result; -octeon_i2c_data_write (i2c, target <<1 );octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB );
@@ -474,13 +468,6 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,int i, result, length = *rlength;bool final_read = false; -if (length <1 ) -return -EINVAL ; - - result =octeon_i2c_start (i2c); -if (result) -return result; -octeon_i2c_data_write (i2c, (target <<1 ) |1 );octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB );
@@ -544,10 +531,10 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,for (i =0 ; ret ==0 && i < num; i++) {struct i2c_msg *pmsg = &msgs[i]; -dev_dbg (i2c->dev, -"Doing %s %d byte(s) to/from 0x%02x - %d of %d messages\n" , - pmsg->flags &I2C_M_RD ?"read" :"write" , - pmsg->len, pmsg->addr, i +1 , num); + ret =octeon_i2c_start (i2c); +if (ret) +return ret; +if (pmsg->flags &I2C_M_RD ) ret =octeon_i2c_read (i2c, pmsg->addr, pmsg->buf, &pmsg->len, pmsg->flags &I2C_M_RECV_LEN );
@@ -560,6 +547,62 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,return (ret !=0 ) ? ret : num; } +static int octeon_i2c_get_scl (struct i2c_adapter *adap) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); +u64 state; + + state =octeon_i2c_read_int (i2c); +return state &TWSI_INT_SCL ; +} + +static void octeon_i2c_set_scl (struct i2c_adapter *adap,int val) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); + +octeon_i2c_write_int (i2c,TWSI_INT_SCL_OVR ); +} + +static int octeon_i2c_get_sda (struct i2c_adapter *adap) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); +u64 state; + + state =octeon_i2c_read_int (i2c); +return state &TWSI_INT_SDA ; +} + +static void octeon_i2c_prepare_recovery (struct i2c_adapter *adap) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); + +/* +* The stop resets the state machine, does not _transmit_ STOP unless +* engine was active. +*/ +octeon_i2c_stop (i2c); + +octeon_i2c_write_int (i2c,0 ); +udelay (5 ); +} + +static void octeon_i2c_unprepare_recovery (struct i2c_adapter *adap) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); + +octeon_i2c_write_int (i2c,0 ); +udelay (5 ); +} + +static struct i2c_bus_recovery_info octeon_i2c_recovery_info = { + .recover_bus = i2c_generic_scl_recovery, + .get_scl = octeon_i2c_get_scl, + .set_scl = octeon_i2c_set_scl, + .get_sda = octeon_i2c_get_sda, + .prepare_recovery = octeon_i2c_prepare_recovery, + .unprepare_recovery = octeon_i2c_unprepare_recovery, +}; +static u32 octeon_i2c_functionality (struct i2c_adapter *adap) { return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
@@ -640,6 +683,7 @@ static int octeon_i2c_probe(struct platform_device *pdev) i2c->adap = octeon_i2c_ops; i2c->adap.timeout =msecs_to_jiffies (2 ); i2c->adap.retries =5 ; + i2c->adap.bus_recovery_info = &octeon_i2c_recovery_info; i2c->adap.dev.parent = &pdev->dev; i2c->adap.dev.of_node = node;i2c_set_adapdata (&i2c->adap, i2c);
Signature · 1 line
1.9.1
Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework
Quoted 5 lines · Jan Glauber
Switch to the i2c bus recovery framework using generic SCL recovery. If this fails try to reset the hardware. The recovery is triggered during START on timeout of the interrupt or failure to reach the START / repeated-START condition.
Quoted 8 lines
The START function is moved to xfer and while at it:
- removed xfer debug message (i2c core already provides debugging)
- removed length is zero checkSigned-off-by Jan Glauber <[email protected]>--- drivers/i2c/busses/i2c-octeon.c | 178 +++++++++++++++++++++++++--------------- 1 file changed, 111 insertions(+), 67 deletions(-)
Interesting, it got larger...
Quoted 3 lines
+/**
+ *octeon_i2c_write_int- read theTWSI_INTregister
read_int
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c) · line 378
Quoted 1 line
drivers/i2c/busses/i2c-octeon.c +1
-0+int ret, retries =2 ;
I don't think 'retries' makes sense here. On failure, you return -EAGAIN, so the core will retry 'adapter->retries' times anyhow.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_read(struct octeon_i2c *i2c, int target, · line 478
Quoted 2 lines
drivers/i2c/busses/i2c-octeon.c +0
-2-if (length <1 ) -return -EINVAL ;
So, the adapter support 0-length messages now? Or why was it there? I have the feeling this is a seperate patch.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, · line 585
Quoted 11 lines
drivers/i2c/busses/i2c-octeon.c +11
-0+static void octeon_i2c_prepare_recovery (struct i2c_adapter *adap) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); + +/* +* The stop resets the state machine, does not _transmit_ STOP unless +* engine was active. +*/ +octeon_i2c_stop (i2c); + +octeon_i2c_write_int (i2c,0 );
Maybe a comment why the delay?
Quoted 2 lines
(quoted diff) +2
-0+ udelay(5); +}
Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework
Quoted 15 lines · Wolfram Sang
Quoted 5 lines · Jan Glauber
Switch to the i2c bus recovery framework using generic SCL recovery. If this fails try to reset the hardware. The recovery is triggered during START on timeout of the interrupt or failure to reach the START / repeated-START condition.
Quoted 8 lines
The START function is moved to xfer and while at it:
- removed xfer debug message (i2c core already provides debugging)
- removed length is zero checkSigned-off-by Jan Glauber <[email protected]>--- drivers/i2c/busses/i2c-octeon.c | 178 +++++++++++++++++++++++++--------------- 1 file changed, 111 insertions(+), 67 deletions(-)
Quoted 7 lines
Interesting, it got larger...
Quoted 3 lines
+/**
+ *octeon_i2c_write_int- read theTWSI_INTregister
read_int
OK
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c) · line 378
Quoted 1 line
drivers/i2c/busses/i2c-octeon.c +1
-0+int ret, retries =2 ;I don't think 'retries' makes sense here. On failure, you return
-EAGAIN, so the core will retry 'adapter->retries' times anyhow.
OK. I'll remove the retry looping and call recovery once if the START condition is not reached. If recovery succeeds I'll return -EAGAIN, otherwise the error code.
Quoted 5 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_read(struct octeon_i2c *i2c, int target, · line 478
Quoted 2 lines
drivers/i2c/busses/i2c-octeon.c +0
-2-if (length <1 ) -return -EINVAL ;So, the adapter support 0-length messages now? Or why was it there? I have the feeling this is a seperate patch.
I assumed this check was bogus and there are no valid 0-length messages...
If 0-length messages can indeed happen I'll keep this check (but why would the i2c core not check for that in a central place and return an error before calling xfer() ?).
Quoted 5 lines
Quoted 5 lines
drivers/i2c/busses/i2c-octeon.c +5
-0+static void octeon_i2c_prepare_recovery (struct i2c_adapter *adap) +{ +struct octeon_i2c *i2c =i2c_get_adapdata (adap); + +/*
Quoted 8 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, · line 585
Quoted 6 lines
drivers/i2c/busses/i2c-octeon.c +6
-0+* The stop resets the state machine, does not _transmit_ STOP unless +* engine was active. +*/ +octeon_i2c_stop (i2c); + +octeon_i2c_write_int (i2c,0 );Maybe a comment why the delay?
Later patch "Flush TWSI writes with readback" adds synchronization after octeon_i2c_write_int(), so the delays in prepare/unprepare_recovery can go away.
Quoted 2 lines
Quoted 2 lines
(quoted diff) +2
-0+ udelay(5); +}
Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework
Quoted 3 lines
I assumed this check was bogus and there are no valid 0-length messages...
They are valid (check SMBUS_QUICK), but not every controller can handle them correctly. Your driver has SMBUS_QUICK enabled, so this is a contradiction to the check above where it rejects it.
So, it looks like it needs to be tested again (and documented this time). If the HW can't do it, the FUNC bit for QUICK needs to be masked out. If it can do SMBUS_QUICK, the check can probably go away.
Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework
Quoted 7 lines · Wolfram Sang
Quoted 3 lines
I assumed this check was bogus and there are no valid 0-length messages...
They are valid (check
SMBUS_QUICK), but not every controller can handle them correctly. Your driver hasSMBUS_QUICKenabled, so this is a contradiction to the check above where it rejects it.
Oops, this mismatch dates back to the inital driver code. From the documentation I would say SMBUS_QUICK is not supported, although nothing terrible happens in the write case.
Quoted 4 lines
So, it looks like it needs to be tested again (and documented this time). If the HW can't do it, the FUNC bit for QUICK needs to be masked out. If it can do
SMBUS_QUICK, the check can probably go away.
I would like to disable SMBUS_QUICK. It never worked for the read case. Could we break something by disabling the quick-write case or is the quick-write emulated by a larger write if the feature bit is not set?
Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework
Quoted 2 lines
I would like to disable
SMBUS_QUICK. It never worked for the read case.
Fine with me.
Quoted 3 lines
Could we break something by disabling the quick-write case or is the quick-write emulated by a larger write if the feature bit is not set?
No emulation. It is simply not supported then. It is actually quite dangerous to simulate it with regular write.
PATCH v6 8/19 i2c: octeon: Enable High-Level Controller
In reply to Jan GlauberFrom: David Daney <[email protected]>
Use High-Level Controller (HLC) when possible. The HLC can read/write up to 8 bytes and is completely optional. The most important difference of the HLC is that it only requires one interrupt for a transfer (up to 8 bytes) where the low-level read/write requires 2 interrupts plus one interrupt per transferred byte. Since the interrupts are costly using the HLC improves the performance. Also, the HLC provides improved error handling.
--- drivers/i2c/busses/i2c-octeon.c | 355 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 345 insertions(+), 10 deletions(-)
drivers/i2c/busses/i2c-octeon.c +345 -10
@@ -29,13 +29,23 @@/* Register offsets */ #define SW_TWSI 0x00#define TWSI_INT 0x10 +#define SW_TWSI_EXT 0x18/* Controller command patterns */ #define SW_TWSI_V BIT_ULL(63)/* Valid bit */ +#define SW_TWSI_EIA BIT_ULL(61)/* Extended internal address */ #define SW_TWSI_R BIT_ULL(56)/* Result or read bit */ +#define SW_TWSI_SOVR BIT_ULL(55)/* Size override */ +#define SW_TWSI_SIZE_SHIFT 52 +#define SW_TWSI_ADDR_SHIFT 40 +#define SW_TWSI_IA_SHIFT 32/* Internal address */ /* Controller opcode word (bits 60:57) */ #define SW_TWSI_OP_SHIFT 57 +#define SW_TWSI_OP_7 (0ULL << SW_TWSI_OP_SHIFT) +#define SW_TWSI_OP_7_IA (1ULL << SW_TWSI_OP_SHIFT) +#define SW_TWSI_OP_10 (2ULL << SW_TWSI_OP_SHIFT) +#define SW_TWSI_OP_10_IA (3ULL << SW_TWSI_OP_SHIFT)#define SW_TWSI_OP_TWSI_CLK (4ULL << SW_TWSI_OP_SHIFT)#define SW_TWSI_OP_EOP (6ULL << SW_TWSI_OP_SHIFT)/* Extended opcode */
@@ -48,7 +58,7 @@#define SW_TWSI_EOP_TWSI_RST (SW_TWSI_OP_EOP | 7ULL << SW_TWSI_EOP_SHIFT)/* Controller command and status bits */ -#define TWSI_CTL_CE 0x80 +#define TWSI_CTL_CE 0x80/* High level controller enable */ #define TWSI_CTL_ENAB 0x40/* Bus enable */ #define TWSI_CTL_STA 0x20/* Master-mode start, HW clears when done */ #define TWSI_CTL_STP 0x10/* Master-mode stop, HW clears when done */
@@ -87,6 +97,11 @@#define STAT_IDLE 0xF8/* TWSI_INT values */ +#define TWSI_INT_ST_INT BIT_ULL(0) +#define TWSI_INT_TS_INT BIT_ULL(1) +#define TWSI_INT_CORE_INT BIT_ULL(2) +#define TWSI_INT_ST_EN BIT_ULL(4) +#define TWSI_INT_TS_EN BIT_ULL(5)#define TWSI_INT_CORE_EN BIT_ULL(6)#define TWSI_INT_SDA_OVR BIT_ULL(8)#define TWSI_INT_SCL_OVR BIT_ULL(9)
@@ -101,6 +116,7 @@ struct octeon_i2c {int sys_freq;void __iomem *twsi_base;struct device *dev; +bool hlc_enabled; };/**
@@ -196,6 +212,47 @@ static void octeon_i2c_int_disable(struct octeon_i2c *i2c)octeon_i2c_write_int (i2c,0 ); } +/* +* Cleanup low-level state & enable high-level controller. +*/ +static void octeon_i2c_hlc_enable (struct octeon_i2c *i2c) +{ +int try =0 ; +u64 val; + +if (i2c->hlc_enabled) +return ; + i2c->hlc_enabled = true; + +while (1 ) { + val =octeon_i2c_ctl_read (i2c); +if (!(val & (TWSI_CTL_STA |TWSI_CTL_STP ))); +break ; + +/* clear IFLG event */ +if (val &TWSI_CTL_IFLG ) +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); + +if (try++ >100 ) { +pr_err ("%s: giving up\n" , __func__); +break ; + } + +/* spin until any start/stop has finished */ +udelay (10 ); + } +octeon_i2c_ctl_write (i2c,TWSI_CTL_CE |TWSI_CTL_AAK |TWSI_CTL_ENAB ); +} + +static void octeon_i2c_hlc_disable (struct octeon_i2c *i2c) +{ +if (!i2c->hlc_enabled) +return ; + + i2c->hlc_enabled = false; +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); +} +/* interrupt service routine */ static irqreturn_t octeon_i2c_isr (int irq,void *dev_id) {
@@ -293,6 +350,253 @@ static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) } } +static bool octeon_i2c_hlc_test_ready (struct octeon_i2c *i2c) +{ +u64 val =__raw_readq (i2c->twsi_base +SW_TWSI ); + +return (val &SW_TWSI_V ) ==0 ; +} + +static void octeon_i2c_hlc_int_enable (struct octeon_i2c *i2c) +{ +octeon_i2c_write_int (i2c,TWSI_INT_ST_EN ); +} + +static void octeon_i2c_hlc_int_clear (struct octeon_i2c *i2c) +{ +/* clear ST/TS events, listen for neither */ +octeon_i2c_write_int (i2c,TWSI_INT_ST_INT |TWSI_INT_TS_INT ); +} + +/** +* octeon_i2c_hlc_wait - wait for an HLC operation to complete +* @i2c: The struct octeon_i2c +* +* Returns 0 on success, otherwise a negative errno. +*/ +static int octeon_i2c_hlc_wait (struct octeon_i2c *i2c) +{ +int time_left; + +octeon_i2c_hlc_int_enable (i2c); + time_left =wait_event_interruptible_timeout (i2c->queue, +octeon_i2c_hlc_test_ready (i2c), + i2c->adap.timeout); +octeon_i2c_int_disable (i2c); +if (!time_left) { +octeon_i2c_hlc_int_clear (i2c); +dev_dbg (i2c->dev,"%s: timeout\n" , __func__); +return -ETIMEDOUT ; + } + +if (time_left <0 ) { +dev_dbg (i2c->dev,"%s: wait interrupted\n" , __func__); +return time_left; + } +return 0 ; +} + +/* high-level-controller pure read of up to 8 bytes */ +static int octeon_i2c_hlc_read (struct octeon_i2c *i2c,struct i2c_msg *msgs) +{ +int i, j, ret =0 ; +u64 cmd; + +octeon_i2c_hlc_enable (i2c); +octeon_i2c_hlc_int_clear (i2c); + + cmd =SW_TWSI_V |SW_TWSI_R |SW_TWSI_SOVR ; +/* SIZE */ + cmd |= (u64) (msgs[0 ].len -1 ) <<SW_TWSI_SIZE_SHIFT ; +/* A */ + cmd |= (u64) (msgs[0 ].addr &0x7full ) <<SW_TWSI_ADDR_SHIFT ; + +if (msgs[0 ].flags &I2C_M_TEN ) + cmd |=SW_TWSI_OP_10 ; +else + cmd |=SW_TWSI_OP_7 ; + +__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); + ret =octeon_i2c_hlc_wait (i2c); +if (ret) + goto err; + + cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); +if ((cmd &SW_TWSI_R ) ==0 ) +return -EAGAIN ; + +for (i =0 , j = msgs[0 ].len -1 ; i < msgs[0 ].len && i <4 ; i++, j--) + msgs[0 ].buf[j] = (cmd >> (8 * i)) &0xff ; + +if (msgs[0 ].len >4 ) { + cmd =__raw_readq (i2c->twsi_base +SW_TWSI_EXT ); +for (i =0 ; i < msgs[0 ].len -4 && i <4 ; i++, j--) + msgs[0 ].buf[j] = (cmd >> (8 * i)) &0xff ; + } + +err: +return ret; +} + +/* high-level-controller pure write of up to 8 bytes */ +static int octeon_i2c_hlc_write (struct octeon_i2c *i2c,struct i2c_msg *msgs) +{ +int i, j, ret =0 ; +u64 cmd; + +octeon_i2c_hlc_enable (i2c); +octeon_i2c_hlc_int_clear (i2c); + + ret =octeon_i2c_check_status (i2c, false); +if (ret) + goto err; + + cmd =SW_TWSI_V |SW_TWSI_SOVR ; +/* SIZE */ + cmd |= (u64) (msgs[0 ].len -1 ) <<SW_TWSI_SIZE_SHIFT ; +/* A */ + cmd |= (u64) (msgs[0 ].addr &0x7full ) <<SW_TWSI_ADDR_SHIFT ; + +if (msgs[0 ].flags &I2C_M_TEN ) + cmd |=SW_TWSI_OP_10 ; +else + cmd |=SW_TWSI_OP_7 ; + +for (i =0 , j = msgs[0 ].len -1 ; i < msgs[0 ].len && i <4 ; i++, j--) + cmd |= (u64 )msgs[0 ].buf[j] << (8 * i); + +if (msgs[0 ].len >4 ) { +u64 ext =0 ; + +for (i =0 ; i < msgs[0 ].len -4 && i <4 ; i++, j--) + ext |= (u64 ) msgs[0 ].buf[j] << (8 * i); +__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT ); + } + +__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); + ret =octeon_i2c_hlc_wait (i2c); +if (ret) + goto err; + + cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); +if ((cmd &SW_TWSI_R ) ==0 ) +return -EAGAIN ; + + ret =octeon_i2c_check_status (i2c, false); + +err: +return ret; +} + +/* high-level-controller composite write+read, msg0=addr, msg1=data */ +static int octeon_i2c_hlc_comp_read (struct octeon_i2c *i2c,struct i2c_msg *msgs) +{ +int i, j, ret =0 ; +u64 cmd; + +octeon_i2c_hlc_enable (i2c); + + cmd =SW_TWSI_V |SW_TWSI_R |SW_TWSI_SOVR ; +/* SIZE */ + cmd |= (u64)(msgs[1 ].len -1 ) <<SW_TWSI_SIZE_SHIFT ; +/* A */ + cmd |= (u64)(msgs[0 ].addr &0x7full ) <<SW_TWSI_ADDR_SHIFT ; + +if (msgs[0 ].flags &I2C_M_TEN ) + cmd |=SW_TWSI_OP_10_IA ; +else + cmd |=SW_TWSI_OP_7_IA ; + +if (msgs[0 ].len ==2 ) { +u64 ext =0 ; + + cmd |=SW_TWSI_EIA ; + ext = (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ; + cmd |= (u64 ) msgs[0 ].buf[1 ] <<SW_TWSI_IA_SHIFT ; +__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT ); + }else + cmd |= (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ; + +octeon_i2c_hlc_int_clear (i2c); +__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); + + ret =octeon_i2c_hlc_wait (i2c); +if (ret) + goto err; + + cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); +if ((cmd &SW_TWSI_R ) ==0 ) +return -EAGAIN ; + +for (i =0 , j = msgs[1 ].len -1 ; i < msgs[1 ].len && i <4 ; i++, j--) + msgs[1 ].buf[j] = (cmd >> (8 * i)) &0xff ; + +if (msgs[1 ].len >4 ) { + cmd =__raw_readq (i2c->twsi_base +SW_TWSI_EXT ); +for (i =0 ; i < msgs[1 ].len -4 && i <4 ; i++, j--) + msgs[1 ].buf[j] = (cmd >> (8 * i)) &0xff ; + } + +err: +return ret; +} + +/* high-level-controller composite write+write, m[0]len<=2, m[1]len<=8 */ +static int octeon_i2c_hlc_comp_write (struct octeon_i2c *i2c,struct i2c_msg *msgs) +{ +bool set_ext = false; +int i, j, ret =0 ; +u64 cmd, ext =0 ; + +octeon_i2c_hlc_enable (i2c); + + cmd =SW_TWSI_V |SW_TWSI_SOVR ; +/* SIZE */ + cmd |= (u64) (msgs[1 ].len -1 ) <<SW_TWSI_SIZE_SHIFT ; +/* A */ + cmd |= (u64) (msgs[0 ].addr &0x7full ) <<SW_TWSI_ADDR_SHIFT ; + +if (msgs[0 ].flags &I2C_M_TEN ) + cmd |=SW_TWSI_OP_10_IA ; +else + cmd |=SW_TWSI_OP_7_IA ; + +if (msgs[0 ].len ==2 ) { + cmd |=SW_TWSI_EIA ; + ext |= (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ; + set_ext = true; + cmd |= (u64 ) msgs[0 ].buf[1 ] <<SW_TWSI_IA_SHIFT ; + }else + cmd |= (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ; + +for (i =0 , j = msgs[1 ].len -1 ; i < msgs[1 ].len && i <4 ; i++, j--) + cmd |= (u64 ) msgs[1 ].buf[j] << (8 * i); + +if (msgs[1 ].len >4 ) { +for (i =0 ; i < msgs[1 ].len -4 && i <4 ; i++, j--) + ext |= (u64 )msgs[1 ].buf[j] << (8 * i); + set_ext = true; + } +if (set_ext) +__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT ); + +octeon_i2c_hlc_int_clear (i2c); +__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); + + ret =octeon_i2c_hlc_wait (i2c); +if (ret) + goto err; + + cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); +if ((cmd &SW_TWSI_R ) ==0 ) +return -EAGAIN ; + + ret =octeon_i2c_check_status (i2c, false); + +err: +return ret; +} +/* calculate and set clock divisors */ static void octeon_i2c_set_clock (struct octeon_i2c *i2c) {
@@ -337,23 +641,29 @@ static void octeon_i2c_set_clock(struct octeon_i2c *i2c)static int octeon_i2c_init_lowlevel (struct octeon_i2c *i2c) { -u8 status; +u8 status =0 ;int tries; -/* disable high level controller, enable bus access */ -octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); -/* reset controller */ octeon_i2c_reg_write (i2c,SW_TWSI_EOP_TWSI_RST ,0 ); -for (tries =10 ; tries; tries--) { +for (tries =10 ; tries && status !=STAT_IDLE ; tries--) {udelay (1 ); status =octeon_i2c_stat_read (i2c);if (status ==STAT_IDLE ) -return 0 ; +break ; } -dev_err (i2c->dev,"%s: TWSI_RST failed! (0x%x)\n" , __func__, status); -return -EIO ; + +if (status !=STAT_IDLE ) { +dev_err (i2c->dev,"%s: TWSI_RST failed! (0x%x)\n" , + __func__, status); +return -EIO ; + } + +/* toggle twice to force both teardowns */ +octeon_i2c_hlc_enable (i2c); +octeon_i2c_hlc_disable (i2c); +return 0 ; }static int octeon_i2c_recovery (struct octeon_i2c *i2c)
@@ -378,6 +688,8 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)int ret, retries =2 ;u8 stat; +octeon_i2c_hlc_disable (i2c); + retry:if (retries >0 ) {octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB |TWSI_CTL_STA );
@@ -528,6 +840,28 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,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; + } + } +for (i =0 ; ret ==0 && i < num; i++) {struct i2c_msg *pmsg = &msgs[i];
@@ -543,7 +877,7 @@ static int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, pmsg->len); }octeon_i2c_stop (i2c); - +out:return (ret !=0 ) ? ret : num; }
@@ -582,6 +916,7 @@ static void octeon_i2c_prepare_recovery(struct i2c_adapter *adap)*/ octeon_i2c_stop (i2c); +octeon_i2c_hlc_disable (i2c);octeon_i2c_write_int (i2c,0 );udelay (5 ); }
Signature · 1 line
1.9.1
Re: [PATCH v6 08/19] i2c: octeon: Enable High-Level Controller
Quoted 9 lines · Jan Glauber
From: David Daney <[email protected]>
Use High-Level Controller (HLC) when possible. The HLC can read/write up to 8 bytes and is completely optional. The most important difference of the HLC is that it only requires one interrupt for a transfer (up to 8 bytes) where the low-level read/write requires 2 interrupts plus one interrupt per transferred byte. Since the interrupts are costly using the HLC improves the performance. Also, the HLC provides improved error handling.
Much better description, thanks!
Quoted 7 lines
drivers/i2c/busses/i2c-octeon.c +7
-0+while (1 ) { + val =octeon_i2c_ctl_read (i2c); +if (!(val & (TWSI_CTL_STA |TWSI_CTL_STP ))); +break ; + +/* clear IFLG event */ +if (val &TWSI_CTL_IFLG )
drivers/i2c/busses/i2c-octeon.c static void octeon_i2c_int_disable(struct octeon_i2c *i2c) · line 243
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +10
-0+octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); + +if (try++ >100 ) { +pr_err ("%s: giving up\n" , __func__); +break ; + } + +/* spin until any start/stop has finished */ +udelay (10 ); + }
Maybe you can use one of the readx_poll_timeout() functions?
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +4
-0+/** +* octeon_i2c_hlc_wait - wait for an HLC operation to complete +* @i2c: The struct octeon_i2c +*
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) · line 384
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +10
-0+* Returns 0 on success, otherwise a negative errno. +*/ +static int octeon_i2c_hlc_wait (struct octeon_i2c *i2c) +{ +int time_left; + +octeon_i2c_hlc_int_enable (i2c); + time_left =wait_event_interruptible_timeout (i2c->queue, +octeon_i2c_hlc_test_ready (i2c), + i2c->adap.timeout);
Have you tested signal handling thoroughly? Most driver dropped the _interruptible after a while. Mostly they found out that the state machine of the interrupt handler couldn't gracefully deal with it and nobody really needed the interruptible. Just saying.
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) · line 397
Quoted 13 lines
drivers/i2c/busses/i2c-octeon.c +13
-0+octeon_i2c_int_disable (i2c); +if (!time_left) { +octeon_i2c_hlc_int_clear (i2c); +dev_dbg (i2c->dev,"%s: timeout\n" , __func__); +return -ETIMEDOUT ; + } + +if (time_left <0 ) { +dev_dbg (i2c->dev,"%s: wait interrupted\n" , __func__); +return time_left; + } +return 0 ; +}
Drop the debug messages?
I can't say much about the HW details, of course. Didn't spot anything suspicious there.
Re: [PATCH v6 08/19] i2c: octeon: Enable High-Level Controller
Quoted 1 line · Wolfram Sang
On Mon, Apr 11, 2016 at 05:28:39PM +0200, Jan Glauber wrote:
[...]
Quoted 6 lines
Quoted 6 lines
+ */
drivers/i2c/busses/i2c-octeon.c +5
-0+static int octeon_i2c_hlc_wait (struct octeon_i2c *i2c) +{ +int time_left; + +octeon_i2c_hlc_int_enable (i2c);
Quoted 8 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) · line 384
Quoted 3 lines
drivers/i2c/busses/i2c-octeon.c +3
-0+ time_left =wait_event_interruptible_timeout (i2c->queue, +octeon_i2c_hlc_test_ready (i2c), + i2c->adap.timeout);Have you tested signal handling thoroughly? Most driver dropped the
_interruptibleafter a while. Mostly they found out that the state machine of the interrupt handler couldn't gracefully deal with it and nobody really needed the interruptible. Just saying.
Good point. We know that exiting with a signal leaves us in an undefined state.
We will have to think on this point.
Quoted 20 lines
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +3
-0+octeon_i2c_int_disable (i2c); +if (!time_left) { +octeon_i2c_hlc_int_clear (i2c);drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) · line 397
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +10
-0+dev_dbg (i2c->dev,"%s: timeout\n" , __func__); +return -ETIMEDOUT ; + } + +if (time_left <0 ) { +dev_dbg (i2c->dev,"%s: wait interrupted\n" , __func__); +return time_left; + } +return 0 ; +}Drop the debug messages?
I can't say much about the HW details, of course. Didn't spot anything suspicious there.
Re: [PATCH v6 08/19] i2c: octeon: Enable High-Level Controller
Quoted 18 lines · David Daney
Quoted 1 line · Wolfram Sang
On Mon, Apr 11, 2016 at 05:28:39PM +0200, Jan Glauber wrote:
[...]
Quoted 6 lines
Quoted 6 lines
+ */
drivers/i2c/busses/i2c-octeon.c +5
-0+static int octeon_i2c_hlc_wait (struct octeon_i2c *i2c) +{ +int time_left; + +octeon_i2c_hlc_int_enable (i2c);Quoted 8 lines
drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) · line 384
Quoted 3 lines
drivers/i2c/busses/i2c-octeon.c +3
-0+ time_left =wait_event_interruptible_timeout (i2c->queue, +octeon_i2c_hlc_test_ready (i2c), + i2c->adap.timeout);Have you tested signal handling thoroughly? Most driver dropped the
_interruptibleafter a while. Mostly they found out that the state machine of the interrupt handler couldn't gracefully deal with it and nobody really needed the interruptible. Just saying.
Quoted 4 lines
Good point. We know that exiting with a signal leaves us in an undefined state.
We will have to think on this point.
I think we should just drop the _interruptible_ and use wait_event_timeout. The same is already used in the octeon_i2c_wait(). The 2ms timeout should not hurt anyone.
Quoted 20 lines
Quoted 20 lines
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +3
-0+octeon_i2c_int_disable (i2c); +if (!time_left) { +octeon_i2c_hlc_int_clear (i2c);drivers/i2c/busses/i2c-octeon.c static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read) · line 397
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +10
-0+dev_dbg (i2c->dev,"%s: timeout\n" , __func__); +return -ETIMEDOUT ; + } + +if (time_left <0 ) { +dev_dbg (i2c->dev,"%s: wait interrupted\n" , __func__); +return time_left; + } +return 0 ; +}Drop the debug messages?
I can't say much about the HW details, of course. Didn't spot anything suspicious there.
Re: [PATCH v6 08/19] i2c: octeon: Enable High-Level Controller
Quoted 4 lines
I think we should just drop the _interruptible_ and use
wait_event_timeout. The same is already used in theocteon_i2c_wait(). The 2ms timeout should not hurt anyone.
This is what most people have good experience with :)
Re: [PATCH v6 08/19] i2c: octeon: Enable High-Level Controller
In reply to Wolfram SangQuoted 24 lines · Wolfram Sang
Quoted 9 lines · Jan Glauber
From: David Daney <[email protected]>
Use High-Level Controller (HLC) when possible. The HLC can read/write up to 8 bytes and is completely optional. The most important difference of the HLC is that it only requires one interrupt for a transfer (up to 8 bytes) where the low-level read/write requires 2 interrupts plus one interrupt per transferred byte. Since the interrupts are costly using the HLC improves the performance. Also, the HLC provides improved error handling.
Much better description, thanks!
Quoted 11 lines
drivers/i2c/busses/i2c-octeon.c +11
-0+while (1 ) { + val =octeon_i2c_ctl_read (i2c); +if (!(val & (TWSI_CTL_STA |TWSI_CTL_STP ))); +break ; + +/* clear IFLG event */ +if (val &TWSI_CTL_IFLG ) +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); + +if (try++ >100 ) { +pr_err ("%s: giving up\n" , __func__);
Quoted 8 lines
drivers/i2c/busses/i2c-octeon.c static void octeon_i2c_int_disable(struct octeon_i2c *i2c) · line 243
Quoted 6 lines
drivers/i2c/busses/i2c-octeon.c +6
-0+break ; + } + +/* spin until any start/stop has finished */ +udelay (10 ); + }Maybe you can use one of the
readx_poll_timeout()functions?
Nice, but I don't think we can use readx_poll_timeout() here because of the octeon_i2c_ctl_write inside the loop...
PATCH v6 9/19 dt-bindings: i2c: Add Octeon cn78xx TWSI
In reply to Jan GlauberAdd compatible string for Cavium Octeon cn78XX SOCs TWSI.
--- Documentation/devicetree/bindings/i2c/i2c-octeon.txt | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/i2c/i2c-octeon.txt +6 -0
@@ -4,6 +4,12 @@ Compatibility with all cn3XXX, cn5XXX and cn6XXX SOCs. + or + + compatible: "cavium,octeon-7890-twsi" + + Compatibility with cn78XX SOCs. + - reg: The base address of the TWSI/I2C bus controller register bank. - #address-cells: Must be <1>.
Signature · 1 line
1.9.1
PATCH v6 10/19 i2c: octeon: Add support for cn78xx chips
In reply to Jan GlauberFrom: David Daney <[email protected]>
cn78xx has a different interrupt architecture, so we have to manage the interrupts differently.
--- drivers/i2c/busses/i2c-octeon.c | 131 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 11 deletions(-)
drivers/i2c/busses/i2c-octeon.c +120 -11
@@ -11,6 +11,7 @@* warranty of any kind, whether express or implied. */ +#include <linux/atomic.h> #include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/kernel.h>
@@ -112,11 +113,18 @@ struct octeon_i2c {wait_queue_head_t queue;struct i2c_adapter adap;int irq; +int hlc_irq;/* For cn7890 only */ u32 twsi_freq;int sys_freq;void __iomem *twsi_base;struct device *dev;bool hlc_enabled; +void (*int_en)(struct octeon_i2c *); +void (*int_dis)(struct octeon_i2c *); +void (*hlc_int_en)(struct octeon_i2c *); +void (*hlc_int_dis)(struct octeon_i2c *); +atomic_t int_en_cnt; +atomic_t hlc_int_en_cnt; };/**
@@ -212,6 +220,58 @@ static void octeon_i2c_int_disable(struct octeon_i2c *i2c)octeon_i2c_write_int (i2c,0 ); } +/** +* octeon_i2c_int_enable78 - enable the CORE interrupt +* @i2c: The struct octeon_i2c +* +* The interrupt will be asserted when there is non-STAT_IDLE state in the +* SW_TWSI_EOP_TWSI_STAT register. +*/ +static void octeon_i2c_int_enable78 (struct octeon_i2c *i2c) +{ +atomic_inc_return (&i2c->int_en_cnt); +enable_irq (i2c->irq); +} + +static void __octeon_i2c_irq_disable (atomic_t *cnt,int irq) +{ +int count; + +/* +* The interrupt can be disabled in two places, but we only +* want to make the disable_irq_nosync() call once, so keep +* track with the atomic variable. +*/ + count =atomic_dec_if_positive (cnt); +if (count >=0 ) +disable_irq_nosync (irq); +} + +/* disable the CORE interrupt */ +static void octeon_i2c_int_disable78 (struct octeon_i2c *i2c) +{ +__octeon_i2c_irq_disable (&i2c->int_en_cnt, i2c->irq); +} + +/** +* octeon_i2c_hlc_int_enable78 - enable the ST interrupt +* @i2c: The struct octeon_i2c +* +* The interrupt will be asserted when there is non-STAT_IDLE state in +* the SW_TWSI_EOP_TWSI_STAT register. +*/ +static void octeon_i2c_hlc_int_enable78 (struct octeon_i2c *i2c) +{ +atomic_inc_return (&i2c->hlc_int_en_cnt); +enable_irq (i2c->hlc_irq); +} + +/* disable the ST interrupt */ +static void octeon_i2c_hlc_int_disable78 (struct octeon_i2c *i2c) +{ +__octeon_i2c_irq_disable (&i2c->hlc_int_en_cnt, i2c->hlc_irq); +} +/* * Cleanup low-level state & enable high-level controller. */
@@ -258,7 +318,18 @@ static irqreturn_t octeon_i2c_isr(int irq, void *dev_id) {struct octeon_i2c *i2c = dev_id; -octeon_i2c_int_disable (i2c); + i2c->int_dis (i2c); +wake_up (&i2c->queue); + +return IRQ_HANDLED ; +} + +/* HLC interrupt service routine */ +static irqreturn_t octeon_i2c_hlc_isr78 (int irq,void *dev_id) +{ +struct octeon_i2c *i2c = dev_id; + + i2c->hlc_int_dis (i2c);wake_up (&i2c->queue);return IRQ_HANDLED ;
@@ -279,10 +350,10 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c) {long time_left; -octeon_i2c_int_enable (i2c); + i2c->int_en (i2c); time_left =wait_event_timeout (i2c->queue,octeon_i2c_test_iflg (i2c), i2c->adap.timeout); -octeon_i2c_int_disable (i2c); + i2c->int_dis (i2c);if (!time_left) {dev_dbg (i2c->dev,"%s: timeout\n" , __func__);return -ETIMEDOUT ;
@@ -378,11 +449,11 @@ static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c) {int time_left; -octeon_i2c_hlc_int_enable (i2c); + i2c->hlc_int_en (i2c); time_left =wait_event_interruptible_timeout (i2c->queue,octeon_i2c_hlc_test_ready (i2c), i2c->adap.timeout); -octeon_i2c_int_disable (i2c); + i2c->hlc_int_dis (i2c);if (!time_left) {octeon_i2c_hlc_int_clear (i2c);dev_dbg (i2c->dev,"%s: timeout\n" , __func__);
@@ -958,14 +1029,26 @@ static struct i2c_adapter octeon_i2c_ops = {static int octeon_i2c_probe (struct platform_device *pdev) {struct device_node *node = pdev->dev.of_node; +int irq, result =0 , hlc_irq =0 ;struct resource *res_mem;struct octeon_i2c *i2c; -int irq, result =0 ; - -/* All adaptors have an irq. */ - irq =platform_get_irq (pdev,0 ); -if (irq <0 ) -return irq; +bool cn78xx_style; + + cn78xx_style =of_device_is_compatible (node,"cavium,octeon-7890-twsi" ); +if (cn78xx_style) { + hlc_irq =platform_get_irq (pdev,0 ); +if (hlc_irq <0 ) +return hlc_irq; + + irq =platform_get_irq (pdev,2 ); +if (irq <0 ) +return irq; + }else { +/* All adaptors have an irq. */ + irq =platform_get_irq (pdev,0 ); +if (irq <0 ) +return irq; + } i2c =devm_kzalloc (&pdev->dev,sizeof (*i2c),GFP_KERNEL );if (!i2c) {
@@ -1000,6 +1083,31 @@ static int octeon_i2c_probe(struct platform_device *pdev) i2c->irq = irq; +if (cn78xx_style) { + i2c->hlc_irq = hlc_irq; + + i2c->int_en = octeon_i2c_int_enable78; + i2c->int_dis = octeon_i2c_int_disable78; + i2c->hlc_int_en = octeon_i2c_hlc_int_enable78; + i2c->hlc_int_dis = octeon_i2c_hlc_int_disable78; + +irq_set_status_flags (i2c->irq,IRQ_NOAUTOEN ); +irq_set_status_flags (i2c->hlc_irq,IRQ_NOAUTOEN ); + + result =devm_request_irq (&pdev->dev, i2c->hlc_irq, + octeon_i2c_hlc_isr78,0 , +DRV_NAME , i2c); +if (result <0 ) { +dev_err (i2c->dev,"failed to attach interrupt\n" ); + goto out; + } + }else { + i2c->int_en = octeon_i2c_int_enable; + i2c->int_dis = octeon_i2c_int_disable; + i2c->hlc_int_en = octeon_i2c_hlc_int_enable; + i2c->hlc_int_dis = octeon_i2c_int_disable; + } + result =devm_request_irq (&pdev->dev, i2c->irq, octeon_i2c_isr,0 ,DRV_NAME , i2c);if (result <0 ) {
@@ -1046,6 +1154,7 @@ static int octeon_i2c_remove(struct platform_device *pdev)static const struct of_device_id octeon_i2c_match[] = { { .compatible ="cavium,octeon-3860-twsi" , }, + { .compatible ="cavium,octeon-7890-twsi" , }, {}, };MODULE_DEVICE_TABLE (of, octeon_i2c_match);
Signature · 1 line
1.9.1
Re: [PATCH v6 10/19] i2c: octeon: Add support for cn78xx chips
Quoted 4 lines · Jan Glauber
From: David Daney <[email protected]>
cn78xx has a different interrupt architecture, so we have to manage the interrupts differently.
I'd appreciate if you could explain here why interrupts use NOAUTOEN and have to be manually en-/disabled? This is surely unusual.
Quoted 23 lines
Signed-off-by David Daney <[email protected]>Signed-off-by Jan Glauber <[email protected]>--- drivers/i2c/busses/i2c-octeon.c | 131 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 11 deletions(-)drivers/i2c/busses/i2c-octeon.c +1
-0@@ -11,6 +11,7 @@* warranty of any kind, whether express or implied. */ +#include <linux/atomic.h> #include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/kernel.h> @@ -112,11 +113,18 @@ struct octeon_i2c {wait_queue_head_t queue;struct i2c_adapter adap;int irq;
drivers/i2c/busses/i2c-octeon.c struct octeon_i2c { · line 125
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +5
-0+int hlc_irq;/* For cn7890 only */ u32 twsi_freq;int sys_freq;void __iomem *twsi_base;struct device *dev;bool hlc_enabled; +void (*int_en)(struct octeon_i2c *); +void (*int_dis)(struct octeon_i2c *); +void (*hlc_int_en)(struct octeon_i2c *); +void (*hlc_int_dis)(struct octeon_i2c *);
I'd prefer having 'enable' and 'disable' written out, but I leave that to you.
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +2
-0+atomic_t int_en_cnt; +atomic_t hlc_int_en_cnt; };
Re: [PATCH v6 10/19] i2c: octeon: Add support for cn78xx chips
Quoted 8 lines · Wolfram Sang
Quoted 4 lines · Jan Glauber
From: David Daney <[email protected]>
cn78xx has a different interrupt architecture, so we have to manage the interrupts differently.
I'd appreciate if you could explain here why interrupts use NOAUTOEN and have to be manually en-/disabled? This is surely unusual.
It is quite nice, isn't it?
Since there were enable bits in both the interrupt controller *and* the I2C bus hardware, we had redundancy that could profitably be eliminated.
Now with the advent of the cn78xx, the system is much better. This redundancy has been eliminated and interrupts are enabled only in the interrupt controller.
Fortunately, the kernel supplies us with convenience functions for manipulating these interrupt enable bits (enable_irq() and disable_irq()). Since we don't want to irqs enabled when the driver is probed we have to set NOAUTOEN before calling request_irq().
Personally, I preferred the old way where each device had interrupt enable bits that could be controlled independently of the interrupt controller. I have been told that I should abandon my preference for that type of antiquated architecture and welcome the brave new world of the unified interrupt enable bit. Never the less, I still feel some nostalgia for the old way.
The good news is that it is not very much work to add software support for the cn78xx style interrupt enabling/masking. The hooks we add can also be used to support the MSI-X interrupts in the Thunder support patch that follows.
David.
Quoted 42 lines
Quoted 24 lines
Signed-off-by David Daney <[email protected]>Signed-off-by Jan Glauber <[email protected]>--- drivers/i2c/busses/i2c-octeon.c | 131 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 11 deletions(-)drivers/i2c/busses/i2c-octeon.c +1
-0@@ -11,6 +11,7 @@* warranty of any kind, whether express or implied. */ +#include <linux/atomic.h> #include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/kernel.h> @@ -112,11 +113,18 @@ struct octeon_i2c {wait_queue_head_t queue;struct i2c_adapter adap;int irq;drivers/i2c/busses/i2c-octeon.c struct octeon_i2c { · line 125
Quoted 10 lines
drivers/i2c/busses/i2c-octeon.c +5
-0+int hlc_irq;/* For cn7890 only */ u32 twsi_freq;int sys_freq;void __iomem *twsi_base;struct device *dev;bool hlc_enabled; +void (*int_en)(struct octeon_i2c *); +void (*int_dis)(struct octeon_i2c *); +void (*hlc_int_en)(struct octeon_i2c *); +void (*hlc_int_dis)(struct octeon_i2c *);I'd prefer having 'enable' and 'disable' written out, but I leave that to you.
Quoted 4 lines
drivers/i2c/busses/i2c-octeon.c +2
-0+atomic_t int_en_cnt; +atomic_t hlc_int_en_cnt; };
Re: [PATCH v6 10/19] i2c: octeon: Add support for cn78xx chips
Quoted 7 lines
Personally, I preferred the old way where each device had interrupt enable bits that could be controlled independently of the interrupt controller. I have been told that I should abandon my preference for that type of antiquated architecture and welcome the brave new world of the unified interrupt enable bit. Never the less, I still feel some nostalgia for the old way.
:) I think I am with you on this one.
PATCH v6 11/19 i2c: octeon: Flush TWSI writes with readback
In reply to Jan GlauberFrom: Peter Swain <[email protected]>
--- drivers/i2c/busses/i2c-octeon.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/i2c/busses/i2c-octeon.c +13 -8
@@ -127,6 +127,12 @@ struct octeon_i2c {atomic_t hlc_int_en_cnt; }; +static void writeqflush (u64 val,void __iomem *addr) +{ +__raw_writeq (val, addr); +__raw_readq (addr);/* wait for write to land */ +} +/** * octeon_i2c_reg_write - write an I2C core register * @i2c: The struct octeon_i2c
@@ -197,8 +203,7 @@ static u64 octeon_i2c_read_int(struct octeon_i2c *i2c)*/ static void octeon_i2c_write_int (struct octeon_i2c *i2c,u64 data) { -__raw_writeq (data, i2c->twsi_base +TWSI_INT ); -__raw_readq (i2c->twsi_base +TWSI_INT ); +writeqflush (data, i2c->twsi_base +TWSI_INT ); }/**
@@ -487,7 +492,7 @@ static int octeon_i2c_hlc_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)else cmd |=SW_TWSI_OP_7 ; -__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI ); ret =octeon_i2c_hlc_wait (i2c);if (ret) goto err;
@@ -541,10 +546,10 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)for (i =0 ; i < msgs[0 ].len -4 && i <4 ; i++, j--) ext |= (u64 ) msgs[0 ].buf[j] << (8 * i); -__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT ); +writeqflush (ext, i2c->twsi_base +SW_TWSI_EXT ); } -__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI ); ret =octeon_i2c_hlc_wait (i2c);if (ret) goto err;
@@ -589,7 +594,7 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs cmd |= (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ;octeon_i2c_hlc_int_clear (i2c); -__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI ); ret =octeon_i2c_hlc_wait (i2c);if (ret)
@@ -649,10 +654,10 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg set_ext = true; }if (set_ext) -__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT ); +writeqflush (ext, i2c->twsi_base +SW_TWSI_EXT );octeon_i2c_hlc_int_clear (i2c); -__raw_writeq (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI ); ret =octeon_i2c_hlc_wait (i2c);if (ret)
Signature · 1 line
1.9.1
PATCH v6 12/19 i2c: octeon: Faster operation when IFLG signals late
In reply to Jan GlauberFrom: Peter Swain <[email protected]>
Some versions can deliver low-level twsi irq before twsi_ctl.iflg is set, leading to timeout-driven i/o.
When an irq signals event, but woken task does not see the expected twsi_ctl.iflg, re-check about 80uS later.
EEPROM reads on 100kHz i2c now measure ~5.2kB/s, about 1/2 what's achievable, and much better than the worst-case 100 bytes/sec before.
--- drivers/i2c/busses/i2c-octeon.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
drivers/i2c/busses/i2c-octeon.c +24 -1
@@ -345,6 +345,28 @@ static int octeon_i2c_test_iflg(struct octeon_i2c *i2c)return (octeon_i2c_ctl_read (i2c) &TWSI_CTL_IFLG ); } +#define I2C_OCTEON_IFLG_WAIT 80/* microseconds */ + +/* +* Wait-helper which addresses the delayed-IFLAG problem by re-polling for +* missing TWSI_CTL[IFLG] a few us later, when irq has signalled an event, +* but none found. Skip this re-poll on the first (non-wakeup) call. +*/ +static int poll_iflg (struct octeon_i2c *i2c,int *first_p) +{ +int iflg =octeon_i2c_test_iflg (i2c); + +if (iflg) +return 1 ; +if (*first_p) + *first_p =0 ; +else { +usleep_range (I2C_OCTEON_IFLG_WAIT ,2 *I2C_OCTEON_IFLG_WAIT ); + iflg =octeon_i2c_test_iflg (i2c); + } +return iflg; +} +/** * octeon_i2c_wait - wait for the IFLG to be set * @i2c: The struct octeon_i2c
@@ -354,9 +376,10 @@ static int octeon_i2c_test_iflg(struct octeon_i2c *i2c)static int octeon_i2c_wait (struct octeon_i2c *i2c) {long time_left; +int first =1 ; i2c->int_en (i2c); - time_left =wait_event_timeout (i2c->queue,octeon_i2c_test_iflg (i2c), + time_left =wait_event_timeout (i2c->queue,poll_iflg (i2c, &first), i2c->adap.timeout); i2c->int_dis (i2c);if (!time_left) {
Signature · 1 line
1.9.1
PATCH v6 13/19 i2c: octeon: Add workaround for broken irqs on CN3860
In reply to Jan GlauberFrom: David Daney <[email protected]>
CN3860 does not interrupt the CPU when the i2c status changes. If we get a timeout, and see the status has in fact changed, we know we have this problem, and drop back to polling.
--- drivers/i2c/busses/i2c-octeon.c | 55 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-octeon.c +53 -2
@@ -119,6 +119,8 @@ struct octeon_i2c {void __iomem *twsi_base;struct device *dev;bool hlc_enabled; +bool broken_irq_mode; +bool broken_irq_check;void (*int_en)(struct octeon_i2c *);void (*int_dis)(struct octeon_i2c *);void (*hlc_int_en)(struct octeon_i2c *);
@@ -378,10 +380,33 @@ static int octeon_i2c_wait(struct octeon_i2c *i2c)long time_left;int first =1 ; +if (i2c->broken_irq_mode) { +/* +* Some chip revisions seem to not assert the irq in +* the interrupt controller. So we must poll for the +* IFLG change. +*/ +u64 end =get_jiffies_64 () + i2c->adap.timeout; + +while (!octeon_i2c_test_iflg (i2c) && +time_before64 (get_jiffies_64 (), end)) +udelay (50 ); + +return octeon_i2c_test_iflg (i2c) ?0 : -ETIMEDOUT ; + } + i2c->int_en (i2c); time_left =wait_event_timeout (i2c->queue,poll_iflg (i2c, &first), i2c->adap.timeout); i2c->int_dis (i2c); + +if (time_left <=0 && i2c->broken_irq_check && +octeon_i2c_test_iflg (i2c)) { +dev_err (i2c->dev, +"broken irq connection detected, switching to polling mode.\n" ); + i2c->broken_irq_mode = true; +return 0 ; + }if (!time_left) {dev_dbg (i2c->dev,"%s: timeout\n" , __func__);return -ETIMEDOUT ;
@@ -477,17 +502,40 @@ static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c) {int time_left; +if (i2c->broken_irq_mode) { +/* +* Some cn38xx boards did not assert the irq in +* the interrupt controller. So we must poll for the +* IFLG change. +*/ +u64 end =get_jiffies_64 () + i2c->adap.timeout; + +while (!octeon_i2c_hlc_test_ready (i2c) && +time_before64 (get_jiffies_64 (), end)) +udelay (50 ); + +return octeon_i2c_hlc_test_ready (i2c) ?0 : -ETIMEDOUT ; + } + i2c->hlc_int_en (i2c); time_left =wait_event_interruptible_timeout (i2c->queue,octeon_i2c_hlc_test_ready (i2c), i2c->adap.timeout); i2c->hlc_int_dis (i2c); -if (!time_left) { +if (!time_left)octeon_i2c_hlc_int_clear (i2c); + +if (time_left <=0 && i2c->broken_irq_check && +octeon_i2c_hlc_test_ready (i2c)) { +dev_err (i2c->dev,"broken irq connection detected, switching to polling mode.\n" ); + i2c->broken_irq_mode = true; +return 0 ; + } + +if (!time_left) {dev_dbg (i2c->dev,"%s: timeout\n" , __func__);return -ETIMEDOUT ; } -if (time_left <0 ) {dev_dbg (i2c->dev,"%s: wait interrupted\n" , __func__);return time_left;
@@ -1143,6 +1191,9 @@ static int octeon_i2c_probe(struct platform_device *pdev) goto out; } +if (OCTEON_IS_MODEL (OCTEON_CN38XX )) + i2c->broken_irq_check = true; + result =octeon_i2c_init_lowlevel (i2c);if (result) {dev_err (i2c->dev,"init low level failed\n" );
Signature · 1 line
1.9.1
PATCH v6 14/19 i2c: octeon: Move read function before write
In reply to Jan GlauberJust sorting the functions to be consistent with the other read/write variants.
--- drivers/i2c/busses/i2c-octeon.c | 78 ++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 39 deletions(-)
drivers/i2c/busses/i2c-octeon.c +39 -39
@@ -871,45 +871,6 @@ static void octeon_i2c_stop(struct octeon_i2c *i2c) }/** -* octeon_i2c_write - send data to the bus via low-level controller -* @i2c: The struct octeon_i2c -* @target: Target address -* @data: Pointer to the data to be sent -* @length: Length of the data -* -* The address is sent over the bus, then the data. -* -* Returns 0 on success, otherwise a negative errno. -*/ -static int octeon_i2c_write (struct octeon_i2c *i2c,int target, -const u8 *data,int length) -{ -int i, result; - -octeon_i2c_data_write (i2c, target <<1 ); -octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); - - result =octeon_i2c_wait (i2c); -if (result) -return result; - -for (i =0 ; i < length; i++) { - result =octeon_i2c_check_status (i2c, false); -if (result) -return result; - -octeon_i2c_data_write (i2c, data[i]); -octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); - - result =octeon_i2c_wait (i2c); -if (result) -return result; - } - -return 0 ; -} - -/** * octeon_i2c_read - receive data from the bus via low-level controller * @i2c: The struct octeon_i2c * @target: Target address
@@ -974,6 +935,45 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target, }/** +* octeon_i2c_write - send data to the bus via low-level controller +* @i2c: The struct octeon_i2c +* @target: Target address +* @data: Pointer to the data to be sent +* @length: Length of the data +* +* The address is sent over the bus, then the data. +* +* Returns 0 on success, otherwise a negative errno. +*/ +static int octeon_i2c_write (struct octeon_i2c *i2c,int target, +const u8 *data,int length) +{ +int i, result; + +octeon_i2c_data_write (i2c, target <<1 ); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); + + result =octeon_i2c_wait (i2c); +if (result) +return result; + +for (i =0 ; i < length; i++) { + result =octeon_i2c_check_status (i2c, false); +if (result) +return result; + +octeon_i2c_data_write (i2c, data[i]); +octeon_i2c_ctl_write (i2c,TWSI_CTL_ENAB ); + + result =octeon_i2c_wait (i2c); +if (result) +return result; + } + +return 0 ; +} + +/** * octeon_i2c_xfer - The driver's master_xfer function * @adap: Pointer to the i2c_adapter structure * @msgs: Pointer to the messages to be processed
Signature · 1 line
1.9.1
PATCH v6 15/19 i2c: octeon: Rename driver to prepare for split
In reply to Jan GlauberThis is an intermediate commit in preparation of the driver split. The module rename in this commit will be reverted in the next patch, this is just done to make the series bisectible.
---
drivers/i2c/busses/Makefile | 2 +-
drivers/i2c/busses/{i2c-octeon.c => i2c-octeon-core.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename drivers/i2c/busses/{i2c-octeon.c => i2c-octeon-core.c} (100%)drivers/i2c/busses/Makefile +1 -1
@@ -91,7 +91,7 @@ obj-$(CONFIG_I2C_UNIPHIER) += i2c-uniphier.o obj-$(CONFIG_I2C_UNIPHIER_F) += i2c-uniphier-f.o obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o obj-$(CONFIG_I2C_WMT) += i2c-wmt.o -obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o +obj-$(CONFIG_I2C_OCTEON) += i2c-octeon-core.o obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o obj-$(CONFIG_I2C_XLR) += i2c-xlr.o obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
drivers/i2c/busses/i2c-octeon-core.c renamed from drivers/i2c/busses/i2c-octeon.c +0 -0
Signature · 1 line
1.9.1
PATCH v6 16/19 i2c: octeon: Split the driver into two parts
In reply to Jan GlauberMove common functionality into a separate file in preparation of the re-use from the ThunderX i2c driver.
--- drivers/i2c/busses/Makefile | 3 +- drivers/i2c/busses/i2c-cavium.c | 805 +++++++++++++++++++++++++++++ drivers/i2c/busses/i2c-cavium.h | 200 +++++++ drivers/i2c/busses/i2c-octeon-core.c | 972 +---------------------------------- 4 files changed, 1008 insertions(+), 972 deletions(-) create mode 100644 drivers/i2c/busses/i2c-cavium.c create mode 100644 drivers/i2c/busses/i2c-cavium.h
drivers/i2c/busses/Makefile +2 -1
@@ -91,7 +91,8 @@ obj-$(CONFIG_I2C_UNIPHIER) += i2c-uniphier.o obj-$(CONFIG_I2C_UNIPHIER_F) += i2c-uniphier-f.o obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o obj-$(CONFIG_I2C_WMT) += i2c-wmt.o -obj-$(CONFIG_I2C_OCTEON) += i2c-octeon-core.o +i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o +obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o obj-$(CONFIG_I2C_XLR) += i2c-xlr.o obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
drivers/i2c/busses/i2c-cavium.c new file +805 -0
805 lines not on this page · raw message
drivers/i2c/busses/i2c-cavium.h new file +200 -0
200 lines not on this page · raw message
drivers/i2c/busses/i2c-octeon-core.c +1 -971
998 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH v6 17/19 i2c: thunderx: Add i2c driver for ThunderX SOC
In reply to Jan GlauberThe ThunderX SOC uses the same i2c block as the Octeon SOC. The main difference is that on ThunderX the device is a PCI device so device probing is done via PCI, interrupts are MSIX and the clock is taken from device tree.
--- drivers/i2c/busses/Kconfig | 10 ++ drivers/i2c/busses/Makefile | 2 + drivers/i2c/busses/i2c-cavium.h | 17 ++- drivers/i2c/busses/i2c-thunderx-core.c | 269 +++++++++++++++++++++++++++++++++ 4 files changed, 295 insertions(+), 3 deletions(-) create mode 100644 drivers/i2c/busses/i2c-thunderx-core.c
drivers/i2c/busses/Kconfig +10 -0
@@ -953,6 +953,16 @@ config I2C_OCTEON This driver can also be built as a module. If so, the module will be called i2c-octeon. +config I2C_THUNDERX + tristate "Cavium ThunderX I2C bus support" + depends on 64BIT && PCI && !CAVIUM_OCTEON_SOC + help + Say yes if you want to support the I2C serial bus on Cavium + ThunderX SOC. + + This driver can also be built as a module. If so, the module + will be called i2c-thunderx. + config I2C_XILINX tristate "Xilinx I2C Controller" depends on HAS_IOMEM
drivers/i2c/busses/Makefile +2 -0
@@ -93,6 +93,8 @@ obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o obj-$(CONFIG_I2C_WMT) += i2c-wmt.o i2c-octeon-objs := i2c-cavium.o i2c-octeon-core.o obj-$(CONFIG_I2C_OCTEON) += i2c-octeon.o +i2c-thunderx-objs := i2c-cavium.o i2c-thunderx-core.o +obj-$(CONFIG_I2C_THUNDERX) += i2c-thunderx.o obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o obj-$(CONFIG_I2C_XLR) += i2c-xlr.o obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
drivers/i2c/busses/i2c-cavium.h +14 -3
@@ -8,9 +8,15 @@#include <linux/pci.h> /* Register offsets */ -#define SW_TWSI 0x00 -#define TWSI_INT 0x10 -#define SW_TWSI_EXT 0x18 +#if IS_ENABLED (CONFIG_I2C_THUNDERX ) +#define SW_TWSI 0x1000 +#define TWSI_INT 0x1010 +#define SW_TWSI_EXT 0x1018 +#else +#define SW_TWSI 0x00 +#define TWSI_INT 0x10 +#define SW_TWSI_EXT 0x18 +#endif /* Controller command patterns */ #define SW_TWSI_V BIT_ULL(63)/* Valid bit */
@@ -92,6 +98,7 @@struct octeon_i2c {wait_queue_head_t queue;struct i2c_adapter adap; +struct clk *clk;int irq;int hlc_irq;/* For cn7890 only */ u32 twsi_freq;
@@ -107,6 +114,10 @@ struct octeon_i2c {void (*hlc_int_dis)(struct octeon_i2c *);atomic_t int_en_cnt;atomic_t hlc_int_en_cnt; + +#if IS_ENABLED (CONFIG_I2C_THUNDERX ) +struct msix_entry i2c_msix; +#endif };static inline void writeqflush (u64 val,void __iomem *addr)
drivers/i2c/busses/i2c-thunderx-core.c new file +269 -0
269 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH v6 18/19 i2c: octeon,thunderx: Move register offsets to struct
In reply to Jan GlauberThe register offsets are different between Octeon and ThunderX so move them into the algorithm struct and get rid of the define.
--- drivers/i2c/busses/i2c-cavium.c | 28 +++++++++++++-------------- drivers/i2c/busses/i2c-cavium.h | 35 +++++++++++++++++----------------- drivers/i2c/busses/i2c-octeon-core.c | 4 ++++ drivers/i2c/busses/i2c-thunderx-core.c | 4 ++++ 4 files changed, 40 insertions(+), 31 deletions(-)
drivers/i2c/busses/i2c-cavium.c +14 -14
@@ -33,7 +33,7 @@ irqreturn_t octeon_i2c_isr(int irq, void *dev_id)static bool octeon_i2c_hlc_test_ready (struct octeon_i2c *i2c) { -u64 val =__raw_readq (i2c->twsi_base +SW_TWSI ); +u64 val =__raw_readq (i2c->twsi_base +SW_TWSI (i2c));return (val &SW_TWSI_V ) ==0 ; }
@@ -443,12 +443,12 @@ static int octeon_i2c_hlc_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)else cmd |=SW_TWSI_OP_7 ; -writeqflush (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI (i2c)); ret =octeon_i2c_hlc_wait (i2c);if (ret) goto err; - cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); + cmd =__raw_readq (i2c->twsi_base +SW_TWSI (i2c));if ((cmd &SW_TWSI_R ) ==0 )return -EAGAIN ;
@@ -456,7 +456,7 @@ static int octeon_i2c_hlc_read(struct octeon_i2c *i2c, struct i2c_msg *msgs) msgs[0 ].buf[j] = (cmd >> (8 * i)) &0xff ;if (msgs[0 ].len >4 ) { - cmd =__raw_readq (i2c->twsi_base +SW_TWSI_EXT ); + cmd =__raw_readq (i2c->twsi_base +SW_TWSI_EXT (i2c));for (i =0 ; i < msgs[0 ].len -4 && i <4 ; i++, j--) msgs[0 ].buf[j] = (cmd >> (8 * i)) &0xff ; }
@@ -497,15 +497,15 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)for (i =0 ; i < msgs[0 ].len -4 && i <4 ; i++, j--) ext |= (u64 ) msgs[0 ].buf[j] << (8 * i); -writeqflush (ext, i2c->twsi_base +SW_TWSI_EXT ); +writeqflush (ext, i2c->twsi_base +SW_TWSI_EXT (i2c)); } -writeqflush (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI (i2c)); ret =octeon_i2c_hlc_wait (i2c);if (ret) goto err; - cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); + cmd =__raw_readq (i2c->twsi_base +SW_TWSI (i2c));if ((cmd &SW_TWSI_R ) ==0 )return -EAGAIN ;
@@ -540,18 +540,18 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs cmd |=SW_TWSI_EIA ; ext = (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ; cmd |= (u64 ) msgs[0 ].buf[1 ] <<SW_TWSI_IA_SHIFT ; -__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT ); +__raw_writeq (ext, i2c->twsi_base +SW_TWSI_EXT (i2c)); }else cmd |= (u64 ) msgs[0 ].buf[0 ] <<SW_TWSI_IA_SHIFT ;octeon_i2c_hlc_int_clear (i2c); -writeqflush (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI (i2c)); ret =octeon_i2c_hlc_wait (i2c);if (ret) goto err; - cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); + cmd =__raw_readq (i2c->twsi_base +SW_TWSI (i2c));if ((cmd &SW_TWSI_R ) ==0 )return -EAGAIN ;
@@ -559,7 +559,7 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs msgs[1 ].buf[j] = (cmd >> (8 * i)) &0xff ;if (msgs[1 ].len >4 ) { - cmd =__raw_readq (i2c->twsi_base +SW_TWSI_EXT ); + cmd =__raw_readq (i2c->twsi_base +SW_TWSI_EXT (i2c));for (i =0 ; i < msgs[1 ].len -4 && i <4 ; i++, j--) msgs[1 ].buf[j] = (cmd >> (8 * i)) &0xff ; }
@@ -605,16 +605,16 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg set_ext = true; }if (set_ext) -writeqflush (ext, i2c->twsi_base +SW_TWSI_EXT ); +writeqflush (ext, i2c->twsi_base +SW_TWSI_EXT (i2c));octeon_i2c_hlc_int_clear (i2c); -writeqflush (cmd, i2c->twsi_base +SW_TWSI ); +writeqflush (cmd, i2c->twsi_base +SW_TWSI (i2c)); ret =octeon_i2c_hlc_wait (i2c);if (ret) goto err; - cmd =__raw_readq (i2c->twsi_base +SW_TWSI ); + cmd =__raw_readq (i2c->twsi_base +SW_TWSI (i2c));if ((cmd &SW_TWSI_R ) ==0 )return -EAGAIN ;
drivers/i2c/busses/i2c-cavium.h +18 -17
@@ -7,17 +7,6 @@#include <linux/kernel.h> #include <linux/pci.h> -/* Register offsets */ -#if IS_ENABLED (CONFIG_I2C_THUNDERX ) -#define SW_TWSI 0x1000 -#define TWSI_INT 0x1010 -#define SW_TWSI_EXT 0x1018 -#else -#define SW_TWSI 0x00 -#define TWSI_INT 0x10 -#define SW_TWSI_EXT 0x18 -#endif -/* Controller command patterns */ #define SW_TWSI_V BIT_ULL(63)/* Valid bit */ #define SW_TWSI_EIA BIT_ULL(61)/* Extended internal address */
@@ -95,9 +84,21 @@#define TWSI_INT_SDA BIT_ULL(10)#define TWSI_INT_SCL BIT_ULL(11) +/* Register offsets */ +struct octeon_i2c_reg_offset { +unsigned int sw_twsi; +unsigned int twsi_int; +unsigned int sw_twsi_ext; +}; + +#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) +struct octeon_i2c {wait_queue_head_t queue;struct i2c_adapter adap; +struct octeon_i2c_reg_offset roff;struct clk *clk;int irq;int hlc_irq;/* For cn7890 only */
@@ -138,9 +139,9 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8 {u64 tmp; -__raw_writeq (SW_TWSI_V | eop_reg | data, i2c->twsi_base +SW_TWSI ); +__raw_writeq (SW_TWSI_V | eop_reg | data, i2c->twsi_base +SW_TWSI (i2c));do { - tmp =__raw_readq (i2c->twsi_base +SW_TWSI ); + tmp =__raw_readq (i2c->twsi_base +SW_TWSI (i2c)); }while ((tmp &SW_TWSI_V ) !=0 ); }
@@ -162,9 +163,9 @@ static u8 inline octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg) {u64 tmp; -__raw_writeq (SW_TWSI_V | eop_reg |SW_TWSI_R , i2c->twsi_base +SW_TWSI ); +__raw_writeq (SW_TWSI_V | eop_reg |SW_TWSI_R , i2c->twsi_base +SW_TWSI (i2c));do { - tmp =__raw_readq (i2c->twsi_base +SW_TWSI ); + tmp =__raw_readq (i2c->twsi_base +SW_TWSI (i2c)); }while ((tmp &SW_TWSI_V ) !=0 );return tmp &0xFF ;
@@ -185,7 +186,7 @@ static u8 inline octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)*/ static inline u64 octeon_i2c_read_int (struct octeon_i2c *i2c) { -return __raw_readq (i2c->twsi_base +TWSI_INT ); +return __raw_readq (i2c->twsi_base +TWSI_INT (i2c)); }/**
@@ -195,7 +196,7 @@ static inline u64 octeon_i2c_read_int(struct octeon_i2c *i2c)*/ static inline void octeon_i2c_write_int (struct octeon_i2c *i2c,u64 data) { -writeqflush (data, i2c->twsi_base +TWSI_INT ); +writeqflush (data, i2c->twsi_base +TWSI_INT (i2c)); }static inline int octeon_i2c_test_iflg (struct octeon_i2c *i2c)
drivers/i2c/busses/i2c-octeon-core.c +4 -0
@@ -163,6 +163,10 @@ static int octeon_i2c_probe(struct platform_device *pdev) } i2c->dev = &pdev->dev; + i2c->roff.sw_twsi =0x00 ; + i2c->roff.twsi_int =0x10 ; + i2c->roff.sw_twsi_ext =0x18 ; + res_mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); i2c->twsi_base =devm_ioremap_resource (&pdev->dev, res_mem);if (IS_ERR (i2c->twsi_base)) {
drivers/i2c/busses/i2c-thunderx-core.c +4 -0
@@ -136,6 +136,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,if (!i2c)return -ENOMEM ; + i2c->roff.sw_twsi =0x1000 ; + i2c->roff.twsi_int =0x1010 ; + i2c->roff.sw_twsi_ext =0x1018 ; + i2c->dev = dev;pci_set_drvdata (pdev, i2c); ret =pci_enable_device (pdev);
Signature · 1 line
1.9.1
PATCH v6 19/19 i2c: thunderx: Add smbus alert support
In reply to Jan GlauberAdd smbus alert interrupt support.
--- drivers/i2c/busses/i2c-cavium.h | 6 ++++++ drivers/i2c/busses/i2c-thunderx-core.c | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)
drivers/i2c/busses/i2c-cavium.h +6 -0
@@ -3,6 +3,7 @@#include <linux/delay.h> #include <linux/device.h> #include <linux/i2c.h> +#include <linux/i2c-smbus.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/pci.h>
@@ -119,6 +120,11 @@ struct octeon_i2c {#if IS_ENABLED (CONFIG_I2C_THUNDERX )struct msix_entry i2c_msix;#endif + +#if IS_ENABLED (CONFIG_I2C_SMBUS ) +struct i2c_smbus_alert_setup alert_data; +struct i2c_client *ara; +#endif };static inline void writeqflush (u64 val,void __iomem *addr)
drivers/i2c/busses/i2c-thunderx-core.c +35 -0
@@ -9,9 +9,11 @@#include <linux/clk.h> #include <linux/delay.h> #include <linux/i2c.h> +#include <linux/i2c-smbus.h> #include <linux/interrupt.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/of_irq.h> #include <linux/pci.h> #include "i2c-cavium.h"
@@ -108,6 +110,35 @@ static void thunder_i2c_clock_disable(struct device *dev, struct clk *clk)devm_clk_put (dev, clk); } +static int thunder_i2c_smbus_setup (struct octeon_i2c *i2c, +struct device_node *node) +{ +#if IS_ENABLED (CONFIG_I2C_SMBUS ) +u32 type; + + i2c->alert_data.irq =irq_of_parse_and_map (node,0 ); +if (!i2c->alert_data.irq) +return -EINVAL ; + + type =irqd_get_trigger_type (irq_get_irq_data (i2c->alert_data.irq)); + i2c->alert_data.alert_edge_triggered = + (type &IRQ_TYPE_LEVEL_MASK ) ?1 :0 ; + + i2c->ara =i2c_setup_smbus_alert (&i2c->adap, &i2c->alert_data); +if (!i2c->ara) +return -ENODEV ; +#endif +return 0 ; +} + +static void thunder_i2c_smbus_remove (struct octeon_i2c *i2c) +{ +#if IS_ENABLED (CONFIG_I2C_SMBUS ) +if (i2c->ara) +i2c_unregister_device (i2c->ara); +#endif +} +static void thunder_i2c_set_name (struct pci_dev *pdev,struct octeon_i2c *i2c,char *name) {
@@ -212,6 +243,9 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev, goto out_irq; } + ret =thunder_i2c_smbus_setup (i2c, node); +if (ret <0 ) +dev_info (dev,"Failed to setup smbus alert\n" );dev_info (i2c->dev,"probed\n" );return 0 ;
@@ -242,6 +276,7 @@ static void thunder_i2c_remove_pci(struct pci_dev *pdev) dev = i2c->dev;thunder_i2c_clock_disable (dev, i2c->clk); +thunder_i2c_smbus_remove (i2c);i2c_del_adapter (&i2c->adap);devm_free_irq (dev, i2c->i2c_msix.vector, i2c);pci_disable_msix (pdev);
Signature · 1 line
1.9.1