Linux-Renesas-SoC Archive on lore.kernel.org
 help / color / Atom feed
From: Daniel Stodden <daniel.stodden@gmail.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-i2c@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH i2c-tools] i2ctransfer: add support for I2C_M_RECV_LEN
Date: Sun, 2 Aug 2020 13:28:29 -0700
Message-ID: <C5EC2F45-41AD-465E-83F9-BDE3640B02AA@gmail.com> (raw)
In-Reply-To: <20200802192842.13527-1-wsa+renesas@sang-engineering.com>



> On Aug 2, 2020, at 12:28 PM, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> 
> Strictly spoken, this is for emulating SMBus transfers, not I2C. But
> hey, we still want to test devices supporting these transfers.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> I created this to be able to play around with I2C_M_RECV_LEN and with
> SMBUS2 and SMBUS3 max block sizes. Tested with a Renesas Lager board and
> my slave-testunit hacked with SMBus block transfer support (to be
> upstreamed later).
> 
> Fun fact: printing the correct length in the output took longer than
> implementing the actual functionality.
> 
> tools/i2ctransfer.8 |  1 +
> tools/i2ctransfer.c | 42 +++++++++++++++++++++++++++++++-----------
> 2 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/i2ctransfer.8 b/tools/i2ctransfer.8
> index d16e34e..152d20d 100644
> --- a/tools/i2ctransfer.8
> +++ b/tools/i2ctransfer.8
> @@ -91,6 +91,7 @@ specifies if the message is read or write
> .B <length_of_message>
> specifies the number of bytes read or written in this message.
> It is parsed as an unsigned 16 bit integer, but note that the Linux Kernel applies an additional upper limit (8192 as of v4.10).
> +For read messages to targets which support SMBus Block transactions, it can also be '?', then the target will determine the length.
> .TP
> .B [@address]
> specifies the 7-bit address of the chip to be accessed for this message, and is an integer.
> diff --git a/tools/i2ctransfer.c b/tools/i2ctransfer.c
> index 7b95d48..2b8a3fb 100644
> --- a/tools/i2ctransfer.c
> +++ b/tools/i2ctransfer.c
> @@ -45,7 +45,8 @@ static void help(void)
> 		"Usage: i2ctransfer [-f] [-y] [-v] [-V] [-a] I2CBUS DESC [DATA] [DESC [DATA]]...\n"
> 		"  I2CBUS is an integer or an I2C bus name\n"
> 		"  DESC describes the transfer in the form: {r|w}LENGTH[@address]\n"
> -		"    1) read/write-flag 2) LENGTH (range 0-65535) 3) I2C address (use last one if omitted)\n"
> +		"    1) read/write-flag 2) LENGTH (range 0-65535, or '?')\n"
> +		"    3) I2C address (use last one if omitted)\n"
> 		"  DATA are LENGTH bytes for a write message. They can be shortened by a suffix:\n"
> 		"    = (keep value constant until LENGTH)\n"
> 		"    + (increase value by 1 until LENGTH)\n"
> @@ -84,17 +85,24 @@ static void print_msgs(struct i2c_msg *msgs, __u32 nmsgs, unsigned flags)
> 
> 	for (i = 0; i < nmsgs; i++) {
> 		int read = msgs[i].flags & I2C_M_RD;
> +		int recv_len = msgs[i].flags & I2C_M_RECV_LEN;
> 		int print_buf = (read && (flags & PRINT_READ_BUF)) ||
> 				(!read && (flags & PRINT_WRITE_BUF));
> -
> -		if (flags & PRINT_HEADER)
> -			fprintf(output, "msg %u: addr 0x%02x, %s, len %u",
> -				i, msgs[i].addr, read ? "read" : "write", msgs[i].len);
> +		__u16 len = recv_len ? msgs[i].buf[0] + 1 : msgs[i].len;

This is post-ioctl()? (Sorry, still not familiar enough with i2c-tools.)

It isn’t wrong, but shouldn’t be necessary.
Unless the adapter driver you’re using went astray. Not ruling that out.

Before I2C_RDWR:
	- msg.len is sizeof(msg.buf)
	- msg.block[0] is <extra_bytes>

After I2C_RDWR:
	- msg.block[0] is <len> of transfer
	- msg.len is <len> + <extra_bytes> (but never > sizeof(msg.buf))

Hth,
Daniel

> +
> +		if (flags & PRINT_HEADER) {
> +			fprintf(output, "msg %u: addr 0x%02x, %s, len ",
> +				i, msgs[i].addr, read ? "read" : "write");
> +			if (!recv_len || flags & PRINT_READ_BUF)
> +				fprintf(output, "%u", len);
> +			else
> +				fprintf(output, "TBD");
> +		}
> 
> 		if (msgs[i].len && print_buf) {
> 			if (flags & PRINT_HEADER)
> 				fprintf(output, ", buf ");
> -			for (j = 0; j < msgs[i].len - 1; j++)
> +			for (j = 0; j < len - 1; j++)
> 				fprintf(output, "0x%02x ", msgs[i].buf[j]);
> 			/* Print final byte with newline */
> 			fprintf(output, "0x%02x\n", msgs[i].buf[j]);
> @@ -192,13 +200,23 @@ int main(int argc, char *argv[])
> 				goto err_out_with_arg;
> 			}
> 
> -			len = strtoul(arg_ptr, &end, 0);
> -			if (len > 0xffff || arg_ptr == end) {
> -				fprintf(stderr, "Error: Length invalid\n");
> -				goto err_out_with_arg;
> +			if (*arg_ptr == '?') {
> +				if (!(flags & I2C_M_RD)) {
> +					fprintf(stderr, "Error: variable length not allowed with write\n");
> +					goto err_out_with_arg;
> +				}
> +				len = 256; /* SMBUS3_MAX_BLOCK_LEN + 1 */
> +				flags |= I2C_M_RECV_LEN;
> +				arg_ptr++;
> +			} else {
> +				len = strtoul(arg_ptr, &end, 0);
> +				if (len > 0xffff || arg_ptr == end) {
> +					fprintf(stderr, "Error: Length invalid\n");
> +					goto err_out_with_arg;
> +				}
> +				arg_ptr = end;
> 			}
> 
> -			arg_ptr = end;
> 			if (*arg_ptr) {
> 				if (*arg_ptr++ != '@') {
> 					fprintf(stderr, "Error: Unknown separator after length\n");
> @@ -237,6 +255,8 @@ int main(int argc, char *argv[])
> 				}
> 				memset(buf, 0, len);
> 				msgs[nmsgs].buf = buf;
> +				if (flags & I2C_M_RECV_LEN)
> +					buf[0] = 1; /* number of extra bytes */
> 			}
> 
> 			if (flags & I2C_M_RD || len == 0) {
> -- 
> 2.27.0
> 


  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-02 19:28 Wolfram Sang
2020-08-02 20:28 ` Daniel Stodden [this message]
2020-08-02 21:38   ` Wolfram Sang
2020-08-03  7:30     ` Daniel Stodden
2020-08-03  8:38       ` Wolfram Sang
2020-08-03 20:25         ` Daniel Stodden
2020-08-03 20:45           ` Wolfram Sang
2020-08-03 19:14 ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C5EC2F45-41AD-465E-83F9-BDE3640B02AA@gmail.com \
    --to=daniel.stodden@gmail.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org
	public-inbox-index linux-renesas-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git