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

[Hans Zhang <18255117159@163.com>]

On 5/1/26 16:30, David Laight wrote:
> On Thu, 30 Apr 2026 23:54:46 +0800
> Hans Zhang <18255117159@163.com> 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@163.com/

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