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