-----Original Message----- From: Simon Glass sjg@chromium.org Sent: 2019年7月7日 1:17 To: Chuanhua Han chuanhua.han@nxp.com Cc: Lukasz Majewski lukma@denx.de; hs@denx.de; Biwen Li biwen.li@nxp.com; u-boot@lists.denx.de Subject: Re: [EXT] Re: [PATCH 1/2] dm: i2c: Add a flag that need generate stop bit
Caution: EXT Email
Hi Chuanhua,
On Sun, 23 Jun 2019 at 22:48, Chuanhua Han chuanhua.han@nxp.com wrote:
- Simon Glass
-----Original Message----- From: Lukasz Majewski lukma@denx.de Sent: 2019年6月18日 16:07 To: Chuanhua Han chuanhua.han@nxp.com Cc: hs@denx.de; sjg@chromium.org; Biwen Li biwen.li@nxp.com; u-boot@lists.denx.de Subject: [EXT] Re: [PATCH 1/2] dm: i2c: Add a flag that need generate stop bit
On Mon, 17 Jun 2019 21:12:40 +0800 Chuanhua Han chuanhua.han@nxp.com wrote:
Usually the i2c bus needs to write the address of the register before reading the internal register data of the device (ignoring the transmission of the slave address).
Generally, the stop signal is not needed before the register is read, but there is a special chip that needs this stop signal (such as pcf2127). However, in the current i2c general code, the dm_i2c_read api encapsulates two messages, the first time is to set the register address message, the second time is a message to read the register data, so that no stop signal is generated.
This patch uses the DM_I2C_CHIP_ADDR_STOP flag for specific i2c chips, so if the i2c slave requires a stop signal, chips driver can set this flag, then call the dm_i2c_write to set the register address (a stop signal is generated after this API call), then call dm_i2c_read to read the register data.
Signed-off-by: Chuanhua Han chuanhua.han@nxp.com
Reviewed-by: Lukasz Majewski lukma@denx.de
I already asked why you cannot use dm_i2c_xfer() to do what you want, but I did not see your response. Can you please send your response again here on this thread? The dm_i2c_xfer() function is designed to handle any sorts of quirk that are needed. I'd like to avoid quirks in the core code if possible.
If you really want to add a quirk, then I would like to put it behind a CONFIG_I2C_QUIRKS option, and use:
if (IS_ENABLED(CONFIG_I2C_QUIRKS) || !(chip->flags & DM_I2C_CHIP_ADDR_STOP)) { if (!i2c_setup_offset(chip, offset, offset_buf, ptr)) ptr++;
so we don't affect code size on other platforms. After all this feature is just for one broken chip and should not negatively affect everyone else that follows the spec correctly.
I am worried about all the SPL-affecting changing that add a few bytes here and there, but really add up.
But my first preference would be to use i2c_xfer() if possible.
OK,thanks,I will send next version!
Regards, Simon