From: Andre Przywara <andre.przywara@arm.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
Chen-Yu Tsai <wens@csie.org>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Samuel Holland <samuel@sholland.org>,
linux-arm-kernel@lists.infradead.org,
linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/8] spi: sun4i: use 'time_left' variable with wait_for_completion_timeout()
Date: Mon, 29 Apr 2024 13:14:38 +0100 [thread overview]
Message-ID: <20240429131438.1f036341@donnerap.manchester.arm.com> (raw)
In-Reply-To: <20240429112843.67628-7-wsa+renesas@sang-engineering.com>
On Mon, 29 Apr 2024 13:28:39 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_completion_timeout() causing patterns like:
>
> timeout = wait_for_completion_timeout(...)
> if (!timeout) return -ETIMEDOUT;
>
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.
>
> Fix to the proper variable type 'unsigned long' while here.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Cheers,
Andre
> ---
> drivers/spi/spi-sun4i.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 11d8bd27b3e9..2ee6755b43f5 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -206,7 +206,8 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
> struct spi_transfer *tfr)
> {
> struct sun4i_spi *sspi = spi_controller_get_devdata(host);
> - unsigned int mclk_rate, div, timeout;
> + unsigned int mclk_rate, div;
> + unsigned long time_left;
> unsigned int start, end, tx_time;
> unsigned int tx_len = 0;
> int ret = 0;
> @@ -327,10 +328,10 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>
> tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
> start = jiffies;
> - timeout = wait_for_completion_timeout(&sspi->done,
> - msecs_to_jiffies(tx_time));
> + time_left = wait_for_completion_timeout(&sspi->done,
> + msecs_to_jiffies(tx_time));
> end = jiffies;
> - if (!timeout) {
> + if (!time_left) {
> dev_warn(&host->dev,
> "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
> dev_name(&spi->dev), tfr->len, tfr->speed_hz,
WARNING: multiple messages have this Message-ID (diff)
From: Andre Przywara <andre.przywara@arm.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
Chen-Yu Tsai <wens@csie.org>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Samuel Holland <samuel@sholland.org>,
linux-arm-kernel@lists.infradead.org,
linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/8] spi: sun4i: use 'time_left' variable with wait_for_completion_timeout()
Date: Mon, 29 Apr 2024 13:14:38 +0100 [thread overview]
Message-ID: <20240429131438.1f036341@donnerap.manchester.arm.com> (raw)
In-Reply-To: <20240429112843.67628-7-wsa+renesas@sang-engineering.com>
On Mon, 29 Apr 2024 13:28:39 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_completion_timeout() causing patterns like:
>
> timeout = wait_for_completion_timeout(...)
> if (!timeout) return -ETIMEDOUT;
>
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.
>
> Fix to the proper variable type 'unsigned long' while here.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Cheers,
Andre
> ---
> drivers/spi/spi-sun4i.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index 11d8bd27b3e9..2ee6755b43f5 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -206,7 +206,8 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
> struct spi_transfer *tfr)
> {
> struct sun4i_spi *sspi = spi_controller_get_devdata(host);
> - unsigned int mclk_rate, div, timeout;
> + unsigned int mclk_rate, div;
> + unsigned long time_left;
> unsigned int start, end, tx_time;
> unsigned int tx_len = 0;
> int ret = 0;
> @@ -327,10 +328,10 @@ static int sun4i_spi_transfer_one(struct spi_controller *host,
>
> tx_time = max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U);
> start = jiffies;
> - timeout = wait_for_completion_timeout(&sspi->done,
> - msecs_to_jiffies(tx_time));
> + time_left = wait_for_completion_timeout(&sspi->done,
> + msecs_to_jiffies(tx_time));
> end = jiffies;
> - if (!timeout) {
> + if (!time_left) {
> dev_warn(&host->dev,
> "%s: timeout transferring %u bytes@%iHz for %i(%i)ms",
> dev_name(&spi->dev), tfr->len, tfr->speed_hz,
_______________________________________________
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-04-29 12:14 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 11:28 [PATCH 0/8] spi: use 'time_left' instead of 'timeout' with wait_for_*() functions Wolfram Sang
2024-04-29 11:28 ` Wolfram Sang
2024-04-29 11:28 ` [PATCH 1/8] spi: armada-3700: use 'time_left' variable with wait_for_completion_timeout() Wolfram Sang
2024-04-29 15:15 ` Mark Brown
2024-04-29 11:28 ` [PATCH 2/8] spi: fsl-lpspi: " Wolfram Sang
2024-04-29 11:28 ` [PATCH 3/8] spi: imx: " Wolfram Sang
2024-04-29 11:28 ` Wolfram Sang
2024-04-30 1:35 ` Peng Fan
2024-04-30 1:35 ` Peng Fan
2024-04-29 11:28 ` [PATCH 4/8] spi: pic32-sqi: " Wolfram Sang
2024-04-29 11:28 ` [PATCH 5/8] spi: pic32: " Wolfram Sang
2024-04-29 11:28 ` [PATCH 6/8] spi: sun4i: " Wolfram Sang
2024-04-29 11:28 ` Wolfram Sang
2024-04-29 12:14 ` Andre Przywara [this message]
2024-04-29 12:14 ` Andre Przywara
2024-04-29 15:19 ` Jernej Škrabec
2024-04-29 15:19 ` Jernej Škrabec
2024-04-29 11:28 ` [PATCH 7/8] spi: sun6i: " Wolfram Sang
2024-04-29 11:28 ` Wolfram Sang
2024-04-29 12:16 ` Andre Przywara
2024-04-29 12:16 ` Andre Przywara
2024-04-29 14:36 ` Wolfram Sang
2024-04-29 14:36 ` Wolfram Sang
2024-04-29 15:20 ` Jernej Škrabec
2024-04-29 15:20 ` Jernej Škrabec
2024-04-29 11:28 ` [PATCH 8/8] spi: xlp: " Wolfram Sang
2024-05-02 3:57 ` [PATCH 0/8] spi: use 'time_left' instead of 'timeout' with wait_for_*() functions Mark Brown
2024-05-02 3:57 ` Mark Brown
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=20240429131438.1f036341@donnerap.manchester.arm.com \
--to=andre.przywara@arm.com \
--cc=broonie@kernel.org \
--cc=jernej.skrabec@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=samuel@sholland.org \
--cc=wens@csie.org \
--cc=wsa+renesas@sang-engineering.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.