pm80xx : Updates for the driver version 0.1.39.

From: Viswas G <[email protected]>

Added the support to get the phy profile which gives information about the phy states, port and errors on phy.

Signed-off-by Deepak Ukey <[email protected]>

Signed-off-by Viswas G <[email protected]>

Signed-off-by Radha Ramachandran <[email protected]>

Reported-by kernel test robot <[email protected]>

---
 drivers/scsi/pm8001/pm8001_ctl.h  | 33 +++++++++++++++
 drivers/scsi/pm8001/pm8001_init.c |  2 +
 drivers/scsi/pm8001/pm8001_sas.h  |  5 +++
 drivers/scsi/pm8001/pm80xx_hwi.c  | 88 ++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/pm8001/pm80xx_hwi.h  |  2 +
 5 files changed, 128 insertions(+), 2 deletions(-)

drivers/scsi/pm8001/pm8001_ctl.h +33 -0

index d0d43a250b9e..f7aadaf5c6aa 100644
@@ -41,6 +41,39 @@
 #ifndef PM8001_CTL_H_INCLUDED
 #define PM8001_CTL_H_INCLUDED
 
+#ifdef __LITTLE_ENDIAN_BITFIELD
+struct phy_status {
+       char            phy_id;
+       unsigned int    phy_state:4;
+       unsigned int    nlr:4;
+       unsigned int    plr:4;
+       unsigned int    reserved1:12;
+       unsigned char   port_id;
+       unsigned int    prts:4;
+       unsigned int    reserved2:20;
+} __packed;
+#else
+struct phy_status {
+       char            phy_id;
+       unsigned int    reserved1:12;
+       unsigned int    plr:4;
+       unsigned int    nlr:4;
+       unsigned int    phy_state:4;
+       unsigned char   port_id;
+       unsigned int    reserved2:20;
+       unsigned int    prts:4;
+} __packed;
+#endif
+
+struct phy_errcnt {
+       unsigned int    InvalidDword;
+       unsigned int    runningDisparityError;
+       unsigned int    codeViolation;
+       unsigned int    LossOfSyncDW;
+       unsigned int    phyResetProblem;
+       unsigned int    inboundCRCError;
+};
+
 #define IOCTL_BUF_SIZE         4096
 #define HEADER_LEN                     28
 #define SIZE_OFFSET                    16

drivers/scsi/pm8001/pm8001_init.c +2 -0

index 9e99262a2b9d..ff65d6cf6d31 100644
@@ -398,6 +398,8 @@ static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha,
                pm8001_ha->ccb_info[i].task = NULL;
                pm8001_ha->ccb_info[i].ccb_tag = 0xffffffff;
                pm8001_ha->ccb_info[i].device = NULL;
+               pm8001_ha->ccb_info[i].completion = NULL;
+               pm8001_ha->ccb_info[i].resp_buf = NULL;
                ++pm8001_ha->tags_num;
        }
        pm8001_ha->flags = PM8001F_INIT_TIME;

drivers/scsi/pm8001/pm8001_sas.h +5 -0

index ae7ba9b3c4bc..488af79dec47 100644
@@ -56,6 +56,7 @@
 #include <scsi/sas_ata.h>
 #include <linux/atomic.h>
 #include "pm8001_defs.h"
+#include "pm8001_ctl.h"
 
 #define DRV_NAME               "pm80xx"
 #define DRV_VERSION            "0.1.39"

@@ -244,6 +245,8 @@ struct pm8001_dispatch {
        int (*sas_diag_execute_req)(struct pm8001_hba_info *pm8001_ha,
                u32 state);
        int (*sas_re_init_req)(struct pm8001_hba_info *pm8001_ha);
+       int (*get_phy_profile_req)(struct pm8001_hba_info *pm8001_ha,
+               int phy, int page, struct completion *comp, void *buf);
 };
 
 struct pm8001_chip_info {

@@ -318,6 +321,8 @@ struct pm8001_ccb_info {
        struct pm8001_prd       buf_prd[PM8001_MAX_DMA_SG];
        struct fw_control_ex    *fw_control_context;
        u8                      open_retry;
+       struct completion       *completion;
+       void                    *resp_buf;
 };
 
 struct mpi_mem {

drivers/scsi/pm8001/pm80xx_hwi.c +86 -2

index 4d205ebaee87..baccb082e65f 100644
@@ -3792,7 +3792,6 @@ static int mpi_set_controller_config_resp(struct pm8001_hba_info *pm8001_ha,
        PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
                        "SET CONTROLLER RESP: status 0x%x qlfr_pgcd 0x%x\n",
                        status, err_qlfr_pgcd));
-
        return 0;
 }
 

