From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Schwarz Date: Fri, 26 Aug 2011 12:10:37 +0200 Subject: [U-Boot] [PATCH V3 3/8] arm: Add savebp implementation for arm In-Reply-To: <4E562D3A.4090103@gmail.com> References: <1314182363-25732-1-git-send-email-simonschwarzcor@gmail.com> <1314261196-23197-1-git-send-email-simonschwarzcor@gmail.com> <1314261196-23197-4-git-send-email-simonschwarzcor@gmail.com> <4E562D3A.4090103@gmail.com> Message-ID: <4E57711D.6020103@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Andreas, On 08/25/2011 01:08 PM, Andreas Bie?mann wrote: > Dear Simon, > > Am 25.08.2011 10:33, schrieb Simon Schwarz: >> This adds the savebp implementation to the arm platform. > > please reorder your series and let this come before 'Add savebp command' > cause that patch uses functionality from this one. > Will do. >> Related CONFIGs: >> CONFIG_CMD_SAVEBP_WRITE_SIZE defines the size of the image to write >> >> Signed-off-by: Simon Schwarz >> --- >> >> V2 changes: >> DEL _cosmetic_ old comment >> >> V3 changes: >> nothing >> --- >> arch/arm/include/asm/savebp.h | 27 ++++++++++++ >> arch/arm/lib/Makefile | 1 + >> arch/arm/lib/savebp.c | 91 +++++++++++++++++++++++++++++++++++++++++ >> include/command.h | 5 ++ >> include/configs/devkit8000.h | 1 + > > documentation of CONFIG_CMD_SAVEBP_WRITE_SIZE is missing > Will add. >> diff --git a/arch/arm/include/asm/savebp.h b/arch/arm/include/asm/savebp.h >> new file mode 100644 >> index 0000000..3774e45 >> --- /dev/null >> +++ b/arch/arm/include/asm/savebp.h >> @@ -0,0 +1,27 @@ >> +/* Copyright (C) 2011 >> + * Corscience GmbH& Co. KG - Simon Schwarz >> + * >> + * 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 >> + */ >> +#ifndef _SAVEBP_H_ >> +#define _SAVEBP_H_ >> + >> +extern bootm_headers_t images; > > You made that available globally in your first patch of this series, > please remove that from first patch and move to this one. ok. > > > > BTW while reading this patch I got another idea to solve problem 'how > get we saved the boot information to'. > The required information regardless of whether it is ATAGS or FDT is > only a blob at some place in ram after the 'bootm x' commands used in > 'Add savebp command'. Saving a blob from location X with size Y to > location Z is easy and already implemented. > > So the only required thing is to get the 'blob' prepared in RAM. In my > opinion this could be a subcommand of bootm instead of a new command. > > How about: > > ---8<--- > # bootm savebp > ...done boot information is @0x80000100 with size 0x100 > # nandecc hw > # nand erase ... > # nand write 80000100 ... > --->8--- This was actually my first implementation (although not with a switch): http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/102669 It was criticised because saving the parameter images is not the responsibility of bootm - which is true. Although adding a switch would be a cleaner solution than my first implementation - would that be acceptable? How about implementing it as bootm subcommand? > > regards > > Andreas Bie?mann Regards & thx for reviewing Simon