2023-04-18 19:02:45

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 0/6] scsi: pm80xx: Enhanced debug logs for HW events

This patch series enhances debug logs for pm80xx HW events, and provides
a minor fix in the case of a hard reset. The log enhancement involves
changing the log severity level to enable logging for HW events
which consequently help debug disk discovery issues.

1. Changed log severity level from MSG to EVENT for HW events.
Enhanced the HW event logs by adding the phyid.

2. Enabled INIT logging.

3. Log portid along with the PHY_UP event.

4. Print phyid and portid sent as part of device registration
request.

5. Log port state during HW events.

6. Update phy_state and phy_attached to correct values after
a hard reset.

Akshat Jain (5):
scsi: pm80xx: Enable init logging
scsi: pm80xx: Print port_id in hardware events
scsi: pm80xx: Log phyid and portid device register request
scsi: pm80xx: Log port state during HW event
scsi: pm80xx: Log some HW events by default

Changyuan Lyu (1):
scsi: pm80xx: Update PHY state after hard reset

drivers/scsi/pm8001/pm8001_init.c | 3 +-
drivers/scsi/pm8001/pm8001_sas.h | 1 +
drivers/scsi/pm8001/pm80xx_hwi.c | 126 +++++++++++++++++++-----------
3 files changed, 85 insertions(+), 45 deletions(-)

--
2.40.0.634.g4ca3ef3211-goog


2023-04-18 19:03:01

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 1/6] scsi: pm80xx: Log some HW events by default

From: Akshat Jain <[email protected]>

Log the following hw_event logs under EVENT log severity
to help debug disk issues:
HW_EVENT_LINK_ERR_INVALID_DWORD
HW_EVENT_LINK_ERR_DISPARITY_ERROR
HW_EVENT_LINK_ERR_CODE_VIOLATION
HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH
HW_EVENT_LINK_ERR_PHY_RESET_FAILED
HW_EVENT_INBOUND_CRC_ERROR
HW_EVENT_PHY_ERROR
HW_EVENT_SAS_PHY_UP
HW_EVENT_SATA_PHY_UP
HW_EVENT_SATA_SPINUP_HOLD
HW_EVENT_PHY_DOWN
HW_EVENT_PORT_INVALID
HW_EVENT_MALFUNCTION
HW_EVENT_PORT_RESET_TIMER_TMO
HW_EVENT_PORT_RECOVERY_TIMER_TMO
HW_EVENT_HARD_RESET_RECEIVED
HW_EVENT_ID_FRAME_TIMEOUT
HW_EVENT_PORT_RECOVER

Signed-off-by: Akshat Jain <[email protected]>
Signed-off-by: Pranav Prasad <[email protected]>
---
drivers/scsi/pm8001/pm8001_init.c | 3 +-
drivers/scsi/pm8001/pm8001_sas.h | 1 +
drivers/scsi/pm8001/pm80xx_hwi.c | 72 ++++++++++++++++++++-----------
3 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 7e589fe3e010..d8dc629c0efb 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -43,7 +43,8 @@
#include "pm8001_chips.h"
#include "pm80xx_hwi.h"

-static ulong logging_level = PM8001_FAIL_LOGGING | PM8001_IOERR_LOGGING;
+static ulong logging_level = PM8001_FAIL_LOGGING | PM8001_IOERR_LOGGING |
+ PM8001_EVENT_LOGGING;
module_param(logging_level, ulong, 0644);
MODULE_PARM_DESC(logging_level, " bits for enabling logging info.");

diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index dc1f4d958e03..953572fc0d9e 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -71,6 +71,7 @@
#define PM8001_DEV_LOGGING 0x80 /* development message logging */
#define PM8001_DEVIO_LOGGING 0x100 /* development io message logging */
#define PM8001_IOERR_LOGGING 0x200 /* development io err message logging */
+#define PM8001_EVENT_LOGGING 0x400 /* HW event logging */

