memory: omap-gpmc: dump "before" state before first modification
PATCH 1/2 memory: omap-gpmc: dump "before" state before first modification
When gpmc_cs_show_timings is called in gpmc_cs_set_timings() gpmc_cs_program_settings() was already run which modifies the CONFIG1 register. So to be more useful do the "before" dump earlier.
--- drivers/memory/omap-gpmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/memory/omap-gpmc.c +1 -1
@@ -696,7 +696,6 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t,int div;u32 l; -gpmc_cs_show_timings (cs,"before gpmc_cs_set_timings" ); div =gpmc_calc_divider (t->sync_clk);if (div <0 )return div;
@@ -1988,6 +1987,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,if (ret <0 ) goto err; +gpmc_cs_show_timings (cs,"before gpmc_cs_program_settings" ); ret =gpmc_cs_program_settings (cs, &gpmc_s);if (ret <0 ) goto err;
Signature · 1 line
2.6.0
PATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Most register values for the chip select setup depend on the frequency of the fck clock.
So add a hint that the values setup by the bootloader might differ from the right setup for Linux if the bootloader uses a different frequency.
--- drivers/memory/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
drivers/memory/Kconfig +3 -0
@@ -64,6 +64,9 @@ config OMAP_GPMC_DEBUG Enables verbose debugging mostly to decode the bootloader provided timings. Enable this during development to configure devices connected to the GPMC bus. + Note that you cannot just tweak your device tree until the registers + setup by linux match what the bootloader did because that one might + use a different fck frequency influencing most register settings. config MVEBU_DEVBUS bool "Marvell EBU Device Bus Controller"
Signature · 1 line
2.6.0
PATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Quoted 15 lines · Uwe Kleine-K?nig
Most register values for the chip select setup depend on the frequency of the fck clock.
So add a hint that the values setup by the bootloader might differ from the right setup for Linux if the bootloader uses a different frequency.Signed-off-by Uwe Kleine-K?nig <[email protected]>--- drivers/memory/Kconfig | 3 +++ 1 file changed, 3 insertions(+)drivers/memory/Kconfig +0
-0
drivers/memory/Kconfig config OMAP_GPMC_DEBUG · line 69
Quoted 6 lines
drivers/memory/Kconfig +3
-0@@ -64,6 +64,9 @@ config OMAP_GPMC_DEBUG Enables verbose debugging mostly to decode the bootloader provided timings. Enable this during development to configure devices connected to the GPMC bus. + Note that you cannot just tweak your device tree until the registers + setup by linux match what the bootloader did because that one might + use a different fck frequency influencing most register settings.
Looks like we can't know for sure the GPMC fclk used at the bootloader else we could have just printed the GPMC fclk pre and post gpmc settings.
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796
drivers/memory/Kconfig config OMAP_GPMC_DEBUG
Quoted 4 lines
drivers/memory/Kconfig +0
-0config MVEBU_DEVBUS bool "Marvell EBU Device Bus Controller"
cheers,
-roger
PATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Hello Roger,
Quoted 26 lines · Roger Quadros
Quoted 15 lines · Uwe Kleine-K?nig
Most register values for the chip select setup depend on the frequency of the fck clock.
So add a hint that the values setup by the bootloader might differ from the right setup for Linux if the bootloader uses a different frequency.Signed-off-by Uwe Kleine-K?nig <[email protected]>--- drivers/memory/Kconfig | 3 +++ 1 file changed, 3 insertions(+)drivers/memory/Kconfig +0
-0drivers/memory/Kconfig config OMAP_GPMC_DEBUG · line 69
Quoted 6 lines
drivers/memory/Kconfig +3
-0@@ -64,6 +64,9 @@ config OMAP_GPMC_DEBUG Enables verbose debugging mostly to decode the bootloader provided timings. Enable this during development to configure devices connected to the GPMC bus. + Note that you cannot just tweak your device tree until the registers + setup by linux match what the bootloader did because that one might + use a different fck frequency influencing most register settings.Looks like we can't know for sure the GPMC fclk used at the bootloader else we could have just printed the GPMC fclk pre and post gpmc settings.
Quoted 5 lines
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796
I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Thanks
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
Tony Lindgren
maintainerPATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Quoted 7 lines · Uwe Kleine-K?nig
Quoted 6 lines · Roger Quadros
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Quoted 6 lines
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Does the following look OK to you guys?
Regards,
Tony
8< ----------------
From: Tony Lindgren <[email protected]>
Date: Tue, 6 Oct 2015 05:36:17 -0700
Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
This probably happened because I had some uncommitted changes and the GPMC option is selected in the platform specific Kconfig.
Let's also update the description a bit, it does not mention that enabling the debug option also disables the reset of GPMC controller during the init as pointed out by Uwe Kleine-K?nig <[email protected]> and Roger Quadros <[email protected]>.
drivers/memory/Kconfig +9 -3
@@ -58,12 +58,18 @@ config OMAP_GPMC memory drives like NOR, NAND, OneNAND, SRAM. config OMAP_GPMC_DEBUG - bool + bool "Enable GPMC debug output and skip reset of GPMC during init" depends on OMAP_GPMC help Enables verbose debugging mostly to decode the bootloader provided - timings. Enable this during development to configure devices - connected to the GPMC bus. + timings. To preserve the bootloader provided timings, the reset + of GPMC is skipped during init. Enable this during development to + configure devices connected to the GPMC bus. + + NOTE: In addition to matching the register setup with the bootloader + you also need to match the GPMC FCLK frequency used by the + bootloader or else the GPMC timings won't be identical with the + bootloader timings. config MVEBU_DEVBUS bool "Marvell EBU Device Bus Controller"
PATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Hello Tony,
Quoted 12 lines · Tony Lindgren
Quoted 11 lines · Uwe Kleine-K?nig
Quoted 6 lines · Roger Quadros
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796
Quoted 7 lines
Quoted 2 lines
I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Does the following look OK to you guys?
Yes,
Thanks
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Quoted 18 lines · Uwe Kleine-K?nig
Hello Tony,
Quoted 15 lines · Tony Lindgren
Quoted 13 lines · Uwe Kleine-K?nig
Quoted 6 lines · Roger Quadros
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Quoted 6 lines
Quoted 4 lines
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Does the following look OK to you guys?
Yes,
Acked-by Uwe Kleine-K?nig <[email protected]>
Looks good to me too.
cheers,
-roger
Tony Lindgren
maintainerPATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Quoted 19 lines · Roger Quadros
Quoted 18 lines · Uwe Kleine-K?nig
Hello Tony,
Quoted 15 lines · Tony Lindgren
Quoted 13 lines · Uwe Kleine-K?nig
Quoted 6 lines · Roger Quadros
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Does the following look OK to you guys?
Yes,
Acked-by Uwe Kleine-K?nig <[email protected]>Looks good to me too.
Are you OK if I use your Acked-by from the previous version on this updated version?
Regards,
Tony
PATCH 2/2 memory: omap-gpmc: expand the description of the debug facility
Quoted 23 lines · Tony Lindgren
Quoted 22 lines · Roger Quadros
Quoted 21 lines · Uwe Kleine-K?nig
Hello Tony,
Quoted 18 lines · Tony Lindgren
Quoted 7 lines · Uwe Kleine-K?nig
Quoted 6 lines · Roger Quadros
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Quoted 6 lines
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Does the following look OK to you guys?
Yes,
Acked-by Uwe Kleine-K?nig <[email protected]>Looks good to me too.
Are you OK if I use your Acked-by from the previous version on this updated version?
Yes please.
cheers,
-roger
please strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors
In reply to Tony LindgrenHello,
when applying the mail below (without the '> ' prefix) using git am --scissors the result looks like:
$ git show commit 26ef0606927cc1979faa4166d7f9f3584b5cdc61 Author: Tony Lindgren <[email protected]> Date: Tue Oct 6 05:36:17 2015 -0700
memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
[...]
$ git version
git version 2.6.0
The obvious improvement is to strip all headers like git am does without --scissors.
If someone wants a bounce of the original mail, just ask per PM.
Best regards
Uwe
Quoted 72 lines · Tony Lindgren
Quoted 7 lines · Uwe Kleine-König
Quoted 6 lines · Roger Quadros
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Quoted 6 lines
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Does the following look OK to you guys?
Regards,
Tony
8< ----------------
From: Tony Lindgren <[email protected]>
Date: Tue, 6 Oct 2015 05:36:17 -0700
Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitCommit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.This probably happened because I had some uncommitted changes and the GPMC option is selected in the platform specific
Kconfig.Let's also update the description a bit, it does not mention that enabling the debug option also disables the reset of GPMC controller during the init as pointed out by Uwe Kleine-König <[email protected]> and Roger Quadros <[email protected]>.
Fixes 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug")Reported-by Uwe Kleine-König <[email protected]>Signed-off-by Tony Lindgren <[email protected]>drivers/memory/Kconfig +9
-3@@ -58,12 +58,18 @@ config OMAP_GPMC memory drives like NOR, NAND, OneNAND, SRAM. config OMAP_GPMC_DEBUG - bool + bool "Enable GPMC debug output and skip reset of GPMC during init" depends on OMAP_GPMC help Enables verbose debugging mostly to decode the bootloader provided - timings. Enable this during development to configure devices - connected to the GPMC bus. + timings. To preserve the bootloader provided timings, the reset + of GPMC is skipped during init. Enable this during development to + configure devices connected to the GPMC bus. + + NOTE: In addition to matching the register setup with the bootloader + you also need to match the GPMC FCLK frequency used by the + bootloader or else the GPMC timings won't be identical with the + bootloader timings. config MVEBU_DEVBUS bool "Marvell EBU Device Bus Controller"
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: please strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors
Quoted 19 lines · Uwe Kleine-König
Hello,
when applying the mail below (without the '> ' prefix) using git am --scissors the result looks like:
$ git show commit 26ef0606927cc1979faa4166d7f9f3584b5cdc61 Author: Tony Lindgren <[email protected]> Date: Tue Oct 6 05:36:17 2015 -0700memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitCommit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
Quoted 7 lines
[...]
$ git version
git version 2.6.0The obvious improvement is to strip all headers like git am does without --scissors.
Does this have anything to do with scissors, though? If you remove everything before "8< ---" in the body of Tony's message (i.e. keep the in-body headers starting with "From:" and ending with CTE) and try again, I would suspect that you will get the same result.
I also think that the "MIME-Version" thing is what gives this; mailinfo and am do not really use it, and consider that the in-body header ends there.
The right approach to tweak mailinfo to cope with this better would be to keep a bit more state inside mailinfo.c::handle_commit_msg() so that if we are (1) using in-body headers, (2) have already seen _some_ valid in-body header like "Subject:" and "From: ", and (3) have not seen a blank line, discard lines that we do not care about (e.g. "MIME-VERSION: 1.0").
Quoted 1 line
If someone wants a bounce of the original mail, just ask per PM.
I have no idea what you are talking about here...
Quoted 38 lines · Tony Lindgren
Quoted 37 lines · Uwe Kleine-König
Quoted 7 lines · Roger Quadros
Quoted 6 lines
How about this instead?
NOTE: Apart from matching the register setup with the bootloader you also need to match the GPMC FCLK frequency used by the bootloader else the GPMC timings won't be identical with the bootloader timings.
Quoted 6 lines
Yeah, sounds better, thanks.
Quoted 2 lines
Also you might need to build this patch on top of
http://article.gmane.org/gmane.linux.kernel/2054796I talked to Tony about this patch yesterday on irc, but I didn't find it in the archives yet when I sent my mail.
Yes sorry here's a repost with your and Roger's changes folded in and edited a bit. Probably best to keep them together with this patch.
Does the following look OK to you guys?
Regards,
Tony
8< ----------------
From: Tony Lindgren <[email protected]>
Date: Tue, 6 Oct 2015 05:36:17 -0700
Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitCommit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.This probably happened because I had some uncommitted changes and the
...
Re: please strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors
Hello Junio,
Quoted 26 lines · Junio C Hamano
Quoted 24 lines · Uwe Kleine-König
Hello,
when applying the mail below (without the '> ' prefix) using git am --scissors the result looks like:
$ git show commit 26ef0606927cc1979faa4166d7f9f3584b5cdc61 Author: Tony Lindgren <[email protected]> Date: Tue Oct 6 05:36:17 2015 -0700memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitCommit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.[...]
$ git version
git version 2.6.0
Quoted 7 lines
Quoted 2 lines
The obvious improvement is to strip all headers like git am does without --scissors.
Does this have anything to do with scissors, though? If you remove everything before "8< ---" in the body of Tony's message (i.e. keep the in-body headers starting with "From:" and ending with CTE) and try again, I would suspect that you will get the same result.
No, you're wrong here:
[email protected]:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS From: Tony Lindgren <[email protected]> Date: Tue, 6 Oct 2015 05:36:17 -0700 Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
[email protected]:~/gsrc/linux$ git am ~/tmp/1444332661.3982_89.ptx\:2\,RS Applying: memory: omap-gpmc: Fix unselectable debug option for GPMC [email protected]:~/gsrc/linux$ git cat-file commit HEAD tree bab01e3e0d0bdd715b86cf7d5c9e8bb9768a30dc parent c6fa8e6de3dc420cba092bf155b2ed25bcd537f7 author Tony Lindgren <[email protected]> 1444134977 -0700 committer Uwe Kleine-König <[email protected]> 1444332782 +0200
memory: omap-gpmc: Fix unselectable debug option for GPMC
Commit 63aa945b1013 ("memory: omap-gpmc: Add Kconfig option for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
This probably happened because I had some uncommitted changes and the GPMC option is selected in the platform specific Kconfig.
Let's also update the description a bit, it does not mention that enabling the debug option also disables the reset of GPMC controller during the init as pointed out by Uwe Kleine-König <[email protected]> and Roger Quadros <[email protected]>.
Quoted 3 lines
I also think that the "MIME-Version" thing is what gives this; mailinfo and am do not really use it, and consider that the in-body header ends there.
I failed to follow you here.
Quoted 6 lines
The right approach to tweak mailinfo to cope with this better would be to keep a bit more state inside
mailinfo.c::handle_commit_msg()so that if we are (1) using in-body headers, (2) have already seen _some_ valid in-body header like "Subject:" and "From: ", and (3) have not seen a blank line, discard lines that we do not care about (e.g. "MIME-VERSION: 1.0").
That sound's right.
Quoted 3 lines
Quoted 1 line
If someone wants a bounce of the original mail, just ask per PM.
I have no idea what you are talking about here...
The result would be that a copy of the original mail would hit your mailbox if you asked per private mail (PM).
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: please strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors
Quoted 14 lines · Uwe Kleine-König
Quoted 4 lines
Does this have anything to do with scissors, though? If you remove everything before "8< ---" in the body of Tony's message (i.e. keep the in-body headers starting with "From:" and ending with CTE) and try again, I would suspect that you will get the same result.
No, you're wrong here:
[email protected]:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS From: Tony Lindgren <[email protected]> Date: Tue, 6 Oct 2015 05:36:17 -0700 Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Quoted 2 lines
Commit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
I think you are the one who misread my question. I said "keep the in-body headers", didn't I? If you did the "head", then you would see something like this:
Quoted 13 lines
[email protected]:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS From: Tony Lindgren <[email protected]> Date: Tue, 6 Oct 2015 05:36:17 -0700 Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC ... probably Received: and all other junk from your mailbox ...From: Tony Lindgren <[email protected]>
Date: Tue, 6 Oct 2015 05:36:17 -0700
Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Quoted 8 lines
Commit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.Quoted 3 lines
I also think that the "MIME-Version" thing is what gives this; mailinfo and am do not really use it, and consider that the in-body header ends there.
I failed to follow you here.
I think if you tried the example with in-body header, you will see what I meant.
Quoted 9 lines
Quoted 7 lines
The right approach to tweak mailinfo to cope with this better would be to keep a bit more state inside
mailinfo.c::handle_commit_msg()so that if we are (1) using in-body headers, (2) have already seen _some_ valid in-body header like "Subject:" and "From: ", and (3) have not seen a blank line, discard lines that we do not care about (e.g. "MIME-VERSION: 1.0").That sound's right.
Re: please strip MIME-Version and Content-T{ype,ransfer-Encoding} in git am --scissors
Hello Junio,
Quoted 16 lines · Junio C Hamano
Quoted 14 lines · Uwe Kleine-König
Quoted 4 lines
Does this have anything to do with scissors, though? If you remove everything before "8< ---" in the body of Tony's message (i.e. keep the in-body headers starting with "From:" and ending with CTE) and try again, I would suspect that you will get the same result.
No, you're wrong here:
[email protected]:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS From: Tony Lindgren <[email protected]> Date: Tue, 6 Oct 2015 05:36:17 -0700 Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Quoted 6 lines
Quoted 2 lines
Commit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.I think you are the one who misread my question. I said "keep the in-body headers", didn't I? If you did the "head", then you would see something like this:
Ah got it. Yes, you're right. (Subject and Date are actually different between real and in-body headers, but that's not important. git am picks up the in-body headers.)
Quoted 16 lines
Quoted 15 lines
[email protected]:~/gsrc/linux$ head ~/tmp/1444332661.3982_89.ptx\:2\,RS From: Tony Lindgren <[email protected]> Date: Tue, 6 Oct 2015 05:36:17 -0700 Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC ... probably Received: and all other junk from your mailbox ...From: Tony Lindgren <[email protected]>
Date: Tue, 6 Oct 2015 05:36:17 -0700
Subject: [PATCH] memory: omap-gpmc: Fix unselectable debug option for GPMC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitCommit
63aa945b1013("memory: omap-gpmc: AddKconfigoption for debug") added a debug option for GPMC, but somehow managed to keep it unselectable.
Quoted 16 lines
Quoted 5 lines
Quoted 3 lines
I also think that the "MIME-Version" thing is what gives this; mailinfo and am do not really use it, and consider that the in-body header ends there.
I failed to follow you here.
I think if you tried the example with in-body header, you will see what I meant.
Quoted 7 lines
Quoted 7 lines
The right approach to tweak mailinfo to cope with this better would be to keep a bit more state inside
mailinfo.c::handle_commit_msg()so that if we are (1) using in-body headers, (2) have already seen _some_ valid in-body header like "Subject:" and "From: ", and (3) have not seen a blank line, discard lines that we do not care about (e.g. "MIME-VERSION: 1.0").
The right thing should also happen if MIME-Version comes above Subject in the body but other than that I'm with you here.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
PATCH 1/1 mailinfo: ignore in-body header that we do not care about
In reply to Junio C Hamano"git mailinfo" (hence "git am") understands some well-known headers, like "Subject: ", "Date: " and "From: ", placed at the beginning of the message body (and the "--scissors" can discard the part of the body before a scissors-mark). However, some people throw other kinds of header-looking things there, expecting them to be discarded.
Finding and discarding anything that looks like RFC2822 header is not a right solution. The body of the message may start with a line that begins with a word followed by a colon that is a legitimate part of the message that should not be discarded.
Instead, keep reading non-blank lines once we see an in-body header at the beginning and discard them. Nobody will be insane enough to reorder the headers to read like this:
Garbage-non-in-body-header: here
Subject: in-body subject
Here is the body of the commit log.
but it is common for lazy or misguided people to leave non-header materials in-body like this:
From: Junio C Hamano <[email protected]> Date: Mon, 28 Sep 2015 19:19:27 -0700 Subject: [PATCH] Git 2.6.1 MIME-Version: 1.0
---
I think it is wrong for the in-body header codepath to pay attention to content-transfer-encodings and stuff, but that is a separate issue.
Also if you remove the "does the line even look like a header?" check, some tests in t5100 starts failing. E.g.
From nobody Mon Sep 17 00:00:00 2001
From: A U Thor <[email protected]>
Subject: check bogus body header (from)
Date: Fri, 9 Jun 2006 00:44:16 -0700
From: bogosity
- a list
- of stuff
wants to make sure the list of two bulletted-items are in the commit log, and the in-body From: line gets used.
So I dunno. I am not entirely convinced that this is a good change.
builtin/mailinfo.c | 34 +++++++++++++++++++++++++++++++--- t/t5100-mailinfo.sh | 3 ++- t/t5100/info0018 | 5 +++++ t/t5100/msg0018 | 2 ++ t/t5100/patch0018 | 6 ++++++ t/t5100/sample.mbox | 18 ++++++++++++++++++ 6 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 t/t5100/info0018 create mode 100644 t/t5100/msg0018 create mode 100644 t/t5100/patch0018
builtin/mailinfo.c +31 -3
@@ -787,18 +787,46 @@ static int is_scissors_line(const struct strbuf *line)static int handle_commit_msg (struct strbuf *line) { +/* +* Are we still scanning and discarding in-body headers? +* It is initially set to 1, set to 2 when we do see a +* valid in-body header. +*/ static int still_looking =1 ; +int is_empty_line;if (!cmitmsg)return 0 ; -if (still_looking) { -if (!line->len || (line->len ==1 && line->buf[0 ] =='\n' )) + is_empty_line = (!line->len || (line->len ==1 && line->buf[0 ] =='\n' )); +if (still_looking ==1 ) { +/* +* Haven't seen a known in-body header; discard an empty line. +*/ +if (is_empty_line)return 0 ; }if (use_inbody_headers && still_looking) { - still_looking =check_header (line, s_hdr_data,0 ); +int is_known_header =check_header (line, s_hdr_data,0 ); + +if (still_looking ==2 ) { +/* +* an empty line after the in-body header block, +* or a line obviously not an attempt to invent +* an unsupported in-body header. +*/ +if (is_empty_line || !is_rfc2822_header (line)) + still_looking =0 ; +if (is_empty_line) +return 0 ; +/* otherwise do not discard the line, but keep going */ + }else if (is_known_header) { + still_looking =2 ; + }else if (still_looking !=2 ) { + still_looking =0 ; + } +if (still_looking)return 0 ; }else
t/t5100-mailinfo.sh +2 -1
@@ -11,7 +11,8 @@ test_expect_success 'split sample box' \ 'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last && last=`cat last` && echo total is $last && - test `cat last` = 17' + test `cat last` = 18 +' check_mailinfo () { mail=$1 opt=$2
t/t5100/info0018 new file +5 -0
@@ -0,0 +1,5 @@ +Author: A U Thor +Email: [email protected] +Subject: A E I O U +Date: Mon, 17 Sep 2012 14:23:49 -0700 +
t/t5100/msg0018 new file +2 -0
@@ -0,0 +1,2 @@ +New content here +
t/t5100/patch0018 new file +6 -0
@@ -0,0 +1,6 @@ +diff --git a/foo b/foo +index e69de29..d95f3ad 100644 +--- a/foo ++++ b/foo +@@ -0,0 +1 @@ ++New content
t/t5100/sample.mbox +18 -0
@@ -406,6 +406,7 @@ Subject: re: [PATCH] another patch From: A U Thor <[email protected]> Subject: [PATCH] another patch + >Here is an empty patch from A U Thor. Hey you forgot the patch!
@@ -699,3 +700,20 @@ index e69de29..d95f3ad 100644 +++ b/foo @@ -0,0 +1 @@ +New content +From nobody Mon Sep 17 00:00:00 2001 +From: A U Thor <[email protected]> +Subject: Re: some discussion title +Date: Mon, 17 Sep 2012 14:23:49 -0700 + +Subject: A E I O U +MIME-VERSION: 1.0 +Garbage: Not a valid in-body header + +New content here + +diff --git a/foo b/foo +index e69de29..d95f3ad 100644 +--- a/foo ++++ b/foo +@@ -0,0 +1 @@ ++New content
Signature · 1 line
2.6.1-296-ge15092e
PATCH 1/2 memory: omap-gpmc: dump "before" state before first modification
In reply to Uwe Kleine-KönigQuoted 5 lines · Uwe Kleine-K?nig
When
gpmc_cs_show_timingsis called ingpmc_cs_set_timings()gpmc_cs_program_settings()was already run which modifies the CONFIG1 register. So to be more useful do the "before" dump earlier.Signed-off-by Uwe Kleine-K?nig <[email protected]>
Quoted 18 lines
--- drivers/memory/omap-gpmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/memory/omap-gpmc.c +0
-1@@ -696,7 +696,6 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t,int div;u32 l; -gpmc_cs_show_timings (cs,"before gpmc_cs_set_timings" ); div =gpmc_calc_divider (t->sync_clk);if (div <0 )return div;
drivers/memory/omap-gpmc.c static int gpmc_probe_generic_child(struct platform_device *pdev, · line 1993
Quoted 7 lines
drivers/memory/omap-gpmc.c +1
-0@@ -1988,6 +1987,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,if (ret <0 ) goto err; +gpmc_cs_show_timings (cs,"before gpmc_cs_program_settings" ); ret =gpmc_cs_program_settings (cs, &gpmc_s);if (ret <0 ) goto err;
cheers,
-roger