[U-Boot] [PATCH v2] mtd: resync with Linux-3.7.1

[Scott Wood]

On 01/17/2013 06:07:39 AM, Sergey Lapin wrote:
> Dear Scott,
> thanks a lot for your review,
> see below.
> 
> On Wed, Jan 16, 2013 at 04:09:45PM -0600, Scott Wood wrote:
> > On 01/14/2013 07:46:50 AM, Sergey Lapin wrote:
> > >This patch is essentially an update of u-boot MTD subsystem to
> > >the state of Linux-3.7.1 with exclusion of some bits:
> > >
> > >- the update is concentrated on NAND, no onenand or CFI/NOR/SPI
> > >flashes interfaces are updated EXCEPT for API changes.
> > >
> > >- new large NAND chips support is there, though some updates
> > >have got in Linux-3.8.-rc1, (which will follow on top of this  
> patch).
> > >
> > >To produce this update I used tag v3.7.1 of linux-stable  
> repository.
> > >
> > >The update was made using application of relevant patches,
> > >with changes relevant to U-Boot-only stuff sticked together
> > >to keep bisectability. Then all changes were grouped together
> > >to this patch.
> > >
> > >Signed-off-by: Sergey Lapin <slapin at ossfans.org>
> > >---
> >
> > Applying: mtd: resync with Linux-3.7.1
> > /home/scott/fsl/git/u-boot/upstream/.git/rebase-apply/patch:4292:
> > space before tab in indent.
> >  		chip->ecc.strength =
> > /home/scott/fsl/git/u-boot/upstream/.git/rebase-apply/patch:835: new
> > blank line at EOF.
> > +
> > /home/scott/fsl/git/u-boot/upstream/.git/rebase-apply/patch:6011:
> > new blank line at EOF.
> > +
> > /home/scott/fsl/git/u-boot/upstream/.git/rebase-apply/patch:7970:
> > new blank line at EOF.
> > +
> > warning: 4 lines add whitespace errors.
> >
> > Are these whitespace errors in Linux?
> Yeah, this thing wonders me why. I can fix these without too much
> hussle, should I?

They don't appear to have come from the Linux sources, so yes, please  
fix them.

> > You'll probably need to go through the various NAND patches between  
> 3.0
> > and 3.7.1 looking for API changes, and make sure that they're all
> > accounted for, beyond just making things build.
> This is what I did most of time, shit happens.

OK...  ecc.strength is going to need to be fixed on several drivers  
(pretty much everything that uses hardware ecc).  When I fixed that,  
elbc worked.

> > >diff --git a/board/ait/cam_enc_4xx/cam_enc_4xx.c
> > >b/board/ait/cam_enc_4xx/cam_enc_4xx.c
> > >index 32b28f9..2a0c31c 100644
> > >--- a/board/ait/cam_enc_4xx/cam_enc_4xx.c
> > >+++ b/board/ait/cam_enc_4xx/cam_enc_4xx.c
> > >@@ -120,7 +120,7 @@ int board_eth_init(bd_t *bis)
> > > #ifdef CONFIG_NAND_DAVINCI
> > > static int
> > > davinci_std_read_page_syndrome(struct mtd_info *mtd, struct
> > >nand_chip *chip,
> > >-				   uint8_t *buf, int page)
> > >+				   uint8_t *buf, int oob_required, int  
> page)
> > > {
> > > 	struct nand_chip *this = mtd->priv;
> > > 	int i, eccsize = chip->ecc.size;
> > >@@ -167,8 +167,9 @@ davinci_std_read_page_syndrome(struct mtd_info
> > >*mtd, struct nand_chip *chip,
> > > 	return 0;
> > > }
> >
> > We really should not be having NAND driver code (stuff that  
> interacts
> > with the NAND API; not hardware setup) outside of drivers/mtd/nand.
> This probably should be split.

Yes, it's not a problem with this patch, just something that this patch  
made me notice (hence the CC to Heiko).

> > >diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> > >index 543c845..99f39fc 100644
> > >--- a/drivers/mtd/Makefile
> > >+++ b/drivers/mtd/Makefile
> > >@@ -25,7 +25,9 @@ include $(TOPDIR)/config.mk
> > >
> > > LIB	:= $(obj)libmtd.o
> > >
> > >-COBJS-$(CONFIG_MTD_DEVICE) += mtdcore.o
> > >+ifneq (,$(findstring
> > >y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)))
> > >+COBJS-y += mtdcore.o
> > >+endif
> >
> > Please just require users of CONFIG_CMD_NAND or CONFIG_CMD_ONENAND  
> to
> > also select CONFIG_MTD_DEVICE, if it's not going to be practical to  
> do
> > without it -- and remove the "#ifdef CONFIG_MTD_DEVICE" from nand.c.
> >
> > Could you explain why it's no longer practical to have NAND by  
> itself?
> 
> I have dilemma here:
> 
> New MTD API instead of calling mtd->foo(mtd...) uses mtd_foo(mtd...)
> functions. In Linux, these are defined in mtdcore.c
> 
> We can either move these functions from mtdcore.c somewhere (where?),  
> or
> #ifndef unrelated parts from mtdcore.c

I'd say move them to another file in drivers/mtd.  mtdapi.c?

> > >+#define pr_info(args...)	MTDDEBUG(MTD_DEBUG_LEVEL0, args)
> > >+#define pr_warn(args...)	MTDDEBUG(MTD_DEBUG_LEVEL0, args)
> > >+#define pr_err(args...)		MTDDEBUG(MTD_DEBUG_LEVEL0, args)
> >
> > These should be ordinary printf, not MTDDEBUG.  Under normal
> > (non-debug) circumstances MTDDEBUG is a no-op.  We want to see
> > errors and
> > warnings always.
> >
> > Plus, these should be defined somewhere that isn't MTD-specific.
> So, should I add separate header?

Put them in include/linux/compat.h

> So, what is next step?

Once ecc.strength and the other review issues are dealt with, it should  
be good to go in.

-Scott