#define pm8001_info(HBA, fmt, ...) \
pr_info("%s:: %s %d: " fmt, \
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 9584cadc4201..ce6a442d2418 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3425,26 +3425,31 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
switch (eventType) {

case HW_EVENT_SAS_PHY_UP:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PHY_START_STATUS\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_SAS_PHY_UP phyid:%#x\n", phy_id);
hw_event_sas_phy_up(pm8001_ha, piomb);
break;
case HW_EVENT_SATA_PHY_UP:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_SATA_PHY_UP\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_SATA_PHY_UP phyid:%#x\n", phy_id);
hw_event_sata_phy_up(pm8001_ha, piomb);
break;
case HW_EVENT_SATA_SPINUP_HOLD:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_SATA_SPINUP_HOLD\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_SATA_SPINUP_HOLD phyid:%#x\n", phy_id);
sas_notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD,
GFP_ATOMIC);
break;
case HW_EVENT_PHY_DOWN:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PHY_DOWN\n");
+ pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_DOWN phyid:%#x\n",
+ phy_id);
hw_event_phy_down(pm8001_ha, piomb);
phy->phy_attached = 0;
phy->phy_state = PHY_LINK_DISABLE;
break;
case HW_EVENT_PORT_INVALID:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_INVALID\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PORT_INVALID phyid:%#x\n", phy_id);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
@@ -3463,7 +3468,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
GFP_ATOMIC);
break;
case HW_EVENT_PHY_ERROR:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PHY_ERROR\n");
+ pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_ERROR phyid:%#x\n",
+ phy_id);
sas_phy_disconnected(&phy->sas_phy);
phy->phy_attached = 0;
sas_notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
@@ -3477,34 +3483,39 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_INVALID_DWORD:
- pm8001_dbg(pm8001_ha, MSG,
- "HW_EVENT_LINK_ERR_INVALID_DWORD\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_LINK_ERR_INVALID_DWORD phyid:%#x\n",
+ phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_INVALID_DWORD, port_id, phy_id, 0, 0);
break;
case HW_EVENT_LINK_ERR_DISPARITY_ERROR:
- pm8001_dbg(pm8001_ha, MSG,
- "HW_EVENT_LINK_ERR_DISPARITY_ERROR\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_LINK_ERR_DISPARITY_ERROR phyid:%#x\n",
+ phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_DISPARITY_ERROR,
port_id, phy_id, 0, 0);
break;
case HW_EVENT_LINK_ERR_CODE_VIOLATION:
- pm8001_dbg(pm8001_ha, MSG,
- "HW_EVENT_LINK_ERR_CODE_VIOLATION\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_LINK_ERR_CODE_VIOLATION phyid:%#x\n",
+ phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_CODE_VIOLATION,
port_id, phy_id, 0, 0);
break;
case HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH:
- pm8001_dbg(pm8001_ha, MSG,
- "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH phyid:%#x\n",
+ phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH,
port_id, phy_id, 0, 0);
break;
case HW_EVENT_MALFUNCTION:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_MALFUNCTION\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_MALFUNCTION phyid:%#x\n", phy_id);
break;
case HW_EVENT_BROADCAST_SES:
pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_BROADCAST_SES\n");
@@ -3515,25 +3526,29 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
GFP_ATOMIC);
break;
case HW_EVENT_INBOUND_CRC_ERROR:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_INBOUND_CRC_ERROR\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_INBOUND_CRC_ERROR phyid:%#x\n", phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_INBOUND_CRC_ERROR,
port_id, phy_id, 0, 0);
break;
case HW_EVENT_HARD_RESET_RECEIVED:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_HARD_RESET_RECEIVED\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_HARD_RESET_RECEIVED phyid:%#x\n", phy_id);
sas_notify_port_event(sas_phy, PORTE_HARD_RESET, GFP_ATOMIC);
break;
case HW_EVENT_ID_FRAME_TIMEOUT:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_ID_FRAME_TIMEOUT\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_ID_FRAME_TIMEOUT phyid:%#x\n", phy_id);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
GFP_ATOMIC);
break;
case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
- pm8001_dbg(pm8001_ha, MSG,
- "HW_EVENT_LINK_ERR_PHY_RESET_FAILED\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_LINK_ERR_PHY_RESET_FAILED phyid:%#x\n",
+ phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
port_id, phy_id, 0, 0);
@@ -3543,7 +3558,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
GFP_ATOMIC);
break;
case HW_EVENT_PORT_RESET_TIMER_TMO:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_RESET_TIMER_TMO\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x\n",
+ phy_id);
if (!pm8001_ha->phy[phy_id].reset_completion) {
pm80xx_hw_event_ack_req(pm8001_ha, 0, HW_EVENT_PHY_DOWN,
port_id, phy_id, 0, 0);
@@ -3560,8 +3577,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
break;
case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
- pm8001_dbg(pm8001_ha, MSG,
- "HW_EVENT_PORT_RECOVERY_TIMER_TMO\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PORT_RECOVERY_TIMER_TMO phyid:%#x\n",
+ phy_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_PORT_RECOVERY_TIMER_TMO,
port_id, phy_id, 0, 0);
@@ -3575,11 +3593,15 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
break;
case HW_EVENT_PORT_RECOVER:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_RECOVER\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PORT_RECOVER phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
hw_event_port_recover(pm8001_ha, piomb);
break;
case HW_EVENT_PORT_RESET_COMPLETE:
- pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_RESET_COMPLETE\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PORT_RESET_COMPLETE phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
if (pm8001_ha->phy[phy_id].reset_completion) {
pm8001_ha->phy[phy_id].port_reset_status =
PORT_RESET_SUCCESS;
--
2.40.0.634.g4ca3ef3211-goog

2023-04-18 19:03:34

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 3/6] scsi: pm80xx: Print port_id in HW events

From: Akshat Jain <[email protected]>

Log port_id and phy_id along with the PHY_UP event.

Signed-off-by: Akshat Jain <[email protected]>
Signed-off-by: Pranav Prasad <[email protected]>
---
drivers/scsi/pm8001/pm80xx_hwi.c | 58 ++++++++++++++++++--------------
1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 61c1bf3d98a0..c5bf65d0ad14 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3426,30 +3426,35 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)

case HW_EVENT_SAS_PHY_UP:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_SAS_PHY_UP phyid:%#x\n", phy_id);
+ "HW_EVENT_SAS_PHY_UP phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
hw_event_sas_phy_up(pm8001_ha, piomb);
break;
case HW_EVENT_SATA_PHY_UP:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_SATA_PHY_UP phyid:%#x\n", phy_id);
+ "HW_EVENT_SATA_PHY_UP phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
hw_event_sata_phy_up(pm8001_ha, piomb);
break;
case HW_EVENT_SATA_SPINUP_HOLD:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_SATA_SPINUP_HOLD phyid:%#x\n", phy_id);
+ "HW_EVENT_SATA_SPINUP_HOLD phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
sas_notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD,
GFP_ATOMIC);
break;
case HW_EVENT_PHY_DOWN:
- pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_DOWN phyid:%#x\n",
- phy_id);
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
hw_event_phy_down(pm8001_ha, piomb);
phy->phy_attached = 0;
phy->phy_state = PHY_LINK_DISABLE;
break;
case HW_EVENT_PORT_INVALID:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_PORT_INVALID phyid:%#x\n", phy_id);
+ "HW_EVENT_PORT_INVALID phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
@@ -3468,8 +3473,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
GFP_ATOMIC);
break;
case HW_EVENT_PHY_ERROR:
- pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_ERROR phyid:%#x\n",
- phy_id);
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_ERROR phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
sas_phy_disconnected(&phy->sas_phy);
phy->phy_attached = 0;
sas_notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
@@ -3484,31 +3490,31 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_LINK_ERR_INVALID_DWORD:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_LINK_ERR_INVALID_DWORD phyid:%#x\n",
- phy_id);
+ "HW_EVENT_LINK_ERR_INVALID_DWORD phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_INVALID_DWORD, port_id, phy_id, 0, 0);
break;
case HW_EVENT_LINK_ERR_DISPARITY_ERROR:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_LINK_ERR_DISPARITY_ERROR phyid:%#x\n",
- phy_id);
+ "HW_EVENT_LINK_ERR_DISPARITY_ERROR phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_DISPARITY_ERROR,
port_id, phy_id, 0, 0);
break;
case HW_EVENT_LINK_ERR_CODE_VIOLATION:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_LINK_ERR_CODE_VIOLATION phyid:%#x\n",
- phy_id);
+ "HW_EVENT_LINK_ERR_CODE_VIOLATION phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_CODE_VIOLATION,
port_id, phy_id, 0, 0);
break;
case HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH phyid:%#x\n",
- phy_id);
+ "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH,
port_id, phy_id, 0, 0);
@@ -3527,7 +3533,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_INBOUND_CRC_ERROR:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_INBOUND_CRC_ERROR phyid:%#x\n", phy_id);
+ "HW_EVENT_INBOUND_CRC_ERROR phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_INBOUND_CRC_ERROR,
port_id, phy_id, 0, 0);
@@ -3547,8 +3554,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_LINK_ERR_PHY_RESET_FAILED phyid:%#x\n",
- phy_id);
+ "HW_EVENT_LINK_ERR_PHY_RESET_FAILED phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
port_id, phy_id, 0, 0);
@@ -3559,8 +3566,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_PORT_RESET_TIMER_TMO:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x\n",
- phy_id);
+ "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
if (!pm8001_ha->phy[phy_id].reset_completion) {
pm80xx_hw_event_ack_req(pm8001_ha, 0, HW_EVENT_PHY_DOWN,
port_id, phy_id, 0, 0);
@@ -3578,8 +3585,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_PORT_RECOVERY_TIMER_TMO phyid:%#x\n",
- phy_id);
+ "HW_EVENT_PORT_RECOVERY_TIMER_TMO phyid:%#x port_id:%#x\n",
+ phy_id, port_id);
pm80xx_hw_event_ack_req(pm8001_ha, 0,
HW_EVENT_PORT_RECOVERY_TIMER_TMO,
port_id, phy_id, 0, 0);
@@ -3613,8 +3620,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
pm8001_dbg(pm8001_ha, MSG, "EVENT_BROADCAST_ASYNCH_EVENT\n");
break;
default:
- pm8001_dbg(pm8001_ha, DEVIO, "Unknown event type 0x%x\n",
- eventType);
+ pm8001_dbg(pm8001_ha, DEVIO,
+ "Unknown event portid:%d phyid:%d event:0x%x status:0x%x\n",
+ port_id, phy_id, eventType, status);
break;
}
return 0;
--
2.40.0.634.g4ca3ef3211-goog

