mbox series

[00/64] i2c: reword i2c_algorithm according to newest specification

Message ID 20240322132619.6389-1-wsa+renesas@sang-engineering.com
Headers show
Series i2c: reword i2c_algorithm according to newest specification | expand

Message

Wolfram Sang March 22, 2024, 1:24 p.m. UTC
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.

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.

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.

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. Also, Renesas deserves a honorable mention here for
increased support of my I2C activities. Thank you!

If you have comments, hints, etc, please let me know.

Happy hacking,

   Wolfram


Wolfram Sang (64):
  i2c: reword i2c_algorithm according to newest specification
  i2c: ali15x3: reword according to newest specification
  i2c: altera: reword according to newest specification
  i2c: amd-mp2-pci: reword according to newest specification
  i2c: aspeed: reword according to newest specification
  i2c: au1550: reword according to newest specification
  i2c: bcm-iproc: reword according to newest specification
  i2c: bcm-kona: reword according to newest specification
  i2c: bcm2835: reword according to newest specification
  i2c: brcmstb: reword according to newest specification
  i2c: cadence: reword according to newest specification
  i2c: cht-wc: reword according to newest specification
  i2c: cp2615: reword according to newest specification
  i2c: cpm: reword according to newest specification
  i2c: davinci: reword according to newest specification
  i2c: digicolor: reword according to newest specification
  i2c: dln2: reword according to newest specification
  i2c: eg20t: reword according to newest specification
  i2c: emev2: reword according to newest specification
  i2c: fsi: reword according to newest specification
  i2c: gpio: reword according to newest specification
  i2c: highlander: reword according to newest specification
  i2c: hix5hd2: reword according to newest specification
  i2c: i801: reword according to newest specification
  i2c: ibm_iic: reword according to newest specification
  i2c: imx-lpi2c: reword according to newest specification
  i2c: iop3xx: reword according to newest specification
  i2c: isch: reword according to newest specification
  i2c: ismt: reword according to newest specification
  i2c: ljca: reword according to newest specification
  i2c: lpc2k: reword according to newest specification
  i2c: ls2x: reword according to newest specification
  i2c: mchp-pci1xxxx: reword according to newest specification
  i2c: microchip-corei2c: reword according to newest specification
  i2c: mlxcpld: reword according to newest specification
  i2c: mpc: reword according to newest specification
  i2c: mt7621: reword according to newest specification
  i2c: mv64xxx: reword according to newest specification
  i2c: octeon-core: reword according to newest specification
  i2c: owl: reword according to newest specification
  i2c: piix4: reword according to newest specification
  i2c: powermac: reword according to newest specification
  i2c: pxa-pci: reword according to newest specification
  i2c: qup: reword according to newest specification
  i2c: rcar: reword according to newest specification
  i2c: riic: reword according to newest specification
  i2c: rk3x: reword according to newest specification
  i2c: sh7760: reword according to newest specification
  i2c: sh_mobile: reword according to newest specification
  i2c: sis5595: reword according to newest specification
  i2c: sis630: reword according to newest specification
  i2c: sprd: reword according to newest specification
  i2c: st: reword according to newest specification
  i2c: stm32f4: reword according to newest specification
  i2c: sun6i-p2wi: reword according to newest specification
  i2c: synquacer: reword according to newest specification
  i2c: taos-evm: reword according to newest specification
  i2c: tiny-usb: reword according to newest specification
  i2c: uniphier-f: reword according to newest specification
  i2c: uniphier: reword according to newest specification
  i2c: viperboard: reword according to newest specification
  i2c: xlp9xx: reword according to newest specification
  i2c: scx200_acb: reword according to newest specification
  i2c: reword i2c_algorithm in drivers according to newest specification

 drivers/i2c/busses/i2c-ali15x3.c           |  2 +-
 drivers/i2c/busses/i2c-altera.c            |  4 +-
 drivers/i2c/busses/i2c-amd-mp2-pci.c       |  8 ++--
 drivers/i2c/busses/i2c-amd-mp2-plat.c      |  2 +-
 drivers/i2c/busses/i2c-aspeed.c            | 26 +++++-----
 drivers/i2c/busses/i2c-at91-master.c       |  2 +-
 drivers/i2c/busses/i2c-at91-slave.c        |  8 ++--
 drivers/i2c/busses/i2c-au1550.c            | 14 +++---
 drivers/i2c/busses/i2c-axxia.c             | 10 ++--
 drivers/i2c/busses/i2c-bcm-iproc.c         | 20 ++++----
 drivers/i2c/busses/i2c-bcm-kona.c          | 14 +++---
 drivers/i2c/busses/i2c-bcm2835.c           |  8 ++--
 drivers/i2c/busses/i2c-brcmstb.c           | 12 ++---
 drivers/i2c/busses/i2c-cadence.c           | 14 +++---
 drivers/i2c/busses/i2c-cht-wc.c            |  8 ++--
 drivers/i2c/busses/i2c-cp2615.c            |  6 +--
 drivers/i2c/busses/i2c-cpm.c               |  4 +-
 drivers/i2c/busses/i2c-cros-ec-tunnel.c    |  2 +-
 drivers/i2c/busses/i2c-davinci.c           | 13 +++--
 drivers/i2c/busses/i2c-designware-master.c |  2 +-
 drivers/i2c/busses/i2c-designware-slave.c  |  8 ++--
 drivers/i2c/busses/i2c-digicolor.c         |  4 +-
 drivers/i2c/busses/i2c-diolan-u2c.c        |  2 +-
 drivers/i2c/busses/i2c-dln2.c              |  4 +-
 drivers/i2c/busses/i2c-eg20t.c             | 10 ++--
 drivers/i2c/busses/i2c-emev2.c             | 10 ++--
 drivers/i2c/busses/i2c-exynos5.c           |  4 +-
 drivers/i2c/busses/i2c-fsi.c               | 56 +++++++++++-----------
 drivers/i2c/busses/i2c-gpio.c              |  8 ++--
 drivers/i2c/busses/i2c-gxp.c               | 12 ++---
 drivers/i2c/busses/i2c-highlander.c        |  2 +-
 drivers/i2c/busses/i2c-hisi.c              |  4 +-
 drivers/i2c/busses/i2c-hix5hd2.c           |  4 +-
 drivers/i2c/busses/i2c-i801.c              | 12 ++---
 drivers/i2c/busses/i2c-ibm_iic.c           | 26 +++++-----
 drivers/i2c/busses/i2c-img-scb.c           |  2 +-
 drivers/i2c/busses/i2c-imx-lpi2c.c         | 10 ++--
 drivers/i2c/busses/i2c-imx.c               | 12 ++---
 drivers/i2c/busses/i2c-iop3xx.c            | 10 ++--
 drivers/i2c/busses/i2c-isch.c              |  2 +-
 drivers/i2c/busses/i2c-ismt.c              |  2 +-
 drivers/i2c/busses/i2c-jz4780.c            |  2 +-
 drivers/i2c/busses/i2c-kempld.c            |  2 +-
 drivers/i2c/busses/i2c-ljca.c              | 20 ++++----
 drivers/i2c/busses/i2c-lpc2k.c             |  8 ++--
 drivers/i2c/busses/i2c-ls2x.c              |  8 ++--
 drivers/i2c/busses/i2c-mchp-pci1xxxx.c     | 40 ++++++++--------
 drivers/i2c/busses/i2c-meson.c             |  4 +-
 drivers/i2c/busses/i2c-microchip-corei2c.c |  4 +-
 drivers/i2c/busses/i2c-mlxbf.c             |  8 ++--
 drivers/i2c/busses/i2c-mlxcpld.c           | 12 ++---
 drivers/i2c/busses/i2c-mpc.c               |  4 +-
 drivers/i2c/busses/i2c-mt65xx.c            |  2 +-
 drivers/i2c/busses/i2c-mt7621.c            | 22 ++++-----
 drivers/i2c/busses/i2c-mv64xxx.c           | 12 ++---
 drivers/i2c/busses/i2c-mxs.c               |  2 +-
 drivers/i2c/busses/i2c-nomadik.c           |  2 +-
 drivers/i2c/busses/i2c-npcm7xx.c           | 12 ++---
 drivers/i2c/busses/i2c-nvidia-gpu.c        |  4 +-
 drivers/i2c/busses/i2c-ocores.c            |  8 ++--
 drivers/i2c/busses/i2c-octeon-core.c       |  6 +--
 drivers/i2c/busses/i2c-octeon-platdrv.c    |  2 +-
 drivers/i2c/busses/i2c-omap.c              |  4 +-
 drivers/i2c/busses/i2c-opal.c              |  4 +-
 drivers/i2c/busses/i2c-owl.c               | 10 ++--
 drivers/i2c/busses/i2c-pasemi-core.c       |  2 +-
 drivers/i2c/busses/i2c-piix4.c             |  2 +-
 drivers/i2c/busses/i2c-pnx.c               |  2 +-
 drivers/i2c/busses/i2c-powermac.c          |  8 ++--
 drivers/i2c/busses/i2c-pxa-pci.c           |  2 +-
 drivers/i2c/busses/i2c-pxa.c               | 12 ++---
 drivers/i2c/busses/i2c-qcom-cci.c          |  2 +-
 drivers/i2c/busses/i2c-qcom-geni.c         |  2 +-
 drivers/i2c/busses/i2c-qup.c               |  6 +--
 drivers/i2c/busses/i2c-rcar.c              | 16 +++----
 drivers/i2c/busses/i2c-riic.c              |  6 +--
 drivers/i2c/busses/i2c-rk3x.c              | 18 +++----
 drivers/i2c/busses/i2c-robotfuzz-osif.c    |  2 +-
 drivers/i2c/busses/i2c-rzv2m.c             |  8 ++--
 drivers/i2c/busses/i2c-s3c2410.c           |  4 +-
 drivers/i2c/busses/i2c-sh7760.c            | 18 +++----
 drivers/i2c/busses/i2c-sh_mobile.c         | 12 ++---
 drivers/i2c/busses/i2c-sis5595.c           |  2 +-
 drivers/i2c/busses/i2c-sis630.c            | 16 +++----
 drivers/i2c/busses/i2c-sprd.c              | 14 +++---
 drivers/i2c/busses/i2c-st.c                | 17 +++----
 drivers/i2c/busses/i2c-stm32f4.c           |  8 ++--
 drivers/i2c/busses/i2c-stm32f7.c           | 14 +++---
 drivers/i2c/busses/i2c-sun6i-p2wi.c        | 20 ++++----
 drivers/i2c/busses/i2c-synquacer.c         | 30 ++++++------
 drivers/i2c/busses/i2c-taos-evm.c          |  2 +-
 drivers/i2c/busses/i2c-tegra-bpmp.c        |  4 +-
 drivers/i2c/busses/i2c-tegra.c             |  4 +-
 drivers/i2c/busses/i2c-thunderx-pcidrv.c   |  2 +-
 drivers/i2c/busses/i2c-tiny-usb.c          |  4 +-
 drivers/i2c/busses/i2c-uniphier-f.c        | 22 ++++-----
 drivers/i2c/busses/i2c-uniphier.c          | 12 ++---
 drivers/i2c/busses/i2c-viperboard.c        |  8 ++--
 drivers/i2c/busses/i2c-virtio.c            |  2 +-
 drivers/i2c/busses/i2c-wmt.c               |  2 +-
 drivers/i2c/busses/i2c-xiic.c              |  2 +-
 drivers/i2c/busses/i2c-xlp9xx.c            |  4 +-
 drivers/i2c/busses/scx200_acb.c            |  4 +-
 include/linux/i2c.h                        | 24 ++++++++--
 104 files changed, 460 insertions(+), 464 deletions(-)

