RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Moore, Eric" <Eric.Moore@xxxxxxx>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Mon, 23 Jul 2007 18:11:34 -0600

> Tomo - do you have any documentation on how to specify a bsg device?

Sorry, I don't. But it's not difficult.

With 2.6.23-rc1 + mptsas smp patch, you get directories /sys/class/bsg
like:

fujita@nice:/sys/class/bsg$ ls
0:0:0:0  0:0:1:0  end_device-0:0  end_device-0:1  sas_host0

I don't have any expanders but if you have, you have a directory like
expander-0:1 under /sys/class/bsg.

I can run smp_rep_manufacturer against "invisible" SMP target in my
LSI SAS HBA like this:

nice:/home/fujita# ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/sas_host0
  SAS-1.1 format: 0
  vendor identification: LSI
  product identification: Virtual SMP Port
  product revision level:

Hopefully, you can do against expanders:

# ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/expander-0:1


I think that James tested this with aic94xx, however, I guess that
nobody has tested this with mptsas.


> I was trying to run the smp_rep_manufacture from sgv4_tools, and I
> got that error.  Due to that, I have not able to test this patch.

What is the error message?


> However, here are some feedback with regards to the patch:

Thanks.

> 
> On Sunday, July 08, 2007 9:52 PM, FUJITA Tomonori wrote: 
> 
> > +
> > +        smpreq->RequestDataLength = req->data_len - 4;
> 
> Our firmware formats the data in little endian, so you'll need to
> convert this inorder for it work under other endian formats, like sparc
> and ppc.  So this code should be:
> 
> smpreq->RequestDataLength = cpu_to_le16(req->data_len - 4);

I see. I'll fix it.


> > +        smpreq->Function = MPI_FUNCTION_SMP_PASSTHROUGH;
> > +
> > +	if (rphy)
> > +		memcpy(&smpreq->SASAddress, &rphy->identify.sas_address,
> > +		       sizeof(smpreq->SASAddress));
> 
> should be using cpu_to_le64 before the data is put into
> smpreq->SASAddress

Thanks, I will fix it.


> > +	else {
> > +		struct mptsas_portinfo *port_info;
> > +
> > +		mutex_lock(&ioc->sas_topology_mutex);
> > +		port_info = mptsas_find_portinfo_by_handle(ioc, 
> > ioc->handle);
> > +		if (port_info && port_info->phy_info)
> > +			memcpy(&smpreq->SASAddress,
> > +			       
> > &(port_info->phy_info[0].phy->identify.sas_address),
> > +			       sizeof(smpreq->SASAddress));
> > +		mutex_unlock(&ioc->sas_topology_mutex);
> > +	}
> 
> I'm not sure what the intent of this else case.

This code is for an "invisible" SMP target in LSI SAS HBAs. There are
better ways to get the target's address, I think. Any suggestions?


>  I think it should be
> deleted, and replaced with a return of ENXIO.    The sas_topology is a
> flat model, with the first object the intiatiator, and the other objects
> in the list are expanders.    What your your attempting to obtain is the
> first port object connected to the initiator, which could be anything,
> for instance it could be an end device, instead of expander.    So I
> think if your rphy object is not returning a valid sas_address, then
> return instead of attempting to find something from the sas_topology.
> I hope the API is making sure this is an expander before mptsas is even
> called, is that handled?

> > +	/* a reply frame is expected */
> > +	if (!(ioc->sas_mgmt.status & MPT_IOCTL_STATUS_RF_VALID)) {
> > +		printk("%s: the smp response invalid!\n", __FUNCTION__);
> > +		ret = -ENXIO;
> > +	}
> > +
> 
> I think in addition to having a valid reply, I think you should look at
> the iocstatus to insure the reply itself was successfull, and that there
> were no data underruns.
> 
> ioc_status = le16_to_cpu(smpReply->IOCStatus) & MPI_IOCSTATUS_MASK;
> if ((ioc_status != MPI_IOCSTATUS_SUCCESS) &&
>     (ioc_status != MPI_IOCSTATUS_SCSI_DATA_UNDERRUN)) {
> 	return -ENXIO;
> }

Yeah, I guess that it would be better to send mpt's smpReply to user
space then user-space tools can examine it. You know, That's what mpt
ioctl and Doug' smp_utils do. But we can't use the in-buffer for this
since it's used for the smp response. We could use sense buffer for
this.
-
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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux