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
next prev parent 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