[U-Boot,1/2] i2c: stm32f7_i2c: fix data abort

Message ID 1507728635-31611-2-git-send-email-patrice.chotard@st.com
State Changes Requested
Delegated to: Heiko Schocher
Headers show
Series
  • stm32f7_i2c fixes
Related show

Commit Message

Patrice CHOTARD Oct. 11, 2017, 1:30 p.m.
From: Christophe Kerello <christophe.kerello@st.com>

As "v" is a local variable in stm32_i2c_choose_solution()
"v" has to be copied into "s" to avoid data abort in
stm32_i2c_compute_timing().

Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
Reviewed-by: Patrick DELAUNAY <patrick.delaunay@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/i2c/stm32f7_i2c.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Heiko Schocher Oct. 17, 2017, 3:45 a.m. | #1
Hello patrice,

Am 11.10.2017 um 15:30 schrieb patrice.chotard@st.com:
> From: Christophe Kerello <christophe.kerello@st.com>
> 
> As "v" is a local variable in stm32_i2c_choose_solution()
> "v" has to be copied into "s" to avoid data abort in
> stm32_i2c_compute_timing().
> 
> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> Reviewed-by: Patrick DELAUNAY <patrick.delaunay@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>   drivers/i2c/stm32f7_i2c.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)

Your patch does not apply to u-boot mainline:

hs@pollux [ 4:41:31] ttbott> git am -3 mbox
Applying: i2c: stm32f7_i2c: fix data abort
Using index info to reconstruct a base tree...
M	drivers/i2c/stm32f7_i2c.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/i2c/stm32f7_i2c.c
CONFLICT (content): Merge conflict in drivers/i2c/stm32f7_i2c.c
error: Failed to merge in the changes.
Patch failed at 0001 i2c: stm32f7_i2c: fix data abort
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
hs@pollux [ 4:41:31] ttbott>

For full log see for example on the corvus board:

http://xeidos.ddns.net/tbot/id_469/html_log.html

unfold the last three 4 "control" lines ...

please fix, thanks!

bye,
Heiko
Patrice CHOTARD Oct. 17, 2017, 9:20 a.m. | #2
Hi Heiko

On 10/17/2017 05:45 AM, Heiko Schocher wrote:
> Hello patrice,

> 

> Am 11.10.2017 um 15:30 schrieb patrice.chotard@st.com:

>> From: Christophe Kerello <christophe.kerello@st.com>

>>

>> As "v" is a local variable in stm32_i2c_choose_solution()

>> "v" has to be copied into "s" to avoid data abort in

>> stm32_i2c_compute_timing().

>>

>> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>

>> Reviewed-by: Patrick DELAUNAY <patrick.delaunay@st.com>

>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

>> ---

>>   drivers/i2c/stm32f7_i2c.c | 21 +++++++++++----------

>>   1 file changed, 11 insertions(+), 10 deletions(-)

> 

> Your patch does not apply to u-boot mainline:

> 

> hs@pollux [ 4:41:31] ttbott> git am -3 mbox

> Applying: i2c: stm32f7_i2c: fix data abort

> Using index info to reconstruct a base tree...

> M    drivers/i2c/stm32f7_i2c.c

> Falling back to patching base and 3-way merge...

> Auto-merging drivers/i2c/stm32f7_i2c.c

> CONFLICT (content): Merge conflict in drivers/i2c/stm32f7_i2c.c

> error: Failed to merge in the changes.

> Patch failed at 0001 i2c: stm32f7_i2c: fix data abort

> The copy of the patch that failed is found in: .git/rebase-apply/patch

> When you have resolved this problem, run "git am --continue".

> If you prefer to skip this patch, run "git am --skip" instead.

> To restore the original branch and stop patching, run "git am --abort".

> hs@pollux [ 4:41:31] ttbott>

> 

> For full log see for example on the corvus board:

> 

> http://xeidos.ddns.net/tbot/id_469/html_log.html

> 

> unfold the last three 4 "control" lines ...

> 

> please fix, thanks!


I will send a v2 to fix this issue

Thanks

Patrice

> 

> bye,

> Heiko

Patch

diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
index bf5fefa..d519c17 100644
--- a/drivers/i2c/stm32f7_i2c.c
+++ b/drivers/i2c/stm32f7_i2c.c
@@ -571,6 +571,7 @@  static int stm32_i2c_choose_solution(struct stm32_i2c_setup *setup,
 	u32 dnf_delay;
 	u32 tsync;
 	u16 l, h;
+	bool sol_found = false;
 	int ret = 0;
 
 	af_delay_min = setup->analog_filter ?
@@ -619,14 +620,15 @@  static int stm32_i2c_choose_solution(struct stm32_i2c_setup *setup,
 						clk_error_prev = clk_error;
 						v->scll = l;
 						v->sclh = h;
-						s = v;
+						sol_found = true;
+						memcpy(s, v, sizeof(*s));
 					}
 				}
 			}
 		}
 	}
 
-	if (!s) {
+	if (!sol_found) {
 		error("%s: no solution at all\n", __func__);
 		ret = -EPERM;
 	}
@@ -638,7 +640,7 @@  static int stm32_i2c_compute_timing(struct stm32_i2c_priv *i2c_priv,
 				      struct stm32_i2c_setup *setup,
 				      struct stm32_i2c_timings *output)
 {
-	struct stm32_i2c_timings *v, *_v, *s;
+	struct stm32_i2c_timings *v, *_v, s;
 	struct list_head solutions;
 	int ret;
 
@@ -669,21 +671,20 @@  static int stm32_i2c_compute_timing(struct stm32_i2c_priv *i2c_priv,
 		return -EINVAL;
 	}
 
-	s = NULL;
 	INIT_LIST_HEAD(&solutions);
 	ret = stm32_i2c_compute_solutions(setup, &solutions);
 	if (ret)
 		goto exit;
 
-	ret = stm32_i2c_choose_solution(setup, &solutions, s);
+	ret = stm32_i2c_choose_solution(setup, &solutions, &s);
 	if (ret)
 		goto exit;
 
-	output->presc = s->presc;
-	output->scldel = s->scldel;
-	output->sdadel = s->sdadel;
-	output->scll = s->scll;
-	output->sclh = s->sclh;
+	output->presc = s.presc;
+	output->scldel = s.scldel;
+	output->sdadel = s.sdadel;
+	output->scll = s.scll;
+	output->sclh = s.sclh;
 
 	debug("%s: Presc: %i, scldel: %i, sdadel: %i, scll: %i, sclh: %i\n",
 	      __func__, output->presc,