linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
To: Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Alessandro Zummo <a.zummo@towertech.it>,
	Doug Anderson <dianders@chromium.org>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	rtc-linux@googlegroups.com, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday
Date: Fri, 19 Sep 2014 23:44:26 +0200	[thread overview]
Message-ID: <541CA3BA.6060905@collabora.co.uk> (raw)
In-Reply-To: <1411156347.24444.31.camel@joe-AO725>

Hello Joe,

On 09/19/2014 09:52 PM, Joe Perches wrote:
> On Fri, 2014-09-19 at 21:27 +0200, Javier Martinez Canillas wrote:
>> On 09/19/2014 04:39 PM, Joe Perches wrote:
>> > On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
>> >> The function max77686_rtc_calculate_wday() is used to
>> >> calculate the day of the week to be filled in struct
>> >> rtc_time but that function only calculates the number
>> >> of bits shifted. So the ffs() function can be used to
>> >> find the first bit set instead of a special function.
>> > 
>> > This isn't the same logic.  Perhaps you want fls.
>> >
>> 
>> Right, the removed function has the same logic than fls() - 1 but the value
>> stored in data[RTC_WEEKDAY] is:
>> 
>> data[RTC_WEEKDAY] = 1 << tm->tm_wday;
>> 
>> so for this particular case, it doesn't matter since ffs() == fls() always.
> 
> I didn't look that far.
> 
> I just wanted to show that the logic wasn't the same.
> 

No worries, thanks a lot for your feedback. Probably it would had been good to
mention that in the commit message.

> Perhaps you can specify that ffs==fls in the changelog.
> 

I won't say I'm thrilled to do a whole re-spin of the series just to change
that ;)

Specially since I already did too many revisions and this series have been
floating around for months but is up to Andrew to decide if is worth it.

Best regards,
Javier



  reply	other threads:[~2014-09-19 21:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-19 10:26 [PATCH v10 0/6] Add Maxim 77802 RTC support Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 1/6] rtc: max77686: Allow the max77686 rtc to wakeup the system Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 2/6] rtc: max77686: Remove dead code for SMPL and WTSR Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 3/6] rtc: max77686: Fail to probe if no RTC regmap irqchip is set Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 4/6] rtc: max77686: Remove unneded info log Javier Martinez Canillas
2014-09-19 10:26 ` [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday Javier Martinez Canillas
2014-09-19 14:39   ` Joe Perches
2014-09-19 19:27     ` Javier Martinez Canillas
2014-09-19 19:52       ` Joe Perches
2014-09-19 21:44         ` Javier Martinez Canillas [this message]
2014-09-19 10:26 ` [PATCH v10 6/6] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock Javier Martinez Canillas
2014-09-20  0:14 ` [PATCH v10 0/6] Add Maxim 77802 RTC support Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=541CA3BA.6060905@collabora.co.uk \
    --to=javier.martinez@collabora.co.uk \
    --cc=a.zummo@towertech.it \
    --cc=akpm@linux-foundation.org \
    --cc=dianders@chromium.org \
    --cc=joe@perches.com \
    --cc=k.kozlowski@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=rtc-linux@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).