| Message ID | 20231221-ufs-reset-ensure-effect-before-delay-v2-0-75af2a9bae51@redhat.com |
|---|---|
| Headers | show |
| Series | scsi: ufs: Remove overzealous memory barriers | expand |
Sorry for the spam, b4 and smtp with google are not playing nice and failed (but worked fine with --reflect). I'll send a v3 the old school way. My apologies, Andrew On Thu, Dec 21, 2023 at 12:25:17PM -0600, Andrew Halaney wrote: > This is an RFC because I'm not all the confident in this topic. 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). > > In this current series I don't do that as I wasn't totally convinced, > but it should be considered when reviewing. > > Hopefully this series gets enough interest where we can confidently > merge the final result after review helps improve it. > > I'll be travelling a good bit the next 2ish weeks, so expect little > response until my return. > > Thanks in advance for the help, > Andrew > > Signed-off-by: Andrew Halaney <ahalaney@redhat.com> > --- > 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: Perform read back 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: Perform read back to commit doorbell > scsi: ufs: core: Perform read back before writing run/stop regs > > drivers/ufs/core/ufshcd.c | 10 +++++----- > drivers/ufs/host/cdns-pltfrm.c | 2 +- > drivers/ufs/host/ufs-qcom.c | 14 ++++++-------- > drivers/ufs/host/ufs-qcom.h | 12 ++++++------ > 4 files changed, 18 insertions(+), 20 deletions(-) > --- > base-commit: 20d857259d7d10cd0d5e8b60608455986167cfad > change-id: 20231208-ufs-reset-ensure-effect-before-delay-6e06899d5419 > > Best regards, > -- > Andrew Halaney <ahalaney@redhat.com> >
This is an RFC because I'm not all the confident in this topic. 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). In this current series I don't do that as I wasn't totally convinced, but it should be considered when reviewing. Hopefully this series gets enough interest where we can confidently merge the final result after review helps improve it. I'll be travelling a good bit the next 2ish weeks, so expect little response until my return. Thanks in advance for the help, Andrew Signed-off-by: Andrew Halaney <ahalaney@redhat.com> --- 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: Perform read back 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: Perform read back to commit doorbell scsi: ufs: core: Perform read back before writing run/stop regs drivers/ufs/core/ufshcd.c | 10 +++++----- drivers/ufs/host/cdns-pltfrm.c | 2 +- drivers/ufs/host/ufs-qcom.c | 14 ++++++-------- drivers/ufs/host/ufs-qcom.h | 12 ++++++------ 4 files changed, 18 insertions(+), 20 deletions(-) --- base-commit: 20d857259d7d10cd0d5e8b60608455986167cfad change-id: 20231208-ufs-reset-ensure-effect-before-delay-6e06899d5419 Best regards,