All of lore.kernel.org
 help / color / mirror / Atom feed
From: Che-liang Chiou <clchiou@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] arm: add 64-64 bit divider
Date: Thu, 1 Sep 2011 18:30:47 +0800	[thread overview]
Message-ID: <CANJuy2KrFE+JSodV7O=cxQStv_yNTsYwLQhsbc7oMWPdAt4sEA@mail.gmail.com> (raw)
In-Reply-To: <201109011216.14124.marek.vasut@gmail.com>

Hi Marek,

I will abandon this patch and submit a new patch that is adapted from
do_div() and lib64.c of the Linux kernel. Does this sound okay to you?

Regards,
Che-Liang

On Thu, Sep 1, 2011 at 6:16 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On Thursday, September 01, 2011 12:09:18 PM Che-liang Chiou wrote:
>> Hi,
>>
>> Thanks for the insightful comments. Here are my responses:
>>
>> * Why don't I implement the divider in C?
>> It is not because I think it's performance critical (I haven't
>> benchmarked it yet), but because I have a probably wrong impression
>> that the divider has to be written in assembly --- all dividers in
>> arch/arm/lib/ are written in ARM assembly. What is the policy here for
>> using assembly or C?
>
> No, C is just fine and is more generic. Those assembler versions are just
> optimized things, you don't need to be bothered by those.
>
>>
>> * When do we need a 64-bit divider?
>> In kernel code do_div() is used for various purposes. So I think it
>> should be quite often that we would need a 64-bit divider in U-Boot.
>
> Not much really ... and for the rare cases, we can do with do_div() as is.
>
>>
>> * Do we need a 64-64 bit divider?
>> do_div() defines 64-32 bit division semantics (dividend is 64-bit and
>> divisor is 32-bit), and this patch implements a 64-64 bit divider
>> (both dividend and divisor are 64-bit). I have to admit that I can't
>> think of scenarios or reasons to justify a 64-64 bit divider instead
>> of a 64-32 bit divider, except that a 64-64 bit divider is more
>> generic than a 64-32 bit one.
>
> So we don't need 64/64 divide at all.
>
>>
>> So I guess we can agree that a 64-bit divider is feature that is nice
>> to have, and we should decide:
>> * Do we need a 64-64 bit divider or a 64-32 bit one?
>
> 64-32 is do_div()
>
>> * Do we write it in C or assembly?
>
> C is OK.
>
>>
>> Depending on our decisions, I will rewrite (or abandon) this patch
>> accordingly.
>
> Look, I don't mean to be rough, but honestly. I see no use for this code. Adding
> code to anywhere so it'd just sit there is bad.
>
> Cheers
>
>>
>> Regards,
>> Che-Liang
>>
>> On Thu, Sep 1, 2011 at 4:03 AM, Wolfgang Denk <wd@denx.de> wrote:
>> > Dear Che-Liang Chiou,
>> >
>> > In message <1314787130-1043-1-git-send-email-clchiou@chromium.org> you
> wrote:
>> >> This patch adds a 64-64 bit divider that supports ARMv4 and above.
>> >
>> > To summarize the misc feedback: ?Please explain in detail which
>> > problem you are trying to fix. ?We see no need for this patch so far.
>> >
>> > Best regards,
>> >
>> > Wolfgang Denk
>> >
>> > --
>> > DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
>> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
>> > "Success covers a multitude of blunders." ? ? ? - George Bernard Shaw
>

  reply	other threads:[~2011-09-01 10:30 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-31 10:38 [U-Boot] [PATCH] arm: add 64-64 bit divider Che-Liang Chiou
2011-08-31 11:56 ` Marek Vasut
2011-08-31 14:32 ` Mike Frysinger
2011-08-31 15:11   ` Marek Vasut
2011-08-31 15:27     ` Mike Frysinger
2011-08-31 15:33       ` Marek Vasut
2011-08-31 16:05         ` Mike Frysinger
2011-08-31 16:30           ` Marek Vasut
2011-08-31 17:13             ` Mike Frysinger
2011-08-31 20:03 ` Wolfgang Denk
2011-09-01 10:09   ` Che-liang Chiou
2011-09-01 10:16     ` Marek Vasut
2011-09-01 10:30       ` Che-liang Chiou [this message]
2011-09-01 10:42         ` Marek Vasut
2011-09-01 12:06           ` Che-liang Chiou
2011-09-01 13:07             ` Wolfgang Denk
2011-09-02  3:12               ` Che-liang Chiou
2011-09-07 21:14     ` Wolfgang Denk
2011-09-20 10:45       ` Graeme Russ
2011-09-20 11:28         ` Wolfgang Denk
2011-09-20 11:40           ` Graeme Russ

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='CANJuy2KrFE+JSodV7O=cxQStv_yNTsYwLQhsbc7oMWPdAt4sEA@mail.gmail.com' \
    --to=clchiou@chromium.org \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.