On Tue, 2010-01-19 at 23:57 +0530, Desai, Kashyap wrote: > > > -----Original Message----- > > From: James Bottomley [mailto:James.Bottomley@xxxxxxx] > > Sent: Monday, January 18, 2010 9:45 PM > > To: Desai, Kashyap > > Cc: linux-scsi@xxxxxxxxxxxxxxx; Moore, Eric; Prakash, Sathya > > Subject: Re: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives > > (Added SAS Transport APIs) > > > > On Wed, 2009-12-16 at 18:56 +0530, Kashyap, Desai wrote: > > > Added tlr_enabled bit in sas_device structure, which will have > > boolean > > > value 0 for disabled and 1 for enabled. > > > > > > As suggested by James B, I have added > > sas_tlr_supported(),sas_enable_tlr(), > > > sas_disable_tlr() functions at SAS transport layer so that any other > > > Low layer driver can use those APIs. > > > > > > SAS transport API sas_tlr_supported() will send vpd page 0x90, > > > to check the TLR support. If TLR is supported for end device, MPT2SAS > > driver > > > will enable the TLR bit in the SCSI_IO for every request. If there is > > a > > > response with MPI2_SCSITASKMGMT_RSP_INVALID_FRAME, the driver will > > turn off > > > the TLR logic. > > > > > > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx> > > > Reviewed-by: Eric Moore <Eric.moore@xxxxxxx> > > > > What's with the xxxx? I've got better things to do than fix up > > changelog signoffs. > > Really Sorry about this gaffe. > > > > > diff --git a/drivers/scsi/scsi_transport_sas.c > > b/drivers/scsi/scsi_transport_sas.c > > > index fd47cb1..41b8eab 100644 > > > --- a/drivers/scsi/scsi_transport_sas.c > > > +++ b/drivers/scsi/scsi_transport_sas.c > > > @@ -358,6 +358,61 @@ void sas_remove_host(struct Scsi_Host *shost) > > > } > > > EXPORT_SYMBOL(sas_remove_host); > > > > > > +/** > > > + * sas_tlr_supported - checking TLR bit in vpd 0x90 > > > + * @sdev: scsi device struct > > > + * > > > + * Check Transport Layer Retries are supported or not. > > > + * If vpd page 0x90 is present, TRL is supported. > > > + * > > > + */ > > > +unsigned int > > > +sas_tlr_supported(struct scsi_device *sdev) > > > +{ > > > + char *buffer; > > > + > > > + buffer = scsi_get_vpd_page(sdev, 0x90); > > > + if (buffer == NULL) > > > + return 0; > > > + > > > + kfree(buffer); > > > + return 1; > > > > This can't be right. TLR supported is the lowest bit of the 8th byte > > of > > the port entry ... you can't condition this on whether the protocol > > page > > exists or not. > Got this point. I will change accordingly. > > > > > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h > > > index 68d185c..7c1361b 100644 > > > --- a/include/scsi/scsi_device.h > > > +++ b/include/scsi/scsi_device.h > > > @@ -112,6 +112,7 @@ struct scsi_device { > > > * scsi_devinfo.[hc]. For now used only to > > > * pass settings from slave_alloc to scsi > > > * core. */ > > > + unsigned tlr_enabled:1; /* set tlr flags */ > > > > And this is wrong: the tlr is SAS specific, it belongs in the end > > device flags like all the other SAS specific entries. > I got it. I will move this into scsi_transport_sas.h in sas_end_device as flags. > > > > We also need to expose it to the user in case they want to change the > > flag. > > > I am little confused on this point. Hope you don’t mind simplifying it. > > > How about separating the two patches and doing this for the transport > > class? If it looks OK, I'll add it to the patch sequence and alter > > this > > commit. > James, What does this mean ? It means does the patch look OK to you? As in storing the data in the transport class and separating it into supported and enabled. The actual API you use for mpt2sas is unchanged. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
