public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] mtd: resync with Linux-3.7.1
Date: Thu, 17 Jan 2013 19:02:04 -0600	[thread overview]
Message-ID: <1358470924.13978.25@snotra> (raw)
In-Reply-To: <20130117120739.GA18338@build.ihdev.net> (from slapin@ossfans.org on Thu Jan 17 06:07:39 2013)

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@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

  reply	other threads:[~2013-01-18  1:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-14 13:46 [U-Boot] [PATCH v2] mtd: resync with Linux-3.7.1 Sergey Lapin
2013-01-16 22:09 ` Scott Wood
2013-01-17 12:07   ` Sergey Lapin
2013-01-18  1:02     ` Scott Wood [this message]
2013-01-18  9:45       ` Sergey Lapin
2013-01-18 10:53         ` Josh Wu
2013-01-18 11:48           ` Sergey Lapin
2013-01-18 12:00         ` Sergey Lapin
2013-01-18 22:33           ` Scott Wood
2013-01-17 14:42   ` Sergey Lapin
2013-05-30 20:55 ` [U-Boot] [U-Boot,v2] " Scott Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1358470924.13978.25@snotra \
    --to=scottwood@freescale.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox