Generally I think it is good, but I think it should be set higher, after the "softc->quirks |= DA_Q_NO_6_BYTE;" line.

In D20892#467145, @rajesh1.kumar_amd.com wrote:

I have submitted the manpage for Review (https://reviews.freebsd.org/D21462). Please have a look and let me know your comments.

I'll tune few minor things, like copyright year in the future, but otherwise it looks good.

In D20892#466434, @rajesh1.kumar_amd.com wrote:

Theoretically, I don't see any trouble in retaining the db_count in case of MSI and INTx fallback. But, how will it be helpful to have multiple doorbell with single interrupt? Can you give an idea please? Also, When you say,

You just need to provide valid amd_ntb_db_vector_mask() for that case.

db_valid_mask will be set as (db_count - 1 ) always (I assume, this should not be changed). So, if we don't reduce the db_count, how amd_ntb_db_vector_mask will create an impact?

In D21409#465864, @0mp wrote:

Also, do we want to MFC this change?

Looks good to me. Just thinking whether -1 or EOF is more appropriate.

Looking through the code (sorry for enormous delay), I found significant number of random issues. That is why I made some efforts to clean up the code as I see fit, not rewriting it too much, but making it build on amd64 and i386 and quickly testing functionality. If you have any objections or improvements, I'll happily discuss them.

The code also seems to not build on i386. I haven't looked close, but when I tried to build other thing, it failed.

And could you also write small man page so that people would know about the driver and to which hardware it applies.

Have you tried to run this code on debug kernel? On fresh head with INVARIANTS, WITNESS, etc. it complains and crashes immediately on attempt to sleep under mutex in many places (sysctl, memory allocations, etc).

Looks good to me. I'll commit it as soon as test build completes.