Re: [PATCH] Revert "scsi: pm80xx: Do not use libsas port ID"

[Igor Pylypiv <ipylypiv@google.com>]

On Thu, Jul 17, 2025 at 06:56:07PM +0200, Niklas Cassel wrote:
> This reverts commit 0f630c58e31afb3dc2373bc1126b555f4b480bb2.
> 
> Commit 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID") causes
> drives behind a SAS enclosure (which is connected to one of the ports
> on the HBA) to no longer be detected.
> 
> Connecting the drives directly to the HBA still works. Thus, the commit
> only broke the detection of drives behind a SAS enclosure.
> 
> Reverting the commit makes the drives behind the SAS enclosure to be
> detected once again.
> 
> The commit log of the offending commit is quite vague, but mentions that:
> "Remove sas_find_local_port_id(). We can use pm8001_ha->phy[phy_id].port
> to get the port ID."
> 
> This assumption appears false, thus revert the offending commit.

Thanks for bisecting and reverting the commit, Niklas!

Let me review the changes and send a proper fix that takes into account
drives behind a SAS enclosure. I would appretiate if you could test that
new fix since I don't have a setup with a SAS enclosure.

Thank you,
Igor

> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Igor Pylypiv <ipylypiv@google.com>

> ---
> dmesg after the offending commit, in case anyone is interested:
> pm80xx0:: pm80xx_chip_phy_start_req 4641: PHY START REQ for phy_id 2
> pm80xx0:: mpi_phy_start_resp 3340: phy start resp status:0x0, phyid:0x2
> pm80xx0:: pm80xx_chip_phy_start_req 4641: PHY START REQ for phy_id 3
> pm80xx0:: mpi_phy_start_resp 3340: phy start resp status:0x0, phyid:0x3
> pm80xx0:: pm80xx_chip_phy_start_req 4641: PHY START REQ for phy_id 4
> pm80xx0:: mpi_phy_start_resp 3340: phy start resp status:0x0, phyid:0x4
> pm80xx0:: mpi_hw_event 3417: HW_EVENT_SAS_PHY_UP phyid:0x4 port_id:0x0
> sas: phy-11:4 added to port-11:0, phy_mask:0x10 (50015b21409aefbf)
> sas: DOING DISCOVERY on port 0, pid:215
> pm80xx0:: pm80xx_chip_reg_dev_req 4742: register device req phy_id 0x0 port_id 0x0
> pm80xx0:: pm8001_mpi_reg_resp 3309: register device status 0 phy_id 0x0 device_id 16385
> sas: executing SMP task failed:-19
> sas: RG to ex 50015b21409aefbf failed:0xffffffed
> pm80xx0:: pm8001_chip_dereg_dev_req 4226: unregister device device_id 16385
> sas: DONE DISCOVERY on port 0, pid:215, result:-19
> pm80xx0:: pm80xx_chip_phy_start_req 4641: PHY START REQ for phy_id 5
> pm80xx0:: mpi_phy_start_resp 3340: phy start resp status:0x0, phyid:0x5
> pm80xx0:: mpi_hw_event 3417: HW_EVENT_SAS_PHY_UP phyid:0x5 port_id:0x0
> sas: phy5 matched wide port0
> sas: phy-11:5 added to port-11:0, phy_mask:0x30 (50015b21409aefbf)
> sas: DOING DISCOVERY on port 0, pid:277
> pm80xx0:: pm80xx_chip_reg_dev_req 4742: register device req phy_id 0x0 port_id 0x0
> pm80xx0:: pm8001_mpi_reg_resp 3309: register device status 0 phy_id 0x0 device_id 16386
> sas: executing SMP task failed:-19
> sas: RG to ex 50015b21409aefbf failed:0xffffffed
> pm80xx0:: pm8001_chip_dereg_dev_req 4226: unregister device device_id 16386
> sas: DONE DISCOVERY on port 0, pid:277, result:-19
> pm80xx0:: pm80xx_chip_phy_start_req 4641: PHY START REQ for phy_id 6
> pm80xx0:: mpi_phy_start_resp 3340: phy start resp status:0x0, phyid:0x6
> pm80xx0:: mpi_hw_event 3417: HW_EVENT_SAS_PHY_UP phyid:0x6 port_id:0x0
> sas: phy6 matched wide port0
> sas: phy-11:6 added to port-11:0, phy_mask:0x70 (50015b21409aefbf)
> sas: DOING DISCOVERY on port 0, pid:277
> pm80xx0:: pm80xx_chip_reg_dev_req 4742: register device req phy_id 0x0 port_id 0x0
> pm80xx0:: pm8001_mpi_reg_resp 3309: register device status 0 phy_id 0x0 device_id 16387
> sas: executing SMP task failed:-19
> sas: RG to ex 50015b21409aefbf failed:0xffffffed
> pm80xx0:: pm8001_chip_dereg_dev_req 4226: unregister device device_id 16387
> sas: DONE DISCOVERY on port 0, pid:277, result:-19
> pm80xx0:: pm80xx_chip_phy_start_req 4641: PHY START REQ for phy_id 7
> pm80xx0:: mpi_phy_start_resp 3340: phy start resp status:0x0, phyid:0x7
> pm80xx0:: mpi_hw_event 3417: HW_EVENT_SAS_PHY_UP phyid:0x7 port_id:0x0
> sas: phy7 matched wide port0
> sas: phy-11:7 added to port-11:0, phy_mask:0xf0 (50015b21409aefbf)
> sas: DOING DISCOVERY on port 0, pid:215
> pm80xx0:: pm80xx_chip_reg_dev_req 4742: register device req phy_id 0x0 port_id 0x0
> pm80xx0:: pm8001_mpi_reg_resp 3309: register device status 0 phy_id 0x0 device_id 16388
> sas: executing SMP task failed:-19
> sas: RG to ex 50015b21409aefbf failed:0xffffffed
> pm80xx0:: pm8001_chip_dereg_dev_req 4226: unregister device device_id 16388
> sas: DONE DISCOVERY on port 0, pid:215, result:-19
> 
>  drivers/scsi/pm8001/pm8001_sas.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index f7067878b34f..8cfdfb77d9b0 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -431,6 +431,23 @@ static int pm8001_task_prep_ssp(struct pm8001_hba_info *pm8001_ha,
>  	return PM8001_CHIP_DISP->ssp_io_req(pm8001_ha, ccb);
>  }
>  
> + /* Find the local port id that's attached to this device */
> +static int sas_find_local_port_id(struct domain_device *dev)
> +{
> +	struct domain_device *pdev = dev->parent;
> +
> +	/* Directly attached device */
> +	if (!pdev)
> +		return dev->port->id;
> +	while (pdev) {
> +		struct domain_device *pdev_p = pdev->parent;
> +		if (!pdev_p)
> +			return pdev->port->id;
> +		pdev = pdev->parent;
> +	}
> +	return 0;
> +}
> +
>  #define DEV_IS_GONE(pm8001_dev)	\
>  	((!pm8001_dev || (pm8001_dev->dev_type == SAS_PHY_UNUSED)))
>  
> @@ -503,10 +520,10 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags)
>  	spin_lock_irqsave(&pm8001_ha->lock, flags);
>  
>  	pm8001_dev = dev->lldd_dev;
> -	port = pm8001_ha->phy[pm8001_dev->attached_phy].port;
> +	port = &pm8001_ha->port[sas_find_local_port_id(dev)];
>  
>  	if (!internal_abort &&
> -	    (DEV_IS_GONE(pm8001_dev) || !port || !port->port_attached)) {
> +	    (DEV_IS_GONE(pm8001_dev) || !port->port_attached)) {
>  		ts->resp = SAS_TASK_UNDELIVERED;
>  		ts->stat = SAS_PHY_DOWN;
>  		if (sas_protocol_ata(task_proto)) {
> -- 
> 2.50.1
>