Re: [PATCH 3/3] patman: Add a tag for when a patch gets added to a series

[Doug Anderson]

Hi,

On Thu, Apr 18, 2024 at 7:36 PM Sean Anderson  wrote:
>
> When a patch is added to a series after the initial version, there are no
> changes to note except that it is new. This is typically done to suppress
> the "(no changes in vN)" message. It's also nice to add a change to the
> cover letter so reviewers know there is an additional patch. Add a tag to
> automate this process a bit.
>
> There are two nits with the current approach:
>
> - It favors '-' as a bullet point, but some people may prefer '*' (or
>   something else)
> - Tags (e.g. 'patman: ' in 'patman: foo bar') are not stripped. They are
>   probably just noise in most series, but they may be useful for treewide
>   series to distinguish 'gpio: frobnicate' from 'reset: frobnicate', so
>   I've left them in.
>
> Suggestions for the above appreciated.
>
> Suggested-by: Douglas Anderson 
> Signed-off-by: Sean Anderson 
> ---
>
>  tools/patman/func_test.py   |  2 ++
>  tools/patman/patchstream.py |  5 +
>  tools/patman/patman.rst | 13 +
>  ...t-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch |  1 +
>  tools/patman/test/test01.txt|  1 +
>  5 files changed, 22 insertions(+)

I love it, thanks!

Reviewed-by: Douglas Anderson 

Re: [PATCH v7 2/2] arm64: boot: Support Flat Image Tree

[Doug Anderson]

Hi,

On Tue, Dec 5, 2023 at 3:16 AM Ahmad Fatoum  wrote:
>
> Hello,
>
> On 04.12.23 18:52, Doug Anderson wrote:> On Sat, Dec 2, 2023 at 8:37 AM Simon 
> Glass  wrote:
> >> On Thu, 30 Nov 2023 at 19:04, Ahmad Fatoum  wrote:
> >>> On 30.11.23 21:30, Simon Glass wrote:
> >>>> On Wed, 29 Nov 2023 at 12:54, Ahmad Fatoum  
> >>>> wrote:
> >>>>> On 29.11.23 20:44, Simon Glass wrote:
> >>>> I don't have an example to hand, but this is the required mechanism of
> >>>> FIT. This feature has been in place for many years and is used by
> >>>> ChromeOS, at least.
> >>>
> >>> I see the utility of a FIT configuration with
> >>>
> >>> compatible = "vendor,board-rev-a", "vendor,board-rev-b";
> >>>
> >>> I fail to see a utility for a configuration with
> >>>
> >>> compatible = "vendor,board", "vendor,SoM", "vendor,SoC";
> >>>
> >>> Any configuration that ends up being booted because "vendor,SoC" was 
> >>> matched is
> >>> most likely doomed to fail. Therefore, I would suggest that only the top 
> >>> level
> >>> configuration is written into the FIT configurations automatically.
> >>
> >> Firstly, I am not an expert on this.
> >>
> >> Say you have a board with variants. The compatible string in U-Boot
> >> may be something like:
> >>
> >> "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron",
> >> "rockchip,rk3288";
> >>
> >> If you then have several FIT configurations, they may be something like:
> >>
> >> "google,veyron-brain-rev0", "google,veyron-brain", "google,veyron",
> >> "rockchip,rk3288";
> >> "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron",
> >> "rockchip,rk3288";
> >> "google,veyron-brain-rev2", "google,veyron-brain", "google,veyron",
> >> "rockchip,rk3288";
> >>
> >> You want to choose the second one, since it is a better match than the 
> >> others.
>
> Now imagine, you are building a kernel that has no DT support for the Veyron,
> but instead has support for the Phytec RK3288 PCM-947:
>
>   phytec,rk3288-pcm-947", "phytec,rk3288-phycore-som", "rockchip,rk3288
>
> As far as I understand U-Boot code, A veyron U-Boot would boot the Phytec DT
> if CONFIG_FIT_BEST_MATCH is set, although it's a bad match and a boot failure
> should rather have occurred.

On depthcharge the bootloader never matches on just a SoC name.


> >> +Doug Anderson who knows a lot more about this than me.
> >
> > Hopefully this is all explained by:
> >
> > https://docs.kernel.org/arch/arm/google/chromebook-boot-flow.html
>
> Thanks Doug, this was helpful. The missing information to me was that
> depthcharge only compares the top-level board compatible in addition
> to runtime generated board compatibles, unlike what U-Boot seems to do.
>
> barebox only compares its top-level compatible against the FIT configuration
> compatibles, so adding a full compatible list to the configuration nodes as 
> done
> by this series should be ok there as well. I think U-Boot could run into
> issues though as described above.
>
> Out of curiosity: I only heard about Depthcharge before as exploitation 
> toolkit
> for U-Boot. Can you point me at some documentation on what the Depthcharge 
> bootloader
> does what U-Boot (which was presumably used before?) doesn't?

I can only assume that the depthcharge you're talking about is
different. The one used by Chromebooks is basically:

https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/refs/heads/main

I assume you're asking: why are we using depthcharge in ChromeOS
instead of U-Boot?

There was much discussion about this back in the day. From what I
recall, part of the reason was that folks wanted the boot flow to be a
bit more standard between x86 Chromebooks and ARM Chromebooks. x86
Chromebooks were using coreboot for the initial hardware booting and
then needed a 2nd stage to actually load Linux and ended up creating
depthcharge. ...and then we switched to the same model for ARM boards.

I didn't personally make that decision and I'm also not on the
firmware team, so that's about all I'll say on the topic. ;-)

Oh, hmmm. Searching found me:

https://www.chromium.org/chromium-os/developer-information-for-chrome-os-devices/custom-firmware/

...which pointed at:

https://www.chromium.org/chromium-os/2014-firmware-summit/ChromeOS%20firmware%20summit%20-%20Depthcharge.pdf

-Doug

Re: [PATCH v7 2/2] arm64: boot: Support Flat Image Tree

[Doug Anderson]

Hi,

On Sat, Dec 2, 2023 at 8:37 AM Simon Glass  wrote:
>
> Hi Ahmad,
>
> On Thu, 30 Nov 2023 at 19:04, Ahmad Fatoum  wrote:
> >
> > Hello Simon,
> >
> > On 30.11.23 21:30, Simon Glass wrote:
> > > On Wed, 29 Nov 2023 at 12:54, Ahmad Fatoum  
> > > wrote:
> > >> On 29.11.23 20:44, Simon Glass wrote:
> > >>> On Wed, 29 Nov 2023 at 12:33, Ahmad Fatoum  
> > >>> wrote:
> > >>>>
> > >>>> On 29.11.23 20:27, Simon Glass wrote:
> > >>>>> On Wed, 29 Nov 2023 at 12:15, Ahmad Fatoum  
> > >>>>> wrote:
> > >>>>>> On 29.11.23 20:02, Simon Glass wrote:
> > >>>>>>> On Wed, 29 Nov 2023 at 11:59, Ahmad Fatoum 
> > >>>>>>>  wrote:
> > >>>>>>>> The specification says that this is the root U-Boot compatible,
> > >>>>>>>> which I presume to mean the top-level compatible, which makes 
> > >>>>>>>> sense to me.
> > >>>>>>>>
> > >>>>>>>> The code here though adds all compatible strings from the device 
> > >>>>>>>> tree though,
> > >>>>>>>> is this intended?
> > >>>>>>>
> > >>>>>>> Yes, since it saves needing to read in each DT just to get the
> > >>>>>>> compatible stringlist.
> > >>>>>>
> > >>>>>> The spec reads as if only one string (root) is supposed to be in the 
> > >>>>>> list.
> > >>>>>> The script adds all compatibles though. This is not really useful as 
> > >>>>>> a bootloader
> > >>>>>> that's compatible with e.g. fsl,imx8mm would just take the first 
> > >>>>>> device tree
> > >>>>>> with that SoC, which is most likely to be wrong. It would be better 
> > >>>>>> to just
> > >>>>>> specify the top-level compatible, so the bootloader fails instead of 
> > >>>>>> taking
> > >>>>>> the first DT it finds.
> > >>>>>
> > >>>>> We do need to have a list, since we have to support different board 
> > >>>>> revs, etc.
> > >>>>
> > >>>> Can you give me an example? The way I see it, a bootloader with
> > >>>> compatible "vendor,board" and a FIT with configuration with 
> > >>>> compatibles:
> > >>>>
> > >>>>   "vendor,board-rev-a", "vendor,board"
> > >>>>   "vendor,board-rev-b", "vendor,board"
> > >>>>
> > >>>> would just result in the bootloader booting the first configuration, 
> > >>>> even if
> > >>>> the device is actually rev-b.
> > >>>
> > >>> You need to find the best match, not just any match. This is
> > >>> documented in the function comment for fit_conf_find_compat().
> > >>
> > >> In my above example, both configuration are equally good.
> > >> Can you give me an example where it makes sense to have multiple
> > >> compatibles automatically extracted from the device tree compatible?
> > >>
> > >> The way I see it having more than one compatible here just has
> > >> downsides.
> > >
> > > I don't have an example to hand, but this is the required mechanism of
> > > FIT. This feature has been in place for many years and is used by
> > > ChromeOS, at least.
> >
> > I see the utility of a FIT configuration with
> >
> > compatible = "vendor,board-rev-a", "vendor,board-rev-b";
> >
> > I fail to see a utility for a configuration with
> >
> > compatible = "vendor,board", "vendor,SoM", "vendor,SoC";
> >
> > Any configuration that ends up being booted because "vendor,SoC" was 
> > matched is
> > most likely doomed to fail. Therefore, I would suggest that only the top 
> > level
> > configuration is written into the FIT configurations automatically.
>
> Firstly, I am not an expert on this.
>
> Say you have a board with variants. The compatible string in U-Boot
> may be something like:
>
> "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
>
> If you then have several FIT configurations, they may be something like:
>
> "google,veyron-brain-rev0", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
> "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
> "google,veyron-brain-rev2", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
>
> You want to choose the second one, since it is a better match than the others.
>
> +Doug Anderson who knows a lot more about this than me.

Hopefully this is all explained by:

https://docs.kernel.org/arch/arm/google/chromebook-boot-flow.html

Re: [PATCH v2 4/4] patman: Check patches in parallel

[Doug Anderson]

Hi,

On Sun, Feb 19, 2023 at 3:50 PM Simon Glass  wrote:
>
> For large series this can take a while. Run checkpatch in parallel to
> try to reduce the time. The checkpatch information is still reported in
> sequential order, so a very slow patch at the start can still slow
> things down. But overall this gives good results.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  tools/patmanu/checkpatch.py | 46 +
>  1 file changed, 26 insertions(+), 20 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v2 3/4] patman: Run get_maintainer.pl in parallel

[Doug Anderson]

Hi,

On Sun, Feb 19, 2023 at 3:50 PM Simon Glass  wrote:
>
> This script can take ages on some series. Try to limit the time by
> using threads. If a few stubborn patches remain, show progress so the
> user has some idea what is going on.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  tools/patmanu/func_test.py |  2 ++
>  tools/patmanu/series.py| 33 ++---
>  2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/tools/patmanu/func_test.py b/tools/patmanu/func_test.py
> index 238fd5b6100..48109ae5725 100644
> --- a/tools/patmanu/func_test.py
> +++ b/tools/patmanu/func_test.py
> @@ -240,6 +240,8 @@ class TestFunctional(unittest.TestCase):
>  self.assertEqual('Change log missing for v3', next(lines))
>  self.assertEqual('Change log for unknown version v4', next(lines))
>  self.assertEqual("Alias 'pci' not found", next(lines))
> +while next(lines) != 'Cc processing complete':
> +pass
>  self.assertIn('Dry run', next(lines))
>  self.assertEqual('', next(lines))
>  self.assertIn('Send a total of %d patches' % count, next(lines))
> diff --git a/tools/patmanu/series.py b/tools/patmanu/series.py
> index 8ead87ef53e..e7a5f91da87 100644
> --- a/tools/patmanu/series.py
> +++ b/tools/patmanu/series.py
> @@ -5,8 +5,11 @@
>  from __future__ import print_function
>
>  import collections
> +import concurrent.futures
>  import itertools
>  import os
> +import sys
> +import time
>
>  from patmanu import get_maintainer
>  from patmanu import gitutil
> @@ -302,10 +305,34 @@ class Series(dict):
>  fd = open(fname, 'w', encoding='utf-8')
>  all_ccs = []
>  all_skips = set()
> +with concurrent.futures.ThreadPoolExecutor(max_workers=16) as 
> executor:
> +for i, commit in enumerate(self.commits):
> +commit.seq = i
> +commit.future = executor.submit(
> +self.GetCcForCommit, commit, process_tags, warn_on_error,
> +add_maintainers, limit, get_maintainer_script, all_skips)
> +
> +# Show progress any commits that are taking forever
> +lastlen = 0
> +while True:
> +left = [commit for commit in self.commits
> +if not commit.future.done()]
> +if not left:
> +break
> +names = ', '.join(f'{c.seq + 1}:{c.subject}'
> +  for c in left[:2])
> +out = f'\r{len(left)} remaining: {names}'[:79]
> +spaces = ' ' * (lastlen - len(out))
> +if lastlen:  # Don't print anything the first time
> +print(out, spaces, end='')
> +sys.stdout.flush()
> +lastlen = len(out)
> +time.sleep(.25)
> +print(f'\rdone{" " * lastlen}\r', end='')
> +print('Cc processing complete')
> +
>  for commit in self.commits:
> -cc = self.GetCcForCommit(commit, process_tags, warn_on_error,
> - add_maintainers, limit,
> - get_maintainer_script, all_skips)
> +cc = commit.future.result()

I've never used "concurrent.futures" before, but looks reasonable to me.

Reviewed-by: Douglas Anderson 

Re: [PATCH v2 2/4] patman: Refactor MakeCcFile() into two functions

[Doug Anderson]

Hi,

