| Message ID | 20200827072030.24655-1-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V2] scsi: ufs-pci: Add LTR support for Intel controllers | expand |
> > Intel host controllers support the setting of latency tolerance. > Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The > raw register values are also exposed via debugfs. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Reviewed-by: Avri Altman <avri.altman@wdc.com>
Adrian, > Intel host controllers support the setting of latency tolerance. > Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The > raw register values are also exposed via debugfs. Does not apply to 5.10/scsi-queue. Please rebase. Thanks!
On 2/09/20 5:12 am, Martin K. Petersen wrote: > > Adrian, > >> Intel host controllers support the setting of latency tolerance. >> Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The >> raw register values are also exposed via debugfs. > > Does not apply to 5.10/scsi-queue. Please rebase. Thanks! > Hi Thanks for processing this. The 5.10/scsi-queue branch seems to be missing the following fix. If you cherry pick that, then it applies. commit 8da76f71fef7d8a1a72af09d48899573feb60065 Author: Adrian Hunter <adrian.hunter@intel.com> Date: Mon Aug 10 17:10:24 2020 +0300 scsi: ufs-pci: Add quirk for broken auto-hibernate for Intel EHL Intel EHL UFS host controller advertises auto-hibernate capability but it does not work correctly. Add a quirk for that. [mkp: checkpatch fix] Link: https://lore.kernel.org/r/20200810141024.28859-1-adrian.hunter@intel.com Fixes: 8c09d7527697 ("scsi: ufshdc-pci: Add Intel PCI IDs for EHL") Acked-by: Stanley Chu <stanley.chu@mediatek.com> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
On 2/09/20 12:27 pm, Adrian Hunter wrote: > On 2/09/20 5:12 am, Martin K. Petersen wrote: >> >> Adrian, >> >>> Intel host controllers support the setting of latency tolerance. >>> Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The >>> raw register values are also exposed via debugfs. >> >> Does not apply to 5.10/scsi-queue. Please rebase. Thanks! >> > > Hi > > Thanks for processing this. > > The 5.10/scsi-queue branch seems to be missing the following fix. If you cherry > pick that, then it applies. Now there seem to be conflicts between 5.10/scsi-queue and v5.9-rc4. I am not sure what I can do? ~/git/scsi-mkp$ git branch -v --list 5.10/scsi-queue * 5.10/scsi-queue 2e9defc7e918 [ahead 427, behind 97] scsi: ufs: Fix a race condition between error handler and runtime PM ops ~/git/scsi-mkp$ git rebase --onto v5.9-rc4 v5.9-rc1 First, rewinding head to replay your work on top of it... Applying: scsi: ufs: Add checks before setting clk-gating states Applying: scsi: ufs: ufs-qcom: Fix race conditions caused by ufs_qcom_testbus_config() Applying: scsi: ufs-qcom: Remove testbus dump in ufs_qcom_dump_dbg_regs Applying: scsi: ufs: Add some debug information to ufshcd_print_host_state() Applying: scsi: ufs: Fix concurrency of error handler and other error recovery paths Applying: scsi: ufs: Recover HBA runtime PM error in error handler Applying: scsi: ufs: Move dumps in IRQ handler to error handler Applying: scsi: ufs: Fix a race condition between error handler and runtime PM ops Applying: scsi: ufs: Properly release resources if a task is aborted successfully Using index info to reconstruct a base tree... M drivers/scsi/ufs/ufshcd.c Falling back to patching base and 3-way merge... Auto-merging drivers/scsi/ufs/ufshcd.c CONFLICT (content): Merge conflict in drivers/scsi/ufs/ufshcd.c error: Failed to merge in the changes. Patch failed at 0009 scsi: ufs: Properly release resources if a task is aborted successfully Use 'git am --show-current-patch' to see the failed patch Resolve all conflicts manually, mark them as resolved with "git add/rm <conflicted_files>", then run "git rebase --continue". You can instead skip this commit: run "git rebase --skip". To abort and get back to the state before "git rebase", run "git rebase --abort". ~/git/scsi-mkp$ git diff diff --cc drivers/scsi/ufs/ufshcd.c index d9386f85c255,efb40b1b95b4..000000000000 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@@ -6640,12 -6636,8 +6640,14 @@@ static int ufshcd_abort(struct scsi_cmn goto out; } ++<<<<<<< HEAD +cleanup: + scsi_dma_unmap(cmd); + ++======= ++>>>>>>> scsi: ufs: Properly release resources if a task is aborted successfully spin_lock_irqsave(host->host_lock, flags); - ufshcd_outstanding_req_clear(hba, tag); - hba->lrb[tag].cmd = NULL; + __ufshcd_transfer_req_compl(hba, (1UL << tag)); spin_unlock_irqrestore(host->host_lock, flags); out: ~/git/scsi-mkp$ git am --show-current-patch | head -25 commit 8bb2dde069d860e7ea379862a7d0e8ee01cec5e9 Author: Can Guo <cang@codeaurora.org> Date: Sun Aug 9 05:15:55 2020 -0700 scsi: ufs: Properly release resources if a task is aborted successfully In current UFS task abort hook, namely ufshcd_abort(), if one task is aborted successfully, clk_gating.active_reqs held by this task is not decreased, which makes clk_gating.active_reqs stay above zero forever, thus clock gating would never happen. Instead of releasing resources of one task "manually", use the existing func __ufshcd_transfer_req_compl(). This change also eliminates a possible race of scsi_dma_unmap() from the real completion in IRQ handler path. Link: https://lore.kernel.org/r/1596975355-39813-10-git-send-email-cang@codeaurora.org Fixes: 1ab27c9cf8b6 ("ufs: Add support for clock gating") CC: Stanley Chu <stanley.chu@mediatek.com> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org> Signed-off-by: Can Guo <cang@codeaurora.org> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
On 9/09/20 5:06 pm, Adrian Hunter wrote: > On 2/09/20 12:27 pm, Adrian Hunter wrote: >> On 2/09/20 5:12 am, Martin K. Petersen wrote: >>> >>> Adrian, >>> >>>> Intel host controllers support the setting of latency tolerance. >>>> Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The >>>> raw register values are also exposed via debugfs. >>> >>> Does not apply to 5.10/scsi-queue. Please rebase. Thanks! >>> >> >> Hi >> >> Thanks for processing this. >> >> The 5.10/scsi-queue branch seems to be missing the following fix. If you cherry >> pick that, then it applies. > > Now there seem to be conflicts between 5.10/scsi-queue and v5.9-rc4. > I am not sure what I can do? Now I see it does apply to James' for-next branch. Can it be applied there?
Adrian, >> Now there seem to be conflicts between 5.10/scsi-queue and v5.9-rc4. >> I am not sure what I can do? > > Now I see it does apply to James' for-next branch. Can it be applied there? > I'll set up a dedicated branch.
Adrian, > Intel host controllers support the setting of latency tolerance. > Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The > raw register values are also exposed via debugfs. Applied to 5.10/scsi-staging. Thanks!
On Thu, 27 Aug 2020 10:20:30 +0300, Adrian Hunter wrote: > Intel host controllers support the setting of latency tolerance. > Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The > raw register values are also exposed via debugfs. Applied to 5.10/scsi-queue, thanks! [1/1] scsi: ufs-pci: Add LTR support for Intel controllers https://git.kernel.org/mkp/scsi/c/247f99445938
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c index 5a95a7bfbab0..df3a564c3e33 100644 --- a/drivers/scsi/ufs/ufshcd-pci.c +++ b/drivers/scsi/ufs/ufshcd-pci.c @@ -13,6 +13,14 @@ #include "ufshcd.h" #include <linux/pci.h> #include <linux/pm_runtime.h> +#include <linux/pm_qos.h> +#include <linux/debugfs.h> + +struct intel_host { + u32 active_ltr; + u32 idle_ltr; + struct dentry *debugfs_root; +}; static int ufs_intel_disable_lcc(struct ufs_hba *hba) { @@ -44,20 +52,134 @@ static int ufs_intel_link_startup_notify(struct ufs_hba *hba, return err; } +#define INTEL_ACTIVELTR 0x804 +#define INTEL_IDLELTR 0x808 + +#define INTEL_LTR_REQ BIT(15) +#define INTEL_LTR_SCALE_MASK GENMASK(11, 10) +#define INTEL_LTR_SCALE_1US (2 << 10) +#define INTEL_LTR_SCALE_32US (3 << 10) +#define INTEL_LTR_VALUE_MASK GENMASK(9, 0) + +static void intel_cache_ltr(struct ufs_hba *hba) +{ + struct intel_host *host = ufshcd_get_variant(hba); + + host->active_ltr = readl(hba->mmio_base + INTEL_ACTIVELTR); + host->idle_ltr = readl(hba->mmio_base + INTEL_IDLELTR); +} + +static void intel_ltr_set(struct device *dev, s32 val) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + struct intel_host *host = ufshcd_get_variant(hba); + u32 ltr; + + pm_runtime_get_sync(dev); + + /* + * Program latency tolerance (LTR) accordingly what has been asked + * by the PM QoS layer or disable it in case we were passed + * negative value or PM_QOS_LATENCY_ANY. + */ + ltr = readl(hba->mmio_base + INTEL_ACTIVELTR); + + if (val == PM_QOS_LATENCY_ANY || val < 0) { + ltr &= ~INTEL_LTR_REQ; + } else { + ltr |= INTEL_LTR_REQ; + ltr &= ~INTEL_LTR_SCALE_MASK; + ltr &= ~INTEL_LTR_VALUE_MASK; + + if (val > INTEL_LTR_VALUE_MASK) { + val >>= 5; + if (val > INTEL_LTR_VALUE_MASK) + val = INTEL_LTR_VALUE_MASK; + ltr |= INTEL_LTR_SCALE_32US | val; + } else { + ltr |= INTEL_LTR_SCALE_1US | val; + } + } + + if (ltr == host->active_ltr) + goto out; + + writel(ltr, hba->mmio_base + INTEL_ACTIVELTR); + writel(ltr, hba->mmio_base + INTEL_IDLELTR); + + /* Cache the values into intel_host structure */ + intel_cache_ltr(hba); +out: + pm_runtime_put(dev); +} + +static void intel_ltr_expose(struct device *dev) +{ + dev->power.set_latency_tolerance = intel_ltr_set; + dev_pm_qos_expose_latency_tolerance(dev); +} + +static void intel_ltr_hide(struct device *dev) +{ + dev_pm_qos_hide_latency_tolerance(dev); + dev->power.set_latency_tolerance = NULL; +} + +static void intel_add_debugfs(struct ufs_hba *hba) +{ + struct dentry *dir = debugfs_create_dir(dev_name(hba->dev), NULL); + struct intel_host *host = ufshcd_get_variant(hba); + + intel_cache_ltr(hba); + + host->debugfs_root = dir; + debugfs_create_x32("active_ltr", 0444, dir, &host->active_ltr); + debugfs_create_x32("idle_ltr", 0444, dir, &host->idle_ltr); +} + +static void intel_remove_debugfs(struct ufs_hba *hba) +{ + struct intel_host *host = ufshcd_get_variant(hba); + + debugfs_remove_recursive(host->debugfs_root); +} + +static int ufs_intel_common_init(struct ufs_hba *hba) +{ + struct intel_host *host; + + host = devm_kzalloc(hba->dev, sizeof(*host), GFP_KERNEL); + if (!host) + return -ENOMEM; + ufshcd_set_variant(hba, host); + intel_ltr_expose(hba->dev); + intel_add_debugfs(hba); + return 0; +} + +static void ufs_intel_common_exit(struct ufs_hba *hba) +{ + intel_remove_debugfs(hba); + intel_ltr_hide(hba->dev); +} + static int ufs_intel_ehl_init(struct ufs_hba *hba) { hba->quirks |= UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8; - return 0; + return ufs_intel_common_init(hba); } static struct ufs_hba_variant_ops ufs_intel_cnl_hba_vops = { .name = "intel-pci", + .init = ufs_intel_common_init, + .exit = ufs_intel_common_exit, .link_startup_notify = ufs_intel_link_startup_notify, }; static struct ufs_hba_variant_ops ufs_intel_ehl_hba_vops = { .name = "intel-pci", .init = ufs_intel_ehl_init, + .exit = ufs_intel_common_exit, .link_startup_notify = ufs_intel_link_startup_notify, }; @@ -162,6 +284,8 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) return err; } + pci_set_drvdata(pdev, hba); + hba->vops = (struct ufs_hba_variant_ops *)id->driver_data; err = ufshcd_init(hba, mmio_base, pdev->irq); @@ -171,7 +295,6 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) return err; } - pci_set_drvdata(pdev, hba); pm_runtime_put_noidle(&pdev->dev); pm_runtime_allow(&pdev->dev);
Intel host controllers support the setting of latency tolerance. Accordingly, implement the PM QoS ->set_latency_tolerance() callback. The raw register values are also exposed via debugfs. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- Changes in V2: Put debugfs code altogether drivers/scsi/ufs/ufshcd-pci.c | 127 +++++++++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 2 deletions(-)