[PATCH 0/5] scsi: pm80xx: Fix expander support

[PATCH 0/5] scsi: pm80xx: Fix expander support

[Niklas Cassel]

Hello all,

Some recent patches broke expander support for the pm80xx driver.

The first two patches in this series makes sure that expanders work with
this driver again.

It also fixes a bug in pm8001_abort_task() that was found through code
review.

The final two patches make the driver more robust, so that is less likely
that the expander support will break again in the future.

Please test and review.


Kind regards,
Niklas


Niklas Cassel (5):
  scsi: pm80xx: Restore support for expanders
  scsi: pm80xx: Fix array-index-out-of-of-bounds on rmmod
  scsi: pm80xx: Add helper function to get the local phy id
  scsi: pm80xx: Fix pm8001_abort_task() for chip_8006 when using an
    expander
  scsi: pm80xx: Use pm80xx_get_local_phy_id() to access phy array

 drivers/scsi/pm8001/pm8001_hwi.c |  8 +++-----
 drivers/scsi/pm8001/pm8001_sas.c | 34 +++++++++++++++++++++++++-------
 drivers/scsi/pm8001/pm8001_sas.h |  1 +
 drivers/scsi/pm8001/pm80xx_hwi.c |  7 ++-----
 4 files changed, 33 insertions(+), 17 deletions(-)

-- 
2.50.1

[PATCH 1/5] scsi: pm80xx: Restore support for expanders

[Niklas Cassel]

Commit 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID") broke
support for expanders. After the commit, devices behind an expander are no
longer detected.

Simply reverting the commit restores support for devices behind an
expander.

Instead of reverting the commit (and reintroducing a helper to get the
port), get the port directly from the lldd_port pointer in struct
asd_sas_port.

Suggested-by: Igor Pylypiv <ipylypiv@google.com>
Fixes: 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/pm8001/pm8001_sas.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index f7067878b34f..753c09363cbb 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -477,7 +477,7 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags)
 	struct pm8001_device *pm8001_dev = dev->lldd_dev;
 	bool internal_abort = sas_is_internal_abort(task);
 	struct pm8001_hba_info *pm8001_ha;
-	struct pm8001_port *port = NULL;
+	struct pm8001_port *port;
 	struct pm8001_ccb_info *ccb;
 	unsigned long flags;
 	u32 n_elem = 0;
@@ -502,8 +502,7 @@ 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 = dev->port->lldd_port;
 
 	if (!internal_abort &&
 	    (DEV_IS_GONE(pm8001_dev) || !port || !port->port_attached)) {
-- 
2.50.1

[PATCH 2/5] scsi: pm80xx: Fix array-index-out-of-of-bounds on rmmod

[Niklas Cassel]

Since commit f7b705c238d1 ("scsi: pm80xx: Set phy_attached to zero when
device is gone") UBSAN reports:
  UBSAN: array-index-out-of-bounds in drivers/scsi/pm8001/pm8001_sas.c:786:17
  index 28 is out of range for type 'pm8001_phy [16]'
on rmmod when using an expander.

For a direct attached device, attached_phy contains the local phy id.
For a device behind an expander, attached_phy contains the remote phy id,
not the local phy id.

I.e. while pm8001_ha will have pm8001_ha->chip->n_phy local phys, for a
device behind an expander, attached_phy can be much larger than
pm8001_ha->chip->n_phy (depending on the amount of phys of the expander).

E.g. on my system pm8001_ha has 8 phys with phy ids 0-7.
One of the ports has an expander connected.
The expander has 31 phys with phy ids 0-30.

The pm8001_ha->phy array only contains the phys of the HBA.
It does not contain the phys of the expander.
Thus, it is wrong to use attached_phy to index the pm8001_ha->phy array
for a device behind an expander.

Thus, we can only clear phy_attached for devices that are directly
attached.

Fixes: f7b705c238d1 ("scsi: pm80xx: Set phy_attached to zero when device is gone")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/pm8001/pm8001_sas.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 753c09363cbb..3e1dac4b820f 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -749,6 +749,7 @@ static void pm8001_dev_gone_notify(struct domain_device *dev)
 	unsigned long flags = 0;
 	struct pm8001_hba_info *pm8001_ha;
 	struct pm8001_device *pm8001_dev = dev->lldd_dev;
+	struct domain_device *parent_dev = dev->parent;
 
 	pm8001_ha = pm8001_find_ha_by_dev(dev);
 	spin_lock_irqsave(&pm8001_ha->lock, flags);
@@ -765,7 +766,13 @@ static void pm8001_dev_gone_notify(struct domain_device *dev)
 			spin_lock_irqsave(&pm8001_ha->lock, flags);
 		}
 		PM8001_CHIP_DISP->dereg_dev_req(pm8001_ha, device_id);
-		pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0;
+
+		/*
+		 * The phy array only contains local phys. Thus, we cannot clear
+		 * phy_attached for a device behind an expander.
+		 */
+		if (!(parent_dev && dev_is_expander(parent_dev->dev_type)))
+			pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0;
 		pm8001_free_dev(pm8001_dev);
 	} else {
 		pm8001_dbg(pm8001_ha, DISC, "Found dev has gone.\n");
-- 
2.50.1

[PATCH 3/5] scsi: pm80xx: Add helper function to get the local phy id

[Niklas Cassel]

Avoid duplicated code by adding a helper to get the local phy id.

No functional changes intended.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/pm8001/pm8001_hwi.c |  8 +++-----
 drivers/scsi/pm8001/pm8001_sas.c | 11 +++++++++++
 drivers/scsi/pm8001/pm8001_sas.h |  1 +
 drivers/scsi/pm8001/pm80xx_hwi.c |  7 ++-----
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 42a4eeac24c9..5d3b958d1dbb 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -4168,7 +4168,6 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	u16 firstBurstSize = 0;
 	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;
 
 	memset(&payload, 0, sizeof(payload));
@@ -4186,10 +4185,9 @@ static int pm8001_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 			dev_is_expander(pm8001_dev->dev_type))
 			stp_sspsmp_sata = 0x01; /*ssp or smp*/
 	}
