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
next prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.