Comments

Heiko Stuebner March 22, 2024, 1:51 p.m. UTC | #1
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,
>  };
>  
>
Bjorn Andersson March 22, 2024, 2:35 p.m. UTC | #2
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
>
Easwar Hariharan March 22, 2024, 5:11 p.m. UTC | #3
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,
>  };
>
Vadim Pasternak March 22, 2024, 10:28 p.m. UTC | #4
> -----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
Andi Shyti March 23, 2024, 9:20 a.m. UTC | #5
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
AngeloGioacchino Del Regno March 25, 2024, 8:06 a.m. UTC | #6
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>
Stefan Roese March 25, 2024, 8:07 a.m. UTC | #7
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
Bartosz Golaszewski March 25, 2024, 3:07 p.m. UTC | #8
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>
Andi Shyti March 25, 2024, 11:46 p.m. UTC | #9
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
Andi Shyti March 26, 2024, 12:12 a.m. UTC | #10
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
Andi Shyti March 26, 2024, 12:36 a.m. UTC | #11
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
Andi Shyti March 26, 2024, 7:27 a.m. UTC | #12
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
Andi Shyti March 26, 2024, 7:29 a.m. UTC | #13
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
Florian Fainelli March 26, 2024, 12:44 p.m. UTC | #14
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>
Andi Shyti March 26, 2024, 7:03 p.m. UTC | #15
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
Andi Shyti March 26, 2024, 7:06 p.m. UTC | #16
Hi Wolfram,

>  static const struct i2c_algorithm cdns_i2c_algo = {
> -	.master_xfer	= cdns_i2c_master_xfer,
> +	.xfer	= cdns_i2c_xfer,

alignment :-)

Andi
Andi Shyti March 26, 2024, 7:06 p.m. UTC | #17
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
Andi Shyti March 26, 2024, 7:16 p.m. UTC | #18
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
Wolfram Sang April 8, 2024, 8:49 a.m. UTC | #19
> > -	/* 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.
Wolfram Sang April 8, 2024, 9:14 a.m. UTC | #20
> > -	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.