On Sun, Feb 19, 2023 at 3:50 PM Simon Glass  wrote:
>
> @@ -234,6 +234,48 @@ class Series(dict):
>  str = 'Change log exists, but no version is set'
>  print(col.build(col.RED, str))
>
> +def GetCcForCommit(self, commit, process_tags, warn_on_error,
> +   add_maintainers, limit, get_maintainer_script,
> +   all_skips):
> +"""Get the email CCs to use with a particular commit
> +
> +Uses subject tags and get_maintainers.pl script to find people to cc
> +on a patch
> +
> +Args:
> +commit (Commit): Commit to process
> +process_tags (bool): Process tags as if they were aliases
> +warn_on_error (bool): True to print a warning when an alias 
> fails to
> +match, False to ignore it.
> +add_maintainers (bool or list of str): Either:
> +True/False to call the get_maintainers to CC maintainers
> +List of maintainers to include (for testing)
> +limit (int): Limit the length of the Cc list (None if no limit)
> +get_maintainer_script (str): The file name of the 
> get_maintainer.pl
> +script (or compatible).
> +all_skips (set of str): Set of bouncing email address that were
> +dropped from the output

It wouldn't hurt to mention that "all_skips" is essentially a return
value from this function (this function updates it to include the
email addresses that were skipped).


> @@ -259,28 +301,18 @@ class Series(dict):
>  fname = '/tmp/patman.%d' % os.getpid()
>  fd = open(fname, 'w', encoding='utf-8')
>  all_ccs = []
> +all_skips = set()
>  for commit in self.commits:
> -cc = []
> -if process_tags:
> -cc += gitutil.build_email_list(commit.tags,
> -   warn_on_error=warn_on_error)
> -cc += gitutil.build_email_list(commit.cc_list,
> -   warn_on_error=warn_on_error)
> -if type(add_maintainers) == type(cc):
> -cc += add_maintainers
> -elif add_maintainers:
> -
> -cc += get_maintainer.get_maintainer(get_maintainer_script,
> -commit.patch)
> -for x in set(cc) & set(settings.bounces):
> -print(col.build(col.YELLOW, 'Skipping "%s"' % x))
> -cc = list(set(cc) - set(settings.bounces))
> -if limit is not None:
> -cc = cc[:limit]
> +cc = self.GetCcForCommit(commit, process_tags, warn_on_error,
> + add_maintainers, limit,
> + get_maintainer_script, all_skips)
>  all_ccs += cc
>  print(commit.patch, '\0'.join(sorted(set(cc))), file=fd)
>  self._generated_cc[commit.patch] = cc
>
> +for x in sorted(list(all_skips)):

Why "sorted(list(all_skips))" and not just "sorted(all_skips)"?

Both of the above are nits, so I'm OK w/:

Reviewed-by: Douglas Anderson 

Re: [PATCH v2 1/4] patman: Drop an incorrect comment about git am

[Doug Anderson]

Hi,

On Sun, Feb 19, 2023 at 3:50 PM Simon Glass  wrote:
>
> Patman does not do this anymore, so drop the comment.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v2:
> - Fix 'uncorrect' typo in subject
>
>  tools/patmanu/control.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I guess this is as-of commit 7428dc14b0f2 ("patman: Remove the -a
option") ? It would be nice to mention that in the commit message...

Reviewed-by: Douglas Anderson 

Re: [PATCH v3 3/5] patman: Make most bool arguments BooleanOptionalAction

[Doug Anderson]

Hi,

On Sat, Jul 16, 2022 at 4:55 AM Simon Glass  wrote:
>
> Hi Doug,
>
> On Thu, 7 Jul 2022 at 09:26, Douglas Anderson  wrote:
> >
> > For boolean arguments it's convenient to be able to specify both the
> > argument and its opposite on the command line. This is especially
> > convenient because you can change the default via the settings file
> > and being able express the opposite can be the only way to override
> > things.
> >
> > Luckily python handles this well--we just need to specify things with
> > BooleanOptionalAction. We'll do that for all options except
> > "full-help" (where it feels silly). This uglifies the help text a
> > little bit but does give maximum flexibility.
> >
> > Signed-off-by: Douglas Anderson 
> > Tested-by: Brian Norris 
> > Reviewed-by: Brian Norris 
> > Reviewed-by: Simon Glass 
> > ---
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - Fix doc string for --ignore-bad-tags
> >
> >  tools/patman/main.py | 55 ++--
> >  1 file changed, 28 insertions(+), 27 deletions(-)
>
> Sadly this patch triggers an error:
>
> Traceback (most recent call last):
>   File "./tools/patman/patman", line 9, in 
> from argparse import ArgumentParser, BooleanOptionalAction
> ImportError: cannot import name 'BooleanOptionalAction' from
> 'argparse' (/usr/lib/python3.8/argparse.py)
>
>
> I've dropped it and the next one. Can you please take a look?

Ugh. I hadn't noticed that it was added in python 3.9. I guess we
don't want to require python 3.9+? Python 3.9 was released October
2020...

-Doug

Re: [PATCH v2 1/6] patman: Fix updating argument defaults from settings

[Doug Anderson]

Hi,

On Thu, Jul 7, 2022 at 8:11 AM Sean Anderson  wrote:
>
> On 7/6/22 8:07 PM, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Jul 5, 2022 at 11:16 AM Sean Anderson  
> > wrote:
> >>
> >> Hi Doug,
> >>
> >> On 7/1/22 4:23 PM, Douglas Anderson wrote:
> >> > Ever since commit 4600767d294d ("patman: Refactor how the default
> >> > subcommand works"), when I use patman on the Linux tree I get grumbles
> >> > about unknown tags. This is because the Linux default making
> >> > process_tags be False wasn't working anymore.
> >> >
> >> > It appears that the comment claiming that the defaults propagates
> >> > through all subparsers no longer works for some reason.
> >> >
> >> > We're already looking at all the subparsers anyway. Let's just update
> >> > each one.
> >> >
> >> > Fixes: 4600767d294d ("patman: Refactor how the default subcommand works")
> >> > Signed-off-by: Douglas Anderson 
> >> > Tested-by: Brian Norris 
> >> > Reviewed-by: Brian Norris 
> >> > ---
> >> >
> >> > (no changes since v1)
> >> >
> >> >  tools/patman/settings.py | 41 +---
> >> >  1 file changed, 22 insertions(+), 19 deletions(-)
> >> >
> >> > diff --git a/tools/patman/settings.py b/tools/patman/settings.py
> >> > index 7c2b5c196c06..5eefe3d1f55e 100644
> >> > --- a/tools/patman/settings.py
> >> > +++ b/tools/patman/settings.py
> >> > @@ -244,28 +244,31 @@ def _UpdateDefaults(main_parser, config):
> >> >if isinstance(action, argparse._SubParsersAction)
> >> >for _, subparser in action.choices.items()]
> >> >
> >> > +unknown_settings = set(name for name, val in 
> >> > config.items('settings'))
> >> > +
> >> >  # Collect the defaults from each parser
> >> > -defaults = {}
> >> >  for parser in parsers:
> >> >  pdefs = parser.parse_known_args()[0]
> >> > -defaults.update(vars(pdefs))
> >> > -
> >> > -# Go through the settings and collect defaults
> >> > -for name, val in config.items('settings'):
> >> > -if name in defaults:
> >> > -default_val = defaults[name]
> >> > -if isinstance(default_val, bool):
> >> > -val = config.getboolean('settings', name)
> >> > -elif isinstance(default_val, int):
> >> > -val = config.getint('settings', name)
> >> > -elif isinstance(default_val, str):
> >> > -val = config.get('settings', name)
> >> > -defaults[name] = val
> >> > -else:
> >> > -print("WARNING: Unknown setting %s" % name)
> >> > -
> >> > -# Set all the defaults (this propagates through all subparsers)
> >> > -main_parser.set_defaults(**defaults)
> >> > +defaults = dict(vars(pdefs))
> >> > +
> >> > +# Go through the settings and collect defaults
> >> > +for name, val in config.items('settings'):
> >> > +if name in defaults:
> >> > +default_val = defaults[name]
> >> > +if isinstance(default_val, bool):
> >> > +val = config.getboolean('settings', name)
> >> > +elif isinstance(default_val, int):
> >> > +val = config.getint('settings', name)
> >> > +elif isinstance(default_val, str):
> >> > +val = config.get('settings', name)
> >> > +defaults[name] = val
> >> > +unknown_settings.discard(name)
> >> > +
> >> > +# Set all the defaults
> >> > +parser.set_defaults(**defaults)
> >> > +
> >> > +for name in sorted(unknown_settings):
> >> > +print("WARNING: Unknown setting %s" % name)
> >>
> >> Can you see if 4780f7d8a6b ("patman: Fix defaults not propagating to
> >> subparsers") [1] addresses this problem? The implementation is different,
> >> but I believe these should have the same effect.
> >
> > To my mind the logic of your patch is a bit harder to follow, but I
> > believe you're correct that it accomplishes the same thing. ...and my
> > quick test also seems to confirm that yours works fine. Too bad it
> > wasn't already in "-next" or it would have saved me a bit of time...
> >
> > I'm curious whether you agree that the logic in my patch is a little
> > simpler. Should I re-post it as a squashed revert of yours and then
> > apply mine and call it a "simplify" instead of a bugfix? ...or just
> > leave yours alone? If we leave yours alone, I guess my patch #2 needs
> > a trivial rebase to fix a merge conflict.
>
> IMO my version is simpler, but that is mainly because I thought of it.
>
> I have no objection to your rearranging, as long as it works afterwards.

No worries then. I'll drop my patch #1 and post a rebase of the rest
of the series.

-Doug

Re: [PATCH v2 1/6] patman: Fix updating argument defaults from settings

[Doug Anderson]

Hi,

On Tue, Jul 5, 2022 at 11:16 AM Sean Anderson  wrote:
>
> Hi Doug,
>
> On 7/1/22 4:23 PM, Douglas Anderson wrote:
> > Ever since commit 4600767d294d ("patman: Refactor how the default
> > subcommand works"), when I use patman on the Linux tree I get grumbles
> > about unknown tags. This is because the Linux default making
> > process_tags be False wasn't working anymore.
> >
> > It appears that the comment claiming that the defaults propagates
> > through all subparsers no longer works for some reason.
> >
> > We're already looking at all the subparsers anyway. Let's just update
> > each one.
> >
> > Fixes: 4600767d294d ("patman: Refactor how the default subcommand works")
> > Signed-off-by: Douglas Anderson 
> > Tested-by: Brian Norris 
> > Reviewed-by: Brian Norris 
> > ---
> >
> > (no changes since v1)
> >
> >  tools/patman/settings.py | 41 +---
> >  1 file changed, 22 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/patman/settings.py b/tools/patman/settings.py
> > index 7c2b5c196c06..5eefe3d1f55e 100644
> > --- a/tools/patman/settings.py
> > +++ b/tools/patman/settings.py
> > @@ -244,28 +244,31 @@ def _UpdateDefaults(main_parser, config):
> >if isinstance(action, argparse._SubParsersAction)
> >for _, subparser in action.choices.items()]
> >
> > +unknown_settings = set(name for name, val in config.items('settings'))
> > +
> >  # Collect the defaults from each parser
> > -defaults = {}
> >  for parser in parsers:
> >  pdefs = parser.parse_known_args()[0]
> > -defaults.update(vars(pdefs))
> > -
> > -# Go through the settings and collect defaults
> > -for name, val in config.items('settings'):
> > -if name in defaults:
> > -default_val = defaults[name]
> > -if isinstance(default_val, bool):
> > -val = config.getboolean('settings', name)
> > -elif isinstance(default_val, int):
> > -val = config.getint('settings', name)
> > -elif isinstance(default_val, str):
> > -val = config.get('settings', name)
> > -defaults[name] = val
> > -else:
> > -print("WARNING: Unknown setting %s" % name)
> > -
> > -# Set all the defaults (this propagates through all subparsers)
> > -main_parser.set_defaults(**defaults)
> > +defaults = dict(vars(pdefs))
> > +
> > +# Go through the settings and collect defaults
> > +for name, val in config.items('settings'):
> > +if name in defaults:
> > +default_val = defaults[name]
> > +if isinstance(default_val, bool):
> > +val = config.getboolean('settings', name)
> > +elif isinstance(default_val, int):
> > +val = config.getint('settings', name)
> > +elif isinstance(default_val, str):
> > +val = config.get('settings', name)
> > +defaults[name] = val
> > +unknown_settings.discard(name)
> > +
> > +# Set all the defaults
> > +parser.set_defaults(**defaults)
> > +
> > +for name in sorted(unknown_settings):
> > +print("WARNING: Unknown setting %s" % name)
>
> Can you see if 4780f7d8a6b ("patman: Fix defaults not propagating to
> subparsers") [1] addresses this problem? The implementation is different,
> but I believe these should have the same effect.

To my mind the logic of your patch is a bit harder to follow, but I
believe you're correct that it accomplishes the same thing. ...and my
quick test also seems to confirm that yours works fine. Too bad it
wasn't already in "-next" or it would have saved me a bit of time...

I'm curious whether you agree that the logic in my patch is a little
simpler. Should I re-post it as a squashed revert of yours and then
apply mine and call it a "simplify" instead of a bugfix? ...or just
leave yours alone? If we leave yours alone, I guess my patch #2 needs
a trivial rebase to fix a merge conflict.

-Doug

Re: [U-Boot] [PATCH v2] patman: Use the Change-Id, version, and prefix in the Message-Id

[Doug Anderson]

Hi,

On Thu, Sep 26, 2019 at 6:50 PM Simon Glass  wrote:
>
> Hi Doug,
>
> On Tue, 3 Sep 2019 at 13:15, Douglas Anderson  wrote:
> >
> > As per the centithread on ksummit-discuss [1], there are folks who
> > feel that if a Change-Id is present in a developer's local commit that
> > said Change-Id could be interesting to include in upstream posts.
> > Specifically if two commits are posted with the same Change-Id there's
> > a reasonable chance that they are either the same commit or a newer
> > version of the same commit.  Specifically this is because that's how
> > gerrit has trained people to work.
> >
> > There is much angst about Change-Id in upstream Linux, but one thing
> > that seems safe and non-controversial is to include the Change-Id as
> > part of the string of crud that makes up a Message-Id.
> >
> > Let's give that a try.
> >
> > In theory (if there is enough adoption) this could help a tool more
> > reliably find various versions of a commit.  This actually might work
> > pretty well for U-Boot where (I believe) quite a number of developers
> > use patman, so there could be critical mass (assuming that enough of
> > these people also use a git hook that adds Change-Id to their
> > commits).  I was able to find this git hook by searching for "gerrit
> > change id git hook" in my favorite search engine.
> >
> > In theory one could imagine something like this could be integrated
> > into other tools, possibly even git-send-email.  Getting it into
> > patman seems like a sane first step, though.
> >
> > NOTE: this patch is being posted using a patman containing this patch,
> > so you should be able to see the Message-Id of this patch and see that
> > it contains my local Change-Id, which ends in 2b9 if you want to
> > check.
> >
> > [1] 
> > https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2019-August/006739.html
> >
> > Signed-off-by: Douglas Anderson 
> > ---
> >
> > Changes in v2:
> > - Add a "v" before the version part of the Message-Id
> > - Reorder the parts of the Message-Id as per Johannes.
> >
> >  tools/patman/README |  8 +-
> >  tools/patman/commit.py  |  3 ++
> >  tools/patman/patchstream.py | 57 +++--
> >  3 files changed, 65 insertions(+), 3 deletions(-)
>
> Sadly this seems to cause a test failure (patman --test). Can you please 
> check?

It's been so long since I worked on patman that I totally forgot about
patman --test.  Doh!

I've posted v3 and tests pass now.  Hopefully it looks OK?

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] patman: Use the Change-Id, version, and prefix in the Message-Id

[Doug Anderson]

Johannes,

On Thu, Sep 5, 2019 at 12:48 PM Johannes Berg  wrote:
>
> On Tue, 2019-09-03 at 13:15 -0700, Douglas Anderson wrote:
> >
> > Let's give that a try.
> >
> > In theory (if there is enough adoption) this could help a tool more
> > reliably find various versions of a commit.
>
> It's not quite as good as this (yet), but a very simple version of it
> for git-send-email could be this:
>
> --- save as .git/hooks/sendemail-validate ---
> #!/bin/sh
>
> set -e
>
> changeid=$(sed 's/^Change-Id: \(I.*\)$/\1/;t;d' $1)
> date=$(date +%s)
> sed 's/^Change-Id: I.*$//;T;d' -i $1
> sed "s/^From: /Message-Id: <$date-$changeid@changeid>\nFrom: /" -i $1
> #--- end script ---
>
>
> It won't do the RFC/which patch of a series etc. but I consider that
> less interesting.
>
> Actually, the important part for me is to be to be able to have change-
> ids locally (e.g. for working with gerrit), and not have that leak out
> to the community (that doesn't like them).
>
> As a maintainer in the community, I'll also need to change the
> .git/hooks/commit-msg script that comes with gerrit to not add a Change-
> Id if the commit comes with a Link: tag already, but that should be
> easy.

Do you have any idea how to encourage adoption of your git hook?
Would it make sense to do something like check it in to the Linux
kernel somewhere?

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH] patman: Use the Change-Id, version, and prefix in the Message-Id

[Doug Anderson]

Hi,

On Wed, Aug 28, 2019 at 2:05 PM Johannes Berg  wrote:
>
> Hi Doug,
>
> Some comments on the actual mechanics here (vs. on the kernel list I
> picked this up from).
>
> It seems like perhaps you could increase the "count" variable. You got:
>
> 20190828132723.0.RFC.Ie6289f437ae533d7fcaddfcee9202f0e92c6b2b9@changeid
>
> but usually patches are numbered starting from 1, not 0. Especially for
> series, it might be easier to understand if that was the same here.
>
> I'd probably also put it after the tag, so you'd have "RFC.1" instead of
> "0.RFC", I'm saying it mostly because I originally thought you somehow
> generated a timestamp with a decimal point :-)
>
> Obviously none of that really matters, but it'd be easier to understand
> (and if necessary reverse-engineer) IMHO.

I'm happy to make these changes--they seem sane to me.  NOTE that most
patches don't actually have a PREFIX but I purposely added one to this
patch to test it.  :-)  The next version will likely have a "version"
but no prefix, so with your suggestion I'll have:

datetimestring.2.1.Ie6289f437ae533d7fcaddfcee9202f0e92c6b2b9

I may add a "v" before the 2 in the next version also.  I was going to
do that but then I forgot to before I posted.  :-P  Thus:

datetimestring.v2.1.Ie6289f437ae533d7fcaddfcee9202f0e92c6b2b9

If I post v2 as an RFC also, it would be:

datetimestring.RFC.v2.1.Ie6289f437ae533d7fcaddfcee9202f0e92c6b2b9


For now I'll wait a few days before posting a v2 in case anyone wants
to provide additional feedback (or tell me that I'm a terrible person
for liking Change-Ids).


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] pwm: add MACRO to limit some code which only for rk3288

[Doug Anderson]

Hi,

On Mon, Jul 11, 2016 at 7:45 PM, Kever Yang  wrote:
> Hi Simon,
>
> CC Doug for this topic.
>
>
> On 07/12/2016 07:54 AM, Simon Glass wrote:
>>
>> Hi Kever,
>>
>> On 11 July 2016 at 00:58, Kever Yang  wrote:
>>>
>>> Hi Simon,
>>>
>>> On 07/09/2016 10:39 PM, Simon Glass wrote:

 Hi Kever,

 On 7 July 2016 at 20:45, Kever Yang  wrote:
>
> The grf setting for rkpwm is only need in rk3288, other SoCs like
> RK3399 which also use rkpwm do not need set the grf, let's add a
> MACRO to make the code only for RK3288.
>
> Change-Id: I167a4e8cf925e840d4bbbcfb1437aaed52b81477

 patman will automatically remove these for you.
>>>
>>> Will use patman for my new patches later, thanks.
>>>
> Signed-off-by: Kever Yang 
> ---
>drivers/pwm/rk_pwm.c | 8 
>1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pwm/rk_pwm.c b/drivers/pwm/rk_pwm.c
> index 2d289a4..e34adf0 100644
> --- a/drivers/pwm/rk_pwm.c
> +++ b/drivers/pwm/rk_pwm.c
> @@ -13,8 +13,10 @@
>#include 
>#include 
>#include 
> +#ifdef CONFIG_ROCKCHIP_RK3288
>#include 
>#include 
> +#endif
>#include 
>#include 
>
> @@ -22,7 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;
>
>struct rk_pwm_priv {
>   struct rk3288_pwm *regs;
> +#ifdef CONFIG_ROCKCHIP_RK3288
>   struct rk3288_grf *grf;
> +#endif
>};
>
>static int rk_pwm_set_config(struct udevice *dev, uint channel, uint
> period_ns,
> @@ -65,19 +69,23 @@ static int rk_pwm_ofdata_to_platdata(struct udevice
> *dev)
>   struct regmap *map;
>
>   priv->regs = (struct rk3288_pwm *)dev_get_addr(dev);
> +#ifdef CONFIG_ROCKCHIP_RK3288
>   map = syscon_get_regmap_by_driver_data(ROCKCHIP_SYSCON_GRF);
>   if (IS_ERR(map))
>   return PTR_ERR(map);
>   priv->grf = regmap_get_range(map, 0);
> +#endif

 I'd like to find a better way. Do all chips have a grf? If so then
 perhaps we can have a function like grf_enable_pwm() in the core SoC
 code and call it here. The #ifdef can be there.

 Or perhaps we should have a general soc.c, with its own struct
 containing pointers to grf, sgrf, etc. It can be set up at the start,
 and then provide a soc_enable_pwm() function to enable the pwm.

 What do you think?
>>>
>>> Every Rockchip soc have grf, and maybe sgrf, pmugrf which much like
>>> grf. The content in grf are very different for different SoC, it gathers
>>> all kinds of system/module control registers .
>>> Back to the grf setting for pwm, this control operation is only need in
>>> RK3288, not in RK3399 and new SoC further. so I think the '#ifdef' is
>>> better to stay in driver file like rk_pwm.c.
>>>
>>> For these system control registers, I'm sure the dedicate general soc.c
>>> is
>>> needed, because they are not so common for different module and different
>>> Soc. We are not able to have a common framework for them, a general soc.c
>>> won't help much except all the system control are gather in one file .
>>>
>>> I think the GRF setting is part of the module and driver, so we can leave
>>> it
>>> as it is,
>>> and a simple syscon driver is enough for grf like what is kernel do.
>>
>> I looked quickly at the Linux pwm driver but I don't see any grf
>> access there. How does the grf register get set in Linux? I really
>> want to avoid SoC-specific #ifdefs in drivers.
>
> Doug(in cc list) send the patch for this pwm setting, but it seems does not
> accept by upstream,
> I think people also don't like this grf access in driver and prefer this
> kind of one time init operation
> to be done in bootloader.
> patchset 1 implement in arch/arm/mach-rockchip/rockchip.c
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280064.html
> patchset 4 implement in drivers/pwm_rockchip.c
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/280715.html
>
> Hi Doug,
> Do you have any suggestion here?

Heiko will skin me for suggesting it, but one option would be to
consider this as a pinmux thing in the linux kernel.  The GRF can
choose between two IP blocks internally: the old PWM IP block or the
new PWM IP block.  ;)

I believe the other option is to handle atop Heiko's patches:

9131959 New  [1/3] dt-bindings: add used but undocumented
rockchip grf compatible values
9131963 New  [2/3] soc: rockchip: add driver handling grf setup
9131957 New  [3/3] ARM: rockchip: drop rk3288 jtag/mmc switch handling

I believe Heiko's patches implement the "grf subclass" talked about in
.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de

Re: [U-Boot] [PATCH v2] patman: make run results better visible

[Doug Anderson]

Vadim,

On Thu, Sep 4, 2014 at 10:45 AM, Vadim Bendebury vben...@chromium.org wrote:
 For an occasional user of patman some failures are not obvious: for
 instance when checkpatch reports warnings, the dry run still reports
 that the email would be sent. If it is not dry run, the warnings are
 shown on the screen, but it is not clear that the email was not sent.

 Add some code to report failure to send email explicitly.

 Tested by running the script on a patch with style violations,
 observed error messages in the script output.

 Signed-off-by: Vadim Bendebury vben...@chromium.org
 ---

 Changes in v2:
   - modified the error message for accuracy

  tools/patman/patman.py | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: make run results better visible

[Doug Anderson]

Vadim,

On Wed, Sep 3, 2014 at 12:16 PM, Vadim Bendebury vben...@chromium.org wrote:
 For an occasional user of patman some failures are not obvious: for
 instance when checkpatch reports warnings, the dry run still reports
 that the email would be sent. If it is not dry run, the warnings are
 shown on the screen, but it is not clear that the email was not sent.

 Add some code to report failure to send email explicitly.

 Tested by running the script on a patch with style violations,
 observed error messages in the script output.

 Signed-off-by: Vadim Bendebury vben...@chromium.org
 ---

  tools/patman/patman.py | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

 diff --git a/tools/patman/patman.py b/tools/patman/patman.py
 index c60aa5a..0163ccd 100755
 --- a/tools/patman/patman.py
 +++ b/tools/patman/patman.py
 @@ -154,13 +154,19 @@ else:

  # Email the patches out (giving the user time to check / cancel)
  cmd = ''
 -if ok or options.ignore_errors:
 +its_a_go = ok or options.ignore_errors
 +if its_a_go:
  cmd = gitutil.EmailPatches(series, cover_fname, args,
  options.dry_run, not options.ignore_bad_tags, cc_file,
  in_reply_to=options.in_reply_to)
 +else:
 +print col.Color(col.RED,
 +Not sending emails due to checkpatch 
 errors/warnings)

Technically it could be due to other problems, too (like errors applying).


  # For a dry run, just show our actions as a sanity check
  if options.dry_run:
  series.ShowActions(args, cmd, options.process_tags)
 +if not its_a_go:
 +print col.Color(col.RED, Email would not be sent)

  os.remove(cc_file)

Looks good to me, other than that.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: make run results better visible

[Doug Anderson]

Vadim,

On Wed, Sep 3, 2014 at 4:00 PM, Vadim Bendebury vben...@chromium.org wrote:
 On Wed, Sep 3, 2014 at 3:14 PM, Doug Anderson diand...@chromium.org wrote:
 Vadim,

 On Wed, Sep 3, 2014 at 12:16 PM, Vadim Bendebury vben...@chromium.org 
 wrote:
 For an occasional user of patman some failures are not obvious: for
 instance when checkpatch reports warnings, the dry run still reports
 that the email would be sent. If it is not dry run, the warnings are
 shown on the screen, but it is not clear that the email was not sent.

 Add some code to report failure to send email explicitly.

 Tested by running the script on a patch with style violations,
 observed error messages in the script output.

 Signed-off-by: Vadim Bendebury vben...@chromium.org
 ---

  tools/patman/patman.py | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

 diff --git a/tools/patman/patman.py b/tools/patman/patman.py
 index c60aa5a..0163ccd 100755
 --- a/tools/patman/patman.py
 +++ b/tools/patman/patman.py
 @@ -154,13 +154,19 @@ else:

  # Email the patches out (giving the user time to check / cancel)
  cmd = ''
 -if ok or options.ignore_errors:
 +its_a_go = ok or options.ignore_errors
 +if its_a_go:
  cmd = gitutil.EmailPatches(series, cover_fname, args,
  options.dry_run, not options.ignore_bad_tags, cc_file,
  in_reply_to=options.in_reply_to)
 +else:
 +print col.Color(col.RED,
 +Not sending emails due to checkpatch 
 errors/warnings)

 Technically it could be due to other problems, too (like errors applying).

 good point, what wording would you suggest?

You don't think that just removing the word checkpatch is enough.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: Only apply patches when we know the original HEAD

[Doug Anderson]

Hi,

On Tue, Jun 24, 2014 at 3:24 AM, Masahiro Yamada
yamad...@jp.panasonic.com wrote:
 OK. It seems better to check.

 I added some patches which include whitespace errors
 and ran Patman.

 It looks like nothing changes with/without -a option.


 $ tools/patman/patman -a -c 3
 1 warnings for 0002-arm-commit-2.patch:
 Line 21/0 has space before tab

 1 warnings for 0003-arm-commit-3.patch:
 Found possible blank line(s) at end of file 'None'

 Cleaned 3 patches
 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch:
 error: common/cmd_mmc.c,22: trailing whitespace

 1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch:
 error: common/cmd_mmc.c,26: code indent should use tabs where possible
 warning: common/cmd_mmc.c,26: please, no space before tabs
 warning: common/cmd_mmc.c,26: please, no spaces at the start of a line

 checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)


 $ tools/patman/patman  -c 3
 1 warnings for 0002-arm-commit-2.patch:
 Line 21/0 has space before tab

 1 warnings for 0003-arm-commit-3.patch:
 Found possible blank line(s) at end of file 'None'

 Cleaned 3 patches
 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch:
 error: common/cmd_mmc.c,22: trailing whitespace

 1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch:
 error: common/cmd_mmc.c,26: code indent should use tabs where possible
 warning: common/cmd_mmc.c,26: please, no space before tabs
 warning: common/cmd_mmc.c,26: please, no spaces at the start of a line

 checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)

It sure looks like checkpatch now catches things so it never gets to
git am.  Making -a off by default seems reasonable to me.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: Don't request full names from get_maintainer

[Doug Anderson]

Simon,

On Tue, May 13, 2014 at 11:17 AM, Simon Glass s...@chromium.org wrote:
 Hi Doug,

 On 18 April 2014 15:32, Doug Anderson diand...@chromium.org wrote:
 Simon,

 On Fri, Apr 18, 2014 at 1:43 PM, Simon Glass s...@chromium.org wrote:
 Hi Doug,

 On 17 April 2014 12:47, Doug Anderson diand...@chromium.org wrote:

 The Linux get_maintainer.pl can often produce a whole lot of results.
 As a result you'll sometimes blow your CC field over 1024 characters
 and that can cause listservs to reject your message.

 As a stopgap, call get_maintainer.pl with --non so it doesn't
 include real names.  This will dramatically reduce the number of
 characters.

 Signed-off-by: Doug Anderson diand...@chromium.org
 ---

  tools/patman/get_maintainer.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tools/patman/get_maintainer.py 
 b/tools/patman/get_maintainer.py
 index 00b4939..a5160bc 100644
 --- a/tools/patman/get_maintainer.py
 +++ b/tools/patman/get_maintainer.py
 @@ -43,5 +43,5 @@ def GetMaintainer(fname, verbose=False):
  print WARNING: Couldn't find get_maintainer.pl
  return []

 -stdout = command.Output(get_maintainer, '--norolestats', fname)
 +stdout = command.Output(get_maintainer, '--norolestats', '--non', 
 fname)
  return stdout.splitlines()
 --
 1.9.1.423.g4596e3a


 Good to avoid this problem, but I wonder if we should check the size
 and re-run the command if too long? That way we can keep names on the
 thread when it is possible.

 Also, why is there a limit on CC - is that a limitation described in
 the RFC, or just certain mailers? Can we get around it by specifying
 multiple Cc tags?

 I believe that we're actually fighting a heuristic that the mailing
 list servers have to avoid spammers.  In the past I've seen my mail
 get through to list servers that weren't run by kernel.org but _not_
 the the ones at kernel.org.  Kind of like how if you want to write
 code that's for generic exynos 5 hardware you need to use exynos_5yyy
 and _not_ replace the y with x (because that would be adult content!)

 We could possibly only run the heuristic if the CC list was  800
 characters?  I'd hesitate to getting too close to 1024 since I think
 there are instances where git will itself add a CC and that might blow
 us out.

 Yes that seems reasonable - that way we get the names most of the time.


 ...another option is to just forget this patch and force people to add
 this to .get_maintainer.conf

 I think the previous option is better.

I started coding this up and then realized one corner case that
wouldn't be handled: the cover letter case.  This is the most likely
message to go over the limit, even if individual patches are not over
the limit.  That's because we add all addresses to the cover letter in
(3118725 patman: Add all CC addresses to the cover letter).

If we want to try to handle this dynamically, we probably need to
handle it at a higher level and manually strip off the full names
right before passing it off to git.

What do you think?

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: Don't request full names from get_maintainer

[Doug Anderson]

Simon,

On Fri, Apr 18, 2014 at 1:43 PM, Simon Glass s...@chromium.org wrote:
 Hi Doug,

 On 17 April 2014 12:47, Doug Anderson diand...@chromium.org wrote:

 The Linux get_maintainer.pl can often produce a whole lot of results.
 As a result you'll sometimes blow your CC field over 1024 characters
 and that can cause listservs to reject your message.

 As a stopgap, call get_maintainer.pl with --non so it doesn't
 include real names.  This will dramatically reduce the number of
 characters.

 Signed-off-by: Doug Anderson diand...@chromium.org
 ---

  tools/patman/get_maintainer.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tools/patman/get_maintainer.py b/tools/patman/get_maintainer.py
 index 00b4939..a5160bc 100644
 --- a/tools/patman/get_maintainer.py
 +++ b/tools/patman/get_maintainer.py
 @@ -43,5 +43,5 @@ def GetMaintainer(fname, verbose=False):
  print WARNING: Couldn't find get_maintainer.pl
  return []

 -stdout = command.Output(get_maintainer, '--norolestats', fname)
 +stdout = command.Output(get_maintainer, '--norolestats', '--non', fname)
  return stdout.splitlines()
 --
 1.9.1.423.g4596e3a


 Good to avoid this problem, but I wonder if we should check the size
 and re-run the command if too long? That way we can keep names on the
 thread when it is possible.

 Also, why is there a limit on CC - is that a limitation described in
 the RFC, or just certain mailers? Can we get around it by specifying
 multiple Cc tags?

I believe that we're actually fighting a heuristic that the mailing
list servers have to avoid spammers.  In the past I've seen my mail
get through to list servers that weren't run by kernel.org but _not_
the the ones at kernel.org.  Kind of like how if you want to write
code that's for generic exynos 5 hardware you need to use exynos_5yyy
and _not_ replace the y with x (because that would be adult content!)

We could possibly only run the heuristic if the CC list was  800
characters?  I'd hesitate to getting too close to 1024 since I think
there are instances where git will itself add a CC and that might blow
us out.

...another option is to just forget this patch and force people to add
this to .get_maintainer.conf

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] patman: Don't request full names from get_maintainer

[Doug Anderson]

The Linux get_maintainer.pl can often produce a whole lot of results.
As a result you'll sometimes blow your CC field over 1024 characters
and that can cause listservs to reject your message.

As a stopgap, call get_maintainer.pl with --non so it doesn't
include real names.  This will dramatically reduce the number of
characters.

Signed-off-by: Doug Anderson diand...@chromium.org
---

 tools/patman/get_maintainer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/patman/get_maintainer.py b/tools/patman/get_maintainer.py
index 00b4939..a5160bc 100644
--- a/tools/patman/get_maintainer.py
+++ b/tools/patman/get_maintainer.py
@@ -43,5 +43,5 @@ def GetMaintainer(fname, verbose=False):
 print WARNING: Couldn't find get_maintainer.pl
 return []
 
-stdout = command.Output(get_maintainer, '--norolestats', fname)
+stdout = command.Output(get_maintainer, '--norolestats', '--non', fname)
 return stdout.splitlines()
-- 
1.9.1.423.g4596e3a

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fdt_support: Use CONFIG_NR_DRAM_BANKS if defined

[Doug Anderson]

Tom,

On Wed, May 15, 2013 at 9:51 AM, Doug Anderson diand...@chromium.org wrote:
 Vadim,

 On Wed, May 15, 2013 at 8:58 AM, Vadim Bendebury vben...@chromium.org wrote:
 This is not a big deal for u-boot (maybe very marginally inefficient
 when determining the actual memory size). Is this a big deal for
 kernel? I mean it is easy to squash these seven memory banks into one
 when filling out the memory node of the device tree, the question is
 is it even necessary?

 I think the kernel can go either way.  It can handle 1 big bank or 7
 banks.  The parts that were broken in the past were:
 * U-boot would refuse to tell the kernel about more than 4 banks
 (that's what my patch fixed).
 * The kernel choked if it was told about a bogus 8th bank that started
 at 0 and was 0 bytes big.

 What about if we just take my patch to support more than 4 banks
 (Vadim now has good justification for needing it)?  ...and then we'll
 fix our U-Boot not to tell the kernel about a bogus 8th bank (that was
 just a bug in our config file).

Do you think it would be OK to apply my patch now given Vadim's
justification of why we need 7 banks in U-Boot.  AKA: we need 7 banks
so banks are a power of 2 and all the same size (which U-Boot
assumes).

...or would you prefer not to have it and come up with some other solution?

Thanks!

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fdt_support: Use CONFIG_NR_DRAM_BANKS if defined

[Doug Anderson]

Tom,

On Fri, May 17, 2013 at 9:40 AM, Tom Rini tr...@ti.com wrote:
 I think my email must have been lost in the shuffle, see
 http://patchwork.ozlabs.org/patch/240687/

 So yes, I've got another fix in mind that should solve this and some
 other problems.

Saw your reply but don't completely understand it.  I think we'd still
like U-Boot to populate the memory property if possible and it sounds
like your patch would prevent that.  One reason is that we'd like to
be able to handle different memory configurations (one of which is
3.5G) with a single binary and have U-Boot just tell the kernel how
much space there is.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fdt_support: Use CONFIG_NR_DRAM_BANKS if defined

[Doug Anderson]

Tom,

On Fri, May 17, 2013 at 9:52 AM, Tom Rini tr...@ti.com wrote:
 Saw your reply but don't completely understand it.  I think we'd still
 like U-Boot to populate the memory property if possible and it sounds
 like your patch would prevent that.  One reason is that we'd like to
 be able to handle different memory configurations (one of which is
 3.5G) with a single binary and have U-Boot just tell the kernel how
 much space there is.

 So, I've been assuming that these are different platforms that you
 already append different device trees on, so that you use the same
 binary still and just different DTs.  Is that not the case here?

Current thought is that we'll even share a device tree between the two
boards since differences between the two are very minimal.  Sorta like
how you can buy a Galaxy Nexus with 8G or 16G.  They're the same
device (as far as I know) just with a different eMMC part stuffed on.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fdt_support: Use CONFIG_NR_DRAM_BANKS if defined

[Doug Anderson]

Vadim,

On Wed, May 15, 2013 at 8:58 AM, Vadim Bendebury vben...@chromium.org wrote:
 This is not a big deal for u-boot (maybe very marginally inefficient
 when determining the actual memory size). Is this a big deal for
 kernel? I mean it is easy to squash these seven memory banks into one
 when filling out the memory node of the device tree, the question is
 is it even necessary?

I think the kernel can go either way.  It can handle 1 big bank or 7
banks.  The parts that were broken in the past were:
* U-boot would refuse to tell the kernel about more than 4 banks
(that's what my patch fixed).
* The kernel choked if it was told about a bogus 8th bank that started
at 0 and was 0 bytes big.

What about if we just take my patch to support more than 4 banks
(Vadim now has good justification for needing it)?  ...and then we'll
fix our U-Boot not to tell the kernel about a bogus 8th bank (that was
just a bug in our config file).
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] fdt_support: Use CONFIG_NR_DRAM_BANKS if defined

[Doug Anderson]

It appears that there are some cases where we have more than 4 banks
of memory.  Use CONFIG_NR_DRAM_BANKS if it's defined to handle this.
This will take up a little extra stack space (64 bytes extra if we go
up to 8 banks), but that seems OK.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Note: nothing in-tree has 8 banks defined yet, but some configs have
it defined that are not in tree yet.

 common/fdt_support.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 812acb4..416100e 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -387,7 +387,11 @@ static void write_cell(u8 *addr, u64 val, int size)
}
 }
 
+#ifdef CONFIG_NR_DRAM_BANKS
+#define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
+#else
 #define MEMORY_BANKS_MAX 4
+#endif
 int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
 {
int err, nodeoffset;
-- 
1.8.2.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fdt_support: Use CONFIG_NR_DRAM_BANKS if defined

[Doug Anderson]

Tom,

On Tue, Apr 30, 2013 at 1:35 PM, Tom Rini tr...@ti.com wrote:
 And I guess having this knowledge correct for the kernel is useful in
 other contexts like when we want to power down some banks of memory
 but not others?  I mean, there's lots of platforms that lie and say
 1 bank since we require contiguous mapping.  Thanks!

Thanks for the review!

At the moment I'm _not_ convinced that there's a good reason to
specify 8 banks.  We appear to have lied and said 1 bank on
exynos5250-snow (ARM Chromebook) and I don't know of any bad side
effects.

The code I'm looking at right now indicates 8 banks.  We need to track
down why someone did that but it doesn't seem totally crazy to allow
specifying the proper number of banks so I figured I'd send this patch
up.

If you prefer, we can leave this patch hanging until we actually track
down if specifying 8 banks was really needed.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: Ignore all Gerrit Commit-* tags

[Doug Anderson]

Simon,

On Wed, Apr 3, 2013 at 2:01 PM, Simon Glass s...@chromium.org wrote:
 These tags are used by Gerrit, so let's ignore all of them.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/patchstream.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/7] patman: Fix up checkpatch parsing to deal with 'CHECK' lines

[Doug Anderson]

Simon,

On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass s...@chromium.org wrote:
 checkpatch has a new type of warning, a 'CHECK'. At present patman fails
 with these, which makes it less than useful.

 Add support for checks, making it backwards compatible with the old
 checkpatch.

 At the same time, clean up formatting of the CheckPatches() output,
 fix erroneous internal error if multiple patches have warnings and
 be more robust to new types of problems.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
 Changes in v2:
 - Update code to remove match_count
 - Update commit message to add detail on what is changing
 - Update tests to check the 'checks' value, and add a new test
 - Use namedtuple to hold the return value from CheckPatch() function

  tools/patman/checkpatch.py | 110 
 -
  tools/patman/test.py   |  64 +-
  2 files changed, 112 insertions(+), 62 deletions(-)

Thanks for fixing and for fixing up the tests as well.

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 2/7] patman: Don't allow spaces in tags

[Doug Anderson]

Simon,

On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass s...@chromium.org wrote:
 At present something like:

Revert arm: Add cache operations

 will try to use

Revert arm

 as a tag. Clearly this is wrong, so fix it.

 If the revert is intended to be tagged, then the tag can come before
 the revert, perhaps. Alternatively the 'Cc' tag can be used in the commit
 messages.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
 Changes in v2:
 - Adjust to not allow any spaces in tags, change commit title

  tools/patman/commit.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 6/7] patman: Add Series-process-log tag to sort/uniq change logs

[Doug Anderson]

Simon,

On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass s...@chromium.org wrote:
 For some series with lots of changes it is annoying that duplicate change
 log items are not caught. It is also helpful sometimes to sort the change
 logs.

 Add a Series-process-log tag to enable this, which can be placed in a
 commit to control this.

 The change to the Cc: line is to fix a checkpatch warning.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
 Changes in v2:
 - Require sort, uniq tags to be comma-separated

  tools/patman/README | 9 -
  tools/patman/patchstream.py | 2 +-
  tools/patman/series.py  | 9 +++--
  3 files changed, 16 insertions(+), 4 deletions(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 4/7] patman: Provide option to ignore bad aliases

[Doug Anderson]

Simon,

On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass s...@chromium.org wrote:
 Often it happens that patches include tags which don't have aliases. It
 is annoying that patman fails in this case, and provides no option to
 continue other than adding empty tags to the .patman file.

 Correct this by adding a '-t' option to ignore tags that don't exist.
 Print a warning instead.

 Since running the tests is not a common operation, move this to --test
 instead, to reserve -t for this new option.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
 Changes in v2:
 - Add comment about meaning of raise_on_error=False
 - Remove 'ignore_errors' and just use 'raise_on_error' everywhere
 - Use keyword args for raise_on_error

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] PATMAN: set In-Reply-To header?

[Doug Anderson]

Wolfgang,

On Sun, Mar 24, 2013 at 3:41 PM, Simon Glass s...@chromium.org wrote:
 is there a way in patman to set the In-Reply-To header, or other
 methods to make sure re-submitted patches are properly threaded?

 I think Doug sent a patch for this recently. I will check with him.

Yup.  It's here http://patchwork.ozlabs.org/patch/228330/.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/8] patman: Fix up checkpatch parsing to deal with 'CHECK' lines

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass s...@chromium.org wrote:
 checkpatch has a new type of warning, a 'CHECK'. At present patman fails
 with these, which makes it less than useful.

 Add support for checks, making it backwards compatible with the old
 checkpatch.

There are also a few other minor fixups in this:
* Cleanup formatting of the CheckPatches() output.
* Fix erroneous internal error if multiple patches have warnings.
* Be more robust to new types of problems.


 @@ -64,10 +64,14 @@ def CheckPatch(fname, verbose=False):
  'msg': text message
  'file' : filename
  'line': line number
 +error_count: Number of errors
 +warning_count: Number of warnings
 +check_count: Number of checks
  lines: Number of lines
 +stdout: Full output of checkpatch

nit: Right above this it says you're returning a 4-tuple.  That's no
longer true.  Could just remove the 4-.

optional: I've found that returning big tuples like this is
problematic.  Whenever you change the number of things returned you've
got to modify any callers that call like:

a, b, c, d = CheckPatch(...)

to now be:

a, b, c, d, e = CheckPatch(...)

...or never use the above syntax and use:

result = CheckPatch(...)
blah = result[0]

Maybe use a namedtuple so that callers can use the result more cleanly?


  if match:
  error_count = int(match.group(1))
  warning_count = int(match.group(2))
 -lines = int(match.group(3))
 +match_count = int(match.group(3))
 +if len(match.groups()) == 4:
 +   check_count = match_count
 +   lines = int(match.group(4))

I'm confused about match_count here.  What is it supposed to contain?
I can't tell from the name of it.  It looks like a temporary variable
holding either check_count or lines.  ...but you forget to assign
lines = match_count in an else case so things are broken with old
versions of checkpatch, right?


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/8] patman: Don't look for tags inside quotes

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass s...@chromium.org wrote:
  # Separates a tag: at the beginning of the subject from the rest of it
 -re_subject_tag = re.compile('([^:]*):\s*(.*)')
 +re_subject_tag = re.compile('([^:]*):\s*(.*)')

I'd go further and prevent all spaces.
  re_subject_tag = re.compile('([^:\s]*):\s*(.*)')

I ran into a similar problem with my patch I sent up earlier:

  patman: Add Cover-letter-cc: tag to Cc cover letter to people

I fixed that by removing the extra colon, but that was sorta lame.
We shouldn't have tags with spaces in them anyway...

  patman: Add Cover-letter-cc tag to Cc cover letter to people


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/8] patman: Minor help message/README fixes

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass s...@chromium.org wrote:
 A few of the help messages are not quite right, and there is a typo
 in the README. Fix these.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/README| 2 +-
  tools/patman/patman.py | 4 ++--
  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/8] patman: Fix the comment in CheckTags to mention multiple tags

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass s...@chromium.org wrote:
 This comment is less than helpful. Since multiple tags are supported, add
 an example of how multiple tags work.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/commit.py | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 5/8] patman: Provide option to ignore bad aliases

[Doug Anderson]

Simon,

Nothing critical and this could go in as-is, but a few nits below.


On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass s...@chromium.org wrote:
 -raw += LookupEmail(item, alias)
 +raw += LookupEmail(item, alias, raise_on_error=not ignore_errors)

optional: Change it so functions are consistent about whether it's
raise_on_error or ignore_errors


 + EmailPatches(series, 'cover', ['p1', 'p2'], True, False, 'cc-fname', 
 \
 +False, alias)

Doctest that actually tests the raise_on_error?  See doctests in
gitutil.py for example syntax.


 -def LookupEmail(lookup_name, alias=None, level=0):
 +def LookupEmail(lookup_name, alias=None, raise_on_error=True, level=0):
  If an email address is an alias, look it up and return the full name

  TODO: Why not just use git's own alias feature?

  Args:
  lookup_name: Alias or email address to look up
 +alias: Dictionary containing aliases (None to use settings default)
 +raise_on_error: True to raise an error when an alias fails to match

...and what happens if you pass False?


 + LookupEmail('odd', alias, False)
 +\033[1;31mAlias 'odd' not found\033[0m
 +[]
 + # In this case the loop part will effectively be ignored.
 + LookupEmail('loop', alias, False)
 +\033[1;31mRecursive email alias at 'other'\033[0m
 +\033[1;31mRecursive email alias at 'john'\033[0m
 +\033[1;31mRecursive email alias at 'mary'\033[0m
 +['j.blo...@napier.co.nz', 'm.popp...@cloud.net']

optional nit: for optional args it's nice to specify them with keywords, like
   LookupEmail('loop', alias=alias, raise_on_error=False)

Please test the raise_on_error=True case.


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 6/8] patman: Add -a option to refrain from test-applying the patches

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass s...@chromium.org wrote:
 Especially with the Linux kernel, it takes a long time (a minute or more)
 to test-apply the patches, so patman becomes significantly less useful.
 The only real problem that is found with this apply step is trailing spaces.
 Provide a -a option to skip this step, for those working with clean patches.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/patman.py | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 8/8] patman: Add Series-process-log tag to sort/uniq change logs

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:43 PM, Simon Glass s...@chromium.org wrote:
 For some series with lots of changes it is annoying that duplicate change
 log items are not caught. It is also helpful sometimes to sort the change
 logs.

 Add a Series-process-log tag to enable this, which can be placed in a
 commit to control this.

 The change to the Cc: line is to fix a checkpatch warning.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/README | 8 +++-
  tools/patman/patchstream.py | 2 +-
  tools/patman/series.py  | 8 ++--
  3 files changed, 14 insertions(+), 4 deletions(-)

Not sure I'd find this terribly useful myself, but I don't see
anything wrong with it.  I think my change log items tend to be more
than one line long for one...

 +if not ('uniq' in process_it and text in out):

optional: My brain had a hard time processing this.  I did the logic
transformation myself:

  if 'uniq' not in process_it or text not in out:


Also: Do you really want the process_it to be so free-form?  That
seems like it might be asking for disaster.  Why not specify that it's
comma-separated and be done.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 7/8] patman: Add Cover-letter-cc tag to Cc cover letter to people

[Doug Anderson]

Simon,

On Wed, Mar 20, 2013 at 7:43 PM, Simon Glass s...@chromium.org wrote:
 The cover letter is sent to everyone who is on the Cc list for any of
 the patches in the series. Sometimes it is useful to send just the cover
 letter to additional people, so that they are aware of the series, but
 don't need to wade through all the individual patches.

 Add a new Cover-letter-cc tag for this purpose.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/README | 12 +++-
  tools/patman/patchstream.py |  8 
  tools/patman/series.py  | 11 ---
  3 files changed, 27 insertions(+), 4 deletions(-)

Identical to the already posted
http://patchwork.ozlabs.org/patch/222755/ so I'll repeat my existing
review.  ;)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] patman: Allow specifying the message ID your series is in reply to

[Doug Anderson]

Some versions of git don't seem to prompt you for the message ID that
your series is in reply to.  Allow specifying this from the command
line.

Signed-off-by: Doug Anderson diand...@chromium.org
Acked-by: Simon Glass s...@chromium.org
---
Changes in v2:
- Adjusted docstring wording as per Otavio Salvador.

 tools/patman/gitutil.py | 7 ++-
 tools/patman/patman.py  | 4 +++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index ca3ba4a..c35d209 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -203,7 +203,7 @@ def BuildEmailList(in_list, tag=None, alias=None):
 return result
 
 def EmailPatches(series, cover_fname, args, dry_run, cc_fname,
-self_only=False, alias=None):
+self_only=False, alias=None, in_reply_to=None):
 Email a patch series.
 
 Args:
@@ -213,6 +213,8 @@ def EmailPatches(series, cover_fname, args, dry_run, 
cc_fname,
 dry_run: Just return the command that would be run
 cc_fname: Filename of Cc file for per-commit Cc
 self_only: True to just email to yourself as a test
+in_reply_to: If set we'll pass this to git as --in-reply-to.
+Should be a message ID that this is in reply to.
 
 Returns:
 Git command that was/would be run
@@ -262,6 +264,9 @@ def EmailPatches(series, cover_fname, args, dry_run, 
cc_fname,
 to = BuildEmailList([os.getenv('USER')], '--to', alias)
 cc = []
 cmd = ['git', 'send-email', '--annotate']
+if in_reply_to:
+cmd.append('--in-reply-to=%s' % in_reply_to)
+
 cmd += to
 cmd += cc
 cmd += ['--cc-cmd', '%s --cc-cmd %s' % (sys.argv[0], cc_fname)]
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index e049081..377408d 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -53,6 +53,8 @@ parser.add_option('-n', '--dry-run', action='store_true', 
dest='dry_run',
 parser.add_option('-p', '--project', default=project.DetectProject(),
   help=Project name; affects default option values and 
   aliases [default: %default])
+parser.add_option('-r', '--in-reply-to', type='string', action='store',
+  help=Message ID that this series is in reply to)
 parser.add_option('-s', '--start', dest='start', type='int',
default=0, help='Commit to start creating patches from (0 = HEAD)')
 parser.add_option('-t', '--test', action='store_true', dest='test',
@@ -163,7 +165,7 @@ else:
 cmd = ''
 if ok or options.ignore_errors:
 cmd = gitutil.EmailPatches(series, cover_fname, args,
-options.dry_run, cc_file)
+options.dry_run, cc_file, in_reply_to=options.in_reply_to)
 
 # For a dry run, just show our actions as a sanity check
 if options.dry_run:
-- 
1.8.1.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] patman: Allow specifying the message ID your series is in reply to

[Doug Anderson]

Some versions of git don't seem to prompt you for the message ID that
your series is in reply to.  Allow specifying this from the command
line.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/gitutil.py | 7 ++-
 tools/patman/patman.py  | 4 +++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index ca3ba4a..f40bbb4 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -203,7 +203,7 @@ def BuildEmailList(in_list, tag=None, alias=None):
 return result
 
 def EmailPatches(series, cover_fname, args, dry_run, cc_fname,
-self_only=False, alias=None):
+self_only=False, alias=None, in_reply_to=None):
 Email a patch series.
 
 Args:
@@ -213,6 +213,8 @@ def EmailPatches(series, cover_fname, args, dry_run, 
cc_fname,
 dry_run: Just return the command that would be run
 cc_fname: Filename of Cc file for per-commit Cc
 self_only: True to just email to yourself as a test
+in_reply_to: If non-None we'll pass this to git as --in-reply-to.
+Should be a message ID that this is in reply to.
 
 Returns:
 Git command that was/would be run
@@ -262,6 +264,9 @@ def EmailPatches(series, cover_fname, args, dry_run, 
cc_fname,
 to = BuildEmailList([os.getenv('USER')], '--to', alias)
 cc = []
 cmd = ['git', 'send-email', '--annotate']
+if in_reply_to:
+cmd.append('--in-reply-to=%s' % in_reply_to)
+
 cmd += to
 cmd += cc
 cmd += ['--cc-cmd', '%s --cc-cmd %s' % (sys.argv[0], cc_fname)]
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index e049081..377408d 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -53,6 +53,8 @@ parser.add_option('-n', '--dry-run', action='store_true', 
dest='dry_run',
 parser.add_option('-p', '--project', default=project.DetectProject(),
   help=Project name; affects default option values and 
   aliases [default: %default])
+parser.add_option('-r', '--in-reply-to', type='string', action='store',
+  help=Message ID that this series is in reply to)
 parser.add_option('-s', '--start', dest='start', type='int',
default=0, help='Commit to start creating patches from (0 = HEAD)')
 parser.add_option('-t', '--test', action='store_true', dest='test',
@@ -163,7 +165,7 @@ else:
 cmd = ''
 if ok or options.ignore_errors:
 cmd = gitutil.EmailPatches(series, cover_fname, args,
-options.dry_run, cc_file)
+options.dry_run, cc_file, in_reply_to=options.in_reply_to)
 
 # For a dry run, just show our actions as a sanity check
 if options.dry_run:
-- 
1.8.1.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] patman: Make Reviewed-by an important tag

[Doug Anderson]

Although Reviewed-by: is a tag that gerrit adds, it's also a tag
used by upstream.  Stripping it is undesirable.  In fact, we should
treat it as important.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/README | 4 ++--
 tools/patman/patchstream.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 1832ebd..86d366f 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -198,8 +198,9 @@ END
override the default signoff that patman automatically adds.
 
  Tested-by: Their Name email
+ Reviewed-by: Their Name email
  Acked-by: Their Name email
-   These indicate that someone has acked or tested your patch.
+   These indicate that someone has tested/reviewed/acked your patch.
When you get this reply on the mailing list, you can add this
tag to the relevant commit and the script will include it when
you send out the next version. If 'Tested-by:' is set to
@@ -231,7 +232,6 @@ TEST=...
 Change-Id:
 Review URL:
 Reviewed-on:
-Reviewed-by:
 
 
 Exercise for the reader: Try adding some tags to one of your current
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index f7ee75a..4ff6ce7 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -31,7 +31,7 @@ from series import Series
 
 # Tags that we detect and remove
 re_remove = re.compile('^BUG=|^TEST=|^BRANCH=|^Change-Id:|^Review URL:'
-'|Reviewed-on:|Reviewed-by:|Commit-Ready:')
+'|Reviewed-on:|Commit-Ready:')
 
 # Lines which are allowed after a TEST= line
 re_allowed_after_test = re.compile('^Signed-off-by:')
@@ -46,7 +46,7 @@ re_cover = re.compile('^Cover-letter:')
 re_series = re.compile('^Series-(\w*): *(.*)')
 
 # Commit tags that we want to collect and keep
-re_tag = re.compile('^(Tested-by|Acked-by|Cc): (.*)')
+re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Cc): (.*)')
 
 # The start of a new commit in the git log
 re_commit = re.compile('^commit (.*)')
-- 
1.8.1.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] patman: Don't barf if the word 'commit' starts a line

[Doug Anderson]

Patman's regular expression for detecting the start of a
commit in a git log was a little simplistic and could be
confused if the git log itself had the word commit as
the start of a line (as this commit does).  Make patman
a little more robust.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/patchstream.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index f7ee75a..cf12362 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -49,7 +49,7 @@ re_series = re.compile('^Series-(\w*): *(.*)')
 re_tag = re.compile('^(Tested-by|Acked-by|Cc): (.*)')
 
 # The start of a new commit in the git log
-re_commit = re.compile('^commit (.*)')
+re_commit = re.compile('^commit ([0-9a-f]*)$')
 
 # We detect these since checkpatch doesn't always do it
 re_space_before_tab = re.compile('^[+].* \t')
-- 
1.8.1.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: Add Cover-letter-cc tag to Cc cover letter to people

[Doug Anderson]

Simon,

On Sat, Feb 23, 2013 at 8:15 PM, Simon Glass s...@chromium.org wrote:
 The cover letter is sent to everyone who is on the Cc list for any of
 the patches in the series. Sometimes it is useful to send just the cover
 letter to additional people, so that they are aware of the series, but
 don't need to wade through all the individual patches.

 Add a new Cover-letter-cc tag for this purpose.

 Signed-off-by: Simon Glass s...@chromium.org
 ---
  tools/patman/README | 12 +++-
  tools/patman/patchstream.py |  8 
  tools/patman/series.py  | 11 ---
  3 files changed, 27 insertions(+), 4 deletions(-)

Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/7] Add cros_ec driver.

[Doug Anderson]

Hung-ying Tyran,

On Fri, Feb 8, 2013 at 3:54 PM, Doug Anderson diand...@chromium.org wrote:
 Hung-ying Tyan,

 On Fri, Feb 8, 2013 at 5:05 AM, Hung-ying Tyan ty...@chromium.org wrote:
 Signed-off-by: Bernie Thompson bhthomp...@chromium.org
 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Che-Liang Chiou clch...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 Signed-off-by: Gabe Black gabebl...@chromium.org
 Signed-off-by: Hung-ying Tyan ty...@chromium.org
 Signed-off-by: Louis Yung-Chieh Lo yj...@chromium.org
 Signed-off-by: Randall Spangler rspang...@chromium.org
 Signed-off-by: Sean Paul seanp...@chromium.org
 Signed-off-by: Simon Glass s...@chromium.org
 Signed-off-by: Vincent Palatin vpala...@chromium.org
 ---
  doc/device-tree-bindings/input/cros-ec-keyb.txt |   79 ++
  doc/device-tree-bindings/misc/cros-ec.txt   |   38 +
  doc/device-tree-bindings/spi/exynos-spi.txt |   55 +
  drivers/misc/Makefile   |1 +
  drivers/misc/cros_ec.c  | 1303 
  include/cros_ec.h   |  449 +++
  include/cros_ec_message.h   |   44 +
  include/ec_commands.h   | 1440 
 +++
  include/fdtdec.h|1 +
  lib/fdtdec.c|1 +
  10 files changed, 3411 insertions(+)

 I think Simon Glass has already started upstreaming this driver (and
 the keyboard driver on top of it).  Perhaps you should refer to:
 http://patchwork.kernel.org/bundle/dianders/keyboard_3.8/

OK, so I'm an idiot.  I've been working in kernel land too long and
assumed that this was a kernel patch (and the paths are all so
similar).  Please disregard my message which was for the kernel
version of this driver.  Sigh.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/7] Add cros_ec driver.

[Doug Anderson]

Hung-ying Tyan,

On Fri, Feb 8, 2013 at 5:05 AM, Hung-ying Tyan ty...@chromium.org wrote:
 Signed-off-by: Bernie Thompson bhthomp...@chromium.org
 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Che-Liang Chiou clch...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 Signed-off-by: Gabe Black gabebl...@chromium.org
 Signed-off-by: Hung-ying Tyan ty...@chromium.org
 Signed-off-by: Louis Yung-Chieh Lo yj...@chromium.org
 Signed-off-by: Randall Spangler rspang...@chromium.org
 Signed-off-by: Sean Paul seanp...@chromium.org
 Signed-off-by: Simon Glass s...@chromium.org
 Signed-off-by: Vincent Palatin vpala...@chromium.org
 ---
  doc/device-tree-bindings/input/cros-ec-keyb.txt |   79 ++
  doc/device-tree-bindings/misc/cros-ec.txt   |   38 +
  doc/device-tree-bindings/spi/exynos-spi.txt |   55 +
  drivers/misc/Makefile   |1 +
  drivers/misc/cros_ec.c  | 1303 
  include/cros_ec.h   |  449 +++
  include/cros_ec_message.h   |   44 +
  include/ec_commands.h   | 1440 
 +++
  include/fdtdec.h|1 +
  lib/fdtdec.c|1 +
  10 files changed, 3411 insertions(+)

I think Simon Glass has already started upstreaming this driver (and
the keyboard driver on top of it).  Perhaps you should refer to:
http://patchwork.kernel.org/bundle/dianders/keyboard_3.8/

Specifically, see the patches:

ID  StateName
--  -
1870361 New  [1/5] mfd: Add ChromeOS EC messages header
1870371 New  [2/5] mfd: Add ChromeOS EC implementation
1870631 New  [3/5] mfd: Add ChromeOS EC I2C driver
1870401 New  [4/5] mfd: Add ChromeOS EC SPI driver
1870381 New  [5/5] Input: Add ChromeOS EC keyboard driver

1963511 New  [v2] input: Extend matrix-keypad device tree binding


Maybe you could review that series and send Reviewed-by and Tested-by
for it?  If there's anything missing from Simon's submission you could
add that.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] patman changes?

[Doug Anderson]

Hi,

I posted a bunch of patman changes a bit ago and they've all been acked by
Simon.  Any chance of them landing?  They're all visible here 
http://patchwork.ozlabs.org/bundle/dianders/patman/

...I think some of the changes have lost their acks on patchwork (I forgot
to include Simon's v1 ack on my v2 changes).  I'm happy to repost with his
acks if that helps.

Thanks!  :)

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Googlers please reply: commiters in U-Boot

[Doug Anderson]

I work for Google and have as long as I have had a chromium.org
account. Anything from diand...@google.com or diand...@chromium.org
can be attributed to Google.

On Sat, Jan 12, 2013 at 9:20 AM, Simon Glass s...@chromium.org wrote:
 Hi,

 You are being copied because you have written U-Boot code which is now
 in mainline.

 The chromium.org domain does not automatically attribute U-Boot
 commits by company. Each author needs to be manually added to the list
 and this can only be done if you confirm your employer.

 So, if you are on the CC list and work at Google, please reply-all
 with a quick email stating this (without top posting).

 Regards,
 Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Make patman usable outside of u-boot tree

[Doug Anderson]

Vadim,

Thanks for the patch!  Looks good in general, though please add the
patman prefix to the first line of your commit message.


On Wed, Jan 9, 2013 at 1:13 PM, Vadim Bendebury vben...@chromium.org wrote:
 To make it usable in git trees not providing a patch checker
 implementation, add a command line option, allowing to suippress patch

s/suippress/suppress

 +parser.add_option('--no-check', action='store_true', dest='no_check',
 +  default=False,
 +  help=Don't check for patch compliance)

IMHO It would be slightly better to use action='store_false',
dest='check', and default=True (just to avoid so many
double-negatives).


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] patman: Allow use outside of u-boot tree

[Doug Anderson]

Vadim,

On Wed, Jan 9, 2013 at 2:07 PM, Vadim Bendebury vben...@chromium.org wrote:
 +parser.add_option('--no-check', action='store_true', dest='no_check',
 +  default=False,
 +  help=Don't check for patch compliance)

 IMHO It would be slightly better to use action='store_false',
 dest='check', and default=True (just to avoid so many
 double-negatives).

 I don't quite agree with this part - I think it's perfectly reasonable
 to use 'no-check' to suppress the check, just as well as to use
 'no-tags' to suppress interpreting tags.

 `--no' communicates that by default the respective feature is enabled,
 and to disable it one needs to add a command line option with no
 parameter.

Sorry--should have been more explicit.  Was still expecting the option
to be --no-check.  Just asking for a change to the way it's stored.
Like this in the python dev guide:

parser.add_option(--clobber, action=store_true, dest=clobber)
parser.add_option(--no-clobber, action=store_false, dest=clobber)

In your case, I don't think you need to add the check option too,
but just store to the check option:

parser.add_option('--no-check', action='store_false', dest='check',
  default=True,
  help=Don't check for patch compliance)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] patman: Allow use outside of u-boot tree

[Doug Anderson]

Vadim,

Looks great with one last nit...

On Wed, Jan 9, 2013 at 4:23 PM, Vadim Bendebury vben...@chromium.org wrote:
 +
 +print  sys.stderr, ('Cannot find checkpatch.pl - please put it in your 
 ' +
 +'~/bin directory or use --no_patch')
 +sys.exit(1)

s/no_patch/no-patch
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] patman: Add all CC addresses to the cover letter

[Doug Anderson]

Simon,

Thanks for the review!

On Mon, Dec 3, 2012 at 3:00 PM, Simon Glass s...@chromium.org wrote:
 On Fri, Nov 30, 2012 at 4:25 PM, Doug Anderson diand...@chromium.org wrote:
 If we're sending a cover letter make sure to CC everyone that we're
 CCing on each of the individual patches.

 Signed-off-by: Doug Anderson diand...@chromium.org

 Looks good, but can you please add a note to the README under the
 'Where Patches Are Sent' header which mentions where the cover letter
 is sent?

Good suggestion.  Done.

 ---
  tools/patman/patman.py |2 +-
  tools/patman/series.py |   12 +++-
  2 files changed, 12 insertions(+), 2 deletions(-)

 diff --git a/tools/patman/patman.py b/tools/patman/patman.py
 index de8314a..4181d80 100755
 --- a/tools/patman/patman.py
 +++ b/tools/patman/patman.py
 @@ -140,7 +140,7 @@ else:
  options.count + options.start):
  ok = False

 -cc_file = series.MakeCcFile(options.process_tags)
 +cc_file = series.MakeCcFile(options.process_tags, cover_fname)

  # Email the patches out (giving the user time to check / cancel)
  cmd = ''
 diff --git a/tools/patman/series.py b/tools/patman/series.py
 index ad8288d..083af0f 100644
 --- a/tools/patman/series.py
 +++ b/tools/patman/series.py
 @@ -19,6 +19,7 @@
  # MA 02111-1307 USA
  #

 +import itertools
  import os

  import gitutil
 @@ -138,6 +139,9 @@ class Series(dict):
  print 'Prefix:\t ', self.get('prefix')
  if self.cover:
  print 'Cover: %d lines' % len(self.cover)
 +all_ccs = itertools.chain(*self._generated_cc.values())
 +for email in set(all_ccs):
 +print '  Cc: ',email
  if cmd:
  print 'Git command: %s' % cmd

 @@ -201,27 +205,33 @@ class Series(dict):
  str = 'Change log exists, but no version is set'
  print col.Color(col.RED, str)

 -def MakeCcFile(self, process_tags):
 +def MakeCcFile(self, process_tags, cover_fname):
  Make a cc file for us to use for per-commit Cc automation

  Also stores in self._generated_cc to make ShowActions() faster.

  Args:
  process_tags: Process tags as if they were aliases
 +cover_fname: If non-None the name of the cover letter.
  Return:
  Filename of temp file created
  
  # Look for commit tags (of the form 'xxx:' at the start of the 
 subject)
  fname = '/tmp/patman.%d' % os.getpid()
  fd = open(fname, 'w')
 +all_ccs = []
  for commit in self.commits:
  list = []
  if process_tags:
  list += gitutil.BuildEmailList(commit.tags)
  list += gitutil.BuildEmailList(commit.cc_list)
 +all_ccs += list
  print fd, commit.patch, ', '.join(list)
  self._generated_cc[commit.patch] = list

 +if cover_fname:
 +print fd, cover_fname, ', '.join(set(all_ccs))
 +
  fd.close()
  return fname

 --
 1.7.7.3


 Regards,
 Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/4] patman: Add the concept of multiple projects

[Doug Anderson]

Simon,

Thanks!

On Mon, Dec 3, 2012 at 3:04 PM, Simon Glass s...@chromium.org wrote:
 Hi Doug,

 On Fri, Nov 30, 2012 at 4:29 PM, Doug Anderson diand...@chromium.org wrote:
 There are cases that we want to support different settings (or maybe
 even different aliases) for different projects.  Add support for this
 by:
 * Adding detection for two big projects: U-Boot and Linux.
 * Adding default settings for Linux (U-Boot is already good with the
   standard patman defaults).
 * Extend the new settings feature in .patman to specify per-project
   settings.

 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  tools/patman/README  |   13 
  tools/patman/patman.py   |9 +++-
  tools/patman/project.py  |   43 +
  tools/patman/settings.py |  147 
 +-
  4 files changed, 208 insertions(+), 4 deletions(-)
  create mode 100644 tools/patman/project.py

 diff --git a/tools/patman/README b/tools/patman/README
 index 6ca5b5b..d294f3d 100644
 --- a/tools/patman/README
 +++ b/tools/patman/README
 @@ -114,6 +114,19 @@ verbose: True
  


 +If you want to adjust settings (or aliases) that affect just a single
 +project you can add a section that looks like [project_settings] or
 +[project_alias].  If you want to use tags for your linux work, you could
 +do:
 +
 +
 +
 +[linux_settings]
 +process_tags: True
 +
 +
 +
 +
  How to run it
  =

 diff --git a/tools/patman/patman.py b/tools/patman/patman.py
 index b327c67..54a252e 100755
 --- a/tools/patman/patman.py
 +++ b/tools/patman/patman.py
 @@ -34,6 +34,7 @@ import checkpatch
  import command
  import gitutil
  import patchstream
 +import project
  import settings
  import terminal
  import test
 @@ -59,6 +60,9 @@ parser.add_option('--cc-cmd', dest='cc_cmd', 
 type='string', action='store',
 default=None, help='Output cc list for patch file (used by git)')
  parser.add_option('--no-tags', action='store_false', dest='process_tags',
default=True, help=Don't process subject tags as aliaes)
 +parser.add_option('--project', default=project.DetectProject(),
 +  help=Project name; affects default option values and 
 +  aliases [default: %default])

 Can you please add a short option also - perhaps -p?

Done.

 [snip]

 Regards,
 Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/4] patman: Add the concept of multiple projects

[Doug Anderson]

There are cases that we want to support different settings (or maybe
even different aliases) for different projects.  Add support for this
by:
* Adding detection for two big projects: U-Boot and Linux.
* Adding default settings for Linux (U-Boot is already good with the
  standard patman defaults).
* Extend the new settings feature in .patman to specify per-project
  settings.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Added requested short option: '-p'.

 tools/patman/README  |   13 
 tools/patman/patman.py   |9 +++-
 tools/patman/project.py  |   43 +
 tools/patman/settings.py |  147 +-
 4 files changed, 208 insertions(+), 4 deletions(-)
 create mode 100644 tools/patman/project.py

diff --git a/tools/patman/README b/tools/patman/README
index 2743da9..1832ebd 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -114,6 +114,19 @@ verbose: True
 
 
 
+If you want to adjust settings (or aliases) that affect just a single
+project you can add a section that looks like [project_settings] or
+[project_alias].  If you want to use tags for your linux work, you could
+do:
+
+
+
+[linux_settings]
+process_tags: True
+
+
+
+
 How to run it
 =
 
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index b327c67..2e9e5dc 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -34,6 +34,7 @@ import checkpatch
 import command
 import gitutil
 import patchstream
+import project
 import settings
 import terminal
 import test
@@ -59,6 +60,9 @@ parser.add_option('--cc-cmd', dest='cc_cmd', type='string', 
action='store',
default=None, help='Output cc list for patch file (used by git)')
 parser.add_option('--no-tags', action='store_false', dest='process_tags',
   default=True, help=Don't process subject tags as aliaes)
+parser.add_option('-p', '--project', default=project.DetectProject(),
+  help=Project name; affects default option values and 
+  aliases [default: %default])
 
 parser.usage = patman [options]
 
@@ -66,7 +70,10 @@ Create patches from commits in a branch, check them and 
email them as
 specified by tags you place in the commits. Use -n to 
 
 
-settings.Setup(parser, '')
+# Parse options twice: first to get the project and second to handle
+# defaults properly (which depends on project).
+(options, args) = parser.parse_args()
+settings.Setup(parser, options.project, '')
 (options, args) = parser.parse_args()
 
 # Run our meagre tests
diff --git a/tools/patman/project.py b/tools/patman/project.py
new file mode 100644
index 000..4f7b2b3
--- /dev/null
+++ b/tools/patman/project.py
@@ -0,0 +1,43 @@
+# Copyright (c) 2012 The Chromium OS Authors.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+import os.path
+
+import gitutil
+
+def DetectProject():
+Autodetect the name of the current project.
+
+This looks for signature files/directories that are unlikely to exist 
except
+in the given project.
+
+Returns:
+The name of the project, like linux or u-boot.  Returns unknown
+if we can't detect the project.
+
+top_level = gitutil.GetTopLevel()
+
+if os.path.exists(os.path.join(top_level, include, u-boot)):
+return u-boot
+elif os.path.exists(os.path.join(top_level, kernel)):
+return linux
+
+return unknown
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 5208f7d..084d1b8 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -26,6 +26,140 @@ import re
 import command
 import gitutil
 
+Default settings per-project.
+
+These are used by _ProjectConfigParser.  Settings names should match
+the dest of the option parser from patman.py.
+
+_default_settings = {
+u-boot: {},
+linux: {
+process_tags: False,
+}
+}
+
+class _ProjectConfigParser(ConfigParser.SafeConfigParser):
+ConfigParser that handles projects.
+
+There are two main goals of this class:
+- Load project-specific default settings.
+- Merge general default settings/aliases with project-specific ones.
+
+# Sample config used for tests below...
+ import StringIO
+ sample_config

[U-Boot] [PATCH v2 4/4] patman: Add settings to the list of modules to doctest

[Doug Anderson]

The settings modules now has doctests, so run them.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2: None

 tools/patman/patman.py |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index 2e9e5dc..e56dd01 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -85,8 +85,9 @@ if options.test:
 result = unittest.TestResult()
 suite.run(result)
 
-suite = doctest.DocTestSuite('gitutil')
-suite.run(result)
+for module in ['gitutil', 'settings']:
+suite = doctest.DocTestSuite(module)
+suite.run(result)
 
 # TODO: Surely we can just 'print' result?
 print result
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/4] patman: Add support for settings in .patman

[Doug Anderson]

This patch adds support for a [settings] section in the .patman file.
In this section you can add settings that will affect the default
values for command-line options.

Support is added in a generic way such that any setting can be updated
by just referring to the dest of the option that is passed to the
option parser.  At the moment options that would make sense to put in
settings are ignore_errors, process_tags, and verbose.  You
could override them like:

 [settings]
 ignore_errors: True
 process_tags: False
 verbose: True

The settings functionality is also used in a future change which adds
support for per-project settings.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2: None

 tools/patman/README  |   16 
 tools/patman/gitutil.py  |2 --
 tools/patman/patman.py   |3 +++
 tools/patman/settings.py |   39 +++
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 16b51eb..2743da9 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -98,6 +98,22 @@ The checkpatch.pl in the U-Boot tools/ subdirectory will be 
located and
 used. Failing that you can put it into your path or ~/bin/checkpatch.pl
 
 
+If you want to change the defaults for patman's command-line arguments,
+you can add a [settings] section to your .patman file.  This can be used
+for any command line option by referring to the dest for the option in
+patman.py.  For reference, the useful ones (at the moment) shown below
+(all with the non-default setting):
+
+
+
+[settings]
+ignore_errors: True
+process_tags: False
+verbose: True
+
+
+
+
 How to run it
 =
 
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 72d37a0..e7753cf 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -377,8 +377,6 @@ def GetDefaultUserEmail():
 
 def Setup():
 Set up git utils, by reading the alias files.
-settings.Setup('')
-
 # Check for a git alias file also
 alias_fname = GetAliasFile()
 if alias_fname:
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index 4181d80..b327c67 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -34,6 +34,7 @@ import checkpatch
 import command
 import gitutil
 import patchstream
+import settings
 import terminal
 import test
 
@@ -64,6 +65,8 @@ parser.usage = patman [options]
 Create patches from commits in a branch, check them and email them as
 specified by tags you place in the commits. Use -n to 
 
+
+settings.Setup(parser, '')
 (options, args) = parser.parse_args()
 
 # Run our meagre tests
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 4dda17b..5208f7d 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -88,13 +88,43 @@ def CreatePatmanConfigFile(config_fname):
 print f, [alias]\nme: %s %s % (name, email)
 f.close();
 
-def Setup(config_fname=''):
+def _UpdateDefaults(parser, config):
+Update the given OptionParser defaults based on config.
+
+We'll walk through all of the settings from the parser
+For each setting we'll look for a default in the option parser.
+If it's found we'll update the option parser default.
+
+The idea here is that the .patman file should be able to update
+defaults but that command line flags should still have the final
+say.
+
+Args:
+parser: An instance of an OptionParser whose defaults will be
+updated.
+config: An instance of SafeConfigParser that we will query
+for settings.
+
+defaults = parser.get_default_values()
+for name, val in config.items('settings'):
+if hasattr(defaults, name):
+default_val = getattr(defaults, name)
+if isinstance(default_val, bool):
+val = config.getboolean('settings', name)
+elif isinstance(default_val, int):
+val = config.getint('settings', name)
+parser.set_default(name, val)
+else:
+print WARNING: Unknown setting %s % name
+
+def Setup(parser, config_fname=''):
 Set up the settings module by reading config files.
 
 Args:
+parser: The parser to update
 config_fname:   Config filename to read ('' for default)
 
-settings = ConfigParser.SafeConfigParser()
+config = ConfigParser.SafeConfigParser()
 if config_fname == '':
 config_fname = '%s/.patman' % os.getenv('HOME')
 
@@ -102,11 +132,12 @@ def Setup(config_fname=''):
 print No config file found ~/.patman\nCreating one...\n
 CreatePatmanConfigFile(config_fname)
 
-settings.read(config_fname)
+config.read(config_fname)
 
-for name, value in settings.items('alias'):
+for name, value in config.items('alias'):
 alias[name] = value.split(',')
 
+_UpdateDefaults(parser, config)
 
 # These are the aliases we understand, indexed by alias. Each

[U-Boot] [PATCH v2 1/4] patman: Add a call to get_maintainer.pl if it exists

[Doug Anderson]

For Linux the best way to figure out where to send a patch is with the
get_maintainer.pl script.  Add support for calling it from patman.
Support is added unconditionally for scripts/get_maintainer.pl in
case it is helpful for any other projects.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2: None

 tools/patman/README|   11 ++-
 tools/patman/get_maintainer.py |   63 
 tools/patman/series.py |2 +
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 tools/patman/get_maintainer.py

diff --git a/tools/patman/README b/tools/patman/README
index 5b6eba0..16b51eb 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -43,6 +43,9 @@ Series-to: fred.bl...@napier.co.nz
 
 in one of your commits, the series will be sent there.
 
+In Linux this will also call get_maintainer.pl on each of your
+patches automatically.
+
 
 How to use this tool
 
@@ -65,8 +68,12 @@ will get a consistent result each time.
 How to configure it
 ===
 
-For most cases patman will locate and use the file 'doc/git-mailrc' in
-your U-Boot directory. This contains most of the aliases you will need.
+For most cases of using patman for U-Boot developement patman will
+locate and use the file 'doc/git-mailrc' in your U-Boot directory.
+This contains most of the aliases you will need.
+
+For Linux the 'scripts/get_maintainer.pl' handles figuring out where
+to send patches pretty well.
 
 During the first run patman creates a config file for you by taking the default
 user name and email address from the global .gitconfig file.
diff --git a/tools/patman/get_maintainer.py b/tools/patman/get_maintainer.py
new file mode 100644
index 000..cb11373
--- /dev/null
+++ b/tools/patman/get_maintainer.py
@@ -0,0 +1,63 @@
+# Copyright (c) 2012 The Chromium OS Authors.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+import command
+import gitutil
+import os
+
+def FindGetMaintainer():
+Look for the get_maintainer.pl script.
+
+Returns:
+If the script is found we'll return a path to it; else None.
+
+try_list = [
+os.path.join(gitutil.GetTopLevel(), 'scripts'),
+]
+# Look in the list
+for path in try_list:
+fname = os.path.join(path, 'get_maintainer.pl')
+if os.path.isfile(fname):
+return fname
+
+return None
+
+def GetMaintainer(fname, verbose=False):
+Run get_maintainer.pl on a file if we find it.
+
+We look for get_maintainer.pl in the 'scripts' directory at the top of
+git.  If we find it we'll run it.  If we don't find get_maintainer.pl
+then we fail silently.
+
+Args:
+fname: Path to the patch file to run get_maintainer.pl on.
+
+Returns:
+A list of email addresses to CC to.
+
+get_maintainer = FindGetMaintainer()
+if not get_maintainer:
+if verbose:
+print WARNING: Couldn't find get_maintainer.pl
+return []
+
+stdout = command.Output(get_maintainer, '--norolestats', fname)
+return stdout.splitlines()
diff --git a/tools/patman/series.py b/tools/patman/series.py
index 083af0f..6c5c570 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -22,6 +22,7 @@
 import itertools
 import os
 
+import get_maintainer
 import gitutil
 import terminal
 
@@ -225,6 +226,7 @@ class Series(dict):
 if process_tags:
 list += gitutil.BuildEmailList(commit.tags)
 list += gitutil.BuildEmailList(commit.cc_list)
+list += get_maintainer.GetMaintainer(commit.patch)
 all_ccs += list
 print fd, commit.patch, ', '.join(list)
 self._generated_cc[commit.patch] = list
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/2] patman: Cache the CC list from MakeCcFile() for use in ShowActions()

[Doug Anderson]

Currently we go through and generate the CC list for patches twice.
This gets slow when (in a future CL) we add a call to
get_maintainer.pl on Linux.  Instead of doing things twice, just cache
the CC list when it is first generated.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2: None

 tools/patman/patman.py |6 --
 tools/patman/series.py |   13 +
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index cfe06d0..de8314a 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -140,14 +140,16 @@ else:
 options.count + options.start):
 ok = False
 
+cc_file = series.MakeCcFile(options.process_tags)
+
 # Email the patches out (giving the user time to check / cancel)
 cmd = ''
 if ok or options.ignore_errors:
-cc_file = series.MakeCcFile(options.process_tags)
 cmd = gitutil.EmailPatches(series, cover_fname, args,
 options.dry_run, cc_file)
-os.remove(cc_file)
 
 # For a dry run, just show our actions as a sanity check
 if options.dry_run:
 series.ShowActions(args, cmd, options.process_tags)
+
+os.remove(cc_file)
diff --git a/tools/patman/series.py b/tools/patman/series.py
index d2971f4..ad8288d 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -46,6 +46,11 @@ class Series(dict):
 self.notes = []
 self.changes = {}
 
+# Written in MakeCcFile()
+#  key: name of patch file
+#  value: list of email addresses
+self._generated_cc = {}
+
 # These make us more like a dictionary
 def __setattr__(self, name, value):
 self[name] = value
@@ -109,10 +114,7 @@ class Series(dict):
 for upto in range(len(args)):
 commit = self.commits[upto]
 print col.Color(col.GREEN, '   %s' % args[upto])
-cc_list = []
-if process_tags:
-cc_list += gitutil.BuildEmailList(commit.tags)
-cc_list += gitutil.BuildEmailList(commit.cc_list)
+cc_list = list(self._generated_cc[commit.patch])
 
 # Skip items in To list
 if 'to' in self:
@@ -202,6 +204,8 @@ class Series(dict):
 def MakeCcFile(self, process_tags):
 Make a cc file for us to use for per-commit Cc automation
 
+Also stores in self._generated_cc to make ShowActions() faster.
+
 Args:
 process_tags: Process tags as if they were aliases
 Return:
@@ -216,6 +220,7 @@ class Series(dict):
 list += gitutil.BuildEmailList(commit.tags)
 list += gitutil.BuildEmailList(commit.cc_list)
 print fd, commit.patch, ', '.join(list)
+self._generated_cc[commit.patch] = list
 
 fd.close()
 return fname
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/2] patman: Add all CC addresses to the cover letter

[Doug Anderson]

If we're sending a cover letter make sure to CC everyone that we're
CCing on each of the individual patches.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Added requested comment in the README to document this.

 tools/patman/README|3 +++
 tools/patman/patman.py |2 +-
 tools/patman/series.py |   12 +++-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index dc3957c..5b6eba0 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -226,6 +226,9 @@ Date:   Mon Nov 7 23:18:44 2011 -0500
 will create a patch which is copied to x86, arm, sandbox, mikef, ag and
 afleming.
 
+If you have a cover letter it will get sent to the union of the CC lists of
+all of the other patches.
+
 
 Example Work Flow
 =
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index de8314a..4181d80 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -140,7 +140,7 @@ else:
 options.count + options.start):
 ok = False
 
-cc_file = series.MakeCcFile(options.process_tags)
+cc_file = series.MakeCcFile(options.process_tags, cover_fname)
 
 # Email the patches out (giving the user time to check / cancel)
 cmd = ''
diff --git a/tools/patman/series.py b/tools/patman/series.py
index ad8288d..083af0f 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -19,6 +19,7 @@
 # MA 02111-1307 USA
 #
 
+import itertools
 import os
 
 import gitutil
@@ -138,6 +139,9 @@ class Series(dict):
 print 'Prefix:\t ', self.get('prefix')
 if self.cover:
 print 'Cover: %d lines' % len(self.cover)
+all_ccs = itertools.chain(*self._generated_cc.values())
+for email in set(all_ccs):
+print '  Cc: ',email
 if cmd:
 print 'Git command: %s' % cmd
 
@@ -201,27 +205,33 @@ class Series(dict):
 str = 'Change log exists, but no version is set'
 print col.Color(col.RED, str)
 
-def MakeCcFile(self, process_tags):
+def MakeCcFile(self, process_tags, cover_fname):
 Make a cc file for us to use for per-commit Cc automation
 
 Also stores in self._generated_cc to make ShowActions() faster.
 
 Args:
 process_tags: Process tags as if they were aliases
+cover_fname: If non-None the name of the cover letter.
 Return:
 Filename of temp file created
 
 # Look for commit tags (of the form 'xxx:' at the start of the subject)
 fname = '/tmp/patman.%d' % os.getpid()
 fd = open(fname, 'w')
+all_ccs = []
 for commit in self.commits:
 list = []
 if process_tags:
 list += gitutil.BuildEmailList(commit.tags)
 list += gitutil.BuildEmailList(commit.cc_list)
+all_ccs += list
 print fd, commit.patch, ', '.join(list)
 self._generated_cc[commit.patch] = list
 
+if cover_fname:
+print fd, cover_fname, ', '.join(set(all_ccs))
+
 fd.close()
 return fname
 
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/4] patman: Add the concept of multiple projects

[Doug Anderson]

There are cases that we want to support different settings (or maybe
even different aliases) for different projects.  Add support for this
by:
* Adding detection for two big projects: U-Boot and Linux.
* Adding default settings for Linux (U-Boot is already good with the
  standard patman defaults).
* Extend the new settings feature in .patman to specify per-project
  settings.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/README  |   13 
 tools/patman/patman.py   |9 +++-
 tools/patman/project.py  |   43 +
 tools/patman/settings.py |  147 +-
 4 files changed, 208 insertions(+), 4 deletions(-)
 create mode 100644 tools/patman/project.py

diff --git a/tools/patman/README b/tools/patman/README
index 6ca5b5b..d294f3d 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -114,6 +114,19 @@ verbose: True
 
 
 
+If you want to adjust settings (or aliases) that affect just a single
+project you can add a section that looks like [project_settings] or
+[project_alias].  If you want to use tags for your linux work, you could
+do:
+
+
+
+[linux_settings]
+process_tags: True
+
+
+
+
 How to run it
 =
 
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index b327c67..54a252e 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -34,6 +34,7 @@ import checkpatch
 import command
 import gitutil
 import patchstream
+import project
 import settings
 import terminal
 import test
@@ -59,6 +60,9 @@ parser.add_option('--cc-cmd', dest='cc_cmd', type='string', 
action='store',
default=None, help='Output cc list for patch file (used by git)')
 parser.add_option('--no-tags', action='store_false', dest='process_tags',
   default=True, help=Don't process subject tags as aliaes)
+parser.add_option('--project', default=project.DetectProject(),
+  help=Project name; affects default option values and 
+  aliases [default: %default])
 
 parser.usage = patman [options]
 
@@ -66,7 +70,10 @@ Create patches from commits in a branch, check them and 
email them as
 specified by tags you place in the commits. Use -n to 
 
 
-settings.Setup(parser, '')
+# Parse options twice: first to get the project and second to handle
+# defaults properly (which depends on project).
+(options, args) = parser.parse_args()
+settings.Setup(parser, options.project, '')
 (options, args) = parser.parse_args()
 
 # Run our meagre tests
diff --git a/tools/patman/project.py b/tools/patman/project.py
new file mode 100644
index 000..4f7b2b3
--- /dev/null
+++ b/tools/patman/project.py
@@ -0,0 +1,43 @@
+# Copyright (c) 2012 The Chromium OS Authors.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+import os.path
+
+import gitutil
+
+def DetectProject():
+Autodetect the name of the current project.
+
+This looks for signature files/directories that are unlikely to exist 
except
+in the given project.
+
+Returns:
+The name of the project, like linux or u-boot.  Returns unknown
+if we can't detect the project.
+
+top_level = gitutil.GetTopLevel()
+
+if os.path.exists(os.path.join(top_level, include, u-boot)):
+return u-boot
+elif os.path.exists(os.path.join(top_level, kernel)):
+return linux
+
+return unknown
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 5208f7d..084d1b8 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -26,6 +26,140 @@ import re
 import command
 import gitutil
 
+Default settings per-project.
+
+These are used by _ProjectConfigParser.  Settings names should match
+the dest of the option parser from patman.py.
+
+_default_settings = {
+u-boot: {},
+linux: {
+process_tags: False,
+}
+}
+
+class _ProjectConfigParser(ConfigParser.SafeConfigParser):
+ConfigParser that handles projects.
+
+There are two main goals of this class:
+- Load project-specific default settings.
+- Merge general default settings/aliases with project-specific ones.
+
+# Sample config used for tests below...
+ import StringIO
+ sample_config = '''
+... [alias]
+... me: Peter P. likesspid

[U-Boot] [PATCH 2/2] patman: Add all CC addresses to the cover letter

[Doug Anderson]

If we're sending a cover letter make sure to CC everyone that we're
CCing on each of the individual patches.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/patman.py |2 +-
 tools/patman/series.py |   12 +++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index de8314a..4181d80 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -140,7 +140,7 @@ else:
 options.count + options.start):
 ok = False
 
-cc_file = series.MakeCcFile(options.process_tags)
+cc_file = series.MakeCcFile(options.process_tags, cover_fname)
 
 # Email the patches out (giving the user time to check / cancel)
 cmd = ''
diff --git a/tools/patman/series.py b/tools/patman/series.py
index ad8288d..083af0f 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -19,6 +19,7 @@
 # MA 02111-1307 USA
 #
 
+import itertools
 import os
 
 import gitutil
@@ -138,6 +139,9 @@ class Series(dict):
 print 'Prefix:\t ', self.get('prefix')
 if self.cover:
 print 'Cover: %d lines' % len(self.cover)
+all_ccs = itertools.chain(*self._generated_cc.values())
+for email in set(all_ccs):
+print '  Cc: ',email
 if cmd:
 print 'Git command: %s' % cmd
 
@@ -201,27 +205,33 @@ class Series(dict):
 str = 'Change log exists, but no version is set'
 print col.Color(col.RED, str)
 
-def MakeCcFile(self, process_tags):
+def MakeCcFile(self, process_tags, cover_fname):
 Make a cc file for us to use for per-commit Cc automation
 
 Also stores in self._generated_cc to make ShowActions() faster.
 
 Args:
 process_tags: Process tags as if they were aliases
+cover_fname: If non-None the name of the cover letter.
 Return:
 Filename of temp file created
 
 # Look for commit tags (of the form 'xxx:' at the start of the subject)
 fname = '/tmp/patman.%d' % os.getpid()
 fd = open(fname, 'w')
+all_ccs = []
 for commit in self.commits:
 list = []
 if process_tags:
 list += gitutil.BuildEmailList(commit.tags)
 list += gitutil.BuildEmailList(commit.cc_list)
+all_ccs += list
 print fd, commit.patch, ', '.join(list)
 self._generated_cc[commit.patch] = list
 
+if cover_fname:
+print fd, cover_fname, ', '.join(set(all_ccs))
+
 fd.close()
 return fname
 
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/4] patman: Add a call to get_maintainer.pl if it exists

[Doug Anderson]

For Linux the best way to figure out where to send a patch is with the
get_maintainer.pl script.  Add support for calling it from patman.
Support is added unconditionally for scripts/get_maintainer.pl in
case it is helpful for any other projects.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/README|   11 ++-
 tools/patman/get_maintainer.py |   63 
 tools/patman/series.py |2 +
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 tools/patman/get_maintainer.py

diff --git a/tools/patman/README b/tools/patman/README
index dc3957c..903d02f 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -43,6 +43,9 @@ Series-to: fred.bl...@napier.co.nz
 
 in one of your commits, the series will be sent there.
 
+In Linux this will also call get_maintainer.pl on each of your
+patches automatically.
+
 
 How to use this tool
 
@@ -65,8 +68,12 @@ will get a consistent result each time.
 How to configure it
 ===
 
-For most cases patman will locate and use the file 'doc/git-mailrc' in
-your U-Boot directory. This contains most of the aliases you will need.
+For most cases of using patman for U-Boot developement patman will
+locate and use the file 'doc/git-mailrc' in your U-Boot directory.
+This contains most of the aliases you will need.
+
+For Linux the 'scripts/get_maintainer.pl' handles figuring out where
+to send patches pretty well.
 
 During the first run patman creates a config file for you by taking the default
 user name and email address from the global .gitconfig file.
diff --git a/tools/patman/get_maintainer.py b/tools/patman/get_maintainer.py
new file mode 100644
index 000..cb11373
--- /dev/null
+++ b/tools/patman/get_maintainer.py
@@ -0,0 +1,63 @@
+# Copyright (c) 2012 The Chromium OS Authors.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+import command
+import gitutil
+import os
+
+def FindGetMaintainer():
+Look for the get_maintainer.pl script.
+
+Returns:
+If the script is found we'll return a path to it; else None.
+
+try_list = [
+os.path.join(gitutil.GetTopLevel(), 'scripts'),
+]
+# Look in the list
+for path in try_list:
+fname = os.path.join(path, 'get_maintainer.pl')
+if os.path.isfile(fname):
+return fname
+
+return None
+
+def GetMaintainer(fname, verbose=False):
+Run get_maintainer.pl on a file if we find it.
+
+We look for get_maintainer.pl in the 'scripts' directory at the top of
+git.  If we find it we'll run it.  If we don't find get_maintainer.pl
+then we fail silently.
+
+Args:
+fname: Path to the patch file to run get_maintainer.pl on.
+
+Returns:
+A list of email addresses to CC to.
+
+get_maintainer = FindGetMaintainer()
+if not get_maintainer:
+if verbose:
+print WARNING: Couldn't find get_maintainer.pl
+return []
+
+stdout = command.Output(get_maintainer, '--norolestats', fname)
+return stdout.splitlines()
diff --git a/tools/patman/series.py b/tools/patman/series.py
index 083af0f..6c5c570 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -22,6 +22,7 @@
 import itertools
 import os
 
+import get_maintainer
 import gitutil
 import terminal
 
@@ -225,6 +226,7 @@ class Series(dict):
 if process_tags:
 list += gitutil.BuildEmailList(commit.tags)
 list += gitutil.BuildEmailList(commit.cc_list)
+list += get_maintainer.GetMaintainer(commit.patch)
 all_ccs += list
 print fd, commit.patch, ', '.join(list)
 self._generated_cc[commit.patch] = list
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/4] patman: Add support for settings in .patman

[Doug Anderson]

This patch adds support for a [settings] section in the .patman file.
In this section you can add settings that will affect the default
values for command-line options.

Support is added in a generic way such that any setting can be updated
by just referring to the dest of the option that is passed to the
option parser.  At the moment options that would make sense to put in
settings are ignore_errors, process_tags, and verbose.  You
could override them like:

 [settings]
 ignore_errors: True
 process_tags: False
 verbose: True

The settings functionality is also used in a future change which adds
support for per-project settings.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/README  |   16 
 tools/patman/gitutil.py  |2 --
 tools/patman/patman.py   |3 +++
 tools/patman/settings.py |   39 +++
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 903d02f..6ca5b5b 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -98,6 +98,22 @@ The checkpatch.pl in the U-Boot tools/ subdirectory will be 
located and
 used. Failing that you can put it into your path or ~/bin/checkpatch.pl
 
 
+If you want to change the defaults for patman's command-line arguments,
+you can add a [settings] section to your .patman file.  This can be used
+for any command line option by referring to the dest for the option in
+patman.py.  For reference, the useful ones (at the moment) shown below
+(all with the non-default setting):
+
+
+
+[settings]
+ignore_errors: True
+process_tags: False
+verbose: True
+
+
+
+
 How to run it
 =
 
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 72d37a0..e7753cf 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -377,8 +377,6 @@ def GetDefaultUserEmail():
 
 def Setup():
 Set up git utils, by reading the alias files.
-settings.Setup('')
-
 # Check for a git alias file also
 alias_fname = GetAliasFile()
 if alias_fname:
diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index 4181d80..b327c67 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -34,6 +34,7 @@ import checkpatch
 import command
 import gitutil
 import patchstream
+import settings
 import terminal
 import test
 
@@ -64,6 +65,8 @@ parser.usage = patman [options]
 Create patches from commits in a branch, check them and email them as
 specified by tags you place in the commits. Use -n to 
 
+
+settings.Setup(parser, '')
 (options, args) = parser.parse_args()
 
 # Run our meagre tests
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 4dda17b..5208f7d 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -88,13 +88,43 @@ def CreatePatmanConfigFile(config_fname):
 print f, [alias]\nme: %s %s % (name, email)
 f.close();
 
-def Setup(config_fname=''):
+def _UpdateDefaults(parser, config):
+Update the given OptionParser defaults based on config.
+
+We'll walk through all of the settings from the parser
+For each setting we'll look for a default in the option parser.
+If it's found we'll update the option parser default.
+
+The idea here is that the .patman file should be able to update
+defaults but that command line flags should still have the final
+say.
+
+Args:
+parser: An instance of an OptionParser whose defaults will be
+updated.
+config: An instance of SafeConfigParser that we will query
+for settings.
+
+defaults = parser.get_default_values()
+for name, val in config.items('settings'):
+if hasattr(defaults, name):
+default_val = getattr(defaults, name)
+if isinstance(default_val, bool):
+val = config.getboolean('settings', name)
+elif isinstance(default_val, int):
+val = config.getint('settings', name)
+parser.set_default(name, val)
+else:
+print WARNING: Unknown setting %s % name
+
+def Setup(parser, config_fname=''):
 Set up the settings module by reading config files.
 
 Args:
+parser: The parser to update
 config_fname:   Config filename to read ('' for default)
 
-settings = ConfigParser.SafeConfigParser()
+config = ConfigParser.SafeConfigParser()
 if config_fname == '':
 config_fname = '%s/.patman' % os.getenv('HOME')
 
@@ -102,11 +132,12 @@ def Setup(config_fname=''):
 print No config file found ~/.patman\nCreating one...\n
 CreatePatmanConfigFile(config_fname)
 
-settings.read(config_fname)
+config.read(config_fname)
 
-for name, value in settings.items('alias'):
+for name, value in config.items('alias'):
 alias[name] = value.split(',')
 
+_UpdateDefaults(parser, config)
 
 # These are the aliases we understand, indexed by alias. Each member is a list

[U-Boot] [PATCH 4/4] patman: Add settings to the list of modules to doctest

[Doug Anderson]

The settings modules now has doctests, so run them.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/patman.py |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index 54a252e..6825de4 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -85,8 +85,9 @@ if options.test:
 result = unittest.TestResult()
 suite.run(result)
 
-suite = doctest.DocTestSuite('gitutil')
-suite.run(result)
+for module in ['gitutil', 'settings']:
+suite = doctest.DocTestSuite(module)
+suite.run(result)
 
 # TODO: Surely we can just 'print' result?
 print result
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] patman: Cache the CC list from MakeCcFile() for use in ShowActions()

[Doug Anderson]

Currently we go through and generate the CC list for patches twice.
This gets slow when (in a future CL) we add a call to
get_maintainer.pl on Linux.  Instead of doing things twice, just cache
the CC list when it is first generated.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/patman.py |6 --
 tools/patman/series.py |   13 +
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index cfe06d0..de8314a 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -140,14 +140,16 @@ else:
 options.count + options.start):
 ok = False
 
+cc_file = series.MakeCcFile(options.process_tags)
+
 # Email the patches out (giving the user time to check / cancel)
 cmd = ''
 if ok or options.ignore_errors:
-cc_file = series.MakeCcFile(options.process_tags)
 cmd = gitutil.EmailPatches(series, cover_fname, args,
 options.dry_run, cc_file)
-os.remove(cc_file)
 
 # For a dry run, just show our actions as a sanity check
 if options.dry_run:
 series.ShowActions(args, cmd, options.process_tags)
+
+os.remove(cc_file)
diff --git a/tools/patman/series.py b/tools/patman/series.py
index d2971f4..ad8288d 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -46,6 +46,11 @@ class Series(dict):
 self.notes = []
 self.changes = {}
 
+# Written in MakeCcFile()
+#  key: name of patch file
+#  value: list of email addresses
+self._generated_cc = {}
+
 # These make us more like a dictionary
 def __setattr__(self, name, value):
 self[name] = value
@@ -109,10 +114,7 @@ class Series(dict):
 for upto in range(len(args)):
 commit = self.commits[upto]
 print col.Color(col.GREEN, '   %s' % args[upto])
-cc_list = []
-if process_tags:
-cc_list += gitutil.BuildEmailList(commit.tags)
-cc_list += gitutil.BuildEmailList(commit.cc_list)
+cc_list = list(self._generated_cc[commit.patch])
 
 # Skip items in To list
 if 'to' in self:
@@ -202,6 +204,8 @@ class Series(dict):
 def MakeCcFile(self, process_tags):
 Make a cc file for us to use for per-commit Cc automation
 
+Also stores in self._generated_cc to make ShowActions() faster.
+
 Args:
 process_tags: Process tags as if they were aliases
 Return:
@@ -216,6 +220,7 @@ class Series(dict):
 list += gitutil.BuildEmailList(commit.tags)
 list += gitutil.BuildEmailList(commit.cc_list)
 print fd, commit.patch, ', '.join(list)
+self._generated_cc[commit.patch] = list
 
 fd.close()
 return fname
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 04/10] arm: Add CONFIG_DELAY_ENVIRONMENT to delay environment loading

[Doug Anderson]

On Fri, Nov 30, 2012 at 3:01 PM, Simon Glass s...@chromium.org wrote:
 This option delays loading of the environment until later, so that only the
 default environment will be available to U-Boot.

 This can address the security risk of untrusted data being used during boot.

 Any time you load untrusted data you expose yourself to a bug in the
 code. The attacker gets to choose the data so can sometimes carefully
 craft it to exploit a bug. We try to avoid touching user-controlled
 data during a verified boot unless strictly necessary. Since the
 default environment is good enough in this case (or you would just
 change it), this gets around the problem by just not loading the
 environment.

 When CONFIG_DELAY_ENVIRONMENT is defined, it is convenient to have a
 run-time way of enabling loading of the environment. Add this to the
 fdt as /config/delay-environment.

 Note: This patch depends on http://patchwork.ozlabs.org/patch/194342/

 Signed-off-by: Doug Anderson diand...@chromium.org
 Signed-off-by: Simon Glass s...@chromium.org
 ---
 Changes in v2:
 - Update commit message to provide more detail

  README   |9 +
  arch/arm/lib/board.c |   29 +++--
  2 files changed, 36 insertions(+), 2 deletions(-)

 diff --git a/README b/README
 index b9a3685..d26ce5b 100644
 --- a/README
 +++ b/README
 @@ -2329,6 +2329,15 @@ CBFS (Coreboot Filesystem) support
 run-time determined information about the hardware to the
 environment.  These will be named board_name, board_rev.

 +   CONFIG_DELAY_ENVIRONMENT
 +
 +   Normally the environment is loaded when the board is
 +   intialised so that it is available to U-Boot. This inhibits
 +   that so that the environment is not available until
 +   explicitly loaded later by U-Boot code. With CONFIG_OF_CONTROL
 +   this is instead controlled by the value of
 +   /config/load-environment.
 +
  - DataFlash Support:
 CONFIG_HAS_DATAFLASH

 diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
 index 262a3ca..7d1927e 100644
 --- a/arch/arm/lib/board.c
 +++ b/arch/arm/lib/board.c
 @@ -40,6 +40,7 @@

  #include common.h
  #include command.h
 +#include environment.h
  #include malloc.h
  #include stdio_dev.h
  #include version.h
 @@ -476,7 +477,28 @@ static char *failed = *** failed ***\n;
  #endif

  /*
 - 
 + * Tell if it's OK to load the environment early in boot.
 + *
 + * If CONFIG_OF_CONFIG is defined, we'll check with the FDT to see
 + * if this is OK (defaulting to saying it's not OK).
 + *
 + * NOTE: Loading the environment early can be a bad idea if security is
 + *   important, since no verification is done on the environment.
 + *
 + * @return 0 if environment should not be loaded, !=0 if it is ok to load
 + */
 +static int should_load_env(void)
 +{
 +#ifdef CONFIG_OF_CONTROL
 +   return fdtdec_get_config_int(gd-fdt_blob, load-environment, 0);
 +#elif defined CONFIG_DELAY_ENVIRONMENT
 +   return 0;
 +#else
 +   return 1;
 +#endif
 +}
 +
 +/
   *
   * This is the next part if the initialization sequence: we are now
   * running from RAM and have a normal C environment, i. e. global
 @@ -583,7 +605,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
  #endif

 /* initialize environment */
 -   env_relocate();
 +   if (should_load_env())
 +   env_relocate();
 +   else
 +   set_default_env(NULL);

  #if defined(CONFIG_CMD_PCI) || defined(CONFIG_PCI)
 arm_pci_init();
 --
 1.7.7.3


Reviewed-by: Doug Anderson diand...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/2] patman: Allow tests to run even if patman is in the path

[Doug Anderson]

Several of the patman doctests assume that patman was run with:
  ./patman

Fix them so that they work even if patman is run with just patman
(because patman is in the path).

Signed-off-by: Doug Anderson diand...@chromium.org
---
 tools/patman/gitutil.py |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 72d37a0..41a74a5 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -217,6 +217,10 @@ def EmailPatches(series, cover_fname, args, dry_run, 
cc_fname,
 Returns:
 Git command that was/would be run
 
+# For the duration of this doctest pretend that we ran patman with ./patman
+ _old_argv0 = sys.argv[0]
+ sys.argv[0] = './patman'
+
  alias = {}
  alias['fred'] = ['f.blo...@napier.co.nz']
  alias['john'] = ['j.blo...@napier.co.nz']
@@ -244,6 +248,9 @@ def EmailPatches(series, cover_fname, args, dry_run, 
cc_fname,
 'git send-email --annotate --to f.blo...@napier.co.nz --cc \
 f.blo...@napier.co.nz --cc j.blo...@napier.co.nz --cc \
 m.popp...@cloud.net --cc-cmd ./patman --cc-cmd cc-fname cover p1 p2'
+
+# Restore argv[0] since we clobbered it.
+ sys.argv[0] = _old_argv0
 
 to = BuildEmailList(series.get('to'), '--to', alias)
 if not to:
@@ -340,8 +347,8 @@ def GetTopLevel():
 
 This test makes sure that we are running tests in the right subdir
 
- os.path.realpath(os.getcwd()) == \
-os.path.join(GetTopLevel(), 'tools', 'scripts', 'patman')
+ os.path.realpath(os.path.dirname(__file__)) == \
+os.path.join(GetTopLevel(), 'tools', 'patman')
 True
 
 return command.OutputOneLine('git', 'rev-parse', '--show-toplevel')
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] patman: Look for checkpatch in the scripts directory

[Doug Anderson]

The Linux kernel stores checkpatch.pl in the scripts directory.  Add
that to the search path to make things more automatic for kernel
development.

Signed-off-by: Doug Anderson diand...@chromium.org

---
 tools/patman/checkpatch.py |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index d831087..f72f8ee 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -26,10 +26,12 @@ import re
 import terminal
 
 def FindCheckPatch():
+top_level = gitutil.GetTopLevel()
 try_list = [
 os.getcwd(),
 os.path.join(os.getcwd(), '..', '..'),
-os.path.join(gitutil.GetTopLevel(), 'tools'),
+os.path.join(top_level, 'tools'),
+os.path.join(top_level, 'scripts'),
 '%s/bin' % os.getenv('HOME'),
 ]
 # Look in current dir
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] patman: Add spaces back into patman test

[Doug Anderson]

The patman test code was failing because some extra spaces got
stripped when it was applied.  These spaces are critical to the test
code working.

Signed-off-by: Doug Anderson diand...@chromium.org

---
 tools/patman/test.py |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/patman/test.py b/tools/patman/test.py
index cf42480..f801ced 100644
--- a/tools/patman/test.py
+++ b/tools/patman/test.py
@@ -119,8 +119,8 @@ index 6f3748d..f9e4e65 100644
 --- a/README
 +++ b/README
 @@ -2026,6 +2026,17 @@ The following options need to be configured:
-   example, some LED's) on your board. At the moment,
-   the following checkpoints are implemented:
+   example, some LED's) on your board. At the moment,
+   the following checkpoints are implemented:
 
 +- Time boot progress
 +  CONFIG_BOOTSTAGE
@@ -134,7 +134,7 @@ index 6f3748d..f9e4e65 100644
 +  You can add calls to bootstage_mark() to set time markers.
 +
  - Standalone program support:
-   CONFIG_STANDALONE_LOAD_ADDR
+   CONFIG_STANDALONE_LOAD_ADDR
 
 diff --git a/common/bootstage.c b/common/bootstage.c
 new file mode 100644
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] tegra: Specify debugging serial port at boot.

[Doug Anderson]

Stephen,


On Wed, Mar 21, 2012 at 9:49 AM, Stephen Warren swar...@wwwdotorg.org wrote:
 I was under the impression that earlyprintk was a boolean option, and
 didn't take any arguments. But, a quick grep shows that some archs do
 allow it to take an argument. The ARM code that handles the option
 doesn't process the argument at present as far as I can tell, although
 many ARM defconfigs do provide one. I'll see if it's possible to rework
 the Tegra kernel code to use any earlyprintk argument.

If you're going to look at getting earlyprintk able to take arguments,
this bug (and the associated links) might be of interest to you:
    http://code.google.com/p/chromium-os/issues/detail?id=19185

It would certainly be nice to get earlyprintk able to take arguments
on ARM in the kernel.  I didn't originally go down that route because
it seemed like there were going to be lots of yaks to shave and it
would involve changes to some of the core Linux ARM boot code.  It
also didn't solve the problem where the ARM kernel wants to do prints
before translation (as Olof points out in in the discussion thread
linked above).  ...but it does seem to be the better route.


 I doubt it will be possible to do this in the ARM zImage decompressor
 though.

The decompressing part of the kernel wouldn't come for free without
a lot of work, but maybe the right answer is to leverage u-boot's
ability to decompress the kernel and stop using the zImage anyway.
That would make Simon happy, I think.


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] env: Add the ability to merge the saved env with the default.

[Doug Anderson]

Mike,


On Mon, Mar 5, 2012 at 8:27 PM, Mike Frysinger vap...@gentoo.org wrote:

 this is kind of a crappy interface.


Agreed.


 also, doesn't the existing `env import`
 do this ?
  * env import [-d] [-t | -b | -c] addr [size]
  *  -d: delete existing environment before importing;
  *  otherwise overwrite / append to existion definitions

 so if we imported the default, we'd get this ?


It's not quite possible to just import the default from the saved for two
reasons:
1. I don't know of any way to import the default.  It's not stored at any
well-known address or with any well-known size.
2. The order of precedence would be a little different (default would
override saved).

...but I think you're right that using env import is the better way to
go.  I can always have the default boot command load the overrides into
memory and the import them.  That works just fine for us.

Thank you for your review and suggestion!

Please consider this patch abandoned.


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] EXYNOS: SMDK5250: Support all 4 UARTs

[Doug Anderson]

Mike,

On Mon, Feb 13, 2012 at 11:00 PM, Mike Frysinger vap...@gentoo.org wrote:

 i don't know the exynos5 pinmux specifics, but speaking in general, you
 shouldn't go configuring pins directly if the user hasn't asked for them.
 config multi would be useful because then the pinmux logic would be in a
 uart-
 specific init func and we wouldn't need ifdefs.  when the user says use
 uart#
 as my i/o device, we know we should configure the pinmuxes for that.

 the uart1 above looks like a good example ... the pins can be UART1 or I2C.
 we shouldn't go blasting them all to UART1 ...

 Doug can speak to the exact pinrouting setup that the smdk5250 board has
 ...


In the case of the smdk5250, all of the above pinmuxes are safe to always
configure as UART.  ...and since this pinmux is in a board-specific file
(as opposed to generic exynos code), I think it's OK to remove the ifdefs.

Eventually this still will change with device tree work, but until then I
think the v2 patch is a reasonable way to do things and does allow all the
serial ports to work.

Thanks!

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] env: Add the ability to merge the saved env with the default.

[Doug Anderson]

This is a useful mechanism any time you have a way to update the
saved environment outside of u-boot.  This can be a tool like
fw_setenv or could be a tool like we use in Chrome OS that
modifies the variables in a binary image before flashing (see
factory_setup/update_firmware_vars.py in
http://git.chromium.org/git/chromiumos/platform/factory-utils).

Signed-off-by: Doug Anderson diand...@chromium.org
---
 common/env_common.c |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/common/env_common.c b/common/env_common.c
index 71811c4..5938732 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -34,6 +34,19 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/*
+ * Create a saved enviroment with this env variable set to 1 to merge the
+ * saved environment on top of the default environment.  The idea is that your
+ * saved environment would just contain variables that you'd like to override
+ * from the default so that as you update u-boot (w/ potential changes to the
+ * default) you get all the updates.
+ *
+ * This is really most useful when you have a tool like fw_setenv to manage
+ * your saved environment.  Using 'saveenv' to save your environment will saved
+ * the _merged_ environment (AKA it won't unmerge things).
+ */
+#define MERGE_WITH_DEFAULT merge_with_default
+
 /
  * Default settings to be used when no valid environment is found
  */
@@ -208,7 +221,18 @@ int env_import(const char *buf, int check)
}
 
if (himport_r(env_htab, (char *)ep-data, ENV_SIZE, '\0', 0)) {
+   char *merge_val;
+
gd-flags |= GD_FLG_ENV_READY;
+   merge_val = getenv(MERGE_WITH_DEFAULT);
+
+   if (merge_val != NULL  merge_val[0] != '0') {
+   set_default_env();
+   himport_r(env_htab, (char *)ep-data, ENV_SIZE, '\0',
+ H_NOCLEAR);
+   hdelete_r(MERGE_WITH_DEFAULT, env_htab);
+   puts(Merged saved with default environment\n\n);
+   }
return 1;
}
 
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] EXYNOS: SMDK5250: Support all 4 UARTs

[Doug Anderson]

This properly configures the mux to enable all UARTs.

This also fixes things so that we don't configure balls XUCTSN_1 and
XURTSN_1 as UART1 configuration (RTS/CTS), since they aren't
connected.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Removed #ifdefs and tested SERIAL_MULTI by setting stdin/out/err to
s5pserN and validating that serial moved.

 board/samsung/smdk5250/smdk5250.c |   44 +++-
 1 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250.c 
b/board/samsung/smdk5250/smdk5250.c
index 928c08f..32786e2 100644
--- a/board/samsung/smdk5250/smdk5250.c
+++ b/board/samsung/smdk5250/smdk5250.c
@@ -224,11 +224,51 @@ static void board_uart_init(void)
(struct exynos5_gpio_part1 *) samsung_get_base_gpio_part1();
int i;
 
-   /* UART1 GPIOs (part1) : GPA0CON[7:4] 0x */
-   for (i = 4; i  8; i++) {
+   /*
+* UART0 GPIOs : GPA0CON[3:0] 0x
+* Must set CFG17 switches to select UART0 to use.
+*/
+   for (i = 0; i = 3; i++) {
s5p_gpio_set_pull(gpio1-a0, i, GPIO_PULL_NONE);
s5p_gpio_cfg_pin(gpio1-a0, i, GPIO_FUNC(0x2));
}
+
+   /*
+* UART1 GPIOs : GPA0CON[5:4] 0x22
+* Must set CFG17 switches to select UART1 to use.
+*
+* This only sets RXD/TXD, as RTS/CTS need a resistor soldered down
+* in order to use them (so that those pins can be used for I2C).
+*/
+   for (i = 4; i = 5; i++) {
+   s5p_gpio_set_pull(gpio1-a0, i, GPIO_PULL_NONE);
+   s5p_gpio_cfg_pin(gpio1-a0, i, GPIO_FUNC(0x2));
+   }
+
+   /*
+* UART2 GPIOs : GPA1CON[1:0] 0x22
+* Must set CFG17 switches to select UART2 to use.
+*
+* This only sets RXD/TXD, as RTS/CTS need a resistor soldered down
+* in order to use them (so that those pins can be used for I2C).
+*/
+   for (i = 0; i = 1; i++) {
+   s5p_gpio_set_pull(gpio1-a1, i, GPIO_PULL_NONE);
+   s5p_gpio_cfg_pin(gpio1-a1, i, GPIO_FUNC(0x2));
+   }
+
+   /*
+* UART3 GPIOs : GPA1CON[5:4] 0x22
+* Must set CFG16 switches to select UART3 to use.
+*/
+   for (i = 4; i = 5; i++) {
+   s5p_gpio_set_pull(gpio1-a1, i, GPIO_PULL_NONE);
+   s5p_gpio_cfg_pin(gpio1-a1, i, GPIO_FUNC(0x2));
+   }
+
+   /*
+* There's no mux for UART4--it's internal only
+*/
 }
 
 #ifdef CONFIG_BOARD_EARLY_INIT_F
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] EXYNOS: SMDK5250: Support all 4 UARTs

[Doug Anderson]

This properly configures the mux to enable the right UART depending on
the setting of CONFIG_SERIALX.

This also fixes things so that we don't configure balls XUCTSN_1 and
XURTSN_1 as UART1 configuration (RTS/CTS), since they aren't
connected.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 board/samsung/smdk5250/smdk5250.c |   49 +++-
 1 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250.c 
b/board/samsung/smdk5250/smdk5250.c
index 928c08f..61b8634 100644
--- a/board/samsung/smdk5250/smdk5250.c
+++ b/board/samsung/smdk5250/smdk5250.c
@@ -224,11 +224,56 @@ static void board_uart_init(void)
(struct exynos5_gpio_part1 *) samsung_get_base_gpio_part1();
int i;
 
-   /* UART1 GPIOs (part1) : GPA0CON[7:4] 0x */
-   for (i = 4; i  8; i++) {
+#if defined(CONFIG_SERIAL0)
+   /*
+* UART0 GPIOs : GPA0CON[3:0] 0x
+* Must set CFG17 switches to select UART0 to use.
+*/
+   for (i = 0; i = 3; i++) {
s5p_gpio_set_pull(gpio1-a0, i, GPIO_PULL_NONE);
s5p_gpio_cfg_pin(gpio1-a0, i, GPIO_FUNC(0x2));
}
+#elif defined(CONFIG_SERIAL1)
+   /*
+* UART1 GPIOs : GPA0CON[5:4] 0x22
+* Must set CFG17 switches to select UART1 to use.
+*
+* This only sets RXD/TXD, as RTS/CTS need a resister soldered down
+* in order to use them (so that those pins can be used for I2C).
+*/
+   for (i = 4; i = 5; i++) {
+   s5p_gpio_set_pull(gpio1-a0, i, GPIO_PULL_NONE);
+   s5p_gpio_cfg_pin(gpio1-a0, i, GPIO_FUNC(0x2));
+   }
+#elif defined(CONFIG_SERIAL2)
+   /*
+* UART2 GPIOs : GPA1CON[1:0] 0x22
+* Must set CFG17 switches to select UART2 to use.
+*
+* This only sets RXD/TXD, as RTS/CTS need a resister soldered down
+* in order to use them (so that those pins can be used for I2C).
+*/
+   for (i = 0; i = 1; i++) {
+   s5p_gpio_set_pull(gpio1-a1, i, GPIO_PULL_NONE);
+   s5p_gpio_cfg_pin(gpio1-a1, i, GPIO_FUNC(0x2));
+   }
+#elif defined(CONFIG_SERIAL3)
+   /*
+* UART3 GPIOs : GPA1CON[5:4] 0x22
+* Must set CFG16 switches to select UART3 to use.
+*/
+   for (i = 4; i = 5; i++) {
+   s5p_gpio_set_pull(gpio1-a1, i, GPIO_PULL_NONE);
+   s5p_gpio_cfg_pin(gpio1-a1, i, GPIO_FUNC(0x2));
+   }
+#elif defined(CONFIG_SERIAL4)
+   /*
+* There's no mux for UART4--it's internal only
+*/
+#error Can't set console to serial 4--it's not exposed
+#else
+#error Unknown serial config
+#endif
 }
 
 #ifdef CONFIG_BOARD_EARLY_INIT_F
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] EXYNOS: SMDK5250: Add support for DDR3 memory.

[Doug Anderson]

On Wed, Feb 8, 2012 at 3:44 AM, Hatim Ali hatim...@samsung.com wrote:
 SMDK5250 development boards have different memory variants like
 DDR3, LPDDR2 and LPDDR3. This patch adds supports for DDR3.
 The DDR3 is configured for 667Mhz and is being enabled by default.

One other note: the SDRAM doesn't appear to be stable with the current
CL as is.  Specifically, I ran this test:

Load two copies of vmlinuz:

 Hit any key to stop autoboot:  0
 SMDK5250 # mmc rescan 0
 SMDK5250 # fatload mmc 0:c 0x4200 vmlinuz
 reading vmlinuz

 3272888 bytes read
 SMDK5250 # fatload mmc 0:c 0x40008000 vmlinuz
 reading vmlinuz

 3272888 bytes read

Now compare them:

 SMDK5250 # cmp.l 0x40008000 0x4200 3272888
 word at 0x400097fc (0xe3160024) != word at 0x420017fc (0xe3160020)
 Total of 1535 words were the same

...but if you look at that memory, it's the same!

 SMDK5250 # md.l 0x420017fc 1
 420017fc: e3160020 ...
 SMDK5250 # md.l 0x400097fc 1
 400097fc: e3160020 ...

...in this case, re-running the cmp.l still showed the difference (?)


Running RAM at 400MHz didn't show the same problem.  NOTE: that
doesn't prove the the problem is related to SDRAM stability, but it
sure looks suspicious.

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] EXYNOS: SMDK5250: Add support for DDR3 memory.

[Doug Anderson]

Hatim,


Overall comments:

* Random delays are evil.  Please explain each sdelay() call.  Be sure
to include information about why exactly 65,536 iterations through a
delay loop is necessary in each case.

* For functions that are nearly the same between dmc_init.c and
dmc_init_ddr3.c should be unified.  If possible, unify everything.

* If we really need to have two files, please submit a separate patch
to rename dmc_init.c to dmc_init_lpddr2.c

* If we really need to have two files, please reorder functions so
that functions that are in both dmc_init_ddr3.c and in
dmc_init_lpddr2.c are in the same order for easy diffing.

* In general, avoid defining hex registers.  Instead, define things
symbolically.  See comments about DMC_MEMCONTROL_VAL below as an
example.

* I've started reviewing this before the patches from Chander.  I'll
probably go back and review that next, if I have time.  Expect similar
comments there.


See below for some scattered comments.  Note that I didn't fully
review everything, so I'd expect another round will be needed.


On Wed, Feb 8, 2012 at 3:44 AM, Hatim Ali hatim...@samsung.com wrote:
 --- a/board/samsung/smdk5250/dmc_init.c
 +++ b/board/samsung/smdk5250/dmc_init.c
 @@ -248,7 +248,7 @@ static void config_rdlvl(struct exynos5_dmc *dmc,
         * ctrl_gateduradj, rdlvl_pass_adj
         * rdlvl_rddataPadj
         */
 -       val = SET_RDLVL_RDDATAPADJ;
 +       val = SET_RDLVL_RDDATA_ADJ;

This doesn't seem related to DDR3.  Perhaps it should be split into a
separate patch?


 @@ -361,8 +361,8 @@ void mem_ctrl_init()
        config_zq(phy0_ctrl, phy1_ctrl);

        /* Operation Mode : LPDDR2 */
 -       val = PHY_CON0_RESET_VAL;
 -       SET_CTRL_DDR_MODE(val, DDR_MODE_LPDDR2);
 +       val = CTRL_DDR_MODE(LPDDR2);
 +       val |= BYTE_RDLVL_EN;

I'd prefer this (and related header change) to be in a separate patch as well.


 +/* APLL : 1GHz */
 +/* MCLK_CDREX: MCLK_CDREX_677*/
 +/* LPDDR support: DDR3 */

I assume that future CLs will relax some of these restrictions?


 +       /* Sending PALL cmd on
 +       * Channel0-Chip0
 +       * Channel0-Chip1
 +       * Channel1-Chip0
 +       * Channel1-Chip1
 +       */

nit: please use proper commenting style.  AKA:

/*
 * Sending PALL cmd on
 * - Channel0-Chip0
 * - Channel0-Chip1
 * - Channel1-Chip0
 * - Channel1-Chip1
 */

 +       for (channel = 0; channel  CONFIG_DMC_CHANNELS; channel++) {
 +               SET_CMD_CHANNEL(mask, channel);
 +               for (chip = 0; chip  CONFIG_CHIPS_PER_CHANNEL; chip++) {
 +                       SET_CMD_CHIP(mask, chip);
 +                       val = DIRECT_CMD_PALL | mask;
 +                       writel(val, dmc-directcmd);
 +                       sdelay(0x1);

As with all sdelays, please explain.

 +               }
 +       }
 +
 +}
 +
 +
 +static void dmc_directcmd(struct exynos5_dmc *dmc)
 +{
 +       unsigned long channel, mask = 0, val;
 +
 +       /* Selecting Channel0-Chip0 */
 +       SET_CMD_CHANNEL(mask, 0);
 +       SET_CMD_CHIP(mask, 0);
 +
 +       /* Sending NOP cmd on Channel0-Chip0 */
 +       val = DIRECT_CMD_NOP | mask;
 +       writel(val, dmc-directcmd);
 +       sdelay(0x1);
 +
 +       dmc_directcmd_PALL(dmc);
 +
 +       /* Selecting Chip 0*/
 +       mask = 0;
 +       SET_CMD_CHIP(mask, 0);
 +       for (channel = 0; channel  CONFIG_DMC_CHANNELS; channel++) {
 +               /* Selecting Channel */
 +               SET_CMD_CHANNEL(mask, channel);
 +
 +               if (channel == 1) {
 +                       /* Sending NOP cmd on Channel1-Chip0 */
 +                       val = DIRECT_CMD_NOP | mask;
 +                       writel(val, dmc-directcmd);
 +                       sdelay(0x1);
 +               }
 +               /* Sending MRS/EMRS command and ZQINIT command
 +               *  on Channel0-Chip0
 +               */
 +               val = DIRECT_CMD_MRS1 | mask;
 +               writel(val, dmc-directcmd);
 +               sdelay(0x1);
 +
 +               val = DIRECT_CMD_MRS2 | mask;
 +               writel(val, dmc-directcmd);
 +               sdelay(0x1);
 +
 +               val = DIRECT_CMD_MRS3 | mask;
 +               writel(val, dmc-directcmd);
 +               sdelay(0x1);
 +
 +               val = DIRECT_CMD_MRS4 | mask;
 +               writel(val, dmc-directcmd);
 +               sdelay(0x1);
 +
 +               val = DIRECT_CMD_ZQINIT | mask;
 +               writel(val, dmc-directcmd);
 +               sdelay(0x1);
 +       }
 +}
 +
 +static void update_reset_dll(struct exynos5_dmc *dmc)
 +{

* Unify with LPDDR2 version and/or explain differences.


 +static void config_memory(struct exynos5_dmc *dmc)
 +{
 +       /*
 +        * Dynamic Clock: Always Running
 +        * Memory Burst length: 8
 +        * Number of chips: 1
 +        * Memory Bus width: 32 bit
 +        * Memory Type: DDR3
 +        * Additional Latancy for PLL: 0 Cycle
 +        */
 +       writel(DMC_MEMCONTROL_VAL, 

Re: [U-Boot] [PATCH v7 4/4] EXYNOS: SMDK5250: Add MMC SPL support

[Doug Anderson]

On Thu, Feb 2, 2012 at 1:21 PM, Mike Frysinger vap...@gentoo.org wrote:

 On Thursday 02 February 2012 04:11:27 Chander Kashyap wrote:
  +int main(int argc, char **argv)
  +{
  ...
  + unsigned char buffer[BUFSIZE] = {0};

 this is an implicit memset() and from what i can see in the code, useless.
 you read() the entire buffer, so there's no need to initialize it.


Funny, I was just about to submit a patch to add this = {0} myself when I
found this message.  ;)  I would say that it (or a memset, whichever people
prefer) is a good idea so that this tool can be used to make a reasonable
SPL out of any source binary executable, even ones that are smaller than
14K.

Specifically, I assembled a bit of quick-and-dirty code for debugging (60
bytes) and wanted to convince the processor to load it as a BL2.  This tool
worked for the process, but it produced a file with some random data in it.
 Ick.


I wouldn't hold up committing this patch series for it, though...

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3] bootm: Avoid 256-byte overflow in fixup_silent_linux()

[Doug Anderson]

This makes fixup_silent_linux() use malloc() to allocate its
working space, meaning that our maximum kernel command line
should only be limited by malloc().  Previously it was silently
overflowing the stack.

Note that nothing about this change increases the kernel's maximum
command line length.  If you have a command line that is 256
bytes it's up to you to make sure that kernel can handle it.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Tried to trim down to just the minimum changes needed with
no extra helper code.

Changes in v3:
- Took Mike Frysinger's suggestion of removing strdup()

 common/cmd_bootm.c |   41 +
 1 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..95ac2d9 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -1229,9 +1229,14 @@ U_BOOT_CMD(
 /* helper routines */
 /***/
 #ifdef CONFIG_SILENT_CONSOLE
+
+#define CONSOLE_ARG console=
+#define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
+
 static void fixup_silent_linux(void)
 {
-   char buf[256], *start, *end;
+   char *buf;
+   char *env_val;
char *cmdline = getenv(bootargs);
 
/* Only fix cmdline when requested */
@@ -1239,25 +1244,37 @@ static void fixup_silent_linux(void)
return;
 
debug(before silent fix-up: %s\n, cmdline);
-   if (cmdline) {
-   start = strstr(cmdline, console=);
+   if (cmdline  (cmdline[0] != '\0')) {
+   char *start = strstr(cmdline, CONSOLE_ARG);
+
+   /* Allocate space for maximum possible new command line */
+   buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1);
+   if (!buf) {
+   debug(%s: out of memory\n, __func__);
+   return;
+   }
+
if (start) {
-   end = strchr(start, ' ');
-   strncpy(buf, cmdline, (start - cmdline + 8));
+   char *end = strchr(start, ' ');
+   int num_start_bytes = start - cmdline + CONSOLE_ARG_LEN;
+
+   strncpy(buf, cmdline, num_start_bytes);
if (end)
-   strcpy(buf + (start - cmdline + 8), end);
+   strcpy(buf + num_start_bytes, end);
else
-   buf[start - cmdline + 8] = '\0';
+   buf[num_start_bytes] = '\0';
} else {
-   strcpy(buf, cmdline);
-   strcat(buf,  console=);
+   sprintf(buf, %s %s, cmdline, CONSOLE_ARG);
}
+   env_val = buf;
} else {
-   strcpy(buf, console=);
+   buf = NULL;
+   env_val = CONSOLE_ARG;
}
 
-   setenv(bootargs, buf);
-   debug(after silent fix-up: %s\n, buf);
+   setenv(bootargs, env_val);
+   debug(after silent fix-up: %s\n, env_val);
+   free(buf);
 }
 #endif /* CONFIG_SILENT_CONSOLE */
 
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3] bootm: Avoid 256-byte overflow in fixup_silent_linux()

[Doug Anderson]

Mike,

On Tue, Jan 17, 2012 at 11:27 AM, Mike Frysinger vap...@gentoo.org wrote:
 On Tuesday 17 January 2012 14:16:53 Doug Anderson wrote:
 --- a/common/cmd_bootm.c
 +++ b/common/cmd_bootm.c

 +     char *env_val;

 did marking this const not work out ?

I coded the patch an old -v2011.06 branch then tested on ToT.  On
-v2011.06 setenv wasn't const.  I'll resubmit with the const.

Thanks!

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v4] bootm: Avoid 256-byte overflow in fixup_silent_linux()

[Doug Anderson]

This makes fixup_silent_linux() use malloc() to allocate its
working space, meaning that our maximum kernel command line
should only be limited by malloc().  Previously it was silently
overflowing the stack.

Note that nothing about this change increases the kernel's maximum
command line length.  If you have a command line that is 256
bytes it's up to you to make sure that kernel can handle it.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Tried to trim down to just the minimum changes needed with
no extra helper code.

Changes in v3:
- Took Mike Frysinger's suggestion of removing strdup()

Changes in v4:
- Added in const

 common/cmd_bootm.c |   41 +
 1 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..0a5ac81 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -1229,9 +1229,14 @@ U_BOOT_CMD(
 /* helper routines */
 /***/
 #ifdef CONFIG_SILENT_CONSOLE
+
+#define CONSOLE_ARG console=
+#define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
+
 static void fixup_silent_linux(void)
 {
-   char buf[256], *start, *end;
+   char *buf;
+   const char *env_val;
char *cmdline = getenv(bootargs);
 
/* Only fix cmdline when requested */
@@ -1239,25 +1244,37 @@ static void fixup_silent_linux(void)
return;
 
debug(before silent fix-up: %s\n, cmdline);
-   if (cmdline) {
-   start = strstr(cmdline, console=);
+   if (cmdline  (cmdline[0] != '\0')) {
+   char *start = strstr(cmdline, CONSOLE_ARG);
+
+   /* Allocate space for maximum possible new command line */
+   buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1);
+   if (!buf) {
+   debug(%s: out of memory\n, __func__);
+   return;
+   }
+
if (start) {
-   end = strchr(start, ' ');
-   strncpy(buf, cmdline, (start - cmdline + 8));
+   char *end = strchr(start, ' ');
+   int num_start_bytes = start - cmdline + CONSOLE_ARG_LEN;
+
+   strncpy(buf, cmdline, num_start_bytes);
if (end)
-   strcpy(buf + (start - cmdline + 8), end);
+   strcpy(buf + num_start_bytes, end);
else
-   buf[start - cmdline + 8] = '\0';
+   buf[num_start_bytes] = '\0';
} else {
-   strcpy(buf, cmdline);
-   strcat(buf,  console=);
+   sprintf(buf, %s %s, cmdline, CONSOLE_ARG);
}
+   env_val = buf;
} else {
-   strcpy(buf, console=);
+   buf = NULL;
+   env_val = CONSOLE_ARG;
}
 
-   setenv(bootargs, buf);
-   debug(after silent fix-up: %s\n, buf);
+   setenv(bootargs, env_val);
+   debug(after silent fix-up: %s\n, env_val);
+   free(buf);
 }
 #endif /* CONFIG_SILENT_CONSOLE */
 
-- 
1.7.7.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] bootm: Avoid 256-byte overflow in fixup_silent_linux()

[Doug Anderson]

This makes fixup_silent_linux() use malloc() to allocate its
working space, meaning that our maximum kernel command line
should only be limited by malloc().  Previously it was silently
overflowing the stack.

Note that nothing about this change increases the kernel's maximum
command line length.  If you have a command line that is 256
bytes it's up to you to make sure that kernel can handle it.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Tried to trim down to just the minimum changes needed with
no extra helper code.

 common/cmd_bootm.c |   38 --
 1 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..9a0c08d 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -1229,9 +1229,13 @@ U_BOOT_CMD(
 /* helper routines */
 /***/
 #ifdef CONFIG_SILENT_CONSOLE
+
+#define CONSOLE_ARG console=
+#define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
+
 static void fixup_silent_linux(void)
 {
-   char buf[256], *start, *end;
+   char *buf;
char *cmdline = getenv(bootargs);
 
/* Only fix cmdline when requested */
@@ -1239,25 +1243,39 @@ static void fixup_silent_linux(void)
return;
 
debug(before silent fix-up: %s\n, cmdline);
-   if (cmdline) {
-   start = strstr(cmdline, console=);
+   if (cmdline  (cmdline[0] != '\0')) {
+   char *start = strstr(cmdline, CONSOLE_ARG);
+
+   /* Allocate space for maximum possible new command line */
+   buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1);
+   if (!buf) {
+   debug(%s: out of memory\n, __func__);
+   return;
+   }
+
if (start) {
-   end = strchr(start, ' ');
-   strncpy(buf, cmdline, (start - cmdline + 8));
+   char *end = strchr(start, ' ');
+   int num_start_bytes = start - cmdline + CONSOLE_ARG_LEN;
+
+   strncpy(buf, cmdline, num_start_bytes);
if (end)
-   strcpy(buf + (start - cmdline + 8), end);
+   strcpy(buf + num_start_bytes, end);
else
-   buf[start - cmdline + 8] = '\0';
+   buf[num_start_bytes] = '\0';
} else {
-   strcpy(buf, cmdline);
-   strcat(buf,  console=);
+   sprintf(buf, %s %s, cmdline, CONSOLE_ARG);
}
} else {
-   strcpy(buf, console=);
+   buf = strdup(CONSOLE_ARG);
+   if (!buf) {
+   debug(%s: strdup failed\n, __func__);
+   return;
+   }
}
 
setenv(bootargs, buf);
debug(after silent fix-up: %s\n, buf);
+   free(buf);
 }
 #endif /* CONFIG_SILENT_CONSOLE */
 
-- 
1.7.3.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 3/3] config: Remove Blackfin CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

[Doug Anderson]

Dear Wolfgang,

On Wed, Jan 11, 2012 at 12:11 AM, Wolfgang Denk w...@denx.de wrote:
 I have to admit that I have no clear opinion here yet.

 The existing code is from a time when all architectures had a pretty
 low limit on the command line size - IIRC even PPC had only 256 bytes
 by then, hard coded.

 I think we have two options:

 1) Try not to exceed any limits imposed by the Linux kernel.
   Advantage: we can catch situations where the user overflows the
           limits and print an error message (IIRC this needs to be
           added), so the user gets aware of the pronlem.
   Disadvantage: we need to add architecture specific definitions for
           the command line size, and keep these in sync with any
           related changes to the Linux kernel.  We know in advance
           that this will not work really well.

 2) We just make sure not to overwrite array bounds in U-Boot, and
   allow passing arbitrary sized buffers to the Linux kernel.
   Advantage: the code in U-Boot can be simple and clean
   Disadvantage: the resulting behaviour is not exactly user-friendly,
           as a silent truncation in the Linux kernel is probably hard
           to spot.

 Hm...

 I agree that the old code needs fixing.  I think it would be nice if
 we could adapt U-Boot behaviour, but I fear that actually it cannot
 work at all, as we don't know which Linux kernel version the user will
 use, and what their limits might be.

 So in the end 2) is probably the most sensible approach here.

Thank you for all of your time on this issue.  I have just sent a
patch implementing 2) with as little extra cruft as possible (I hope).
 Let me know if there is anything else you need here.  Thanks!

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/2] bootm: Move silencing of linux console to deprecated config option.

[Doug Anderson]

If you would like the old behavior of having bootm modify the bootargs
to silence the linux console when CONFIG_SILENT_CONSOLE is defined,
you now need to define the config CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE.
A previous change already added this new config to all known users of
CONFIG_SILENT_CONSOLE.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 README |5 ++---
 common/cmd_bootm.c |   10 +-
 doc/README.silent  |   13 ++---
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 7916777..9843dd1 100644
--- a/README
+++ b/README
@@ -606,9 +606,8 @@ The following options need to be configured:
environment 'console=serial'.
 
When CONFIG_SILENT_CONSOLE is defined, all console
-   messages (by U-Boot and Linux!) can be silenced with
-   the silent environment variable. See
-   doc/README.silent for more information.
+   messages can be silenced with the silent environment
+   variable. See doc/README.silent for more information.
 
 - Console Baudrate:
CONFIG_BAUDRATE - in bps
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..8d1899e 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -83,8 +83,8 @@ extern flash_info_t flash_info[]; /* info for FLASH chips */
 static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 
-#ifdef CONFIG_SILENT_CONSOLE
-static void fixup_silent_linux(void);
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
+static void fixup_silent_linux(void) __attribute__ ((deprecated));
 #endif
 
 static image_header_t *image_get_kernel(ulong img_addr, int verify);
@@ -673,7 +673,7 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
 
show_boot_progress(8);
 
-#ifdef CONFIG_SILENT_CONSOLE
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
if (images.os.os == IH_OS_LINUX)
fixup_silent_linux();
 #endif
@@ -1228,7 +1228,7 @@ U_BOOT_CMD(
 /***/
 /* helper routines */
 /***/
-#ifdef CONFIG_SILENT_CONSOLE
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 static void fixup_silent_linux(void)
 {
char buf[256], *start, *end;
@@ -1259,7 +1259,7 @@ static void fixup_silent_linux(void)
setenv(bootargs, buf);
debug(after silent fix-up: %s\n, buf);
 }
-#endif /* CONFIG_SILENT_CONSOLE */
+#endif /* CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE */
 
 
 /***/
diff --git a/doc/README.silent b/doc/README.silent
index a26e3df..8b4f892 100644
--- a/doc/README.silent
+++ b/doc/README.silent
@@ -15,6 +15,13 @@ The following actions are taken if silent is set at boot 
time:
suppressed automatically. Make sure to enable nulldev by
#defining CONFIG_SYS_DEVICE_NULLDEV in your board config file.
 
- - When booting a linux kernel, the bootargs are fixed up so that
-   the argument console= will be in the command line, no matter how
-   it was set in bootargs before.
+
+The config option CONFIG_SILENT_CONSOLE previously also caused u-boot
+to silence the Linux console (also based on the silent environment
+variable) by modifying the bootargs so that the argument console=
+would be in the command line no matter how it was set in bootargs
+before.  That behavior is now relegated to the config option
+CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE with the warning that using
+the option opens you up to a buffer overrun if your linux bootargs
+can be 256 bytes.  A non-deprecated solution is to use scripts to
+dynamically construct your console= argument.
-- 
1.7.3.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/2] Deprecate Linux bootargs munging with CONFIG_SILENT_CONSOLE.

[Doug Anderson]

As discussed previously on the U-Boot mailing list (see comments on
Fix fixup_silent_linux() buffer overrun patchset), relying on
bootm to mangle the Linux bootargs is not a suggested way to go.
We now officially deprecate it and provide a way to avoid it (but
still get the other CONFIG_SILENT_CONSOLE behavior).


Doug Anderson (2):
  config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
  bootm: Move silencing of linux console to deprecated config option.

 README |5 ++---
 common/cmd_bootm.c |   10 +-
 doc/README.silent  |   13 ++---
 include/configs/KUP4K.h|3 +++
 include/configs/KUP4X.h|3 +++
 include/configs/QS823.h|1 +
 include/configs/QS850.h|1 +
 include/configs/QS860T.h   |1 +
 include/configs/TQM5200.h  |3 +++
 include/configs/a4m072.h   |3 +++
 include/configs/bfin_adi_common.h  |3 +++
 include/configs/cm5200.h   |3 +++
 include/configs/cpu9260.h  |3 +++
 include/configs/cpuat91.h  |3 +++
 include/configs/mcc200.h   |3 +++
 include/configs/mimc200.h  |3 +++
 include/configs/omap3_evm_quick_mmc.h  |3 +++
 include/configs/omap3_evm_quick_nand.h |3 +++
 include/configs/pdm360ng.h |3 +++
 include/configs/sc3.h  |3 +++
 20 files changed, 62 insertions(+), 11 deletions(-)

-- 
1.7.3.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/2] config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

[Doug Anderson]

I have set this config option based on the existing usage of
CONFIG_SILENT_CONSOLE.  This is to support a future change
deprecating the silencing of the linux console in bootm by
having bootm modify the linux command-line arguments.

Signed-off-by: Doug Anderson diand...@chromium.org
---
 include/configs/KUP4K.h|3 +++
 include/configs/KUP4X.h|3 +++
 include/configs/QS823.h|1 +
 include/configs/QS850.h|1 +
 include/configs/QS860T.h   |1 +
 include/configs/TQM5200.h  |3 +++
 include/configs/a4m072.h   |3 +++
 include/configs/bfin_adi_common.h  |3 +++
 include/configs/cm5200.h   |3 +++
 include/configs/cpu9260.h  |3 +++
 include/configs/cpuat91.h  |3 +++
 include/configs/mcc200.h   |3 +++
 include/configs/mimc200.h  |3 +++
 include/configs/omap3_evm_quick_mmc.h  |3 +++
 include/configs/omap3_evm_quick_nand.h |3 +++
 include/configs/pdm360ng.h |3 +++
 include/configs/sc3.h  |3 +++
 17 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/include/configs/KUP4K.h b/include/configs/KUP4K.h
index c0035e6..7f55748 100644
--- a/include/configs/KUP4K.h
+++ b/include/configs/KUP4K.h
@@ -502,6 +502,9 @@
 #define CONFIG_SYS_DEVICE_NULLDEV  1   /* enble null device
*/
 #define CONFIG_VERSION_VARIABLE1
 
+/* Added based on usage of CONFIG_SILENT_CONSOLE; please remove when possible 
*/
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE 1
+
 /* pass open firmware flat tree */
 #define CONFIG_OF_LIBFDT   1
 #define CONFIG_OF_BOARD_SETUP  1
diff --git a/include/configs/KUP4X.h b/include/configs/KUP4X.h
index 5084ccc..07233fa 100644
--- a/include/configs/KUP4X.h
+++ b/include/configs/KUP4X.h
@@ -452,6 +452,9 @@
 #define CONFIG_AUTOBOOT_STOP_STR   . /* easy to stop for now 
*/
 #define CONFIG_SILENT_CONSOLE  1
 
+/* Added based on usage of CONFIG_SILENT_CONSOLE; please remove when possible 
*/
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE 1
+
 #define CONFIG_USB_STORAGE 1
 #define CONFIG_USB_SL811HS 1
 
diff --git a/include/configs/QS823.h b/include/configs/QS823.h
index 36efbf2..84aea2a 100644
--- a/include/configs/QS823.h
+++ b/include/configs/QS823.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/QS850.h b/include/configs/QS850.h
index 5c6ed07..15e6adb 100644
--- a/include/configs/QS850.h
+++ b/include/configs/QS850.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/QS860T.h b/include/configs/QS860T.h
index b0bee82..99ac280 100644
--- a/include/configs/QS860T.h
+++ b/include/configs/QS860T.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/TQM5200.h b/include/configs/TQM5200.h
index 6ea3faa..9415f29 100644
--- a/include/configs/TQM5200.h
+++ b/include/configs/TQM5200.h
@@ -71,6 +71,9 @@
 #define CONFIG_SILENT_CONSOLE  1   /* enable silent startup */
 #define CONFIG_BOARD_EARLY_INIT_F  1   /* used to detect S1 switch 
position */
 #define CONFIG_USB_BIN_FIXUP   1   /* for a buggy USB device */
+
+/* Added based on usage of CONFIG_SILENT_CONSOLE; please remove when possible 
*/
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE 1
 #if 0
 #define FO300_SILENT_CONSOLE_WHEN_S1_CLOSED1   /* silent console on 
PSC1 when S1 */
/* switch is closed */
diff --git a/include/configs/a4m072.h b/include/configs/a4m072.h
index 1c13904..bbd34d4 100644
--- a/include/configs/a4m072.h
+++ b/include/configs/a4m072.h
@@ -55,6 +55,9 @@
 #define CONFIG_SILENT_CONSOLE
 #define CONFIG_SYS_DEVICE_NULLDEV  1   /* include nulldev device */
 
+/* Added

Re: [U-Boot] [PATCH 1/2] config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

[Doug Anderson]

Dear Wolfgang,

On Tue, Jan 10, 2012 at 11:46 AM, Wolfgang Denk w...@denx.de wrote:
 Hm...actually - why do we need both now?  This is ugly and makes
 little sense to me.

Having both CONFIG_SILENT_CONSOLE and
CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE separate allows us to continue
to use CONFIG_SILENT_CONSOLE and then undefine the
CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE setting from boards one by one
as people either decide that they don't need it or come up with other
options.

Being able to undefine CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE allows
you to avoid the 256-byte buffer overrun while still using the rest of
the CONFIG_SILENT_CONSOLE feature.

At the moment, I haven't sent up any patches that have
CONFIG_SILENT_CONSOLE defined but _not_
CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE, since the config file I'm
currently working with hadn't made it all the way upstream yet.



I will resubmit the series with your other suggestions.  Thanks!
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 0/2] Deprecate Linux bootargs munging with CONFIG_SILENT_CONSOLE.

[Doug Anderson]

On Tue, Jan 10, 2012 at 12:18 PM, Mike Frysinger vap...@gentoo.org wrote:
 personally i found the current behavior useful, but the code to implement it
 is crappy.  oh well.

Agreed.  However, in the previous thread Wolfgang was of the view that
the behavior of silencing linux is best achieved with scripts.  I'm OK
with that approach which is why I've submitted the current patch.

I believe that you can do a script something like this (where
normal_bootargs is the old bootargs without the console= part,
console_args is the non-silent console settings, and old_bootcmd is
the old bootcmd):

setenv generate_bootargs 'if test -n $silent; then \
setenv bootargs $normal_bootargs console=; \
  else \
setenv bootargs $normal_bootargs $console_args;
  fi'
setenv bootcmd 'run generate_bootargs; run old_bootcmd'

Hopefully this works for you.  I'll add it to the documentation, too.


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/2] Deprecate Linux bootargs munging with CONFIG_SILENT_CONSOLE.

[Doug Anderson]

As discussed previously on the U-Boot mailing list (see comments on
Fix fixup_silent_linux() buffer overrun patchset), relying on
bootm to mangle the Linux bootargs is not a suggested way to go.
We now officially deprecate it and provide a way to avoid it (but
still get the other CONFIG_SILENT_CONSOLE behavior).

Changes in v2:
- Define without a value, since this selects a feature only.
- Moved define of CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE to
always be right next to the define of CONFIG_SILENT_CONSOLE
- Better description of CONFIG_SILENT_CONSOLE in README
- Example of how to use a script to silence Linux console in a non-
deprecated way in doc/README.silent

Doug Anderson (2):
  config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
  bootm: Move silencing of linux console to deprecated config option.

 README |   10 ++
 common/cmd_bootm.c |   10 +-
 doc/README.silent  |   27 +++
 include/configs/KUP4K.h|1 +
 include/configs/KUP4X.h|1 +
 include/configs/QS823.h|1 +
 include/configs/QS850.h|1 +
 include/configs/QS860T.h   |1 +
 include/configs/TQM5200.h  |1 +
 include/configs/a4m072.h   |1 +
 include/configs/bfin_adi_common.h  |1 +
 include/configs/cm5200.h   |1 +
 include/configs/cpu9260.h  |1 +
 include/configs/cpuat91.h  |1 +
 include/configs/mcc200.h   |1 +
 include/configs/mimc200.h  |1 +
 include/configs/omap3_evm_quick_mmc.h  |1 +
 include/configs/omap3_evm_quick_nand.h |1 +
 include/configs/pdm360ng.h |1 +
 include/configs/sc3.h  |1 +
 20 files changed, 51 insertions(+), 13 deletions(-)

-- 
1.7.3.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/2] config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

[Doug Anderson]

I have set this config option based on the existing usage of
CONFIG_SILENT_CONSOLE.  This is to support a future change
deprecating the silencing of the linux console in bootm by
having bootm modify the linux command-line arguments.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Define without a value, since this selects a feature only.
- Moved define of CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE to
always be right next to the define of CONFIG_SILENT_CONSOLE

 include/configs/KUP4K.h|1 +
 include/configs/KUP4X.h|1 +
 include/configs/QS823.h|1 +
 include/configs/QS850.h|1 +
 include/configs/QS860T.h   |1 +
 include/configs/TQM5200.h  |1 +
 include/configs/a4m072.h   |1 +
 include/configs/bfin_adi_common.h  |1 +
 include/configs/cm5200.h   |1 +
 include/configs/cpu9260.h  |1 +
 include/configs/cpuat91.h  |1 +
 include/configs/mcc200.h   |1 +
 include/configs/mimc200.h  |1 +
 include/configs/omap3_evm_quick_mmc.h  |1 +
 include/configs/omap3_evm_quick_nand.h |1 +
 include/configs/pdm360ng.h |1 +
 include/configs/sc3.h  |1 +
 17 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/include/configs/KUP4K.h b/include/configs/KUP4K.h
index c0035e6..3cbe199 100644
--- a/include/configs/KUP4K.h
+++ b/include/configs/KUP4K.h
@@ -499,6 +499,7 @@
 #define CONFIG_AUTOBOOT_KEYED  /* use key strings to stop autoboot */
 #define CONFIG_AUTOBOOT_STOP_STR   .
 #define CONFIG_SILENT_CONSOLE  1
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #define CONFIG_SYS_DEVICE_NULLDEV  1   /* enble null device
*/
 #define CONFIG_VERSION_VARIABLE1
 
diff --git a/include/configs/KUP4X.h b/include/configs/KUP4X.h
index 5084ccc..c092a90 100644
--- a/include/configs/KUP4X.h
+++ b/include/configs/KUP4X.h
@@ -451,6 +451,7 @@
 
 #define CONFIG_AUTOBOOT_STOP_STR   . /* easy to stop for now 
*/
 #define CONFIG_SILENT_CONSOLE  1
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 
 #define CONFIG_USB_STORAGE 1
 #define CONFIG_USB_SL811HS 1
diff --git a/include/configs/QS823.h b/include/configs/QS823.h
index 36efbf2..84aea2a 100644
--- a/include/configs/QS823.h
+++ b/include/configs/QS823.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/QS850.h b/include/configs/QS850.h
index 5c6ed07..15e6adb 100644
--- a/include/configs/QS850.h
+++ b/include/configs/QS850.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/QS860T.h b/include/configs/QS860T.h
index b0bee82..99ac280 100644
--- a/include/configs/QS860T.h
+++ b/include/configs/QS860T.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/TQM5200.h b/include/configs/TQM5200.h
index 6ea3faa..c9860fa 100644
--- a/include/configs/TQM5200.h
+++ b/include/configs/TQM5200.h
@@ -69,6 +69,7 @@
 #ifdef CONFIG_FO300
 #define CONFIG_SYS_DEVICE_NULLDEV  1   /* enable null device */
 #define CONFIG_SILENT_CONSOLE  1   /* enable silent startup */
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #define CONFIG_BOARD_EARLY_INIT_F  1   /* used to detect S1 switch 
position */
 #define CONFIG_USB_BIN_FIXUP   1   /* for a buggy USB device */
 #if 0
diff --git a/include/configs/a4m072.h b/include/configs/a4m072.h
index 1c13904..ebb1a32 100644
--- a/include/configs/a4m072.h
+++ b/include/configs/a4m072.h
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_BAUDRATE_TABLE  { 9600, 19200, 38400, 57600, 115200, 
230400 }
 /* define to enable silent console */
 #define CONFIG_SILENT_CONSOLE
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #define

[U-Boot] [PATCH v2 2/2] bootm: Move silencing of linux console to deprecated config option.

[Doug Anderson]

If you would like the old behavior of having bootm modify the bootargs
to silence the linux console when CONFIG_SILENT_CONSOLE is defined,
you now need to define the config CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE.
A previous change already added this new config to all known users of
CONFIG_SILENT_CONSOLE.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Better description of CONFIG_SILENT_CONSOLE in README
- Example of how to use a script to silence Linux console in a non-
deprecated way in doc/README.silent

 README |   10 ++
 common/cmd_bootm.c |   10 +-
 doc/README.silent  |   27 +++
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/README b/README
index 7916777..5ba8482 100644
--- a/README
+++ b/README
@@ -605,10 +605,12 @@ The following options need to be configured:
default i/o. Serial console can be forced with
environment 'console=serial'.
 
-   When CONFIG_SILENT_CONSOLE is defined, all console
-   messages (by U-Boot and Linux!) can be silenced with
-   the silent environment variable. See
-   doc/README.silent for more information.
+   When CONFIG_SILENT_CONSOLE is defined, all U-Boot console
+   messages can be silenced with the silent environment
+   variable. Linux console messages will not be silenced
+   based on the silent environment variable unless
+   CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE is defined.
+   See doc/README.silent for more information.
 
 - Console Baudrate:
CONFIG_BAUDRATE - in bps
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..8d1899e 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -83,8 +83,8 @@ extern flash_info_t flash_info[]; /* info for FLASH chips */
 static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 
-#ifdef CONFIG_SILENT_CONSOLE
-static void fixup_silent_linux(void);
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
+static void fixup_silent_linux(void) __attribute__ ((deprecated));
 #endif
 
 static image_header_t *image_get_kernel(ulong img_addr, int verify);
@@ -673,7 +673,7 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
 
show_boot_progress(8);
 
-#ifdef CONFIG_SILENT_CONSOLE
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
if (images.os.os == IH_OS_LINUX)
fixup_silent_linux();
 #endif
@@ -1228,7 +1228,7 @@ U_BOOT_CMD(
 /***/
 /* helper routines */
 /***/
-#ifdef CONFIG_SILENT_CONSOLE
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 static void fixup_silent_linux(void)
 {
char buf[256], *start, *end;
@@ -1259,7 +1259,7 @@ static void fixup_silent_linux(void)
setenv(bootargs, buf);
debug(after silent fix-up: %s\n, buf);
 }
-#endif /* CONFIG_SILENT_CONSOLE */
+#endif /* CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE */
 
 
 /***/
diff --git a/doc/README.silent b/doc/README.silent
index a26e3df..a4b8a97 100644
--- a/doc/README.silent
+++ b/doc/README.silent
@@ -1,5 +1,5 @@
 The config option CONFIG_SILENT_CONSOLE can be used to quiet messages
-on the console.  If the option has been enabled, the output can be
+on the U-Boot console.  If the option has been enabled, the output can be
 silenced by setting the environment variable silent.  The variable
 is latched into the global data at an early stage in the boot process
 so deleting it with setenv will not take effect until the system is
@@ -15,6 +15,25 @@ The following actions are taken if silent is set at boot 
time:
suppressed automatically. Make sure to enable nulldev by
#defining CONFIG_SYS_DEVICE_NULLDEV in your board config file.
 
- - When booting a linux kernel, the bootargs are fixed up so that
-   the argument console= will be in the command line, no matter how
-   it was set in bootargs before.
+
+The config option CONFIG_SILENT_CONSOLE previously also caused u-boot
+to silence the Linux console (also based on the silent environment
+variable) by modifying the bootargs so that the argument console=
+would be in the command line no matter how it was set in bootargs
+before.  That behavior is now relegated to the config option
+CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE with the warning that using
+the option opens you up to a buffer overrun if your linux bootargs
+can be 256 bytes.
+
+If you were relying on the old behavior of CONFIG_SILENT_CONSOLE to
+also silence the Linux console, a script like this may help you (where
+normal_bootargs is the old bootargs without the console= part,
+console_args is the non-silent console settings, and old_bootcmd is
+the old bootcmd):
+
+ setenv generate_bootargs 'if test -n

Re: [U-Boot] [PATCH 0/2] Deprecate Linux bootargs munging with CONFIG_SILENT_CONSOLE.

[Doug Anderson]

Dear Wolfgang Denk,

On Tue, Jan 10, 2012 at 2:16 PM, Wolfgang Denk w...@denx.de wrote:
 In which way would this approach avoid the problem (potential overflow
 of cmdline max size) that you are trying to fix with your patch?

The overflow will be avoided on any boards that don't define
CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE.  As of the third version of
this patch (will be sent shortly), the Blackfin board will be fixed so
that the overflow can't happen on that board.

 So you actually do not fix any problem, but you remove existing
 functionality that has been used in a number of projects, so you
 actually break a number of boards.

With the third version of this patch the Blackfin board is one example
of a board that is fixed.  On all other boards, no functionality is
lost and no bugs are fixed.


Previously I had submitted a patch to fix the overflow itself, which
you NAKed (saying that fixup_silent_linux() was deprecated and
shouldn't get any bugfixes).  I am happy to dig that up and re-post it
if you'd prefer.  Either option is fine with me.

My problem is that I have a board that would like to use
CONFIG_SILENT_CONSOLE (to silence the U-Boot console) and have a Linux
command line that is 256 characters.  I either need the overflow
fixed (my previous patch) or some way to avoid it (this patch).


Thanks much!

-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 0/3] Deprecate Linux bootargs munging with CONFIG_SILENT_CONSOLE.

[Doug Anderson]

As discussed previously on the U-Boot mailing list (see comments on
Fix fixup_silent_linux() buffer overrun patchset), relying on
bootm to mangle the Linux bootargs is not a suggested way to go.
We now officially deprecate it and provide a way to avoid it (but
still get the other CONFIG_SILENT_CONSOLE behavior).

Changes in v2:
- Define without a value, since this selects a feature only.
- Moved define of CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE to
always be right next to the define of CONFIG_SILENT_CONSOLE
- Better description of CONFIG_SILENT_CONSOLE in README
- Example of how to use a script to silence Linux console in a non-
deprecated way in doc/README.silent

Changes in v3:
- Use __deprecated #define instead of direct gcc attribute
- Minor wording updates in doc/README.silent
- Added part 3 of the patch to turn off the deprecated config option
for Blackfin, where it was confirmed that it's not needed.

Doug Anderson (3):
  config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
  bootm: Move silencing of linux console to deprecated config option.
  config: Remove Blackfin CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

 README |   10 ++
 common/cmd_bootm.c |   10 +-
 doc/README.silent  |   27 +++
 include/configs/KUP4K.h|1 +
 include/configs/KUP4X.h|1 +
 include/configs/QS823.h|1 +
 include/configs/QS850.h|1 +
 include/configs/QS860T.h   |1 +
 include/configs/TQM5200.h  |1 +
 include/configs/a4m072.h   |1 +
 include/configs/cm5200.h   |1 +
 include/configs/cpu9260.h  |1 +
 include/configs/cpuat91.h  |1 +
 include/configs/mcc200.h   |1 +
 include/configs/mimc200.h  |1 +
 include/configs/omap3_evm_quick_mmc.h  |1 +
 include/configs/omap3_evm_quick_nand.h |1 +
 include/configs/pdm360ng.h |1 +
 include/configs/sc3.h  |1 +
 19 files changed, 50 insertions(+), 13 deletions(-)

-- 
1.7.3.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 1/3] config: Add CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

[Doug Anderson]

I have set this config option based on the existing usage of
CONFIG_SILENT_CONSOLE.  This is to support a future change
deprecating the silencing of the linux console in bootm by
having bootm modify the linux command-line arguments.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Define without a value, since this selects a feature only.
- Moved define of CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE to
always be right next to the define of CONFIG_SILENT_CONSOLE

 include/configs/KUP4K.h|1 +
 include/configs/KUP4X.h|1 +
 include/configs/QS823.h|1 +
 include/configs/QS850.h|1 +
 include/configs/QS860T.h   |1 +
 include/configs/TQM5200.h  |1 +
 include/configs/a4m072.h   |1 +
 include/configs/bfin_adi_common.h  |1 +
 include/configs/cm5200.h   |1 +
 include/configs/cpu9260.h  |1 +
 include/configs/cpuat91.h  |1 +
 include/configs/mcc200.h   |1 +
 include/configs/mimc200.h  |1 +
 include/configs/omap3_evm_quick_mmc.h  |1 +
 include/configs/omap3_evm_quick_nand.h |1 +
 include/configs/pdm360ng.h |1 +
 include/configs/sc3.h  |1 +
 17 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/include/configs/KUP4K.h b/include/configs/KUP4K.h
index c0035e6..3cbe199 100644
--- a/include/configs/KUP4K.h
+++ b/include/configs/KUP4K.h
@@ -499,6 +499,7 @@
 #define CONFIG_AUTOBOOT_KEYED  /* use key strings to stop autoboot */
 #define CONFIG_AUTOBOOT_STOP_STR   .
 #define CONFIG_SILENT_CONSOLE  1
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #define CONFIG_SYS_DEVICE_NULLDEV  1   /* enble null device
*/
 #define CONFIG_VERSION_VARIABLE1
 
diff --git a/include/configs/KUP4X.h b/include/configs/KUP4X.h
index 5084ccc..c092a90 100644
--- a/include/configs/KUP4X.h
+++ b/include/configs/KUP4X.h
@@ -451,6 +451,7 @@
 
 #define CONFIG_AUTOBOOT_STOP_STR   . /* easy to stop for now 
*/
 #define CONFIG_SILENT_CONSOLE  1
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 
 #define CONFIG_USB_STORAGE 1
 #define CONFIG_USB_SL811HS 1
diff --git a/include/configs/QS823.h b/include/configs/QS823.h
index 36efbf2..84aea2a 100644
--- a/include/configs/QS823.h
+++ b/include/configs/QS823.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/QS850.h b/include/configs/QS850.h
index 5c6ed07..15e6adb 100644
--- a/include/configs/QS850.h
+++ b/include/configs/QS850.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/QS860T.h b/include/configs/QS860T.h
index b0bee82..99ac280 100644
--- a/include/configs/QS860T.h
+++ b/include/configs/QS860T.h
@@ -37,6 +37,7 @@
 /* various debug settings */
 #undef CONFIG_SYS_DEVICE_NULLDEV   /* null device */
 #undef CONFIG_SILENT_CONSOLE   /* silent console */
+#undef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #undef CONFIG_SYS_CONSOLE_INFO_QUIET   /* silent console ? */
 #undef DEBUG_FLASH /* debug flash code */
 #undef FLASH_DEBUG /* debug fash code */
diff --git a/include/configs/TQM5200.h b/include/configs/TQM5200.h
index 6ea3faa..c9860fa 100644
--- a/include/configs/TQM5200.h
+++ b/include/configs/TQM5200.h
@@ -69,6 +69,7 @@
 #ifdef CONFIG_FO300
 #define CONFIG_SYS_DEVICE_NULLDEV  1   /* enable null device */
 #define CONFIG_SILENT_CONSOLE  1   /* enable silent startup */
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #define CONFIG_BOARD_EARLY_INIT_F  1   /* used to detect S1 switch 
position */
 #define CONFIG_USB_BIN_FIXUP   1   /* for a buggy USB device */
 #if 0
diff --git a/include/configs/a4m072.h b/include/configs/a4m072.h
index 1c13904..ebb1a32 100644
--- a/include/configs/a4m072.h
+++ b/include/configs/a4m072.h
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_BAUDRATE_TABLE  { 9600, 19200, 38400, 57600, 115200, 
230400 }
 /* define to enable silent console */
 #define CONFIG_SILENT_CONSOLE
+#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #define

[U-Boot] [PATCH v3 3/3] config: Remove Blackfin CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE

[Doug Anderson]

As Mike Frysinger writes:
  We didn't enable silent=1 by default in any of the Blackfin boards
  Just made the functionality available to people if they wanted to
  test things out with it when prototyping on dev boards.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v3:
- Added part 3 of the patch to turn off the deprecated config option
for Blackfin, where it was confirmed that it's not needed.

 include/configs/bfin_adi_common.h |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/include/configs/bfin_adi_common.h 
b/include/configs/bfin_adi_common.h
index 4729e7b..3fbf5c6 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -108,7 +108,6 @@
 #define CONFIG_LOADS_ECHO  1
 #define CONFIG_JTAG_CONSOLE
 #define CONFIG_SILENT_CONSOLE
-#define CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 #ifndef CONFIG_BAUDRATE
 # define CONFIG_BAUDRATE   57600
 #endif
-- 
1.7.3.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v3 2/3] bootm: Move silencing of linux console to deprecated config option.

[Doug Anderson]

If you would like the old behavior of having bootm modify the bootargs
to silence the linux console when CONFIG_SILENT_CONSOLE is defined,
you now need to define the config CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE.
A previous change already added this new config to all known users of
CONFIG_SILENT_CONSOLE.

Signed-off-by: Doug Anderson diand...@chromium.org
---
Changes in v2:
- Better description of CONFIG_SILENT_CONSOLE in README
- Example of how to use a script to silence Linux console in a non-
deprecated way in doc/README.silent

Changes in v3:
- Use __deprecated #define instead of direct gcc attribute
- Minor wording updates in doc/README.silent

 README |   10 ++
 common/cmd_bootm.c |   10 +-
 doc/README.silent  |   27 +++
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/README b/README
index 7916777..5ba8482 100644
--- a/README
+++ b/README
@@ -605,10 +605,12 @@ The following options need to be configured:
default i/o. Serial console can be forced with
environment 'console=serial'.
 
-   When CONFIG_SILENT_CONSOLE is defined, all console
-   messages (by U-Boot and Linux!) can be silenced with
-   the silent environment variable. See
-   doc/README.silent for more information.
+   When CONFIG_SILENT_CONSOLE is defined, all U-Boot console
+   messages can be silenced with the silent environment
+   variable. Linux console messages will not be silenced
+   based on the silent environment variable unless
+   CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE is defined.
+   See doc/README.silent for more information.
 
 - Console Baudrate:
CONFIG_BAUDRATE - in bps
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d5745b1..775a7dc 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -83,8 +83,8 @@ extern flash_info_t flash_info[]; /* info for FLASH chips */
 static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 
-#ifdef CONFIG_SILENT_CONSOLE
-static void fixup_silent_linux(void);
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
+static void fixup_silent_linux(void) __deprecated;
 #endif
 
 static image_header_t *image_get_kernel(ulong img_addr, int verify);
@@ -673,7 +673,7 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * 
const argv[])
 
show_boot_progress(8);
 
-#ifdef CONFIG_SILENT_CONSOLE
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
if (images.os.os == IH_OS_LINUX)
fixup_silent_linux();
 #endif
@@ -1228,7 +1228,7 @@ U_BOOT_CMD(
 /***/
 /* helper routines */
 /***/
-#ifdef CONFIG_SILENT_CONSOLE
+#ifdef CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE
 static void fixup_silent_linux(void)
 {
char buf[256], *start, *end;
@@ -1259,7 +1259,7 @@ static void fixup_silent_linux(void)
setenv(bootargs, buf);
debug(after silent fix-up: %s\n, buf);
 }
-#endif /* CONFIG_SILENT_CONSOLE */
+#endif /* CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE */
 
 
 /***/
diff --git a/doc/README.silent b/doc/README.silent
index a26e3df..32739ed 100644
--- a/doc/README.silent
+++ b/doc/README.silent
@@ -1,5 +1,5 @@
 The config option CONFIG_SILENT_CONSOLE can be used to quiet messages
-on the console.  If the option has been enabled, the output can be
+on the U-Boot console.  If the option has been enabled, the output can be
 silenced by setting the environment variable silent.  The variable
 is latched into the global data at an early stage in the boot process
 so deleting it with setenv will not take effect until the system is
@@ -15,6 +15,25 @@ The following actions are taken if silent is set at boot 
time:
suppressed automatically. Make sure to enable nulldev by
#defining CONFIG_SYS_DEVICE_NULLDEV in your board config file.
 
- - When booting a linux kernel, the bootargs are fixed up so that
-   the argument console= will be in the command line, no matter how
-   it was set in bootargs before.
+
+The config option CONFIG_SILENT_CONSOLE previously also caused u-boot
+to silence the Linux console (also based on the silent environment
+variable) by modifying the bootargs so that the argument console=
+would be in the command line no matter how it was set in bootargs
+before.  That behavior is now relegated to the config option
+CONFIG_DEPRECATED_SILENT_LINUX_CONSOLE with the caveat that using
+the option opens you up to a buffer overrun if your linux bootargs
+are 256 bytes.
+
+If you were relying on the old behavior of CONFIG_SILENT_CONSOLE to
+also silence the Linux console, a script like this might help you (where
+normal_bootargs is the old bootargs without the console= part,
+console_args is the non-silent

Re: [U-Boot] [PATCH v2 0/2] Deprecate Linux bootargs munging with CONFIG_SILENT_CONSOLE.

[Doug Anderson]

Dear Wolfgang Denk

On Tue, Jan 10, 2012 at 2:30 PM, Wolfgang Denk w...@denx.de wrote:
 As discussed previously on the U-Boot mailing list (see comments on
 Fix fixup_silent_linux() buffer overrun patchset), relying on

 Please provide a link.

Sure!  This is the message I'm referring to:
http://lists.denx.de/pipermail/u-boot/2011-October/106255.html.  I
know it was a long time ago--I got a bit sidetracked for a while.
Specifically, I offered to provide a very simple version of
fixup_silent_linux() that just fixed the buffer overrun and you said
Please consider it NAKed.


 bootm to mangle the Linux bootargs is not a suggested way to go.

 So what is a suggested way to go to silence the Linux kenrel
 messages?

Please see doc/README.silent in part 1 of this patchset for a sample script.


 You are removing functionality that is in active use by a number of
 boards, without providing any replacement, i. e. you are breaking a
 number of boards.

 Do you think this is a good idea?

This change doesn't remove any functionality but instead deprecates it
and provides a path for boards to move forward to a non-deprecated
solution.  The non-deprecated solution is documented in
doc/README.silent.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] bootm: Avoid 256-byte overflow in fixup_silent_linux()

[Doug Anderson]

Dear Wolfgang Denk,

On Tue, Jan 10, 2012 at 2:28 PM, Wolfgang Denk w...@denx.de wrote:
 This makes fixup_silent_linux() use malloc() to allocate its
 working space, meaning that our maximum kernel command line
 should only be limited by malloc().  Previously it was silently
 overflowing the stack.
 ...
  static void fixup_silent_linux(void)
  {
 -     char buf[256], *start, *end;

 Are you sure that the kernel's buffer is long enough?

The kernel's buffer might be big enough, depending on the
architecture.  For ARM (which is what I'm on), I see that the command
line is 1024 bytes.

Here's where I'm looking:
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=arch/arm/include/asm/setup.h;h=23ebc0c82a3975ae5c455dd39598e93ab33922e7;hb=refs/heads/master#l19


 I think your patch is likely to break all these architectures?

I'm not sure how my patch would break these architectures.  My patch
removes a buffer overrun--it doesn't actually increase any particular
board's command line length.  I need this because my board uses a
command line that is ~300 bytes--under the kernel limit but currently
over u-boot's.

I agree completely that this patch doesn't remove all limits on Linux
command line length.  However, it does allow you to use the full Linux
command line length on kernels that have a #define with something
greater than 256.

In addition, a buffer overrun is a particularly gnarly failure case
(opens you up to all sorts of security attacks if someone can figure
out how to manipulate the command line), so is generally good to fix.

If you'd rather, I'd be happy to rework the patch to change the
hardcoded 256 to a CONFIG_COMMAND_LINE_SIZE, then add overflow
checking to the function.  That would allow my use case and also
prevent future buffer overruns.


-Doug
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot