[PATCH] [MTD] orion_nand: add chip_delay parameter

[PATCH] [MTD] orion_nand: add chip_delay parameter

[Nicolas Pitre]

From: Saeed Bishara <saeed@marvell.com>

Some SoC's need a different chip_delay value.

Signed-off-by: Saeed Bishara <saeed@marvell.com>
Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
Signed-off-by: Nicolas Pitre <nico@marvell.com>

diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
index 59e05a1..ee2ac39 100644
--- a/drivers/mtd/nand/orion_nand.c
+++ b/drivers/mtd/nand/orion_nand.c
@@ -85,6 +85,9 @@ static int __init orion_nand_probe(struct platform_device *pdev)
 	nc->cmd_ctrl = orion_nand_cmd_ctrl;
 	nc->ecc.mode = NAND_ECC_SOFT;
 
+	if (board->chip_delay)
+		nc->chip_delay = board->chip_delay;
+
 	if (board->width == 16)
 		nc->options |= NAND_BUSWIDTH_16;
 
diff --git a/include/asm-arm/plat-orion/orion_nand.h b/include/asm-arm/plat-orion/orion_nand.h
index ffd3852..ad4ce94 100644
--- a/include/asm-arm/plat-orion/orion_nand.h
+++ b/include/asm-arm/plat-orion/orion_nand.h
@@ -18,6 +18,7 @@ struct orion_nand_data {
 	u8 ale;		/* address line number connected to ALE */
 	u8 cle;		/* address line number connected to CLE */
 	u8 width;	/* buswidth */
+	u8 chip_delay;
 };
 
 

Re: [PATCH] [MTD] orion_nand: add chip_delay parameter

[Nicolas Pitre]

On Mon, 23 Jun 2008, Nicolas Pitre wrote:

> From: Saeed Bishara <saeed@marvell.com>
> 
> Some SoC's need a different chip_delay value.
> 
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> Signed-off-by: Nicolas Pitre <nico@marvell.com>

I posted this patch on Monday for inclusion in 2.6.27.  However we have 
additional patches meant to be pushed through the ARM tree that depend 
on this one.  So to simplify dependency issues, I would like for this 
patch to be acked and then we'll simply push it along with the others 
through the ARM path.

Any objections??

> diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
> index 59e05a1..ee2ac39 100644
> --- a/drivers/mtd/nand/orion_nand.c
> +++ b/drivers/mtd/nand/orion_nand.c
> @@ -85,6 +85,9 @@ static int __init orion_nand_probe(struct platform_device *pdev)
>  	nc->cmd_ctrl = orion_nand_cmd_ctrl;
>  	nc->ecc.mode = NAND_ECC_SOFT;
>  
> +	if (board->chip_delay)
> +		nc->chip_delay = board->chip_delay;
> +
>  	if (board->width == 16)
>  		nc->options |= NAND_BUSWIDTH_16;
>  
> diff --git a/include/asm-arm/plat-orion/orion_nand.h b/include/asm-arm/plat-orion/orion_nand.h
> index ffd3852..ad4ce94 100644
> --- a/include/asm-arm/plat-orion/orion_nand.h
> +++ b/include/asm-arm/plat-orion/orion_nand.h
> @@ -18,6 +18,7 @@ struct orion_nand_data {
>  	u8 ale;		/* address line number connected to ALE */
>  	u8 cle;		/* address line number connected to CLE */
>  	u8 width;	/* buswidth */
> +	u8 chip_delay;
>  };
>  
>  
> 


Nicolas

Re: [PATCH] [MTD] orion_nand: add chip_delay parameter

[Jörn Engel]

On Thu, 26 June 2008 13:24:14 -0400, Nicolas Pitre wrote:
> 
> I posted this patch on Monday for inclusion in 2.6.27.  However we have 
> additional patches meant to be pushed through the ARM tree that depend 
> on this one.  So to simplify dependency issues, I would like for this 
> patch to be acked and then we'll simply push it along with the others 
> through the ARM path.
> 
> Any objections??

While I don't really understand what this patch does, it seems harmless
enough and I generally trust your judgement.  Good enough for me.

> > diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
> > index 59e05a1..ee2ac39 100644
> > --- a/drivers/mtd/nand/orion_nand.c
> > +++ b/drivers/mtd/nand/orion_nand.c
> > @@ -85,6 +85,9 @@ static int __init orion_nand_probe(struct platform_device *pdev)
> >  	nc->cmd_ctrl = orion_nand_cmd_ctrl;
> >  	nc->ecc.mode = NAND_ECC_SOFT;
> >  
> > +	if (board->chip_delay)
> > +		nc->chip_delay = board->chip_delay;
> > +

What I'm wondering about is whether the assignment should be made
unconditional.  After all, if board->chip_delay is zero, the assignment
should simply write zero to a variable that already is zero.  Or if it
isn't, the driver is missing a memset() somewhere.

Jörn

-- 
There is no worse hell than that provided by the regrets
for wasted opportunities.
-- Andre-Louis Moreau in Scarabouche

Re: [PATCH] [MTD] orion_nand: add chip_delay parameter

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1500 bytes --]

On Fri, 27 Jun 2008, Jörn Engel wrote:

> On Thu, 26 June 2008 13:24:14 -0400, Nicolas Pitre wrote:
> > 
> > I posted this patch on Monday for inclusion in 2.6.27.  However we have 
> > additional patches meant to be pushed through the ARM tree that depend 
> > on this one.  So to simplify dependency issues, I would like for this 
> > patch to be acked and then we'll simply push it along with the others 
> > through the ARM path.
> > 
> > Any objections??
> 
> While I don't really understand what this patch does, it seems harmless
> enough and I generally trust your judgement.  Good enough for me.

OK thanks.

> > > diff --git a/drivers/mtd/nand/orion_nand.c b/drivers/mtd/nand/orion_nand.c
> > > index 59e05a1..ee2ac39 100644
> > > --- a/drivers/mtd/nand/orion_nand.c
> > > +++ b/drivers/mtd/nand/orion_nand.c
> > > @@ -85,6 +85,9 @@ static int __init orion_nand_probe(struct platform_device *pdev)
> > >  	nc->cmd_ctrl = orion_nand_cmd_ctrl;
> > >  	nc->ecc.mode = NAND_ECC_SOFT;
> > >  
> > > +	if (board->chip_delay)
> > > +		nc->chip_delay = board->chip_delay;
> > > +
> 
> What I'm wondering about is whether the assignment should be made
> unconditional.  After all, if board->chip_delay is zero, the assignment
> should simply write zero to a variable that already is zero.  Or if it
> isn't, the driver is missing a memset() somewhere.

There is the case that a default could be set by the driver and having 
board->chip_delay == 0 should mean the driver default is fine.


Nicolas