[linux-sunxi] Re: [PATCH 04/12] mtd: nand: brcm: rely on generic DT parsing done in nand_scan_ident()

[Brian Norris]

On Fri, Apr 01, 2016 at 02:54:24PM +0200, Boris Brezillon wrote:
> The core now takes care of parsing generic DT properties in
> nand_scan_ident() when nand_set_flash_node() has been called.
> Rely on this initialization instead of calling of_get_nand_xxx()
> manually.
> 
> Signed-off-by: Boris Brezillon <boris.brezil...@free-electrons.com>

Acked-by: Brian Norris <computersforpe...@gmail.com>

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 00/52] mtd: rework ECC layout definition

[Brian Norris]

On Mon, Mar 07, 2016 at 10:46:50AM +0100, Boris Brezillon wrote:
> Hello,
> 
> This patchset aims at getting rid of the nand_ecclayout limitations.
> struct nand_ecclayout is defining fixed eccpos and oobfree arrays which
> can only be increased by modifying the MTD_MAX_ECCPOS_ENTRIES_LARGE and
> MTD_MAX_OOBFREE_ENTRIES_LARGE macros.
> This approach forces us to modify the macro values each time we add a
> new NAND chip with a bigger OOB area, and increasing these arrays also
> penalize all platforms, even those who only support small NAND devices
> (with small OOB area).
> 
> The idea to overcome this limitation, is to define the ECC/OOB layout
> by the mean of two functions: ->ecc() and ->free(), which will
> basically return the same information has those stored in the
> nand_ecclayout struct.
> 
> Another advantage of this solution is that ECC layouts are usually
> following a repetitive pattern (i.e. leave X bytes free and put Y bytes
> of ECC per ECC chunk), which allows one to implement the ->ecc()
> and ->free() functions with a simple logic that can be applied
> to any size of OOB.
> 
> Patches 1 to 4 are just cleanups or trivial fixes that can be taken
> independently.
> 
> Also note that the last two commits are removing the nand_ecclayout
> definition, thus preventing any new driver to use this structure.
> Of course, this step can be delayed if some of the previous patches
> are not accepted.
> 
> All those changes are available here [1].
> 
> Best Regards,
> 
> Boris
> 
> [1]https://github.com/bbrezillon/linux-0day/tree/nand/ecclayout

FYI, I've pushed patches 1-4 to l2-mtd.git. I'll take another look at
them this week I hope (or your new fellow, Richard, can!), then you can
queue them up for the next cycle.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 16/52] mtd: nand: use mtd_set_ecclayout() where appropriate

[Brian Norris]

On Fri, Feb 26, 2016 at 01:57:24AM +0100, Boris Brezillon wrote:
> Use the mtd_set_ecclayout() helper instead of directly assigning the
> mtd->ecclayout field.
> 
> Signed-off-by: Boris Brezillon 
> ---
>  drivers/mtd/nand/nand_base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 17504f2..5093a3c 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4288,7 +4288,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>   ecc->write_oob_raw = ecc->write_oob;
>  
>   /* propagate ecc info to mtd_info */
> - mtd->ecclayout = ecc->layout;
> + mtd_set_ecclayout(mtd, ecc->layout);

I'm having trouble applying this one. For the life of me, I can't figure
out where you got this context from. This block only appears much later
in nand_scan_tail()...

Do you think you could post a git tree with your intended changes? I may
just try to pull something in like that instead.

BTW, I'm not sure the OMAP refactorings are going to come in time, but I
was planning to pull those directly from the TI folks (i.e., they won't
be rebased on l2-mtd.git), since there's some intermingling of platform
changes there. I think I can fix the conflicts fine, but FYI.

Brian

>   mtd->ecc_strength = ecc->strength;
>   mtd->ecc_step_size = ecc->size;
>  
> -- 
> 2.1.4
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] mtd: nand: sunxi: remove direct mtd->priv accesses

[Brian Norris]

On Sat, Mar 05, 2016 at 12:21:20AM +0100, Boris Brezillon wrote:
> mtd->priv is no longer pointing to the struct nand_chip it is attached to.
> Replace those accesses by mtd_to_nand() calls.
> 
> Signed-off-by: Boris Brezillon 
> Fixes: 4be4e03efc7f ("mtd: nand: sunxi: add randomizer support")
> ---

Applied, thanks!

> Sorry for the inconvenience, but apparently I was not expecting
> the mtd_to_nand() rework to be taken before the randomizer patch,
> and I forgot to repost a version replacing mtd->priv by mtd_to_nand().

Eh, probably not entirely your fault. I can be erratic with my
review/acceptance times.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 00/23] mtd: rework ECC layout definition

[Brian Norris]

Hi Boris,

On Mon, Dec 07, 2015 at 11:25:55PM +0100, Boris Brezillon wrote:
> Hello,
> 
> This patchset aims at getting rid of the nand_ecclayout limitations.
> struct nand_ecclayout is defining fixed eccpos and oobfree arrays which
> can only be increased by modifying the MTD_MAX_ECCPOS_ENTRIES_LARGE and
> MTD_MAX_OOBFREE_ENTRIES_LARGE macros.
> This approach forces us to modify the macro values each time we add a
> new NAND chip with a bigger OOB area, and increasing these arrays also
> penalize all platforms, even those who only support small NAND devices
> (with small OOB area).
> 
> The idea to overcome this limitation, is to define the ECC/OOB layout
> by the mean of two functions: ->eccpos() and ->oobfree(), which will
> basically return the same information has those stored in the
> nand_ecclayout struct.
> 
> Another advantage of this solution is that ECC layouts are usually
> following a repetitive pattern (i.e. leave X bytes free and put Y bytes
> of ECC per ECC chunk), which allows one to implement the ->eccpos()
> and ->oobfree() functions with a simple logic that can be applied
> to any size of OOB.

Thanks for the work! This definitely needed done. I imagined that it
might be best if we just changed the data structure format and have
drivers allocate it dynamically during probe(), but actually, I kinda
like generating it on the fly. The only concern I'd have is if there is
significant penalty to doing this sort of computation on the fly during
(e.g.) AUTO-layout OOB reads/writes. But I guess if there is such a
penalty, nothing would stop us from caching the results in the MTD/NAND
core code.

> Patches 1 to 10 are just cleanups or trivial fixes that can be taken
> independently.

There were some comments for patch 1, and I want to look more closely at
patch 10. But patches 2 to 9 are pushed to l2-mtd.git. Thanks!

> Patch 19 is just an aggregate of several smaller commits (one per
> driver), and has been submitted this way to limit the size of the
> series. If everybody agrees on this approach, I'll resubmit the series
> will those changes separated in different commits (as done here [1]).
> 
> Also note that the last two commits are removing the nand_ecclayout
> definition, thus preventing any new driver to use this structure.
> Of course, this step can be delayed if some of the previous patches
> are not accepted.

I haven't looked in detail at the second half of the series, but I like
the concept. I'll look closer once you fix up things in v2.

Thanks,
Brian

> Best Regards,
> 
> Boris
> 
> [1]https://github.com/bbrezillon/linux-sunxi/commits/nand/ecclayout2

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 3/3] mtd: nand: sunxi: add randomizer support

[Brian Norris]

On Sat, Jan 23, 2016 at 09:18:44AM +0100, Boris Brezillon wrote:
> On Fri, 22 Jan 2016 18:57:13 -0800
> Brian Norris <computersforpe...@gmail.com> wrote:
> > 
> > From: Brian Norris <computersforpe...@gmail.com>
> > Date: Fri, 22 Jan 2016 18:54:02 -0800
> > Subject: [PATCH] mtd: nand: sunxi: use mtd_div_by_ws() helper
> > 
> > Suggested-by: Richard Weinberger <rich...@nod.at>
> > Signed-off-by: Brian Norris <computersforpe...@gmail.com>
> 
> Acked-by: Boris Brezillon <boris.brezil...@free-electrons.com>

Applied.

> > ---
> >  drivers/mtd/nand/sunxi_nand.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> > index 5f700719d5c2..b5ea6b312df0 100644
> > --- a/drivers/mtd/nand/sunxi_nand.c
> > +++ b/drivers/mtd/nand/sunxi_nand.c
> > @@ -624,7 +624,7 @@ static u16 sunxi_nfc_randomizer_step(u16 state, int 
> > count)
> >  static u16 sunxi_nfc_randomizer_state(struct mtd_info *mtd, int page, bool 
> > ecc)
> >  {
> > const u16 *seeds = sunxi_nfc_randomizer_page_seeds;
> > -   int mod = mtd->erasesize / mtd->writesize;
> > +   int mod = mtd_div_by_ws(mtd->erasesize, mtd);
> 
> Just a comment (which should not prevent you from applying this patch).
> Isn't it a bit overkill to cast the erasesize to a 64 bit value, and
> then do a do_div on it. Shouldn't happen often though, because
> ->writesize_shift should be != 0 in most (all?) cases.

drivers/mtd/nand/ assumes power-of-2 dimensions for many things, so
probably.

> Another related remark: with the MLC/paired pages stuff I'll have to
> retrieve this information (number of write units per erase block) quite
> often, so maybe we should have a field (and/or an helper) for that.

Perhaps.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 3/3] mtd: nand: sunxi: add randomizer support

[Brian Norris]

All three look good, so pushed to l2-mtd.git/next. One comment below:

On Wed, Dec 02, 2015 at 12:01:07PM +0100, Boris Brezillon wrote:

...

> +static u16 sunxi_nfc_randomizer_state(struct mtd_info *mtd, int page, bool 
> ecc)
> +{
> + const u16 *seeds = sunxi_nfc_randomizer_page_seeds;
> + int mod = mtd->erasesize / mtd->writesize;

Richard suggested you use the mtd.h helper here. Patch below.

> +
> + if (mod > ARRAY_SIZE(sunxi_nfc_randomizer_page_seeds))
> + mod = ARRAY_SIZE(sunxi_nfc_randomizer_page_seeds);
> +
> + if (ecc) {
> + if (mtd->ecc_step_size == 512)
> + seeds = sunxi_nfc_randomizer_ecc512_seeds;
> + else
> + seeds = sunxi_nfc_randomizer_ecc1024_seeds;
> + }
> +
> + return seeds[page % mod];
> +}

From: Brian Norris <computersforpe...@gmail.com>
Date: Fri, 22 Jan 2016 18:54:02 -0800
Subject: [PATCH] mtd: nand: sunxi: use mtd_div_by_ws() helper

Suggested-by: Richard Weinberger <rich...@nod.at>
Signed-off-by: Brian Norris <computersforpe...@gmail.com>
---
 drivers/mtd/nand/sunxi_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 5f700719d5c2..b5ea6b312df0 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -624,7 +624,7 @@ static u16 sunxi_nfc_randomizer_step(u16 state, int count)
 static u16 sunxi_nfc_randomizer_state(struct mtd_info *mtd, int page, bool ecc)
 {
const u16 *seeds = sunxi_nfc_randomizer_page_seeds;
-   int mod = mtd->erasesize / mtd->writesize;
+   int mod = mtd_div_by_ws(mtd->erasesize, mtd);
 
if (mod > ARRAY_SIZE(sunxi_nfc_randomizer_page_seeds))
mod = ARRAY_SIZE(sunxi_nfc_randomizer_page_seeds);
-- 
2.7.0.rc3.207.g0ac5344

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 00/58] mtd: nand: refactor the NAND subsystem (part 1)

[Brian Norris]

Hi,

On Thu, Dec 10, 2015 at 08:59:44AM +0100, Boris Brezillon wrote:
> Hello,
> 
> This huge series aims at clarifying the relationship between the mtd and
> nand_chip structures and hiding NAND framework internals to NAND
> controller drivers.
> 
> The first part of the series (patch 1 to 4) is a set of fixes/simple
> reworks easing the migration to mtd_to_nand().
> 
> The second part of the series embeds the mtd structure into the nand_chip
> one so that NAND controller drivers don't have to bother allocating the
> MTD device and linking it with the NAND chip.
> 
> The last part of the series hides accesses to the chip->priv field behind
> two helper functions.
> 
> This allows removal of some of the boilerplate code done in all NAND
> controller drivers, but most importantly, it unifies a bit the way NAND
> chip structures are instantiated (even though we still have two different
> kinds of drivers: those embedding the nand_chip struct into their private
> nand chip representation, and those allocating two different structures
> and linking them together with the chip->priv field).
> 
> As said in the title, this refactoring is only the first step. I plan to
> rework the NAND controller / NAND chip separation for pretty much the same
> reasons: clarifying the separation between the two concepts, and getting
> rid of more boilerplate code in NAND controller drivers.
> 
> Stay tuned ;-).
> 
> Best Regards,
> 
> Boris

Thanks a lot for all the work here! Nice cleanups. I think I've pushed
everything correctly to l2-mtd.git. Please verify that things look sane
to you, if possible. There were a few v5's thrown in after the fact, as
well as other cleanups that required apply-conflicts.

I'll summarize below anything I remember that's changed since the cover
letter, for posterity.

> Changes since v3:
> - fix some bugs introduced when migrating to nand_to_mtd()
> - split the huge commit switching all drivers to nand_to_mtd() into several
>   commits (one per driver) to ease review and integration
> - add a simple fixes/reworks at the beginning of the series (mainly to
>   ease migration to nand_to_mtd())
> - drop already applied patches.
> 
> Changes since v2:
> - fix some build warnings/erros
> 
> Changes since v1:
> - dropped already applied patches
> - fixed some typos
> - manually fixed some modifications omitted by the coccinelle scripts
> - manually reworked modifactions done by coccinelle scripts to improve
>   readability and fix coding style issues
> 
> *** BLURB HERE ***

Nice blurb :)

> 
> Boris Brezillon (58):
>   mtd: nand: denali: add missing nand_release() call in denali_remove()

You sent v5 of this, and I added in the comment that was
written/requested afterward. That caused some small conflicts later.

>   mtd: nand: fsmc: create and use mtd_to_fsmc()
>   mtd: nand: nuc900: create and use mtd_to_nuc900()
>   mtd: nand: omap2: create and use mtd_to_omap()
>   mtd: nand: ams-delta: use the mtd instance embedded in struct
> nand_chip
>   mtd: nand: atmel: use the mtd instance embedded in struct nand_chip
>   mtd: nand: au1550nd: use the mtd instance embedded in struct nand_chip
>   mtd: nand: bcm47xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: bf5xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: brcm: use the mtd instance embedded in struct nand_chip
>   mtd: nand: cafe: use the mtd instance embedded in struct nand_chip
>   mtd: nand: cmx270: use the mtd instance embedded in struct nand_chip

^^ I dropped one comment in this one

>   mtd: nand: cs553x: use the mtd instance embedded in struct nand_chip
>   mtd: nand: davinci: use the mtd instance embedded in struct nand_chip
>   mtd: nand: denali: use the mtd instance embedded in struct nand_chip

You sent v5, but there were still trivial conflicts.

>   mtd: nand: diskonchip: use the mtd instance embedded in struct
> nand_chip
>   mtd: nand: docg4: use the mtd instance embedded in struct nand_chip

I cleaned some things up after this one, causing more trivial conflicts
later in the controller_data accessor patches.

>   mtd: nand: fsl_elbc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: fsl_ifc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: fsl_upm: use the mtd instance embedded in struct nand_chip
>   mtd: nand: fsmc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: gpio: use the mtd instance embedded in struct nand_chip
>   mtd: nand: gpmi: use the mtd instance embedded in struct nand_chip
>   mtd: nand: hisi504: use the mtd instance embedded in struct nand_chip
>   mtd: nand: jz4740: use the mtd instance embedded in struct nand_chip
>   mtd: nand: lpc32xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: mpc5121: use the mtd instance embedded in struct nand_chip
>   mtd: nand: mxc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: nandsim: use the mtd instance embedded in struct nand_chip
>   mtd: nand: ndfc: 

[linux-sunxi] Re: [PATCH v4 55/58] mtd: nand: add helpers to access ->priv

[Brian Norris]

Hi Boris,

