On 6/12/2024 5:52 AM, Soumyadeep Hore wrote:
These patches integrate the latest changes in MEV TS IDPF Base driver.

---
v3:
- Removed additional whitespace changes
- Fixed warnings of CI
- Updated documentation relating to MEV TS FW release
---

Off list,

I don't know anything about idpf driver, but as far as I'm aware, usually when base code is updated, we resolve all #ifdef's by stripping these tags and assuming that they were (or were not) defined at compile time, so the resulting code is devoid of any compile-time switching. This is the biggest question I have for this review.

Is there any reason why you've kept the #ifdef's in this base code update? Should we perhaps strip them out and assume NVME_CPF is always defined? And if not, then perhaps we should at least provide feedback to base driver developers that we'd rather their code handle both cases when NVME_CPF is defined, rather than switching between two different code paths at compile time with no ability to combine them?

--
Thanks,
Anatoly

Reply via email to