[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Stefan Roese]

By extracting these defines into a header, they can be re-used by other
C sources as well. This will be done by the SPL framework OS boot
support.

Signed-off-by: Stefan Roese <sr@denx.de>
---
Changes in v6:
- Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined

 arch/powerpc/cpu/mpc85xx/release.S   | 1 -
 arch/powerpc/include/asm/processor.h | 6 ++++++
 arch/powerpc/lib/bootm.c             | 6 ------
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/release.S b/arch/powerpc/cpu/mpc85xx/release.S
index 4ba44a9..d2e94b8 100644
--- a/arch/powerpc/cpu/mpc85xx/release.S
+++ b/arch/powerpc/cpu/mpc85xx/release.S
@@ -351,7 +351,6 @@ __secondary_reset_vector:
 	.align L1_CACHE_SHIFT
 	.global __second_half_boot_page
 __second_half_boot_page:
-#define EPAPR_MAGIC		0x45504150
 #define ENTRY_ADDR_UPPER	0
 #define ENTRY_ADDR_LOWER	4
 #define ENTRY_R3_UPPER		8
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 7aa3231..19fe250 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -1342,4 +1342,10 @@ void _nmask_and_or_msr(unsigned long nmask, unsigned long or_val);
 #endif
 #endif /* CONFIG_MACH_SPECIFIC */
 
+#if defined(CONFIG_MPC85xx) || defined(CONFIG_440)
+ #define EPAPR_MAGIC	(0x45504150)
+#else
+ #define EPAPR_MAGIC	(0x65504150)
+#endif
+
 #endif /* __ASM_PPC_PROCESSOR_H */
diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
index 53dc4df..e3fee0b 100644
--- a/arch/powerpc/lib/bootm.c
+++ b/arch/powerpc/lib/bootm.c
@@ -87,12 +87,6 @@ static void boot_jump_linux(bootm_headers_t *images)
 		 *   r8: 0
 		 *   r9: 0
 		 */
-#if defined(CONFIG_MPC85xx) || defined(CONFIG_440)
- #define EPAPR_MAGIC	(0x45504150)
-#else
- #define EPAPR_MAGIC	(0x65504150)
-#endif
-
 		debug ("   Booting using OF flat tree...\n");
 		WATCHDOG_RESET ();
 		(*kernel) ((bd_t *)of_flat_tree, 0, 0, EPAPR_MAGIC,
-- 
1.8.0

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Wolfgang Denk]

Dear Stefan Roese,

In message <1351590321-20368-1-git-send-email-sr@denx.de> you wrote:
> By extracting these defines into a header, they can be re-used by other
> C sources as well. This will be done by the SPL framework OS boot
> support.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
> Changes in v6:
> - Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined

Please re-read
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

You are supposed to provide a _history_ of changes here, but instead
you describe only the latest change.  This is not how it's suppoosed
to be done.

Also, your submission includes neither any "In-reply-to:" nor any
"References:" header, i. e. there is no way to match it to any
previous mail thread.  This is very bad.  How are we supposed to know
what you are actually talking about, or where we would find any of the
previous patches or the other 6 patches of this series?

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
Es ist offensichtlich, dass das menschliche Gehirn wie  ein  Computer
funktioniert.  Da  es  keine  dummen Computer gibt, gibt es also auch
keine dummen Menschen. Nur ein paar Leute, die unter DOS laufen.
                                                       -- <unbekannt>

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Stefan Roese]

Hi Wolfgang,

On 10/30/2012 11:04 AM, Wolfgang Denk wrote:
>> By extracting these defines into a header, they can be re-used by other
>> C sources as well. This will be done by the SPL framework OS boot
>> support.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> ---
>> Changes in v6:
>> - Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined
> 
> Please re-read
> http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions
> 
> You are supposed to provide a _history_ of changes here, but instead
> you describe only the latest change.  This is not how it's suppoosed
> to be done.

As you know this patch is part of a patch-series. And this is the first
time that this patch has a change. So this summary covers the complete
history for this patch.

> Also, your submission includes neither any "In-reply-to:" nor any
> "References:" header, i. e. there is no way to match it to any
> previous mail thread.

Yes, I should have done this. Sorry.

>  This is very bad.  How are we supposed to know
> what you are actually talking about, or where we would find any of the
> previous patches or the other 6 patches of this series?

In this version of the patch series, I only made this small change to
this patch 1/7. I wanted to spare the list a resending of the complete
patchset for such a small change.

