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

On Fri, Jan 26, 2024 at 9:23 PM Tudor Ambarus [off-list ref] wrote:

Hi, Sam,

Thanks for the review feedback!

On 1/26/24 20:12, Sam Protsenko wrote:

quoted

quoted

-               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.

quoted

Also I think new patch series are being submitted a bit too fast,
people might not have enough time to provide the review.

This patch set contains patches that are already reviewed or too simple
to being lagged.

Ok, agreed.

Cheers,
ta

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel