[U-Boot] [PATCH 0/2] stm32f7_i2c fixes
From: Patrice Chotard patrice.chotard@st.com
This series : _ fixes data abort for stm32f7_i2c driver _ removes useless local variable
Christophe Kerello (1): i2c: stm32f7_i2c: fix data abort
Patrice Chotard (1): i2c: stm32f7_i2c: fix usage of useless local variable
drivers/i2c/stm32f7_i2c.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)
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(-)
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,
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
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
From: Patrice Chotard patrice.chotard@st.com
Remove useless local variable "s" and use directly function's parameter "output"
Signed-off-by: Patrice Chotard patrice.chotard@st.com --- drivers/i2c/stm32f7_i2c.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index d519c17..610a9ef 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -640,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; struct list_head solutions; int ret;
@@ -676,16 +676,10 @@ static int stm32_i2c_compute_timing(struct stm32_i2c_priv *i2c_priv, if (ret) goto exit;
- ret = stm32_i2c_choose_solution(setup, &solutions, &s); + ret = stm32_i2c_choose_solution(setup, &solutions, output); if (ret) goto exit;
- 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, output->scldel, output->sdadel,
On 11 October 2017 at 15:30, patrice.chotard@st.com wrote:
From: Patrice Chotard patrice.chotard@st.com
Remove useless local variable "s" and use directly function's parameter "output"
Signed-off-by: Patrice Chotard patrice.chotard@st.com
drivers/i2c/stm32f7_i2c.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (4)
-
Heiko Schocher -
Patrice CHOTARD -
patrice.chotard＠st.com
-
Simon Glass