Linux-SPI Archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Mark Brown <broonie@kernel.org>,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	thomas.petazzoni@bootlin.com, yen-mei.goh@keysight.com,
	jeremie.dautheribes@bootlin.com
Subject: Re: [PATCH v2] spi: omap2-mcspi: Ensure that CS is correctly asserted when switching from MULTI mode to SINGLE mode
Date: Mon, 6 May 2024 16:53:36 +0200	[thread overview]
Message-ID: <20240506165336.6ab8213e@xps-13> (raw)
In-Reply-To: <20240506-fix-omap2-mcspi-v2-1-d9c77ba8b9c7@bootlin.com>

Hi Louis,

Minor nits below.

louis.chauvet@bootlin.com wrote on Mon, 06 May 2024 16:27:59 +0200:

> The OMAP2 support features a "multi" mode, where the controller

s/The //

> automatically sets the Chip Select (CS) when necessary. This mode allows
> for shorter delays between CS selection and word content, but the CS is
> always deasserted at the end.
> 
> The default transfer_one_message function expects the last transfer of a
> message to keep the CS enabled. However, when two consecutive messages
> use "multi" mode followed by "single" mode, the chipselect is not enabled

Above you wrote Chip Select and CS, maybe you want to reuse one of them
to be more consistent?

> during the second message. This discrepancy causes the driver to time
> out.
> 
> To address this issue, the CS is now reasserted for each new message that

				needs to be

> follows a "multi"-mode message. This change ensures proper CS handling
> and prevents driver timeouts.
> 
> Fixes: d153ff4056cb ("spi: omap2-mcspi: Add support for MULTI-mode")
> 

No new line needed here, but more importantly, I see no valid Git object
with this hash, can you double check?

> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
> Changes in v2:
> - Fix build error, I used the old API (master instead of controller)
> - Link to v1: https://lore.kernel.org/r/20240409-fix-omap2-mcspi-v1-1-f2b881ae196d@bootlin.com
> ---
>  drivers/spi/spi-omap2-mcspi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> index 7e3083b83534..dd514a7504ec 100644
> --- a/drivers/spi/spi-omap2-mcspi.c
> +++ b/drivers/spi/spi-omap2-mcspi.c
> @@ -1264,6 +1264,15 @@ static int omap2_mcspi_prepare_message(struct spi_controller *ctlr,
>  	struct spi_transfer	*tr;
>  	u8 bits_per_word;
>  
> +	/*
> +	 * When the last transfer used multi-mode, the CS is not keep down by the controller.

	   If the						 kept

> +	 * The default transfer_one_message expect that the last transfer of a message keep the CS

					    expects			               keeps

> +	 * active.
> +	 * Ensure that the next call to spi_set_cs will work as expected if single-mode is used.
> +	 */
> +	if (mcspi->use_multi_mode)
> +		ctlr->last_cs[0] = -1;

Maybe you should use SPI_INVALID_CS?

> +
>  	/*
>  	 * The conditions are strict, it is mandatory to check each transfer of the list to see if
>  	 * multi-mode is applicable.
> 
> ---
> base-commit: dfca7eb7615116b696ecdc85417c9e2df0b546dc
> change-id: 20240408-fix-omap2-mcspi-a6b41142ad9a
> 
> Best regards,


Thanks,
Miquèl

      reply	other threads:[~2024-05-06 14:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-06 14:27 [PATCH v2] spi: omap2-mcspi: Ensure that CS is correctly asserted when switching from MULTI mode to SINGLE mode Louis Chauvet
2024-05-06 14:53 ` Miquel Raynal [this message]

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=20240506165336.6ab8213e@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=broonie@kernel.org \
    --cc=jeremie.dautheribes@bootlin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=louis.chauvet@bootlin.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=yen-mei.goh@keysight.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 read-only IMAP folder(s) and NNTP newsgroup(s).