-----Original Message-----
From: Vignesh Raghavendra <vigneshr@ti.com>
Sent: 21 November 2019 16:26
To: Alim Akhtar <redacted>; sheebab <redacted>
Cc: Alim Akhtar <alim.akhtar@samsung.com>; Avri Altman
[off-list ref]; Pedro Sousa [off-list ref]; James
E.J. Bottomley [off-list ref]; Martin K. Petersen
[off-list ref]; Stanley Chu [off-list ref];
Bean Huo (beanhuo) [off-list ref]; yuehaibing@huawei.com; linux-
scsi@vger.kernel.org; open list [off-list ref]; linux-
block@vger.kernel.org; rafalc@cadence.com; mparab@cadence.com
Subject: Re: [PATCH RESEND 2/2] scsi: ufs: Update L4 attributes on manual
hibern8 exit in Cadence UFS.



On 20/11/19 9:50 PM, Alim Akhtar wrote:

quoted

Hi Sheebab

On Tue, Nov 19, 2019 at 12:38 PM sheebab [off-list ref] wrote:

quoted

Backup L4 attributes duirng manual hibern8 entry and restore the L4
attributes on manual hibern8 exit as per JESD220C.

Can you point me to the relevant section on the spec?

Per JESD 220C 9.4 UniPro/UFS Control Interface (Control Plane):

"NOTE After exit from Hibernate all UniPro Transport Layer attributes (including
L4 T_PeerDeviceID,

L4 T_PeerCPortID, L4 T_ConnectionState, etc.) will be reset to their reset values.
All required attributes

must be restored properly on both ends before communication can resume."

But its not clear whether SW needs to restore these attributes or hardware

Thanks Vignesh for pointing out the spec section, yes it is not clear, one way to confirm this is just by read L4 attributes before 
And after hinern8 entry/exit.
(at least in the current platform it is not being done)
AFA this patch is concerns, this looks ok to me.
@ Avri , any thought on this?

Regards
Vignesh

quoted

quoted

Signed-off-by: sheebab <redacted>
---
 drivers/scsi/ufs/cdns-pltfrm.c | 97
+++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 95 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/cdns-pltfrm.c

b/drivers/scsi/ufs/cdns-pltfrm.c index adbbd60..5510567 100644

--- a/drivers/scsi/ufs/cdns-pltfrm.c
+++ b/drivers/scsi/ufs/cdns-pltfrm.c

@@ -19,6 +19,14 @@

 #define CDNS_UFS_REG_HCLKDIV   0xFC
 #define CDNS_UFS_REG_PHY_XCFGD1        0x113C
