Hi Jorgen,
On Wed, May 23, 2012 at 12:26 PM, Jorgen Lundman <lund...@lundman.net> wrote:
> commit bc192bb0716b02b2b711dc2df62ed15e1160ea50
> Author: Jorgen Lundman <lund...@lundman.net>
> Date: Wed May 23 01:55:02 2012 +0000
>
[snip]
>
> commit bea9588d98f52d95a325f3b71a7ae448242c7b64
> Author: Jorgen Lundman <lund...@lundman.net>
> Date: Thu May 10 05:11:03 2012 +0000
What are all these commit references? Are they from an external git-repo?
If so I think the commit message might make more sense if you simply
summarise what you have done to adapt the original source code in order to
integrate it into U-Boot
>
> Adding ZFS
> ---
> Makefile | 2 +-
> common/Makefile | 1 +
> common/cmd_zfs.c | 244 +++++
> fs/Makefile | 1 +
> fs/{ => zfs}/Makefile | 43 +-
> fs/zfs/dev.c | 139 +++
> fs/zfs/zfs.c | 2414
> ++++++++++++++++++++++++++++++++++++++++++++++
> fs/zfs/zfs_fletcher.c | 84 ++
> fs/zfs/zfs_lzjb.c | 94 ++
> fs/zfs/zfs_sha256.c | 145 +++
> include/config_cmd_all.h | 1 +
> include/zfs_common.h | 94 ++
> 12 files changed, 3246 insertions(+), 16 deletions(-)
> create mode 100644 common/cmd_zfs.c
> copy fs/{ => zfs}/Makefile (52%)
> create mode 100644 fs/zfs/dev.c
> create mode 100644 fs/zfs/zfs.c
> create mode 100644 fs/zfs/zfs_fletcher.c
> create mode 100644 fs/zfs/zfs_lzjb.c
> create mode 100644 fs/zfs/zfs_sha256.c
> create mode 100644 include/zfs_common.h
>
> diff --git a/Makefile b/Makefile
> index 351a8f0..d3b84bf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -244,7 +244,7 @@ endif
> LIBS += arch/$(ARCH)/lib/lib$(ARCH).o
> LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o
> fs/jffs2/libjffs2.o \
> fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o \
> - fs/ubifs/libubifs.o
> + fs/ubifs/libubifs.o fs/zfs/libzfs.o
> LIBS += net/libnet.o
> LIBS += disk/libdisk.o
> LIBS += drivers/bios_emulator/libatibiosemu.o
> diff --git a/common/Makefile b/common/Makefile
> index 6e23baa..181a9ad 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -90,6 +90,7 @@ COBJS-$(CONFIG_CMD_ELF) += cmd_elf.o
> COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_exit.o
> COBJS-$(CONFIG_CMD_EXT2) += cmd_ext2.o
> COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
> +COBJS-$(CONFIG_CMD_ZFS) += cmd_zfs.o
> COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o
> COBJS-$(CONFIG_OF_LIBFDT) += cmd_fdt.o fdt_support.o
> COBJS-$(CONFIG_CMD_FDOS) += cmd_fdos.o
Please keep list sorted
> diff --git a/common/cmd_zfs.c b/common/cmd_zfs.c
> new file mode 100644
> index 0000000..99c4318
> --- /dev/null
> +++ b/common/cmd_zfs.c
> @@ -0,0 +1,244 @@
> +/*
> + *
> + * ZFS filesystem implementation in Uboot by
> + * Jorgen Lundman <lundman at lundman.net>
> + *
> + * zfsfs support
> + * made from existing GRUB Sources by Sun, GNU and others.
> + *
> + * 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
> + *
> + */
> +
> +
> +/*
> + * Changelog:
> + * 0.1 - The Epoch
> + * - lundman
> + */
Don't include changelogs in source files - We have git for that :)
> +
> +#include <common.h>
> +#include <part.h>
> +#include <config.h>
> +#include <command.h>
> +#include <image.h>
> +#include <linux/ctype.h>
> +#include <asm/byteorder.h>
> +#include <zfs_common.h>
> +#include <linux/stat.h>
> +#include <malloc.h>
> +
> +#if defined(CONFIG_CMD_USB) && defined(CONFIG_USB_STORAGE)
> +#include <usb.h>
> +#endif
> +
> +#if !defined(CONFIG_DOS_PARTITION) && !defined(CONFIG_EFI_PARTITION)
> +#error DOS or EFI partition support must be selected
> +#endif
> +
> +#define DOS_PART_MAGIC_OFFSET 0x1fe
> +#define DOS_FS_TYPE_OFFSET 0x36
> +#define DOS_FS32_TYPE_OFFSET 0x52
> +
> +static int do_zfs_load(cmd_tbl_t *cmdtp, int flag, int argc,
> + char *argv[])
Can this be aligned better? Personally I prefer to put each parameter on
it's own line once I hit the 80 character limit
> +{
[snip]
> diff --git a/fs/Makefile b/fs/Makefile
> index 22aad12..b0d62c6 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -25,6 +25,7 @@
> subdirs-$(CONFIG_CMD_CRAMFS) := cramfs
> subdirs-$(CONFIG_CMD_EXT2) += ext2
> subdirs-$(CONFIG_CMD_FAT) += fat
> +subdirs-$(CONFIG_CMD_ZFS) += zfs
> subdirs-$(CONFIG_CMD_FDOS) += fdos
> subdirs-$(CONFIG_CMD_JFFS2) += jffs2
> subdirs-$(CONFIG_CMD_REISER) += reiserfs
Keep sorted
> diff --git a/fs/Makefile b/fs/zfs/Makefile
> similarity index 52%
> copy from fs/Makefile
> copy to fs/zfs/Makefile
> index 22aad12..00ab9e6 100644
> --- a/fs/Makefile
> +++ b/fs/zfs/Makefile
> @@ -1,6 +1,10 @@
> #
> -# (C) Copyright 2000-2006
> -# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> +# (C) Copyright 2006
> +# Wolfgang Denk, DENX Software Engineering, <wd at denx.de>
Please don't mess with other devs copyright lines
> +#
> +# (C) Copyright 2003
> +# Pavel Bartusek, Sysgo Real-Time Solutions AG, <pba at sysgo.de>
> +#
> #
> # See file CREDITS for list of people who contributed to this
> # project.
> @@ -20,19 +24,28 @@
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> # MA 02111-1307 USA
> #
> -#
>
> -subdirs-$(CONFIG_CMD_CRAMFS) := cramfs
> -subdirs-$(CONFIG_CMD_EXT2) += ext2
> -subdirs-$(CONFIG_CMD_FAT) += fat
> -subdirs-$(CONFIG_CMD_FDOS) += fdos
> -subdirs-$(CONFIG_CMD_JFFS2) += jffs2
> -subdirs-$(CONFIG_CMD_REISER) += reiserfs
> -subdirs-$(CONFIG_YAFFS2) += yaffs2
> -subdirs-$(CONFIG_CMD_UBIFS) += ubifs
> +include $(TOPDIR)/config.mk
> +
> +LIB = $(obj)libzfs.o
> +
> +AOBJS =
> +COBJS-$(CONFIG_CMD_ZFS) := dev.o zfs.o zfs_fletcher.o zfs_sha256.o zfs_lzjb.o
> +
> +SRCS := $(AOBJS:.o=.S) $(COBJS-y:.o=.c)
> +OBJS := $(addprefix $(obj),$(AOBJS) $(COBJS-y))
> +
> +
> +all: $(LIB) $(AOBJS)
> +
> +$(LIB): $(obj).depend $(OBJS)
> + $(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
>
> -SUBDIRS := $(subdirs-y)
> +sinclude $(obj).depend
>
> -$(obj).depend all:
> - @for dir in $(SUBDIRS) ; do \
> - $(MAKE) -C $$dir $@ ; done
> +#########################################################################
This looks to be an unrelated change - If needed, move this into a seperate
patch
> diff --git a/fs/zfs/dev.c b/fs/zfs/dev.c
> new file mode 100644
> index 0000000..d61ff80
> --- /dev/null
> +++ b/fs/zfs/dev.c
> @@ -0,0 +1,139 @@
> +
> +int zfs_set_blk_dev(block_dev_desc_t *rbdd, int part)
> +{
> + zfs_block_dev_desc = rbdd;
> +
> + if (part == 0) {
> + /* disk doesn't use partition table */
> + part_info.start = 0;
> + part_info.size = rbdd->lba;
> + part_info.blksz = rbdd->blksz;
> + } else {
> + if (get_partition_info
> + (zfs_block_dev_desc, part, &part_info)) {
> + return 0;
> + }
> + }
Insert a blank line
> + return part_info.size;
> +}
> +
> +/* err */
> +int zfs_devread(int sector, int byte_offset, int byte_len, char *buf)
> +{
> + short sec_buffer[SECTOR_SIZE/sizeof(short)];
> + char *sec_buf = sec_buffer;
> + unsigned block_len;
> +
> + /*
> + * Check partition boundaries
> + */
> + if ((sector < 0)
> + || ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS))
> >=
> + part_info.size)) {
Put the || operator on the previous line and fix vertical alignment of
subsequent lines
> + /* errnum = ERR_OUTSIDE_PART; */
> + printf(" ** zfs_devread() read outside partition sector
> %d\n", sector);
> + return 1;
> + }
> +
> + /*
> + * Get the read to the beginning of a partition.
> + */
> + sector += byte_offset >> SECTOR_BITS;
> + byte_offset &= SECTOR_SIZE - 1;
> +
> + debug(" <%d, %d, %d>\n", sector, byte_offset, byte_len);
> +
> + if (zfs_block_dev_desc == NULL) {
> + printf("** Invalid Block Device Descriptor (NULL)\n");
> + return 1;
> + }
> +
> + if (byte_offset != 0) {
> + /* read first part which isn't aligned with start of sector */
> + if (zfs_block_dev_desc->
Ewww, that's a very ugly split
> + block_read(zfs_block_dev_desc->dev,
> + part_info.start + sector, 1,
> + (unsigned long *) sec_buf) != 1) {
alignment - should look more like:
+ if (zfs_block_dev_desc->block_read(zfs_block_dev_desc->dev,
+ part_info.start + sector,
+ 1,
+ (unsigned long *)
sec_buf) != 1) {
[snip]
> diff --git a/fs/zfs/zfs.c b/fs/zfs/zfs.c
> new file mode 100644
> index 0000000..e7369cd
> --- /dev/null
> +++ b/fs/zfs/zfs.c
> @@ -0,0 +1,2414 @@
> +/*
> + * ZFS filesystem implementation in u-boot by
'ported to' versus 'implementation' ?
> + * Jorgen Lundman <lundman at lundman.net>
> + *
> + * ZFS-fs support
> + * made from existing GRUB Sources by Sun, GNU and others.
Is this an identical copyright attribution from the original source?
> + *
> + * 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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <linux/stat.h>
> +#include <linux/time.h>
> +#include <linux/ctype.h>
> +#include <asm/byteorder.h>
> +#include "zfs_common.h"
> +
> +
> +block_dev_desc_t *zfs_dev_desc;
> +
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 1999,2000,2001,2002,2003,2004,2009,2010
> + * Free Software Foundation, Inc.
> + * Copyright 2010 Sun Microsystems, Inc.
> + *
> + * GRUB 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 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB 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 GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
Wow, I can't believe they put the copyright notice here! Maybe move it
above the #includes
I quickly scanned the rest - nothing else not mentioned above jumped out at
me. For a first cut, this looks pretty good
Regards,
Graeme
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