2023-04-18 19:03:40

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 2/6] scsi: pm80xx: Enable init logging

From: Akshat Jain <[email protected]>

Enable init logging to debug drive discovery issues.

Signed-off-by: Akshat Jain <[email protected]>
Signed-off-by: Pranav Prasad <[email protected]>
---
drivers/scsi/pm8001/pm8001_init.c | 2 +-
drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index d8dc629c0efb..041cdc41af80 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -44,7 +44,7 @@
#include "pm80xx_hwi.h"

static ulong logging_level = PM8001_FAIL_LOGGING | PM8001_IOERR_LOGGING |
- PM8001_EVENT_LOGGING;
+ PM8001_EVENT_LOGGING | PM8001_INIT_LOGGING;
module_param(logging_level, ulong, 0644);
MODULE_PARM_DESC(logging_level, " bits for enabling logging info.");

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index ce6a442d2418..61c1bf3d98a0 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4837,7 +4837,7 @@ static void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
payload.tag = cpu_to_le32(tag);
payload.ppc_phyid =
cpu_to_le32(((operation & 0xF) << 8) | (phyid & 0xFF));
- pm8001_dbg(pm8001_ha, INIT,
+ pm8001_dbg(pm8001_ha, DISC,
" phy profile command for phy %x ,length is %d\n",
le32_to_cpu(payload.ppc_phyid), length);
for (i = length; i < (length + PHY_DWORD_LENGTH - 1); i++) {
--
2.40.0.634.g4ca3ef3211-goog

2023-04-18 19:03:54

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 4/6] scsi: pm80xx: Log phy_id and port_id in the device registration request

From: Akshat Jain <[email protected]>

Print phy_id and port_id sent as part of device registration
request.

Signed-off-by: Akshat Jain <[email protected]>
Signed-off-by: Pranav Prasad <[email protected]>
---
drivers/scsi/pm8001/pm80xx_hwi.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index c5bf65d0ad14..8571f6222eb8 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4756,6 +4756,9 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
memcpy(payload.sas_addr, pm8001_dev->sas_device->sas_addr,
SAS_ADDR_SIZE);

+ pm8001_dbg(pm8001_ha, INIT,
+ "register device req phy_id 0x%x port_id 0x%x\n", phy_id,
+ (port->port_id & 0xFF));
rc = pm8001_mpi_build_cmd(pm8001_ha, 0, opc, &payload,
sizeof(payload), 0);
if (rc)
--
2.40.0.634.g4ca3ef3211-goog

2023-04-18 19:04:56

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 5/6] scsi: pm80xx: Log port state during HW event

From: Akshat Jain <[email protected]>

Log port state during PHY_DOWN event to understand reasoning for PHY_DOWNs.

Signed-off-by: Akshat Jain <[email protected]>
Signed-off-by: Pranav Prasad <[email protected]>
---
drivers/scsi/pm8001/pm80xx_hwi.c | 43 ++++++++++++++++----------------
1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 8571f6222eb8..85908068b8d7 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3239,9 +3239,9 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
struct pm8001_port *port = &pm8001_ha->port[port_id];
struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
unsigned long flags;
- pm8001_dbg(pm8001_ha, DEVIO,
- "port id %d, phy id %d link_rate %d portstate 0x%x\n",
- port_id, phy_id, link_rate, portstate);
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_SATA_PHY_UP phyid:%#x port_id:%#x link_rate:%d portstate:%#x\n",
+ phy_id, port_id, link_rate, portstate);

