Please use ">>" in case we ever add more of these.

Thanks for your feedback. We will update this in the review.
Locally checked the change and it is reflecting as desired.

Thanks for the suggestions on grammar and typos.
The typos fixes we made currently in this review are done to align with the Linux codebase.

The files are shared with Linux/Windows platforms as well and in case we correct here, we will have some additional reworks in the internal repos as well to align the files across platforms. We will plan to consider the suggestions made in a future review.

Thanks - we will update this in this review and respond here

Our internal verify job has a spellcheck and formatting tool that had identified this and it has been fixed when fixing multiple other typos

Thanks - We will update this review correcting it

Thanks - this EXPORT_SYMBOL_GPL() is not required. We will update this review removing it.
The debugfs is a qat specific sysctl entry which will display few config parameters when requested.

We wanted to invoke a custom handler adf_4xxx_sysctl_disable_safe_dc_mode when the sysctl attribute is set and hence SYSCTL_ADD_OID was preferred.

Yes the capabilities handling logic has been updated and refactored in the code.
We are aligning the logic with the Linux drivers.

Thanks Mark - Agree to this.
We will fix in this review.

Thanks for highlighting - There needs a cleanup regarding this function which we will do in a future release. Our current scope did not cover the RAS part. The privilege check was added uniformly for all sysctl functions

We currently do not have a plan to add more options.
The options and file are qat specific.
We need this file to be generated freshly. In case we want to add new option we will ensure we append to the file.

opt_qat.h:
:> ${.TARGET}
.if defined(QAT_DISABLE_SAFE_DC_MODE) && ${QAT_DISABLE_SAFE_DC_MODE} == "1"
@echo "#define QAT_DISABLE_SAFE_DC_MODE 1" > ${.TARGET}
.endif
.if defined(QAT_ENABLE_NEW_FEATURE) && ${QAT_ENABLE_NEW_FEATURE} == "1"
@echo "#define QAT_ENABLE_NEW_FEATURE 1" >> ${.TARGET}
.endif
.endif

Thanks.
Yes - we identified this as part of coverity scan and fixed it.

Thanks for the suggestion. We will plan this for a future release.

Thanks for the suggestion. We will plan this for a future release.

Thanks for the feedback. We understand there are inconsistencies here and in some other portions as well.
We will check and plan this for a future release.

Agree - we will plan to fix this and more typos in a future release.

Totally agree! We will check the inconsistencies and plan in a future release.

This was missed in an earlier release and has been corrected now.

Thanks for the suggestion.
As mentioned - the files are shared with other platforms as well. And we will plan internally and correct and update in a future release

We discussed this internally and it looks it is the same code in Linux as well.
We also note there can be more places which do this style. Hence, we will check for these occurrences and plan for a future release.

This was identified as part of our coverity analysis on the code. There can be instances where the code flow can have device as non-NULL - and device structure not properly filled with device name. This could lead to subsequent issues - and hence a device name check here was essential

The cpaGetNumInstances API is the one that is exposed outside and recommended for usage with the applications to get the number of instances in general by requesting with the service type. The Lac_GetCyNumInstancesByType() function is only internal wrapper function for the crypto instances.

We believe this should not cause issues.

Thanks. Totally agree. The formatting needs correction. We will correct this in a future release.

Sure - we will add the SPDX identifier and remove the longer form of license and ensure consistency across files. We will update this review.

Thanks Eric and all for the good feedback on these typos/formatting items. This review is just a first step in us improving these items (as we rebase internally to a later version of the code shared with other teams). Some of the suggestions may already be fixed internally and we will be picking up in future QAT FreeBSD releases. Others that are not, we will be tracking internally and try to influence change in the shared internal code. Bottom line is that I expect this is just the first of several iterations we will be making to improve the quality. Just some background to try and explain why we are not immediately making changes for some of the suggestions.

Changes are updated as suggested in the latest update.

The latest version is updated to add SPDX identifier and removing longer form license thereby maintaining consistency

Agree - We will feedback this and check in a future release.

We believe this was corrected to match the functionality - "and atleast one" of the capabilities was required.

It has been corrected in the latest update.

EXPORT_SYMBOL_GPL is not required and hence removed in the latest version

bus master disable is called in the detach flow as well in the latest revision.

We have updated the code now. The bus master disable is called in the failure scenarios of driver attach.

We reviewed it again and saw it is okay to append instead of overwrite.
The code has been updated to use ">>" append

Thanks. The code has been updated to use ">>" append

Thanks. The code has been updated to use ">>" append

Thanks. The code has been updated to use ">>" append

Mandoc does not allow leading zeros in dates.

The roff(7) language does not allow blank lines, this produces a warning in the linter. You can check for roff errors using the manual compiler's linter: mandoc -Tlint src/share/man/man4/qat.4.

Thanks, We noticed this by running mandoc command run as suggested.

mandoc -Tlint share/man/man4/qat.4
mandoc: share/man/man4/qat.4:118:1: WARNING: blank line in fill mode, using .sp
mandoc: share/man/man4/qat.4:3:5: STYLE: normalizing date format to: Dd June 2, 2025

We have fixed it in the latest revision.

Blank line is removed. Thanks for the knowledge regarding mandoc.

We will ensure this to be run as part of any of our future qat man page updates as well.