From: Tudor Ambarus <tudor.ambarus@linaro.org>
To: Sam Protsenko <semen.protsenko@linaro.org>
Cc: broonie@kernel.org, andi.shyti@kernel.org,
krzysztof.kozlowski@linaro.org, alim.akhtar@samsung.com,
jassi.brar@samsung.com, linux-spi@vger.kernel.org,
linux-samsung-soc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, andre.draszik@linaro.org,
peter.griffin@linaro.org, kernel-team@android.com,
willmcvicker@google.com
Subject: Re: [PATCH v3 17/17] spi: s3c64xx: use bitfield access macros
Date: Sat, 27 Jan 2024 03:44:24 +0000 [thread overview]
Message-ID: <facbcbf3-7dba-43a1-b4fe-ac77b5bef545@linaro.org> (raw)
In-Reply-To: <CAPLW+4nN--gG9XsOu6a-mo5vcM-GycRrhPQFOtNidfVTM=KonQ@mail.gmail.com>
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-01-27 3:44 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-26 17:15 [PATCH v3 00/17] spi: s3c64xx: straightforward cleanup Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 01/17] spi: s3c64xx: explicitly include <linux/io.h> Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 02/17] spi: s3c64xx: explicitly include <linux/bits.h> Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 03/17] spi: s3c64xx: avoid possible negative array index Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 04/17] spi: s3c64xx: fix typo, s/configuartion/configuration Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 05/17] spi: s3c64xx: sort headers alphabetically Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 06/17] spi: s3c64xx: remove unneeded (void *) casts in of_match_table Tudor Ambarus
2024-01-26 20:17 ` Sam Protsenko
2024-01-27 3:41 ` Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 07/17] spi: s3c64xx: remove else after return Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 08/17] spi: s3c64xx: move common code outside if else Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 09/17] spi: s3c64xx: check return code of dmaengine_slave_config() Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 10/17] spi: s3c64xx: propagate the dma_submit_error() error code Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 11/17] spi: s3c64xx: rename prepare_dma() to s3c64xx_prepare_dma() Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 12/17] spi: s3c64xx: return ETIMEDOUT for wait_for_completion_timeout() Tudor Ambarus
2024-01-26 20:13 ` Sam Protsenko
2024-01-27 3:26 ` Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 13/17] spi: s3c64xx: drop blank line between declarations Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 14/17] spi: s3c64xx: downgrade dev_warn to dev_dbg for optional dt props Tudor Ambarus
2024-01-26 17:15 ` [PATCH v3 15/17] spi: s3c64xx: remove duplicated definition Tudor Ambarus
2024-01-26 20:06 ` Sam Protsenko
2024-01-26 17:15 ` [PATCH v3 16/17] spi: s3c64xx: drop a superfluous bitwise NOT operation Tudor Ambarus
2024-01-26 20:09 ` Sam Protsenko
2024-01-26 17:15 ` [PATCH v3 17/17] spi: s3c64xx: use bitfield access macros Tudor Ambarus
2024-01-26 20:12 ` Sam Protsenko
2024-01-27 3:23 ` Tudor Ambarus
2024-01-27 3:38 ` Sam Protsenko
2024-01-27 3:44 ` Tudor Ambarus [this message]
2024-01-29 16:42 ` Mark Brown
2024-01-29 17:59 ` Tudor Ambarus
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=facbcbf3-7dba-43a1-b4fe-ac77b5bef545@linaro.org \
--to=tudor.ambarus@linaro.org \
--cc=alim.akhtar@samsung.com \
--cc=andi.shyti@kernel.org \
--cc=andre.draszik@linaro.org \
--cc=broonie@kernel.org \
--cc=jassi.brar@samsung.com \
--cc=kernel-team@android.com \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=peter.griffin@linaro.org \
--cc=semen.protsenko@linaro.org \
--cc=willmcvicker@google.com \
/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;
as well as URLs for NNTP newsgroup(s).