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

John Garry <[email protected]>
Newsgroups org.kernel.vger.linux-scsi,org.kernel.vger.linux-kernel
Organization Oracle Corporation
Message-ID <[email protected]>
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 <[email protected]>
> ---
>   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,
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.