> -----Original Message----- > From: James Bottomley [mailto:James.Bottomley@xxxxxxx] > Sent: Wednesday, January 20, 2010 12:48 AM > 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 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 Thanks James. Now I understood your change. This looks OK to me. - Kashyap > actual API you use for mpt2sas is unchanged. > > James > ��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
