| Message ID | 20240322132619.6389-1-wsa+renesas@sang-engineering.com |
|---|---|
| Headers | show |
| Series | i2c: reword i2c_algorithm according to newest specification | expand |
Am Freitag, 22. März 2024, 14:25:40 CET schrieb Wolfram Sang: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/i2c/busses/i2c-rk3x.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c > index 086fdf262e7b..01febd886bdd 100644 > --- a/drivers/i2c/busses/i2c-rk3x.c > +++ b/drivers/i2c/busses/i2c-rk3x.c > @@ -28,8 +28,8 @@ > /* Register Map */ > #define REG_CON 0x00 /* control register */ > #define REG_CLKDIV 0x04 /* clock divisor register */ > -#define REG_MRXADDR 0x08 /* slave address for REGISTER_TX */ > -#define REG_MRXRADDR 0x0c /* slave register address for REGISTER_TX */ > +#define REG_MRXADDR 0x08 /* client address for REGISTER_TX */ > +#define REG_MRXRADDR 0x0c /* client register address for REGISTER_TX */ > #define REG_MTXCNT 0x10 /* number of bytes to be transmitted */ > #define REG_MRXCNT 0x14 /* number of bytes to be received */ > #define REG_IEN 0x18 /* interrupt enable */ > @@ -68,8 +68,8 @@ enum { > /* REG_IEN/REG_IPD bits */ > #define REG_INT_BTF BIT(0) /* a byte was transmitted */ > #define REG_INT_BRF BIT(1) /* a byte was received */ > -#define REG_INT_MBTF BIT(2) /* master data transmit finished */ > -#define REG_INT_MBRF BIT(3) /* master data receive finished */ > +#define REG_INT_MBTF BIT(2) /* host data transmit finished */ > +#define REG_INT_MBRF BIT(3) /* host data receive finished */ > #define REG_INT_START BIT(4) /* START condition generated */ > #define REG_INT_STOP BIT(5) /* STOP condition generated */ > #define REG_INT_NAKRCV BIT(6) /* NACK received */ > @@ -184,7 +184,7 @@ struct rk3x_i2c_soc_data { > * @wait: the waitqueue to wait for i2c transfer > * @busy: the condition for the event to wait for > * @msg: current i2c message > - * @addr: addr of i2c slave device > + * @addr: addr of i2c client device > * @mode: mode of i2c transfer > * @is_last_msg: flag determines whether it is the last msg in this transfer > * @state: state of i2c transfer > @@ -979,7 +979,7 @@ static int rk3x_i2c_setup(struct rk3x_i2c *i2c, struct i2c_msg *msgs, int num) > /* > * The I2C adapter can issue a small (len < 4) write packet before > * reading. This speeds up SMBus-style register reads. > - * The MRXADDR/MRXRADDR hold the slave address and the slave register > + * The MRXADDR/MRXRADDR hold the client address and the client register > * address in this case. > */ > > @@ -1016,7 +1016,7 @@ static int rk3x_i2c_setup(struct rk3x_i2c *i2c, struct i2c_msg *msgs, int num) > addr |= 1; /* set read bit */ > > /* > - * We have to transmit the slave addr first. Use > + * We have to transmit the client addr first. Use > * MOD_REGISTER_TX for that purpose. > */ > i2c->mode = REG_CON_MOD_REGISTER_TX; > @@ -1162,8 +1162,8 @@ static u32 rk3x_i2c_func(struct i2c_adapter *adap) > } > > static const struct i2c_algorithm rk3x_i2c_algorithm = { > - .master_xfer = rk3x_i2c_xfer, > - .master_xfer_atomic = rk3x_i2c_xfer_polling, > + .xfer = rk3x_i2c_xfer, > + .xfer_atomic = rk3x_i2c_xfer_polling, > .functionality = rk3x_i2c_func, > }; > >
On Fri, Mar 22, 2024 at 02:25:37PM +0100, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Bjorn Andersson <andersson@kernel.org> Regards, Bjorn > --- > drivers/i2c/busses/i2c-qup.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > index 598102d16677..2aeb5c33a711 100644 > --- a/drivers/i2c/busses/i2c-qup.c > +++ b/drivers/i2c/busses/i2c-qup.c > @@ -380,7 +380,7 @@ static int qup_i2c_poll_state_mask(struct qup_i2c_dev *qup, > u32 state; > > /* > - * State transition takes 3 AHB clocks cycles + 3 I2C master clock > + * State transition takes 3 AHB clocks cycles + 3 I2C host clock > * cycles. So retry once after a 1uS delay. > */ > do { > @@ -1607,12 +1607,12 @@ static u32 qup_i2c_func(struct i2c_adapter *adap) > } > > static const struct i2c_algorithm qup_i2c_algo = { > - .master_xfer = qup_i2c_xfer, > + .xfer = qup_i2c_xfer, > .functionality = qup_i2c_func, > }; > > static const struct i2c_algorithm qup_i2c_algo_v2 = { > - .master_xfer = qup_i2c_xfer_v2, > + .xfer = qup_i2c_xfer_v2, > .functionality = qup_i2c_func, > }; > > -- > 2.43.0 >
On 3/22/2024 6:25 AM, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > drivers/i2c/busses/i2c-st.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c > index ce2333408904..9bd45ae83c0c 100644 > --- a/drivers/i2c/busses/i2c-st.c > +++ b/drivers/i2c/busses/i2c-st.c > @@ -2,7 +2,7 @@ > /* > * Copyright (C) 2013 STMicroelectronics > * > - * I2C master mode controller driver, used in STMicroelectronics devices. > + * I2C host controller driver, used in STMicroelectronics devices. > * > * Author: Maxime Coquelin <maxime.coquelin@st.com> > */ > @@ -150,7 +150,7 @@ struct st_i2c_timings { > > /** > * struct st_i2c_client - client specific data > - * @addr: 8-bit slave addr, including r/w bit > + * @addr: 8-bit client addr, including r/w bit > * @count: number of bytes to be transfered > * @xfered: number of bytes already transferred > * @buf: data buffer > @@ -647,7 +647,7 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg, > { > struct st_i2c_client *c = &i2c_dev->client; > u32 ctl, i2c, it; > - unsigned long timeout; > + unsigned long time_left; Thanks for doing this. Is the timeout v/s time_left language also due to the specification change? A link to the specification(s) in the commit message would be nice to have > int ret; > > c->addr = i2c_8bit_addr_from_msg(msg); > @@ -667,7 +667,7 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg, > i2c |= SSC_I2C_ACKG; > st_i2c_set_bits(i2c_dev->base + SSC_I2C, i2c); > > - /* Write slave address */ > + /* Write client address */ > st_i2c_write_tx_fifo(i2c_dev, c->addr); > > /* Pre-fill Tx fifo with data in case of write */ > @@ -685,15 +685,12 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg, > st_i2c_set_bits(i2c_dev->base + SSC_I2C, SSC_I2C_STRTG); > } > > - timeout = wait_for_completion_timeout(&i2c_dev->complete, > + time_left = wait_for_completion_timeout(&i2c_dev->complete, > i2c_dev->adap.timeout); > ret = c->result; > > - if (!timeout) { > - dev_err(i2c_dev->dev, "Write to slave 0x%x timed out\n", > - c->addr); > + if (!time_left) > ret = -ETIMEDOUT; > - } Why did we lost the dev_err() here? > > i2c = SSC_I2C_STOPG | SSC_I2C_REPSTRTG; > st_i2c_clr_bits(i2c_dev->base + SSC_I2C, i2c); > @@ -769,7 +766,7 @@ static u32 st_i2c_func(struct i2c_adapter *adap) > } > > static const struct i2c_algorithm st_i2c_algo = { > - .master_xfer = st_i2c_xfer, > + .xfer = st_i2c_xfer, > .functionality = st_i2c_func, > }; >
> -----Original Message----- > From: Wolfram Sang <wsa+renesas@sang-engineering.com> > Sent: Friday, 22 March 2024 15:25 > To: linux-i2c@vger.kernel.org > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>; Vadim Pasternak > <vadimp@nvidia.com>; Michael Shych <michaelsh@nvidia.com>; Andi Shyti > <andi.shyti@kernel.org>; linux-kernel@vger.kernel.org > Subject: [PATCH 35/64] i2c: mlxcpld: reword according to newest specification > > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Acked-by: Vadim Pasternak <vadimp@nvidia.com> > --- > drivers/i2c/busses/i2c-mlxcpld.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mlxcpld.c b/drivers/i2c/busses/i2c- > mlxcpld.c > index 099291a0411d..786d4c51f65a 100644 > --- a/drivers/i2c/busses/i2c-mlxcpld.c > +++ b/drivers/i2c/busses/i2c-mlxcpld.c > @@ -197,8 +197,8 @@ static int mlxcpld_i2c_check_status(struct > mlxcpld_i2c_priv *priv, int *status) > if (val & MLXCPLD_LPCI2C_TRANS_END) { > if (val & MLXCPLD_LPCI2C_STATUS_NACK) > /* > - * The slave is unable to accept the data. No such > - * slave, command not understood, or unable to > accept > + * The client is unable to accept the data. No such > + * client, command not understood, or unable to > accept > * any more data. > */ > *status = MLXCPLD_LPCI2C_NACK_IND; > @@ -280,7 +280,7 @@ static int mlxcpld_i2c_wait_for_free(struct > mlxcpld_i2c_priv *priv) } > > /* > - * Wait for master transfer to complete. > + * Wait for host transfer to complete. > * It puts current process to sleep until we get interrupt or timeout expires. > * Returns the number of transferred or read bytes or error (<0). > */ > @@ -315,7 +315,7 @@ static int mlxcpld_i2c_wait_for_tc(struct > mlxcpld_i2c_priv *priv) > /* > * Actual read data len will be always the same as > * requested len. 0xff (line pull-up) will be returned > - * if slave has no data to return. Thus don't read > + * if client has no data to return. Thus don't read > * MLXCPLD_LPCI2C_NUM_DAT_REG reg from CPLD. Only in > case of > * SMBus block read transaction data len can be different, > * check this case. > @@ -375,7 +375,7 @@ static void mlxcpld_i2c_xfer_msg(struct > mlxcpld_i2c_priv *priv) > } > > /* > - * Set target slave address with command for master transfer. > + * Set client address with command for host transfer. > * It should be latest executed function before CPLD transaction. > */ > cmd = (priv->xfer.msg[0].addr << 1) | priv->xfer.cmd; @@ -449,7 > +449,7 @@ static u32 mlxcpld_i2c_func(struct i2c_adapter *adap) } > > static const struct i2c_algorithm mlxcpld_i2c_algo = { > - .master_xfer = mlxcpld_i2c_xfer, > + .xfer = mlxcpld_i2c_xfer, > .functionality = mlxcpld_i2c_func > }; > > -- > 2.43.0
Hi Wolfram, On Fri, Mar 22, 2024 at 02:24:53PM +0100, Wolfram Sang wrote: > Okay, we need to begin somewhere... > > Start changing the wording of the I2C main header wrt. the newest I2C > v7, SMBus 3.2, I3C specifications and replace "master/slave" with more > appropriate terms. This first step renames the members of struct > i2c_algorithm. Once all in-tree users are converted, the anonymous union > will go away again. All this work will also pave the way for finally > seperating the monolithic header into more fine-grained headers like > "i2c/clients.h" etc. So, this is not a simple renaming-excercise but > also a chance to update the I2C core to recent Linux standards. yes, very good! It's clearly stated in all three documentations that Target replaces Slave and Controller replaces Master (i3c is at the 1.1.1 version). > My motivation is to improve the I2C core API, in general. My motivation > is not to clean each and every driver. I think this is impossible > because register names based on official documentation will need to stay > as they are. But the Linux-internal names should be updated IMO. Also because some drivers have been written based on previous specifications where master/slave was used. > That being said, I worked on 62 drivers in this series beyond plain > renames inside 'struct i2c_algorithm' because the fruits were so > low-hanging. Before this series, 112 files in the 'busses/' directory > contained 'master' and/or 'slave'. After the series, only 57. Why not? > > Next step is updating the drivers outside the 'i2c'-folder regarding > 'struct i2c_algorithm' so we can remove the anonymous union ASAP. To be > able to work on this with minimal dependencies, I'd like to apply this > series between -rc1 and -rc2. > > I hope this will work for you guys. The changes are really minimal. If > you are not comfortable with changes to your driver or need more time to > review, please NACK the patch and I will drop the patch and/or address > the issues separeately. > > @Andi: are you okay with this approach? It means you'd need to merge > -rc2 into your for-next branch. Or rebase if all fails. I think it's a good plan, I'll try to support you with it. > Speaking of Andi, thanks a lot to him taking care of the controller > drivers these days. His work really gives me the freedom to work on I2C > core issues again. Thank you, Wolfram! Andi
Il 22/03/24 14:25, Wolfram Sang ha scritto: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On 3/22/24 14:25, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > drivers/i2c/busses/i2c-mt7621.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c > index 81d46169bc1f..c567a2cf4a90 100644 > --- a/drivers/i2c/busses/i2c-mt7621.c > +++ b/drivers/i2c/busses/i2c-mt7621.c > @@ -117,26 +117,26 @@ static int mtk_i2c_check_ack(struct mtk_i2c *i2c, u32 expected) > return ((ack & ack_expected) == ack_expected) ? 0 : -ENXIO; > } > > -static int mtk_i2c_master_start(struct mtk_i2c *i2c) > +static int mtk_i2c_host_start(struct mtk_i2c *i2c) > { > iowrite32(SM0CTL1_START | SM0CTL1_TRI, i2c->base + REG_SM0CTL1_REG); > return mtk_i2c_wait_idle(i2c); > } > > -static int mtk_i2c_master_stop(struct mtk_i2c *i2c) > +static int mtk_i2c_host_stop(struct mtk_i2c *i2c) > { > iowrite32(SM0CTL1_STOP | SM0CTL1_TRI, i2c->base + REG_SM0CTL1_REG); > return mtk_i2c_wait_idle(i2c); > } > > -static int mtk_i2c_master_cmd(struct mtk_i2c *i2c, u32 cmd, int page_len) > +static int mtk_i2c_host_cmd(struct mtk_i2c *i2c, u32 cmd, int page_len) > { > iowrite32(cmd | SM0CTL1_TRI | SM0CTL1_PGLEN(page_len), > i2c->base + REG_SM0CTL1_REG); > return mtk_i2c_wait_idle(i2c); > } > > -static int mtk_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > +static int mtk_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > int num) > { > struct mtk_i2c *i2c; > @@ -157,7 +157,7 @@ static int mtk_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > goto err_timeout; > > /* start sequence */ > - ret = mtk_i2c_master_start(i2c); > + ret = mtk_i2c_host_start(i2c); > if (ret) > goto err_timeout; > > @@ -169,14 +169,14 @@ static int mtk_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > if (pmsg->flags & I2C_M_RD) > addr |= 1; > iowrite32(addr, i2c->base + REG_SM0D0_REG); > - ret = mtk_i2c_master_cmd(i2c, SM0CTL1_WRITE, 2); > + ret = mtk_i2c_host_cmd(i2c, SM0CTL1_WRITE, 2); > if (ret) > goto err_timeout; > } else { > /* 7 bits address */ > addr = i2c_8bit_addr_from_msg(pmsg); > iowrite32(addr, i2c->base + REG_SM0D0_REG); > - ret = mtk_i2c_master_cmd(i2c, SM0CTL1_WRITE, 1); > + ret = mtk_i2c_host_cmd(i2c, SM0CTL1_WRITE, 1); > if (ret) > goto err_timeout; > } > @@ -202,7 +202,7 @@ static int mtk_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > cmd = SM0CTL1_WRITE; > } > > - ret = mtk_i2c_master_cmd(i2c, cmd, page_len); > + ret = mtk_i2c_host_cmd(i2c, cmd, page_len); > if (ret) > goto err_timeout; > > @@ -222,7 +222,7 @@ static int mtk_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > } > } > > - ret = mtk_i2c_master_stop(i2c); > + ret = mtk_i2c_host_stop(i2c); > if (ret) > goto err_timeout; > > @@ -230,7 +230,7 @@ static int mtk_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, > return i; > > err_ack: > - ret = mtk_i2c_master_stop(i2c); > + ret = mtk_i2c_host_stop(i2c); > if (ret) > goto err_timeout; > return -ENXIO; > @@ -247,7 +247,7 @@ static u32 mtk_i2c_func(struct i2c_adapter *a) > } > > static const struct i2c_algorithm mtk_i2c_algo = { > - .master_xfer = mtk_i2c_master_xfer, > + .xfer = mtk_i2c_xfer, > .functionality = mtk_i2c_func, > }; > Viele Grüße, Stefan Roese
On Fri, Mar 22, 2024 at 2:26 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Hi Wolfram, On Fri, Mar 22, 2024 at 02:24:55PM +0100, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
Hi Wolfram, > > - /* Mask all master interrupt bits */ > + /* Mask all interrupt bits */ "Mask all controller's interrupt bits" would have been redundant. Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
Hi Wolfram, > > @Andi: are you okay with this approach? It means you'd need to merge > > -rc2 into your for-next branch. Or rebase if all fails. > > I think it's a good plan, I'll try to support you with it. Do you feel more comfortable if I take the patches as soon as they are reviewd? So far I have tagged patch 1-4 and I can already merge 2,3,4 as long as you merge patch 1. Andi
Hi Wolfram, On Fri, Mar 22, 2024 at 02:24:59PM +0100, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). ... > -static int wait_master_done(struct i2c_au1550_data *adap) > +static int wait_host_done(struct i2c_au1550_data *adap) > { > int i; > > - /* Wait for Master Done. */ > + /* Wait for Host Done. */ here, rather than Master/Controller, the change is Master/Host, which is different from what is stated in the commit log. ... > @@ -246,7 +246,7 @@ static u32 au1550_func(struct i2c_adapter *adap) > } > > static const struct i2c_algorithm au1550_algo = { > - .master_xfer = au1550_xfer, > + .xfer = au1550_xfer, > .functionality = au1550_func, Here there was some alignment which now is gone. Andi
Hi Wolfram, On Fri, Mar 22, 2024 at 02:25:00PM +0100, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
On 3/22/2024 6:25 AM, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Hi Wolfram, > @@ -391,7 +391,7 @@ static u32 bcm2835_i2c_func(struct i2c_adapter *adap) > } > > static const struct i2c_algorithm bcm2835_i2c_algo = { > - .master_xfer = bcm2835_i2c_xfer, > + .xfer = bcm2835_i2c_xfer, Here you are breaking the alignment (even though I think a "tab" alignment is not needed). Andi
Hi Wolfram, > static const struct i2c_algorithm cdns_i2c_algo = { > - .master_xfer = cdns_i2c_master_xfer, > + .xfer = cdns_i2c_xfer, alignment :-) Andi
Hi Wolfram, On Fri, Mar 22, 2024 at 02:25:05PM +0100, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
Hi Wolfram, On Fri, Mar 22, 2024 at 02:25:10PM +0100, Wolfram Sang wrote: > Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C > specifications and replace "master/slave" with more appropriate terms. > They are also more specific because we distinguish now between a remote > entity ("client") and a local one ("target"). > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
> > - /* Wait for Master Done. */ > > + /* Wait for Host Done. */ > > here, rather than Master/Controller, the change is Master/Host, > which is different from what is stated in the commit log. Yes, it should be "controller" here. Preempting the following patches where I used host: I sometimes used it because "host" is shorter than "controller", so e.g. variable names would not grow too much in size. I missed that SMBus has a dedicated meaning for "host", clearly described in chapter 6.1.3 form version 3.0 onwards. So, I should switch to controller consistently. I am also working on a small series updating the I2C documentation so we have a defined terminology. This should also go in before this series and is hopefully ready tomorrow.
> > - unsigned long timeout; > > + unsigned long time_left; > > Thanks for doing this. Is the timeout v/s time_left language also due to the specification change? > A link to the specification(s) in the commit message would be nice to have I admit it is probably a seperate change... > > - if (!timeout) { > > - dev_err(i2c_dev->dev, "Write to slave 0x%x timed out\n", > > - c->addr); ... motivated by this "if (!timeout) dev_err("timeout!")" which is super confusing to read because the logic is paradox. > > + if (!time_left) > > ret = -ETIMEDOUT; > > - } > > Why did we lost the dev_err() here? Agreed. Another seperate change. A timeout is not something the user need to be aware of. It can regularly happen while an EEPROM is erasing a page. The client driver will probably handle it correctly by trying again. Only if the client driver sees a problem, then the user should be notified. But not in the controller driver.