On Thu, Dec 10, 2015 at 09:00:39AM +0100, Boris Brezillon wrote:
> Add two helpers to access the field reserved for private controller data.
> This makes it clearer what this field is reserved for and ease future
> refactoring.

I agree with the refactoring part, but I'm not sure about the name. Is
it really "controller" data? That sounds like something that has 1
instance per controller. But the way this is sometimes used, we get 1
instance per NAND chip, and there may be more than one NAND chip per
controller.

So at the moment, this is more like opaque "driver data", like
dev_{get,set}_drvdata(), which doesn't really have a prescribed use --
it's up to the driver.

Notably, we already have a (sort of) 1-per-controler-instance field:
struct nand_hw_control (I notice we have both the 'controller' and
'hwcontrol' fields in nand_chip; that's pretty ugly too...). Those don't
have private data fields, but we could of course extend that if we
really want "controller" data.

Anyway, I don't feel like this question is resolved well enough to say
that we should go change all drivers to use these accessors. I know you
have bigger plans for putting more "controller" infrastructure into the
core drivers/mtd/nand/ code, so I'd like to see how that fits in here.

(If we're going to discuss this much more, I'd suggest a smaller CC
list. I'm mostly putting this here to show why I'm not taking the last
4 patches right now.)

Regards,
Brian

> Signed-off-by: Boris Brezillon 
> ---
>  include/linux/mtd/nand.h | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 2bee2e4..4aed4b2 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -739,6 +739,16 @@ static inline struct mtd_info *nand_to_mtd(struct 
> nand_chip *chip)
>   return >mtd;
>  }
>  
> +static inline void *nand_get_controller_data(struct nand_chip *chip)
> +{
> + return chip->priv;
> +}
> +
> +static inline void nand_set_controller_data(struct nand_chip *chip, void 
> *priv)
> +{
> + chip->priv = priv;
> +}
> +
>  /*
>   * NAND Flash Manufacturer ID Codes
>   */
> -- 
> 2.1.4
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 01/58] mtd: nand: denali: add missing nand_release() call in denali_remove()

[Brian Norris]

Hi Boris,

On Fri, Dec 11, 2015 at 11:03:05PM +0100, Boris Brezillon wrote:
> On Thu, 10 Dec 2015 16:40:08 -0800
> Brian Norris <computersforpe...@gmail.com> wrote:
> > On Thu, Dec 10, 2015 at 08:59:45AM +0100, Boris Brezillon wrote:
> > > Unregister the NAND device from the NAND subsystem when removing a denali
> > > NAND controller, otherwise the MTD attached to the NAND device is still
> > > exposed by the MTD layer, and accesses to this device will likely crash
> > > the system.
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezil...@free-electrons.com>
> > > Cc: <sta...@vger.kernel.org> #3.8+
> > 
> > Does this follow these rules, from
> > Documentation/stable_kernel_rules.txt?
> > 
> >  - It must be obviously correct and tested.
> > 
> >  - It must fix a real bug that bothers people (not a, "This could be a
> >problem..." type thing).
> 
> Sorry to bring the "stable or not stable (that is the question :-))"
> debate back, but after thinking a bit more about the implications of
> this missing nand_release() call, I think it is worth backporting the
> fix to all stable kernels.
> The reason is, it can potentially introduce a security hole, because if
> the mtd device is not unregister but the underlying mtd object is freed
> and the kernel reuses the same memory region for a different object,
> the MTD layer will possibly call one of the mtd->_method() function,
> and this field might point to another completely different function.
> 
> You'll say that denali devices are probably never removed and this is
> the reason why people have never seen this problem before, which would
> be a good reason to not bother backporting the patch.
> But, given that the driver can be compiled as a module (the user can
> possibly load/unload it, which will in turn create/destroy the
> NAND/MTD device), and that the denali controller can be exposed through
> a PCI bus (which, AFAIK is hotpluggable), I really think this fix
> should be sent to stable.

That's all well and good, but still nobody has told me they've tested
this.

I've pushed your v5 (+ comments, + ack) to l2-mtd.git. If it gets
testing and this request is made again at that point, we can easily send
it to stable after it hits Linus' tree. See option 2 in
Documentation/stable_kernel_rules.txt. You can even send the email
yourself, just CC me and anyone else relevant. I'll ack it if it's been
tested.

Regards,
Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 01/58] mtd: nand: denali: add missing nand_release() call in denali_remove()

[Brian Norris]

On Thu, Dec 10, 2015 at 08:59:45AM +0100, Boris Brezillon wrote:
> Unregister the NAND device from the NAND subsystem when removing a denali
> NAND controller, otherwise the MTD attached to the NAND device is still
> exposed by the MTD layer, and accesses to this device will likely crash
> the system.
> 
> Signed-off-by: Boris Brezillon 
> Cc:  #3.8+

Does this follow these rules, from
Documentation/stable_kernel_rules.txt?

 - It must be obviously correct and tested.

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing).