@@ -3818,9 +3817,63 @@ static int mpi_get_controller_config_resp(struct pm8001_hba_info *pm8001_ha,
 static int mpi_get_phy_profile_resp(struct pm8001_hba_info *pm8001_ha,
                        void *piomb)
 {
+       u32 tag, page_code;
+       struct phy_status *phy_status, *phy_stat;
+       struct phy_errcnt *phy_err, *phy_err_cnt;
+       struct pm8001_ccb_info *ccb;
+       struct get_phy_profile_resp *pPayload =
+               (struct get_phy_profile_resp *)(piomb + 4);
+       u32 status = le32_to_cpu(pPayload->status);
+
+       page_code = (u8)((pPayload->ppc_phyid & 0xFF00) >> 8);
+
        PM8001_MSG_DBG(pm8001_ha,
-                       pm8001_printk(" pm80xx_addition_functionality\n"));
+               pm8001_printk(" pm80xx_addition_functionality\n"));
+       if (status) {
+               /* status is FAILED */
+               PM8001_FAIL_DBG(pm8001_ha, pm8001_printk(
+                       "mpiGetPhyProfileReq failed  with status 0x%08x\n",
+                       status));
+       }
 
+       tag = le32_to_cpu(pPayload->tag);
+       ccb = &pm8001_ha->ccb_info[tag];
+       if (ccb->completion != NULL) {
+               if (status) {
+                       /* signal fail status */
+                       memset(&ccb->resp_buf, 0xff, sizeof(ccb->resp_buf));
+               } else if (page_code == SAS_PHY_GENERAL_STATUS_PAGE) {
+                       phy_status = (struct phy_status *)ccb->resp_buf;
+                       phy_stat =
+                       (struct phy_status *)pPayload->ppc_specific_rsp;
+                       phy_status->phy_id =
+                               le32_to_cpu((__force __le32)phy_stat->phy_id);
+                       phy_status->phy_state =
+                               le32_to_cpu((__force __le32)phy_stat->phy_state);
+                       phy_status->plr =
+                               le32_to_cpu((__force __le32)phy_stat->plr);
+                       phy_status->nlr =
+                               le32_to_cpu((__force __le32)phy_stat->nlr);
+                       phy_status->port_id =
+                               le32_to_cpu((__force __le32)phy_stat->port_id);
+                       phy_status->prts =
+                               le32_to_cpu((__force __le32)phy_stat->prts);
+               } else if (page_code == SAS_PHY_ERR_COUNTERS_PAGE) {
+                       phy_err = (struct phy_errcnt *)ccb->resp_buf;
+                       phy_err_cnt =
+                       (struct phy_errcnt *)pPayload->ppc_specific_rsp;
+                       phy_err->InvalidDword =
+                       le32_to_cpu((__force __le32)phy_err_cnt->InvalidDword);
+                       phy_err->runningDisparityError = le32_to_cpu
+                       ((__force __le32)phy_err_cnt->runningDisparityError);
+                       phy_err->LossOfSyncDW = le32_to_cpu
+                       ((__force __le32)phy_err_cnt->LossOfSyncDW);
+                       phy_err->phyResetProblem = le32_to_cpu
+                       ((__force __le32)phy_err_cnt->phyResetProblem);
+               }
+               complete(ccb->completion);
+       }
+       pm8001_tag_free(pm8001_ha, tag);
        return 0;
 }
 

@@ -5013,6 +5066,36 @@ pm80xx_chip_isr(struct pm8001_hba_info *pm8001_ha, u8 vec)
        return IRQ_HANDLED;
 }
 
