All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Schocher <hs@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3] arm926ejs: timer: Replace bss variable by gdr
Date: Mon, 24 Jan 2011 07:42:37 +0100	[thread overview]
Message-ID: <4D3D1F5D.8050800@denx.de> (raw)
In-Reply-To: <4D3AA1A0.4090907@ahsoftware.de>

Hello Alexander,

Alexander Holler wrote:
> Am 22.01.2011 08:46, schrieb Albert ARIBAUD:
>> Le 22/01/2011 06:39, Alexander Holler a ?crit :
>>> Hello,
>>>
>>> Am 21.01.2011 09:56, schrieb Heiko Schocher:
>>>
>>>> -static ulong timestamp;
>>>> -static ulong lastdec;
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +#define timestamp gd->tbl
>>>> +#define lastdec gd->lastinc
>>>
>>> I'm the only one who doesn't like such defines? They might be handy for
>>> quick fixes, but in regard to style and readablity I don't like them.
>>> When looking at teh code where they will used, you won't see the actual
>>> place where they are stored. And in more complex expression they might
>>> become dangerous to use because they hide the operator "->".
>>
>> I accept the patch because it un-breaks support for ARM cpus, and I
>> prefer a working fix to a perfect fix in this specific, transitional,
>> situation.
> 
> My experience is that such quick fixes (or workarounds) usually
> manifests (because they become forgotten) and later on might even be

As this is a open source project, feel free to work with us, that
this get not forgotten!

> copied to other places. ;)

Yes, it is not the best solution, but it fixes a bug. The patch is
some days old (and made as a quick fix), and I vote also to make a
better approach for timers on arm, but in the time this patch was
pending none did a better (accepted) approach...

> Anyway, I'll have to thank for that patch, because it fixes at least one
> of the problems I have while trying to chainload a 2010.12 from a
> 2010.12 on a kirkwood system.
> 
> Regards, and again, thanks for the patch, even if I found it ugly,

As I said above, feel free to post a patch with a better approach.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

      reply	other threads:[~2011-01-24  6:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-10  9:33 [U-Boot] [PATCH] arm926ejs: timer: Replace bss variable by gdr Heiko Schocher
2010-12-11 11:41 ` Prafulla Wadaskar
2010-12-13  7:29 ` Stefano Babic
2011-01-20 20:43 ` Albert ARIBAUD
2011-01-20 20:49   ` Albert ARIBAUD
2011-01-21  8:33     ` Heiko Schocher
2011-01-21  8:48 ` [U-Boot] [PATCH v2] " Heiko Schocher
2011-01-21  8:56 ` [U-Boot] [PATCH v3] " Heiko Schocher
2011-01-21 17:37   ` Albert ARIBAUD
2011-01-22  5:39   ` Alexander Holler
2011-01-22  7:46     ` Albert ARIBAUD
2011-01-22  8:14       ` Reinhard Meyer
2011-01-22  9:21       ` Alexander Holler
2011-01-24  6:42         ` Heiko Schocher [this message]

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=4D3D1F5D.8050800@denx.de \
    --to=hs@denx.de \
    --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.