Page MenuHomeFreeBSD

Define xpt_path_inq.
ClosedPublic

Authored by imp on Dec 6 2017, 1:23 AM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 13 2024, 2:51 AM
Unknown Object (File)
Jan 6 2024, 8:03 PM
Unknown Object (File)
Jan 6 2024, 8:03 PM
Unknown Object (File)
Jan 6 2024, 8:03 PM
Unknown Object (File)
Dec 20 2023, 1:23 AM
Unknown Object (File)
Sep 3 2023, 4:24 AM
Unknown Object (File)
Sep 3 2023, 4:21 AM
Unknown Object (File)
Sep 3 2023, 4:18 AM
Subscribers

Details

Summary

This provides a nice wrarpper around the XPT_PATH_INQ ccb creation and
calling.

Sponsored by: Netflix

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/cam/ata/ata_xpt.c
2101 ↗(On Diff #36276)

was NORMAL

sys/cam/cam_periph.c
746 ↗(On Diff #36276)

was NORMAL

Fix the ooopses cem@ noticed.

sys/cam/ata/ata_xpt.c
2101 ↗(On Diff #36276)

Doh!

sys/cam/cam_periph.c
746 ↗(On Diff #36276)

Doh!

This revision is now accepted and ready to land.Dec 6 2017, 1:52 AM
asomers added inline comments.
sys/cam/cam_xpt.h
41 ↗(On Diff #36281)

I think this #include should lie within the #ifdef _KERNEL block.

This revision now requires review to proceed.Dec 6 2017, 4:34 PM
imp marked an inline comment as done.Dec 6 2017, 4:39 PM

Fix the ooopses cem@ noticed.

rpokala added a subscriber: rpokala.

Heh. I've had a half-done, semi-related change sitting in one of my sandboxes for months.

This change de-dupes code in the PERIPHs, which is good.

Months ago, I somehow noticed that most of the SIMs use the exact same values when populating ccb_pathinq: version_num (1), sim_vid ("FreeBSD"), dev_name (cam_sim_name(sim)), unit_number (cam_sim_unit(sim)), and bus_id (cam_sim_bus(sim)). Thus:

static __inline void
cam_init_pathinq(struct ccb_pathinq *cpi, const char* hba_vid,
		struct cam_sim *sim)
{

	cpi->version_num = 1;
	strlcpy(cpi->sim_vid, "FreeBSD", SIM_IDLEN);
	strlcpy(cpi->hba_vid, hba_vid, HBA_IDLEN);
	strlcpy(cpi->dev_name, cam_sim_name(sim), DEV_IDLEN);
	cpi->unit_number = cam_sim_unit(sim);
	cpi->bus_id = cam_sim_bus(sim);
}

And (from sys/dev/ahci/ahci.c):

--- sys/dev/ahci/ahci.c	(revision 326289)
+++ sys/dev/ahci/ahci.c	(working copy)
@@ -2688,7 +2688,7 @@
 	{
 		struct ccb_pathinq *cpi = &ccb->cpi;
 
-		cpi->version_num = 1; /* XXX??? */
+		cam_init_pathinq(cpi, "AHCI", sim);
 		cpi->hba_inquiry = PI_SDTR_ABLE;
 		if (ch->caps & AHCI_CAP_SNCQ)
 			cpi->hba_inquiry |= PI_TAG_ABLE;
@@ -2705,12 +2705,7 @@
 			cpi->max_target = 0;
 		cpi->max_lun = 0;
 		cpi->initiator_id = 0;
-		cpi->bus_id = cam_sim_bus(sim);
 		cpi->base_transfer_speed = 150000;
-		strlcpy(cpi->sim_vid, "FreeBSD", SIM_IDLEN);
-		strlcpy(cpi->hba_vid, "AHCI", HBA_IDLEN);
-		strlcpy(cpi->dev_name, cam_sim_name(sim), DEV_IDLEN);
-		cpi->unit_number = cam_sim_unit(sim);
 		cpi->transport = XPORT_SATA;
 		cpi->transport_version = XPORT_VERSION_UNSPECIFIED;
 		cpi->protocol = PROTO_ATA;

I forget why I abandoned it... probably got busy, and didn't have hardware to test against. Let me know, and I'll shoot you the patch.

In any case, this change looks good to me.

This revision is now accepted and ready to land.Dec 6 2017, 5:12 PM

After talking to scottl@, we don't need priority here at all. It's not
a queued command. So pick a priority to use in xpt_path_inq() and just
use that. All these subtle differences turn out to be uninteresting
and we should go for simplicity.

This revision now requires review to proceed.Dec 6 2017, 9:24 PM
This revision is now accepted and ready to land.Dec 6 2017, 9:26 PM

Looks good to me! Thanks for doing this!

In D13387#279956, @ken wrote:

Looks good to me! Thanks for doing this!

Excellent. There's about half a dozen of these that I wanted to do. Glad you liked the first one, since the rest look to be identical.

This revision was automatically updated to reflect the committed changes.