Re: [PATCH v3 17/17] spi: s3c64xx: use bitfield access macros

From: Tudor Ambarus
Date: Fri Jan 26 2024 - 22:44:38 EST




On 1/27/24 03:38, Sam Protsenko wrote:
>>>> - val |= S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD;
>>>> - val |= S3C64XX_SPI_MODE_CH_TSZ_HALFWORD;
>>>> + val |= FIELD_PREP(S3C64XX_SPI_MODE_BUS_TSZ_MASK,
>>>> + S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD) |
>>>> + FIELD_PREP(S3C64XX_SPI_MODE_CH_TSZ_MASK,
>>>> + S3C64XX_SPI_MODE_CH_TSZ_HALFWORD);
>>> Two people complained it makes the code harder to read. Yet it's not
>>> addressed in v3. Please see my comments for your previous submission
>>> explaining what can be done, and also Andi's comment on that matter.
>> I kept these intentionally. Please read my reply on that matter or the
>> cover letter to this patch set.
>>
> I read it. But still don't like it 🙂 I'm sure it's possible to do
> this modification, but at the same time keep the code clean an easy to
> read. The code above -- I don't like at all, sorry. It was much better
> before this patch, IMHO.

Yeah, I guess Mark will tip the scale.

Cheers,
ta