-	if (parent_dev && dev_is_expander(parent_dev->dev_type))
-		phy_id = parent_dev->ex_dev.ex_phy->phy_id;
-	else
-		phy_id = pm8001_dev->attached_phy;
+
+	phy_id = pm80xx_get_local_phy_id(dev);
+
 	opc = OPC_INB_REG_DEV;
 	linkrate = (pm8001_dev->sas_device->linkrate < dev->port->linkrate) ?
 			pm8001_dev->sas_device->linkrate : dev->port->linkrate;
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 3e1dac4b820f..56d0309d5984 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -130,6 +130,17 @@ static void pm80xx_get_tag_opcodes(struct sas_task *task, int *ata_op,
 	}
 }
 
+u32 pm80xx_get_local_phy_id(struct domain_device *dev)
+{
+	struct pm8001_device *pm8001_dev = dev->lldd_dev;
+	struct domain_device *parent_dev = dev->parent;
+
+	if (parent_dev && dev_is_expander(parent_dev->dev_type))
+		return parent_dev->ex_dev.ex_phy->phy_id;
+	else
+		return pm8001_dev->attached_phy;
+}
+
 void pm80xx_show_pending_commands(struct pm8001_hba_info *pm8001_ha,
 				  struct pm8001_device *target_pm8001_dev)
 {
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 334485bb2c12..91b2cdf3535c 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -798,6 +798,7 @@ void pm8001_setds_completion(struct domain_device *dev);
 void pm8001_tmf_aborted(struct sas_task *task);
 void pm80xx_show_pending_commands(struct pm8001_hba_info *pm8001_ha,
 				  struct pm8001_device *dev);
+u32 pm80xx_get_local_phy_id(struct domain_device *dev);
 
 #endif
 
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index c1bae995a412..53f25d1bce74 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4780,7 +4780,6 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	u16 firstBurstSize = 0;
 	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;
 
 	memset(&payload, 0, sizeof(payload));
@@ -4799,10 +4798,8 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 			dev_is_expander(pm8001_dev->dev_type))
 			stp_sspsmp_sata = 0x01; /*ssp or smp*/
 	}
-	if (parent_dev && dev_is_expander(parent_dev->dev_type))
-		phy_id = parent_dev->ex_dev.ex_phy->phy_id;
-	else
-		phy_id = pm8001_dev->attached_phy;
+
+	phy_id = pm80xx_get_local_phy_id(dev);
 
 	opc = OPC_INB_REG_DEV;
 
-- 
2.50.1

[PATCH 4/5] scsi: pm80xx: Fix pm8001_abort_task() for chip_8006 when using an expander

[Niklas Cassel]

