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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 13372
Build 13604: arc lint + arc unit

Event Timeline

sys/cam/ata/ata_xpt.c
2101

was NORMAL

sys/cam/cam_periph.c
746

was NORMAL

Fix the ooopses cem@ noticed.

sys/cam/ata/ata_xpt.c
2101

Doh!

sys/cam/cam_periph.c
746

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

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.