From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tobias Jakobi Subject: Re: [PATCH 2/3] exynos: fix G2D_DOUBLE_TO_FIXED Date: Sun, 01 Jun 2014 03:14:16 +0200 Message-ID: <538A7E68.90503@math.uni-bielefeld.de> References: <1401400700-3725-1-git-send-email-tjakobi@math.uni-bielefeld.de> <1401400700-3725-2-git-send-email-tjakobi@math.uni-bielefeld.de> <538A7852.8080403@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.math.uni-bielefeld.de (smtp.math.uni-bielefeld.de [129.70.45.10]) by gabe.freedesktop.org (Postfix) with ESMTP id DB75B6E217 for ; Sat, 31 May 2014 18:14:25 -0700 (PDT) In-Reply-To: <538A7852.8080403@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomasz Figa , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 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