From: Andrew Halaney <ahalaney@redhat.com>
To: Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Manivannan Sadhasivam <mani@kernel.org>,
"James E.J. Bottomley" <jejb@linux.ibm.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Hannes Reinecke <hare@suse.de>, Janek Kotas <jank@cadence.com>,
Alim Akhtar <alim.akhtar@samsung.com>,
Avri Altman <avri.altman@wdc.com>,
Bart Van Assche <bvanassche@acm.org>,
Can Guo <quic_cang@quicinc.com>,
Anjana Hari <quic_ahari@quicinc.com>
Cc: Andrew Halaney <ahalaney@redhat.com>,
Will Deacon <will@kernel.org>,
linux-arm-msm@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org,
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Subject: [PATCH v5 00/11] scsi: ufs: Remove overzealous memory barriers
Date: Fri, 29 Mar 2024 15:46:42 -0500 [thread overview]
Message-ID: <20240329-ufs-reset-ensure-effect-before-delay-v5-0-181252004586@redhat.com> (raw)
Please review with care as I'm not all that confident in this subject.
UFS has a lot of mb() variants used, most with comments saying "ensure this
takes effect before continuing". mb()'s aren't really the way to
guarantee that, a read back is the best method.
Some of these though I think could go a step further and remove the mb()
variant without a read back. As far as I can tell there's no real reason
to ensure it takes effect in most cases (there's no delay() or anything
afterwards, and eventually another readl()/writel() happens which is by
definition ordered). Some of the patches in this series do that if I was
confident it was safe (or a reviewer pointed out prior that they thought
it was safe to do so).
Thanks in advance for the help,
Andrew
To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Manivannan Sadhasivam <mani@kernel.org>
To: James E.J. Bottomley <jejb@linux.ibm.com>
To: Martin K. Petersen <martin.petersen@oracle.com>
To: Hannes Reinecke <hare@suse.de>
To: Janek Kotas <jank@cadence.com>
To: Alim Akhtar <alim.akhtar@samsung.com>
To: Avri Altman <avri.altman@wdc.com>
To: Bart Van Assche <bvanassche@acm.org>
To: Can Guo <quic_cang@quicinc.com>
To: Anjana Hari <quic_ahari@quicinc.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
Changes in v5:
- Rebased on top of next-20240327
- v4 changes need review, so please pay attention to that bit
still :)
- Link to v4: https://lore.kernel.org/r/20240122-ufs-reset-ensure-effect-before-delay-v4-0-6c48432151cc@redhat.com
Changes in v4:
- Collected Reviewed-by tags
- Changed patches 3, 4, 10, and 11 to drop the read back && mb():
- Please note all of those patches got reviewed-by tags by either
Can, Mani, or Bart, but one of the three pointed out that they
thought it could be dropped altogether (some of Mani's comments
are on my foobar'ed v2). After some consideration I
agree. Therefore I'd appreciate re-review on those patches by
you three to make sure that's appropriate
- Link to v3: https://lore.kernel.org/r/20231221-ufs-reset-ensure-effect-before-delay-v3-0-2195a1b66d2e@redhat.com
Changes in v3:
- Nothing changed, I just failed to send with b4 (resulting in 2 half
sent v2 series on list)
- Link to v2: https://lore.kernel.org/r/pnwsdz3i2liivjxvtfwq6tijotgh5adyqipjjb5wdvo4jpu7yv@j6fkshm5ipue
Changes in v2:
- Added review tags for original patch
- Added new patches to address all other memory barriers used
- Link to v1: https://lore.kernel.org/r/20231208-ufs-reset-ensure-effect-before-delay-v1-1-8a0f82d7a09e@redhat.com
---
Andrew Halaney (11):
scsi: ufs: qcom: Perform read back after writing reset bit
scsi: ufs: qcom: Perform read back after writing REG_UFS_SYS1CLK_1US
scsi: ufs: qcom: Remove unnecessary mb() after writing testbus config
scsi: ufs: qcom: Perform read back after writing unipro mode
scsi: ufs: qcom: Perform read back after writing CGC enable
scsi: ufs: cdns-pltfrm: Perform read back after writing HCLKDIV
scsi: ufs: core: Perform read back after writing UTP_TASK_REQ_LIST_BASE_H
scsi: ufs: core: Perform read back after disabling interrupts
scsi: ufs: core: Perform read back after disabling UIC_COMMAND_COMPL
scsi: ufs: core: Remove unnecessary wmb() after ringing doorbell
scsi: ufs: core: Remove unnecessary wmb() prior to writing run/stop regs
drivers/ufs/core/ufshcd.c | 15 +++------------
drivers/ufs/host/cdns-pltfrm.c | 2 +-
drivers/ufs/host/ufs-qcom.c | 12 ++----------
drivers/ufs/host/ufs-qcom.h | 12 ++++++------
4 files changed, 12 insertions(+), 29 deletions(-)
---
base-commit: 26074e1be23143b2388cacb36166766c235feb7c
change-id: 20231208-ufs-reset-ensure-effect-before-delay-6e06899d5419
Best regards,
--
Andrew Halaney <ahalaney@redhat.com>
next reply other threads:[~2024-03-29 20:48 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 20:46 Andrew Halaney [this message]
2024-03-29 20:46 ` [PATCH v5 01/11] scsi: ufs: qcom: Perform read back after writing reset bit Andrew Halaney
2024-03-29 20:46 ` [PATCH v5 02/11] scsi: ufs: qcom: Perform read back after writing REG_UFS_SYS1CLK_1US Andrew Halaney
2024-04-02 4:45 ` Manivannan Sadhasivam
2024-03-29 20:46 ` [PATCH v5 03/11] scsi: ufs: qcom: Remove unnecessary mb() after writing testbus config Andrew Halaney
2024-04-02 4:46 ` Manivannan Sadhasivam
2024-03-29 20:46 ` [PATCH v5 04/11] scsi: ufs: qcom: Perform read back after writing unipro mode Andrew Halaney
2024-04-02 4:47 ` Manivannan Sadhasivam
2024-03-29 20:46 ` [PATCH v5 05/11] scsi: ufs: qcom: Perform read back after writing CGC enable Andrew Halaney
2024-03-29 20:46 ` [PATCH v5 06/11] scsi: ufs: cdns-pltfrm: Perform read back after writing HCLKDIV Andrew Halaney
2024-03-29 21:47 ` Bart Van Assche
2024-03-29 20:46 ` [PATCH v5 07/11] scsi: ufs: core: Perform read back after writing UTP_TASK_REQ_LIST_BASE_H Andrew Halaney
2024-03-29 20:46 ` [PATCH v5 08/11] scsi: ufs: core: Perform read back after disabling interrupts Andrew Halaney
2024-03-29 20:46 ` [PATCH v5 09/11] scsi: ufs: core: Perform read back after disabling UIC_COMMAND_COMPL Andrew Halaney
2024-03-29 20:46 ` [PATCH v5 10/11] scsi: ufs: core: Remove unnecessary wmb() after ringing doorbell Andrew Halaney
2024-03-29 21:48 ` Bart Van Assche
2024-03-29 21:48 ` Bart Van Assche
2024-04-02 4:49 ` Manivannan Sadhasivam
2024-03-29 20:46 ` [PATCH v5 11/11] scsi: ufs: core: Remove unnecessary wmb() prior to writing run/stop regs Andrew Halaney
2024-03-29 21:49 ` Bart Van Assche
2024-03-29 21:49 ` Bart Van Assche
2024-04-02 4:49 ` Manivannan Sadhasivam
2024-04-06 1:09 ` [PATCH v5 00/11] scsi: ufs: Remove overzealous memory barriers Martin K. Petersen
2024-04-09 3:08 ` Martin K. Petersen
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=20240329-ufs-reset-ensure-effect-before-delay-v5-0-181252004586@redhat.com \
--to=ahalaney@redhat.com \
--cc=agross@kernel.org \
--cc=alim.akhtar@samsung.com \
--cc=andersson@kernel.org \
--cc=avri.altman@wdc.com \
--cc=bvanassche@acm.org \
--cc=hare@suse.de \
--cc=jank@cadence.com \
--cc=jejb@linux.ibm.com \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=mani@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=martin.petersen@oracle.com \
--cc=quic_ahari@quicinc.com \
--cc=quic_cang@quicinc.com \
--cc=will@kernel.org \
/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).