> Fixes: 2a0a288ec258 ("mtd: denali: split the generic driver and PCI layer")
> ---
>  drivers/mtd/nand/denali.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 67eb2be..8feece3 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1622,6 +1622,7 @@ EXPORT_SYMBOL(denali_init);
>  /* driver exit point */
>  void denali_remove(struct denali_nand_info *denali)
>  {
> + nand_release(>mtd);
>   denali_irq_cleanup(denali->irq, denali);
>   dma_unmap_single(denali->dev, denali->buf.dma_buf,
>denali->mtd.writesize + denali->mtd.oobsize,

It feels a bit odd to allow usage of MTD fields after it has been
unregistered. Maybe precompute this before the nand_release()?

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 bis 12/25] mtd: nand: use the mtd instance embedded in struct nand_chip

[Brian Norris]

Hi Boris,

On Wed, Dec 02, 2015 at 09:50:01AM +0100, Boris Brezillon wrote:
> struct nand_chip now embeds an mtd device. Patch all drivers to make use
> of this mtd instance instead of using the instance embedded in their
> private struct or dynamically allocated.
> 
> Signed-off-by: Boris Brezillon 
> Cc: Julia Lawall 
> ---
> Most of those changes were generated with the coccinelle script added
> in commit c671312 "coccinelle: nand: detect and correct drivers embedding
> an mtd_info object"
> ---
> Changes since v2:
> - fix several compilation errors/warnings
> 
>  drivers/mtd/nand/ams-delta.c   | 13 ++--
>  drivers/mtd/nand/atmel_nand.c  | 13 ++--
>  drivers/mtd/nand/au1550nd.c| 19 ++---
>  drivers/mtd/nand/bcm47xxnflash/bcm47xxnflash.h |  1 -
>  drivers/mtd/nand/bcm47xxnflash/main.c  |  8 ++-
>  drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c   |  2 +-
>  drivers/mtd/nand/bf5xx_nand.c  | 12 ++--
>  drivers/mtd/nand/brcmnand/brcmnand.c   | 13 ++--
>  drivers/mtd/nand/cafe_nand.c   |  8 +--
>  drivers/mtd/nand/cmx270_nand.c | 11 ++-

There's another error here, I think ^^^

>  drivers/mtd/nand/cs553x_nand.c | 13 ++--
>  drivers/mtd/nand/davinci_nand.c| 30 
>  drivers/mtd/nand/denali.c  | 68 ++
>  drivers/mtd/nand/denali.h  |  1 -
>  drivers/mtd/nand/diskonchip.c  | 11 ++-
>  drivers/mtd/nand/docg4.c   | 23 +++---
>  drivers/mtd/nand/fsl_elbc_nand.c   | 26 ---
>  drivers/mtd/nand/fsl_ifc_nand.c| 28 
>  drivers/mtd/nand/fsl_upm.c | 28 
>  drivers/mtd/nand/fsmc_nand.c   | 56 ---
>  drivers/mtd/nand/gpio.c| 20 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  2 +-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 23 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  1 -
>  drivers/mtd/nand/hisi504_nand.c| 13 ++--
>  drivers/mtd/nand/jz4740_nand.c |  9 ++-
>  drivers/mtd/nand/lpc32xx_mlc.c |  7 +-
>  drivers/mtd/nand/lpc32xx_slc.c |  7 +-
>  drivers/mtd/nand/mpc5121_nfc.c |  3 +-
>  drivers/mtd/nand/mxc_nand.c|  5 +-
>  drivers/mtd/nand/nandsim.c | 12 ++--
>  drivers/mtd/nand/ndfc.c| 24 ---
>  drivers/mtd/nand/nuc900_nand.c | 24 +++
>  drivers/mtd/nand/omap2.c   | 98 
> +++---
>  drivers/mtd/nand/orion_nand.c  |  4 +-
>  drivers/mtd/nand/pasemi_nand.c | 12 ++--
>  drivers/mtd/nand/plat_nand.c   | 15 ++--
>  drivers/mtd/nand/pxa3xx_nand.c | 33 -
>  drivers/mtd/nand/r852.c| 34 -
>  drivers/mtd/nand/r852.h|  1 -
>  drivers/mtd/nand/s3c2410.c | 23 +++---
>  drivers/mtd/nand/sh_flctl.c|  8 +--
>  drivers/mtd/nand/sharpsl.c | 22 +++---
>  drivers/mtd/nand/socrates_nand.c   |  5 +-
>  drivers/mtd/nand/sunxi_nand.c  | 13 ++--
>  drivers/mtd/nand/tmio_nand.c   | 10 +--
>  drivers/mtd/nand/txx9ndfmc.c   |  3 +-
>  drivers/mtd/nand/vf610_nfc.c   |  8 ++-
>  include/linux/mtd/sh_flctl.h   |  3 +-
>  49 files changed, 432 insertions(+), 394 deletions(-)
> 

...

> diff --git a/drivers/mtd/nand/cmx270_nand.c b/drivers/mtd/nand/cmx270_nand.c
> index 43bded6..84d027e 100644
> --- a/drivers/mtd/nand/cmx270_nand.c
> +++ b/drivers/mtd/nand/cmx270_nand.c
> @@ -160,10 +160,8 @@ static int __init cmx270_init(void)
>   gpio_direction_input(GPIO_NAND_RB);
>  
>   /* Allocate memory for MTD device structure and private data */
> - cmx270_nand_mtd = kzalloc(sizeof(struct mtd_info) +
> -   sizeof(struct nand_chip),
> -   GFP_KERNEL);
> - if (!cmx270_nand_mtd) {
> + this = kzalloc(sizeof(struct nand_chip), GFP_KERNEL);
> + if (!this) {
>   ret = -ENOMEM;
>   goto err_kzalloc;
>   }
> @@ -175,8 +173,7 @@ static int __init cmx270_init(void)
>   goto err_ioremap;
>   }
>  
> - /* Get pointer to private data */
> - this = (struct nand_chip *)(_nand_mtd[1]);
> + cmx270_nand_mtd = nand_to_mtd(this);

So, you make cmx270_nand_mtd no longer kzalloc()'d, but I still see the
cmx270_init() function end with:

err_scan:
iounmap(cmx270_nand_io);
err_ioremap:
kfree(cmx270_nand_mtd);  <- *** this! ***
err_kzalloc:
gpio_free(GPIO_NAND_RB);
err_gpio_request:

[linux-sunxi] Re: [PATCH v2 12/25] mtd: nand: use the mtd instance embedded in struct nand_chip

[Brian Norris]

Hi Boris,

On Tue, Dec 01, 2015 at 12:03:09PM +0100, Boris Brezillon wrote:
> struct nand_chip now embeds an mtd device. Patch all drivers to make use
> of this mtd instance instead of using the instance embedded in their
> private struct or dynamically allocated.
> 
> Signed-off-by: Boris Brezillon 
> Cc: Julia Lawall 
> ---
> Most of those changes were generated with the coccinelle script added
> in commit c671312 "coccinelle: nand: detect and correct drivers embedding
> an mtd_info object"
> ---
>  drivers/mtd/nand/ams-delta.c   | 13 ++--
>  drivers/mtd/nand/atmel_nand.c  | 13 ++--
>  drivers/mtd/nand/au1550nd.c| 19 ++---
>  drivers/mtd/nand/bcm47xxnflash/bcm47xxnflash.h |  1 -
>  drivers/mtd/nand/bcm47xxnflash/main.c  |  8 ++-
>  drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c   |  2 +-
>  drivers/mtd/nand/bf5xx_nand.c  | 12 ++--
>  drivers/mtd/nand/brcmnand/brcmnand.c   | 13 ++--
>  drivers/mtd/nand/cafe_nand.c   |  8 +--
>  drivers/mtd/nand/cmx270_nand.c | 11 ++-
>  drivers/mtd/nand/cs553x_nand.c | 13 ++--
>  drivers/mtd/nand/davinci_nand.c| 30 
>  drivers/mtd/nand/denali.c  | 68 ++
>  drivers/mtd/nand/denali.h  |  1 -
>  drivers/mtd/nand/diskonchip.c  | 11 ++-
>  drivers/mtd/nand/docg4.c   | 16 ++---
>  drivers/mtd/nand/fsl_elbc_nand.c   | 26 ---
>  drivers/mtd/nand/fsl_ifc_nand.c| 28 
>  drivers/mtd/nand/fsl_upm.c | 28 
>  drivers/mtd/nand/fsmc_nand.c   | 56 ---
>  drivers/mtd/nand/gpio.c| 20 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  2 +-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 20 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  1 -
>  drivers/mtd/nand/hisi504_nand.c| 13 ++--
>  drivers/mtd/nand/jz4740_nand.c |  9 ++-
>  drivers/mtd/nand/lpc32xx_mlc.c |  7 +-
>  drivers/mtd/nand/lpc32xx_slc.c |  7 +-
>  drivers/mtd/nand/mpc5121_nfc.c |  3 +-
>  drivers/mtd/nand/mxc_nand.c|  5 +-
>  drivers/mtd/nand/nandsim.c | 12 ++--
>  drivers/mtd/nand/ndfc.c| 24 ---
>  drivers/mtd/nand/nuc900_nand.c | 24 +++
>  drivers/mtd/nand/omap2.c   | 98 
> +++---
>  drivers/mtd/nand/orion_nand.c  |  4 +-
>  drivers/mtd/nand/pasemi_nand.c | 12 ++--
>  drivers/mtd/nand/plat_nand.c   | 15 ++--
>  drivers/mtd/nand/pxa3xx_nand.c | 33 -
>  drivers/mtd/nand/r852.c| 34 -
>  drivers/mtd/nand/r852.h|  1 -
>  drivers/mtd/nand/s3c2410.c | 23 +++---

^^ some errors in this one

>  drivers/mtd/nand/sh_flctl.c|  8 +--
>  drivers/mtd/nand/sharpsl.c | 22 +++---
>  drivers/mtd/nand/socrates_nand.c   |  5 +-
>  drivers/mtd/nand/sunxi_nand.c  | 13 ++--
>  drivers/mtd/nand/tmio_nand.c   | 10 +--
>  drivers/mtd/nand/txx9ndfmc.c   |  3 +-
>  drivers/mtd/nand/vf610_nfc.c   |  8 ++-
>  include/linux/mtd/sh_flctl.h   |  3 +-
>  49 files changed, 426 insertions(+), 390 deletions(-)
> 

[...]

> diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> index e658b29..3f29734 100644
> --- a/drivers/mtd/nand/s3c2410.c
> +++ b/drivers/mtd/nand/s3c2410.c
> @@ -104,7 +104,6 @@ struct s3c2410_nand_info;
>   * @scan_res: The result from calling nand_scan_ident().
>  */
>  struct s3c2410_nand_mtd {
> - struct mtd_info mtd;
>   struct nand_chipchip;
>   struct s3c2410_nand_set *set;
>   struct s3c2410_nand_info*info;
> @@ -168,7 +167,8 @@ struct s3c2410_nand_info {
>  
>  static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd)
>  {
> - return container_of(mtd, struct s3c2410_nand_mtd, mtd);
> + return container_of(mtd_to_nand(mtd), struct s3c2410_nand_mtd,
> + chip);
>  }
>  
>  static struct s3c2410_nand_info *s3c2410_nand_mtd_toinfo(struct mtd_info 
> *mtd)
> @@ -745,7 +745,7 @@ static int s3c24xx_nand_remove(struct platform_device 
> *pdev)
>  
>   for (mtdno = 0; mtdno < info->mtd_count; mtdno++, ptr++) {
>   pr_debug("releasing mtd %d (%p)\n", mtdno, ptr);
> - nand_release(>mtd);
> + nand_release(nand_to_mtd(>chip));
>   }
>   }
>  
> @@ -762,9 +762,11 @@ static int s3c2410_nand_add_partition(struct 
> 

[linux-sunxi] Re: [PATCH v2 17/25] mtd: nand: remove useless mtd->priv = chip assignments

[Brian Norris]

On Tue, Dec 01, 2015 at 12:03:14PM +0100, Boris Brezillon wrote:
> mtd_to_nand() now uses the container_of() approach to transform an
> mtd_info pointer into a nand_chip one. Drop useless mtd->priv
> assignments from NAND controller drivers.
> 
> Signed-off-by: Boris Brezillon 
> ---
> Patch generated with the following coccinelle script:
> 
> ---8<
> virtual patch
> 
> @@
> struct mtd_info m;
> struct mtd_info *mp;
> struct nand_chip *c;
> @@
> (
> -(m).priv = c;
> |
> -(mp)->priv = c;
> |
> -(mp)->priv = (void *)c;
> )
> ---8<
> ---

...

> diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> index 3f29734..ed4184c 100644
> --- a/drivers/mtd/nand/s3c2410.c
> +++ b/drivers/mtd/nand/s3c2410.c
> @@ -834,7 +834,6 @@ static void s3c2410_nand_init_chip(struct 
> s3c2410_nand_info *info,
>   chip->IO_ADDR_R = chip->IO_ADDR_W;
>  
>   nmtd->info = info;
> - mtd->priv  = chip;

After this one, we have:

drivers/mtd/nand/s3c2410.c: In function ‘s3c2410_nand_init_chip’:
drivers/mtd/nand/s3c2410.c:791:19: warning: unused variable ‘mtd’ 
[-Wunused-variable]


>   nmtd->set  = set;
>  
>  #ifdef CONFIG_MTD_NAND_S3C2410_HWECC
 
Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 12/25] mtd: nand: use the mtd instance embedded in struct nand_chip

[Brian Norris]

On Tue, Dec 01, 2015 at 12:03:09PM +0100, Boris Brezillon wrote:
> struct nand_chip now embeds an mtd device. Patch all drivers to make use
> of this mtd instance instead of using the instance embedded in their
> private struct or dynamically allocated.
> 
> Signed-off-by: Boris Brezillon 
> Cc: Julia Lawall 
> ---
> Most of those changes were generated with the coccinelle script added
> in commit c671312 "coccinelle: nand: detect and correct drivers embedding
> an mtd_info object"
> ---
>  drivers/mtd/nand/ams-delta.c   | 13 ++--
>  drivers/mtd/nand/atmel_nand.c  | 13 ++--
>  drivers/mtd/nand/au1550nd.c| 19 ++---
>  drivers/mtd/nand/bcm47xxnflash/bcm47xxnflash.h |  1 -
>  drivers/mtd/nand/bcm47xxnflash/main.c  |  8 ++-
>  drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c   |  2 +-
>  drivers/mtd/nand/bf5xx_nand.c  | 12 ++--
>  drivers/mtd/nand/brcmnand/brcmnand.c   | 13 ++--
>  drivers/mtd/nand/cafe_nand.c   |  8 +--
>  drivers/mtd/nand/cmx270_nand.c | 11 ++-
>  drivers/mtd/nand/cs553x_nand.c | 13 ++--
>  drivers/mtd/nand/davinci_nand.c| 30 
>  drivers/mtd/nand/denali.c  | 68 ++
>  drivers/mtd/nand/denali.h  |  1 -
>  drivers/mtd/nand/diskonchip.c  | 11 ++-
>  drivers/mtd/nand/docg4.c   | 16 ++---

^^ some errors here too

>  drivers/mtd/nand/fsl_elbc_nand.c   | 26 ---
>  drivers/mtd/nand/fsl_ifc_nand.c| 28 
>  drivers/mtd/nand/fsl_upm.c | 28 
>  drivers/mtd/nand/fsmc_nand.c   | 56 ---
>  drivers/mtd/nand/gpio.c| 20 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  2 +-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 20 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  1 -
>  drivers/mtd/nand/hisi504_nand.c| 13 ++--
>  drivers/mtd/nand/jz4740_nand.c |  9 ++-
>  drivers/mtd/nand/lpc32xx_mlc.c |  7 +-
>  drivers/mtd/nand/lpc32xx_slc.c |  7 +-
>  drivers/mtd/nand/mpc5121_nfc.c |  3 +-
>  drivers/mtd/nand/mxc_nand.c|  5 +-
>  drivers/mtd/nand/nandsim.c | 12 ++--
>  drivers/mtd/nand/ndfc.c| 24 ---
>  drivers/mtd/nand/nuc900_nand.c | 24 +++
>  drivers/mtd/nand/omap2.c   | 98 
> +++---
>  drivers/mtd/nand/orion_nand.c  |  4 +-
>  drivers/mtd/nand/pasemi_nand.c | 12 ++--
>  drivers/mtd/nand/plat_nand.c   | 15 ++--
>  drivers/mtd/nand/pxa3xx_nand.c | 33 -
>  drivers/mtd/nand/r852.c| 34 -
>  drivers/mtd/nand/r852.h|  1 -
>  drivers/mtd/nand/s3c2410.c | 23 +++---
>  drivers/mtd/nand/sh_flctl.c|  8 +--
>  drivers/mtd/nand/sharpsl.c | 22 +++---
>  drivers/mtd/nand/socrates_nand.c   |  5 +-
>  drivers/mtd/nand/sunxi_nand.c  | 13 ++--
>  drivers/mtd/nand/tmio_nand.c   | 10 +--
>  drivers/mtd/nand/txx9ndfmc.c   |  3 +-
>  drivers/mtd/nand/vf610_nfc.c   |  8 ++-
>  include/linux/mtd/sh_flctl.h   |  3 +-
>  49 files changed, 426 insertions(+), 390 deletions(-)
> 

...

> diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
> index da93d7f..f8d5e27 100644
> --- a/drivers/mtd/nand/docg4.c
> +++ b/drivers/mtd/nand/docg4.c
> @@ -1305,14 +1305,13 @@ static int __init probe_docg4(struct platform_device 
> *pdev)
>   return -EIO;
>   }
>  
> - len = sizeof(struct mtd_info) + sizeof(struct nand_chip) +
> - sizeof(struct docg4_priv);
> - mtd = kzalloc(len, GFP_KERNEL);
> - if (mtd == NULL) {
> + len = sizeof(struct nand_chip) + sizeof(struct docg4_priv);
> + nand = kzalloc(len, GFP_KERNEL);
> + if (nand == NULL) {
>   retval = -ENOMEM;
>   goto fail;
>   }
> - nand = (struct nand_chip *) (mtd + 1);
> + mtd = nand_to_mtd(nand);
>   doc = (struct docg4_priv *) (nand + 1);
>   mtd->priv = nand;
>   nand->priv = doc;
> @@ -1355,13 +1354,12 @@ static int __init probe_docg4(struct platform_device 
> *pdev)
>  
>   fail:
>   iounmap(virtadr);
> - if (mtd) {
> + if (nand) {
>   /* re-declarations avoid compiler warning */
> - struct nand_chip *nand = mtd_to_nand(mtd);
>   struct docg4_priv *doc = nand->priv;
>   nand_release(mtd); /* deletes partitions and mtd devices */

drivers/mtd/nand/docg4.c: In function ‘probe_docg4’:

[linux-sunxi] Re: [PATCH 02/27] mtd: nand: add an mtd_to_nand() helper

[Brian Norris]

On Mon, Nov 16, 2015 at 02:37:35PM +0100, Boris Brezillon wrote:
> Some drivers are retrieving the nand_chip pointer using the container_of
> macro on a struct wrapping both the nand_chip and the mtd_info struct while
> the standard way of retrieving this pointer is through mtd->priv.
> Provide an helper to do that.
> 
> Signed-off-by: Boris Brezillon 
> ---
>  include/linux/mtd/nand.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 4f7c9b9..056d165 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -730,6 +730,11 @@ static inline struct device_node 
> *nand_get_flash_node(struct nand_chip *chip)
>   return chip->flash_node;
>  }
>  
> +static inline struct nand_chip *mtd_to_nand(struct mtd_info *mtd)
> +{
> + return mtd->priv;
> +}
> +

Pushed this patch to l2-mtd.git, though I rebased it directly onto
4.4-rc1 and then merged it in. That way, if any other subsystems want to
start using this in the 4.5 -next cycle (e.g., for patches 4-8), they
can pull it in without taking the rest of MTD.

I can push this to a separate branch or signed tag, if anyone wants.

Or, I can take the arch/ patches via MTD, if that's a better option.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 01/27] mtd: nand: fix drivers abusing mtd->priv

[Brian Norris]

On Mon, Nov 16, 2015 at 02:37:34PM +0100, Boris Brezillon wrote:
> The ->priv field of the mtd_info object attached to a nand_chip device
> should point to the nand_chip device. The pxa and cafe drivers are
> assigning this field their own private structure, which works fine as long
> as the nand_chip field is the first one in the driver private struct but
> seems a bit fragile.
> Fix that by setting mtd->priv to point the nand_chip field and assigning
> chip->priv to the private structure head.
> 
> Signed-off-by: Boris Brezillon 

Applied to l2-mtd.git

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 14/27] mtd: nand: use the mtd instance embedded in struct nand_chip

[Brian Norris]

Hi Boris,

On Mon, Nov 16, 2015 at 02:37:47PM +0100, Boris Brezillon wrote:
> struct nand_chip now embeds an mtd device. Patch all drivers to make use
> of this mtd instance instead of using the instance embedded in their
> private struct or dynamically allocated.
> 
> Signed-off-by: Boris Brezillon 
> Cc: Julia Lawall 
> ---
> Most of those changes were generate with this coccinelle script:
> http://code.bulix.org/5vxuih-89429

I appreciate that this patch is mostly autogenerated (a good thing for
preventing errors!), but there are some issues that I don't think play
out very well stylistically. Hopefully the cocci script can be improved
to handle some of this?

I'll try to point out a few snippets below.

Also, in case others are interested in reviewing your cocci script
directly, it might be better to paste it inline than to link to it.
Given the size of the patch, I don't think people would mind a few dozen
extra lines to show how it wsa generated. Or maybe stick some in the
cover letter too, if you end up reusing them in several patches.

> ---
>  drivers/mtd/nand/ams-delta.c   | 13 ++--
>  drivers/mtd/nand/atmel_nand.c  | 11 ++-
>  drivers/mtd/nand/au1550nd.c| 18 ++---
>  drivers/mtd/nand/bcm47xxnflash/bcm47xxnflash.h |  1 -
>  drivers/mtd/nand/bcm47xxnflash/main.c  |  7 +-
>  drivers/mtd/nand/bcm47xxnflash/ops_bcm4706.c   |  2 +-
>  drivers/mtd/nand/bf5xx_nand.c  | 14 ++--
>  drivers/mtd/nand/brcmnand/brcmnand.c   | 11 ++-
>  drivers/mtd/nand/cafe_nand.c   | 10 +--
>  drivers/mtd/nand/cmx270_nand.c | 11 ++-
>  drivers/mtd/nand/cs553x_nand.c | 13 ++--
>  drivers/mtd/nand/davinci_nand.c| 25 +++
>  drivers/mtd/nand/denali.c  | 61 +
>  drivers/mtd/nand/denali.h  |  1 -
>  drivers/mtd/nand/diskonchip.c  | 11 ++-
>  drivers/mtd/nand/docg4.c   | 18 +++--
>  drivers/mtd/nand/fsl_elbc_nand.c   | 22 +++---
>  drivers/mtd/nand/fsl_ifc_nand.c| 23 +++
>  drivers/mtd/nand/fsl_upm.c | 26 +++
>  drivers/mtd/nand/fsmc_nand.c   | 59 +---
>  drivers/mtd/nand/gpio.c| 16 ++---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  2 +-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 20 +++---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  1 -
>  drivers/mtd/nand/hisi504_nand.c| 11 ++-
>  drivers/mtd/nand/jz4740_nand.c |  9 ++-
>  drivers/mtd/nand/lpc32xx_mlc.c |  7 +-
>  drivers/mtd/nand/lpc32xx_slc.c |  7 +-
>  drivers/mtd/nand/mpc5121_nfc.c |  3 +-
>  drivers/mtd/nand/mxc_nand.c|  5 +-
>  drivers/mtd/nand/nandsim.c | 12 ++--
>  drivers/mtd/nand/ndfc.c| 22 +++---
>  drivers/mtd/nand/nuc900_nand.c | 21 +++---
>  drivers/mtd/nand/omap2.c   | 94 
> +++---
>  drivers/mtd/nand/orion_nand.c  |  4 +-
>  drivers/mtd/nand/pasemi_nand.c | 14 ++--
>  drivers/mtd/nand/plat_nand.c   | 14 ++--
>  drivers/mtd/nand/pxa3xx_nand.c | 33 -

^^ BTW, this file already has a few conflicts. Sorry :(

I'll try to keep any eye out for things like this once we're close to
being able to apply something like this, so I don't merge unnecessary
churn. But for now, I hope we can review this series, and it won't be
too much work to rebase/resend once the bigger things have been worked
out.

>  drivers/mtd/nand/r852.c| 34 --
>  drivers/mtd/nand/r852.h|  1 -
>  drivers/mtd/nand/s3c2410.c | 19 +++---
>  drivers/mtd/nand/sh_flctl.c|  8 +--
>  drivers/mtd/nand/sharpsl.c | 18 ++---
>  drivers/mtd/nand/socrates_nand.c   |  5 +-
>  drivers/mtd/nand/sunxi_nand.c  | 13 ++--
>  drivers/mtd/nand/tmio_nand.c   |  7 +-
>  drivers/mtd/nand/txx9ndfmc.c   |  3 +-
>  drivers/mtd/nand/vf610_nfc.c   |  5 +-
>  include/linux/mtd/sh_flctl.h   |  3 +-
>  49 files changed, 383 insertions(+), 385 deletions(-)
> 

...

> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index f8aac0a..51748b4 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c

...

> @@ -318,7 +317,7 @@ static int nfc_set_sram_bank(struct atmel_nand_host 
> *host, unsigned int bank)
>  
>   if (bank) {
>   /* Only for a 2k-page or lower flash, NFC can handle 2 banks */
> - if (host->mtd.writesize > 2048)
> + if 

[linux-sunxi] Re: [PATCH 18/27] mtd: nand: update mtd_to_nand()

[Brian Norris]

On Mon, Nov 16, 2015 at 02:37:51PM +0100, Boris Brezillon wrote:
> Now that all drivers are using the mtd instance embedded in the nand_chip

Do you have a script that verifies this? I thought you did at some
point, and it'd be nice to note it, so I can also use it to verify
things once it gets applied.

> struct we can safely update the mtd_to_nand_chip() implementation to use

Nit: s/mtd_to_nand_chip/mtd_to_nand/

> the container_of macro instead of returning the content of mtd->priv.
> This will allow us to remove mtd->priv = chip assignments done in all
> NAND controller drivers.
> 
> Signed-off-by: Boris Brezillon 
> ---
>  include/linux/mtd/nand.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 8ec071e..873646d 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -734,7 +734,7 @@ static inline struct device_node 
> *nand_get_flash_node(struct nand_chip *chip)
>  
>  static inline struct nand_chip *mtd_to_nand(struct mtd_info *mtd)
>  {
> - return mtd->priv;
> + return container_of(mtd, struct nand_chip, mtd);
>  }
>  
>  static inline struct mtd_info *nand_to_mtd(struct nand_chip *chip)
> -- 
> 2.1.4
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 1/2] mtd: nand: sunxi: fix sunxi_nfc_hw_ecc_read/write_chunk()

[Brian Norris]

On Tue, Oct 20, 2015 at 10:16:00PM +0200, Boris Brezillon wrote:
> The sunxi_nfc_hw_ecc_read/write_chunk() functions try to avoid changing
> the column address if unnecessary, but the logic to determine whether it's
> necessary or not is currently wrong: it adds the ecc->bytes value to the
> current offset where it should actually add ecc->size.
> 
> Signed-off-by: Boris Brezillon 
> Fixes 913821bdd211 "mtd: nand: sunxi: introduce 
> sunxi_nfc_hw_ecc_read/write_chunk()"

Pushed both to l2-mtd.git

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 0/7] mtd: nand: sunxi: cleanup and improvements

[Brian Norris]

On Wed, Sep 30, 2015 at 11:45:22PM +0200, Boris Brezillon wrote:
> Hello,
> 
> This patch series aims at cleaning up the sunxi_nand driver by factorizing
> the duplicated code found in hw_ecc and hw_syndrome_ecc implementations.
> 
> It also adds support for OOB bytes protection (only on a limited amount of
> OOB bytes), and add code to correctly handle the 'bitflips in erased pages'
> case.
> 
> Best Regards,
> 
> Boris
> 
> Changes since v1:
> - drop the first patch (already applied)
> - split the second patch to ease the review
> - add the 'fix bitflips in erased pages' patch
> 
> Boris Brezillon (7):
>   mtd: nand: sunxi: create sunxi_nfc_hw_ecc_enable()/disable() functions
>   mtd: nand: sunxi: introduce sunxi_nfc_hw_ecc_read/write_chunk()
>   mtd: nand: sunxi: make use of sunxi_nfc_hw_ecc_read/write_chunk()
>   mtd: nand: sunxi: factorize extra OOB bytes handling
>   mtd: nand: sunxi: retrieve corrected OOB bytes
>   mtd: nand: sunxi: replace the NFC_BUF_TO_USER_DATA() macro by an
> inline function
>   mtd: nand: sunxi: fix bitflips in erased pages

All look good to me. Pushed to l2-mtd.git. Thanks.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 2/2] mtd: sunxi: nand: refactor ->read_page()/->write_page() code

[Brian Norris]

On Wed, Sep 16, 2015 at 09:46:37AM +0200, Boris Brezillon wrote:
> Most of the logic to read/write pages with the HW ECC engine enabled
> is common to the HW_ECC and NAND_ECC_HW_SYNDROME scheme.
> Refactor the code to avoid code duplication.

Hmm, a benign commit description to describe a somewhat complicated
patch. This seems to do several different types of refactoring all at
once, and it makes it a bit hard to review. Can you perhaps refactor
this into 2 or more patches? e.g., I think the NFC_USER_DATA_TO_BUF()
stuff can be orthogonal from the introduction of
sunxi_nfc_hw_ecc_read_chunk() and sunxi_nfc_hw_ecc_read_extra_oob().

> Signed-off-by: Boris Brezillon 
> ---
>  drivers/mtd/nand/sunxi_nand.c | 404 
> ++
>  1 file changed, 212 insertions(+), 192 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> index dc44435..640b96c 100644
> --- a/drivers/mtd/nand/sunxi_nand.c
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -159,6 +159,13 @@
>  /* NFC_USER_DATA helper macros */
>  #define NFC_BUF_TO_USER_DATA(buf)((buf)[0] | ((buf)[1] << 8) | \
>   ((buf)[2] << 16) | ((buf)[3] << 24))
> +#define NFC_USER_DATA_TO_BUF(buf, val)   \
> + {   \
> + (buf)[0] = val; \
> + (buf)[1] = val >> 8;\
> + (buf)[2] = val >> 16;   \
> + (buf)[3] = val >> 24;   \
> + }

