Re: [PATCH 2/3] exynos: fix G2D_DOUBLE_TO_FIXED

[Tobias Jakobi <tjakobi@math.uni-bielefeld.de>]

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