diff mbox series

soc: qcom-geni-se: Don't use relaxed writes when writing commands

Message ID 20200722150113.1.Ia50ab5cb8a6d3a73d302e6bdc25542d48ffd27f4@changeid (mailing list archive)
State Accepted
Commit 0feea33d79825d05b5ede30947db4df34722b463
Headers show
Series soc: qcom-geni-se: Don't use relaxed writes when writing commands | expand

Commit Message

Doug Anderson July 22, 2020, 10:01 p.m. UTC
Writing the command is the final step in kicking off a transfer.
Let's use writel() to ensure that any other memory accesses are done
before the command kicks off.  It's expected that this is mostly
relevant if we're in DMA mode but since it doesn't appear to regress
performance in a measurable way [1] even in PIO mode and it's easier
to reason about then let's just always use it.

NOTE: this patch came about due to code inspection.  No actual
problems were observed that this patch fixes.

[1] Tested by timing "flashrom -p ec" on a Chromebook which stresses
GENI SPI a lot.

Suggested-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 include/linux/qcom-geni-se.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stephen Boyd July 23, 2020, 12:50 a.m. UTC | #1
Quoting Douglas Anderson (2020-07-22 15:01:20)
> Writing the command is the final step in kicking off a transfer.
> Let's use writel() to ensure that any other memory accesses are done
> before the command kicks off.  It's expected that this is mostly
> relevant if we're in DMA mode but since it doesn't appear to regress
> performance in a measurable way [1] even in PIO mode and it's easier
> to reason about then let's just always use it.
> 
> NOTE: this patch came about due to code inspection.  No actual
> problems were observed that this patch fixes.
> 
> [1] Tested by timing "flashrom -p ec" on a Chromebook which stresses
> GENI SPI a lot.
> 
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Akash Asthana July 23, 2020, 5:13 a.m. UTC | #2
On 7/23/2020 3:31 AM, Douglas Anderson wrote:
> Writing the command is the final step in kicking off a transfer.
> Let's use writel() to ensure that any other memory accesses are done
> before the command kicks off.  It's expected that this is mostly
> relevant if we're in DMA mode but since it doesn't appear to regress
> performance in a measurable way [1] even in PIO mode and it's easier
> to reason about then let's just always use it.
>
> NOTE: this patch came about due to code inspection.  No actual
> problems were observed that this patch fixes.
>
> [1] Tested by timing "flashrom -p ec" on a Chromebook which stresses
> GENI SPI a lot.
>
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Akash Asthana <akashast@codeaurora.org>
Mukesh, Savaliya July 23, 2020, 8:57 a.m. UTC | #3
On 7/23/2020 3:31 AM, Douglas Anderson wrote:
> Writing the command is the final step in kicking off a transfer.
> Let's use writel() to ensure that any other memory accesses are done
> before the command kicks off.  It's expected that this is mostly
> relevant if we're in DMA mode but since it doesn't appear to regress
> performance in a measurable way [1] even in PIO mode and it's easier
> to reason about then let's just always use it.
>
> NOTE: this patch came about due to code inspection.  No actual
> problems were observed that this patch fixes.
>
> [1] Tested by timing "flashrom -p ec" on a Chromebook which stresses
> GENI SPI a lot.
>
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Mukesh Kumar Savaliya <msavaliy@codeaurora.org>
> ---
>
>   include/linux/qcom-geni-se.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
> index dd464943f717..f50c73be1428 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -262,7 +262,7 @@ static inline void geni_se_setup_m_cmd(struct geni_se *se, u32 cmd, u32 params)
>   	u32 m_cmd;
>   
>   	m_cmd = (cmd << M_OPCODE_SHFT) | (params & M_PARAMS_MSK);
> -	writel_relaxed(m_cmd, se->base + SE_GENI_M_CMD0);
> +	writel(m_cmd, se->base + SE_GENI_M_CMD0);
>   }
>   
>   /**
> @@ -282,7 +282,7 @@ static inline void geni_se_setup_s_cmd(struct geni_se *se, u32 cmd, u32 params)
>   	s_cmd &= ~(S_OPCODE_MSK | S_PARAMS_MSK);
>   	s_cmd |= (cmd << S_OPCODE_SHFT);
>   	s_cmd |= (params & S_PARAMS_MSK);
> -	writel_relaxed(s_cmd, se->base + SE_GENI_S_CMD0);
> +	writel(s_cmd, se->base + SE_GENI_S_CMD0);
>   }
>   
>   /**
diff mbox series

Patch

diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index dd464943f717..f50c73be1428 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -262,7 +262,7 @@  static inline void geni_se_setup_m_cmd(struct geni_se *se, u32 cmd, u32 params)
 	u32 m_cmd;
 
 	m_cmd = (cmd << M_OPCODE_SHFT) | (params & M_PARAMS_MSK);
-	writel_relaxed(m_cmd, se->base + SE_GENI_M_CMD0);
+	writel(m_cmd, se->base + SE_GENI_M_CMD0);
 }
 
 /**
@@ -282,7 +282,7 @@  static inline void geni_se_setup_s_cmd(struct geni_se *se, u32 cmd, u32 params)
 	s_cmd &= ~(S_OPCODE_MSK | S_PARAMS_MSK);
 	s_cmd |= (cmd << S_OPCODE_SHFT);
 	s_cmd |= (params & S_PARAMS_MSK);
-	writel_relaxed(s_cmd, se->base + SE_GENI_S_CMD0);
+	writel(s_cmd, se->base + SE_GENI_S_CMD0);
 }
 
 /**