Re: [PATCH 00/10] spi: Use FIELD_MODIFY() for bitfield operations

[Hans Zhang]

On 5/1/26 16:30, David Laight wrote:
> On Thu, 30 Apr 2026 23:54:46 +0800
> Hans Zhang <18255117159@xxxxxxx> wrote:
>
> > Replace open-coded bitfield modifications with the standard FIELD_MODIFY()
> > macro across multiple SPI controller drivers. This improves readability and
> > adds compile-time checking without functional changes.
>
> I don't think these changes are worth the effort.
> The readability doesn't change much - you need to know what a slightly
> more obscure 'helper' does.
> The extra compile-time checks are pretty unlikely to ever find a problem
> and mostly just slow down the compile.
> The generated code is likely be slightly worse.
> And, with the best will in the world, it is easy to make silly mistakes.
>
> 	David

Hi David,

FIELD_MODIFY() is a standard kernel helper (bitfield.h), not an obscure
one. My recent power domain series using similar patterns was accepted:

https://patchwork.kernel.org/project/linux-pm/cover/20260430163213.44695-1-18255117159@xxxxxxx/

The PCIe maintainer also values this kind of code simplification, which
encouraged me to send these SPI patches.

The macro offers compile-time overflow checks, and I've verified the
generated assembly is identical (GCC/Clang). I believe the trade‑off
favours readability and safety.

If you still prefer to keep the open‑coded versions, I'll drop the
series. Please let me know.


Best regards,
Hans

> > Each patch modifies a single driver, allowing independent review and
> > application.
> >
> > Hans Zhang (10):
> >    spi: amlogic-spifc-a1: Use FIELD_MODIFY()
> >    spi: amlogic-spisg: Use FIELD_MODIFY()
> >    spi: cadence-xspi: Use FIELD_MODIFY()
> >    spi: meson-spicc: Use FIELD_MODIFY()
> >    spi: nxp-xspi: Use FIELD_MODIFY()
> >    spi: sn-f-ospi: Use FIELD_MODIFY()
> >    spi: stm32-ospi: Use FIELD_MODIFY()
> >    spi: stm32-qspi: Use FIELD_MODIFY()
> >    spi: sunplus-sp7021: Use FIELD_MODIFY()
> >    spi: uniphier: Use FIELD_MODIFY()
> >
> >   drivers/spi/spi-amlogic-spifc-a1.c |  5 ++---
> >   drivers/spi/spi-amlogic-spisg.c    | 13 +++++--------
> >   drivers/spi/spi-cadence-xspi.c     |  3 +--
> >   drivers/spi/spi-meson-spicc.c      |  5 ++---
> >   drivers/spi/spi-nxp-xspi.c         | 12 ++++--------
> >   drivers/spi/spi-sn-f-ospi.c        |  5 ++---
> >   drivers/spi/spi-stm32-ospi.c       |  7 +++----
> >   drivers/spi/spi-stm32-qspi.c       |  5 ++---
> >   drivers/spi/spi-sunplus-sp7021.c   |  3 +--
> >   drivers/spi/spi-uniphier.c         | 13 +++++--------
> >   10 files changed, 27 insertions(+), 44 deletions(-)
> >
> >
> > base-commit: 3b3bea6d4b9c162f9e555905d96b8c1da67ecd5b