On 12/20/2022 7:11 PM, Andy Shevchenko wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
On Tue, Dec 20, 2022 at 04:48:06PM +0000, Hanna Hawa wrote:
From: Lareine Khawaly <lareine@xxxxxxxxxx>
In functions i2c_dw_scl_lcnt() and i2c_dw_scl_hcnt() may have overflow
by depending on the values of the given parameters including the ic_clk.
For example in our use case where ic_clk is larger than one million,
multiplication of ic_clk * 4700 will result in 32 bit overflow.
Add cast of u64 to the calculation to avoid multiplication overflow, and
use the corresponding define for divide.
...
- return DIV_ROUND_CLOSEST(ic_clk * tSYMBOL, MICRO) - 8 + offset;
+ return DIV_ROUND_CLOSEST_ULL((u64)ic_clk * tSYMBOL, MICRO) - 8 +
+ offset;
Broken indentation.
...
- return DIV_ROUND_CLOSEST(ic_clk * (tSYMBOL + tf), MICRO) - 3 + offset;
+ return DIV_ROUND_CLOSEST_ULL((u64)ic_clk * (tSYMBOL + tf),
+ MICRO) - 3 + offset;
I would still go with 'MICRO) -' part to be on the previous line despite being
over 80, this is logical split which increases readability.
Okay.. will move the 'MICRO) -' one line before
- return DIV_ROUND_CLOSEST(ic_clk * (tLOW + tf), MICRO) - 1 + offset;
+ return DIV_ROUND_CLOSEST_ULL((u64)ic_clk * (tLOW + tf), MICRO) - 1 +
+ offset;
Broken indentation.
Why it's broken indentation? I'm asking to know for the next time. The
word 'offset' is not part of DIV_ROUND_CLOSEST_ULL parentheses. In wrong
indentation the checkpatch shout about it, but it didn't happen with the
above.
Does the below the correct indentation?
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -351,7 +351,8 @@ u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf,
int cond, int offset)
*
* If your hardware is free from tHD;STA issue, try
this one.
*/
- return DIV_ROUND_CLOSEST(ic_clk * tSYMBOL, MICRO) - 8 +
offset;
+ return DIV_ROUND_CLOSEST_ULL((u64)ic_clk * tSYMBOL, MICRO) -
+ 8 + offset;
else
/*
* Conditional expression:
@@ -367,7 +368,8 @@ u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf,
int cond, int offset)
* The reason why we need to take into account "tf" here,
* is the same as described in i2c_dw_scl_lcnt().
*/
- return DIV_ROUND_CLOSEST(ic_clk * (tSYMBOL + tf), MICRO)
- 3 + offset;
+ return DIV_ROUND_CLOSEST_ULL((u64)ic_clk * (tSYMBOL +
tf), MICRO) -
+ 3 + offset;
}
u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
@@ -383,7 +385,8 @@ u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf,
int offset)
* account the fall time of SCL signal (tf). Default tf value
* should be 0.3 us, for safety.
*/
- return DIV_ROUND_CLOSEST(ic_clk * (tLOW + tf), MICRO) - 1 + offset;
+ return DIV_ROUND_CLOSEST_ULL((u64)ic_clk * (tLOW + tf), MICRO) -
+ 1 + offset;
}
--
With Best Regards,
Andy Shevchenko