+static int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha,
+       int phy_id, int page_code, struct completion *comp, void *buf)
+{
+       u32 tag;
+       struct get_phy_profile_req payload;
+       struct inbound_queue_table *circularQ;
+       struct pm8001_ccb_info *ccb;
+       int rc, ppc_phyid;
+       u32 opc = OPC_INB_GET_PHY_PROFILE;
+
+       memset(&payload, 0, sizeof(payload));
+
+       rc = pm8001_tag_alloc(pm8001_ha, &tag);
+       if (rc)
+               PM8001_FAIL_DBG(pm8001_ha, pm8001_printk("Invalid tag\n"));
+
+       ccb = &pm8001_ha->ccb_info[tag];
+       ccb->completion = comp;
+       ccb->resp_buf = buf;
+       circularQ = &pm8001_ha->inbnd_q_tbl[0];
+
+       payload.tag = cpu_to_le32(tag);
+       ppc_phyid = (page_code & 0xFF)  << 8 | (phy_id & 0xFF);
+       payload.ppc_phyid = cpu_to_le32(ppc_phyid);
+
+       pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &payload,
+                       sizeof(payload), 0);
+       return rc;
+}
+
 void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
        u32 operation, u32 phyid, u32 length, u32 *buf)
 {

@@ -5113,4 +5196,5 @@ const struct pm8001_dispatch pm8001_80xx_dispatch = {
        .set_nvmd_req           = pm8001_chip_set_nvmd_req,
        .fw_flash_update_req    = pm8001_chip_fw_flash_update_req,
        .set_dev_state_req      = pm8001_chip_set_dev_state_req,
+       .get_phy_profile_req    = pm8001_chip_get_phy_profile,
 };

drivers/scsi/pm8001/pm80xx_hwi.h +2 -1

index 701951a0f715..b5119c5479da 100644
@@ -175,7 +175,9 @@
 #define PHY_STOP_ERR_DEVICE_ATTACHED   0x1046
 
 /* phy_profile */
+#define SAS_PHY_ERR_COUNTERS_PAGE      0x01
 #define SAS_PHY_ANALOG_SETTINGS_PAGE   0x04
+#define SAS_PHY_GENERAL_STATUS_PAGE    0x05
 #define PHY_DWORD_LENGTH               0xC
 
 /* Thermal related */
--

2.16.3

Hi Christoph,

Yes, It is better to be implemented in libsas. Since the out of box pm80xx driver has this support, we would like to push this for the time being. We will see how this can be moved to libsas.

Regards,
Deepak

-----Original Message-----
From: Christoph Hellwig [mailto:[email protected]]
Sent: Tuesday, August 4, 2020 11:33 AM
To: Deepak Ukey - I31172 <[email protected]>
Cc: [email protected]; Vasanthalakshmi Tharmarajan - I30664 <[email protected]>; Viswas G - I30667 <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V6 2/2] pm80xx : Staggered spin up support.

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

As mentioned before - this should be a libsas or transport class policy, and not a module parameter hack in one driver. .

Hi Christoph,

Yes, It is better to be implemented in libsas. Since the out of box pm80xx driver has this support, we would like to push this for the time being. We will see how this can be moved to libsas.

-----Original Message-----
From: Christoph Hellwig [mailto:[email protected]]
Sent: Tuesday, August 4, 2020 11:33 AM
To: Deepak Ukey - I31172 <[email protected]>
Cc: [email protected]; Vasanthalakshmi Tharmarajan - I30664 <[email protected]>; Viswas G - I30667 <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V6 2/2] pm80xx : Staggered spin up support.

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

As mentioned before - this should be a libsas or transport class policy, and not a module parameter hack in one driver. .

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 4 lines · On 04/08/2020 07:33,[email protected] wrote:

Hi Christoph,

Yes, It is better to be implemented in libsas. Since the out of box pm80xx driver has this support, we would like to push this for the time being. We will see how this can be moved to libsas.

Other libsas users may like this feature. And libsas does already support SATA spin-up hold events - as does pm8001 - but there's not really much to that in libsas.

Question: why have a module param to enable this feature? Why not solely rely on the seeprom spin-up interval, whereby a value of 0 means no staggered spin-up?
- Setting spin-up interval may increase the time for device discovery. Customer who has a valid spin up interval

  	pm8001_ha = sha->lldd_ha;
  	/* SAS_RE_INITIALIZATION not available in SPCv/ve */
  	if (pm8001_ha->chip_id == chip_8001)
  		PM8001_CHIP_DISP->sas_re_init_req(pm8001_ha);

drivers/scsi/pm8001/pm8001_sas.c +16 -2

