Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932381AbbFRN7X (ORCPT ); Thu, 18 Jun 2015 09:59:23 -0400 Received: from mailgw02.mediatek.com ([210.61.82.184]:60440 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S932091AbbFRN7F (ORCPT ); Thu, 18 Jun 2015 09:59:05 -0400 X-Listener-Flag: 11101 Message-ID: <1434635939.22029.7.camel@mtksdaap41> Subject: Re: [PATCH v2 2/2] pwm: add MediaTek display PWM driver support From: Yingjoe Chen To: YH Huang CC: Thierry Reding , Matthias Brugger , Mark Rutland , Rob Herring , Pawel Moll , , , , , , , "Sascha Hauer" Date: Thu, 18 Jun 2015 21:58:59 +0800 In-Reply-To: <1434622784.18278.39.camel@mtksdaap41> References: <1432214964-40644-1-git-send-email-yh.huang@mediatek.com> <1432214964-40644-3-git-send-email-yh.huang@mediatek.com> <20150612102046.GF19400@ulmo.nvidia.com> <1434622784.18278.39.camel@mtksdaap41> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1296 Lines: 40 On Thu, 2015-06-18 at 18:19 +0800, YH Huang wrote: > On Fri, 2015-06-12 at 12:20 +0200, Thierry Reding wrote: > > > +/* Shift log2(PWM_PERIOD_MAX + 1) as divisor */ > > > +#define PWM_PERIOD_BIT_SHIFT 12 > > > > I wasn't very clear about this in my earlier review, so let me try to > > explain why I think this is confusing. You use this as a divisor, but > > you encode it as a shift. It's also PWM_PERIOD_MAX + 1, so I think it > > would make more sense to drop this, keep PWM_PERIOD_MAX as above and > > then replace the > > > > >> PWM_PERIOD_BIT_SHIFT > > > > below by > > > > / (PWM_PERIOD_MAX + 1) > > > > Maybe I can change in this way: > Remove this: #define PWM_PERIOD_MAX 0x00000fff > Using ">> PWM_PERIOD_BIT_SHIFT" is faster than "/ (PWM_PERIOD_MAX + 1)" > Is this right? The place which use this shift is: clk_div = div_u64(rate * period_ns, NSEC_PER_SEC) >> PWM_PERIOD_BIT_SHIFT; div_u64 return u64. If we change >> to /, and somehow compiler didn't optimize that div into shift, it will cause build error. Joe.C -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/