Hi Heiko,
On Mon, Mar 21, 2016 at 05:10:45PM +0800, Peng Fan wrote:
Hi Heiko,
On Mon, Mar 21, 2016 at 07:50:32AM +0100, Heiko Schocher wrote:
Hello Peng Fan,
Sorry for the late reply ...
Am 11.03.2016 um 09:47 schrieb Peng Fan:
Implement i2c_idle_bus in driver, then setup_i2c can be dropped for boards which enable DM_I2C/DM_GPIO/PINCTRL. The i2c_idle_bus force bus idle flow follows setup_i2c in arch/arm/imx-common/i2c-mxv7.c
This patch is an implementation following linux kernel patch: " commit 1c4b6c3bcf30d0804db0d0647d8ebeb862c6f7e5 Author: Gao Pan b54642@freescale.com Date: Fri Oct 23 20:28:54 2015 +0800
i2c: imx: implement bus recovery Implement bus recovery methods for i2c-imx so we can recover from situations where SCL/SDA are stuck low. Once i2c bus SCL/SDA are stuck low during transfer, config the i2c pinctrl to gpio mode by calling pinctrl sleep set function, and then use GPIO to emulate the i2c protocol to send nine dummy clock to recover i2c device. After recovery, set i2c pinctrl to default group setting."
See Documentation/devicetree/bindings/i2c/i2c-imx.txt for detailed description.
- Introuduce scl_gpio/sda_gpio/bus in mxc_i2c_bus.
- Discard the __weak attribute for i2c_idle_bus and implement it, since we have pinctrl driver/driver model gpio driver. We can use device tree, but not let board code to do this.
- gpio state for mxc_i2c is not a must, but it is recommended. If there is no gpio state, driver will give tips, but not fail.
- The i2c controller was first probed, default pinctrl state will be used, so when need to use gpio function, need to do "pinctrl_select_state(dev, "gpio")" and after force bus idle, need to switch back "pinctrl_select_state(dev, "default")".
This is example about how to use the gpio force bus idle function: " &i2c1 { clock-frequency = <100000>; pinctrl-names = "default", "gpio"; pinctrl-0 = <&pinctrl_i2c1>; pinctrl-1 = <&pinctrl_i2c1_gpio>; scl-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; sda-gpios = <&gpio1 29 GPIO_ACTIVE_HIGH>; status = "okay"; [....] };
[.....]
pinctrl_i2c1_gpio: i2c1grp_gpio { fsl,pins = < MX6UL_PAD_UART4_TX_DATA__GPIO1_IO28 0x1b8b0 MX6UL_PAD_UART4_RX_DATA__GPIO1_IO29 0x1b8b0 >; }; "
Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Stefano Babic sbabic@denx.de Cc: Heiko Schocher hs@denx.de Cc: Simon Glass sjg@chromium.org Cc: York Sun york.sun@nxp.com
arch/arm/include/asm/imx-common/mxc_i2c.h | 10 +++ drivers/i2c/mxc_i2c.c | 101 +++++++++++++++++++++++++++--- 2 files changed, 102 insertions(+), 9 deletions(-)
Thanks for this patch. In principle it looks very good ... I have only a "nitpick" ... Couldn;t we split this patch into a common piece, which does the deblocking of the bus, and a "board/driver" specific part, where we do the pinmux changes?
There is nothing "imx" special in the deblocking of the i2c bus, beside switching the pinmux ... and as you use DM and DT there is not even in your patch a imx special part ...
So I tend to say, we can move this piece of code into a more common place (drivers/i2c/i2c_core.c or into a new file i2c_deblock.c) instead having it in the imx i2c driver ...
Can you rework this?
The idea of this patch is from Linux I2C patch for IMX: " commit 1c4b6c3bcf30d0804db0d0647d8ebeb862c6f7e5 Author: Gao Pan b54642@freescale.com Date: Fri Oct 23 20:28:54 2015 +0800
i2c: imx: implement bus recovery Implement bus recovery methods for i2c-imx so we can recover from situations where SCL/SDA are stuck low."
so I would like to keep it in mxc i2c driver for now. When other i2c drivers have such requirement to force bus idle, then we can think of a new common way to support them.
Will you reject this patch or pick up this patch?
Thanks, Peng.
Thanks, Peng.
Thanks a lot!
bye, Heiko
[.....]