Two things about this macro:

  1) you should probably wrap 'val' in parentheses

  2) the use of 'val' 4 times in this macro means it will be evaluated 4
  times; this *can* be OK, except the 'val' parameter as used in context
  is actually a register read (readl()). Is this intentional? Anyway,
  such a construct kinda hides the actual behavior, whether or not it's
  intentional.

To kill off all concerns, perhaps this should be a static inline
function instead. And we might do the same with NFC_BUF_TO_USER_DATA()
then, to match.

Brian

>  
>  #define NFC_DEFAULT_TIMEOUT_MS   1000
>  

[snip rest of patch]

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 1/2] mtd: nand: sunxi: rework macros

[Brian Norris]

On Wed, Sep 16, 2015 at 09:46:36AM +0200, Boris Brezillon wrote:
> Suffix mask macros with _MSK and add new helper macros to avoid manually
> shifting values.
> 
> Signed-off-by: Boris Brezillon 

Pushed patch 1 to l2-mtd.git

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 0/2] mtd: nand: sunxi: cleanup

[Brian Norris]

On Wed, Sep 30, 2015 at 08:36:34AM +0200, Boris Brezillon wrote:
> On Tue, 29 Sep 2015 15:21:44 -0700
> Brian Norris <computersforpe...@gmail.com> wrote:
> 
> > On Wed, Sep 16, 2015 at 09:46:35AM +0200, Boris Brezillon wrote:
> > > Hello,
> > > 
> > > Those two patches aim at cleaning up the sunxi_nand driver by first adding
> > > some consistency in the macro definitions, and then factorizing the code
> > > duplicated code found in hw_ecc and hw_syndrome_ecc implementations.
> > > 
> > > Best Regards,
> > > 
> > > Boris
> > 
> > I believe this series no longer applies cleanly
> 
> Hm, actually it depends on fixes you have in your linux-mtd tree and
> features in your l2-mtd tree.
> 
> I don't know how you usually deal with those problems (merging the
> linux-mtd/master branch into the l2-mtd/master one, or merging
> last linus' -rc into l2-mtd/master should work), but I'd really
> like to have this in 4.4 :-).

linux-mtd.git is prepped for a pull request. I'll do that before the
week ends. And I'll either merge that back into l2-mtd.git at that time,
or just pull in v4.3-rc4.

> I still have the patch using the nand_check_erased_ecc_chunk() function
> which depends on those 2 patches, and I'd like to have it in 4.4 too.
> 
> I know I'm asking a lot, but as explained earlier, I still have a
> bunch of patches on top of those already posted (mainly to support
> the hardware randomizer), and I'd like the trivial ones to be merged
> quickly.

I'll see what I can apply after pulling together all the latest, but if
I'm still missing things then, feel free to resend.

BTW, I thought there were some things to address on the erased ECC stuff
still. I'll re-check that series too.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 0/2] mtd: nand: sunxi: cleanup

[Brian Norris]

On Wed, Sep 16, 2015 at 09:46:35AM +0200, Boris Brezillon wrote:
> Hello,
> 
> Those two patches aim at cleaning up the sunxi_nand driver by first adding
> some consistency in the macro definitions, and then factorizing the code
> duplicated code found in hw_ecc and hw_syndrome_ecc implementations.
> 
> Best Regards,
> 
> Boris

I believe this series no longer applies cleanly

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] mtd: nand: sunxi: fix sunxi_nand_chips_cleanup()

[Brian Norris]

On Sun, Sep 13, 2015 at 06:14:43PM +0200, Boris Brezillon wrote:
> The sunxi_nand_chips_cleanup() function is missing a call to list_del()
> which generates a double free error.

And if you could manage to get past that...an infinite loop, no?

> Reported-by: Priit Laes 
> Signed-off-by: Boris Brezillon 
> Cc:  # 3.19+
> Fixes: 1fef62c1423b ("mtd: nand: add sunxi NAND flash controller support")

Applied to linux-mtd.git, as this is a -stable fix.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4] mtd: nand: sunxi: fix OOB handling in ->write_xxx() functions

[Brian Norris]

On Mon, Sep 14, 2015 at 10:02:04AM -0700, Brian Norris wrote:
> On Mon, Sep 14, 2015 at 10:41:03AM +0200, Boris Brezillon wrote:
> > The USER_DATA register cannot be accessed using byte accessors on A13
> > SoCs, thus triggering a bug when using memcpy_toio on this register.
> > Declare an helper macros to convert an OOB buffer into a suitable
> > USER_DATA value and vice-versa.
> > 
> > This patch also fixes an error in the oob_required logic (some OOB data
> > are not written even if the user required it) by removing the
> > oob_required condition, which is perfectly valid since the core already
> > fill ->oob_poi with FFs when oob_required is false.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezil...@free-electrons.com>
> > Cc: <sta...@vger.kernel.org> # 3.19+
> > Fixes: 1fef62c1423b ("mtd: nand: add sunxi NAND flash controller support")
> > 
> > ---
> > Changes since v3:
> > - drop the NFC_USER_DATA_TO_BUF() macro
> 
> I don't have any real objections to this version, and I think some IRC
> discussion helped clear up a few questions. I'll take this for 4.3 soon,
> unless I see any objections.

Pushed to linux-mtd.git.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4] mtd: nand: sunxi: fix OOB handling in ->write_xxx() functions

[Brian Norris]

On Mon, Sep 14, 2015 at 10:41:03AM +0200, Boris Brezillon wrote:
> The USER_DATA register cannot be accessed using byte accessors on A13
> SoCs, thus triggering a bug when using memcpy_toio on this register.
> Declare an helper macros to convert an OOB buffer into a suitable
> USER_DATA value and vice-versa.
> 
> This patch also fixes an error in the oob_required logic (some OOB data
> are not written even if the user required it) by removing the
> oob_required condition, which is perfectly valid since the core already
> fill ->oob_poi with FFs when oob_required is false.
> 
> Signed-off-by: Boris Brezillon 
> Cc:  # 3.19+
> Fixes: 1fef62c1423b ("mtd: nand: add sunxi NAND flash controller support")
> 
> ---
> Changes since v3:
> - drop the NFC_USER_DATA_TO_BUF() macro

I don't have any real objections to this version, and I think some IRC
discussion helped clear up a few questions. I'll take this for 4.3 soon,
unless I see any objections.

> Changes since v2:
> - add the NFC_USER_DATA_TO_BUF() macro and rename NFC_USER_DATA() into
>   NFC_BUF_TO_USER_DATA()
> - rework the commit message
> 
> Changes since v1:
> - drop the !oob_required conditional path
> - replace endianness conversions by a macro relying on byte shifting
>   operations
> ---
>  drivers/mtd/nand/sunxi_nand.c | 26 +-
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> index f97a58d..279cafd 100644
> --- a/drivers/mtd/nand/sunxi_nand.c
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -147,6 +147,10 @@
>  #define NFC_ECC_MODE GENMASK(15, 12)
>  #define NFC_RANDOM_SEED  GENMASK(30, 16)
>  
> +/* NFC_USER_DATA helper macros */
> +#define NFC_BUF_TO_USER_DATA(buf)((buf)[0] | ((buf)[1] << 8) | \
> + ((buf)[2] << 16) | ((buf)[3] << 24))
> +
>  #define NFC_DEFAULT_TIMEOUT_MS   1000
>  
>  #define NFC_SRAM_SIZE1024
> @@ -646,15 +650,9 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info 
> *mtd,
>   offset = layout->eccpos[i * ecc->bytes] - 4 + mtd->writesize;
>  
>   /* Fill OOB data in */
> - if (oob_required) {
> - tmp = 0x;
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, ,
> - 4);
> - } else {
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE,
> - chip->oob_poi + offset - mtd->writesize,
> - 4);
> - }
> + writel(NFC_BUF_TO_USER_DATA(chip->oob_poi +
> + layout->oobfree[i].offset),
> +nfc->regs + NFC_REG_USER_DATA_BASE);

I believe the few remaining questionable points were:

 * you don't actually need to add the barrier, thus __raw_writel() would
   suffice
 * you're still doing a double swap for the (likely theoretical) big
   endian case

Neither point matters much to me, so as noted above, LGTM.

>  
>   chip->cmdfunc(mtd, NAND_CMD_RNDIN, offset, -1);
>  
> @@ -784,14 +782,8 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct 
> mtd_info *mtd,
>   offset += ecc->size;
>  
>   /* Fill OOB data in */
> - if (oob_required) {
> - tmp = 0x;
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, ,
> - 4);
> - } else {
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, oob,
> - 4);
> - }
> + writel(NFC_BUF_TO_USER_DATA(oob),
> +nfc->regs + NFC_REG_USER_DATA_BASE);

Same here.

>  
>   tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ACCESS_DIR |
> (1 << 30);

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3] mtd: nand: sunxi: fix OOB handling in ->write_xxx() functions

[Brian Norris]

On Thu, Sep 03, 2015 at 09:06:48AM +0200, Boris Brezillon wrote:
> The USER_DATA register cannot be accessed using byte accessors on A13
> SoCs, thus triggering a bug when using memcpy_toio on this register.
> Declare two helper macros to convert an OOB buffer into a suitable
> USER_DATA value and vice-versa.
> 
> This patch also fixes an error in the oob_required logic (some OOB data
> are not written even if the user required it) by removing the
> oob_required condition, which is perfectly valid since the core already
> fill ->oob_poi with FFs when oob_required is false.
> 
> Signed-off-by: Boris Brezillon 
> Cc:  # 3.19+
> Fixes: 1fef62c1423b ("mtd: nand: add sunxi NAND flash controller support")
> 
> ---
> Changes since v2:
> - add the NFC_USER_DATA_TO_BUF() macro and rename NFC_USER_DATA() into
>   NFC_BUF_TO_USER_DATA()
> - rework the commit message
> 
> Changes since v1:
> - drop the !oob_required conditional path
> - replace endianness conversions by a macro relying on byte shifting
>   operations
> ---
>  drivers/mtd/nand/sunxi_nand.c | 33 -
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> index f97a58d..f9b5a4c 100644
> --- a/drivers/mtd/nand/sunxi_nand.c
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -147,6 +147,17 @@
>  #define NFC_ECC_MODE GENMASK(15, 12)
>  #define NFC_RANDOM_SEED  GENMASK(30, 16)
>  
> +/* NFC_USER_DATA helper macros */
> +#define NFC_BUF_TO_USER_DATA(buf)((buf)[0] | ((buf)[1] << 8) | \
> + ((buf)[2] << 16) | ((buf)[3] << 24))

I thought you were treading on thin ice with bit-shifting a uint8_t, but
it appears that C integer promotion rules are on your side here. (The
result of this operation should be an int.)

> +#define NFC_USER_DATA_TO_BUF(buf, val)   \

BTW, why do you need this macro? It's currently unused. Just as an
example? I can take it as-is if you'd like, but I was curious why you
felt the need for this in v3, especially for -stable.

Brian

> + {   \
> + (buf)[0] = val; \
> + (buf)[1] = val >> 8;\
> + (buf)[2] = val >> 16;   \
> + (buf)[3] = val >> 24;   \
> + }
> +
>  #define NFC_DEFAULT_TIMEOUT_MS   1000
>  
>  #define NFC_SRAM_SIZE1024
> @@ -646,15 +657,9 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info 
> *mtd,
>   offset = layout->eccpos[i * ecc->bytes] - 4 + mtd->writesize;
>  
>   /* Fill OOB data in */
> - if (oob_required) {
> - tmp = 0x;
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, ,
> - 4);
> - } else {
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE,
> - chip->oob_poi + offset - mtd->writesize,
> - 4);
> - }
> + writel(NFC_BUF_TO_USER_DATA(chip->oob_poi +
> + layout->oobfree[i].offset),
> +nfc->regs + NFC_REG_USER_DATA_BASE);
>  
>   chip->cmdfunc(mtd, NAND_CMD_RNDIN, offset, -1);
>  
> @@ -784,14 +789,8 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct 
> mtd_info *mtd,
>   offset += ecc->size;
>  
>   /* Fill OOB data in */
> - if (oob_required) {
> - tmp = 0x;
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, ,
> - 4);
> - } else {
> - memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, oob,
> - 4);
> - }
> + writel(NFC_BUF_TO_USER_DATA(oob),
> +nfc->regs + NFC_REG_USER_DATA_BASE);
>  
>   tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ACCESS_DIR |
> (1 << 30);
> -- 
> 1.9.1
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3] mtd: nand: sunxi: rely on nand_dt_init initialization

