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

Hi John,

Thanks for the review.

On 2026/8/10 23:51, John Garry wrote:
> 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.

No current caller passes a NULL ex_dev. The check was added in v7 based 
on an automated Sashiko AI review suggestion, which may contain some 
noise. Removed in v10.

>> 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.

Ok, Renamed to sas_ex_phy_to_dev() for consistency with 
sas_ex_phy_discover() and other phy-related helpers.


Thanks.
Xingui