From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Lucas Tanure <tanureal@opensource.cirrus.com>
Cc: Mark Brown <broonie@kernel.org>,
Sanjay R Mehta <sanju.mehta@amd.com>,
Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@amd.com>,
<linux-kernel@vger.kernel.org>, <linux-spi@vger.kernel.org>,
<patches@opensource.cirrus.com>
Subject: Re: [PATCH 4/4] spi: amd: Check for idle bus before execute opcode
Date: Fri, 10 Sep 2021 08:57:30 +0000 [thread overview]
Message-ID: <20210910085730.GX9223@ediswmail.ad.cirrus.com> (raw)
In-Reply-To: <20210909111005.304101-4-tanureal@opensource.cirrus.com>
On Thu, Sep 09, 2021 at 12:10:05PM +0100, Lucas Tanure wrote:
> Check if the bus is not in use before starting the
> transfer
>
> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
> ---
> drivers/spi/spi-amd.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
> index 97838b57871c..99b2b0ccff08 100644
> --- a/drivers/spi/spi-amd.c
> +++ b/drivers/spi/spi-amd.c
> @@ -115,11 +115,18 @@ static int amd_spi_busy_wait(struct amd_spi *amd_spi)
> return 0;
> }
>
> -static void amd_spi_execute_opcode(struct amd_spi *amd_spi)
> +static int amd_spi_execute_opcode(struct amd_spi *amd_spi)
> {
> + int ret;
> +
> + ret = amd_spi_busy_wait(amd_spi);
> + if (ret)
> + return ret;
> +
> /* Set ExecuteOpCode bit in the CTRL0 register */
> amd_spi_setclear_reg32(amd_spi, AMD_SPI_CTRL0_REG, AMD_SPI_EXEC_CMD, AMD_SPI_EXEC_CMD);
> - amd_spi_busy_wait(amd_spi);
> +
> + return 0;
> }
This feels like the commit message could use come additional
explanation. The message states we are moving the wait, but not
why? Also the original code looks slightly more logical, as
in amd_spi_fifo_xfer we read the receive buffer immediately after
calling this function.
Thanks,
Charles
next prev parent reply other threads:[~2021-09-10 8:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-09 11:10 [PATCH 1/4] spi: amd: Refactor code to use less spi_master_get_devdata Lucas Tanure
2021-09-09 11:10 ` [PATCH 2/4] spi: amd: Refactor amd_spi_busy_wait Lucas Tanure
2021-09-10 8:48 ` Charles Keepax
2021-09-09 11:10 ` [PATCH 3/4] spi: amd: Remove unneeded variable Lucas Tanure
2021-09-10 8:49 ` Charles Keepax
2021-09-09 11:10 ` [PATCH 4/4] spi: amd: Check for idle bus before execute opcode Lucas Tanure
2021-09-10 8:57 ` Charles Keepax [this message]
2021-09-10 8:47 ` [PATCH 1/4] spi: amd: Refactor code to use less spi_master_get_devdata Charles Keepax
2021-09-13 10:53 ` 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=20210910085730.GX9223@ediswmail.ad.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=Nehal-Bakulchandra.shah@amd.com \
--cc=broonie@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=sanju.mehta@amd.com \
--cc=tanureal@opensource.cirrus.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
all inboxes | Powered by JetHome®