All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antoine Tenart <antoine.tenart@free-electrons.com>
To: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: thomas.petazzoni@free-electrons.com,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Antoine Tenart <antoine.tenart@free-electrons.com>,
	linux-kernel@vger.kernel.org, zmxu@marvell.com,
	linux-mtd@lists.infradead.org,
	ezequiel.garcia@free-electrons.com, jszhang@marvell.com,
	computersforpeace@gmail.com, dwmw2@infradead.org,
	linux-arm-kernel@lists.infradead.org,
	sebastian.hesselbarth@gmail.com
Subject: Re: [PATCH 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller
Date: Wed, 11 Feb 2015 17:33:43 +0100	[thread overview]
Message-ID: <20150211163343.GH11169@kwain> (raw)
In-Reply-To: <871tlx7dgu.fsf@free.fr>

Robert,

On Tue, Feb 10, 2015 at 08:50:41PM +0100, Robert Jarzmik wrote:
> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
> > On Tue, 27 Jan 2015 15:10:12 +0100
> > Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> >
> >> The nand controller on Marvell Berlin SoC reuse the pxa3xx nand driver
> >> as it quite close. The process of sending commands can be compared to
> >> the one of the Marvell armada 370: read and write commands are done in
> >> chunks.
> >> 
> >> But the Berlin nand controller has some other specificities which
> >> require some modifications of the pxa3xx nand driver:
> >> - there are no IRQ available so we need to poll the status register: we
> >>   have to use our own cmdfunc Berlin function, and early on the probing
> >>   function.
> >> - PAGEPROG are very different from the one used in the pxa3xx driver,
> >>   so we're using a specific process for this one
> >> - the SEQIN command is equivalent to a READ0 command
> >> - the RNDOUT command must be used to perform a read operation, and the
> >>   command is not NAND_CMD_RNDOUT
> >> - the ERASE1 command is specific (0xd060)
> >> 
> >> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> Given all the differences, the PIO mode instead of the current interrupt model,
> and the berlin specific functions (nand_start, ...), wouldn't it be an idea to :
>  - extract the common functions to mrvl-nand-lib.c
>  - the remaining would be in pxa3xx_nand.c
>  - the new code will be in berlin_nand.c
> 
> All these ifs per variant will add complexity to the current driver, won't they
> ?

Given the current state of this driver I believe this would be a better
idea to first rework it to use the nand framework properly. Then it will
be possible to have a look on what we have, to decide whether or not a
mrvl-nand-lib is needed.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: antoine.tenart@free-electrons.com (Antoine Tenart)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller
Date: Wed, 11 Feb 2015 17:33:43 +0100	[thread overview]
Message-ID: <20150211163343.GH11169@kwain> (raw)
In-Reply-To: <871tlx7dgu.fsf@free.fr>

Robert,

On Tue, Feb 10, 2015 at 08:50:41PM +0100, Robert Jarzmik wrote:
> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
> > On Tue, 27 Jan 2015 15:10:12 +0100
> > Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> >
> >> The nand controller on Marvell Berlin SoC reuse the pxa3xx nand driver
> >> as it quite close. The process of sending commands can be compared to
> >> the one of the Marvell armada 370: read and write commands are done in
> >> chunks.
> >> 
> >> But the Berlin nand controller has some other specificities which
> >> require some modifications of the pxa3xx nand driver:
> >> - there are no IRQ available so we need to poll the status register: we
> >>   have to use our own cmdfunc Berlin function, and early on the probing
> >>   function.
> >> - PAGEPROG are very different from the one used in the pxa3xx driver,
> >>   so we're using a specific process for this one
> >> - the SEQIN command is equivalent to a READ0 command
> >> - the RNDOUT command must be used to perform a read operation, and the
> >>   command is not NAND_CMD_RNDOUT
> >> - the ERASE1 command is specific (0xd060)
> >> 
> >> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> Given all the differences, the PIO mode instead of the current interrupt model,
> and the berlin specific functions (nand_start, ...), wouldn't it be an idea to :
>  - extract the common functions to mrvl-nand-lib.c
>  - the remaining would be in pxa3xx_nand.c
>  - the new code will be in berlin_nand.c
> 
> All these ifs per variant will add complexity to the current driver, won't they
> ?

Given the current state of this driver I believe this would be a better
idea to first rework it to use the nand framework properly. Then it will
be possible to have a look on what we have, to decide whether or not a
mrvl-nand-lib is needed.

Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Antoine Tenart <antoine.tenart@free-electrons.com>
To: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
	Antoine Tenart <antoine.tenart@free-electrons.com>,
	thomas.petazzoni@free-electrons.com, zmxu@marvell.com,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	ezequiel.garcia@free-electrons.com, jszhang@marvell.com,
	computersforpeace@gmail.com, dwmw2@infradead.org,
	linux-arm-kernel@lists.infradead.org,
	sebastian.hesselbarth@gmail.com
Subject: Re: [PATCH 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller
Date: Wed, 11 Feb 2015 17:33:43 +0100	[thread overview]
Message-ID: <20150211163343.GH11169@kwain> (raw)
In-Reply-To: <871tlx7dgu.fsf@free.fr>

Robert,

On Tue, Feb 10, 2015 at 08:50:41PM +0100, Robert Jarzmik wrote:
> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
> > On Tue, 27 Jan 2015 15:10:12 +0100
> > Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> >
> >> The nand controller on Marvell Berlin SoC reuse the pxa3xx nand driver
> >> as it quite close. The process of sending commands can be compared to
> >> the one of the Marvell armada 370: read and write commands are done in
> >> chunks.
> >> 
> >> But the Berlin nand controller has some other specificities which
> >> require some modifications of the pxa3xx nand driver:
> >> - there are no IRQ available so we need to poll the status register: we
> >>   have to use our own cmdfunc Berlin function, and early on the probing
> >>   function.
> >> - PAGEPROG are very different from the one used in the pxa3xx driver,
> >>   so we're using a specific process for this one
> >> - the SEQIN command is equivalent to a READ0 command
> >> - the RNDOUT command must be used to perform a read operation, and the
> >>   command is not NAND_CMD_RNDOUT
> >> - the ERASE1 command is specific (0xd060)
> >> 
> >> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> Given all the differences, the PIO mode instead of the current interrupt model,
> and the berlin specific functions (nand_start, ...), wouldn't it be an idea to :
>  - extract the common functions to mrvl-nand-lib.c
>  - the remaining would be in pxa3xx_nand.c
>  - the new code will be in berlin_nand.c
> 
> All these ifs per variant will add complexity to the current driver, won't they
> ?

Given the current state of this driver I believe this would be a better
idea to first rework it to use the nand framework properly. Then it will
be possible to have a look on what we have, to decide whether or not a
mrvl-nand-lib is needed.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2015-02-11 16:33 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-27 14:10 [PATCH 0/9] ARM: berlin: add nand support Antoine Tenart
2015-01-27 14:10 ` Antoine Tenart
2015-01-27 14:10 ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 1/9] mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0 Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 2/9] mtd: pxa3xx_nand: add a non mandatory ECC clock Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 15:50   ` Andrew Lunn
2015-01-27 15:50     ` Andrew Lunn
2015-01-27 15:50     ` Andrew Lunn
2015-01-28 14:14     ` Antoine Tenart
2015-01-28 14:14       ` Antoine Tenart
2015-01-28 14:14       ` Antoine Tenart
2015-02-08 19:55       ` Boris Brezillon
2015-02-08 19:55         ` Boris Brezillon
2015-02-08 19:55         ` Boris Brezillon
2015-01-28  3:35   ` Jisheng Zhang
2015-01-28  3:35     ` Jisheng Zhang
2015-01-28  3:35     ` Jisheng Zhang
2015-01-28 14:17     ` Antoine Tenart
2015-01-28 14:17       ` Antoine Tenart
2015-01-28 14:17       ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 3/9] mtd: pxa3xx_nand: set NDCR_PG_PER_BLK if page per block is 128 Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-02-08 20:00   ` Boris Brezillon
2015-02-08 20:00     ` Boris Brezillon
2015-02-08 20:00     ` Boris Brezillon
2015-01-27 14:10 ` [PATCH 4/9] mtd: pxa3xx_nand: add a default chunk size Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-02-08 20:15   ` Boris Brezillon
2015-02-08 20:15     ` Boris Brezillon
2015-02-08 20:15     ` Boris Brezillon
2015-02-08 20:18     ` Boris Brezillon
2015-02-08 20:18       ` Boris Brezillon
2015-02-08 20:18       ` Boris Brezillon
2015-01-27 14:10 ` [PATCH 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:42   ` Ezequiel Garcia
2015-01-27 14:42     ` Ezequiel Garcia
2015-01-27 14:42     ` Ezequiel Garcia
2015-02-06  1:25     ` Brian Norris
2015-02-06  1:25       ` Brian Norris
2015-02-06  1:25       ` Brian Norris
2015-02-08 21:06   ` Boris Brezillon
2015-02-08 21:06     ` Boris Brezillon
2015-02-08 21:06     ` Boris Brezillon
2015-02-11 16:27     ` Antoine Tenart
2015-02-11 16:27       ` Antoine Tenart
2015-02-11 16:27       ` Antoine Tenart
2015-02-08 23:55   ` Boris Brezillon
2015-02-08 23:55     ` Boris Brezillon
2015-02-08 23:55     ` Boris Brezillon
2015-02-10 19:50     ` Robert Jarzmik
2015-02-10 19:50       ` Robert Jarzmik
2015-02-10 19:50       ` Robert Jarzmik
2015-02-11 16:33       ` Antoine Tenart [this message]
2015-02-11 16:33         ` Antoine Tenart
2015-02-11 16:33         ` Antoine Tenart
2015-02-12 16:26         ` Robert Jarzmik
2015-02-12 16:26           ` Robert Jarzmik
2015-02-12 16:26           ` Robert Jarzmik
2015-02-17  9:52           ` Antoine Tenart
2015-02-17  9:52             ` Antoine Tenart
2015-02-17  9:52             ` Antoine Tenart
2015-02-11 16:31     ` Antoine Tenart
2015-02-11 16:31       ` Antoine Tenart
2015-02-11 16:31       ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 6/9] Documentation: bindings: add the Berlin nand controller compatible Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 7/9] mtd: nand: let Marvell Berlin SoCs select the pxa3xx driver Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 8/9] ARM: berlin: add BG2Q node for the nand Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10 ` [PATCH 9/9] ARM: berlin: enable flash on the BG2Q DMP Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart
2015-01-27 14:10   ` Antoine Tenart

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20150211163343.GH11169@kwain \
    --to=antoine.tenart@free-electrons.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=ezequiel.garcia@free-electrons.com \
    --cc=jszhang@marvell.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=robert.jarzmik@free.fr \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=zmxu@marvell.com \
    /path/to/YOUR_REPLY

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

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