Re: [PATCH 1/4] scsi: libsas: Add sas_execute_internal_abort_single()

From: John Garry
Date: Mon Apr 25 2022 - 04:51:38 EST


On 25/04/2022 09:34, Hannes Reinecke wrote:
+/* LLDDs rely on these values */
+enum sas_internal_abort {
+    SAS_INTERNAL_ABORT_SINGLE    = 0,
+};
+

Why don't you use the existing TMF_XXX values here?
Your 'single' method pretty much _is_ a TMF_ABORT_TASK, and the 'device' method _is_ a TMF_ABORT_TASK_SET, no?

Sure, they are doing the same as TMFs and there is equivalence in the 'single' and 'device' methods, as you say.

However, as mentioned in the comment, the LLDDs rely on the values in enum sas_internal_abort, which do not match the values in TMF_ABORT{_TASK, _TASK_SET}.

How can they rely on a value which you just introduced?

I am relying on no one changing those values in enum sas_internal_abort.

Both hisi_sas and pm8001 use value of 0 for single abort and 1 for device abort in their own internal abort HW frames structs.

And if some other controller comes along which wants to support this feature and the values in enum sas_internal_abort don't match then they would need to do some translation.

I could use TMF values and do the translation in hisi_sas and pm8001 drivers today, but I don't see much much gain in that.

Thanks,
John