[U-Boot] [EXT] Re: [PATCH 1/2] dm: i2c: Add a flag that need generate stop bit

[Chuanhua Han]

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: 2019年7月7日 1:17
> To: Chuanhua Han <chuanhua.han at nxp.com>
> Cc: Lukasz Majewski <lukma at denx.de>; hs at denx.de; Biwen Li
> <biwen.li at nxp.com>; u-boot at 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 at nxp.com>
> wrote:
> >
> > + Simon Glass
> >
> > > -----Original Message-----
> > > From: Lukasz Majewski <lukma at denx.de>
> > > Sent: 2019年6月18日 16:07
> > > To: Chuanhua Han <chuanhua.han at nxp.com>
> > > Cc: hs at denx.de; sjg at chromium.org; Biwen Li <biwen.li at nxp.com>;
> > > u-boot at 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 at 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 at nxp.com>
> > >
> > > Reviewed-by: Lukasz Majewski <lukma at 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