[Brian Norris]

On Wed, Sep 02, 2015 at 10:30:25AM +0200, Boris Brezillon wrote:
> nand_dt_init(), called from nand_scan_ident(), is already parsing the
> generic MTD/NAND DT properties, and initializing the nand_chip struct
> accordingly.
> Rely on this initialization instead of manually parsing those properties.
> 
> Signed-off-by: Boris Brezillon 
> ---
> Changes since v2:
> - fix a bug forcing ecc.mode to HW_ECC
> Changes since v1:
> - drop unused strength and blk_size variables
> ---
>  drivers/mtd/nand/sunxi_nand.c | 27 ++-
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> index 0c29f6c..d644548 100644
> --- a/drivers/mtd/nand/sunxi_nand.c
> +++ b/drivers/mtd/nand/sunxi_nand.c
[...]
> @@ -1224,15 +1211,18 @@ static int sunxi_nand_chip_init(struct device *dev, 
> struct sunxi_nfc *nfc,
>   /* Default tR value specified in the ONFI spec (chapter 4.15.1) */
>   nand->chip_delay = 200;
>   nand->controller = >controller;
> + /*
> +  * Set the ECC mode to the default value in case nothing is specified
> +  * in the DT.
> +  */
> + nand->ecc.mode = NAND_ECC_HW;
> + nand->dn = np;

Renamed dn -> flash_node, due to [1], and applied to l2-mtd.git/next

>   nand->select_chip = sunxi_nfc_select_chip;
>   nand->cmd_ctrl = sunxi_nfc_cmd_ctrl;
>   nand->read_buf = sunxi_nfc_read_buf;
>   nand->write_buf = sunxi_nfc_write_buf;
>   nand->read_byte = sunxi_nfc_read_byte;
>  
> - if (of_get_nand_on_flash_bbt(np))
> - nand->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> -
>   mtd = >mtd;
>   mtd->dev.parent = dev;
>   mtd->priv = nand;

Brian

[1] http://patchwork.ozlabs.org/patch/514205/

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2] mtd: nand: sunxi: rely on nand_dt_init initialization

[Brian Norris]

On Wed, Sep 02, 2015 at 10:21:08AM +0200, Boris Brezillon wrote:
> On Tue, 1 Sep 2015 18:16:16 -0700
> Brian Norris <computersforpe...@gmail.com> wrote:
> > There's also a subtle change here that affects more than just here: you
> > don't have a good way to tell the difference between "no ECC mode
> > provide" and NAND_ECC_NONE. Perhaps we can modify the nand_ecc_modes_t
> > enum to include a "uninitialized" value? I'm not sure if that'd work
> > best as 0, -1, or something else, in case there are people making
> > assumptions elsewhere...
> 
> It really depends on what we want to do. I see at least two solutions
> for this problem:
> 1/ keep the nand_dt_init() implementation as is and ask
>nand_scan_ident() callers to pre-set their default value in ecc.mode.
>This should work since ecc.mode is only overridden if ecc_mode is >= 0.
> 2/ modify the nand_dt_init function to assign the ecc_mode even if it
>is negative, or create a new value in the enum (NAND_ECC_UNSPECIFIED
>= -1) and assign it to ecc.mode when ecc_mode is negative.
> 
> 1# does not require any change, but if we choose this solution, we
> should make it clear that NAND controller drivers should set their
> default value before calling nand_scan_ident().

I'm OK with your choice of #1. Your v3 patch looks fine.

> And if go for 2#, that's not like we'll have to patch a lot of
> drivers (AFAICT, the only user in 4.2 is the brcmnand driver) ;-).

:) Well, I thought there were others in the pipeline. I've at least
reviewed another addition somewhat recently, but it wasn't quite ready.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2] nand: sunxi: fix write to USER_DATA reg

[Brian Norris]

On Wed, Sep 02, 2015 at 09:57:01AM +0200, Boris Brezillon wrote:
> Also drop the oob_required contidion, since ->oob_pio already contains
> FFs when oob_required is false.

I was pointing out this issues not just because you didn't note it in
the commit message, but because the previous behavior actually
potentially has user-visible effects. Particularly, because you had the
conditions backwards, you were programming 0xff to the spare area, no
matter what the user asked for. Normally, this isn't a big issue, since
most users aren't storing data in the OOB region. But there may be cases
where your driver was subtly broken (marking bad blocks, perhaps?), and
this patch is subtly fixing them. Especially since this will get
backported to -stable, it'd be nice to have this noted completely.

Did this driver pass mtd_oobtest previously?

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] nand: sunxi: fix write to USER_DATA reg

[Brian Norris]

Hi Boris,

On Mon, Aug 24, 2015 at 11:56:30AM +0200, Boris Brezillon wrote:
> On Thu, 30 Jul 2015 12:01:06 +0200
> Boris Brezillon  wrote:
> 
> > The USER_DATA register cannot be accessed using byte accessors on A13
> > SoCs, thus triggering a bug when using memcpy_toio on this register.
> > Declare a temporary u32 variable to store the USER_DATA value and access
> > the register with writel.
> > 
> > Signed-off-by: Boris Brezillon 
> 
> Could you consider taking this patch for 4.3 (if it's too late for
> 4.3-rc1, could you queue it for -rc2)?

Sorry, didn't notice this until after my pull request. I can take some
version of this for 4.3-rcX for sure. Remains to be seen which one...

> BTW, I'd like to add
> 
> Cc:  # 3.19+
> 
> should I resend a new version or could you add it while you're applying
> the patch on your branch?

I can add it if I take this one, or you can add it yourself if we make
it to v2.

> Thanks,
> 
> Boris
> 
> > ---
> >  drivers/mtd/nand/sunxi_nand.c | 24 +---
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> > index 6f93b29..5e374ab 100644
> > --- a/drivers/mtd/nand/sunxi_nand.c
> > +++ b/drivers/mtd/nand/sunxi_nand.c
> > @@ -624,6 +624,8 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info 
> > *mtd,
> > writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
> >  
> > for (i = 0; i < ecc->steps; i++) {
> > +   u32 user_data;
> > +
> > if (i)
> > chip->cmdfunc(mtd, NAND_CMD_RNDIN, i * ecc->size, -1);
> >  
> > @@ -632,16 +634,16 @@ static int sunxi_nfc_hw_ecc_write_page(struct 
> > mtd_info *mtd,
> > offset = layout->eccpos[i * ecc->bytes] - 4 + mtd->writesize;
> >  
> > /* Fill OOB data in */
> > -   if (oob_required) {
> > -   tmp = 0x;
> > -   memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, ,
> > -   4);
> > +   if (!oob_required) {

Hmm, seems like you're flipping the condition:

  oob_required --> !oob_required

That looks like the right change, but it's not mentioned in the commit
message.

> > +   user_data = 0x;

While we're at it: this oob_required dance looks a little unnecessary.
If (as it seems here) you need to fill in the OOB data with *something*
(even if it's just 0x), then it's safe to just ignore
the oob_required parameter and just use chip->oob_poi, which will have
been filled with 0xff. (The semantic meaning is that even if programming
the OOB from ->oob_poi is "not required", it is still allowed.)

That would help simplify this code too, I think, as you can eliminate
the !oob_required case.

> > } else {
> > -   memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE,
> > -   chip->oob_poi + offset - mtd->writesize,
> > -   4);
> > +   memcpy(_data,
> > +  chip->oob_poi + layout->oobfree[i].offset, 4);
> > +   user_data = le32_to_cpu(user_data);

sparse doesn't like this (here and below):

drivers/mtd/nand/sunxi_nand.c:656:37: warning: cast to restricted __le32 
[sparse]
drivers/mtd/nand/sunxi_nand.c:793:31: warning: cast to restricted __le32 
[sparse]

Seems you're trying to shoehorn yourself into using writel(), when you
don't actually want the endian swapping. I think this is a valid case
for using __raw_writel(), actually (Arnd even noted this last time we
discussed the "right" way to use some of these accessors). Maybe put a
comment for it too.

> > }
> >  
> > +   writel(user_data, nfc->regs + NFC_REG_USER_DATA_BASE);
> > +
> > chip->cmdfunc(mtd, NAND_CMD_RNDIN, offset, -1);
> >  
> > ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
> > @@ -772,13 +774,13 @@ static int 
> > sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
> > /* Fill OOB data in */
> > if (oob_required) {
> > tmp = 0x;
> > -   memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, ,
> > -   4);
> > } else {
> > -   memcpy_toio(nfc->regs + NFC_REG_USER_DATA_BASE, oob,
> > -   4);
> > +   memcpy(, oob, sizeof(tmp));
> > +   tmp = le32_to_cpu(tmp);
> > }
> >  
> > +   writel(tmp, nfc->regs + NFC_REG_USER_DATA_BASE);
> > +
> > tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ACCESS_DIR |
> >   (1 << 30);
> > writel(tmp, nfc->regs + NFC_REG_CMD);

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving 

[linux-sunxi] Re: [PATCH v2] mtd: nand: sunxi: rely on nand_dt_init initialization

[Brian Norris]

On Mon, Aug 24, 2015 at 12:10:31PM +0200, Boris Brezillon wrote:
> nand_dt_init(), called from nand_scan_ident(), is already parsing the
> generic MTD/NAND DT properties, and initializing the nand_chip struct
> accordingly.
> Rely on this initialization instead of manually parsing those properties.
> 
> Signed-off-by: Boris Brezillon 
> ---
> Changes since v1:
> - drop unused strength and blk_size variables

Good catch, I was about to comment on v1 :)