phy->port = port;
port->port_id = port_id;
@@ -3291,10 +3291,14 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
phy->phy_attached = 0;
switch (portstate) {
case PORT_VALID:
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_VALID\n",
+ phy_id, port_id);
break;
case PORT_INVALID:
- pm8001_dbg(pm8001_ha, MSG, " PortInvalid portID %d\n",
- port_id);
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_INVALID\n",
+ phy_id, port_id);
pm8001_dbg(pm8001_ha, MSG,
" Last phy Down and port invalid\n");
if (port_sata) {
@@ -3306,18 +3310,21 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
sas_phy_disconnected(&phy->sas_phy);
break;
case PORT_IN_RESET:
- pm8001_dbg(pm8001_ha, MSG, " Port In Reset portID %d\n",
- port_id);
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_IN_RESET\n",
+ phy_id, port_id);
break;
case PORT_NOT_ESTABLISHED:
- pm8001_dbg(pm8001_ha, MSG,
- " Phy Down and PORT_NOT_ESTABLISHED\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_NOT_ESTABLISHED\n",
+ phy_id, port_id);
port->port_attached = 0;
break;
case PORT_LOSTCOMM:
- pm8001_dbg(pm8001_ha, MSG, " Phy Down and PORT_LOSTCOMM\n");
- pm8001_dbg(pm8001_ha, MSG,
- " Last phy Down and port invalid\n");
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_LOSTCOMM\n",
+ phy_id, port_id);
+ pm8001_dbg(pm8001_ha, MSG, " Last phy Down and port invalid\n");
if (port_sata) {
port->port_attached = 0;
phy->phy_type = 0;
@@ -3328,9 +3335,9 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
default:
port->port_attached = 0;
- pm8001_dbg(pm8001_ha, DEVIO,
- " Phy Down and(default) = 0x%x\n",
- portstate);
+ pm8001_dbg(pm8001_ha, EVENT,
+ "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate:%#x\n",
+ phy_id, port_id, portstate);
break;

}
@@ -3431,9 +3438,6 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
hw_event_sas_phy_up(pm8001_ha, piomb);
break;
case HW_EVENT_SATA_PHY_UP:
- pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_SATA_PHY_UP phyid:%#x port_id:%#x\n",
- phy_id, port_id);
hw_event_sata_phy_up(pm8001_ha, piomb);
break;
case HW_EVENT_SATA_SPINUP_HOLD:
@@ -3444,9 +3448,6 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
GFP_ATOMIC);
break;
case HW_EVENT_PHY_DOWN:
- pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x\n",
- phy_id, port_id);
hw_event_phy_down(pm8001_ha, piomb);
phy->phy_attached = 0;
phy->phy_state = PHY_LINK_DISABLE;
--
2.40.0.634.g4ca3ef3211-goog

2023-04-18 19:05:14

by Pranav Prasad

[permalink] [raw]
Subject: [PATCH 6/6] scsi: pm80xx: Update PHY state after hard reset

From: Changyuan Lyu <[email protected]>

Update phy_attached, phy_state, and port_state to correct values
after a hard rest. Without this patch, after a successful hard reset,
phy_attached is still 0, as a result, any following hard reset will
cause a PHY START to be issued first.

Signed-off-by: Changyuan Lyu <[email protected]>
Signed-off-by: Pranav Prasad <[email protected]>
---
drivers/scsi/pm8001/pm80xx_hwi.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 85908068b8d7..39a12ee94a72 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3417,6 +3417,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
u8 port_id = (u8)(lr_status_evt_portid & 0x000000FF);
u8 phy_id =
(u8)((phyid_npip_portstate & 0xFF0000) >> 16);
+ u8 portstate = (u8)(phyid_npip_portstate & 0x0000000F);
u16 eventType =
(u16)((lr_status_evt_portid & 0x00FFFF00) >> 8);
u8 status =
@@ -3449,7 +3450,6 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_PHY_DOWN:
hw_event_phy_down(pm8001_ha, piomb);
- phy->phy_attached = 0;
phy->phy_state = PHY_LINK_DISABLE;
break;
case HW_EVENT_PORT_INVALID:
@@ -3567,14 +3567,15 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_PORT_RESET_TIMER_TMO:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x port_id:%#x\n",
- phy_id, port_id);
+ "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x port_id:%#x portstate:%#x\n",
+ phy_id, port_id, portstate);
if (!pm8001_ha->phy[phy_id].reset_completion) {
pm80xx_hw_event_ack_req(pm8001_ha, 0, HW_EVENT_PHY_DOWN,
port_id, phy_id, 0, 0);
}
sas_phy_disconnected(sas_phy);
phy->phy_attached = 0;
+ port->port_state = portstate;
sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
GFP_ATOMIC);
if (pm8001_ha->phy[phy_id].reset_completion) {
@@ -3608,14 +3609,17 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
break;
case HW_EVENT_PORT_RESET_COMPLETE:
pm8001_dbg(pm8001_ha, EVENT,
- "HW_EVENT_PORT_RESET_COMPLETE phyid:%#x port_id:%#x\n",
- phy_id, port_id);
+ "HW_EVENT_PORT_RESET_COMPLETE phyid:%#x port_id:%#x portstate:%#x\n",
+ phy_id, port_id, portstate);
if (pm8001_ha->phy[phy_id].reset_completion) {
pm8001_ha->phy[phy_id].port_reset_status =
PORT_RESET_SUCCESS;
complete(pm8001_ha->phy[phy_id].reset_completion);
pm8001_ha->phy[phy_id].reset_completion = NULL;
}
+ phy->phy_attached = 1;
+ phy->phy_state = PHY_STATE_LINK_UP_SPCV;
+ port->port_state = portstate;
break;
case EVENT_BROADCAST_ASYNCH_EVENT:
pm8001_dbg(pm8001_ha, MSG, "EVENT_BROADCAST_ASYNCH_EVENT\n");
--
2.40.0.634.g4ca3ef3211-goog

2023-04-19 07:19:55

by Jinpu Wang

[permalink] [raw]
Subject: Re: [PATCH 1/6] scsi: pm80xx: Log some HW events by default