-	for (i = 0; i < pm8001_ha->chip->n_phy; ++i)
-		PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+
+	if (pm8001_ha->pdev->device == 0x8001 ||
+		pm8001_ha->pdev->device == 0x8081 ||
+		(pm8001_ha->spinup_interval != 0)) {
+		for (i = 0; i < pm8001_ha->chip->n_phy; ++i)
+			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+	} else {
+		for (i = 0; i < pm8001_ha->chip->n_phy; ++i) {
+			spin_lock_irqsave(&pm8001_ha->lock, lock_flags);
+			pm8001_ha->phy_started = i;
+			pm8001_ha->scan_completion = &comp;
+			pm8001_ha->phystart_timedout = 1;
+			spin_unlock_irqrestore(&pm8001_ha->lock, lock_flags);
+			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+			wait_for_completion_timeout(&comp,
+				msecs_to_jiffies(500));

-----Original Message-----
From: John Garry [mailto:[email protected]]
Sent: Wednesday, August 5, 2020 9:56 PM
To: Deepak Ukey - I31172 <[email protected]>; [email protected]
Cc: [email protected]; Vasanthalakshmi Tharmarajan - I30664 <[email protected]>; Viswas G - I30667 <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V6 2/2] pm80xx : Staggered spin up support.

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 7 lines · On 04/08/2020 10:51, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 3 lines · On 04/08/2020 07:33,[email protected] wrote:

Hi Christoph,

Yes, It is better to be implemented in libsas. Since the out of box pm80xx

driver has this support, we would like to push this for the time being. We will see how this can be moved to libsas.

Quoted 2 lines

Quoted 1 line

Other libsas users may like this feature. And libsas does already support SATA

spin-up hold events - as does pm8001 - but there's not really much to that in libsas.

Quoted 2 lines

Question: why have a module param to enable this feature? Why not solely

rely on the seeprom spin-up interval, whereby a value of 0 means no staggered spin-up?

Quoted 2 lines

- Setting spin-up interval may increase the time for device discovery. Customer who has a valid spin up interval

Please use standard mailing list practices in replying.

- configured can still turn of this using module parameter. Or else, they have
to reflash the seeprom.

So another knob to control the driver. Anyway I think that it would be good for libsas to support this feature to benefit all users.

All the tunables from the LLDD can be fed to libsas, and libsas just needs to do what you do in pm8001_spinup_timedout() to callback to the LLDD to enable the SPINUP.

BTW, out of interest, what is this change for:

drivers/scsi/pm8001/pm8001_sas.c int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func, · line 271

Quoted 7 lines

  void pm8001_scan_start(struct Scsi_Host *shost)
  {
     int i;

drivers/scsi/pm8001/pm8001_sas.c +2 -0

+    unsigned long lock_flags;
     struct pm8001_hba_info *pm8001_ha;
     struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
+    DECLARE_COMPLETION(comp);

note: this should be the _ONSTACK() variant, and is it safe to even reuse in the
loop, below?
Thank you John for pointing this out. We will change this to ‘DECLARE_COMPLETION_ONSTACK’ and submit the updated patch. “
phy start” request is sequential and driver submit the new phy_start request after completing the current request. So I think it is safe to use that in a loop. Please share you thought.

Quoted 23 lines

     pm8001_ha = sha->lldd_ha;
     /* SAS_RE_INITIALIZATION not available in SPCv/ve */
     if (pm8001_ha->chip_id == chip_8001)
             PM8001_CHIP_DISP->sas_re_init_req(pm8001_ha);

drivers/scsi/pm8001/pm8001_sas.c +16 -2

-    for (i = 0; i < pm8001_ha->chip->n_phy; ++i)
-            PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+
+    if (pm8001_ha->pdev->device == 0x8001 ||
+            pm8001_ha->pdev->device == 0x8081 ||
+            (pm8001_ha->spinup_interval != 0)) {
+            for (i = 0; i < pm8001_ha->chip->n_phy; ++i)
+                    PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+    } else {
+            for (i = 0; i < pm8001_ha->chip->n_phy; ++i) {
+                    spin_lock_irqsave(&pm8001_ha->lock, lock_flags);
+                    pm8001_ha->phy_started = i;
+                    pm8001_ha->scan_completion = &comp;
+                    pm8001_ha->phystart_timedout = 1;
+                    spin_unlock_irqrestore(&pm8001_ha->lock, lock_flags);
+                    PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+                    wait_for_completion_timeout(&comp,
+                            msecs_to_jiffies(500));

drivers/scsi/pm8001/pm8001_sas.c int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func, · line 301

Quoted 10 lines

drivers/scsi/pm8001/pm8001_sas.c +9 -0

+                    if (pm8001_ha->phystart_timedout)
+                            PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
+                            "Timeout happened for phyid = %d\n", i));
+            }
+            spin_lock_irqsave(&pm8001_ha->lock, lock_flags);
+            pm8001_ha->phy_started = -1;
+            pm8001_ha->scan_completion = NULL;
+            spin_unlock_irqrestore(&pm8001_ha->lock, lock_flags);
+    }
  }

Thanks,
John