[PATCH linux dev-5.7 6/6] eeprom: at25: Split reads into chunks and cap write size

[Milton Miller II]

On July 20,2020 Eddie James wrote:
>From: Brad Bishop <bradleyb at fuzziesquirrel.com>
>
>Make use of spi_max_transfer_size to avoid requesting transfers that
>are
>too large for some spi controllers.
>
>Signed-off-by: Eddie James <eajames at linux.ibm.com>
>Signed-off-by: Brad Bishop <bradleyb at fuzziesquirrel.com>

When forwarding patches from others your SOB goes after theirs.


>---> drivers/misc/eeprom/at25.c | 94
>++++++++++++++++++++++----------------
> 1 file changed, 54 insertions(+), 40 deletions(-)
>
>diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
>index cde9a2fc1325..3ed041cb3083 100644
>--- a/drivers/misc/eeprom/at25.c
>+++ b/drivers/misc/eeprom/at25.c
>@@ -64,12 +64,17 @@ static int at25_ee_read(void *priv, unsigned int
>offset,
> {
> 	struct at25_data *at25 = priv;
> 	char *buf = val;
>+	size_t max_chunk = spi_max_transfer_size(at25->spi);
>+	size_t num_msgs = count / max_chunk + (bool)(count % max_chunk);

Casting to bool to use result as 1 or 0 ?  Obsfucated.

Use DIV_ROUND_UP instead.

>+	size_t			nr_bytes = 0;> 	u8			command[EE_MAXADDRLEN + 1];
> 	u8			*cp;
> 	ssize_t			status;
> 	struct spi_transfer	t[2];
> 	struct spi_message	m;
> 	u8			instr;
>+	unsigned		msg_offset;

I expect checkpatch to complain
# check for declarations of signed or unsigned without int
	

>+	size_t			msg_count;> 
> 	if (unlikely(offset >= at25->chip.byte_len))
> 		return -EINVAL;
>@@ -78,57 +83,64 @@ static int at25_ee_read(void *priv, unsigned int
>offset,
> 	if (unlikely(!count))
> 		return -EINVAL;
> 
>-	cp = command;
>-
>-	instr = AT25_READ;
>-	if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
>-		if (offset >= (1U << (at25->addrlen * 8)))
>-			instr |= AT25_INSTR_BIT3;
>-	*cp++ = instr;
>-
>-	/* 8/16/24-bit address is written MSB first */
>-	switch (at25->addrlen) {
>-	default:	/* case 3 */
>-		*cp++ = offset >> 16;
>-		/* fall through */
>-	case 2:
>-		*cp++ = offset >> 8;
>-		/* fall through */
>-	case 1:
>-	case 0:	/* can't happen: for better codegen */
>-		*cp++ = offset >> 0;
>-	}
>+	msg_offset = (unsigned) offset;
>+	msg_count = min(count, max_chunk);
>+	while (num_msgs) {
>+		cp = command;
> 
>-	spi_message_init(&m);
>-	memset(t, 0, sizeof(t));
>+		instr = AT25_READ;
>+		if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
>+			if (msg_offset >= (1U << (at25->addrlen * 8)))
>+				instr |= AT25_INSTR_BIT3;
>+		*cp++ = instr;
> 
>-	t[0].tx_buf = command;
>-	t[0].len = at25->addrlen + 1;
>-	spi_message_add_tail(&t[0], &m);
>+		/* 8/16/24-bit address is written MSB first */
>+		switch (at25->addrlen) {
>+			default:	/* case 3 */
>+				*cp++ = msg_offset >> 16;
>+				/* fall through */
>+			case 2:
>+				*cp++ = msg_offset >> 8;
>+				/* fall through */
>+			case 1:
>+			case 0:	/* can't happen: for better codegen */
>+				*cp++ = msg_offset >> 0;
>+		}
> 
>-	t[1].rx_buf = buf;
>-	t[1].len = count;
>-	spi_message_add_tail(&t[1], &m);
>+		spi_message_init(&m);
>+		memset(t, 0, sizeof(t));
> 
>-	mutex_lock(&at25->lock);
>+		t[0].tx_buf = command;
>+		t[0].len = at25->addrlen + 1;
>+		spi_message_add_tail(&t[0], &m);
> 
>-	/* Read it all at once.
>-	 *
>-	 * REVISIT that's potentially a problem with large chips, if
>-	 * other devices on the bus need to be accessed regularly or
>-	 * this chip is clocked very slowly
>-	 */
>-	status = spi_sync(at25->spi, &m);
>-	dev_dbg(&at25->spi->dev, "read %zu bytes at %d --> %zd\n",
>-		count, offset, status);
>+		t[1].rx_buf = buf + nr_bytes;
>+		t[1].len = msg_count;
>+		spi_message_add_tail(&t[1], &m);
> 
>-	mutex_unlock(&at25->lock);
>-	return status;
>+		mutex_lock(&at25->lock);
>+
>+		status = spi_sync(at25->spi, &m);
>+
>+		mutex_unlock(&at25->lock);
>+
>+		if (status)
>+			return status;
>+
>+		--num_msgs;
>+		msg_offset += msg_count;
>+		nr_bytes += msg_count;
>+	}
>+
>+	dev_dbg(&at25->spi->dev, "read %zu bytes at %d\n",
>+				count, offset);
>+	return 0;
> }
> 
> static int at25_ee_write(void *priv, unsigned int off, void *val,
>size_t count)
> {
> 	struct at25_data *at25 = priv;
>+	size_t maxsz = spi_max_transfer_size(at25->spi);
> 	const char *buf = val;
> 	int			status = 0;
> 	unsigned		buf_size;
>@@ -191,6 +203,8 @@ static int at25_ee_write(void *priv, unsigned int
>off, void *val, size_t count)
> 		segment = buf_size - (offset % buf_size);
> 		if (segment > count)
> 			segment = count;
>+		if (segment > maxsz)
>+			segment = maxsz;
> 		memcpy(cp, buf, segment);
> 		status = spi_write(at25->spi, bounce,
> 				segment + at25->addrlen + 1);
>-- 
>2.24.0
>
>

Regards,
milton