On Tue, Apr 18, 2023 at 9:01 PM Pranav Prasad <[email protected]> wrote:
>
> From: Akshat Jain <[email protected]>
>
> Log the following hw_event logs under EVENT log severity
> to help debug disk issues:
> HW_EVENT_LINK_ERR_INVALID_DWORD
> HW_EVENT_LINK_ERR_DISPARITY_ERROR
> HW_EVENT_LINK_ERR_CODE_VIOLATION
> HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH
> HW_EVENT_LINK_ERR_PHY_RESET_FAILED
> HW_EVENT_INBOUND_CRC_ERROR
> HW_EVENT_PHY_ERROR
> HW_EVENT_SAS_PHY_UP
> HW_EVENT_SATA_PHY_UP
> HW_EVENT_SATA_SPINUP_HOLD
> HW_EVENT_PHY_DOWN
> HW_EVENT_PORT_INVALID
> HW_EVENT_MALFUNCTION
> HW_EVENT_PORT_RESET_TIMER_TMO
> HW_EVENT_PORT_RECOVERY_TIMER_TMO
> HW_EVENT_HARD_RESET_RECEIVED
> HW_EVENT_ID_FRAME_TIMEOUT
> HW_EVENT_PORT_RECOVER
>
> Signed-off-by: Akshat Jain <[email protected]>
> Signed-off-by: Pranav Prasad <[email protected]>
Acked-by: Jack Wang <[email protected]>
> ---
> drivers/scsi/pm8001/pm8001_init.c | 3 +-
> drivers/scsi/pm8001/pm8001_sas.h | 1 +
> drivers/scsi/pm8001/pm80xx_hwi.c | 72 ++++++++++++++++++++-----------
> 3 files changed, 50 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 7e589fe3e010..d8dc629c0efb 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -43,7 +43,8 @@
> #include "pm8001_chips.h"
> #include "pm80xx_hwi.h"
>
> -static ulong logging_level = PM8001_FAIL_LOGGING | PM8001_IOERR_LOGGING;
> +static ulong logging_level = PM8001_FAIL_LOGGING | PM8001_IOERR_LOGGING |
> + PM8001_EVENT_LOGGING;
> module_param(logging_level, ulong, 0644);
> MODULE_PARM_DESC(logging_level, " bits for enabling logging info.");
>
> diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
> index dc1f4d958e03..953572fc0d9e 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.h
> +++ b/drivers/scsi/pm8001/pm8001_sas.h
> @@ -71,6 +71,7 @@
> #define PM8001_DEV_LOGGING 0x80 /* development message logging */
> #define PM8001_DEVIO_LOGGING 0x100 /* development io message logging */
> #define PM8001_IOERR_LOGGING 0x200 /* development io err message logging */
> +#define PM8001_EVENT_LOGGING 0x400 /* HW event logging */
>
> #define pm8001_info(HBA, fmt, ...) \
> pr_info("%s:: %s %d: " fmt, \
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 9584cadc4201..ce6a442d2418 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -3425,26 +3425,31 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> switch (eventType) {
>
> case HW_EVENT_SAS_PHY_UP:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PHY_START_STATUS\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_SAS_PHY_UP phyid:%#x\n", phy_id);
> hw_event_sas_phy_up(pm8001_ha, piomb);
> break;
> case HW_EVENT_SATA_PHY_UP:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_SATA_PHY_UP\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_SATA_PHY_UP phyid:%#x\n", phy_id);
> hw_event_sata_phy_up(pm8001_ha, piomb);
> break;
> case HW_EVENT_SATA_SPINUP_HOLD:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_SATA_SPINUP_HOLD\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_SATA_SPINUP_HOLD phyid:%#x\n", phy_id);
> sas_notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD,
> GFP_ATOMIC);
> break;
> case HW_EVENT_PHY_DOWN:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PHY_DOWN\n");
> + pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_DOWN phyid:%#x\n",
> + phy_id);
> hw_event_phy_down(pm8001_ha, piomb);
> phy->phy_attached = 0;
> phy->phy_state = PHY_LINK_DISABLE;
> break;
> case HW_EVENT_PORT_INVALID:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_INVALID\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PORT_INVALID phyid:%#x\n", phy_id);
> sas_phy_disconnected(sas_phy);
> phy->phy_attached = 0;
> sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
> @@ -3463,7 +3468,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> GFP_ATOMIC);
> break;
> case HW_EVENT_PHY_ERROR:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PHY_ERROR\n");
> + pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_ERROR phyid:%#x\n",
> + phy_id);
> sas_phy_disconnected(&phy->sas_phy);
> phy->phy_attached = 0;
> sas_notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
> @@ -3477,34 +3483,39 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> GFP_ATOMIC);
> break;
> case HW_EVENT_LINK_ERR_INVALID_DWORD:
> - pm8001_dbg(pm8001_ha, MSG,
> - "HW_EVENT_LINK_ERR_INVALID_DWORD\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_LINK_ERR_INVALID_DWORD phyid:%#x\n",
> + phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_INVALID_DWORD, port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_LINK_ERR_DISPARITY_ERROR:
> - pm8001_dbg(pm8001_ha, MSG,
> - "HW_EVENT_LINK_ERR_DISPARITY_ERROR\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_LINK_ERR_DISPARITY_ERROR phyid:%#x\n",
> + phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_DISPARITY_ERROR,
> port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_LINK_ERR_CODE_VIOLATION:
> - pm8001_dbg(pm8001_ha, MSG,
> - "HW_EVENT_LINK_ERR_CODE_VIOLATION\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_LINK_ERR_CODE_VIOLATION phyid:%#x\n",
> + phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_CODE_VIOLATION,
> port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH:
> - pm8001_dbg(pm8001_ha, MSG,
> - "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH phyid:%#x\n",
> + phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH,
> port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_MALFUNCTION:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_MALFUNCTION\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_MALFUNCTION phyid:%#x\n", phy_id);
> break;
> case HW_EVENT_BROADCAST_SES:
> pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_BROADCAST_SES\n");
> @@ -3515,25 +3526,29 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> GFP_ATOMIC);
> break;
> case HW_EVENT_INBOUND_CRC_ERROR:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_INBOUND_CRC_ERROR\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_INBOUND_CRC_ERROR phyid:%#x\n", phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_INBOUND_CRC_ERROR,
> port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_HARD_RESET_RECEIVED:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_HARD_RESET_RECEIVED\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_HARD_RESET_RECEIVED phyid:%#x\n", phy_id);
> sas_notify_port_event(sas_phy, PORTE_HARD_RESET, GFP_ATOMIC);
> break;
> case HW_EVENT_ID_FRAME_TIMEOUT:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_ID_FRAME_TIMEOUT\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_ID_FRAME_TIMEOUT phyid:%#x\n", phy_id);
> sas_phy_disconnected(sas_phy);
> phy->phy_attached = 0;
> sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
> GFP_ATOMIC);
> break;
> case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
> - pm8001_dbg(pm8001_ha, MSG,
> - "HW_EVENT_LINK_ERR_PHY_RESET_FAILED\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_LINK_ERR_PHY_RESET_FAILED phyid:%#x\n",
> + phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
> port_id, phy_id, 0, 0);
> @@ -3543,7 +3558,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> GFP_ATOMIC);
> break;
> case HW_EVENT_PORT_RESET_TIMER_TMO:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_RESET_TIMER_TMO\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x\n",
> + phy_id);
> if (!pm8001_ha->phy[phy_id].reset_completion) {
> pm80xx_hw_event_ack_req(pm8001_ha, 0, HW_EVENT_PHY_DOWN,
> port_id, phy_id, 0, 0);
> @@ -3560,8 +3577,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> break;
> case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
> - pm8001_dbg(pm8001_ha, MSG,
> - "HW_EVENT_PORT_RECOVERY_TIMER_TMO\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PORT_RECOVERY_TIMER_TMO phyid:%#x\n",
> + phy_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_PORT_RECOVERY_TIMER_TMO,
> port_id, phy_id, 0, 0);
> @@ -3575,11 +3593,15 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> break;
> case HW_EVENT_PORT_RECOVER:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_RECOVER\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PORT_RECOVER phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> hw_event_port_recover(pm8001_ha, piomb);
> break;
> case HW_EVENT_PORT_RESET_COMPLETE:
> - pm8001_dbg(pm8001_ha, MSG, "HW_EVENT_PORT_RESET_COMPLETE\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PORT_RESET_COMPLETE phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> if (pm8001_ha->phy[phy_id].reset_completion) {
> pm8001_ha->phy[phy_id].port_reset_status =
> PORT_RESET_SUCCESS;
> --
> 2.40.0.634.g4ca3ef3211-goog
>

2023-04-19 07:21:27

by Jinpu Wang

[permalink] [raw]
Subject: Re: [PATCH 2/6] scsi: pm80xx: Enable init logging

On Tue, Apr 18, 2023 at 9:01 PM Pranav Prasad <[email protected]> wrote:
>
> From: Akshat Jain <[email protected]>
>
> Enable init logging to debug drive discovery issues.
>
> Signed-off-by: Akshat Jain <[email protected]>
> Signed-off-by: Pranav Prasad <[email protected]>
Acked-by: Jack Wang <[email protected]>
> ---
> drivers/scsi/pm8001/pm8001_init.c | 2 +-
> drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index d8dc629c0efb..041cdc41af80 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -44,7 +44,7 @@
> #include "pm80xx_hwi.h"
>
> static ulong logging_level = PM8001_FAIL_LOGGING | PM8001_IOERR_LOGGING |
> - PM8001_EVENT_LOGGING;
> + PM8001_EVENT_LOGGING | PM8001_INIT_LOGGING;
> module_param(logging_level, ulong, 0644);
> MODULE_PARM_DESC(logging_level, " bits for enabling logging info.");
>
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index ce6a442d2418..61c1bf3d98a0 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -4837,7 +4837,7 @@ static void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
> payload.tag = cpu_to_le32(tag);
> payload.ppc_phyid =
> cpu_to_le32(((operation & 0xF) << 8) | (phyid & 0xFF));
> - pm8001_dbg(pm8001_ha, INIT,
> + pm8001_dbg(pm8001_ha, DISC,
> " phy profile command for phy %x ,length is %d\n",
> le32_to_cpu(payload.ppc_phyid), length);
> for (i = length; i < (length + PHY_DWORD_LENGTH - 1); i++) {
> --
> 2.40.0.634.g4ca3ef3211-goog
>

2023-04-19 07:22:57

by Jinpu Wang

[permalink] [raw]
Subject: Re: [PATCH 3/6] scsi: pm80xx: Print port_id in HW events

On Tue, Apr 18, 2023 at 9:01 PM Pranav Prasad <[email protected]> wrote:
>
> From: Akshat Jain <[email protected]>
>
> Log port_id and phy_id along with the PHY_UP event.
>
> Signed-off-by: Akshat Jain <[email protected]>
> Signed-off-by: Pranav Prasad <[email protected]>
Acked-by: Jack Wang <[email protected]>
> ---
> drivers/scsi/pm8001/pm80xx_hwi.c | 58 ++++++++++++++++++--------------
> 1 file changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 61c1bf3d98a0..c5bf65d0ad14 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -3426,30 +3426,35 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
>
> case HW_EVENT_SAS_PHY_UP:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_SAS_PHY_UP phyid:%#x\n", phy_id);
> + "HW_EVENT_SAS_PHY_UP phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> hw_event_sas_phy_up(pm8001_ha, piomb);
> break;
> case HW_EVENT_SATA_PHY_UP:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_SATA_PHY_UP phyid:%#x\n", phy_id);
> + "HW_EVENT_SATA_PHY_UP phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> hw_event_sata_phy_up(pm8001_ha, piomb);
> break;
> case HW_EVENT_SATA_SPINUP_HOLD:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_SATA_SPINUP_HOLD phyid:%#x\n", phy_id);
> + "HW_EVENT_SATA_SPINUP_HOLD phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> sas_notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD,
> GFP_ATOMIC);
> break;
> case HW_EVENT_PHY_DOWN:
> - pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_DOWN phyid:%#x\n",
> - phy_id);
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> hw_event_phy_down(pm8001_ha, piomb);
> phy->phy_attached = 0;
> phy->phy_state = PHY_LINK_DISABLE;
> break;
> case HW_EVENT_PORT_INVALID:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_PORT_INVALID phyid:%#x\n", phy_id);
> + "HW_EVENT_PORT_INVALID phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> sas_phy_disconnected(sas_phy);
> phy->phy_attached = 0;
> sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
> @@ -3468,8 +3473,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> GFP_ATOMIC);
> break;
> case HW_EVENT_PHY_ERROR:
> - pm8001_dbg(pm8001_ha, EVENT, "HW_EVENT_PHY_ERROR phyid:%#x\n",
> - phy_id);
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_ERROR phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> sas_phy_disconnected(&phy->sas_phy);
> phy->phy_attached = 0;
> sas_notify_phy_event(&phy->sas_phy, PHYE_OOB_ERROR, GFP_ATOMIC);
> @@ -3484,31 +3490,31 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_LINK_ERR_INVALID_DWORD:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_LINK_ERR_INVALID_DWORD phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_LINK_ERR_INVALID_DWORD phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_INVALID_DWORD, port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_LINK_ERR_DISPARITY_ERROR:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_LINK_ERR_DISPARITY_ERROR phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_LINK_ERR_DISPARITY_ERROR phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_DISPARITY_ERROR,
> port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_LINK_ERR_CODE_VIOLATION:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_LINK_ERR_CODE_VIOLATION phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_LINK_ERR_CODE_VIOLATION phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_CODE_VIOLATION,
> port_id, phy_id, 0, 0);
> break;
> case HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_LOSS_OF_DWORD_SYNCH,
> port_id, phy_id, 0, 0);
> @@ -3527,7 +3533,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_INBOUND_CRC_ERROR:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_INBOUND_CRC_ERROR phyid:%#x\n", phy_id);
> + "HW_EVENT_INBOUND_CRC_ERROR phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_INBOUND_CRC_ERROR,
> port_id, phy_id, 0, 0);
> @@ -3547,8 +3554,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_LINK_ERR_PHY_RESET_FAILED phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_LINK_ERR_PHY_RESET_FAILED phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_LINK_ERR_PHY_RESET_FAILED,
> port_id, phy_id, 0, 0);
> @@ -3559,8 +3566,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_PORT_RESET_TIMER_TMO:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> if (!pm8001_ha->phy[phy_id].reset_completion) {
> pm80xx_hw_event_ack_req(pm8001_ha, 0, HW_EVENT_PHY_DOWN,
> port_id, phy_id, 0, 0);
> @@ -3578,8 +3585,8 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_PORT_RECOVERY_TIMER_TMO phyid:%#x\n",
> - phy_id);
> + "HW_EVENT_PORT_RECOVERY_TIMER_TMO phyid:%#x port_id:%#x\n",
> + phy_id, port_id);
> pm80xx_hw_event_ack_req(pm8001_ha, 0,
> HW_EVENT_PORT_RECOVERY_TIMER_TMO,
> port_id, phy_id, 0, 0);
> @@ -3613,8 +3620,9 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> pm8001_dbg(pm8001_ha, MSG, "EVENT_BROADCAST_ASYNCH_EVENT\n");
> break;
> default:
> - pm8001_dbg(pm8001_ha, DEVIO, "Unknown event type 0x%x\n",
> - eventType);
> + pm8001_dbg(pm8001_ha, DEVIO,
> + "Unknown event portid:%d phyid:%d event:0x%x status:0x%x\n",
> + port_id, phy_id, eventType, status);
> break;
> }
> return 0;
> --
> 2.40.0.634.g4ca3ef3211-goog
>