So what is the recommended way to do this? Is it really
recommended/required to repost the complete patch series upon a small
change in only one patch? No problem, I can do this. patman makes it
very easy. :)

Should I repost the complete series again?

Thanks,
Stefan

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Wolfgang Denk]

Dear Stefan Roese,

In message <508FA904.4070402@denx.de> you wrote:
> 
> >> ---
> >> Changes in v6:
> >> - Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined
...
> 
> As you know this patch is part of a patch-series. And this is the first
> time that this patch has a change. So this summary covers the complete
> history for this patch.

But exactly this is information which I do not have, and which is not
included in your patch.  As is, I can only intepret this to be version
6 of this specific commit, and I wonder which changes were made in the
previous 5 versions.


> In this version of the patch series, I only made this small change to
> this patch 1/7. I wanted to spare the list a resending of the complete
> patchset for such a small change.
> 
> So what is the recommended way to do this? Is it really
> recommended/required to repost the complete patch series upon a small
> change in only one patch? No problem, I can do this. patman makes it
> very easy. :)
> 
> Should I repost the complete series again?

No, not at all!


I understand you are using patman for patch management.  So I added
Simon on Cc: to have his oponion, too.

I see two options:

1) Versioning is done on a per-patch base.  In this case, this patch
   should have been submitted as "[PATCH v2 1/7]", in which case it
   would have been clear to everybody that this is the first and only
   change compared to previous submission(s).

   I don't dare to say "most", but at least some people have worked
   like this when submitting patch series (manually) in the past.
   
   I can understand if somebody argues that it is not exactly easy to
   collect the correspondign patches to a series if individual patches
   contain different version numbers.  Correct threading of the
   messages is essential here.

2) Versioning is done on a per-series base.

   One problem here is that it becomes difficult to keep track if
   what is what when only single patches of the series change and get
   reposted - on the other hand it has always been a major PITA when
   people repost whole series after only changing a line of two in on
   of their many patches, so we strongly encourage posting of only the
   changed patches.  Once more, proper threading appears to be
   essential.

   Another problem is what we are running into here: after severl
   versions of the patch series one patch that has been untouches
   previously gets changed.  Now it gets posted as "V6", and it's
   impossible to know how many previous versions of this patch might
   have been posted before - one? 2? 3? 4? or 5?

   When the version ID refers to the patch series rather than to the
   individual patch, then I think it is mandatory to take this into
   consideration in the patch history, whih then must refer to all
   versions of the _series_.  In the present case, the patch history
   should have looked like this:

	V2: no changes
	V3: no changes
	V4: no changes
	V5: no changes
	V6: Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined


Is there any clear majority of preferences for patch versioning?
My gut feeling is that more people would like version IDs on a
per-series base, but I would like to see some confirmation for this,
and the we should document such expectations.


It appears that patman is oriented toward using a single version ID
per series.  Simon - would it be possible to automatically add such
"no changes" information when generating the patches?



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
I will also, for an appropriate fee, certify that  your  keyboard  is
object-oriented,  and  that  the bits on your hard disk are template-
compatible.            - Jeffrey S. Haemer in <411akr$3ga@cygnus.com>

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Stefan Roese]

Hi Wolfgang,

On 10/30/2012 12:05 PM, Wolfgang Denk wrote:
>> As you know this patch is part of a patch-series. And this is the first
>> time that this patch has a change. So this summary covers the complete
>> history for this patch.
> 
> But exactly this is information which I do not have, and which is not
> included in your patch.  As is, I can only intepret this to be version
> 6 of this specific commit, and I wonder which changes were made in the
> previous 5 versions.

*If* we agree upon a per patch series versioning (see below), then this
would be enough. To only list the changes that have been made to this
patch. Your suggestion from below is even better. To document that no
changes have been made:

	V2: no changes
	...

I'm pretty sure that Simon (or other people with a bit of python
knowledge) can easily add this to patman.

>> In this version of the patch series, I only made this small change to
>> this patch 1/7. I wanted to spare the list a resending of the complete
>> patchset for such a small change.
>>
>> So what is the recommended way to do this? Is it really
>> recommended/required to repost the complete patch series upon a small
>> change in only one patch? No problem, I can do this. patman makes it
>> very easy. :)
>>
>> Should I repost the complete series again?
> 
> No, not at all!

Okay.