> ---
>  drivers/mtd/nand/sunxi_nand.c | 20 +---
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
> index 96dda70..92a2c58 100644
> --- a/drivers/mtd/nand/sunxi_nand.c
> +++ b/drivers/mtd/nand/sunxi_nand.c
> @@ -1164,16 +1164,9 @@ static int sunxi_nand_ecc_init(struct mtd_info *mtd, 
> struct nand_ecc_ctrl *ecc,
>  struct device_node *np)
>  {
>   struct nand_chip *nand = mtd->priv;
> - int strength;
> - int blk_size;
>   int ret;
>  
> - blk_size = of_get_nand_ecc_step_size(np);
> - strength = of_get_nand_ecc_strength(np);
> - if (blk_size > 0 && strength > 0) {
> - ecc->size = blk_size;
> - ecc->strength = strength;
> - } else {
> + if (!ecc->size) {
>   ecc->size = nand->ecc_step_ds;
>   ecc->strength = nand->ecc_strength_ds;
>   }
> @@ -1183,10 +1176,6 @@ static int sunxi_nand_ecc_init(struct mtd_info *mtd, 
> struct nand_ecc_ctrl *ecc,
>  
>   ecc->mode = NAND_ECC_HW;
>  
> - ret = of_get_nand_ecc_mode(np);
> - if (ret >= 0)
> - ecc->mode = ret;
> -

I think you've missed something here. You're now *only* allowing HW ECC!
You now want to make this a default case.

There's also a subtle change here that affects more than just here: you
don't have a good way to tell the difference between "no ECC mode
provide" and NAND_ECC_NONE. Perhaps we can modify the nand_ecc_modes_t
enum to include a "uninitialized" value? I'm not sure if that'd work
best as 0, -1, or something else, in case there are people making
assumptions elsewhere...

>   switch (ecc->mode) {
>   case NAND_ECC_SOFT_BCH:
>   break;
> @@ -1312,15 +1301,13 @@ static int sunxi_nand_chip_init(struct device *dev, 
> struct sunxi_nfc *nfc,
>   /* Default tR value specified in the ONFI spec (chapter 4.15.1) */
>   nand->chip_delay = 200;
>   nand->controller = >controller;
> + nand->dn = np;
>   nand->select_chip = sunxi_nfc_select_chip;
>   nand->cmd_ctrl = sunxi_nfc_cmd_ctrl;
>   nand->read_buf = sunxi_nfc_read_buf;
>   nand->write_buf = sunxi_nfc_write_buf;
>   nand->read_byte = sunxi_nfc_read_byte;
>  
> - if (of_get_nand_on_flash_bbt(np))
> - nand->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> -
>   mtd = >mtd;
>   mtd->dev.parent = dev;
>   mtd->priv = nand;
> @@ -1330,6 +1317,9 @@ static int sunxi_nand_chip_init(struct device *dev, 
> struct sunxi_nfc *nfc,
>   if (ret)
>   return ret;
>  
> + if (nand->bbt_options & NAND_BBT_USE_FLASH)
> + nand->bbt_options |= NAND_BBT_NO_OOB;
> +
>   ret = sunxi_nand_chip_init_timings(chip, np);
>   if (ret) {
>   dev_err(dev, "could not configure chip timings: %d\n", ret);

Brian

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 1/4] mtd: nand: Fix NAND_* options to use unique values.

[Brian Norris]

On Wed, Jul 29, 2015 at 07:53:51PM +0200, Hans de Goede wrote:
 From: Michal Suchanek hramr...@gmail.com
 
 NAND_BUSWIDTH_AUTO (64b37b2a6) and NAND_USE_BOUNCE_BUFFER (66507c7bc)
 are the same value. Change the later introduced NAND_USE_BOUNCE_BUFFER
 to a different value.
 
 Signed-off-by: Michal Suchanek hramr...@gmail.com
 Signed-off-by: Hans de Goede hdego...@redhat.com

Thanks for the fix! But Scott already caught this one. The fix is in
4.2-rc already as commit 5f867db63473 (mtd: nand: Fix
NAND_USE_BOUNCE_BUFFER flag conflict).

(Obligatory: please base your MTD patches on l2-mtd.git, or
linux-next.git if necessary [1].)

Regards,
Brian

[1] http://linux-mtd.infradead.org/source.html

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v5 1/2] mtd: nand: sunxi: Replace failsafe timing cfg with calculated value

[Brian Norris]

On Fri, Jun 26, 2015 at 11:00:10AM +0200, Roy Spliet wrote:
 The TIMING_CFG register was previously statically set to a magic value
 (extracted from Allwinner's BSP) when initializing the NAND controller.
 Now that we have more details about the TIMING_CFG register layout
 (extracted from the A83 user manual) we can dynamically calculate the
 appropriate value for each NAND chip and set it when selecting the
 chip.
 
 Signed-off-by: Roy Spliet r.spl...@ultimaker.com
 Acked-by: Boris Brezillon boris.brezil...@free-electrons.com
 
 ---
 V2:
 - Fix crippled comments
 
 V3:
 - Warn for invalid timings
 - Style
 
 V4:
 - Make better use of return types
 - Style and comments
 - Remove superfluous initialisation
 
 V5:
 - Tidy up macros

Pushed both to l2-mtd.git. Thanks!

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 2/3] MTD: spi-nor: check for short writes in spi_nor_write.

[Brian Norris]

On Thu, May 21, 2015 at 11:28:02AM +0100, Mark Brown wrote:
 On Thu, May 21, 2015 at 10:39:13AM +0200, Michal Suchanek wrote:
  On 21 May 2015 at 01:38, Brian Norris computersforpe...@gmail.com wrote:
 
 Why is this thread about SPI error handling CCed to quite so many
 people?

I can't really speak for the original patch author, who created the CC
list. I added you for comment on the SPI API (thanks BTW).

This is part of a series that included an ill-conceived DT binding for
recording a max transfer length property in the flash node. That's one
step that easily blows up the CC list for the series, as there are 5 DT
maintainers and a mailing list. Others are contributors to the m25p80 /
spi-nor drivers. (All in all, you can probably trace this back to
scripts/get_maintainer.pl.)

   Check the amount of data actually written by the driver.
 
   I'm not sure if we should just reactively use the retlen, or if we
   should be communicating such a limitation via the SPI API. Thoughts?
 
  Is there any driver that would break if the SPI master truncated
  writes when the message is too long rather than returning an error an
  refusing the transfer?
 
 Any such driver is buggy, the actual length transferred has always been
 a part of the SPI API.

OK, so m25p80.c currently checks the retlen
(spi_message::actual_length), but it doesn't actually handle it well if
the SPI master can't write a full page-size chunk in one go. It looks
like we'd leave holes in the programmed data right now.

So that qualifies as buggy, and that's part of what Michal is trying to
fix, I think. Admittedly, as he's using an out-of-tree driver, I'm not
sure I know exactly what failure modes he is hitting yet.

 We should probably expose limitations so clients
 can query in advance (we don't at the minute) but it'd be a while before
 clients could rely on that information being useful and it's still
 possible things can be truncated by error.

That might help in the long run. In this case, I think we might be able
to get by (for a properly-functioning SPI driver with a limited transfer
size) with the current API, and handling the 'return length  message
length' case better.

BTW, one extra note for Michal regarding your SPI controller/driver
transfer length limitation: you would do well to try as much as possible
to allow full NOR pages to be programmed in one SPINOR_OP_PP sequence. I
know of some SPI NOR that, though they are byte addressable, actually
have opaque internal ECC which is encoded on page boundaries, so you
get better flash lifetime if you program on page boundaries, rather than
on whatever smaller chunk size your SPI driver supports. So that might
require a little more work on your SPI driver.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 2/3] MTD: spi-nor: check for short writes in spi_nor_write.

[Brian Norris]

(trimming CC a little this time, though it's still a bit large)

On Fri, May 22, 2015 at 12:17:27AM -0700, Brian Norris wrote:
 Admittedly, as he's using an out-of-tree driver, I'm not
 sure I know exactly what failure modes he is hitting yet.

Sorry, I realized I misread here. He's using spi-sunxi. Given that...

... is this code even valid?

static int sun6i_spi_transfer_one(struct spi_master *master,
  struct spi_device *spi,
  struct spi_transfer *tfr)
{
...
/* We don't support transfer larger than the FIFO */
if (tfr-len  SUN6I_FIFO_DEPTH)
return -EINVAL;

Seems like it should be looping over the transfer in multiple chunks
instead.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 0/3] Using SPI NOR flah on sunxi.

[Brian Norris]

On Thu, Apr 30, 2015 at 08:34:36PM +0200, Marek Vasut wrote:
 On Thursday, April 30, 2015 at 06:56:18 PM, Michal Suchanek wrote:
  On 30 April 2015 at 18:30,  thomas.bet...@rohde-schwarz.com wrote:
   Hello Michal:
   I tried to connect a SPI NOR flash to my sunxi board and due to the
   
   current
   
   sunxi SPI driver limitations it does not work.
   
   The SPI driver returns an error when more than 64 bytes are
   transferred at once
   due to lack of DMA support.
   
   Wouldn't it be easier to fix the SPI driver to handle transfers larger
   than 64 bytes, filling and draining the FIFO multiple times if
   neccessary? (As far as I can tell, most SPI drivers do this.)
  
  Yes, the intent is to fix this by adding dma support to the driver,
  eventually.
  
  The patch might be still useful for other hardware with developing SPI
  support.
 
 Please just fix the controller driver to correctly handle arbitrary transfer
 lengths.

Just noticed this. Yes, that would definitely be a better option!

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] [PATCH 3/3] MTD: m25p80: Add option to limit SPI transfer size.

[Brian Norris]

On Fri, May 01, 2015 at 12:58:52AM +1000, Julian Calaby wrote:
 On Thu, Apr 30, 2015 at 11:46 PM, Michal Suchanek hramr...@gmail.com wrote:
  My SPI driver does not support DMA so sizes larger than 64 bytes return
  an error. Add an option to limit transfer size so m25p80 can be used
  with reduced speed with such controller.
 
 I suspect that this will be rejected as the maximum transfer size is a
 property of the SPI controller, not the device and should be exported
 to the device driver from the controller, not specified in the
 device's binding.

Absolutely correct. This does not belong in the device tree.

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 2/3] MTD: spi-nor: check for short writes in spi_nor_write.

[Brian Norris]

+ linux-spi, Mark

On Thu, Apr 30, 2015 at 03:38:50PM +0200, Michal Suchanek wrote:
 My SPI controller driver does not support DMA so writes are truncated to
 FIFO size.

Which SPI master driver?

 Check the amount of data actually written by the driver.

I'm not sure if we should just reactively use the retlen, or if we
should be communicating such a limitation via the SPI API. Thoughts?

Brian

 Signed-off-by: Michal Suchanek hramr...@gmail.com
 ---
  drivers/mtd/spi-nor/spi-nor.c | 42 +-
  1 file changed, 17 insertions(+), 25 deletions(-)
 
 diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
 index 14a5d23..75904b5 100644
 --- a/drivers/mtd/spi-nor/spi-nor.c
 +++ b/drivers/mtd/spi-nor/spi-nor.c
 @@ -807,7 +807,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, 
 size_t len,
   size_t *retlen, const u_char *buf)
  {
   struct spi_nor *nor = mtd_to_spi_nor(mtd);
 - u32 page_offset, page_size, i;
 + u32 page_offset, page_remainder, page_size, i;
   int ret;
  
   dev_dbg(nor-dev, to 0x%08x, len %zd\n, (u32)to, len);
 @@ -816,36 +816,28 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t 
 to, size_t len,
   if (ret)
   return ret;
  
 - write_enable(nor);
 -
 - page_offset = to  (nor-page_size - 1);
 -
 - /* do all the bytes fit onto one page? */
 - if (page_offset + len = nor-page_size) {
 - nor-write(nor, to, len, retlen, buf);
 - } else {
 - /* the size of data remaining on the first page */
 - page_size = nor-page_size - page_offset;
 - nor-write(nor, to, page_size, retlen, buf);
 + /* write everything in nor-page_size chunks */
 + /* check the size of data actually written */
 + for (i = 0; i  len; i += *retlen) {
 + page_size = len - i;
 + page_offset = (to + i)  (nor-page_size - 1);
 + page_remainder = nor-page_size - page_offset;
 + if (page_size  nor-page_size)
 + page_size = nor-page_size;
 + if (page_remainder  (page_size  page_remainder))
 + page_size = page_remainder;
  
 - /* write everything in nor-page_size chunks */
 - for (i = page_size; i  len; i += page_size) {
 - page_size = len - i;
 - if (page_size  nor-page_size)
 - page_size = nor-page_size;
 -
 - ret = spi_nor_wait_till_ready(nor);
 - if (ret)
 - goto write_err;
 + write_enable(nor);
  
 - write_enable(nor);
 + nor-write(nor, to + i, page_size, retlen, buf + i);
  
 - nor-write(nor, to + i, page_size, retlen, buf + i);
 - }
 + ret = spi_nor_wait_till_ready(nor);
 + if (ret)
 + goto write_err;
   }
  
 - ret = spi_nor_wait_till_ready(nor);
  write_err:
 + *retlen = i;
   spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE);
   return ret;
  }

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 1/2] mtd: nand: add sunxi NAND flash controller support

[Brian Norris]

On Wed, Oct 29, 2014 at 02:02:51PM +0100, Boris Brezillon wrote:
 On Tue, 28 Oct 2014 11:13:11 -0700 Brian Norris computersforpe...@gmail.com 
 wrote:
  On Tue, Oct 21, 2014 at 03:08:41PM +0200, Boris Brezillon wrote:
   +static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
   +struct nand_ecc_ctrl *ecc,
   +struct device_node *np)
   +{
   + struct nand_ecclayout *layout;
   + int i, j;
   + int ret;
   +
   + ret = sunxi_nand_hw_common_ecc_ctrl_init(mtd, ecc, np);
   + if (ret)
   + return ret;
   +
   + ecc-read_page = sunxi_nfc_hw_ecc_read_page;
   + ecc-write_page = sunxi_nfc_hw_ecc_write_page;
   + layout = ecc-layout;
   +
   + for (i = 0; i  ecc-steps; i++) {
  
  Hmm, did you retest this since changing this to ecc-steps? I think this
  won't work, since ecc-steps is only initialized in nand_scan_tail(),
  which comes after this. I only recommended the change for the
  {read,write}_page() type of functions, not the initialization functions.
  
  [snip the rest]
  
  So I'd suggest the following additional patch, and otherwise, the series
  is fine with me. If you give your ack, I can just squash this in and
  apply it to my tree:
 
 It works, you can squash those changes into my first patch.

OK, pushed both (+ my fixes) to l2-mtd.git. Thanks!

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 1/2] mtd: nand: add sunxi NAND flash controller support

[Brian Norris]

Hi Boris,

On Tue, Oct 21, 2014 at 03:08:41PM +0200, Boris Brezillon wrote:
 +static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 +struct nand_ecc_ctrl *ecc,
 +struct device_node *np)
 +{
 + struct nand_ecclayout *layout;
 + int i, j;
 + int ret;
 +
 + ret = sunxi_nand_hw_common_ecc_ctrl_init(mtd, ecc, np);
 + if (ret)
 + return ret;
 +
 + ecc-read_page = sunxi_nfc_hw_ecc_read_page;
 + ecc-write_page = sunxi_nfc_hw_ecc_write_page;
 + layout = ecc-layout;
 +
 + for (i = 0; i  ecc-steps; i++) {

Hmm, did you retest this since changing this to ecc-steps? I think this
won't work, since ecc-steps is only initialized in nand_scan_tail(),
which comes after this. I only recommended the change for the
{read,write}_page() type of functions, not the initialization functions.

[snip the rest]

So I'd suggest the following additional patch, and otherwise, the series
is fine with me. If you give your ack, I can just squash this in and
apply it to my tree:

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index ea615dc442a4..ccaa8e283388 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -931,6 +931,7 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct 
mtd_info *mtd,
struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand-nand.controller);
struct sunxi_nand_hw_ecc *data;
struct nand_ecclayout *layout;
+   int nsectors;
int ret;
int i;
 
@@ -959,13 +960,14 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct 
mtd_info *mtd,
ecc-bytes = ALIGN(ecc-bytes, 2);
 
layout = data-layout;
+   nsectors = mtd-writesize / ecc-size;
 
-   if (mtd-oobsize  ((ecc-bytes + 4) * ecc-steps)) {
+   if (mtd-oobsize  ((ecc-bytes + 4) * nsectors)) {
ret = -EINVAL;
goto err;
}
 
-   layout-eccbytes = (ecc-bytes * ecc-steps);
+   layout-eccbytes = (ecc-bytes * nsectors);
 
ecc-layout = layout;
ecc-priv = data;
@@ -988,6 +990,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
   struct device_node *np)
 {
struct nand_ecclayout *layout;
+   int nsectors;
int i, j;
int ret;
 
@@ -998,8 +1001,9 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info 
*mtd,
ecc-read_page = sunxi_nfc_hw_ecc_read_page;
ecc-write_page = sunxi_nfc_hw_ecc_write_page;
layout = ecc-layout;
+   nsectors = mtd-writesize / ecc-size;
 
-   for (i = 0; i  ecc-steps; i++) {
+   for (i = 0; i  nsectors; i++) {
if (i) {
layout-oobfree[i].offset =
layout-oobfree[i - 1].offset +
@@ -1022,13 +1026,13 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info 
*mtd,
layout-oobfree[i].length + j;
}
 
-   if (mtd-oobsize  (ecc-bytes + 4) * ecc-steps) {
-   layout-oobfree[ecc-steps].offset =
-   layout-oobfree[ecc-steps - 1].offset +
-   layout-oobfree[ecc-steps - 1].length +
+   if (mtd-oobsize  (ecc-bytes + 4) * nsectors) {
+   layout-oobfree[nsectors].offset =
+   layout-oobfree[nsectors - 1].offset +
+   layout-oobfree[nsectors - 1].length +
ecc-bytes;
-   layout-oobfree[ecc-steps].length = mtd-oobsize -
-   ((ecc-bytes + 4) * ecc-steps);
+   layout-oobfree[nsectors].length = mtd-oobsize -
+   ((ecc-bytes + 4) * nsectors);
}
 
return 0;
@@ -1039,6 +1043,7 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct 
mtd_info *mtd,
struct device_node *np)
 {
struct nand_ecclayout *layout;
+   int nsectors;
int i;
int ret;
 
@@ -1051,8 +1056,9 @@ static int sunxi_nand_hw_syndrome_ecc_ctrl_init(struct 
mtd_info *mtd,
ecc-write_page = sunxi_nfc_hw_syndrome_ecc_write_page;
 
layout = ecc-layout;
+   nsectors = mtd-writesize / ecc-size;
 
-   for (i = 0; i  (ecc-bytes * ecc-steps); i++)
+   for (i = 0; i  (ecc-bytes * nsectors); i++)
layout-eccpos[i] = i;
 
layout-oobfree[0].length = mtd-oobsize - i;

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v6 1/2] mtd: nand: add sunxi NAND flash controller support

[Brian Norris]

On Mon, Oct 20, 2014 at 01:45:19PM +0200, Boris Brezillon wrote:
 Add support for the sunxi NAND Flash Controller (NFC).
 
 Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com

This driver looks mostly good. Sorry for the delays, and thanks for the
patience.

 ---
  drivers/mtd/nand/Kconfig  |6 +
  drivers/mtd/nand/Makefile |1 +
  drivers/mtd/nand/sunxi_nand.c | 1400 
 +
  3 files changed, 1407 insertions(+)
  create mode 100644 drivers/mtd/nand/sunxi_nand.c
 
 diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
 index dd10646..4c51d2c 100644
 --- a/drivers/mtd/nand/Kconfig
 +++ b/drivers/mtd/nand/Kconfig
 @@ -516,4 +516,10 @@ config MTD_NAND_XWAY
 Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is 
 attached
 to the External Bus Unit (EBU).
  
 +config MTD_NAND_SUNXI
 + tristate Support for NAND on Allwinner SoCs
 + depends on ARCH_SUNXI
 + help
 +   Enables support for NAND Flash chips on Allwinner SoCs.
 +
  endif # MTD_NAND
 diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
 index 9c847e4..bd38f21 100644
 --- a/drivers/mtd/nand/Makefile
 +++ b/drivers/mtd/nand/Makefile
 @@ -50,5 +50,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740)   += jz4740_nand.o
  obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/
  obj-$(CONFIG_MTD_NAND_XWAY)  += xway_nand.o
  obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/
 +obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
  
  nand-objs := nand_base.o nand_bbt.o nand_timings.o
 diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
 new file mode 100644
 index 000..c4e0559
 --- /dev/null
 +++ b/drivers/mtd/nand/sunxi_nand.c
 @@ -0,0 +1,1400 @@
 +/*
 + * Copyright (C) 2013 Boris BREZILLON b.brezillon@gmail.com
 + *
 + * Derived from:
 + *   https://github.com/yuq/sunxi-nfc-mtd
 + *   Copyright (C) 2013 Qiang Yu yuq...@gmail.com
 + *
 + *   https://github.com/hno/Allwinner-Info
 + *   Copyright (C) 2013 Henrik Nordström Henrik Nordström
 + *
 + *   Copyright (C) 2013 Dmitriy B. rzk...@gmail.com
 + *   Copyright (C) 2013 Sergey Lapin sla...@ossfans.org
 + *
 + * 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.
 + */
 +
 +#define pr_fmt(fmt)  KBUILD_MODNAME :  fmt
 +
 +#include linux/dma-mapping.h
 +#include linux/slab.h
 +#include linux/module.h
 +#include linux/moduleparam.h
 +#include linux/platform_device.h
 +#include linux/of.h
 +#include linux/of_device.h
 +#include linux/of_gpio.h
 +#include linux/of_mtd.h
 +#include linux/mtd/mtd.h
 +#include linux/mtd/nand.h
 +#include linux/mtd/partitions.h
 +#include linux/clk.h
 +#include linux/delay.h
 +#include linux/dmaengine.h
 +#include linux/gpio.h
 +#include linux/interrupt.h
 +#include linux/io.h
 +
 +#define NFC_REG_CTL  0x
 +#define NFC_REG_ST   0x0004
 +#define NFC_REG_INT  0x0008
 +#define NFC_REG_TIMING_CTL   0x000C
 +#define NFC_REG_TIMING_CFG   0x0010
 +#define NFC_REG_ADDR_LOW 0x0014
 +#define NFC_REG_ADDR_HIGH0x0018
 +#define NFC_REG_SECTOR_NUM   0x001C
 +#define NFC_REG_CNT  0x0020
 +#define NFC_REG_CMD  0x0024
 +#define NFC_REG_RCMD_SET 0x0028
 +#define NFC_REG_WCMD_SET 0x002C
 +#define NFC_REG_IO_DATA  0x0030
 +#define NFC_REG_ECC_CTL  0x0034
 +#define NFC_REG_ECC_ST   0x0038
 +#define NFC_REG_DEBUG0x003C
 +#define NFC_REG_ECC_CNT0 0x0040
 +#define NFC_REG_ECC_CNT1 0x0044
 +#define NFC_REG_ECC_CNT2 0x0048
 +#define NFC_REG_ECC_CNT3 0x004c
 +#define NFC_REG_USER_DATA_BASE   0x0050
 +#define NFC_REG_SPARE_AREA   0x00A0
 +#define NFC_RAM0_BASE0x0400
 +#define NFC_RAM1_BASE0x0800
 +
 +/* define bit use in NFC_CTL */
 +#define NFC_EN   BIT(0)
 +#define NFC_RESETBIT(1)
 +#define NFC_BUS_WIDYHBIT(2)
 +#define NFC_RB_SEL   BIT(3)
 +#define NFC_CE_SEL   GENMASK(26, 24)
 +#define NFC_CE_CTL   BIT(6)
 +#define NFC_CE_CTL1  BIT(7)
 +#define NFC_PAGE_SIZEGENMASK(11, 8)
 +#define NFC_SAM  BIT(12)
 +#define NFC_RAM_METHOD   BIT(14)
 +#define NFC_DEBUG_CTLBIT(31)
 +
 +/* define bit use in NFC_ST */
 +#define NFC_RB_B2R   BIT(0)
 +#define NFC_CMD_INT_FLAG BIT(1)
 +#define NFC_DMA_INT_FLAG BIT(2)
 +#define NFC_CMD_FIFO_STATUS  BIT(3)
 +#define NFC_STA   

[linux-sunxi] Re: [PATCH v6 2/2] mtd: nand: add sunxi NFC dt bindings doc

[Brian Norris]

Hi Boris,

On Mon, Oct 20, 2014 at 01:45:20PM +0200, Boris Brezillon wrote:
 Add the sunxi NAND Flash Controller dt bindings documentation.
 
 Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com
 ---
  .../devicetree/bindings/mtd/sunxi-nand.txt | 45 
 ++
  1 file changed, 45 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt
 
 diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt 
 b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
 new file mode 100644
 index 000..0273adb
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
 @@ -0,0 +1,45 @@
 +Allwinner NAND Flash Controller (NFC)
 +
 +Required properties:
 +- compatible : allwinner,sun4i-a10-nand.
 +- reg : shall contain registers location and length for data and reg.
 +- interrupts : shall define the nand controller interrupt.
 +- #address-cells: shall be set to 1. Encode the nand CS.
 +- #size-cells : shall be set to 0.
 +- clocks : shall reference nand controller clocks.
 +- clock-names : nand controller internal clock names. Shall contain :
 +* ahb : AHB gating clock
 +* mod : nand controller clock
 +
 +Optional children nodes:
 +Children nodes represent the available nand chips.
 +
 +Optional properties:
 +- allwinner,rb : shall contain the native Ready/Busy ids.
 + or
 +- rb-gpios : shall contain the gpios used as R/B pins.

I think you're relying on a named GPIO in your driver (nand-rb). That
should be documented here.

 +- nand-ecc-mode : one of the supported ECC modes (hw, hw_syndrome, 
 soft,
 +  soft_bch or none)

I think you're utilizing an undocumented 'nand-name' property for this
node in your driver too. Please document it. (That also goes for any
other undocumented properties I may have missed.)

 +
 +see Documentation/devicetree/mtd/nand.txt for generic bindings.
 +
 +
 +Examples:
 +nfc: nand@01c03000 {
 + compatible = allwinner,sun4i-a10-nand;
 + reg = 0x01c03000 0x1000;
 + interrupts = 0 37 1;
 + clocks = ahb_gates 13, nand_clk;
 + clock-names = ahb, mod;
 + #address-cells = 1;
 + #size-cells = 0;
 + pinctrl-names = default;
 + pinctrl-0 = nand_pins_a nand_cs0_pins_a nand_rb0_pins_a;
 + status = okay;
 +
 + nand@0 {
 + reg = 0;
 + allwinner,rb = 0;
 + nand-ecc-mode = soft_bch;
 + };
 +};

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 1/2] mtd: nand: support ONFI timing mode retrieval for non-ONFI NANDs

[Brian Norris]

On Mon, Sep 22, 2014 at 04:25:10PM +0200, Boris BREZILLON wrote:
 Add an onfi_timing_mode_default field to nand_chip and nand_flash_dev in
 order to support NAND timings definition for non-ONFI NAND.
 
 NAND that support better timings mode than the default one have to define
 a new entry in the nand_ids table.
 
 The default timing mode should be deduced from timings description from
 the datasheet and the ONFI specification
 (www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf, chapter 4.15
 Timing Parameters).
 You should choose the closest mode that fit the timings requirements of
 your NAND chip.
 
 Signed-off-by: Boris BREZILLON boris.brezil...@free-electrons.com

You have some (new?) checkpatch warnings:

WARNING: please, no space before tabs
#49: FILE: include/linux/mtd/nand.h:591:
+ * ^I^I^I  either deduced from the datasheet if the NAND$

WARNING: please, no space before tabs
#50: FILE: include/linux/mtd/nand.h:592:
+ * ^I^I^I  chip is not ONFI compliant or set to 0 if it is$

WARNING: please, no space before tabs
#51: FILE: include/linux/mtd/nand.h:593:
+ * ^I^I^I  (an ONFI chip is always configured in mode 0$

WARNING: please, no space before tabs
#52: FILE: include/linux/mtd/nand.h:594:
+ * ^I^I^I  after a NAND reset)$

total: 0 errors, 4 warnings, 43 lines checked

Your patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

Brian

 ---
  drivers/mtd/nand/nand_base.c |  2 ++
  include/linux/mtd/nand.h | 11 +++
  2 files changed, 13 insertions(+)
 
 diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
 index ae6e7c4..c37fa2a 100644
 --- a/drivers/mtd/nand/nand_base.c
 +++ b/drivers/mtd/nand/nand_base.c
 @@ -3594,6 +3594,8 @@ static bool find_full_id_nand(struct mtd_info *mtd, 
 struct nand_chip *chip,
   chip-options |= type-options;
   chip-ecc_strength_ds = NAND_ECC_STRENGTH(type);
   chip-ecc_step_ds = NAND_ECC_STEP(type);
 + chip-onfi_timing_mode_default =
 + type-onfi_timing_mode_default;
  
   *busw = type-options  NAND_BUSWIDTH_16;
  
 diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
 index b7c1199..e795fbf 100644
 --- a/include/linux/mtd/nand.h
 +++ b/include/linux/mtd/nand.h
 @@ -587,6 +587,11 @@ struct nand_buffers {
   * @ecc_step_ds: [INTERN] ECC step required by the @ecc_strength_ds,
   *  also from the datasheet. It is the recommended ECC 
 step
   *   size, if known; if unknown, set to zero.
 + * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field 
 is
 + * either deduced from the datasheet if the NAND
 + * chip is not ONFI compliant or set to 0 if it is
 + * (an ONFI chip is always configured in mode 0
 + * after a NAND reset)
   * @numchips:[INTERN] number of physical chips
   * @chipsize:[INTERN] the size of one chip for multichip 
 arrays
   * @pagemask:[INTERN] page number mask = number of (pages / 
 chip) - 1
 @@ -671,6 +676,7 @@ struct nand_chip {
   uint8_t bits_per_cell;
   uint16_t ecc_strength_ds;
   uint16_t ecc_step_ds;
 + int onfi_timing_mode_default;
   int badblockpos;
   int badblockbits;
  
 @@ -773,6 +779,10 @@ struct nand_chip {
   *   @ecc_step_ds in nand_chip{}, also from the datasheet.
   *   For example, the 4bit ECC for each 512Byte can be set with
   *   NAND_ECC_INFO(4, 512).
 + * @onfi_timing_mode_default: the default ONFI timing mode entered after a 
 NAND
 + * reset. Should be deduced from timings described
 + * in the datasheet.
 + *
   */
  struct nand_flash_dev {
   char *name;
 @@ -793,6 +803,7 @@ struct nand_flash_dev {
   uint16_t strength_ds;
   uint16_t step_ds;
   } ecc;
 + int onfi_timing_mode_default;
  };
  
  /**
 -- 
 1.9.1
 

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 1/2] mtd: nand: support ONFI timing mode retrieval for non-ONFI NANDs

[Brian Norris]

 diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
 index b7c1199..b0b74cc 100644
 --- a/include/linux/mtd/nand.h
 +++ b/include/linux/mtd/nand.h
 @@ -587,6 +587,11 @@ struct nand_buffers {
   * @ecc_step_ds: [INTERN] ECC step required by the @ecc_strength_ds,
   *  also from the datasheet. It is the recommended ECC 
 step
   *   size, if known; if unknown, set to zero.
 + * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field 
 is
 + * either deduced from the datasheet if the NAND
 + * chip is not ONFI compliant or set to 0 if it is
 + * (an ONFI chip is always configured in mode 0
 + * after a NAND reset)

This is probably OK only if every NAND chip is at least as fast as ONFI
mode 0. For older / legacy flash, I'm not sure if that's 100% true.
Maybe we'll need an UNKNOWN value, for those whose timing information is
not known?

Anyway, I think this is OK for now. Pushed the series to l2-mtd.git.
Thanks!

   * @numchips:[INTERN] number of physical chips
   * @chipsize:[INTERN] the size of one chip for multichip 
 arrays
   * @pagemask:[INTERN] page number mask = number of (pages / 
 chip) - 1

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 0/2] mtd: nand: add sunxi NAND flash controller support

[Brian Norris]

Hi Boris,

On Mon, Aug 18, 2014 at 07:26:26PM +0200, Boris BREZILLON wrote:
 This patch series adds support for the sunxi NAND Flash Controller (NFC)
 block.
 
 These two patches only add support for the basic NAND stuff:
  - NAND controller operations
  - SW and HW ECC handling (with both syndrome and normal ECC scheme)
 
 If you want support for advanced features you can find it on my github
 repo [1]:
  - HW randomization support
  - per partition ECC/Randomizer to handle bootloader partitions
 
 DMA transfers are not supported yet, but I have reworked the OOB layout
 when using the HW ECC scheme to match the one used when accessing the NAND
 with DMA transfers (the available OOB bytes are placed at the end of the
 OOB area).
 
 This patch series depends on this other one [2] which adds support for ONFI
 timing mode retrieval on non-ONFI NANDs.

Were you planning to send v2 of your series [2]? I see you made a
mistake you were planning on fixing:

  https://lkml.org/lkml/2014/7/30/423

Brian

 Best Regards,
 
 Boris
 
 [1]https://github.com/bbrezillon/linux-sunxi/tree/sunxi-nand-v4
 [2]https://lkml.org/lkml/2014/7/28/156
 
 Changes since v3:
  - removed nand core code modifications from the patch series (submitted
separately)
  - added documentation to the code
  - forced timeout (a default timeout is used when none is provided by the
caller) on controller operations
  - fixed coding style issues
  - removed unneeded irq field from the sunxi_nfc struct
  - fixed several memory leaks
  - reworked the NFC reset code (to avoid potential garbage config from the
bootloader)
  - made use of ECC_EXCEPTION flag to prevent erased page from generating
ECC errors
  - changed the OOB layout for HW ECC scheme
 
 Changes since v2:
  - merge HW ECC implementation in base implementation patch
  - fix timing config when interfacing with an ONFI compatible chip
 
 Changes since v1:
  - add HW ECC support
  - rework NAND timings retrieval (use ONFI timing mode instead of raw timings)
  - add nand-ecc-level property to specify NAND ECC requirements from DT
 
 Boris BREZILLON (2):
   mtd: nand: add sunxi NAND flash controller support
   mtd: nand: add sunxi NFC dt bindings doc
 
  .../devicetree/bindings/mtd/sunxi-nand.txt |   45 +
  drivers/mtd/nand/Kconfig   |6 +
  drivers/mtd/nand/Makefile  |1 +
  drivers/mtd/nand/sunxi_nand.c  | 1362 
 
  4 files changed, 1414 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt
  create mode 100644 drivers/mtd/nand/sunxi_nand.c
 

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 2/9] mtd: nand: add ONFI timing mode to nand_timings converter

[Brian Norris]

Hi Boris,

Since I think you were planning on revisiting this soon, I have one more
comment:

On Wed, Mar 12, 2014 at 07:07:37PM +0100, Boris BREZILLON wrote:
 diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
 index c970ce7..0b8a822 100644
 --- a/drivers/mtd/nand/Makefile
 +++ b/drivers/mtd/nand/Makefile
 @@ -2,7 +2,7 @@
  # linux/drivers/nand/Makefile
  #
  
 -obj-$(CONFIG_MTD_NAND)   += nand.o
 +obj-$(CONFIG_MTD_NAND)   += nand.o nand_timings.o

This is not the right place, as it will generate a new module
'nand_timings' (nand_timings.ko, if CONFIG_MTD_NAND=m). You probably
want to just add nand_timings.o to the 'nand-objs' list at the bottom of
the Makefile, like this:

nand-objs := nand_base.o nand_bbt.o nand_timings.o

  obj-$(CONFIG_MTD_NAND_ECC)   += nand_ecc.o
  obj-$(CONFIG_MTD_NAND_BCH)   += nand_bch.o
  obj-$(CONFIG_MTD_NAND_IDS)   += nand_ids.o nand_hynix.o

Thanks,
Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

[Brian Norris]

Hi Boris,

Looking back at this thread, there's at least one or two things I forgot
to answer. Sorry.

On Tue, May 20, 2014 at 11:32:04PM +0200, Boris BREZILLON wrote:
 On 20/05/2014 21:52, Brian Norris wrote:
[...]
 If the ECC bindings don't encode the minimum required ECC strength but
 rather the ECC config on a specific board then I guess minimum
 required ECC strength for non-ONFI chips should be defined somewhere
 else (stored in the device ID table ?).

They are. See nand_flash_dev::ecc, which holds fields for
ecc_strength_ds and step_ds. If we have to, we can add a timing mode
field to this struct.

  So you're saying that even though the chip actually specifies a single
  set of timings, you would describe this as a bitmask of several
  supported ONFI timing modes, up to the max performance?
 
  Is there ever a case where (for instance) a non-ONFI flash supports the
  equivalent of timing mode 3, but it does not support mode 2 or 1?
 
 I don't think so.

OK, then I don't think the mask approach is necessary, if we do ever
settle on using a DT binding here. (I hope we can avoid this.)

  But I can modify the bindings to just encode the maximum supported
  timing mode.
  AIUI, the non-ONFI datasheets really only specify a single timing mode,
  so I think we should only specify the max. And as a bonus, this
  actually makes the binding easier to use. A driver does not care about
  how many different modes are supported; it only needs to know what the
  max is.
 
 Agreed, actually my first binding was defining it this way.

Was there a good reason for changing it?

Thanks,
Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support

[Brian Norris]

On Tue, May 20, 2014 at 11:49:42AM -0700, Brian Norris wrote:
 On Fri, May 09, 2014 at 06:47:22PM +0200, Boris BREZILLON wrote:
  On 09/05/2014 18:03, Ezequiel Garcia wrote:
   On 12 Mar 07:07 PM, Boris BREZILLON wrote:
   --- /dev/null
   +++ b/drivers/mtd/nand/sunxi_nand.c
   @@ -0,0 +1,1276 @@
 ...
   +static int sunxi_nand_ecc_init(struct mtd_info *mtd, struct 
   nand_ecc_ctrl *ecc,
   +   struct device_node *np)
   +{
   +struct nand_chip *nand = mtd-priv;
   +int ecc_step_size, ecc_strength;
   +int ret;
   +
   +ecc_step_size = of_get_nand_ecc_step_size(np);
   +ecc_strength = of_get_nand_ecc_strength(np);
   +if (ecc_step_size  0  ecc_strength  0) {
   +ecc-size = ecc_step_size;
   +ecc-strength = ecc_strength;
   +} else {
   +ecc-size = nand-ecc_step_ds;
   +ecc-strength = nand-ecc_strength_ds;
   +}
   +
   Shouldn't you check the devicetree value is not weaker than the 
   ONFI-obtained?
  
  I can definitely do that.
 
 You can do that here, but take a look at the discussion Ezequiel and I
 had about this:
 
   http://thread.gmane.org/gmane.linux.drivers.devicetree/67462
 
 We probably don't want to be doing anything drastic like overriding the
 device tree configuration. Instead, we might want to stick a warning
 into the core nand_base code that does the proper comparison of the
 '*_ds' values with the actual values chosen in
 chip-ecc-{size,strength}. The comparison is kind of subtle, actually,
 so it'd be good to do it exactly once for everyone.

I forgot, Ezequiel already submitted this. I'll look at it soon:

  http://patchwork.ozlabs.org/patch/348901/

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

[Brian Norris]

On Tue, May 20, 2014 at 09:30:33PM +0200, Boris BREZILLON wrote:
 Hi Brian,
 
 On 20/05/2014 20:25, Brian Norris wrote:
  Hi Boris,
 
  On Wed, Mar 12, 2014 at 07:07:39PM +0100, Boris BREZILLON wrote:
  Add documentation for the ONFI NAND timing mode property.
 
  Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
  ---
   Documentation/devicetree/bindings/mtd/nand.txt |8 
   1 file changed, 8 insertions(+)
 
  diff --git a/Documentation/devicetree/bindings/mtd/nand.txt 
  b/Documentation/devicetree/bindings/mtd/nand.txt
  index b53f92e..2046027 100644
  --- a/Documentation/devicetree/bindings/mtd/nand.txt
  +++ b/Documentation/devicetree/bindings/mtd/nand.txt
  @@ -19,3 +19,11 @@ errors per {size} bytes.
   The interpretation of these parameters is implementation-defined, so not 
  all
   implementations must support all possible combinations. However, 
  implementations
   are encouraged to further specify the value(s) they support.
  +
  +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing 
  modes of
  +  the NAND chip. Each supported mode is represented as a bit position 
  (i.e. :
  +  mode 0 and 1 = (1  0) | (1  1) = 0x3).
  +  This is only used when the chip does not support the ONFI standard.
  +  The last bit set represent the closest mode fulfilling the NAND chip 
  timings.
  +  For a full description of the different timing modes see this document:
  +  www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
  I'm not 100% convinced this property should go in the device tree. With
  most other flash properties (device size, page size, and even minimum
  ECC requirements), we try to auto-detect these parameters to some
  extent. ONFI makes it easy for some class of chips, but for others, we
  typically rely on an in-kernel device ID table or ID decoding heuristic
  -- we don't require a DT description of every property of the flash. So
  what makes this property different?
 
 AFAICT nothing, but the same goes for the ECC requirements, and we've
 recently added DT bindings to define these requirements.
 I'm not telling we should drop these ECC requirements bindings (actually
 I'm using them :-)), but what's different with the timings requirements ?

ECC selection is not quite as scientific; with ECC, there are external
factors that influence the ECC mode that you should use, since any data
read/written from Linux has to be compatible with any data read/written
with another entity (e.g., bootloader). Note that the ECC bindings do
not represent a property of the flash chip itself (i.e., they don't hold
the minimum required ECC strength), but of the entire flash system
(i.e., what ECC must I use to play nicely with the rest of the world).

With timing modes, this is purely a property of the flash chip, and we
do not have to synchronize it with the bootloader. We don't exactly care
if a bootloader and Linux use slightly different timing modes.

 Moreover, we will end up with a lot of new entries in the device ID
 table if we decide to put these informations in this table.

Yes, that could be a problem.

What sort of non-ONFI flash chips do you have that need this property?
And what timing mode(s) do they use? Is there, for instance, a pattern
such that all Hynix MLC of a certain generation use a particular timing
mode?

  I realize that we may not include device ID entries for every flash that
  you need in the ID table (although we still are able to detect the
  important properties accurately, like page and block size). But would it
  suffice to default these flash to a lowest common timing mode, like mode
  0?
 
 IMHO this is not a good solution: you'll end up with lower perfomances
 on most of the supported NAND chips and I'm not sure this is what we want.

No, we wouldn't want to always use mode 0. But it's possible we can get
good enough heuristics for most flash, if we can integrate timing modes
into the current extended ID decoding. Not sure.

I'm also concerned here that this kind of binding will be difficult to
use properly. A user/developer/board-designer would have to read the
datasheet and compare all its values to the ONFI spec to find the
closest match, and they would have to do this for each new flash they
use. If we can help them by autodetecting this, that would be great.

  If no other option works well, then I am still open to describing the
  supported timing modes in the DT.
 
  BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash
  typically support multiple timing modes? And if so, how are we supposed
  to *change* modes? AFAIK, ONFI provides the only standard for
  configuring the flash's timing mode. So maybe you're really only wanting
  a default timing mode property that is a single integer, not a
  bitfield.
 
 Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell
 me if I'm wrong), it should work because most of the timings are min
 requirements.
 This means, even if you provide slower signals

[linux-sunxi] Re: [PATCH v3 1/9] mtd: nand: define struct nand_timings

[Brian Norris]

Hi Boris,

On Wed, Mar 12, 2014 at 07:07:36PM +0100, Boris BREZILLON wrote:
 Define a struct containing the standard NAND timings as described in NAND
 datasheets.
 
 Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
 ---
  include/linux/mtd/nand.h |   49 
 ++
  1 file changed, 49 insertions(+)
 
 diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
 index 389b3c5..f3ff3a3 100644
 --- a/include/linux/mtd/nand.h
 +++ b/include/linux/mtd/nand.h
 @@ -846,4 +846,53 @@ static inline bool nand_is_slc(struct nand_chip *chip)
  {
   return chip-bits_per_cell == 1;
  }
 +
 +/**
 + * struct nand_sdr_timings - SDR NAND chip timings
 + *
 + * This struct defines the timing requirements of a SDR NAND chip.
 + * These informations can be found in every NAND datasheets and the timings
 + * meaning are described in the ONFI specifications:
 + * www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf‎ (chapter 4.15 Timing

Can you remove the unicode U+200E character?

 + * Parameters)

Please document the units for these fields here. It looks like you're
using picoseconds.

 + *
 + */
 +

Extra blank line.

 +struct nand_sdr_timings {
 + u32 tALH_min;
 + u32 tADL_min;
 + u32 tALS_min;
 + u32 tAR_min;
 + u32 tCEA_max;
 + u32 tCEH_min;
 + u32 tCH_min;
 + u32 tCHZ_max;
 + u32 tCLH_min;
 + u32 tCLR_min;
 + u32 tCLS_min;
 + u32 tCOH_min;
 + u32 tCS_min;
 + u32 tDH_min;
 + u32 tDS_min;
 + u32 tFEAT_max;
 + u32 tIR_min;
 + u32 tITC_max;
 + u32 tRC_min;
 + u32 tREA_max;
 + u32 tREH_min;
 + u32 tRHOH_min;
 + u32 tRHW_min;
 + u32 tRHZ_max;
 + u32 tRLOH_min;
 + u32 tRP_min;
 + u32 tRR_min;
 + u64 tRST_max;
 + u32 tWB_max;
 + u32 tWC_min;
 + u32 tWH_min;
 + u32 tWHR_min;
 + u32 tWP_min;
 + u32 tWW_min;
 +};
 +
  #endif /* __LINUX_MTD_NAND_H */

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 2/9] mtd: nand: add ONFI timing mode to nand_timings converter

[Brian Norris]

Hi,

On Wed, Mar 12, 2014 at 07:07:37PM +0100, Boris BREZILLON wrote:
 Add a converter to retrieve NAND timings from an ONFI NAND timing mode.
 This only support SDR NAND timings for now.
 
 Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
 ---
  drivers/mtd/nand/Makefile   |2 +-
  drivers/mtd/nand/nand_timings.c |  248 
 +++
  include/linux/mtd/nand.h|4 +
  3 files changed, 253 insertions(+), 1 deletion(-)
  create mode 100644 drivers/mtd/nand/nand_timings.c
 
 diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
 index c970ce7..0b8a822 100644
 --- a/drivers/mtd/nand/Makefile
 +++ b/drivers/mtd/nand/Makefile
 @@ -2,7 +2,7 @@
  # linux/drivers/nand/Makefile
  #
  
 -obj-$(CONFIG_MTD_NAND)   += nand.o
 +obj-$(CONFIG_MTD_NAND)   += nand.o nand_timings.o
  obj-$(CONFIG_MTD_NAND_ECC)   += nand_ecc.o
  obj-$(CONFIG_MTD_NAND_BCH)   += nand_bch.o
  obj-$(CONFIG_MTD_NAND_IDS)   += nand_ids.o nand_hynix.o

This patch doesn't apply to l2-mtd.git. Are you basing this on your
Hynix patches? I thought some (all?) of them were determined
unnecessary. Anyway, please rebase on l2-mtd.git, since there are a
couple of other bits that don't apply properly.

 diff --git a/drivers/mtd/nand/nand_timings.c b/drivers/mtd/nand/nand_timings.c
 new file mode 100644
 index 000..f66fe95
 --- /dev/null
 +++ b/drivers/mtd/nand/nand_timings.c
 @@ -0,0 +1,248 @@
 +/*
 + *  Copyright (C) 2014 Boris BREZILLON b.brezillon@gmail.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + */
 +#include linux/mtd/nand.h
 +
 +static const struct nand_sdr_timings onfi_sdr_timings[] = {
 + /* Mode 0 */
 + {
 + .tADL_min = 20,
 + .tALH_min = 2,
 + .tALS_min = 5,
 + .tAR_min = 25000,
 + .tCEA_max = 10,
 + .tCEH_min = 2,
 + .tCH_min = 2,
 + .tCHZ_max = 10,
 + .tCLH_min = 2,
 + .tCLR_min = 2,
 + .tCLS_min = 5,
 + .tCOH_min = 0,
 + .tCS_min = 7,
 + .tDH_min = 2,
 + .tDS_min = 4,
 + .tFEAT_max = 100,
 + .tIR_min = 1,
 + .tITC_max = 100,
 + .tRC_min = 10,
 + .tREA_max = 4,
 + .tREH_min = 3,
 + .tRHOH_min = 0,
 + .tRHW_min = 20,
 + .tRHZ_max = 20,
 + .tRLOH_min = 0,
 + .tRP_min = 5,
 + .tRST_max = 2500,

I was initially wary of potential overflow here, but apparently the C
standard (section 6.4.4.1) ensures that literal constants like this will
be promoted to a sufficiently-large type.

 + .tWB_max = 20,
 + .tRR_min = 4,
 + .tWC_min = 10,
 + .tWH_min = 3,
 + .tWHR_min = 12,
 + .tWP_min = 5,
 + .tWW_min = 10,
 + },
 + /* Mode 1 */
 + {
 + .tADL_min = 10,
 + .tALH_min = 1,
 + .tALS_min = 25000,
 + .tAR_min = 1,
 + .tCEA_max = 45000,
 + .tCEH_min = 2,
 + .tCH_min = 1,
 + .tCHZ_max = 5,
 + .tCLH_min = 1,
 + .tCLR_min = 1,
 + .tCLS_min = 25000,
 + .tCOH_min = 15000,
 + .tCS_min = 35000,
 + .tDH_min = 1,
 + .tDS_min = 2,
 + .tFEAT_max = 100,
 + .tIR_min = 0,
 + .tITC_max = 100,
 + .tRC_min = 5,
 + .tREA_max = 3,
 + .tREH_min = 15000,
 + .tRHOH_min = 15000,
 + .tRHW_min = 10,
 + .tRHZ_max = 10,
 + .tRLOH_min = 0,
 + .tRP_min = 25000,
 + .tRR_min = 2,
 + .tRST_max = 5,
 + .tWB_max = 10,
 + .tWC_min = 45000,
 + .tWH_min = 15000,
 + .tWHR_min = 8,
 + .tWP_min = 25000,
 + .tWW_min = 10,
 + },
 + /* Mode 2 */
 + {
 + .tADL_min = 10,
 + .tALH_min = 1,
 + .tALS_min = 15000,
 + .tAR_min = 1,
 + .tCEA_max = 3,
 + .tCEH_min = 2,
 + .tCH_min = 1,
 + .tCHZ_max = 5,
 + .tCLH_min = 1,
 + .tCLR_min = 1,
 + .tCLS_min = 15000,
 + .tCOH_min = 15000,
 + .tCS_min = 25000,
 + .tDH_min = 5000,
 + .tDS_min = 15000,
 + 

[linux-sunxi] Re: [PATCH v3 3/9] of: mtd: add NAND timing mode retrieval support

[Brian Norris]

On Wed, Mar 12, 2014 at 07:07:38PM +0100, Boris BREZILLON wrote:
 Add a function to retrieve NAND timing mode (ONFI timing mode) from a given
 DT node.
 
 Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
 ---
  drivers/of/of_mtd.c|   19 +++
  include/linux/of_mtd.h |8 
  2 files changed, 27 insertions(+)
 
 diff --git a/drivers/of/of_mtd.c b/drivers/of/of_mtd.c
 index b7361ed..8bdaa0b 100644
 --- a/drivers/of/of_mtd.c
 +++ b/drivers/of/of_mtd.c
 @@ -117,3 +117,22 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)
   return of_property_read_bool(np, nand-on-flash-bbt);
  }
  EXPORT_SYMBOL_GPL(of_get_nand_on_flash_bbt);
 +
 +/**
 + * of_get_nand_timings - Get nand timings for the given device_node
 + * @np:  Pointer to the given device_node
 + *
 + * return 0 on success errno other wise
 + */
 +int of_get_nand_onfi_timing_mode(struct device_node *np)
 +{
 + int err;
 + u32 mode;
 +
 + err = of_property_read_u32(np, onfi,nand-timing-mode, mode);
 + if (err)
 + return err;
 +
 + return mode;

To fit the style of the rest of this file (and to save some lines) this
might as well be:

return ret ? ret : mode;

 +}
 +EXPORT_SYMBOL_GPL(of_get_nand_onfi_timing_mode);
 diff --git a/include/linux/of_mtd.h b/include/linux/of_mtd.h
 index e266caa..c8310ae 100644
 --- a/include/linux/of_mtd.h
 +++ b/include/linux/of_mtd.h
 @@ -9,6 +9,8 @@
  #ifndef __LINUX_OF_MTD_H
  #define __LINUX_OF_MTD_H
  
 +#include linux/mtd/nand.h
 +

What's this header used for here? I think you can drop it.

  #ifdef CONFIG_OF_MTD
  
  #include linux/of.h
 @@ -17,6 +19,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);
  int of_get_nand_ecc_strength(struct device_node *np);
  int of_get_nand_bus_width(struct device_node *np);
  bool of_get_nand_on_flash_bbt(struct device_node *np);
 +int of_get_nand_onfi_timing_mode(struct device_node *np);
  
  #else /* CONFIG_OF_MTD */
  
 @@ -45,6 +48,11 @@ static inline bool of_get_nand_on_flash_bbt(struct 
 device_node *np)
   return false;
  }
  
 +static inline int of_get_nand_onfi_timing_mode(struct device_node *np)
 +{
 + return -ENOSYS;
 +}
 +
  #endif /* CONFIG_OF_MTD */
  
  #endif /* __LINUX_OF_MTD_H */

Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.