2023-04-19 07:24:41

by Jinpu Wang

[permalink] [raw]
Subject: Re: [PATCH 4/6] scsi: pm80xx: Log phy_id and port_id in the device registration request

On Tue, Apr 18, 2023 at 9:01 PM Pranav Prasad <[email protected]> wrote:
>
> From: Akshat Jain <[email protected]>
>
> Print phy_id and port_id sent as part of device registration
> request.
>
> Signed-off-by: Akshat Jain <[email protected]>
> Signed-off-by: Pranav Prasad <[email protected]>
Acked-by: Jack Wang <[email protected]>
> ---
> drivers/scsi/pm8001/pm80xx_hwi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index c5bf65d0ad14..8571f6222eb8 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -4756,6 +4756,9 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
> memcpy(payload.sas_addr, pm8001_dev->sas_device->sas_addr,
> SAS_ADDR_SIZE);
>
> + pm8001_dbg(pm8001_ha, INIT,
> + "register device req phy_id 0x%x port_id 0x%x\n", phy_id,
> + (port->port_id & 0xFF));
> rc = pm8001_mpi_build_cmd(pm8001_ha, 0, opc, &payload,
> sizeof(payload), 0);
> if (rc)
> --
> 2.40.0.634.g4ca3ef3211-goog
>

2023-04-19 07:26:44

by Jinpu Wang

[permalink] [raw]
Subject: Re: [PATCH 5/6] scsi: pm80xx: Log port state during HW event

On Tue, Apr 18, 2023 at 9:01 PM Pranav Prasad <[email protected]> wrote:
>
> From: Akshat Jain <[email protected]>
>
> Log port state during PHY_DOWN event to understand reasoning for PHY_DOWNs.
>
> Signed-off-by: Akshat Jain <[email protected]>
> Signed-off-by: Pranav Prasad <[email protected]>
Acked-by: Jack Wang <[email protected]>
> ---
> drivers/scsi/pm8001/pm80xx_hwi.c | 43 ++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 8571f6222eb8..85908068b8d7 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -3239,9 +3239,9 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
> struct pm8001_port *port = &pm8001_ha->port[port_id];
> struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
> unsigned long flags;
> - pm8001_dbg(pm8001_ha, DEVIO,
> - "port id %d, phy id %d link_rate %d portstate 0x%x\n",
> - port_id, phy_id, link_rate, portstate);
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_SATA_PHY_UP phyid:%#x port_id:%#x link_rate:%d portstate:%#x\n",
> + phy_id, port_id, link_rate, portstate);
>
> phy->port = port;
> port->port_id = port_id;
> @@ -3291,10 +3291,14 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
> phy->phy_attached = 0;
> switch (portstate) {
> case PORT_VALID:
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_VALID\n",
> + phy_id, port_id);
> break;
> case PORT_INVALID:
> - pm8001_dbg(pm8001_ha, MSG, " PortInvalid portID %d\n",
> - port_id);
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_INVALID\n",
> + phy_id, port_id);
> pm8001_dbg(pm8001_ha, MSG,
> " Last phy Down and port invalid\n");
> if (port_sata) {
> @@ -3306,18 +3310,21 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
> sas_phy_disconnected(&phy->sas_phy);
> break;
> case PORT_IN_RESET:
> - pm8001_dbg(pm8001_ha, MSG, " Port In Reset portID %d\n",
> - port_id);
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_IN_RESET\n",
> + phy_id, port_id);
> break;
> case PORT_NOT_ESTABLISHED:
> - pm8001_dbg(pm8001_ha, MSG,
> - " Phy Down and PORT_NOT_ESTABLISHED\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_NOT_ESTABLISHED\n",
> + phy_id, port_id);
> port->port_attached = 0;
> break;
> case PORT_LOSTCOMM:
> - pm8001_dbg(pm8001_ha, MSG, " Phy Down and PORT_LOSTCOMM\n");
> - pm8001_dbg(pm8001_ha, MSG,
> - " Last phy Down and port invalid\n");
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate: PORT_LOSTCOMM\n",
> + phy_id, port_id);
> + pm8001_dbg(pm8001_ha, MSG, " Last phy Down and port invalid\n");
> if (port_sata) {
> port->port_attached = 0;
> phy->phy_type = 0;
> @@ -3328,9 +3335,9 @@ hw_event_phy_down(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> default:
> port->port_attached = 0;
> - pm8001_dbg(pm8001_ha, DEVIO,
> - " Phy Down and(default) = 0x%x\n",
> - portstate);
> + pm8001_dbg(pm8001_ha, EVENT,
> + "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x portstate:%#x\n",
> + phy_id, port_id, portstate);
> break;
>
> }
> @@ -3431,9 +3438,6 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> hw_event_sas_phy_up(pm8001_ha, piomb);
> break;
> case HW_EVENT_SATA_PHY_UP:
> - pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_SATA_PHY_UP phyid:%#x port_id:%#x\n",
> - phy_id, port_id);
> hw_event_sata_phy_up(pm8001_ha, piomb);
> break;
> case HW_EVENT_SATA_SPINUP_HOLD:
> @@ -3444,9 +3448,6 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> GFP_ATOMIC);
> break;
> case HW_EVENT_PHY_DOWN:
> - pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_PHY_DOWN phyid:%#x port_id:%#x\n",
> - phy_id, port_id);
> hw_event_phy_down(pm8001_ha, piomb);
> phy->phy_attached = 0;
> phy->phy_state = PHY_LINK_DISABLE;
> --
> 2.40.0.634.g4ca3ef3211-goog
>

