LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
From: LiuShuo @ 2011-11-23  1:56 UTC (permalink / raw)
  To: Scott Wood
  Cc: Artem.Bityutskiy, linuxppc-dev, linux-kernel, Liu Shuo,
	Shengzhou Liu, linux-mtd, akpm, dwmw2
In-Reply-To: <4ECC368C.3010801@freescale.com>

=E4=BA=8E 2011=E5=B9=B411=E6=9C=8823=E6=97=A5 07:55, Scott Wood =E5=86=99=
=E9=81=93:
> On 11/15/2011 03:29 AM, b35362@freescale.com wrote:
>> From: Liu Shuo<b35362@freescale.com>
>>
>> Freescale FCM controller has a 2K size limitation of buffer RAM. In or=
der
>> to support the Nand flash chip whose page size is larger than 2K bytes=
,
>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and sa=
ve
>> them to a large buffer.
>>
>> Signed-off-by: Liu Shuo<Shuo.Liu@freescale.com>
>> Signed-off-by: Shengzhou Liu<Shengzhou.Liu@freescale.com>
>> Signed-off-by: Li Yang<leoli@freescale.com>
>> ---
>>   drivers/mtd/nand/fsl_elbc_nand.c |  216 ++++++++++++++++++++++++++++=
+++++++---
>>   1 files changed, 199 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_e=
lbc_nand.c
>> index c2c231b..415f87e 100644
>> --- a/drivers/mtd/nand/fsl_elbc_nand.c
>> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
>> @@ -55,7 +55,9 @@ struct fsl_elbc_mtd {
>>   	struct device *dev;
>>   	int bank;               /* Chip select bank number           */
>>   	u8 __iomem *vbase;      /* Chip select base virtual address  */
>> -	int page_size;          /* NAND page size (0=3D512, 1=3D2048)    */
>> +	int page_size;          /* NAND page size (0=3D512, 1=3D2048, 2=3D40=
96...),
>> +				 * the mutiple of 2048.
>> +				 */
> That "..." isn't very descriptive.  What happens with 8192-byte pages?
> Is it 3 or 4?
>
> Please just get rid of this and use mtd->writesize.
>
>> +		for (i =3D 1; i<  priv->page_size; i++) {
>> +			/*
>> +			 * Maybe there are some reasons of FCM hardware timming,
>> +			 * we must insert a FIR_OP_NOP(0x00) before FIR_OP_RB.
>> +			 */
> s/timming/timing/
>
>>   	/* PAGEPROG reuses all of the setup from SEQIN and adds the length =
*/
>>   	case NAND_CMD_PAGEPROG: {
>> +		int len;
>>   		dev_vdbg(priv->dev,
>>   			 "fsl_elbc_cmdfunc: NAND_CMD_PAGEPROG "
>>   			 "writing %d bytes.\n", elbc_fcm_ctrl->index);
>> -
>>   		/* if the write did not start at 0 or is not a full page
>>   		 * then set the exact length, otherwise use a full page
>>   		 * write so the HW generates the ECC.
>>   		 */
>> -		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column !=3D 0 ||
>> +		if (elbc_fcm_ctrl->column>=3D mtd->writesize) {
>> +			/* write oob */
>> +			if (priv->page_size>  1) {
>> +				/* when pagesize of chip is greater than 2048,
>> +				 * we have to write full page to write spare
>> +				 * region, so we fill '0xff' to main region
>> +				 * and some bytes of spare region which we
>> +				 * don't want to rewrite.
>> +				 * (write '1' won't change the original value)
>> +				 */
>> +				memset(elbc_fcm_ctrl->buffer, 0xff,
>> +						elbc_fcm_ctrl->column);
> I don't like relying on this -- can we use RNDIN instead to do a
> discontiguous write?
>
>> +				len =3D 2112;
> len =3D min(elbc_fcm_ctrl->index, 2112);
>
>> +			} else
>> +				len =3D mtd->writesize + mtd->oobsize -
>> +					elbc_fcm_ctrl->column;
>> +			out_be32(&lbc->fbcr, len);
> len =3D elbc_fcm_ctrl->index - elbc_fcm_ctrl->column;
>
> Use braces on both sides of the if/else if it's needed on one side.
>
>> +		} else if (elbc_fcm_ctrl->column !=3D 0 ||
>>   		    elbc_fcm_ctrl->index !=3D mtd->writesize + mtd->oobsize)
>>   			out_be32(&lbc->fbcr, elbc_fcm_ctrl->index);
> This should have set fbcr to index - column as well (after adjusting --
> though really it's not a supported use case.  We should only be seeing
> column !=3D 0 for oob.
>
>> @@ -625,10 +776,16 @@ static int fsl_elbc_verify_buf(struct mtd_info *=
mtd, const u_char *buf, int len)
>>   		return -EINVAL;
>>   	}
>>
>> -	for (i =3D 0; i<  len; i++)
>> -		if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
>> -				!=3D buf[i])
>> -			break;
>> +	if (mtd->writesize>  2048)
>> +		for (i =3D 0; i<  len; i++)
>> +			if (elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index + i]
>> +					!=3D buf[i])
>> +				break;
>> +	else
>> +		for (i =3D 0; i<  len; i++)
>> +			if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
>> +					!=3D buf[i])
>> +				break;
> Please use braces around multiline if/for bodies, even if they're
> technically a single statement -- especially when you've got a dangling
> else.
>
>>   	elbc_fcm_ctrl->index +=3D len;
>>   	return i =3D=3D len&&  elbc_fcm_ctrl->status =3D=3D LTESR_CC ? 0 : =
-EIO;
>> @@ -657,6 +814,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info=
 *mtd)
