Make sure we check for CAM_CDB_POINTER for all drivers. Also, for the drivers I've touched, filter out CAM_CDB_PHYS.

[scottl]

sys/dev/isci/isci_controller.c

748 ↗

(On Diff #14176)

What do you think about this stylistic change:

if (ccb->ccb_h.flags & (CAM_CDB_POINTER | CAM_CDB_PHYS)) {

sys/dev/ppbus/vpo.c

195 ↗

(On Diff #14176)

Does this block need to get moved up here? Was it moved accidentally?

[imp]

sys/dev/isci/isci_controller.c

748 ↗

(On Diff #14176)

that's subtly different.

However, in other places in the tree we just check CAM_CDB_PHYS, and I could do that here as well.

sys/dev/ncr/ncr.c

3866–3867 ↗

(On Diff #14176)

I could just check CAM_CDB_PHYS here, as is done in other places in the tree.

sys/dev/ppbus/vpo.c

195 ↗

(On Diff #14176)

No. It was done on purpose so we could compute ptr just once. And it doesn't change anything in terms of what is printed since we always call vpo_intr from the place it was moved from, and only from there.

[scottl]

sys/dev/isci/isci_controller.c

748 ↗

(On Diff #14176)

I suppose that the driver can accidentally handle CAM_CCB_POINTER thanks to the magic of relaxed syntax of referencing pointers vs arrays, and it never tries to reference a single byte in the array directly.

CAM_CCB_PHYS has to imply CAM_CCB_POINTER. You could just remove the check for CAM_CCB_POINTER.

sys/dev/ppbus/vpo.c

195 ↗

(On Diff #14176)

I don't understand. The existing design style was to print the various commands in vpo_action(). What you've changes is not technically incorrect, but it's now inconsistent.

[imp]

Update per scott.

Updating D5585: Make sure we check for CAM_CDB_POINTER for all drivers. Also, for the

drivers I've touched, filter out CAM_CDB_PHYS.

[imp]

sys/dev/ppbus/vpo.c

195 ↗

(On Diff #14179)

Yes. I suppose now that I've created the convenience function, I could move it back.