LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	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 2/9] mtd: pxa3xx_nand: add a non mandatory ECC clock
Date: Sun, 8 Feb 2015 20:55:05 +0100	[thread overview]
Message-ID: <20150208205505.316f2624@bbrezillon> (raw)
In-Reply-To: <20150128141451.GA11705@kwain>

Hi Antoine,

On Wed, 28 Jan 2015 15:14:51 +0100
Antoine Tenart <antoine.tenart@free-electrons.com> wrote:

> Hi Andrew,
> 
> On Tue, Jan 27, 2015 at 04:50:47PM +0100, Andrew Lunn wrote:
> > On Tue, Jan 27, 2015 at 03:10:09PM +0100, Antoine Tenart wrote:
> > > Some controllers (as the coming Berlin nand controller) need to enable
> > > an ECC clock. Add support for this clock in the pxa3xx nand driver, and
> > > leave it as non mandatory.
> > 
> > It would be good to document this clock in the device tree binding.
> 
> Yes! I'll add the ECC clock into the documentation.
> 
> > 
> > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > > ---
> > >  drivers/mtd/nand/pxa3xx_nand.c | 26 +++++++++++++++++++-------
> > >  1 file changed, 19 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > > index d00ac392d1c4..2681ec4abafa 100644
> > > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > > @@ -180,7 +180,7 @@ struct pxa3xx_nand_info {
> > >  	struct nand_hw_control	controller;
> > >  	struct platform_device	 *pdev;
> > >  
> > > -	struct clk		*clk;
> > > +	struct clk		*clk, *ecc_clk;
> > >  	void __iomem		*mmio_base;
> > >  	unsigned long		mmio_phys;
> > >  	struct completion	cmd_complete, dev_ready;
> > > @@ -1608,7 +1608,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
> > >  
> > >  	spin_lock_init(&chip->controller->lock);
> > >  	init_waitqueue_head(&chip->controller->wq);
> > > -	info->clk = devm_clk_get(&pdev->dev, NULL);
> > > +	info->clk = devm_clk_get(&pdev->dev, "nfc");
> > 
> > Does this keep backwards compatibility? Not all the existing users
> > have there clocks named. What i think happens here is that
> > of_clk_get_by_name() calls of_property_match_string(np, "clock-names",
> > name); That returns -EINVAL, and then we ask of_clk_get() to get the
> > -EINVAL'th clock. I don't think that is going to end well.
> > 
> > I think you need to keep the NULL here, but should use "ecc" for the
> > second clock and require the use of the clock-names property when an
> > ecc clock is required.
> 
> I agree, I'll go back to NULL in v2.

I'd rather try with "nfc", and if it fails try with NULL as the name
argument, because passing NULL in all cases will enforce the clock
declaration order (which is not something we usually do when naming
clocks).
If you keep the NULL name, please document the clock order in your DT
documentation.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

  reply	other threads:[~2015-02-08 19:55 UTC|newest]

Thread overview: 28+ 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 ` [PATCH 1/9] mtd: pxa3xx_nand: initialiaze pxa3xx_flash_ids to 0 Antoine Tenart
2015-01-27 14:10 ` [PATCH 2/9] mtd: pxa3xx_nand: add a non mandatory ECC clock Antoine Tenart
2015-01-27 15:50   ` Andrew Lunn
2015-01-28 14:14     ` Antoine Tenart
2015-02-08 19:55       ` Boris Brezillon [this message]
2015-01-28  3:35   ` Jisheng Zhang
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-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-02-08 20:15   ` 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:42   ` Ezequiel Garcia
2015-02-06  1:25     ` Brian Norris
2015-02-08 21:06   ` Boris Brezillon
2015-02-11 16:27     ` Antoine Tenart
2015-02-08 23:55   ` Boris Brezillon
2015-02-10 19:50     ` Robert Jarzmik
2015-02-11 16:33       ` Antoine Tenart
2015-02-12 16:26         ` Robert Jarzmik
2015-02-17  9:52           ` 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 ` [PATCH 7/9] mtd: nand: let Marvell Berlin SoCs select the pxa3xx driver 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 ` [PATCH 9/9] ARM: berlin: enable flash on the BG2Q DMP 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=20150208205505.316f2624@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=andrew@lunn.ch \
    --cc=antoine.tenart@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=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=zmxu@marvell.com \
    --subject='Re: [PATCH 2/9] mtd: pxa3xx_nand: add a non mandatory ECC clock' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).