[dpdk-dev] [PATCH] bus/fslmc: update MC to 10.3.x

Shreyansh Jain shreyansh.jain at nxp.com
Fri Oct 6 15:11:35 CEST 2017


Hello Thomas,

On Friday 06 October 2017 05:04 AM, Thomas Monjalon wrote:
>> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
> 
> This is a huge patch without any comment!
> It will be hard to find bugs in it when debugging in future.
> And it will be also pointless to reference it in future fixes.
>  From a quality point of view, it is bad.

Well, this patch is actually changing the lowest glue code for DPAA2. I 
did split into multiple patches (component wise) but it didn't make much 
sense as they were still un-reviewable. I merged them back again.

Ironically, this series was actually to make code easier to read - 
improving overall glue code quality. Improving comments and variable 
naming, removing large macros - with minimal functional impact.

> 
> As it is touching only to dpaa2 stuff, it is accepted.
> I do not want to be the one digging in it.

Fair enough. I am not expecting that someone will review. It is internal 
stuff to DPAA2 and not something which can be reviewed without context.

> Please avoid such patch in future.

This is somewhere I have doubt.
The core model of DPAA2/DPAA1 drivers is hardware interfacing code which 
is large in size. This layer tends to change when newer internal 
versions are released or some update is made (bug fixes).
We don't send changes out very frequently - that is why they gather over 
a longer period. Keeping them in original state (multiple patches) only 
ends up spoiling DPDK commit history.

Being a single commit, I think bisects would be easier in the sense that 
a bisect would either contain or not contain these internal changes.

But, I will definitely take care to make them as commented/documented as 
much as possible in future.

(Though, I do take care of splitting any patch which touches the DPDK 
API layer into logical splits as that is reflected in commit history.)

> 
> Applied
> 

Thank a lot.


More information about the dev mailing list