>>   	struct fsl_elbc_mtd *priv =3D chip->priv;
>>   	struct fsl_lbc_ctrl *ctrl =3D priv->ctrl;
>>   	struct fsl_lbc_regs __iomem *lbc =3D ctrl->regs;
>> +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl =3D ctrl->nand;
>>   	unsigned int al;
>>
>>   	/* calculate FMR Address Length field */
>> @@ -707,12 +865,17 @@ static int fsl_elbc_chip_init_tail(struct mtd_in=
fo *mtd)
>>   	dev_dbg(priv->dev, "fsl_elbc_init: mtd->oobsize =3D %d\n",
>>   		mtd->oobsize);
>>
>> +	kfree(elbc_fcm_ctrl->buffer);
>> +	elbc_fcm_ctrl->buffer =3D NULL;
>> +
>>   	/* adjust Option Register and ECC to match Flash page size */
>>   	if (mtd->writesize =3D=3D 512) {
>>   		priv->page_size =3D 0;
>>   		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
>> -	} else if (mtd->writesize =3D=3D 2048) {
>> -		priv->page_size =3D 1;
>> +	} else if (mtd->writesize>=3D 2048) {
>> +		/* page_size =3D writesize / 2048 */
>> +		priv->page_size =3D mtd->writesize>>  11;
> Why not just write priv->page_size =3D mtd->writesize / 2048 in the cod=
e
> rather than the comment (it compiles to the same code)?  Other than
> because you were requested to remove priv->page_size, that is. :-)
>
>>   		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
>>   		/* adjust ecc setup if needed */
>>   		if ((in_be32(&lbc->bank[priv->bank].br)&  BR_DECC) =3D=3D
>> @@ -723,6 +886,14 @@ static int fsl_elbc_chip_init_tail(struct mtd_inf=
o *mtd)
>>   					&fsl_elbc_oob_lp_eccm0;
>>   			chip->badblock_pattern =3D&largepage_memorybased;
>>   		}
>> +
>> +		/* re-malloc if pagesize>  2048*/
>> +		if (mtd->writesize>  2048) {
>> +			elbc_fcm_ctrl->buffer =3D kmalloc(mtd->writesize +
>> +						    mtd->oobsize, GFP_KERNEL);
>> +			if (!elbc_fcm_ctrl->buffer)
>> +				return -ENOMEM;
>> +		}
>>   	} else {
>>   		dev_err(priv->dev,
>>   			"fsl_elbc_init: page size %d is not supported\n",
> buffer is a controller-wide resource, but you're setting it based on th=
e
> most-recently-probed NAND chip.  It should be large enough to
> accommodate the largest page size in use on any connected chip.  Even i=
f
> all chips in the system have the same page size, you're introducing a
> race if a previously-probed chip is already in use (the controller lock
> is not held here).
>
> Just allocate a buffer large enough for a 16K page size in the main ini=
t
> function, and print an error in the tail if you encounter a larger page
> size.
>
>> @@ -886,6 +1057,17 @@ static int __devinit fsl_elbc_nand_probe(struct =
platform_device *pdev)
>>   			goto err;
>>   		}
>>   		elbc_fcm_ctrl->counter++;
>> +		/*
>> +		 * Freescale FCM controller has a 2K size limitation of buffer
>> +		 * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
>> +		 * of chip is greater than 2048.
>> +		 * We malloc a large enough buffer at this point, because we
>> +		 * don't know writesize before calling nand_scan(). We will
>> +		 * re-malloc later if needed.
>> +		 */
>> +		elbc_fcm_ctrl->buffer =3D kmalloc(4096 * 6, GFP_KERNEL);
>> +		if (!elbc_fcm_ctrl->buffer)
>> +			return -ENOMEM;
> Clean up properly if you fail to allocate the buffer.  This includes
> freeing elbc_fcm_ctrl, setting fsl_lbc_ctrl_dev->nand to NULL, and
> releasing fsl_elbc_nand_mutex.
>
> -Scott
Thanks for your suggestions. I will make it better.

-LiuShuo

^ permalink raw reply

* Re: powerpc: dts: Fix canyonlands EMAC interrupt map
From: Benjamin Herrenschmidt @ 2011-11-23  1:42 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Tanmay Inamdar, linux-kernel
In-Reply-To: <20111123011517.GA5183@truffala.fritz.box>

On Wed, 2011-11-23 at 12:15 +1100, David Gibson wrote:
> On Wed, Nov 23, 2011 at 08:16:40AM +1100, Benjamin Herrenschmidt wrote:
> > On Tue, 2011-11-22 at 12:41 +0530, Tanmay Inamdar wrote:
> > > Fixing interrupt mapping of EMAC for canyonlands
> > 
> > The previous stuff was odd .... but was it broken ?
> > 
> > It was done this way because the EMAC actually has more interrupts than
> > that which are routed to different UICs, and so doing a local map this
> > way allows to target multiple parents.
> 
> Well, in the canyonlands case, it appears that the interrupts went to
> the same pic, so the simpler representation should be correct as
> well.  However, there certainly are boards where they go to multiple
> pics, so we need this interrupt-map trick.

Doesn't emac has something like 4 more interrupts that we simply haven't
been bothered wiring up (because we don't use them ?)

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] powerpc: 44x: Add mtd ndfc to the ppx44x defconfig
From: Benjamin Herrenschmidt @ 2011-11-23  1:34 UTC (permalink / raw)
  To: Josh Boyer; +Cc: LinuxPPC-dev
In-Reply-To: <CA+5PVA7js5kmK3JGDKiYa+xVqojmM2ZedifPHHWFsY7CYrqzYA@mail.gmail.com>

On Tue, 2011-11-22 at 20:04 -0500, Josh Boyer wrote:
> On Tue, Nov 22, 2011 at 6:50 PM, Tony Breeds <tony@bakeyournoodle.com> wrote:
> > Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> 
> Acked-by: Josh Boyer <jwboyer@gmail.com>
> 
> Ben, I don't have anything particularly urgent for 3.2 and this seems
> like it is well within the 3.2 window (defconfig updates usually come
> later).  Want to pick this up yourself, or do you want me to prep a
> tree somewhere?

I'm working on 3.3 right now, so I'd rather you throw this in there, I
doesn't seem to be particularily urgent for 3.2 is it? (who actually
uses 4xx_defconfig ?)

Cheers,
Ben.

^ permalink raw reply

* Re: powerpc: dts: Fix canyonlands EMAC interrupt map
From: David Gibson @ 2011-11-23  1:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Tanmay Inamdar, linux-kernel
In-Reply-To: <1321996600.14573.0.camel@pasglop>

On Wed, Nov 23, 2011 at 08:16:40AM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2011-11-22 at 12:41 +0530, Tanmay Inamdar wrote:
> > Fixing interrupt mapping of EMAC for canyonlands
> 
> The previous stuff was odd .... but was it broken ?
> 
> It was done this way because the EMAC actually has more interrupts than
> that which are routed to different UICs, and so doing a local map this
> way allows to target multiple parents.

Well, in the canyonlands case, it appears that the interrupts went to
the same pic, so the simpler representation should be correct as
well.  However, there certainly are boards where they go to multiple
pics, so we need this interrupt-map trick.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH] powerpc: 44x: Add mtd ndfc to the ppx44x defconfig
From: Josh Boyer @ 2011-11-23  1:04 UTC (permalink / raw)
  To: LinuxPPC-dev, Benjamin Herrenschmidt
In-Reply-To: <20111122234933.GA28400@thor.bakeyournoodle.com>

On Tue, Nov 22, 2011 at 6:50 PM, Tony Breeds <tony@bakeyournoodle.com> wrote:
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

Acked-by: Josh Boyer <jwboyer@gmail.com>

Ben, I don't have anything particularly urgent for 3.2 and this seems
like it is well within the 3.2 window (defconfig updates usually come
later).  Want to pick this up yourself, or do you want me to prep a
tree somewhere?

josh

^ permalink raw reply