For a direct attached device, attached_phy contains the local phy id.
For a device behind an expander, attached_phy contains the remote phy id,
not the local phy id.

The pm8001_ha->phy array only contains the phys of the HBA.
It does not contain the phys of the expander.

Thus, you cannot use attached_phy to index the pm8001_ha->phy array,
without first verifying that the device is directly attached.

Use the pm80xx_get_local_phy_id() helper to make sure that we use the
local phy id to index the array, regardless if the device is directly
attached or not.

Fixes: 869ddbdcae3b ("scsi: pm80xx: corrected SATA abort handling sequence.")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/pm8001/pm8001_sas.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 56d0309d5984..b3bd61827ad6 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -1065,7 +1065,7 @@ int pm8001_abort_task(struct sas_task *task)
 	struct pm8001_hba_info *pm8001_ha;
 	struct pm8001_device *pm8001_dev;
 	int rc = TMF_RESP_FUNC_FAILED, ret;
-	u32 phy_id, port_id;
+	u32 port_id;
 	struct sas_task_slow slow_task;
 
 	if (!task->lldd_task || !task->dev)
@@ -1074,7 +1074,6 @@ int pm8001_abort_task(struct sas_task *task)
 	dev = task->dev;
 	pm8001_dev = dev->lldd_dev;
 	pm8001_ha = pm8001_find_ha_by_dev(dev);