> I understand you are using patman for patch management.  So I added
> Simon on Cc: to have his oponion, too.
> 
> I see two options:
> 
> 1) Versioning is done on a per-patch base.  In this case, this patch
>    should have been submitted as "[PATCH v2 1/7]", in which case it
>    would have been clear to everybody that this is the first and only
>    change compared to previous submission(s).
> 
>    I don't dare to say "most", but at least some people have worked
>    like this when submitting patch series (manually) in the past.
>    
>    I can understand if somebody argues that it is not exactly easy to
>    collect the correspondign patches to a series if individual patches
>    contain different version numbers.  Correct threading of the
>    messages is essential here.

Yes, this is my main concern about option a). Very hard to match the
single patches (and its versions) to the patch series version. Without
proper threading. And I personally don't use threading in my mail client
(my problem, I know).

> 2) Versioning is done on a per-series base.
> 
>    One problem here is that it becomes difficult to keep track if
>    what is what when only single patches of the series change and get
>    reposted - on the other hand it has always been a major PITA when
>    people repost whole series after only changing a line of two in on
>    of their many patches, so we strongly encourage posting of only the
>    changed patches.  Once more, proper threading appears to be
>    essential.
> 
>    Another problem is what we are running into here: after severl
>    versions of the patch series one patch that has been untouches
>    previously gets changed.  Now it gets posted as "V6", and it's
>    impossible to know how many previous versions of this patch might
>    have been posted before - one? 2? 3? 4? or 5?
> 
>    When the version ID refers to the patch series rather than to the
>    individual patch, then I think it is mandatory to take this into
>    consideration in the patch history, whih then must refer to all
>    versions of the _series_.  In the present case, the patch history
>    should have looked like this:
> 
> 	V2: no changes
> 	V3: no changes
> 	V4: no changes
> 	V5: no changes
> 	V6: Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined
> 
> 
> Is there any clear majority of preferences for patch versioning?
> My gut feeling is that more people would like version IDs on a
> per-series base, but I would like to see some confirmation for this,
> and the we should document such expectations.

As you have already guessed, I'm in favoring the 2nd option, versioning
on a per-series base.

What do other developers have to say? What's the recommended way to do
this in the Linux world? Even if we don't need to do everything in the
same way as done in Linux development, it is much easier to do it in a
similar fashion for users working in both projects (U-Boot & Linux)
regularly.

> It appears that patman is oriented toward using a single version ID
> per series.  Simon - would it be possible to automatically add such
> "no changes" information when generating the patches?

A little motivation: Simon, you could earn yourself another beer the
next time we meet! ;)

Thanks,
Stefan

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Simon Glass]

Hi Wolfgang, Stefan,

On Tue, Oct 30, 2012 at 6:33 AM, Stefan Roese <sr@denx.de> wrote:
> Hi Wolfgang,
>
> On 10/30/2012 12:05 PM, Wolfgang Denk wrote:
>>> As you know this patch is part of a patch-series. And this is the first
>>> time that this patch has a change. So this summary covers the complete
>>> history for this patch.
>>
>> But exactly this is information which I do not have, and which is not
>> included in your patch.  As is, I can only intepret this to be version
>> 6 of this specific commit, and I wonder which changes were made in the
>> previous 5 versions.
>
> *If* we agree upon a per patch series versioning (see below), then this
> would be enough. To only list the changes that have been made to this
> patch. Your suggestion from below is even better. To document that no
> changes have been made:
>
>         V2: no changes
>         ...
>
> I'm pretty sure that Simon (or other people with a bit of python
> knowledge) can easily add this to patman.

[snip]

>> It appears that patman is oriented toward using a single version ID
>> per series.  Simon - would it be possible to automatically add such
>> "no changes" information when generating the patches?
>
> A little motivation: Simon, you could earn yourself another beer the
> next time we meet! ;)

Sold :-) It's pretty trivial I think - I will take a look.

Re the threading, and this is to some extent a separate issue, if I am
resending a single patch, I sometimes copy in the message ID when
patman (actually git send-email, called by patman) asks for it. We
could perhaps automate this - in two ways:

1. Patman could automatically send only the patches that have changed
for v6 (I suggest that unless this is combined with some sort of
automatic patchwork state change, it could get a bit tricky for
maintainers since they will be applying many different patch versions
in a series). At present you have to manually type 'y' or 'n' to each
patch.

2. Patman could (with a bit of effort) attach the message ID for the
previous version of the patch to the 'in reply to' tag of the next
version. This would mean that each patch would be in reply to its
earlier version. My understanding was that this was not desirable, and
that it is better to have the series stand alone. I recall some
discussion on this.

It may be that neither of these is desirable.

>
> Thanks,
> Stefan

