i2c: mxc: add a condition in case the parameter is NULL

PATCH New Gong Qianyu 4 participants 5 messages Dec 18, 2015 – Jan 3, 2016

Subsystems (2)

Arm i.MX and Layerscape I2C

lore

Share & tools

lore

Gong Qianyu

author

2015-12-18 09:38 UTC #0 lore raw

PATCH v3 1/1 i2c: mxc: add a condition in case the parameter is NULL

This could avoid executing the code that only applies to i.MX platforms.

The bus_i2c_init() is called before relocation and will assgin value to a static variable. If U-Boot is then still running in a flash device, it's theoretically not allowed to write data to flash without an erasing operation. For i.MX platforms, the U-Boot is always running in DDR.

Actually it causes asynchronous error when the ARM64 system error report is enabled and the flash write protect is set.

Signed-off-by Gong Qianyu <[email protected]>

---
V3:
- Sorry..Remove an unrelated line in other file.

 drivers/i2c/mxc_i2c.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

drivers/i2c/mxc_i2c.c +10 -2

index fa4c82f..b2d15c9 100644
@@ -581,8 +581,16 @@ void bus_i2c_init(int index, int speed, int unused,
 		return;
 	}
 
-	mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
-	mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
+	/*
+	 * Warning: Be careful to allow the assignment to a static
+	 * variable here. This function could be called while U-Boot is
+	 * still running in flash memory. So such assignment is equal
+	 * to write data to flash without erasing.
+	 */
+	if (idle_bus_fn)
+		mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
+	if (idle_bus_data)
+		mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
 
 	ret = enable_i2c_clk(1, index);
 	if (ret < 0) {

Signature · 1 line

2.1.0.27.g96db324

Heiko Schocher

2015-12-18 10:24 UTC #1 lore raw

PATCH v3 1/1 i2c: mxc: add a condition in case the parameter is NULL

Hello Gong Qianyu,

added Stefano Babic to cc as he is the imx maintainer.

Am 18.12.2015 um 10:38 schrieb Gong Qianyu:

Quoted 11 lines

This could avoid executing the code that only applies to i.MX platforms.
The bus_i2c_init() is called before relocation and will assgin value to a static variable. If U-Boot is then still running in a flash device, it's theoretically not allowed to write data to flash without an erasing operation. For i.MX platforms, the U-Boot is always running in DDR.
Actually it causes asynchronous error when the ARM64 system error report is enabled and the flash write protect is set.

Quoted 7 lines

Signed-off-by Gong Qianyu <[email protected]>
---
V3:
- Sorry..Remove an unrelated line in other file.
  drivers/i2c/mxc_i2c.c | 12 ++++++++++--
  1 file changed, 10 insertions(+), 2 deletions(-)

Ok, from my side, but it would be better to switch your board(s) to support DM and get rid of this old stuff from this driver.

Reviewed-by Heiko Schocher <[email protected]>

bye,
Heiko

Quoted 25 lines

drivers/i2c/mxc_i2c.c +10 -2

index fa4c82f..b2d15c9 100644
@@ -581,8 +581,16 @@ void bus_i2c_init(int index, int speed, int unused,
  		return;
  	}
 
-	mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
-	mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
+	/*
+	 * Warning: Be careful to allow the assignment to a static
+	 * variable here. This function could be called while U-Boot is
+	 * still running in flash memory. So such assignment is equal
+	 * to write data to flash without erasing.
+	 */
+	if (idle_bus_fn)
+		mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
+	if (idle_bus_data)
+		mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
 
  	ret = enable_i2c_clk(1, index);
  	if (ret < 0) {

Signature · 2 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Qianyu Gong

2015-12-21 05:41 UTC #2 lore raw

PATCH v3 1/1 i2c: mxc: add a condition in case the parameter is NULL

Quoted 34 lines

-----Original Message-----
From: Heiko Schocher [mailto:hs at denx.de]
Sent: Friday, December 18, 2015 6:24 PM
To: Gong Qianyu-B52263
Cc: u-boot at lists.denx.de; Stefano Babic
Subject: Re: [U-Boot] [Patch V3] i2c: mxc: add a condition in case the parameter is NULL
Hello Gong Qianyu,
added Stefano Babic to cc as he is the imx maintainer.
Am 18.12.2015 um 10:38 schrieb Gong Qianyu:
Quoted 11 lines
This could avoid executing the code that only applies to i.MX platforms.
The bus_i2c_init() is called before relocation and will assgin value to a static variable. If U-Boot is then still running in a flash device, it's theoretically not allowed to write data to flash without an erasing operation. For i.MX platforms, the U-Boot is always running in DDR.
Actually it causes asynchronous error when the ARM64 system error report is enabled and the flash write protect is set.
Quoted 7 lines
Signed-off-by Gong Qianyu <[email protected]>
---
V3:
- Sorry..Remove an unrelated line in other file.
  drivers/i2c/mxc_i2c.c | 12 ++++++++++--
  1 file changed, 10 insertions(+), 2 deletions(-)

Quoted 7 lines

Ok, from my side, but it would be better to switch your board(s) to support DM and get rid of this old stuff from this driver.
Reviewed-by Heiko Schocher <[email protected]>
bye,
Heiko

Thanks. But I have no control over this driver. Maybe they are considering to convert it later.

Regards,
Qianyu

Quoted 29 lines

Quoted 15 lines
drivers/i2c/mxc_i2c.c index renamed from drivers/i2c/mxc_i2c.c +0 -0
fa4c82f..b2d15c9 100644
drivers/i2c/mxc_i2c.c +3 -2
@@ -581,8 +581,16 @@ void bus_i2c_init(int index, int speed, int unused,
  		return;
  	}
 
-	mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
-	mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
+	/*
+	 * Warning: Be careful to allow the assignment to a static
+	 * variable here. This function could be called while U-Boot is
drivers/i2c/mxc_i2c.c void bus_i2c_init(int index, int speed, int unused, · line 596
Quoted 10 lines
drivers/i2c/mxc_i2c.c +7 -0
+	 * still running in flash memory. So such assignment is equal
+	 * to write data to flash without erasing.
+	 */
+	if (idle_bus_fn)
+		mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
+	if (idle_bus_data)
+		mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
 
  	ret = enable_i2c_clk(1, index);
  	if (ret < 0) {
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Qianyu Gong

2015-12-30 09:27 UTC #3 lore raw

PATCH v3 1/1 i2c: mxc: add a condition in case the parameter is NULL

Hi Stefano,

Could you please help to merge this patch? Thanks very much.

Regards,
Qianyu

Quoted 85 lines

-----Original Message-----
From: Qianyu Gong
Sent: Monday, December 21, 2015 1:42 PM
To: hs at denx.de; Gong Qianyu-B52263 <[email protected]>
Cc: u-boot at lists.denx.de; Stefano Babic <[email protected]>
Subject: RE: [U-Boot] [Patch V3] i2c: mxc: add a condition in case the parameter is NULL
Quoted 32 lines
-----Original Message-----
From: Heiko Schocher [mailto:hs at denx.de]
Sent: Friday, December 18, 2015 6:24 PM
To: Gong Qianyu-B52263
Cc: u-boot at lists.denx.de; Stefano Babic
Subject: Re: [U-Boot] [Patch V3] i2c: mxc: add a condition in case the parameter is NULL
Hello Gong Qianyu,
added Stefano Babic to cc as he is the imx maintainer.
Am 18.12.2015 um 10:38 schrieb Gong Qianyu:
Quoted 11 lines
This could avoid executing the code that only applies to i.MX platforms.
The bus_i2c_init() is called before relocation and will assgin value to a static variable. If U-Boot is then still running in a flash device, it's theoretically not allowed to write data to flash without an erasing operation. For i.MX platforms, the U-Boot is always running in DDR.
Actually it causes asynchronous error when the ARM64 system error report is enabled and the flash write protect is set.
Quoted 7 lines
Signed-off-by Gong Qianyu <[email protected]>
---
V3:
- Sorry..Remove an unrelated line in other file.
  drivers/i2c/mxc_i2c.c | 12 ++++++++++--
  1 file changed, 10 insertions(+), 2 deletions(-)
Quoted 7 lines
Ok, from my side, but it would be better to switch your board(s) to support DM and get rid of this old stuff from this driver.
Reviewed-by Heiko Schocher <[email protected]>
bye,
Heiko
Thanks. But I have no control over this driver. Maybe they are considering to convert it later.
Regards,
Qianyu
Quoted 29 lines
Quoted 15 lines
drivers/i2c/mxc_i2c.c index renamed from drivers/i2c/mxc_i2c.c +0 -0
fa4c82f..b2d15c9 100644
drivers/i2c/mxc_i2c.c +3 -2
@@ -581,8 +581,16 @@ void bus_i2c_init(int index, int speed, int unused,
  		return;
  	}
 
-	mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
-	mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
+	/*
+	 * Warning: Be careful to allow the assignment to a static
+	 * variable here. This function could be called while U-Boot is
drivers/i2c/mxc_i2c.c void bus_i2c_init(int index, int speed, int unused, · line 596
Quoted 10 lines
drivers/i2c/mxc_i2c.c +7 -0
+	 * still running in flash memory. So such assignment is equal
+	 * to write data to flash without erasing.
+	 */
+	if (idle_bus_fn)
+		mxc_i2c_buses[index].idle_bus_fn = idle_bus_fn;
+	if (idle_bus_data)
+		mxc_i2c_buses[index].idle_bus_data = idle_bus_data;
 
  	ret = enable_i2c_clk(1, index);
  	if (ret < 0) {
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Stefano Babic

2016-01-03 15:02 UTC #4 lore raw

PATCH v3 1/1 i2c: mxc: add a condition in case the parameter is NULL

In reply to Gong Qianyu

Quoted 8 lines · Gong Qianyu

This could avoid executing the code that only applies to i.MX platforms.
The bus_i2c_init() is called before relocation and will assgin value to a static variable. If U-Boot is then still running in a flash device, it's theoretically not allowed to write data to flash without an erasing operation. For i.MX platforms, the U-Boot is always running in DDR.

Quoted 5 lines

Actually it causes asynchronous error when the ARM64 system error report is enabled and the flash write protect is set.
Signed-off-by Gong Qianyu <[email protected]>
---

Applied to u-boot-imx, thanks!

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================