Hello Tomasz!
Tomasz Figa wrote:
However looking at libdrm repo history, your patches don't seem to follow formatting guidelines used there: they lack commit messages (which should say what is changed and why) and your signed-off-by tags.
Originally I sent these to Rob Clark (with Inki Dae in CC), but he wanted me to use git-send-email to send the patches to dri-devel for review. Which I then did.
I don't see how the patches lack commit messages? I don't think any additional explanation is needed to what these changes do. They are one-liners and the issues they address are obvious copy&paste errors (which the fimg2d tests don't discover though). Or am I supposed to name them something like "exynos: fix typo in xyz"?
I can resend them with my signed-off if that' fine? I assume adding '--signoff' to git-send-email should do the trick?
Also it is usually a good idea to include respective maintainers on Cc list, although unfortunately I'm not sure who would that be in case of libdrm.
One more comment inline.
On 29.05.2014 23:58, Tobias Jakobi wrote:
exynos/fimg2d.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/exynos/fimg2d.h b/exynos/fimg2d.h index 1aac378..bc45ab5 100644 --- a/exynos/fimg2d.h +++ b/exynos/fimg2d.h @@ -25,7 +25,7 @@ #define G2D_MAX_CMD_LIST_NR 64 #define G2D_PLANE_MAX_NR 2
-#define G2D_DOUBLE_TO_FIXED(d) ((unsigned int)(d) * 65536.0) +#define G2D_DOUBLE_TO_FIXED(d) ((unsigned int)(d * 65536.0))
You should also keep the parentheses around d, so that the macro evaluates correctly even if an expression is passed as the argument.
I don't think that affects the code using the macro, but you're right of course. I'm going to fix this!
Best regards, Tomasz
With best wishes, Tobias