Re: [PATCH RFC 2/6] scsi: scsi_transport_sas: Allocate end device target id in the rphy alloc

[John Garry <john.garry@xxxxxxxxxx>]

On 20/09/2022 23:02, Damien Le Moal wrote:
> >       return &rdev->rphy;
> >   }
> >   EXPORT_SYMBOL(sas_end_device_alloc);
> > @@ -1500,6 +1505,16 @@ struct sas_rphy *sas_expander_alloc(struct 
> > sas_port *parent,
> >   }
> >   EXPORT_SYMBOL(sas_expander_alloc);
> > +static bool sas_rphy_end_device_valid_tproto(struct sas_rphy *rphy)
> > +{
> > +    struct sas_identify *identify = &rphy->identify;
> > +
> > +    if (identify->target_port_protocols &
> > +        (SAS_PROTOCOL_SSP | SAS_PROTOCOL_STP | SAS_PROTOCOL_SATA))
> > +        return true;
> > +    return false;
>
> You could just do:
>
> return identify->target_port_protocols &
>      (SAS_PROTOCOL_SSP | SAS_PROTOCOL_STP | SAS_PROTOCOL_SATA))

OK

> > +}
> > +
> >   /**
> >    * sas_rphy_add  -  add a SAS remote PHY to the device hierarchy
> >    * @rphy:    The remote PHY to be added
> > @@ -1529,16 +1544,10 @@ int sas_rphy_add(struct sas_rphy *rphy)
> >       mutex_lock(&sas_host->lock);
> >       list_add_tail(&rphy->list, &sas_host->rphy_list);
> > -    if (identify->device_type == SAS_END_DEVICE &&
> > -        (identify->target_port_protocols &
> > -         (SAS_PROTOCOL_SSP | SAS_PROTOCOL_STP | SAS_PROTOCOL_SATA)))
> > -        rphy->scsi_target_id = sas_host->next_target_id++;
> > -    else if (identify->device_type == SAS_END_DEVICE)
> > -        rphy->scsi_target_id = -1;
> >       mutex_unlock(&sas_host->lock);
> >       if (identify->device_type == SAS_END_DEVICE &&
>
> You could move this check inside sas_rphy_end_device_valid_tproto(),
> no ?

Yeah, I suppose I could. I might require a function rename with that.

> > -        rphy->scsi_target_id != -1) {
> > +        sas_rphy_end_device_valid_tproto(rphy)) {
> >           int lun;
> >           if (identify->target_port_protocols & SAS_PROTOCOL_SSP)
> > @@ -1667,7 +1676,7 @@ static int sas_user_scan(struct Scsi_Host 
> > *shost, uint channel,
> >       mutex_lock(&sas_host->lock);
> >       list_for_each_entry(rphy, &sas_host->rphy_list, list) {
> >           if (rphy->identify.device_type != SAS_END_DEVICE ||
> > -            rphy->scsi_target_id == -1)
> > +            !sas_rphy_end_device_valid_tproto(rphy))
> >               continue; 

Thanks,
John