+#define CDNS_UFS_MAX 12
+
+struct cdns_ufs_host {
+       /**
+        * cdns_ufs_dme_attr_val - for storing L4 attributes
+        */
+       u32 cdns_ufs_dme_attr_val[CDNS_UFS_MAX];
+};

 /**
  * cdns_ufs_enable_intr - enable interrupts @@ -47,6 +55,77 @@

static void cdns_ufs_disable_intr(struct ufs_hba *hba, u32 intrs)  }

 /**
+ * cdns_ufs_get_l4_attr - get L4 attributes on local side
+ * @hba: per adapter instance
+ *
+ */
+static void cdns_ufs_get_l4_attr(struct ufs_hba *hba) {
+       struct cdns_ufs_host *host = ufshcd_get_variant(hba);
+
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_PEERDEVICEID),
+                      &host->cdns_ufs_dme_attr_val[0]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_PEERCPORTID),
+                      &host->cdns_ufs_dme_attr_val[1]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_TRAFFICCLASS),
+                      &host->cdns_ufs_dme_attr_val[2]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_PROTOCOLID),
+                      &host->cdns_ufs_dme_attr_val[3]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_CPORTFLAGS),
+                      &host->cdns_ufs_dme_attr_val[4]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_TXTOKENVALUE),
+                      &host->cdns_ufs_dme_attr_val[5]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_RXTOKENVALUE),
+                      &host->cdns_ufs_dme_attr_val[6]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_LOCALBUFFERSPACE),
+                      &host->cdns_ufs_dme_attr_val[7]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_PEERBUFFERSPACE),
+                      &host->cdns_ufs_dme_attr_val[8]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_CREDITSTOSEND),
+                      &host->cdns_ufs_dme_attr_val[9]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_CPORTMODE),
+                      &host->cdns_ufs_dme_attr_val[10]);
+       ufshcd_dme_get(hba, UIC_ARG_MIB(T_CONNECTIONSTATE),
+                      &host->cdns_ufs_dme_attr_val[11]);
+}
+
+/**
+ * cdns_ufs_set_l4_attr - set L4 attributes on local side
+ * @hba: per adapter instance
+ *
+ */
+static void cdns_ufs_set_l4_attr(struct ufs_hba *hba) {
+       struct cdns_ufs_host *host = ufshcd_get_variant(hba);
+
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_CONNECTIONSTATE), 0);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_PEERDEVICEID),
+                      host->cdns_ufs_dme_attr_val[0]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_PEERCPORTID),
+                      host->cdns_ufs_dme_attr_val[1]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_TRAFFICCLASS),
+                      host->cdns_ufs_dme_attr_val[2]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_PROTOCOLID),
+                      host->cdns_ufs_dme_attr_val[3]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_CPORTFLAGS),
+                      host->cdns_ufs_dme_attr_val[4]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_TXTOKENVALUE),
+                      host->cdns_ufs_dme_attr_val[5]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_RXTOKENVALUE),
+                      host->cdns_ufs_dme_attr_val[6]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_LOCALBUFFERSPACE),
+                      host->cdns_ufs_dme_attr_val[7]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_PEERBUFFERSPACE),
+                      host->cdns_ufs_dme_attr_val[8]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_CREDITSTOSEND),
+                      host->cdns_ufs_dme_attr_val[9]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_CPORTMODE),
+                      host->cdns_ufs_dme_attr_val[10]);
+       ufshcd_dme_set(hba, UIC_ARG_MIB(T_CONNECTIONSTATE),
+                      host->cdns_ufs_dme_attr_val[11]); }
+
+/**
  * Sets HCLKDIV register value based on the core_clk
  * @hba: host controller instance
  *

@@ -134,6 +213,7 @@ static void cdns_ufs_hibern8_notify(struct ufs_hba

*hba, enum uic_cmd_dme cmd,

quoted

quoted

                 * before manual hibernate entry.
                 */
                cdns_ufs_enable_intr(hba, UFSHCD_UIC_HIBERN8_MASK);
+               cdns_ufs_get_l4_attr(hba);
        }
        if (status == POST_CHANGE && cmd == UIC_CMD_DME_HIBER_EXIT) {
                /**

@@ -141,6 +221,7 @@ static void cdns_ufs_hibern8_notify(struct ufs_hba

*hba, enum uic_cmd_dme cmd,

quoted

quoted

                 * after manual hibern8 exit.
                 */
                cdns_ufs_disable_intr(hba, UFSHCD_UIC_HIBERN8_MASK);
+               cdns_ufs_set_l4_attr(hba);
        }
 }

@@ -245,15 +326,27 @@ static int cdns_ufs_pltfrm_probe(struct

platform_device *pdev)

quoted

quoted

        const struct of_device_id *of_id;
        struct ufs_hba_variant_ops *vops;
        struct device *dev = &pdev->dev;
+       struct cdns_ufs_host *host;
+       struct ufs_hba *hba;

        of_id = of_match_node(cdns_ufs_of_match, dev->of_node);
        vops = (struct ufs_hba_variant_ops *)of_id->data;

        /* Perform generic probe */
        err = ufshcd_pltfrm_init(pdev, vops);
-       if (err)
+       if (err) {
                dev_err(dev, "ufshcd_pltfrm_init() failed %d\n",
err);
-
+               goto out;
+       }
+       host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
+       if (!host) {
+               err = -ENOMEM;
+               dev_err(dev, "%s: no memory for cdns host\n", __func__);
+               goto out;
+       }
+       hba =  platform_get_drvdata(pdev);
+       ufshcd_set_variant(hba, host);
+out:
        return err;
 }

--
2.7.4

--
Regards
Vignesh