* Re: [PATCH v3 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
From: Scott Wood @ 2011-11-22 23:55 UTC (permalink / raw)
  To: b35362
  Cc: Artem.Bityutskiy, linuxppc-dev, linux-kernel, Liu Shuo,
	Shengzhou Liu, linux-mtd, akpm, dwmw2
In-Reply-To: <1321349355-1639-3-git-send-email-b35362@freescale.com>

On 11/15/2011 03:29 AM, b35362@freescale.com wrote:
> From: Liu Shuo <b35362@freescale.com>
> 
> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
> to support the Nand flash chip whose page size is larger than 2K bytes,
> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
> them to a large buffer.
> 
> Signed-off-by: Liu Shuo <Shuo.Liu@freescale.com>
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  drivers/mtd/nand/fsl_elbc_nand.c |  216 +++++++++++++++++++++++++++++++++++---
>  1 files changed, 199 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index c2c231b..415f87e 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -55,7 +55,9 @@ struct fsl_elbc_mtd {
>  	struct device *dev;
>  	int bank;               /* Chip select bank number           */
>  	u8 __iomem *vbase;      /* Chip select base virtual address  */
> -	int page_size;          /* NAND page size (0=512, 1=2048)    */
> +	int page_size;          /* NAND page size (0=512, 1=2048, 2=4096...),
> +				 * the mutiple of 2048.
> +				 */

That "..." isn't very descriptive.  What happens with 8192-byte pages?
Is it 3 or 4?

Please just get rid of this and use mtd->writesize.

> +		for (i = 1; i < priv->page_size; i++) {
> +			/*
> +			 * Maybe there are some reasons of FCM hardware timming,
> +			 * we must insert a FIR_OP_NOP(0x00) before FIR_OP_RB.
> +			 */

s/timming/timing/

>  	/* PAGEPROG reuses all of the setup from SEQIN and adds the length */
>  	case NAND_CMD_PAGEPROG: {
> +		int len;
>  		dev_vdbg(priv->dev,
>  			 "fsl_elbc_cmdfunc: NAND_CMD_PAGEPROG "
>  			 "writing %d bytes.\n", elbc_fcm_ctrl->index);
> -
>  		/* if the write did not start at 0 or is not a full page
>  		 * then set the exact length, otherwise use a full page
>  		 * write so the HW generates the ECC.
>  		 */
> -		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
> +		if (elbc_fcm_ctrl->column >= mtd->writesize) {
> +			/* write oob */
> +			if (priv->page_size > 1) {
> +				/* when pagesize of chip is greater than 2048,
> +				 * we have to write full page to write spare
> +				 * region, so we fill '0xff' to main region
> +				 * and some bytes of spare region which we
> +				 * don't want to rewrite.
> +				 * (write '1' won't change the original value)
> +				 */
> +				memset(elbc_fcm_ctrl->buffer, 0xff,
> +						elbc_fcm_ctrl->column);

I don't like relying on this -- can we use RNDIN instead to do a
discontiguous write?

> +				len = 2112;

len = min(elbc_fcm_ctrl->index, 2112);

> +			} else
> +				len = mtd->writesize + mtd->oobsize -
> +					elbc_fcm_ctrl->column;
> +			out_be32(&lbc->fbcr, len);

len = elbc_fcm_ctrl->index - elbc_fcm_ctrl->column;

Use braces on both sides of the if/else if it's needed on one side.

> +		} else if (elbc_fcm_ctrl->column != 0 ||
>  		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
>  			out_be32(&lbc->fbcr, elbc_fcm_ctrl->index);

This should have set fbcr to index - column as well (after adjusting --
though really it's not a supported use case.  We should only be seeing
column != 0 for oob.

> @@ -625,10 +776,16 @@ static int fsl_elbc_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
>  		return -EINVAL;
>  	}
>  
> -	for (i = 0; i < len; i++)
> -		if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
> -				!= buf[i])
> -			break;
> +	if (mtd->writesize > 2048)
> +		for (i = 0; i < len; i++)
> +			if (elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index + i]
> +					!= buf[i])
> +				break;
> +	else
> +		for (i = 0; i < len; i++)
> +			if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
> +					!= buf[i])
> +				break;

Please use braces around multiline if/for bodies, even if they're
technically a single statement -- especially when you've got a dangling
else.

>  	elbc_fcm_ctrl->index += len;
>  	return i == len && elbc_fcm_ctrl->status == LTESR_CC ? 0 : -EIO;
> @@ -657,6 +814,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
>  	struct fsl_elbc_mtd *priv = chip->priv;
>  	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
>  	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
>  	unsigned int al;
>  
>  	/* calculate FMR Address Length field */
> @@ -707,12 +865,17 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
>  	dev_dbg(priv->dev, "fsl_elbc_init: mtd->oobsize = %d\n",
>  		mtd->oobsize);
>  
> +	kfree(elbc_fcm_ctrl->buffer);
> +	elbc_fcm_ctrl->buffer = NULL;
> +
>  	/* adjust Option Register and ECC to match Flash page size */
>  	if (mtd->writesize == 512) {
>  		priv->page_size = 0;
>  		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
> -	} else if (mtd->writesize == 2048) {
> -		priv->page_size = 1;
> +	} else if (mtd->writesize >= 2048) {
> +		/* page_size = writesize / 2048 */
> +		priv->page_size = mtd->writesize >> 11;

Why not just write priv->page_size = mtd->writesize / 2048 in the code
rather than the comment (it compiles to the same code)?  Other than
because you were requested to remove priv->page_size, that is. :-)

>  		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
>  		/* adjust ecc setup if needed */
>  		if ((in_be32(&lbc->bank[priv->bank].br) & BR_DECC) ==
> @@ -723,6 +886,14 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
>  					   &fsl_elbc_oob_lp_eccm0;
>  			chip->badblock_pattern = &largepage_memorybased;
>  		}
> +
> +		/* re-malloc if pagesize > 2048*/
> +		if (mtd->writesize > 2048) {
> +			elbc_fcm_ctrl->buffer = kmalloc(mtd->writesize +
> +						    mtd->oobsize, GFP_KERNEL);
> +			if (!elbc_fcm_ctrl->buffer)
> +				return -ENOMEM;
> +		}
>  	} else {
>  		dev_err(priv->dev,
>  			"fsl_elbc_init: page size %d is not supported\n",

buffer is a controller-wide resource, but you're setting it based on the
most-recently-probed NAND chip.  It should be large enough to
accommodate the largest page size in use on any connected chip.  Even if
all chips in the system have the same page size, you're introducing a
race if a previously-probed chip is already in use (the controller lock
is not held here).

Just allocate a buffer large enough for a 16K page size in the main init
function, and print an error in the tail if you encounter a larger page
size.

> @@ -886,6 +1057,17 @@ static int __devinit fsl_elbc_nand_probe(struct platform_device *pdev)
>  			goto err;
>  		}
>  		elbc_fcm_ctrl->counter++;
> +		/*
> +		 * Freescale FCM controller has a 2K size limitation of buffer
> +		 * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> +		 * of chip is greater than 2048.
> +		 * We malloc a large enough buffer at this point, because we
> +		 * don't know writesize before calling nand_scan(). We will
> +		 * re-malloc later if needed.
> +		 */
> +		elbc_fcm_ctrl->buffer = kmalloc(4096 * 6, GFP_KERNEL);
> +		if (!elbc_fcm_ctrl->buffer)
> +			return -ENOMEM;

Clean up properly if you fail to allocate the buffer.  This includes
freeing elbc_fcm_ctrl, setting fsl_lbc_ctrl_dev->nand to NULL, and
releasing fsl_elbc_nand_mutex.

-Scott

^ permalink raw reply

* [PATCH] powerpc: 44x: Add mtd ndfc to the ppx44x defconfig
From: Tony Breeds @ 2011-11-22 23:50 UTC (permalink / raw)
  To: LinuxPPC-dev, Josh Boyer

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
---

Again appologies for the SPAM trying to send this message.

 arch/powerpc/configs/ppc44x_defconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/configs/ppc44x_defconfig b/arch/powerpc/configs/ppc44x_defconfig
index 6cdf1c0..3b98d73 100644
--- a/arch/powerpc/configs/ppc44x_defconfig
+++ b/arch/powerpc/configs/ppc44x_defconfig
@@ -52,6 +52,8 @@ CONFIG_MTD_CFI=y
 CONFIG_MTD_JEDECPROBE=y
 CONFIG_MTD_CFI_AMDSTD=y
 CONFIG_MTD_PHYSMAP_OF=y
+CONFIG_MTD_NAND=m
+CONFIG_MTD_NAND_NDFC=m
 CONFIG_MTD_UBI=m
 CONFIG_MTD_UBI_GLUEBI=m
 CONFIG_PROC_DEVICETREE=y
-- 
1.7.6.4


Yours Tony

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* Re: your mail
From: Tony Breeds @ 2011-11-22 23:47 UTC (permalink / raw)
  To: LinuxPPC-dev
In-Reply-To: <20111122234245.GA28234@thor.bakeyournoodle.com>

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

On Wed, Nov 23, 2011 at 10:42:45AM +1100, Tony Breeds wrote:
> From 1a44c074e3ce572cbf60d31ac704e6ce42be4708 Mon Sep 17 00:00:00 2001
> From: Tony Breeds <tony@bakeyournoodle.com>
> Date: Wed, 23 Nov 2011 10:16:40 +1100
> Subject: [PATCH] powerpc: 44x: Add mtd ndfc to the ppx44x defconfig

Sorry all for the SPAM.  I screwed up trying to use mutt and git to
send-email

Yours Tony

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* (no subject)
From: Tony Breeds @ 2011-11-22 23:42 UTC (permalink / raw)
  To: LinuxPPC-dev

>From 1a44c074e3ce572cbf60d31ac704e6ce42be4708 Mon Sep 17 00:00:00 2001
From: Tony Breeds <tony@bakeyournoodle.com>
Date: Wed, 23 Nov 2011 10:16:40 +1100
Subject: [PATCH] powerpc: 44x: Add mtd ndfc to the ppx44x defconfig

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
---
 arch/powerpc/configs/ppc44x_defconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/configs/ppc44x_defconfig b/arch/powerpc/configs/ppc44x_defconfig
index 6cdf1c0..3b98d73 100644
--- a/arch/powerpc/configs/ppc44x_defconfig
+++ b/arch/powerpc/configs/ppc44x_defconfig
@@ -52,6 +52,8 @@ CONFIG_MTD_CFI=y
 CONFIG_MTD_JEDECPROBE=y
 CONFIG_MTD_CFI_AMDSTD=y
 CONFIG_MTD_PHYSMAP_OF=y
+CONFIG_MTD_NAND=m
+CONFIG_MTD_NAND_NDFC=m
 CONFIG_MTD_UBI=m
 CONFIG_MTD_UBI_GLUEBI=m
 CONFIG_PROC_DEVICETREE=y
-- 
1.7.6.4

^ permalink raw reply related

* (no subject)
From: Tony Breeds @ 2011-11-22 23:42 UTC (permalink / raw)
  To: LinuxPPC-dev

>From 1a44c074e3ce572cbf60d31ac704e6ce42be4708 Mon Sep 17 00:00:00 2001
From: Tony Breeds <tony@bakeyournoodle.com>
Date: Wed, 23 Nov 2011 10:16:40 +1100
Subject: [PATCH] powerpc: 44x: Add mtd ndfc to the ppx44x defconfig

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
---
 arch/powerpc/configs/ppc44x_defconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/configs/ppc44x_defconfig b/arch/powerpc/configs/ppc44x_defconfig
index 6cdf1c0..3b98d73 100644
--- a/arch/powerpc/configs/ppc44x_defconfig
+++ b/arch/powerpc/configs/ppc44x_defconfig
@@ -52,6 +52,8 @@ CONFIG_MTD_CFI=y
 CONFIG_MTD_JEDECPROBE=y
 CONFIG_MTD_CFI_AMDSTD=y
 CONFIG_MTD_PHYSMAP_OF=y
+CONFIG_MTD_NAND=m
+CONFIG_MTD_NAND_NDFC=m
 CONFIG_MTD_UBI=m
 CONFIG_MTD_UBI_GLUEBI=m
 CONFIG_PROC_DEVICETREE=y
-- 
1.7.6.4

^ permalink raw reply related

* Re: Please revert commit dc9372808412edbc653a675a526c2ee6c0c14a91
From: Rob Herring @ 2011-11-22 23:36 UTC (permalink / raw)
  To: Grant Likely
  Cc: devicetree-discuss, linuxppc-dev, Rob Herring, Tanmay Inamdar,
	Linus Torvalds
In-Reply-To: <CACxGe6tydq8yGnEMea-cL2Et_vWEBrmpE_Xn9U-rb1nZskxztQ@mail.gmail.com>

On 11/22/2011 04:43 PM, Grant Likely wrote:
> On Tue, Nov 22, 2011 at 3:11 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> Hi Linus !
>>
>> Please, revert commit dc9372808412edbc653a675a526c2ee6c0c14a91
>>
>> "of/irq: of_irq_find_parent: check for parent equal to child"
>>
>> This breaks some powerpc platforms at least. The practice of having a
>> node provide an explicit "interrupt-parent" property pointing to itself
>> is an old trick that we've used in the past to allow a device-node to
>> have interrupts routed to different controllers.
>>
>> In that case, the node also contains an interrupt-map, so the node is
>> its own parent, the interrupt resolution hits the map, which then can
>> route each individual interrupt to a different parent.
> 
> Ah, nuts, yes that is broken then.  Yes, please revert the commit and
> Rob & I will come up with a better solution.
> 
> Rob, I think it can be done by explicitly checking for np ==
> desc->interrupt_parent in of_irq_init() instead of relying on
> of_irq_find_parent() returning NULL.

Okay. I'll prepare a patch to do that.

Rob

^ permalink raw reply

* Re: Please revert commit dc9372808412edbc653a675a526c2ee6c0c14a91
From: Grant Likely @ 2011-11-22 22:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, devicetree-discuss, Linus Torvalds, Tanmay Inamdar,
	Rob Herring
In-Reply-To: <1321999910.14573.11.camel@pasglop>

On Tue, Nov 22, 2011 at 3:11 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Hi Linus !
>
> Please, revert commit dc9372808412edbc653a675a526c2ee6c0c14a91
>
> "of/irq: of_irq_find_parent: check for parent equal to child"
>
> This breaks some powerpc platforms at least. The practice of having a
> node provide an explicit "interrupt-parent" property pointing to itself
> is an old trick that we've used in the past to allow a device-node to
> have interrupts routed to different controllers.
>
> In that case, the node also contains an interrupt-map, so the node is
> its own parent, the interrupt resolution hits the map, which then can
> route each individual interrupt to a different parent.

Ah, nuts, yes that is broken then.  Yes, please revert the commit and
Rob & I will come up with a better solution.

Rob, I think it can be done by explicitly checking for np ==
desc->interrupt_parent in of_irq_init() instead of relying on
of_irq_find_parent() returning NULL.

g.

^ permalink raw reply

* Re: curious why mpc85xx/edac/pcie patch series was never picked up
From: Benjamin Herrenschmidt @ 2011-11-22 22:14 UTC (permalink / raw)
  To: Chris Friesen; +Cc: b25806, Paul Mackerras, linuxppc-dev
In-Reply-To: <4ECBDE98.8090705@genband.com>

On Tue, 2011-11-22 at 11:40 -0600, Chris Friesen wrote:
> We're doing some work with an mpc85xx-based board and someone pointed to 
> a patch adding PCI/PCIE error interrupt edac support that was proposed 
> by Lan Chunhe over a year ago:
> 
> http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-November/086930.html
> 
> For some reason this doesn't appear to have been picked up for mainline, 
> and I'm curious why not--is there something wrong with it?

Kumar ? What's up there ? Is that a miscommunication with the edac folks
or we just lost it between the cracks ?

Cheers,
Ben.

^ permalink raw reply

* Please revert commit dc9372808412edbc653a675a526c2ee6c0c14a91
From: Benjamin Herrenschmidt @ 2011-11-22 22:11 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: devicetree-discuss, linuxppc-dev, Tanmay Inamdar, Rob Herring

Hi Linus !

Please, revert commit dc9372808412edbc653a675a526c2ee6c0c14a91

"of/irq: of_irq_find_parent: check for parent equal to child"

This breaks some powerpc platforms at least. The practice of having a
node provide an explicit "interrupt-parent" property pointing to itself
is an old trick that we've used in the past to allow a device-node to
have interrupts routed to different controllers.

In that case, the node also contains an interrupt-map, so the node is
its own parent, the interrupt resolution hits the map, which then can
route each individual interrupt to a different parent.

Cheers,
Ben.

^ permalink raw reply

* Re: powerpc: dts: Fix canyonlands EMAC interrupt map
From: Benjamin Herrenschmidt @ 2011-11-22 22:05 UTC (permalink / raw)
  To: Tanmay Inamdar; +Cc: linuxppc-dev, linux-kernel, Rob Herring
In-Reply-To: <CACoXjc=RBpB6eYu1zB-kCcYmsV8rdbzYtdxyBJDLTdt6uu04ig@mail.gmail.com>

On Tue, 2011-11-22 at 19:45 +0530, Tanmay Inamdar wrote:
> 
> On Tue, Nov 22, 2011 at 5:00 PM, Josh Boyer <jwboyer@gmail.com> wrote:
>         On Tue, Nov 22, 2011 at 2:11 AM, Tanmay Inamdar
>         <tinamdar@apm.com> wrote:
>         > Fixing interrupt mapping of EMAC for canyonlands
>         >
>         > Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>         
>         
>         As far as I can tell, your changes aren't really changing
>         anything
>         just making it a bit clearer, correct?  If so, do you mind if
>         I change
>         the commit log to "clear up" instead of fix?
> 
> Actually Rob Herring's commit
> (http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=dc9372808412edbc653a675a526c2ee6c0c14a91) breaks the interrupt mapping in EMAC driver.
> I am trying to fix this issue by mapping interrupts in different way.

No. This commit needs to be reverted. It breaks existing practices.
Pointing to yourself as a parent in order to provide a map is an old
trick and it should be supported.

I'll ask Linus to revert.

Cheers,
Ben.

> If you think "clear up" is fine, then please go ahead.
> 
> Thanks,
> Tanmay
> 
>         
>         josh
>         
>         
>         > ---
>         >  arch/powerpc/boot/dts/canyonlands.dts |   16
>         ++++++----------
>         >  1 files changed, 6 insertions(+), 10 deletions(-)
>         >
>         > diff --git a/arch/powerpc/boot/dts/canyonlands.dts
>         b/arch/powerpc/boot/dts/canyonlands.dts
>         > index 3dc75de..c76bbcd 100644
>         > --- a/arch/powerpc/boot/dts/canyonlands.dts
>         > +++ b/arch/powerpc/boot/dts/canyonlands.dts
>         > @@ -360,13 +360,11 @@
>         >                        EMAC0: ethernet@ef600e00 {
>         >                                device_type = "network";
>         >                                compatible =
>         "ibm,emac-460ex", "ibm,emac4sync";
>         > -                               interrupt-parent = <&EMAC0>;
>         > -                               interrupts = <0x0 0x1>;
>         > -                               #interrupt-cells = <1>;
>         > +                               interrupt-parent = <&UIC2>;
>         >                                #address-cells = <0>;
>         >                                #size-cells = <0>;
>         > -                               interrupt-map = </*Status*/
>         0x0 &UIC2 0x10 0x4
>         > -                                                /*Wake*/
>         0x1 &UIC2 0x14 0x4>;
>         > +                               interrupts = </*Status*/0x10
>         0x4
>         > +                                               /*Wake*/0x14
>         0x4>;
>         >                                reg = <0xef600e00
>         0x000000c4>;
>         >                                local-mac-address =
>         [000000000000]; /* Filled in by U-Boot */
>         >                                mal-device = <&MAL0>;
>         > @@ -390,13 +388,11 @@
>         >                        EMAC1: ethernet@ef600f00 {
>         >                                device_type = "network";
>         >                                compatible =
>         "ibm,emac-460ex", "ibm,emac4sync";
>         > -                               interrupt-parent = <&EMAC1>;
>         > -                               interrupts = <0x0 0x1>;
>         > -                               #interrupt-cells = <1>;
>         > +                               interrupt-parent = <&UIC2>;
>         >                                #address-cells = <0>;
>         >                                #size-cells = <0>;
>         > -                               interrupt-map = </*Status*/
>         0x0 &UIC2 0x11 0x4
>         > -                                                /*Wake*/
>         0x1 &UIC2 0x15 0x4>;
>         > +                               interrupts = </*Status*/0x11
>         0x4
>         > +                                               /*Wake*/0x15
>         0x4>;
>         >                                reg = <0xef600f00
>         0x000000c4>;
>         >                                local-mac-address =
>         [000000000000]; /* Filled in by U-Boot */
>         >                                mal-device = <&MAL0>;
>         > --
>         > 1.6.1.rc3
>         >
>         
>         > --
>         > To unsubscribe from this list: send the line "unsubscribe
>         linux-kernel" in
>         > the body of a message to majordomo@vger.kernel.org
>         > More majordomo info at
>          http://vger.kernel.org/majordomo-info.html
>         > Please read the FAQ at  http://www.tux.org/lkml/
>         >
> 
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
> is for the sole use of the intended recipient(s) and contains information 
> that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries. 
> It is to be used solely for the purpose of furthering the parties' business relationship. 
> All unauthorized review, use, disclosure or distribution is prohibited. 
> If you are not the intended recipient, please contact the sender by reply e-mail 
> and destroy all copies of the original message.

^ permalink raw reply

* Re: powerpc: dts: Fix canyonlands EMAC interrupt map
From: Benjamin Herrenschmidt @ 2011-11-22 21:16 UTC (permalink / raw)
  To: Tanmay Inamdar; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1321945877-22802-1-git-send-email-tinamdar@apm.com>

On Tue, 2011-11-22 at 12:41 +0530, Tanmay Inamdar wrote:
> Fixing interrupt mapping of EMAC for canyonlands

The previous stuff was odd .... but was it broken ?

It was done this way because the EMAC actually has more interrupts than
that which are routed to different UICs, and so doing a local map this
way allows to target multiple parents.

Cheers,
Ben.

> Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
> ---
>  arch/powerpc/boot/dts/canyonlands.dts |   16 ++++++----------
>  1 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
> index 3dc75de..c76bbcd 100644
> --- a/arch/powerpc/boot/dts/canyonlands.dts
> +++ b/arch/powerpc/boot/dts/canyonlands.dts
> @@ -360,13 +360,11 @@
>  			EMAC0: ethernet@ef600e00 {
>  				device_type = "network";
>  				compatible = "ibm,emac-460ex", "ibm,emac4sync";
> -				interrupt-parent = <&EMAC0>;
> -				interrupts = <0x0 0x1>;
> -				#interrupt-cells = <1>;
> +				interrupt-parent = <&UIC2>;
>  				#address-cells = <0>;
>  				#size-cells = <0>;
> -				interrupt-map = </*Status*/ 0x0 &UIC2 0x10 0x4
> -						 /*Wake*/   0x1 &UIC2 0x14 0x4>;
> +				interrupts = </*Status*/0x10 0x4
> +						/*Wake*/0x14 0x4>;
>  				reg = <0xef600e00 0x000000c4>;
>  				local-mac-address = [000000000000]; /* Filled in by U-Boot */
>  				mal-device = <&MAL0>;
> @@ -390,13 +388,11 @@
>  			EMAC1: ethernet@ef600f00 {
>  				device_type = "network";
>  				compatible = "ibm,emac-460ex", "ibm,emac4sync";
> -				interrupt-parent = <&EMAC1>;
> -				interrupts = <0x0 0x1>;
> -				#interrupt-cells = <1>;
> +				interrupt-parent = <&UIC2>;
>  				#address-cells = <0>;
>  				#size-cells = <0>;
> -				interrupt-map = </*Status*/ 0x0 &UIC2 0x11 0x4
> -						 /*Wake*/   0x1 &UIC2 0x15 0x4>;
> +				interrupts = </*Status*/0x11 0x4
> +						/*Wake*/0x15 0x4>;
>  				reg = <0xef600f00 0x000000c4>;
>  				local-mac-address = [000000000000]; /* Filled in by U-Boot */
>  				mal-device = <&MAL0>;

^ permalink raw reply

* Re: [PATCH 2/3] mtd/nand : set Nand flash page address to FBAR and FPAR correctly
From: Artem Bityutskiy @ 2011-11-22 21:05 UTC (permalink / raw)
  To: LiuShuo
  Cc: Artem.Bityutskiy, linuxppc-dev, linux-kernel, Tang Yuantian,
	linux-mtd, Jerry Huang, scottwood, akpm, dwmw2
In-Reply-To: <4EC5BE16.9080205@freescale.com>

On Fri, 2011-11-18 at 10:08 +0800, LiuShuo wrote:
> Ok and I want to add another patch before 3/3.
> 
> -LiuShuo
> > On Tue, 2011-11-15 at 17:29 +0800, b35362@freescale.com wrote:
> >> From: Liu Shuo<b35362@freescale.com>
> >>
> >> If we use the Nand flash chip whose number of pages in a block is greater
> >> than 64(for large page), we must treat the low bit of FBAR as being the
> >> high bit of the page address due to the limitation of FCM, it simply uses
> >> the low 6-bits (for large page) of the combined block/page address as the
> >> FPAR component, rather than considering the actual block size.
> > Looks like this patch depends on the previous white-space clean-up patch
> > - could you please refactor it (and 3/3 too) and resend?
> Ok and I am going to add another new patch before 3/3.

Sure, send 3/3 as well because this one depends on the cleanup patch, so
cannot be applied independently.

Artem.

^ permalink raw reply

* Re: [PATCH v3 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
From: Artem Bityutskiy @ 2011-11-22 21:04 UTC (permalink / raw)
  To: b35362
  Cc: Artem.Bityutskiy, linuxppc-dev, linux-kernel, Liu Shuo,
	Shengzhou Liu, linux-mtd, scottwood, akpm, dwmw2
In-Reply-To: <1321349355-1639-3-git-send-email-b35362@freescale.com>

On Tue, 2011-11-15 at 17:29 +0800, b35362@freescale.com wrote:
> +               /*
> +                * Freescale FCM controller has a 2K size limitation of buffer
> +                * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> +                * of chip is greater than 2048.
> +                * We malloc a large enough buffer at this point, because we
> +                * don't know writesize before calling nand_scan(). We will
> +                * re-malloc later if needed.
> +                */
> +               elbc_fcm_ctrl->buffer = kmalloc(4096 * 6, GFP_KERNEL);
> +               if (!elbc_fcm_ctrl->buffer)
> +                       return -ENOMEM; 

Why 4096*6? Judging from the comment it should be 4096.

Artem.

^ permalink raw reply

* [PATCH] powerpc: Fix building ppc64 w/hugetlbfs enabled
From: Kumar Gala @ 2011-11-22 20:25 UTC (permalink / raw)
  To: linuxppc-dev

arch/powerpc/kernel/setup_64.c: In function 'early_setup':
arch/powerpc/kernel/setup_64.c:226:2: error: implicit declaration of function 'reserve_hugetlb_gpages'

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/setup_64.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 66648cc..6a98051 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -65,6 +65,7 @@
 #include <asm/kexec.h>
 #include <asm/mmu_context.h>
 #include <asm/code-patching.h>
+#include <asm/hugetlb.h>
 
 #include "setup.h"
 
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
From: Ira W. Snyder @ 2011-11-22 18:59 UTC (permalink / raw)
  To: b29237; +Cc: vinod.koul, linux-kernel, zw, dan.j.williams, linuxppc-dev
In-Reply-To: <1321937705-19587-1-git-send-email-b29237@freescale.com>

On Tue, Nov 22, 2011 at 12:55:05PM +0800, b29237@freescale.com wrote:
> From: Forrest Shi <b29237@freescale.com>
> 
>     dma status check function fsl_tx_status is heavily called in
>     a tight loop and the desc lock in fsl_tx_status contended by
>     the dma status update function. this caused the dma performance
>     degrades much.
> 
>     this patch releases the lock in the fsl_tx_status function.
>     I believe it has no neglect impact on the following call of
>     dma_async_is_complete(...).
> 
>     we can see below three conditions will be identified as success
>     a)  x < complete < use
>     b)  x < complete+N < use+N
>     c)  x < complete < use+N
>     here complete is the completed_cookie, use is the last_used
>     cookie, x is the querying cookie, N is MAX cookie
> 
>     when chan->completed_cookie is being read, the last_used may
>     be incresed. Anyway it has no neglect impact on the dma status
>     decision.
> 
>     Signed-off-by: Forrest Shi <xuelin.shi@freescale.com>
> ---
>  drivers/dma/fsldma.c |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 8a78154..1dca56f 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -986,15 +986,10 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
>  	struct fsldma_chan *chan = to_fsl_chan(dchan);
>  	dma_cookie_t last_complete;
>  	dma_cookie_t last_used;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&chan->desc_lock, flags);
>  

This will cause a bug. See below for a detailed explanation. You need
this instead:

	/*
	 * On an SMP system, we must ensure that this CPU has seen the
	 * memory accesses performed by another CPU under the
	 * chan->desc_lock spinlock.
	 */
	smp_mb();
>  	last_complete = chan->completed_cookie;
>  	last_used = dchan->cookie;
>  
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> -
>  	dma_set_tx_state(txstate, last_complete, last_used, 0);
>  	return dma_async_is_complete(cookie, last_complete, last_used);
>  }

Facts:
- dchan->cookie is the same member as chan->common.cookie (same memory location)
- chan->common.cookie is the "last allocated cookie for a pending transaction"
- chan->completed_cookie is the "last completed transaction"

I have replaced "dchan->cookie" with "chan->common.cookie" in the below
explanation, to keep everything referenced from the same structure.

Variable usage before your change. Everything is used locked.
- RW chan->common.cookie		(fsl_dma_tx_submit)
- R  chan->common.cookie		(fsl_tx_status)
- R  chan->completed_cookie		(fsl_tx_status)
- W  chan->completed_cookie		(dma_do_tasklet)

Variable usage after your change:
- RW chan->common.cookie		LOCKED
- R  chan->common.cookie		NO LOCK
- R  chan->completed_cookie		NO LOCK
- W  chan->completed_cookie             LOCKED

What if we assume that you have a 2 CPU system (such as a P2020). After
your changes, one possible sequence is:

=== CPU1 - allocate + submit descriptor: fsl_dma_tx_submit() ===
spin_lock_irqsave
descriptor->cookie = 20		(x in your example)
chan->common.cookie = 20	(used in your example)
spin_unlock_irqrestore

=== CPU2 - immediately calls fsl_tx_status() ===
chan->common.cookie == 19
chan->completed_cookie == 19
descriptor->cookie == 20

Since we don't have locks anymore, CPU2 may not have seen the write to
chan->common.cookie yet.

Also assume that the DMA hardware has not started processing the
transaction yet. Therefore dma_do_tasklet() has not been called, and
chan->completed_cookie has not been updated.

In this case, dma_async_is_complete() (on CPU2) returns DMA_SUCCESS,
even though the DMA operation has not succeeded. The DMA operation has
not even started yet!

The smp_mb() fixes this, since it forces CPU2 to have seen all memory
operations that happened before CPU1 released the spinlock. Spinlocks
are implicit SMP memory barriers.

Therefore, the above example becomes:
smp_mb();
chan->common.cookie == 20
chan->completed_cookie == 19
descriptor->cookie == 20

Then dma_async_is_complete() returns DMA_IN_PROGRESS, which is correct.

Thanks,
Ira

^ permalink raw reply

* Enabling MBX in MPC5121 - OGLES kernel modules
From: Einar Már Björgvinsson @ 2011-11-22 19:15 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

Hi

I've been working on enabling the PowerVR GPU core in MPC5121 chip. I've been following the documentation from here:

http://www.datasheetarchive.com/indexdl/Datasheet-076/DSAE0026055.pdf

This documentation was released in a bundle with MBX libraries and MBX kernel patches. Also have I downloaded the OGLES SDK from Imgtech where I've successfully built some demos for MPC5121.

The last piece of the puzzle is to have the right kernel modules. There are some kernel modules provided in the documentation bundle but they are build against older kernel version (2.6.24) but I'm am using kernel version 2.6.33 with RT 29.

What I'm hoping is that somebody have the source code for the following kernel modules:

- pvr.ko
- clcdc.ko
- swcamera.ko
- dgbdrv.ko

Hope somebody out there can assist

Regards
Einar M. Bjorgvinsson
Embedded Software Engineer
Marel ehf
Iceland


[-- Attachment #2: Type: text/html, Size: 1495 bytes --]

^ permalink raw reply

* curious why mpc85xx/edac/pcie patch series was never picked up
From: Chris Friesen @ 2011-11-22 17:40 UTC (permalink / raw)
  To: linuxppc-dev, Paul Mackerras, Benjamin Herrenschmidt, b25806


We're doing some work with an mpc85xx-based board and someone pointed to 
a patch adding PCI/PCIE error interrupt edac support that was proposed 
by Lan Chunhe over a year ago:

http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-November/086930.html

For some reason this doesn't appear to have been picked up for mainline, 
and I'm curious why not--is there something wrong with it?

Thanks,
Chris

-- 
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com

^ permalink raw reply

* Re: [PATCH v2 1/7] powerpc/85xx: re-enable timebase sync disabled by KEXEC patch
From: Kumar Gala @ 2011-11-22 16:05 UTC (permalink / raw)
  To: Li Yang-R58472
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	Zhao Chenhui-B35336
In-Reply-To: <3F607A5180246847A760FD34122A1E052D942C@039-SN1MPN1-003.039d.mgd.msft.net>


On Nov 22, 2011, at 3:29 AM, Li Yang-R58472 wrote:

>> Subject: Re: [PATCH v2 1/7] powerpc/85xx: re-enable timebase sync =
disabled
>> by KEXEC patch
>>=20
>> On Fri, Nov 18, 2011 at 08:35:02AM -0600, Kumar Gala wrote:
>>>=20
>>> On Nov 16, 2011, at 12:42 PM, Scott Wood wrote:
>>>=20
>>>> On 11/16/2011 03:55 AM, Zhao Chenhui wrote:
>>>>> From: Li Yang <leoli@freescale.com>
>>>>>=20
>>>>> The timebase sync is not only necessary when using KEXEC. It =
should
>> also
>>>>> be used by normal boot up and cpu hotplug. Remove the ifdef added =
by
>>>>> the KEXEC patch.
>>>>=20
>>>> Again, no it should not be used by normal boot up (whether KEXEC
>> support
>>>> is enabled or not).  We should only do timebase sync when we =
actually
>>>> need to (when we've actually just reset a core), and we should do =
it
>> the
>>>> way U-Boot does rather than with smp-tbsync.c.
>=20
> While looking into the timebase sync code in u-boot, I have a few =
questions.
>=20
>        /* enable time base at the platform */
>        if (whoami)
>                devdisr |=3D MPC85xx_DEVDISR_TB1;
>        else
>                devdisr |=3D MPC85xx_DEVDISR_TB0;
>        out_be32(&gur->devdisr, devdisr);
>=20
>        /* readback to sync write */
>        in_be32(&gur->devdisr);
>=20
>        mtspr(SPRN_TBWU, 0);
>        mtspr(SPRN_TBWL, 0);
>=20
> What are the TBWU/TBWL registers?  I can't find the definition of them =
in either e500 RM or booke RM.  Are they valid to be used?  What is the =
result of writing to them?  Aren't the SPR registers core specific?  How =
can we set the TB for the other cores?

TBWU/TBWL are SPR 284/285 (they might be called something a little =
different in the manual).

You can only set the TB on the core itself.  The way the u-boot code =
works is it sets TBL/TBU (TB) to 0 in release.S for secondary cores.  In =
the code you reference we are setting TB on the 'master' core to 0 since =
we turned TB on earlier for the master core and now are resetting it to =
0 to sync all the cores.

>=20
>        devdisr &=3D ~(MPC85xx_DEVDISR_TB0 | MPC85xx_DEVDISR_TB1);
>        out_be32(&gur->devdisr, devdisr);
>=20
> Also in the UM, I read "Blocks disabled by DEVDISR must not be =
re-enabled without a hard reset."  Is it safe we enable it here?

Yes this is safe, TB isn't truly a block.  Manual should technically be =
updated to exclude TB.

- k=

^ permalink raw reply

* Re: powerpc: dts: Fix canyonlands EMAC interrupt map
From: Josh Boyer @ 2011-11-22 14:31 UTC (permalink / raw)
  To: Tanmay Inamdar; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CACoXjc=RBpB6eYu1zB-kCcYmsV8rdbzYtdxyBJDLTdt6uu04ig@mail.gmail.com>

On Tue, Nov 22, 2011 at 9:15 AM, Tanmay Inamdar <tinamdar@apm.com> wrote:
>
> On Tue, Nov 22, 2011 at 5:00 PM, Josh Boyer <jwboyer@gmail.com> wrote:
>>
>> On Tue, Nov 22, 2011 at 2:11 AM, Tanmay Inamdar <tinamdar@apm.com> wrote=
:
>> > Fixing interrupt mapping of EMAC for canyonlands
>> >
>> > Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>>
>> As far as I can tell, your changes aren't really changing anything
>> just making it a bit clearer, correct? =A0If so, do you mind if I change
>> the commit log to "clear up" instead of fix?
>
> Actually Rob Herring's commit
> (http://git.kernel.org/?p=3Dlinux/kernel/git/torvalds/linux.git;a=3Dcommi=
t;h=3Ddc9372808412edbc653a675a526c2ee6c0c14a91)
> breaks the interrupt mapping in EMAC driver.
> I am trying to fix this issue by mapping interrupts in different way.

I see.  That should have been in the commit log then.  I'll add
something like that to it.

josh

^ permalink raw reply

* Re: powerpc: dts: Fix canyonlands EMAC interrupt map
From: Tanmay Inamdar @ 2011-11-22 14:15 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <CA+5PVA7BD9jNoV9LrXjk2+UPBW8MSTPkwpYPFj2y-CNkXCrAhA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4224 bytes --]

On Tue, Nov 22, 2011 at 5:00 PM, Josh Boyer <jwboyer@gmail.com> wrote:

> On Tue, Nov 22, 2011 at 2:11 AM, Tanmay Inamdar <tinamdar@apm.com> wrote:
> > Fixing interrupt mapping of EMAC for canyonlands
> >
> > Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>
> As far as I can tell, your changes aren't really changing anything
> just making it a bit clearer, correct?  If so, do you mind if I change
> the commit log to "clear up" instead of fix?
>

Actually Rob Herring's commit (
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=dc9372808412edbc653a675a526c2ee6c0c14a91)
breaks the interrupt mapping in EMAC driver.
I am trying to fix this issue by mapping interrupts in different way.

If you think "clear up" is fine, then please go ahead.

Thanks,
Tanmay

>
> josh
>
> > ---
> >  arch/powerpc/boot/dts/canyonlands.dts |   16 ++++++----------
> >  1 files changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/canyonlands.dts
> b/arch/powerpc/boot/dts/canyonlands.dts
> > index 3dc75de..c76bbcd 100644
> > --- a/arch/powerpc/boot/dts/canyonlands.dts
> > +++ b/arch/powerpc/boot/dts/canyonlands.dts
> > @@ -360,13 +360,11 @@
> >                        EMAC0: ethernet@ef600e00 {
> >                                device_type = "network";
> >                                compatible = "ibm,emac-460ex",
> "ibm,emac4sync";
> > -                               interrupt-parent = <&EMAC0>;
> > -                               interrupts = <0x0 0x1>;
> > -                               #interrupt-cells = <1>;
> > +                               interrupt-parent = <&UIC2>;
> >                                #address-cells = <0>;
> >                                #size-cells = <0>;
> > -                               interrupt-map = </*Status*/ 0x0 &UIC2
> 0x10 0x4
> > -                                                /*Wake*/   0x1 &UIC2
> 0x14 0x4>;
> > +                               interrupts = </*Status*/0x10 0x4
> > +                                               /*Wake*/0x14 0x4>;
> >                                reg = <0xef600e00 0x000000c4>;
> >                                local-mac-address = [000000000000]; /*
> Filled in by U-Boot */
> >                                mal-device = <&MAL0>;
> > @@ -390,13 +388,11 @@
> >                        EMAC1: ethernet@ef600f00 {
> >                                device_type = "network";
> >                                compatible = "ibm,emac-460ex",
> "ibm,emac4sync";
> > -                               interrupt-parent = <&EMAC1>;
> > -                               interrupts = <0x0 0x1>;
> > -                               #interrupt-cells = <1>;
> > +                               interrupt-parent = <&UIC2>;
> >                                #address-cells = <0>;
> >                                #size-cells = <0>;
> > -                               interrupt-map = </*Status*/ 0x0 &UIC2
> 0x11 0x4
> > -                                                /*Wake*/   0x1 &UIC2
> 0x15 0x4>;
> > +                               interrupts = </*Status*/0x11 0x4
> > +                                               /*Wake*/0x15 0x4>;
> >                                reg = <0xef600f00 0x000000c4>;
> >                                local-mac-address = [000000000000]; /*
> Filled in by U-Boot */
> >                                mal-device = <&MAL0>;
> > --
> > 1.6.1.rc3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
>

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
is for the sole use of the intended recipient(s) and contains information 
that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries. 
It is to be used solely for the purpose of furthering the parties' business relationship. 
All unauthorized review, use, disclosure or distribution is prohibited. 
If you are not the intended recipient, please contact the sender by reply e-mail 
and destroy all copies of the original message.

[-- Attachment #2: Type: text/html, Size: 5687 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox