[U-Boot] [EXT] Re: [PATCH 1/2] dm: i2c: Add a flag that not call i2c_setup_offset

Tue Jun 4 07:59:44 UTC 2019


> -----Original Message-----
> From: Lukasz Majewski <lukma at denx.de>
> Sent: 2019年6月4日 15:56
> To: Chuanhua Han <chuanhua.han at nxp.com>
> Cc: hs at denx.de; u-boot at lists.denx.de; Biwen Li <biwen.li at nxp.com>
> Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/2] dm: i2c: Add a flag that not call
> i2c_setup_offset
> 
> On Tue, 4 Jun 2019 06:48:59 +0000
> Chuanhua Han <chuanhua.han at nxp.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukasz Majewski <lukma at denx.de>
> > > Sent: 2019年6月4日 14:46
> > > To: Chuanhua Han <chuanhua.han at nxp.com>
> > > Cc: hs at denx.de; u-boot at lists.denx.de; Biwen Li <biwen.li at nxp.com>
> > > Subject: Re: [EXT] Re: [U-Boot] [PATCH 1/2] dm: i2c: Add a flag that
> > > not call i2c_setup_offset
> > >
> > > On Tue, 4 Jun 2019 02:20:49 +0000
> > > Chuanhua Han <chuanhua.han at nxp.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Lukasz Majewski <lukma at denx.de>
> > > > > Sent: 2019年6月4日 6:08
> > > > > To: Chuanhua Han <chuanhua.han at nxp.com>
> > > > > Cc: hs at denx.de; u-boot at lists.denx.de; Biwen Li
> > > > > <biwen.li at nxp.com> Subject: [EXT] Re: [U-Boot] [PATCH 1/2] dm:
> > > > > i2c: Add a flag that not call i2c_setup_offset
> > > > >
> > > > > On Thu, 30 May 2019 18:31:48 +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_RD_NO_I2C_SETUP_OFFSET 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>
> > > > > > ---
> > > > > > Changes in v3:
> > > > > > 	- Use the new flag DM_I2C_CHIP_RD_NO_I2C_SETUP_OFFSET
> > > > > >
> > > > > > Changes in v2:
> > > > > >         - Split the original patch into 3 patches
> > > > > >         - Add detailed description information for each patch
> > > > > >
> > > > > >  drivers/i2c/i2c-uclass.c | 6 ++++--
> > > > > >  include/i2c.h            | 1 +
> > > > > >  2 files changed, 5 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/i2c/i2c-uclass.c
> > > > > > b/drivers/i2c/i2c-uclass.c index e47abf1833..9804b5e8c7 100644
> > > > > > --- a/drivers/i2c/i2c-uclass.c
> > > > > > +++ b/drivers/i2c/i2c-uclass.c
> > > > > > @@ -135,8 +135,10 @@ int dm_i2c_read(struct udevice *dev, uint
> > > > > > offset, uint8_t *buffer, int len) if (chip->flags &
> > > > > > DM_I2C_CHIP_RD_ADDRESS) return i2c_read_bytewise(dev, offset,
> > > > > > buffer, len); ptr = msg;
> > > > > > -	if (!i2c_setup_offset(chip, offset, offset_buf, ptr))
> > > > > > -		ptr++;
> > > > > > +	if (!(chip->flags &
> > > > > > DM_I2C_CHIP_RD_NO_I2C_SETUP_OFFSET)) {
> > > > > > +		if (!i2c_setup_offset(chip, offset,
> > > > > > offset_buf, ptr))
> > > > > > +			ptr++;
> > > > > > +	}
> > > > > >
> > > > > >  	if (len) {
> > > > > >  		ptr->addr = chip->chip_addr; diff --git a/include/i2c.h
> > > > > > b/include/i2c.h index
> > > > > > a5c760c711..3123cbf280 100644
> > > > > > --- a/include/i2c.h
> > > > > > +++ b/include/i2c.h
> > > > > > @@ -28,6 +28,7 @@ enum dm_i2c_chip_flags {
> > > > > >  	DM_I2C_CHIP_10BIT	= 1 << 0,
> > > > > >       /* Use 10-bit addressing */
> > > > > >       DM_I2C_CHIP_RD_ADDRESS	= 1 << 1,
> > > > > >       /* Send address for each read byte */
> > > > > >       DM_I2C_CHIP_WR_ADDRESS	= 1 << 2,
> > > > > >       /* Send address for each write byte */
> > > > >
> > > > > Aren't those two above flags describe exactly what you need?
> > > > > They send address for each read/written byte. (or do you need to
> > > > > send more than a single byte)?
> > > > This is not what I need. I need to generate a stop signal after
> > > > setting the register address,
> > >
> > > Please correct me if I'm wrong, but the above flag says that you
> > > need to send the I2C stop after you send one byte?
> > >
> > > Would it be possible to send address with I2C stop and then read
> > > data (single byte) with I2C stop?
> > >
> > > > so I need the following flag.
> > > > >
> > > > > >	DM_I2C_CHIP_RD_NO_I2C_SETUP_OFFSET  = 1 << 3,
> > >
> > > I do find this name as a bit misleading - is the
> > > RD_NO_I2C_SETUP_OFFSET name reflecting the purpose (to send I2C stop
> > > after sending address)?
> > The purpose is to send a stop signal after setting the slave register
> > address.
> 
> What I'm trying to point out is that the above description doesn't match the
> flag name. Maybe there is a better, more descriptive name?
Because the stop signal is generated by not calling the i2c_setup_offset function, this flag is used.
> 
> Also - what is byte length of send address to this device? One byte ?
Yes,one byte
> 
> > >
> > > Maybe there would be a better name?
> > >
> > > > > >       /* No i2c_setup_offset*/ };
> > > > > >
> > > > > >  struct udevice;
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Lukasz Majewski
> > > > >
> > > > > --
> > > > >
> > > > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194
> > > > > Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax:
> > > > > (+49)-8142-66989-80 Email: lukma at denx.de
> > >
> > >
> > >
> > >
> > > Best regards,
> > >
> > > Lukasz Majewski
> > >
> > > --
> > >
> > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > > lukma at denx.de
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma at denx.de