On 12/13/2019 8:01 PM, Michał Krawczyk wrote: > pt., 13 gru 2019 o 17:34 Stephen Hemminger <step...@networkplumber.org> > napisał(a): >> >> On Fri, 13 Dec 2019 14:32:15 +0100 >> Michal Krawczyk <m...@semihalf.com> wrote: >> >>> This version of the HAL allows to use the latest HW features, like >>> rx offsets. >>> >>> Driver was adjusted to the new version to fix the build. >>> >>> Signed-off-by: Michal Krawczyk <m...@semihalf.com> >>> Signed-off-by: Maciej Bielski <m...@semihalf.com> >> >> You are mixing multiple changes into one patch. >> This makes it harder to review (find the real bits) and also >> harder for bisection. >> >> It makes sense to fix whitespace and related stuff in one >> patch if you are fixing one function and the nearby code >> already needed work. But please avoid larger scale change >> put together. >> >> In your patch I see: >> - remove unnecessary whitespace >> - drop unnecessary inline >> - add missing newline in log messages >> - check for NULL pointer >> >> If possible could you preserve the per-commit updates for base >> code, rather than one lump diff. > > Hi Stephen, > > We are not developing HAL (ena_com) on our own. We are getting it from the > Amazon team and we don't have history of it's development. Tha's why we are > upstreaming a diff between two versions of the ena_com. > > Moreover this HAL is common for Linux kernel driver, FreeBSD kernel driver > and Windows kernel driver. Because of that, we are trying to avoid to > modify it on our side unless it's really necessary (for example, if > compilation fails on for the DPDK).
Hi Michał, Overall this is the common situation for most of the base code, that is why we are a little more flexible on them and not enforcing the coding convention etc.. But as a high level rule, still please try to split the base code drop to logical pieces as much as possible, and I am aware that this requires additional effort. I will proceed with this patchset, but please bare in mind this for next time. Regards, ferruh > > In fact, we are doing changes to the PMD and our platform file > (ena_plat_dpdk.h which is a glue) in this patch, but that is required in > order to do not have patch which is not compiling. > Both the PMD code and the glue code can be changed without affecting other > platforms. >