Regards,
Simon

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/30/12 06:33, Stefan Roese wrote:
> Hi Wolfgang,
> 
> On 10/30/2012 12:05 PM, Wolfgang Denk wrote:
[snip]
>> 2) Versioning is done on a per-series base.
>> 
>> One problem here is that it becomes difficult to keep track if 
>> what is what when only single patches of the series change and
>> get reposted - on the other hand it has always been a major PITA
>> when people repost whole series after only changing a line of two
>> in on of their many patches, so we strongly encourage posting of
>> only the changed patches.  Once more, proper threading appears to
>> be essential.
>> 
>> Another problem is what we are running into here: after severl 
>> versions of the patch series one patch that has been untouches 
>> previously gets changed.  Now it gets posted as "V6", and it's 
>> impossible to know how many previous versions of this patch
>> might have been posted before - one? 2? 3? 4? or 5?
>> 
>> When the version ID refers to the patch series rather than to
>> the individual patch, then I think it is mandatory to take this
>> into consideration in the patch history, whih then must refer to
>> all versions of the _series_.  In the present case, the patch
>> history should have looked like this:
>> 
>> V2: no changes V3: no changes V4: no changes V5: no changes V6:
>> Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC"
>> redefined
>> 
>> 
>> Is there any clear majority of preferences for patch versioning? 
>> My gut feeling is that more people would like version IDs on a 
>> per-series base, but I would like to see some confirmation for
>> this, and the we should document such expectations.
> 
> As you have already guessed, I'm in favoring the 2nd option,
> versioning on a per-series base.
> 
> What do other developers have to say? What's the recommended way to
> do this in the Linux world? Even if we don't need to do everything
> in the same way as done in Linux development, it is much easier to
> do it in a similar fashion for users working in both projects
> (U-Boot & Linux) regularly.

So, I am in favor of per-series versioning.  I also prefer whole
reposting (which I believe to be the norm in the kernel) with a dash
of common sense (posting just 1/7 makes sense here) due to how
patchwork bundles work.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQkAP+AAoJENk4IS6UOR1WNMkP/i7oxy0L7W2N6h730IsDgE9g
tHlgbBUsm5pWnqBC8/57mOmsWWv+j3zgojc3OYMasIyAkHpAJ2yM7qoAFxeGmDzS
TyHvb2bswSIoL2pwlems2m0lfx3V7H8StRcZjpeztfbWRbQXIkKeAon0Bd5R+iwm
v/mIMR6Sdfq+x0klnRIjkO++652nKRQ+JAHLNWNVxZ6DpOqqLBtTAXXyYHLpBEKz
hGdrk2gzryoMAZKiKuiz5mgTmeHoBvsCkIiAsnYoZg94KXohvQUkQzVGV4WA4qOQ
1jk4v7vjGR3gg7c/gOjauwsTalv/6SGZ+f8kSUVV8zKBUOAXhckF0o2+nC70G74W
hoOiSuS2hTz//xkGWmLl9mANCK9iYm/RtQIj4NlrwYabmQ0oUpHRv2HU2C47tffD
u/9RrRqj4onjNR4GtYiy8M4iDFZSiVRpNF6TozxKWyXt2fG01tAPmkdy2mLqfdD5
Mbn0KKBV+/PkPrvzmX0xrFEJVhiMo4P4gUPESLADTX0Xd3oziVKpKjQTRIOpztIT
ib98JeA5fMKPQJJnMnKwldU/0gek5JYpqFihwZMPf5lXi2G3beAXRPTOgZGif621
dCyWbfMEd/fI6393wFvODLjzKT09Q1uf6KxMitrziUEZBB6QVZYRQMKmz5Adz9v0
E9tj91WHgpF9zXOhQgkq
=OWfZ
-----END PGP SIGNATURE-----

[U-Boot] [PATCH v6 1/7] powerpc: Extract EPAPR_MAGIC constants into processor.h

[Anatolij Gustschin]

On Tue, 30 Oct 2012 10:45:21 +0100
Stefan Roese <sr@denx.de> wrote:

> By extracting these defines into a header, they can be re-used by other
> C sources as well. This will be done by the SPL framework OS boot
> support.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
> Changes in v6:
> - Fix compile warning: release.S:354:0: warning: "EPAPR_MAGIC" redefined
> 
>  arch/powerpc/cpu/mpc85xx/release.S   | 1 -
>  arch/powerpc/include/asm/processor.h | 6 ++++++
>  arch/powerpc/lib/bootm.c             | 6 ------
>  3 files changed, 6 insertions(+), 7 deletions(-)

Applied to staging/agust at denx.de. Thanks!

Anatolij