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 <[email protected]> >>>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. >>>1. Introuduce scl_gpio/sda_gpio/bus in mxc_i2c_bus. >>>2. 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. >>>3. 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. >>>4. 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 <[email protected]> >>>Cc: Albert Aribaud <[email protected]> >>>Cc: Stefano Babic <[email protected]> >>>Cc: Heiko Schocher <[email protected]> >>>Cc: Simon Glass <[email protected]> >>>Cc: York Sun <[email protected]> >>>--- >>> 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 <[email protected]> >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 >>> > >[.....] _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