-	phy_id = pm8001_dev->attached_phy;
 
 	if (PM8001_CHIP_DISP->fatal_errors(pm8001_ha)) {
 		// If the controller is seeing fatal errors
@@ -1106,7 +1105,8 @@ int pm8001_abort_task(struct sas_task *task)
 		if (pm8001_ha->chip_id == chip_8006) {
 			DECLARE_COMPLETION_ONSTACK(completion_reset);
 			DECLARE_COMPLETION_ONSTACK(completion);
-			struct pm8001_phy *phy = pm8001_ha->phy + phy_id;
+			u32 phy_id = pm80xx_get_local_phy_id(dev);
+			struct pm8001_phy *phy = &pm8001_ha->phy[phy_id];
 			port_id = phy->port->port_id;
 
 			/* 1. Set Device state as Recovery */
-- 
2.50.1

[PATCH 5/5] scsi: pm80xx: Use pm80xx_get_local_phy_id() to access phy array

[Niklas Cassel]

While the current code is perfectly fine (because we verify that the
device is directly attached before using attached_phy to index the
pm8001_ha->phy array), let's use the pm80xx_get_local_phy_id() helper
anyway, to reduce the chance that someone will copy paste this pattern to
other parts of the driver.

Note that in this specific case, we still need to keep the check that the
device is not behind an expander, because we do not want to clear
attached_phy of the expander if a device behind the expander disappears
(as that would disable all the other devices behind the expander).

However, if it is the expander itself that disappears, attached_phy will
be cleared, just like it would for any other directly attached device.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/pm8001/pm8001_sas.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index b3bd61827ad6..96ecc5e63f53 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -782,8 +782,11 @@ static void pm8001_dev_gone_notify(struct domain_device *dev)
 		 * The phy array only contains local phys. Thus, we cannot clear
 		 * phy_attached for a device behind an expander.
 		 */
-		if (!(parent_dev && dev_is_expander(parent_dev->dev_type)))
-			pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0;
+		if (!(parent_dev && dev_is_expander(parent_dev->dev_type))) {
+			u32 phy_id = pm80xx_get_local_phy_id(dev);
+
+			pm8001_ha->phy[phy_id].phy_attached = 0;
+		}
 		pm8001_free_dev(pm8001_dev);
 	} else {
 		pm8001_dbg(pm8001_ha, DISC, "Found dev has gone.\n");
-- 
2.50.1

Re: [PATCH 1/5] scsi: pm80xx: Restore support for expanders

[Igor Pylypiv]

On Wed, Aug 13, 2025 at 01:41:08PM +0200, Niklas Cassel wrote:
> Commit 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID") broke
> support for expanders. After the commit, devices behind an expander are no
> longer detected.
> 
> Simply reverting the commit restores support for devices behind an
> expander.
> 
> Instead of reverting the commit (and reintroducing a helper to get the
> port), get the port directly from the lldd_port pointer in struct
> asd_sas_port.
> 
> Suggested-by: Igor Pylypiv <ipylypiv@google.com>
> Fixes: 0f630c58e31a ("scsi: pm80xx: Do not use libsas port ID")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Thank you for fixing this, Niklas!

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

Re: [PATCH 2/5] scsi: pm80xx: Fix array-index-out-of-of-bounds on rmmod

[Igor Pylypiv]

On Wed, Aug 13, 2025 at 01:41:09PM +0200, Niklas Cassel wrote:
> Since commit f7b705c238d1 ("scsi: pm80xx: Set phy_attached to zero when
> device is gone") UBSAN reports:
>   UBSAN: array-index-out-of-bounds in drivers/scsi/pm8001/pm8001_sas.c:786:17
>   index 28 is out of range for type 'pm8001_phy [16]'
> on rmmod when using an expander.
> 
> For a direct attached device, attached_phy contains the local phy id.
> For a device behind an expander, attached_phy contains the remote phy id,
> not the local phy id.
> 
> I.e. while pm8001_ha will have pm8001_ha->chip->n_phy local phys, for a
> device behind an expander, attached_phy can be much larger than
> pm8001_ha->chip->n_phy (depending on the amount of phys of the expander).
> 
> E.g. on my system pm8001_ha has 8 phys with phy ids 0-7.
> One of the ports has an expander connected.
> The expander has 31 phys with phy ids 0-30.
> 
> The pm8001_ha->phy array only contains the phys of the HBA.
> It does not contain the phys of the expander.
> Thus, it is wrong to use attached_phy to index the pm8001_ha->phy array
> for a device behind an expander.
> 
> Thus, we can only clear phy_attached for devices that are directly
> attached.
> 
> Fixes: f7b705c238d1 ("scsi: pm80xx: Set phy_attached to zero when device is gone")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

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

Re: [PATCH 3/5] scsi: pm80xx: Add helper function to get the local phy id

[Igor Pylypiv]

On Wed, Aug 13, 2025 at 01:41:10PM +0200, Niklas Cassel wrote:
> Avoid duplicated code by adding a helper to get the local phy id.
> 
> No functional changes intended.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

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

Re: [PATCH 4/5] scsi: pm80xx: Fix pm8001_abort_task() for chip_8006 when using an expander

[Igor Pylypiv]

On Wed, Aug 13, 2025 at 01:41:11PM +0200, Niklas Cassel wrote:
> For a direct attached device, attached_phy contains the local phy id.
> For a device behind an expander, attached_phy contains the remote phy id,
> not the local phy id.
> 
> The pm8001_ha->phy array only contains the phys of the HBA.
> It does not contain the phys of the expander.
> 
> Thus, you cannot use attached_phy to index the pm8001_ha->phy array,
> without first verifying that the device is directly attached.
> 
> Use the pm80xx_get_local_phy_id() helper to make sure that we use the
> local phy id to index the array, regardless if the device is directly
> attached or not.
> 
> Fixes: 869ddbdcae3b ("scsi: pm80xx: corrected SATA abort handling sequence.")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

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

Re: [PATCH 5/5] scsi: pm80xx: Use pm80xx_get_local_phy_id() to access phy array

[Igor Pylypiv]

On Wed, Aug 13, 2025 at 01:41:12PM +0200, Niklas Cassel wrote:
> While the current code is perfectly fine (because we verify that the
> device is directly attached before using attached_phy to index the
> pm8001_ha->phy array), let's use the pm80xx_get_local_phy_id() helper
> anyway, to reduce the chance that someone will copy paste this pattern to
> other parts of the driver.
> 
> Note that in this specific case, we still need to keep the check that the
> device is not behind an expander, because we do not want to clear
> attached_phy of the expander if a device behind the expander disappears
> (as that would disable all the other devices behind the expander).
> 
> However, if it is the expander itself that disappears, attached_phy will
> be cleared, just like it would for any other directly attached device.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

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

Re: [PATCH 3/5] scsi: pm80xx: Add helper function to get the local phy id

[Damien Le Moal]

On 8/13/25 8:41 PM, Niklas Cassel wrote:
> Avoid duplicated code by adding a helper to get the local phy id.
> 
> No functional changes intended.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

[...]

> +u32 pm80xx_get_local_phy_id(struct domain_device *dev)
> +{
> +	struct pm8001_device *pm8001_dev = dev->lldd_dev;
> +	struct domain_device *parent_dev = dev->parent;
> +
> +	if (parent_dev && dev_is_expander(parent_dev->dev_type))
> +		return parent_dev->ex_dev.ex_phy->phy_id;
> +	else
> +		return pm8001_dev->attached_phy;

Nit: no need for else after a return.
Also, maybe reverse the condition to have this a tad faster for direct attach
devices ?

	if (!parent_dev || !dev_is_expander(parent_dev->dev_type))
		return pm8001_dev->attached_phy;

	return parent_dev->ex_dev.ex_phy->phy_id;

Other than this, this looks good !

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 5/5] scsi: pm80xx: Use pm80xx_get_local_phy_id() to access phy array

[Damien Le Moal]

On 8/13/25 8:41 PM, Niklas Cassel wrote:
> While the current code is perfectly fine (because we verify that the
> device is directly attached before using attached_phy to index the
> pm8001_ha->phy array), let's use the pm80xx_get_local_phy_id() helper
> anyway, to reduce the chance that someone will copy paste this pattern to
> other parts of the driver.
> 
> Note that in this specific case, we still need to keep the check that the
> device is not behind an expander, because we do not want to clear
> attached_phy of the expander if a device behind the expander disappears
> (as that would disable all the other devices behind the expander).
> 
> However, if it is the expander itself that disappears, attached_phy will
> be cleared, just like it would for any other directly attached device.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/scsi/pm8001/pm8001_sas.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index b3bd61827ad6..96ecc5e63f53 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -782,8 +782,11 @@ static void pm8001_dev_gone_notify(struct domain_device *dev)
>  		 * The phy array only contains local phys. Thus, we cannot clear
>  		 * phy_attached for a device behind an expander.
>  		 */
> -		if (!(parent_dev && dev_is_expander(parent_dev->dev_type)))
> -			pm8001_ha->phy[pm8001_dev->attached_phy].phy_attached = 0;
> +		if (!(parent_dev && dev_is_expander(parent_dev->dev_type))) {

This pattern/check is repeated a lot. It may be good to have a
dev_parent_is_expander() helper:

static inline bool dev_parent_is_expander(struct domain_device *dev)
{
	if (!dev->parent)
		return false;

	return dev_is_expander(dev->parent->dev_type);
}

Which will allow simplifying the the test everywhere to:

	if (dev_parent_is_expander(dev))
or
	if (!dev_parent_is_expander(dev))

No ?

> +			u32 phy_id = pm80xx_get_local_phy_id(dev);
> +
> +			pm8001_ha->phy[phy_id].phy_attached = 0;
> +		}
>  		pm8001_free_dev(pm8001_dev);
>  	} else {
>  		pm8001_dbg(pm8001_ha, DISC, "Found dev has gone.\n");


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 0/5] scsi: pm80xx: Fix expander support

[Damien Le Moal]

On 8/13/25 8:41 PM, Niklas Cassel wrote:
> Hello all,
> 
> Some recent patches broke expander support for the pm80xx driver.
> 
> The first two patches in this series makes sure that expanders work with
> this driver again.
> 
> It also fixes a bug in pm8001_abort_task() that was found through code
> review.
> 
> The final two patches make the driver more robust, so that is less likely
> that the expander support will break again in the future.
> 
> Please test and review.

Tested device hot removal & insertion with a JBOD (with expander) and with
direct attached disks. All good.

Of note is that hot-unplug & re-plugging the JBOD does not result in the drives
coming back. The drives are correctly removed when the JBOD is unplugged but
when replugging it, all I see is the PHY UP events but no rescan/no drives. And
forcing a scan of the JBOD host does nothing either. I think that is a
different issue though that we should look into.

> 
> 
> Kind regards,
> Niklas
> 
> 
> Niklas Cassel (5):
>   scsi: pm80xx: Restore support for expanders
>   scsi: pm80xx: Fix array-index-out-of-of-bounds on rmmod
>   scsi: pm80xx: Add helper function to get the local phy id
>   scsi: pm80xx: Fix pm8001_abort_task() for chip_8006 when using an
>     expander
>   scsi: pm80xx: Use pm80xx_get_local_phy_id() to access phy array
> 
>  drivers/scsi/pm8001/pm8001_hwi.c |  8 +++-----
>  drivers/scsi/pm8001/pm8001_sas.c | 34 +++++++++++++++++++++++++-------
>  drivers/scsi/pm8001/pm8001_sas.h |  1 +
>  drivers/scsi/pm8001/pm80xx_hwi.c |  7 ++-----
>  4 files changed, 33 insertions(+), 17 deletions(-)
> 


-- 
Damien Le Moal
Western Digital Research