From: Timur Tabi <timur@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] powerpc: add support for the Freescale P1022DS reference board
Date: Fri, 21 May 2010 09:33:43 -0500 [thread overview]
Message-ID: <4BF699C7.50903@freescale.com> (raw)
In-Reply-To: <20100521142256.8AC57E67644@gemini.denx.de>
Wolfgang Denk wrote:
> Unless you pay me for this (or FSL) you should not even rely on this.
Heh.
> Well, there are things and things.
>
> When we discover that crap has piled up here and there, it's a good
> idea to perform a clean up.
I agree 100%.
> When the first one adds some feature or new architecture or similar,
> it is pretty normal that he cannot yet always decide correctly what
> needs to go where, or which parts are specific to CPU or board and
> which are common. But when more boards get added, this usually becomes
> clear pretty quickly.
Ok.
> It has always been more efficient to prevent code that is known to
> need such cleanup to prevent from going into mainline in the first
> place, than to allow it to go in and hope that somebody, sometime will
> find time and resources and zest for a clean-up.
I'm not sure I agree with that completely. In many cases, it's more
efficient to work on tasks in bite-size chunks.
> Is it easier for you to convince your manager that you need N more
> hours to get that code accepted for mainline, or to get allownce for
> N hours to perform some cleanup in U-Boot that is not related to your
> current project?
Definitely the latter. Cleaning up U-Boot (and Linux) code so that it's
overall easier to maintain and more efficient is one of the primary tasks of
my department.
> [I have somemore tasks at hand if the latter should be the case :-) ]
Please, send me a list. We have an internal bug tracking system, and I
would be more than happy to log all the items that concern you (as long as
they affect Freescale PowerPC products in some way).
>> I guess I should clarify. Kumar said it was based on the order of the
>> addresses of the PCI devices within CCSR space, but he didn't offer any
>> insight as to whether it should be fixed or how.
>
> So please check this again. It looks broken to me.
Ok.
>> Ok, will you allow me to merge those two functions into a single driver at a
>> later time, when I have the opportunity to examine the code and test it on
>> both boards?
>
> Please let's do it now, instead of adding code that is known to need
> fixing. See above for my reasoning.
And see my above response for why I'd prefer not to solve this now.
I'll try to fix it anyway, though.
>>>>>> +#ifndef __ASSEMBLY__
>>>>>> +extern unsigned long calculate_board_sys_clk(void);
>>>>>> +extern unsigned long calculate_board_ddr_clk(void);
>>>>>> +#endif
>>>>>
>>>>> Please move to appropriate header file.
> ...
>> Well, this particular change would probably need to be made later, because I
>> would probably need to change all the board header files at once. It's not
>> trivial.
>
> What exactly is not trivial in moving some prototype declarations to
> another header file? Please elucidate. The most difficult part is
> probably determining where they actually belong to.
Well, I'll take a closer look, but my concern is that there is no good
header file to move these into that wouldn't require modifying dozens of
files. Since I'm trying to add support for one particular board, I don't
want to modify every other board in the same patch set.
I'll try to fix it anyway, though.
--
Timur Tabi
Linux kernel developer at Freescale
next prev parent reply other threads:[~2010-05-21 14:33 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-20 22:01 [U-Boot] [PATCH] powerpc: add support for the Freescale P1022DS reference board Timur Tabi
2010-05-20 22:33 ` Wolfgang Denk
2010-05-20 23:13 ` Kumar Gala
2010-05-21 6:50 ` Wolfgang Denk
2010-05-20 23:23 ` Timur Tabi
2010-05-21 7:07 ` Wolfgang Denk
2010-05-21 13:45 ` Timur Tabi
2010-05-21 14:22 ` Wolfgang Denk
2010-05-21 14:33 ` Timur Tabi [this message]
2010-05-21 16:07 ` Timur Tabi
2010-05-26 18:12 ` Timur Tabi
2010-05-26 18:17 ` Scott Wood
2010-05-26 18:19 ` Timur Tabi
2010-05-26 19:04 ` Scott Wood
2010-05-26 19:34 ` Timur Tabi
2010-05-26 19:46 ` Scott Wood
2010-05-26 21:59 ` Timur Tabi
2010-05-25 18:30 ` Timur Tabi
2010-05-26 20:10 ` Wolfgang Denk
2010-05-26 20:18 ` Timur Tabi
2010-05-26 20:24 ` Timur Tabi
2010-05-27 7:02 ` Wolfgang Denk
2010-05-27 14:31 ` Timur Tabi
2010-05-27 18:11 ` Wolfgang Denk
2010-05-27 18:25 ` Timur Tabi
2010-05-27 19:03 ` Scott Wood
2010-05-27 19:07 ` Timur Tabi
2010-05-27 19:10 ` Scott Wood
2010-05-27 19:54 ` Wolfgang Denk
2010-05-27 19:53 ` Wolfgang Denk
2010-05-27 20:11 ` Timur Tabi
2010-05-27 21:10 ` Wolfgang Denk
2010-05-27 19:53 ` Wolfgang Denk
2010-05-27 20:03 ` Timur Tabi
2010-05-27 20:59 ` Wolfgang Denk
2010-05-27 20:05 ` Scott Wood
2010-05-27 21:03 ` Wolfgang Denk
2010-05-27 19:45 ` Wolfgang Denk
2010-05-27 19:54 ` Timur Tabi
2010-05-27 20:00 ` Wolfgang Denk
2010-05-27 20:10 ` Scott Wood
2010-05-21 0:26 ` [U-Boot] [PATCH] powerpc: add support for the FreescaleP1022DS " Liu Dave-R63238
2010-05-21 15:25 ` Timur Tabi
2010-05-21 9:46 ` [U-Boot] [PATCH] powerpc: add support for the Freescale P1022DS " Kumar Gala
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=4BF699C7.50903@freescale.com \
--to=timur@freescale.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox