public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tabrez Ahmed <tabreztalks@gmail.com>
To: David Laight <david.laight.linux@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>,
	linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
	shuah@kernel.org, me@brighamcampbell.com,
	Sashiko <sashiko-bot@kernel.org>
Subject: Re: [PATCH v5 1/3] hwmon: (ads7871) Fix endianness bug in 16-bit register reads
Date: Fri, 1 May 2026 20:02:15 +0530	[thread overview]
Message-ID: <2b3b7c42-95ed-4195-83ac-63e6fe171e6c@gmail.com> (raw)
In-Reply-To: <20260501094902.05ce6d37@pumpkin>

On 5/1/26 14:19, David Laight wrote:

> Isn't it enough to just byteswap the result? so:
> 	return le16toh(ret);
> The whole thing can be:
> 	return le16toh(spi_w8r16(spi, reg | INST_READ_BM | INST_16BIT_BM));
> (although I suspect sparse bleats and needs an annoying (__force __le16) cast)

Hi David,

Good point, spi_w8r16() is definitely cleaner and cuts out the buffer 
allocations entirely.

I tried the one-liner approach, but le16toh caused a build error because 
it's not available in the kernel headers. I will swap it for le16_to_cpu 
in the final patch.

I'm going to split the implementation slightly in v6 to catch negative 
error codes before the conversion and to match the bitwise assignment 
style used in the rest of this driver:


static int ads7871_read_reg16(struct spi_device *spi, int reg)
{
	int ret;

	reg = reg | INST_READ_BM | INST_16BIT_BM;
	ret = spi_w8r16(spi, reg);
	if (ret < 0)
		return ret;

	return le16_to_cpu((__force __le16)ret);
}


Guenter this also pulls in your suggestion to combine the bitwise flags.

I'll get v6 out shortly.

Thanks,
Tabrez

  reply	other threads:[~2026-05-01 14:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-01  2:35 [PATCH v5 0/3] hwmon: (ads7871) Fix endianness and modernize driver Tabrez Ahmed
2026-05-01  2:35 ` [PATCH v5 1/3] hwmon: (ads7871) Fix endianness bug in 16-bit register reads Tabrez Ahmed
2026-05-01  5:45   ` Guenter Roeck
2026-05-01  8:49   ` David Laight
2026-05-01 14:32     ` Tabrez Ahmed [this message]
2026-05-01 16:28     ` Guenter Roeck
2026-05-02 18:16   ` Guenter Roeck
2026-05-01  2:35 ` [PATCH v5 2/3] hwmon: (ads7871) Convert to hwmon_device_register_with_info Tabrez Ahmed
2026-05-01  2:35 ` [PATCH v5 3/3] hwmon: (ads7871) Use DMA-safe buffer for SPI writes Tabrez Ahmed

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=2b3b7c42-95ed-4195-83ac-63e6fe171e6c@gmail.com \
    --to=tabreztalks@gmail.com \
    --cc=david.laight.linux@gmail.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=me@brighamcampbell.com \
    --cc=sashiko-bot@kernel.org \
    --cc=shuah@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox