[PATCH v2 0/4] pm80xx updates

[PATCH v2 0/4] pm80xx updates

[Ajish Koshy]

This patchset includes bugfixes for pm80xx driver

Changes from v1 to v2:
	- For fix incorrect port value patch
		- More detailed commit message now
		  mentioning problem and fix.
	- For fix lockup due to commit patch
		- Commit message includes little detail
		  of previous commit.
		- Added 'Fixes' tag.
		  

Ajish Koshy (3):
  scsi: pm80xx: fix incorrect port value when registering a device
  scsi: pm80xx: fix lockup due to commit <1f02beff224e>
  scsi: pm80xx: fix memory leak during rmmod

Viswas G (1):
  scsi: pm80xx: Corrected Inbound and Outbound queue logging

 drivers/scsi/pm8001/pm8001_ctl.c  |  6 ++--
 drivers/scsi/pm8001/pm8001_hwi.c  |  7 +++-
 drivers/scsi/pm8001/pm8001_init.c | 12 +++++++
 drivers/scsi/pm8001/pm8001_sas.c  | 15 ++++++++
 drivers/scsi/pm8001/pm8001_sas.h  |  6 ++--
 drivers/scsi/pm8001/pm80xx_hwi.c  | 60 +++++++++++++++++++++++++------
 6 files changed, 91 insertions(+), 15 deletions(-)

-- 
2.27.0

[PATCH v2 1/4] scsi: pm80xx: fix incorrect port value when registering a device

[Ajish Koshy]

During phyup event, firmware gives the phy_id and port_id
and driver is supposed to use the same during device handle
registration. Earlier, driver was using port id value from
libsas during device handle registration and at times, it is
different from firmware assigned port id. This will lead to
wrong device registration and eventually we would not see
those drives.

Fix is to use firmware assigned portid during device
registration.

Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
Signed-off-by: Viswas G <Viswas.G@microchip.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c  |  7 ++++++-
 drivers/scsi/pm8001/pm8001_init.c |  1 +
 drivers/scsi/pm8001/pm8001_sas.c  | 15 +++++++++++++++
 drivers/scsi/pm8001/pm8001_sas.h  |  2 ++
 drivers/scsi/pm8001/pm80xx_hwi.c  |  7 ++++++-
 5 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 63690508313b..c9ecddd0d719 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -3358,6 +3358,8 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
 	unsigned long flags;
 	u8 deviceType = pPayload->sas_identify.dev_type;
+	phy->port = port;
+	port->port_id = port_id;
 	port->port_state =  portstate;
 	phy->phy_state = PHY_STATE_LINK_UP_SPC;
 	pm8001_dbg(pm8001_ha, MSG,
@@ -3434,6 +3436,8 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	unsigned long flags;
 	pm8001_dbg(pm8001_ha, DEVIO, "HW_EVENT_SATA_PHY_UP port id = %d, phy id = %d\n",
 		   port_id, phy_id);
+	phy->port = port;
+	port->port_id = port_id;
 	port->port_state =  portstate;
 	phy->phy_state = PHY_STATE_LINK_UP_SPC;
 	port->port_attached = 1;
@@ -4460,6 +4464,7 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	u16 ITNT = 2000;
 	struct domain_device *dev = pm8001_dev->sas_device;
 	struct domain_device *parent_dev = dev->parent;
+	struct pm8001_port *port = dev->port->lldd_port;
 	circularQ = &pm8001_ha->inbnd_q_tbl[0];
 
 	memset(&payload, 0, sizeof(payload));
@@ -4488,7 +4493,7 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	linkrate = (pm8001_dev->sas_device->linkrate < dev->port->linkrate) ?
 			pm8001_dev->sas_device->linkrate : dev->port->linkrate;
 	payload.phyid_portid =
-		cpu_to_le32(((pm8001_dev->sas_device->port->id) & 0x0F) |
+		cpu_to_le32(((port->port_id) & 0x0F) |
 		((phy_id & 0x0F) << 4));
 	payload.dtype_dlr_retry = cpu_to_le32((retryFlag & 0x01) |
 		((linkrate & 0x0F) * 0x1000000) |
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 47db7e0beae6..613455a3e686 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -128,6 +128,7 @@ static struct sas_domain_function_template pm8001_transport_ops = {
 	.lldd_I_T_nexus_reset   = pm8001_I_T_nexus_reset,
 	.lldd_lu_reset		= pm8001_lu_reset,
 	.lldd_query_task	= pm8001_query_task,
+	.lldd_port_formed	= pm8001_port_formed,
 };
 
 /**
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 32e60f0c3b14..83e73009db5c 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -1355,3 +1355,18 @@ int pm8001_clear_task_set(struct domain_device *dev, u8 *lun)
 	tmf_task.tmf = TMF_CLEAR_TASK_SET;
 	return pm8001_issue_ssp_tmf(dev, lun, &tmf_task);
 }
+
+void pm8001_port_formed(struct asd_sas_phy *sas_phy)
+{
+	struct sas_ha_struct *sas_ha = sas_phy->ha;
+	struct pm8001_hba_info *pm8001_ha = sas_ha->lldd_ha;
+	struct pm8001_phy *phy = sas_phy->lldd_phy;
+	struct asd_sas_port *sas_port = sas_phy->port;
+	struct pm8001_port *port = phy->port;
+
+	if (!sas_port) {
+		pm8001_dbg(pm8001_ha, FAIL, "Received null port\n");
+		return;
+	}
+	sas_port->lldd_port = port;
+}
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 62d08b535a4b..1a016a421280 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -230,6 +230,7 @@ struct pm8001_port {
 	u8			port_attached;
 	u16			wide_port_phymap;
 	u8			port_state;
+	u8			port_id;
 	struct list_head	list;
 };
 
@@ -651,6 +652,7 @@ int pm8001_lu_reset(struct domain_device *dev, u8 *lun);
 int pm8001_I_T_nexus_reset(struct domain_device *dev);
 int pm8001_I_T_nexus_event_handler(struct domain_device *dev);
 int pm8001_query_task(struct sas_task *task);
+void pm8001_port_formed(struct asd_sas_phy *sas_phy);
 void pm8001_open_reject_retry(
 	struct pm8001_hba_info *pm8001_ha,
 	struct sas_task *task_to_close,
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 6ffe17b849ae..cec932f830b8 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3299,6 +3299,8 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
 	unsigned long flags;
 	u8 deviceType = pPayload->sas_identify.dev_type;
+	phy->port = port;
+	port->port_id = port_id;
 	port->port_state = portstate;
 	port->wide_port_phymap |= (1U << phy_id);
 	phy->phy_state = PHY_STATE_LINK_UP_SPCV;
@@ -3380,6 +3382,8 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		   "port id %d, phy id %d link_rate %d portstate 0x%x\n",
 		   port_id, phy_id, link_rate, portstate);
 
+	phy->port = port;
+	port->port_id = port_id;
 	port->port_state = portstate;
 	phy->phy_state = PHY_STATE_LINK_UP_SPCV;
 	port->port_attached = 1;
@@ -4808,6 +4812,7 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	u16 ITNT = 2000;
 	struct domain_device *dev = pm8001_dev->sas_device;
 	struct domain_device *parent_dev = dev->parent;
+	struct pm8001_port *port = dev->port->lldd_port;
 	circularQ = &pm8001_ha->inbnd_q_tbl[0];
 
 	memset(&payload, 0, sizeof(payload));
@@ -4840,7 +4845,7 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 			pm8001_dev->sas_device->linkrate : dev->port->linkrate;
 
 	payload.phyid_portid =
-		cpu_to_le32(((pm8001_dev->sas_device->port->id) & 0xFF) |
+		cpu_to_le32(((port->port_id) & 0xFF) |
 		((phy_id & 0xFF) << 8));
 
 	payload.dtype_dlr_mcn_ir_retry = cpu_to_le32((retryFlag & 0x01) |
-- 
2.27.0

[PATCH v2 2/4] scsi: pm80xx: fix lockup due to commit <1f02beff224e>

[Ajish Koshy]

Commit 1f02beff224e ("scsi: pm80xx: Remove global lock from
outbound queue processing") introduced a lock per outbound
queue, where the driver before that was using a global lock
for all outbound queues. While processing the IO responses
and events, driver takes the outbound queue spinlock and
later it is supposed to release the same spin lock in
pm8001_ccb_task_free_done() before calling command done().
Since the older code was using a global lock,
pm8001_ccb_task_free_done() was also releasing the global
spin lock. With the commit <1f02beff224e>,
pm8001_ccb_task_free_done() remains the same and it was
still using the global lock.

So when driver completes a SATA command,the global spinlock
will be in a locked state.
mpi_sata_completion()->spin_lock(&pm8001_ha->lock);

Later when driver gets a scsi command for SATA drive,
pm8001_task_exec() tries to acquire the global lock and leads
to lockup crash.

Fixes: 1f02beff224e ("scsi: pm80xx: Remove global lock from outbound queue processing")
Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
Signed-off-by: Viswas G <Viswas.G@microchip.com>
---
 drivers/scsi/pm8001/pm8001_sas.h |  3 +-
 drivers/scsi/pm8001/pm80xx_hwi.c | 53 ++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 1a016a421280..3274d88a9ccc 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -458,6 +458,7 @@ struct outbound_queue_table {
 	__le32			producer_index;
 	u32			consumer_idx;
 	spinlock_t		oq_lock;
+	unsigned long		lock_flags;
 };
 struct pm8001_hba_memspace {
 	void __iomem  		*memvirtaddr;
@@ -740,9 +741,7 @@ pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,
 {
 	pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb_idx);
 	smp_mb(); /*in order to force CPU ordering*/
-	spin_unlock(&pm8001_ha->lock);
 	task->task_done(task);
-	spin_lock(&pm8001_ha->lock);
 }
 
 #endif
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index cec932f830b8..1ae2f5c6042c 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2379,7 +2379,8 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
 
 /*See the comments for mpi_ssp_completion */
 static void
-mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
+mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
+		struct outbound_queue_table *circularQ, void *piomb)
 {
 	struct sas_task *t;
 	struct pm8001_ccb_info *ccb;
@@ -2616,7 +2617,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 				IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
 			ts->resp = SAS_TASK_UNDELIVERED;
 			ts->stat = SAS_QUEUE_FULL;
+			spin_unlock_irqrestore(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+			spin_lock_irqsave(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			return;
 		}
 		break;
@@ -2632,7 +2637,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 				IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
 			ts->resp = SAS_TASK_UNDELIVERED;
 			ts->stat = SAS_QUEUE_FULL;
+			spin_unlock_irqrestore(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+			spin_lock_irqsave(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			return;
 		}
 		break;
@@ -2656,7 +2665,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 				IO_OPEN_CNX_ERROR_STP_RESOURCES_BUSY);
 			ts->resp = SAS_TASK_UNDELIVERED;
 			ts->stat = SAS_QUEUE_FULL;
+			spin_unlock_irqrestore(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+			spin_lock_irqsave(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			return;
 		}
 		break;
@@ -2727,7 +2740,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 					IO_DS_NON_OPERATIONAL);
 			ts->resp = SAS_TASK_UNDELIVERED;
 			ts->stat = SAS_QUEUE_FULL;
+			spin_unlock_irqrestore(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+			spin_lock_irqsave(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			return;
 		}
 		break;
@@ -2747,7 +2764,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 					IO_DS_IN_ERROR);
 			ts->resp = SAS_TASK_UNDELIVERED;
 			ts->stat = SAS_QUEUE_FULL;
+			spin_unlock_irqrestore(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+			spin_lock_irqsave(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			return;
 		}
 		break;
@@ -2785,12 +2806,17 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 	} else {
 		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irqrestore(&circularQ->oq_lock,
+				circularQ->lock_flags);
 		pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+		spin_lock_irqsave(&circularQ->oq_lock,
+				circularQ->lock_flags);
 	}
 }
 
 /*See the comments for mpi_ssp_completion */
-static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
+static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
+		struct outbound_queue_table *circularQ, void *piomb)
 {
 	struct sas_task *t;
 	struct task_status_struct *ts;
@@ -2890,7 +2916,11 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
 				IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
 			ts->resp = SAS_TASK_COMPLETE;
 			ts->stat = SAS_QUEUE_FULL;
+			spin_unlock_irqrestore(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+			spin_lock_irqsave(&circularQ->oq_lock,
+					circularQ->lock_flags);
 			return;
 		}
 		break;
@@ -3002,7 +3032,11 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 	} else {
 		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irqrestore(&circularQ->oq_lock,
+				circularQ->lock_flags);
 		pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
+		spin_lock_irqsave(&circularQ->oq_lock,
+				circularQ->lock_flags);
 	}
 }
 
@@ -3906,7 +3940,8 @@ static int ssp_coalesced_comp_resp(struct pm8001_hba_info *pm8001_ha,
  * @pm8001_ha: our hba card information
  * @piomb: IO message buffer
  */
-static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
+static void process_one_iomb(struct pm8001_hba_info *pm8001_ha,
+		struct outbound_queue_table *circularQ, void *piomb)
 {
 	__le32 pHeader = *(__le32 *)piomb;
 	u32 opc = (u32)((le32_to_cpu(pHeader)) & 0xFFF);
@@ -3948,11 +3983,11 @@ static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		break;
 	case OPC_OUB_SATA_COMP:
 		pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SATA_COMP\n");
-		mpi_sata_completion(pm8001_ha, piomb);
+		mpi_sata_completion(pm8001_ha, circularQ, piomb);
 		break;
 	case OPC_OUB_SATA_EVENT:
 		pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SATA_EVENT\n");
-		mpi_sata_event(pm8001_ha, piomb);
+		mpi_sata_event(pm8001_ha, circularQ, piomb);
 		break;
 	case OPC_OUB_SSP_EVENT:
 		pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SSP_EVENT\n");
@@ -4121,7 +4156,6 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
 	void *pMsg1 = NULL;
 	u8 bc;
 	u32 ret = MPI_IO_STATUS_FAIL;
-	unsigned long flags;
 	u32 regval;
 
 	if (vec == (pm8001_ha->max_q_num - 1)) {
@@ -4138,7 +4172,7 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
 		}
 	}
 	circularQ = &pm8001_ha->outbnd_q_tbl[vec];
-	spin_lock_irqsave(&circularQ->oq_lock, flags);
+	spin_lock_irqsave(&circularQ->oq_lock, circularQ->lock_flags);
 	do {
 		/* spurious interrupt during setup if kexec-ing and
 		 * driver doing a doorbell access w/ the pre-kexec oq
@@ -4149,7 +4183,8 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
 		ret = pm8001_mpi_msg_consume(pm8001_ha, circularQ, &pMsg1, &bc);
 		if (MPI_IO_STATUS_SUCCESS == ret) {
 			/* process the outbound message */
-			process_one_iomb(pm8001_ha, (void *)(pMsg1 - 4));
+			process_one_iomb(pm8001_ha, circularQ,
+						(void *)(pMsg1 - 4));
 			/* free the message from the outbound circular buffer */
 			pm8001_mpi_msg_free_set(pm8001_ha, pMsg1,
 							circularQ, bc);
@@ -4164,7 +4199,7 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
 				break;
 		}
 	} while (1);
-	spin_unlock_irqrestore(&circularQ->oq_lock, flags);
+	spin_unlock_irqrestore(&circularQ->oq_lock, circularQ->lock_flags);
 	return ret;
 }
 
-- 
2.27.0

[PATCH v2 3/4] scsi: pm80xx: Corrected Inbound and Outbound queue logging

[Ajish Koshy]

From: Viswas G <Viswas.G@microchip.com>

Corrected inbound queue and outbound queue size in 'ib_log'
and 'ob_log' sysfs entries.

Signed-off-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/scsi/pm8001/pm8001_ctl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index ec05c42e8ee6..b25e447aa3bd 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -409,6 +409,7 @@ static ssize_t pm8001_ctl_ib_queue_log_show(struct device *cdev,
 	char *str = buf;
 	int start = 0;
 	u32 ib_offset = pm8001_ha->ib_offset;
+	u32 queue_size = pm8001_ha->max_q_num * PM8001_MPI_QUEUE * 128;
 #define IB_MEMMAP(c)	\
 		(*(u32 *)((u8 *)pm8001_ha->	\
 		memoryMap.region[ib_offset].virt_ptr +	\
@@ -419,7 +420,7 @@ static ssize_t pm8001_ctl_ib_queue_log_show(struct device *cdev,
 		start = start + 4;
 	}
 	pm8001_ha->evtlog_ib_offset += SYSFS_OFFSET;
-	if (((pm8001_ha->evtlog_ib_offset) % (PM80XX_IB_OB_QUEUE_SIZE)) == 0)
+	if (((pm8001_ha->evtlog_ib_offset) % queue_size) == 0)
 		pm8001_ha->evtlog_ib_offset = 0;
 
 	return str - buf;
@@ -445,6 +446,7 @@ static ssize_t pm8001_ctl_ob_queue_log_show(struct device *cdev,
 	char *str = buf;
 	int start = 0;
 	u32 ob_offset = pm8001_ha->ob_offset;
+	u32 queue_size = pm8001_ha->max_q_num * PM8001_MPI_QUEUE * 128;
 #define OB_MEMMAP(c)	\
 		(*(u32 *)((u8 *)pm8001_ha->	\
 		memoryMap.region[ob_offset].virt_ptr +	\
@@ -455,7 +457,7 @@ static ssize_t pm8001_ctl_ob_queue_log_show(struct device *cdev,
 		start = start + 4;
 	}
 	pm8001_ha->evtlog_ob_offset += SYSFS_OFFSET;
-	if (((pm8001_ha->evtlog_ob_offset) % (PM80XX_IB_OB_QUEUE_SIZE)) == 0)
+	if (((pm8001_ha->evtlog_ob_offset) % queue_size) == 0)
 		pm8001_ha->evtlog_ob_offset = 0;
 
 	return str - buf;
-- 
2.27.0

[PATCH v2 4/4] scsi: pm80xx: fix memory leak during rmmod

[Ajish Koshy]

Driver fails to release memory allocated. This will lead
to memory leak during driver removal.

Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
Signed-off-by: Viswas G <Viswas.G@microchip.com>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
---
 drivers/scsi/pm8001/pm8001_init.c | 11 +++++++++++
 drivers/scsi/pm8001/pm8001_sas.h  |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 613455a3e686..7082fecf7ce8 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1199,6 +1199,7 @@ pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha, struct Scsi_Host *shost,
 		goto err_out;
 
 	/* Memory region for ccb_info*/
+	pm8001_ha->ccb_count = ccb_count;
 	pm8001_ha->ccb_info =
 		kcalloc(ccb_count, sizeof(struct pm8001_ccb_info), GFP_KERNEL);
 	if (!pm8001_ha->ccb_info) {
@@ -1260,6 +1261,16 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
 			tasklet_kill(&pm8001_ha->tasklet[j]);
 #endif
 	scsi_host_put(pm8001_ha->shost);
+
+	for (i = 0; i < pm8001_ha->ccb_count; i++) {
+		dma_free_coherent(&pm8001_ha->pdev->dev,
+			sizeof(struct pm8001_prd) * PM8001_MAX_DMA_SG,
+			pm8001_ha->ccb_info[i].buf_prd,
+			pm8001_ha->ccb_info[i].ccb_dma_handle);
+	}
+	kfree(pm8001_ha->ccb_info);
+	kfree(pm8001_ha->devices);
+
 	pm8001_free(pm8001_ha);
 	kfree(sha->sas_phy);
 	kfree(sha->sas_port);
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 3274d88a9ccc..7e999768bfd2 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -518,6 +518,7 @@ struct pm8001_hba_info {
 	u32			iomb_size; /* SPC and SPCV IOMB size */
 	struct pm8001_device	*devices;
 	struct pm8001_ccb_info	*ccb_info;
+	u32			ccb_count;
 #ifdef PM8001_USE_MSIX
 	int			number_of_intr;/*will be used in remove()*/
 	char			intr_drvname[PM8001_MAX_MSIX_VEC]
-- 
2.27.0

Re: [PATCH v2 1/4] scsi: pm80xx: fix incorrect port value when registering a device

[Jinpu Wang]

On Mon, Sep 6, 2021 at 6:09 PM Ajish Koshy <Ajish.Koshy@microchip.com> wrote:
>
> During phyup event, firmware gives the phy_id and port_id
> and driver is supposed to use the same during device handle
> registration. Earlier, driver was using port id value from
> libsas during device handle registration and at times, it is
> different from firmware assigned port id. This will lead to
> wrong device registration and eventually we would not see
> those drives.
>
> Fix is to use firmware assigned portid during device
> registration.
>
> Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
> Signed-off-by: Viswas G <Viswas.G@microchip.com>
Thanks for the update.
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_hwi.c  |  7 ++++++-
>  drivers/scsi/pm8001/pm8001_init.c |  1 +
>  drivers/scsi/pm8001/pm8001_sas.c  | 15 +++++++++++++++
>  drivers/scsi/pm8001/pm8001_sas.h  |  2 ++
>  drivers/scsi/pm8001/pm80xx_hwi.c  |  7 ++++++-
>  5 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index 63690508313b..c9ecddd0d719 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -3358,6 +3358,8 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
>         struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
>         unsigned long flags;
>         u8 deviceType = pPayload->sas_identify.dev_type;
> +       phy->port = port;
> +       port->port_id = port_id;
>         port->port_state =  portstate;
>         phy->phy_state = PHY_STATE_LINK_UP_SPC;
>         pm8001_dbg(pm8001_ha, MSG,
> @@ -3434,6 +3436,8 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
>         unsigned long flags;
>         pm8001_dbg(pm8001_ha, DEVIO, "HW_EVENT_SATA_PHY_UP port id = %d, phy id = %d\n",
>                    port_id, phy_id);
> +       phy->port = port;
> +       port->port_id = port_id;
>         port->port_state =  portstate;
>         phy->phy_state = PHY_STATE_LINK_UP_SPC;
>         port->port_attached = 1;
> @@ -4460,6 +4464,7 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
>         u16 ITNT = 2000;
>         struct domain_device *dev = pm8001_dev->sas_device;
>         struct domain_device *parent_dev = dev->parent;
> +       struct pm8001_port *port = dev->port->lldd_port;
>         circularQ = &pm8001_ha->inbnd_q_tbl[0];
>
>         memset(&payload, 0, sizeof(payload));
> @@ -4488,7 +4493,7 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
>         linkrate = (pm8001_dev->sas_device->linkrate < dev->port->linkrate) ?
>                         pm8001_dev->sas_device->linkrate : dev->port->linkrate;
>         payload.phyid_portid =
> -               cpu_to_le32(((pm8001_dev->sas_device->port->id) & 0x0F) |
> +               cpu_to_le32(((port->port_id) & 0x0F) |
>                 ((phy_id & 0x0F) << 4));
>         payload.dtype_dlr_retry = cpu_to_le32((retryFlag & 0x01) |
>                 ((linkrate & 0x0F) * 0x1000000) |
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 47db7e0beae6..613455a3e686 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -128,6 +128,7 @@ static struct sas_domain_function_template pm8001_transport_ops = {
>         .lldd_I_T_nexus_reset   = pm8001_I_T_nexus_reset,
>         .lldd_lu_reset          = pm8001_lu_reset,
>         .lldd_query_task        = pm8001_query_task,
> +       .lldd_port_formed       = pm8001_port_formed,
>  };
>
>  /**
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index 32e60f0c3b14..83e73009db5c 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -1355,3 +1355,18 @@ int pm8001_clear_task_set(struct domain_device *dev, u8 *lun)
>         tmf_task.tmf = TMF_CLEAR_TASK_SET;
>         return pm8001_issue_ssp_tmf(dev, lun, &tmf_task);
>  }
> +
> +void pm8001_port_formed(struct asd_sas_phy *sas_phy)
> +{
> +       struct sas_ha_struct *sas_ha = sas_phy->ha;
> +       struct pm8001_hba_info *pm8001_ha = sas_ha->lldd_ha;
> +       struct pm8001_phy *phy = sas_phy->lldd_phy;
> +       struct asd_sas_port *sas_port = sas_phy->port;
> +       struct pm8001_port *port = phy->port;
> +
> +       if (!sas_port) {
> +               pm8001_dbg(pm8001_ha, FAIL, "Received null port\n");
> +               return;
> +       }
> +       sas_port->lldd_port = port;
> +}
> diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
> index 62d08b535a4b..1a016a421280 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.h
> +++ b/drivers/scsi/pm8001/pm8001_sas.h
> @@ -230,6 +230,7 @@ struct pm8001_port {
>         u8                      port_attached;
>         u16                     wide_port_phymap;
>         u8                      port_state;
> +       u8                      port_id;
>         struct list_head        list;
>  };
>
> @@ -651,6 +652,7 @@ int pm8001_lu_reset(struct domain_device *dev, u8 *lun);
>  int pm8001_I_T_nexus_reset(struct domain_device *dev);
>  int pm8001_I_T_nexus_event_handler(struct domain_device *dev);
>  int pm8001_query_task(struct sas_task *task);
> +void pm8001_port_formed(struct asd_sas_phy *sas_phy);
>  void pm8001_open_reject_retry(
>         struct pm8001_hba_info *pm8001_ha,
>         struct sas_task *task_to_close,
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 6ffe17b849ae..cec932f830b8 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -3299,6 +3299,8 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
>         struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
>         unsigned long flags;
>         u8 deviceType = pPayload->sas_identify.dev_type;
> +       phy->port = port;
> +       port->port_id = port_id;
>         port->port_state = portstate;
>         port->wide_port_phymap |= (1U << phy_id);
>         phy->phy_state = PHY_STATE_LINK_UP_SPCV;
> @@ -3380,6 +3382,8 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                    "port id %d, phy id %d link_rate %d portstate 0x%x\n",
>                    port_id, phy_id, link_rate, portstate);
>
> +       phy->port = port;
> +       port->port_id = port_id;
>         port->port_state = portstate;
>         phy->phy_state = PHY_STATE_LINK_UP_SPCV;
>         port->port_attached = 1;
> @@ -4808,6 +4812,7 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
>         u16 ITNT = 2000;
>         struct domain_device *dev = pm8001_dev->sas_device;
>         struct domain_device *parent_dev = dev->parent;
> +       struct pm8001_port *port = dev->port->lldd_port;
>         circularQ = &pm8001_ha->inbnd_q_tbl[0];
>
>         memset(&payload, 0, sizeof(payload));
> @@ -4840,7 +4845,7 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
>                         pm8001_dev->sas_device->linkrate : dev->port->linkrate;
>
>         payload.phyid_portid =
> -               cpu_to_le32(((pm8001_dev->sas_device->port->id) & 0xFF) |
> +               cpu_to_le32(((port->port_id) & 0xFF) |
>                 ((phy_id & 0xFF) << 8));
>
>         payload.dtype_dlr_mcn_ir_retry = cpu_to_le32((retryFlag & 0x01) |
> --
> 2.27.0
>

Re: [PATCH v2 2/4] scsi: pm80xx: fix lockup due to commit <1f02beff224e>

[Jinpu Wang]

On Mon, Sep 6, 2021 at 6:09 PM Ajish Koshy <Ajish.Koshy@microchip.com> wrote:
>
> Commit 1f02beff224e ("scsi: pm80xx: Remove global lock from
> outbound queue processing") introduced a lock per outbound
> queue, where the driver before that was using a global lock
> for all outbound queues. While processing the IO responses
> and events, driver takes the outbound queue spinlock and
> later it is supposed to release the same spin lock in
> pm8001_ccb_task_free_done() before calling command done().
> Since the older code was using a global lock,
> pm8001_ccb_task_free_done() was also releasing the global
> spin lock. With the commit <1f02beff224e>,
> pm8001_ccb_task_free_done() remains the same and it was
> still using the global lock.
>
> So when driver completes a SATA command,the global spinlock
> will be in a locked state.
> mpi_sata_completion()->spin_lock(&pm8001_ha->lock);
>
> Later when driver gets a scsi command for SATA drive,
> pm8001_task_exec() tries to acquire the global lock and leads
> to lockup crash.
>
> Fixes: 1f02beff224e ("scsi: pm80xx: Remove global lock from outbound queue processing")
> Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
> Signed-off-by: Viswas G <Viswas.G@microchip.com>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_sas.h |  3 +-
>  drivers/scsi/pm8001/pm80xx_hwi.c | 53 ++++++++++++++++++++++++++------
>  2 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
> index 1a016a421280..3274d88a9ccc 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.h
> +++ b/drivers/scsi/pm8001/pm8001_sas.h
> @@ -458,6 +458,7 @@ struct outbound_queue_table {
>         __le32                  producer_index;
>         u32                     consumer_idx;
>         spinlock_t              oq_lock;
> +       unsigned long           lock_flags;
>  };
>  struct pm8001_hba_memspace {
>         void __iomem            *memvirtaddr;
> @@ -740,9 +741,7 @@ pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,
>  {
>         pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb_idx);
>         smp_mb(); /*in order to force CPU ordering*/
> -       spin_unlock(&pm8001_ha->lock);
>         task->task_done(task);
> -       spin_lock(&pm8001_ha->lock);
>  }
>
>  #endif
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index cec932f830b8..1ae2f5c6042c 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -2379,7 +2379,8 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
>
>  /*See the comments for mpi_ssp_completion */
>  static void
> -mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
> +mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
> +               struct outbound_queue_table *circularQ, void *piomb)
>  {
>         struct sas_task *t;
>         struct pm8001_ccb_info *ccb;
> @@ -2616,7 +2617,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                                 IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
>                         ts->resp = SAS_TASK_UNDELIVERED;
>                         ts->stat = SAS_QUEUE_FULL;
> +                       spin_unlock_irqrestore(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +                       spin_lock_irqsave(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         return;
>                 }
>                 break;
> @@ -2632,7 +2637,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                                 IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
>                         ts->resp = SAS_TASK_UNDELIVERED;
>                         ts->stat = SAS_QUEUE_FULL;
> +                       spin_unlock_irqrestore(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +                       spin_lock_irqsave(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         return;
>                 }
>                 break;
> @@ -2656,7 +2665,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                                 IO_OPEN_CNX_ERROR_STP_RESOURCES_BUSY);
>                         ts->resp = SAS_TASK_UNDELIVERED;
>                         ts->stat = SAS_QUEUE_FULL;
> +                       spin_unlock_irqrestore(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +                       spin_lock_irqsave(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         return;
>                 }
>                 break;
> @@ -2727,7 +2740,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                                         IO_DS_NON_OPERATIONAL);
>                         ts->resp = SAS_TASK_UNDELIVERED;
>                         ts->stat = SAS_QUEUE_FULL;
> +                       spin_unlock_irqrestore(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +                       spin_lock_irqsave(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         return;
>                 }
>                 break;
> @@ -2747,7 +2764,11 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                                         IO_DS_IN_ERROR);
>                         ts->resp = SAS_TASK_UNDELIVERED;
>                         ts->stat = SAS_QUEUE_FULL;
> +                       spin_unlock_irqrestore(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +                       spin_lock_irqsave(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         return;
>                 }
>                 break;
> @@ -2785,12 +2806,17 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                 pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
>         } else {
>                 spin_unlock_irqrestore(&t->task_state_lock, flags);
> +               spin_unlock_irqrestore(&circularQ->oq_lock,
> +                               circularQ->lock_flags);
>                 pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +               spin_lock_irqsave(&circularQ->oq_lock,
> +                               circularQ->lock_flags);
>         }
>  }
>
>  /*See the comments for mpi_ssp_completion */
> -static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> +static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
> +               struct outbound_queue_table *circularQ, void *piomb)
>  {
>         struct sas_task *t;
>         struct task_status_struct *ts;
> @@ -2890,7 +2916,11 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                                 IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
>                         ts->resp = SAS_TASK_COMPLETE;
>                         ts->stat = SAS_QUEUE_FULL;
> +                       spin_unlock_irqrestore(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +                       spin_lock_irqsave(&circularQ->oq_lock,
> +                                       circularQ->lock_flags);
>                         return;
>                 }
>                 break;
> @@ -3002,7 +3032,11 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                 pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
>         } else {
>                 spin_unlock_irqrestore(&t->task_state_lock, flags);
> +               spin_unlock_irqrestore(&circularQ->oq_lock,
> +                               circularQ->lock_flags);
>                 pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
> +               spin_lock_irqsave(&circularQ->oq_lock,
> +                               circularQ->lock_flags);
>         }
>  }
>
> @@ -3906,7 +3940,8 @@ static int ssp_coalesced_comp_resp(struct pm8001_hba_info *pm8001_ha,
>   * @pm8001_ha: our hba card information
>   * @piomb: IO message buffer
>   */
> -static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
> +static void process_one_iomb(struct pm8001_hba_info *pm8001_ha,
> +               struct outbound_queue_table *circularQ, void *piomb)
>  {
>         __le32 pHeader = *(__le32 *)piomb;
>         u32 opc = (u32)((le32_to_cpu(pHeader)) & 0xFFF);
> @@ -3948,11 +3983,11 @@ static void process_one_iomb(struct pm8001_hba_info *pm8001_ha, void *piomb)
>                 break;
>         case OPC_OUB_SATA_COMP:
>                 pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SATA_COMP\n");
> -               mpi_sata_completion(pm8001_ha, piomb);
> +               mpi_sata_completion(pm8001_ha, circularQ, piomb);
>                 break;
>         case OPC_OUB_SATA_EVENT:
>                 pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SATA_EVENT\n");
> -               mpi_sata_event(pm8001_ha, piomb);
> +               mpi_sata_event(pm8001_ha, circularQ, piomb);
>                 break;
>         case OPC_OUB_SSP_EVENT:
>                 pm8001_dbg(pm8001_ha, MSG, "OPC_OUB_SSP_EVENT\n");
> @@ -4121,7 +4156,6 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
>         void *pMsg1 = NULL;
>         u8 bc;
>         u32 ret = MPI_IO_STATUS_FAIL;
> -       unsigned long flags;
>         u32 regval;
>
>         if (vec == (pm8001_ha->max_q_num - 1)) {
> @@ -4138,7 +4172,7 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
>                 }
>         }
>         circularQ = &pm8001_ha->outbnd_q_tbl[vec];
> -       spin_lock_irqsave(&circularQ->oq_lock, flags);
> +       spin_lock_irqsave(&circularQ->oq_lock, circularQ->lock_flags);
>         do {
>                 /* spurious interrupt during setup if kexec-ing and
>                  * driver doing a doorbell access w/ the pre-kexec oq
> @@ -4149,7 +4183,8 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
>                 ret = pm8001_mpi_msg_consume(pm8001_ha, circularQ, &pMsg1, &bc);
>                 if (MPI_IO_STATUS_SUCCESS == ret) {
>                         /* process the outbound message */
> -                       process_one_iomb(pm8001_ha, (void *)(pMsg1 - 4));
> +                       process_one_iomb(pm8001_ha, circularQ,
> +                                               (void *)(pMsg1 - 4));
>                         /* free the message from the outbound circular buffer */
>                         pm8001_mpi_msg_free_set(pm8001_ha, pMsg1,
>                                                         circularQ, bc);
> @@ -4164,7 +4199,7 @@ static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
>                                 break;
>                 }
>         } while (1);
> -       spin_unlock_irqrestore(&circularQ->oq_lock, flags);
> +       spin_unlock_irqrestore(&circularQ->oq_lock, circularQ->lock_flags);
>         return ret;
>  }
>
> --
> 2.27.0
>

Re: [PATCH v2 0/4] pm80xx updates

[Martin K. Petersen]

Ajish,

> This patchset includes bugfixes for pm80xx driver
>
> Ajish Koshy (3):
>   scsi: pm80xx: fix incorrect port value when registering a device
>   scsi: pm80xx: fix lockup due to commit <1f02beff224e>
>   scsi: pm80xx: fix memory leak during rmmod
>
> Viswas G (1):
>   scsi: pm80xx: Corrected Inbound and Outbound queue logging

Applied to 5.16/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 0/4] pm80xx updates

[Martin K. Petersen]

On Mon, 6 Sep 2021 22:34:00 +0530, Ajish Koshy wrote:

> This patchset includes bugfixes for pm80xx driver
> 
> Changes from v1 to v2:
> 	- For fix incorrect port value patch
> 		- More detailed commit message now
> 		  mentioning problem and fix.
> 	- For fix lockup due to commit patch
> 		- Commit message includes little detail
> 		  of previous commit.
> 		- Added 'Fixes' tag.
> 
> [...]

Applied to 5.16/scsi-queue, thanks!

[1/4] scsi: pm80xx: fix incorrect port value when registering a device
      https://git.kernel.org/mkp/scsi/c/08d0a992131a
[2/4] scsi: pm80xx: fix lockup due to commit <1f02beff224e>
      https://git.kernel.org/mkp/scsi/c/b27a40534ef7
[3/4] scsi: pm80xx: Corrected Inbound and Outbound queue logging
      https://git.kernel.org/mkp/scsi/c/c29737d03c74
[4/4] scsi: pm80xx: fix memory leak during rmmod
      https://git.kernel.org/mkp/scsi/c/51e6ed83bb4a

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH V2 0/4] pm80xx updates

[Viswas G]

From: Viswas G <Viswas.G@microchip.com>

This patch set include some bug fixes for pm80xx driver.

Changes from v1:
	- Improved commit message for "make mpi_build_cmd locking
	  consistent.patch"

Viswas G (1):
  pm80xx: make running_req atomic.

akshatzen (1):
  pm80xx: Avoid busywait in FW ready check

peter chang (1):
  pm80xx: make mpi_build_cmd locking consistent

yuuzheng (1):
  pm80xx: make pm8001_mpi_set_nvmd_resp free of race condition

 drivers/scsi/pm8001/pm8001_hwi.c  |  86 +++++++++++++++++++------
 drivers/scsi/pm8001/pm8001_init.c |   2 +-
 drivers/scsi/pm8001/pm8001_sas.c  |  11 ++--
 drivers/scsi/pm8001/pm8001_sas.h  |   2 +-
 drivers/scsi/pm8001/pm80xx_hwi.c  | 130 ++++++++++++++++++++++++++++++--------
 drivers/scsi/pm8001/pm80xx_hwi.h  |   6 ++
 6 files changed, 185 insertions(+), 52 deletions(-)

-- 
2.16.3

Re: [PATCH V2 0/4] pm80xx updates.

[Martin K. Petersen]

On Mon, 5 Oct 2020 20:20:07 +0530, Viswas G wrote:

> Changes from v1:
> 	- Improved commit messages.
> 	- Fixed compiler warning for
> 		 "Increase the number of outstanding IO supported" patch
> 
> Viswas G (4):
>   pm80xx: Increase number of supported queues.
>   pm80xx: Remove DMA memory allocation for ccb and device structures.
>   pm80xx: Increase the number of outstanding IO supported to 1024.
>   pm80xx: Driver version update
> 
> [...]

Applied to 5.10/scsi-queue, thanks!

[1/4] scsi: pm80xx: Increase number of supported queues
      https://git.kernel.org/mkp/scsi/c/05c6c029a44d
[2/4] scsi: pm80xx: Remove DMA memory allocation for ccb and device structures
      https://git.kernel.org/mkp/scsi/c/27bc43bd7c42
[3/4] scsi: pm80xx: Increase the number of outstanding I/O supported to 1024
      https://git.kernel.org/mkp/scsi/c/5a141315ed7c
[4/4] scsi: pm80xx: Driver version update
      https://git.kernel.org/mkp/scsi/c/39a45d538dba

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH V2 0/4] pm80xx updates.

[Martin K. Petersen]

Hello Doug,

> As for "imperative mood", I believe there is no such thing in English
> grammar.

"Imperative mood" is how it is described in submitting-patches.rst. I
deliberately use the same term in my feedback emails to make it easy for
the recipient to locate the relevant section in that file.

If you believe that the official process document should be amended,
feel free to reach out to linux-doc.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH V2 0/4] pm80xx updates.

[Bart Van Assche]

On 2020-10-06 20:03, Douglas Gilbert wrote:
> Is the "imperative mood" something in Danish or German grammar?

Have you already encountered this?
https://en.wikipedia.org/wiki/Imperative_mood#English

Thanks,

Bart.

Re: [PATCH V2 0/4] pm80xx updates.

[Douglas Gilbert]

On 2020-10-06 10:06 p.m., Martin K. Petersen wrote:
> 
> Viswas,
> 
>> Changes from v1:
>> 	- Improved commit messages.
>> 	- Fixed compiler warning for
>> 		 "Increase the number of outstanding IO supported" patch
> 
> Applied to 5.10/scsi-staging.
> 
> In the future please run checkpatch and make sure that the commit
> messages are using imperative mood (see
> Documentation/process/submitting-patches.rst, section 2).

Get thee to a nunnery! [W. Shakespeare; translation: "fuck off"]
Now that is imperative.

As for "imperative mood", I believe there is no such thing in English
grammar. My mother taught grammar and I studied French and Latin at
school. Markus Elfring objected to my:
     [PATCH] lib/scatterlist: Fix memory leak in sgl_alloc_order()  ***

with the same "imperative mood" line. In English, including British
(i.e. "international") English taught in south Asia, that is the
_imperative_ . Basically if you can stick "You" in front of the
verb at the start of the sentence and the sense is the same, then
it is the imperative.

Is the "imperative mood" something in Danish or German grammar?

Doug Gilbert


*** That patch was ack-ed by Bart (the culprit) and as far as I
     know hasn't gone any further. My sgl-to-sgl copy, compare
     and sgl_memset await that bug being sorted.

Re: [PATCH V2 0/4] pm80xx updates.

[Martin K. Petersen]

Viswas,

> Changes from v1:
> 	- Improved commit messages.
> 	- Fixed compiler warning for 
> 		 "Increase the number of outstanding IO supported" patch 

Applied to 5.10/scsi-staging.

In the future please run checkpatch and make sure that the commit
messages are using imperative mood (see
Documentation/process/submitting-patches.rst, section 2).

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH V2 0/4] pm80xx updates.

[Viswas G]

From: Viswas G <Viswas.G@microchip.com>

Changes from v1:
	- Improved commit messages.
	- Fixed compiler warning for 
		 "Increase the number of outstanding IO supported" patch 

Viswas G (4):
  pm80xx: Increase number of supported queues.
  pm80xx: Remove DMA memory allocation for ccb and device structures.
  pm80xx: Increase the number of outstanding IO supported to 1024.
  pm80xx: Driver version update

 drivers/scsi/pm8001/pm8001_ctl.c  |   6 +-
 drivers/scsi/pm8001/pm8001_defs.h |  27 +++--
 drivers/scsi/pm8001/pm8001_hwi.c  |  38 +++----
 drivers/scsi/pm8001/pm8001_init.c | 221 ++++++++++++++++++++++++++------------
 drivers/scsi/pm8001/pm8001_sas.h  |  15 ++-
 drivers/scsi/pm8001/pm80xx_hwi.c  | 109 ++++++++++---------
 6 files changed, 254 insertions(+), 162 deletions(-)

-- 
2.16.3