Re: [PATCH v9 1/2] scsi: libsas: refactor sas_ex_to_ata() using new helper sas_ex_to_dev()

[John Garry <john.g.garry@oracle.com>]

On 24/06/2026 07:32, Xingui Yang wrote:
> Introduce sas_ex_to_dev() to return any device type attached to an
> expander phy. The new helper is then used by sas_ex_to_ata() to reduce
> code duplication.
> 
> Also add a defensive NULL check for ex_dev to guard against callers
> passing a NULL device.
> 

Why would someone want to lookup a device attached to an expander device 
which is NULL? If this is possible later, then mention it.

> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> ---
>   drivers/scsi/libsas/sas_expander.c | 19 ++++++++++++++-----
>   drivers/scsi/libsas/sas_internal.h |  1 +
>   2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index f471ab464a78..fc6d8f3c9dca 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -345,13 +345,16 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
>   		 SAS_ADDR(phy->attached_sas_addr), type);
>   }
>   
> -/* check if we have an existing attached ata device on this expander phy */
> -struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
> +/* return the domain device attached to an expander phy */
> +struct domain_device *sas_ex_to_dev(struct domain_device *ex_dev, int phy_id)
>   {

sas_ex_phy_to_dev() seems a better name, as sas_ex_to_dev() is quite 
vague. I know that it matches sas_ex_to_ata(), but it is not a good 
pattern to match.

> -	struct ex_phy *ex_phy = &ex_dev->ex_dev.ex_phy[phy_id];
> -	struct domain_device *dev;
> +	struct ex_phy *ex_phy;
>   	struct sas_rphy *rphy;
>   
> +	if (!ex_dev)
> +		return NULL;
> +
> +	ex_phy = &ex_dev->ex_dev.ex_phy[phy_id];
>   	if (!ex_phy->port)
>   		return NULL;
>   
> @@ -359,7 +362,13 @@ struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
>   	if (!rphy)
>   		return NULL;
>   
> -	dev = sas_find_dev_by_rphy(rphy);
> +	return sas_find_dev_by_rphy(rphy);
> +}
> +
> +/* check if we have an existing attached ata device on this expander phy */
> +struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
> +{
> +	struct domain_device *dev = sas_ex_to_dev(ex_dev, phy_id);
>   
>   	if (dev && dev_is_sata(dev))
>   		return dev;
> diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
> index 7dce0f587149..350a70484bde 100644
> --- a/drivers/scsi/libsas/sas_internal.h
> +++ b/drivers/scsi/libsas/sas_internal.h
> @@ -91,6 +91,7 @@ int sas_smp_get_phy_events(struct sas_phy *phy);
>   
>   void sas_device_set_phy(struct domain_device *dev, struct sas_port *port);
>   struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
> +struct domain_device *sas_ex_to_dev(struct domain_device *ex_dev, int phy_id);
>   struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id);
>   int sas_ex_phy_discover(struct domain_device *dev, int single);
>   int sas_get_report_phy_sata(struct domain_device *dev, int phy_id,