2023-04-19 07:36:31

by Jinpu Wang

[permalink] [raw]
Subject: Re: [PATCH 6/6] scsi: pm80xx: Update PHY state after hard reset

On Tue, Apr 18, 2023 at 9:01 PM Pranav Prasad <[email protected]> wrote:
>
> From: Changyuan Lyu <[email protected]>
>
> Update phy_attached, phy_state, and port_state to correct values
> after a hard rest. Without this patch, after a successful hard reset,
typo, s/rest/reset
> phy_attached is still 0, as a result, any following hard reset will
> cause a PHY START to be issued first.
>
> Signed-off-by: Changyuan Lyu <[email protected]>
> Signed-off-by: Pranav Prasad <[email protected]>
Acked-by: Jack Wang <[email protected]>
> ---
> drivers/scsi/pm8001/pm80xx_hwi.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 85908068b8d7..39a12ee94a72 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -3417,6 +3417,7 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> u8 port_id = (u8)(lr_status_evt_portid & 0x000000FF);
> u8 phy_id =
> (u8)((phyid_npip_portstate & 0xFF0000) >> 16);
> + u8 portstate = (u8)(phyid_npip_portstate & 0x0000000F);
> u16 eventType =
> (u16)((lr_status_evt_portid & 0x00FFFF00) >> 8);
> u8 status =
> @@ -3449,7 +3450,6 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_PHY_DOWN:
> hw_event_phy_down(pm8001_ha, piomb);
> - phy->phy_attached = 0;
> phy->phy_state = PHY_LINK_DISABLE;
> break;
> case HW_EVENT_PORT_INVALID:
> @@ -3567,14 +3567,15 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_PORT_RESET_TIMER_TMO:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x port_id:%#x\n",
> - phy_id, port_id);
> + "HW_EVENT_PORT_RESET_TIMER_TMO phyid:%#x port_id:%#x portstate:%#x\n",
> + phy_id, port_id, portstate);
> if (!pm8001_ha->phy[phy_id].reset_completion) {
> pm80xx_hw_event_ack_req(pm8001_ha, 0, HW_EVENT_PHY_DOWN,
> port_id, phy_id, 0, 0);
> }
> sas_phy_disconnected(sas_phy);
> phy->phy_attached = 0;
> + port->port_state = portstate;
> sas_notify_port_event(sas_phy, PORTE_LINK_RESET_ERR,
> GFP_ATOMIC);
> if (pm8001_ha->phy[phy_id].reset_completion) {
> @@ -3608,14 +3609,17 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> break;
> case HW_EVENT_PORT_RESET_COMPLETE:
> pm8001_dbg(pm8001_ha, EVENT,
> - "HW_EVENT_PORT_RESET_COMPLETE phyid:%#x port_id:%#x\n",
> - phy_id, port_id);
> + "HW_EVENT_PORT_RESET_COMPLETE phyid:%#x port_id:%#x portstate:%#x\n",
> + phy_id, port_id, portstate);
> if (pm8001_ha->phy[phy_id].reset_completion) {
> pm8001_ha->phy[phy_id].port_reset_status =
> PORT_RESET_SUCCESS;
> complete(pm8001_ha->phy[phy_id].reset_completion);
> pm8001_ha->phy[phy_id].reset_completion = NULL;
> }
> + phy->phy_attached = 1;
> + phy->phy_state = PHY_STATE_LINK_UP_SPCV;
> + port->port_state = portstate;
> break;
> case EVENT_BROADCAST_ASYNCH_EVENT:
> pm8001_dbg(pm8001_ha, MSG, "EVENT_BROADCAST_ASYNCH_EVENT\n");
> --
> 2.40.0.634.g4ca3ef3211-goog
>

2023-05-06 22:36:20

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 0/6] scsi: pm80xx: Enhanced debug logs for HW events


Pranav,

> This patch series enhances debug logs for pm80xx HW events, and
> provides a minor fix in the case of a hard reset. The log enhancement
> involves changing the log severity level to enable logging for HW
> events which consequently help debug disk discovery issues.

Applied to 6.5/scsi-staging, thanks!

--
Martin K. Petersen Oracle Linux Engineering

2023-05-17 02:19:28

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 0/6] scsi: pm80xx: Enhanced debug logs for HW events

On Tue, 18 Apr 2023 19:00:55 +0000, Pranav Prasad wrote:

> This patch series enhances debug logs for pm80xx HW events, and provides
> a minor fix in the case of a hard reset. The log enhancement involves
> changing the log severity level to enable logging for HW events
> which consequently help debug disk discovery issues.
>
> 1. Changed log severity level from MSG to EVENT for HW events.
> Enhanced the HW event logs by adding the phyid.
>
> [...]

Applied to 6.5/scsi-queue, thanks!

[1/6] scsi: pm80xx: Log some HW events by default
https://git.kernel.org/mkp/scsi/c/b7d26c1d8c51
[2/6] scsi: pm80xx: Enable init logging
https://git.kernel.org/mkp/scsi/c/6a516506aad6
[3/6] scsi: pm80xx: Print port_id in HW events
https://git.kernel.org/mkp/scsi/c/a6cf6b8bd6e2
[4/6] scsi: pm80xx: Log phy_id and port_id in the device registration request
https://git.kernel.org/mkp/scsi/c/5f797120d853
[5/6] scsi: pm80xx: Log port state during HW event
https://git.kernel.org/mkp/scsi/c/d309422d7dc6
[6/6] scsi: pm80xx: Update PHY state after hard reset
https://git.kernel.org/mkp/scsi/c/3aa65f7e25ac

--
Martin K. Petersen Oracle Linux Engineering