LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: PCI-PCI bridge scanning broken on 460EX
From: Benjamin Herrenschmidt @ 2010-01-11 20:53 UTC (permalink / raw)
  To: Felix Radensky; +Cc: Stef van Os, Stefan Roese, Feng Kan, linuxppc-dev
In-Reply-To: <4B4B55CF.9000707@embedded-sol.com>

> It seems I was wrong. I've manually applied the patch at the wrong 
> place. After patching the correct function
> I'm not getting hard resets any more, which is a great improvement ! 
> Thanks a lot, I really appreciate your help !

This is somewhat funny... I wonder how it would have managed to find
anything behind the root complex P2P bridge with broken type 1 cycles...
very very strange.

> Unfortunately not all problems are gone. PLX is now identified 
> correctly, but device behind it is not detected,
> although u-boot detects it correctly. See below.

You have removed all your changes to that code right ?

Also the log still looks weird:

> pci 0000:01:02.0: scanning behind bridge, config 010100, pass 0
> pci 0000:01:02.0: bus configuration invalid, reconfiguring
> pci 0000:01:02.0: scanning behind bridge, config 010100, pass 1
> pci_bus 0000:01: bus scan returning with max=01
> pci 0000:00:02.0: scanning behind bridge, config 010100, pass 1
> pci_bus 0000:00: bus scan returning with max=01

Unless you left some experimental changes in, the above isn't right, the
"config" value should have changed due to the write of ~ffffff to it

If the code running is indeed unmodified from upsteam, can you try with
just that change:

        /* Check if setup is sensible at all */
-        if (!pass &&
-            ((buses & 0xff) != bus->number || ((buses >> 8) & 0xff) <= bus->number)) {
+        if (((buses & 0xff) != bus->number || ((buses >> 8) & 0xff) <= bus->number)) {
                 dev_dbg(&dev->dev, "bus configuration invalid, reconfiguring\n");
                 broken = 1;
         }

(IE remove the check for !pass)

Cheers,
Ben.

> pci 0000:00:02.0: disabling bridge window [mem 0xd80000000-0xd8c0fffff] 
> to [bus 01-01] (unused)
> pci 0000:00:02.0: PCI bridge to [bus 01-01]
> pci 0000:00:02.0:   bridge window [io  disabled]
> pci 0000:00:02.0:   bridge window [mem disabled]
> pci 0000:00:02.0:   bridge window [mem pref disabled]
> pci_bus 0000:00: resource 0 [io  0x0000-0xffff]
> pci_bus 0000:00: resource 1 [mem 0xd80000000-0xdffffffff]
> pci_bus 0000:01: resource 1 [??? 57982058496-58184433663 flags 0x0]
> 
> Any ideas what could be wrong now ?
> 
> Thanks a lot.
> 
> Felix.
> 

^ permalink raw reply

* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
From: Wolfgang Denk @ 2010-01-11 20:50 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Roman Fietze
In-Reply-To: <fa686aa41001111121u5c943fc7p789c0ec0b41af883@mail.gmail.com>

Dear Grant Likely,

In message <fa686aa41001111121u5c943fc7p789c0ec0b41af883@mail.gmail.com> you wrote:
>
> >  /* mpc52xx_lpbfifo.c */
> >  #define MPC52XX_LPBFIFO_FLAG_READ              (0)
> > -#define MPC52XX_LPBFIFO_FLAG_WRITE             (1<<0)
> > -#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT      (1<<1)
> > -#define MPC52XX_LPBFIFO_FLAG_NO_DMA            (1<<2)
> > -#define MPC52XX_LPBFIFO_FLAG_POLL_DMA          (1<<3)
> > +#define MPC52XX_LPBFIFO_FLAG_WRITE             BIT(0)
> > +#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT      BIT(1)
> > +#define MPC52XX_LPBFIFO_FLAG_NO_DMA            BIT(2)
> > +#define MPC52XX_LPBFIFO_FLAG_POLL_DMA          BIT(3)
> 
> I prefer the (1<<n) style myself.

Indeed, especially as one can argue that "(1<<0)" should be "BIT(31)"
on Power Architecture systems, where bit 0 is the MSB by definition.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Do not simplify the design of a program if a way can be found to make
it complex and wonderful.

^ permalink raw reply

* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
From: Wolfgang Denk @ 2010-01-11 20:43 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Roman Fietze
In-Reply-To: <fa686aa41001111115g3451c9b9h4d6c551afd5698e1@mail.gmail.com>

Dear Grant,

In message <fa686aa41001111115g3451c9b9h4d6c551afd5698e1@mail.gmail.com> you wrote:
> 
> Please don't.  I know that a lot of other 5200 code uses register map
> structures in this way, but I consider it bad practice.  I coded this

May I ask _why_ you consider this bad practice?

Is a structure not the most natural way to encode the specifics of a
hardware interface (address offet, bus width, etc.) in C?

What do you recommend instead?  Using lists of register offsets
(without any type information) as for example ARM is doing?

> driver without a structure for a reason.  The reason I haven't removed

Could you please explain this reason?


I'm trying to understand if this is a MPC52xx specific reasoning, or
if you apply this to all of PowerPC, or generally to all kernel code?

And: is this just your personal preferences, or generally agreed on?

Thanks in advance, and sorry for asking stupid questions, but your
reply surprised me...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Gods don't like people not doing much work. People  who  aren't  busy
all the time might start to _think_.  - Terry Pratchett, _Small Gods_

^ permalink raw reply

* Re: [PATCH 13/13] powerpc/5200: LocalPlus driver: clean up comments
From: Grant Likely @ 2010-01-11 20:24 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220813.04321.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:13 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>

If this description is no longer correct, then you must also add
comments describing the new behaviour.

Thanks for all the work on this.  I hope I haven't been too brutal on
my comments, but this device is subtle and the driver supports lots of
different transfer modes so I'm being cautious.  It will help if you
can tighten up your patches to split apart unrelated changes and to
write proper patch descriptions.

I look forward to your respin.

Cheers,
g.

> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 14 --------------
> =A01 files changed, 0 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index b2c92f5..a89072a 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -206,17 +206,6 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbf=
ifo_request *req)
> =A0/**
> =A0* mpc52xx_lpbfifo_sclpc_irq - IRQ handler for LPB FIFO
> =A0*
> - * On transmit, the dma completion irq triggers before the fifo
> - * completion triggers. =A0Handle the dma completion here instead of the
> - * LPB FIFO Bestcomm task completion irq because everything is not
> - * really done until the LPB FIFO completion irq triggers.
> - *
> - * In other words:
> - * For DMA, on receive, the "Fat Lady" is the bestcom completion irq. on
> - * transmit, the fifo completion irq is the "Fat Lady". The opera (or in
> - * this case the DMA/FIFO operation) is not finished until the "Fat
> - * Lady" sings.
> - *
> =A0* Reasons for entering this routine:
> =A0* 1) PIO mode rx and tx completion irq
> =A0* 2) DMA interrupt mode tx completion irq
> @@ -411,9 +400,6 @@ void mpc52xx_lpbfifo_poll(void)
> =A0{
> =A0 =A0 =A0 =A0struct mpc52xx_lpbfifo_request *req =3D lpbfifo.req;
>
> - =A0 =A0 =A0 /*
> - =A0 =A0 =A0 =A0* For more information, see comments on the "Fat Lady"
> - =A0 =A0 =A0 =A0*/
> =A0 =A0 =A0 =A0if (mpc52xx_lpbfifo_is_dma(req->flags) && (req->flags & MP=
C52XX_LPBFIFO_FLAG_WRITE))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc52xx_lpbfifo_sclpc_irq(0, NULL);
> =A0 =A0 =A0 =A0else
> --
> 1.6.5.5
>
>
>
> --
> Roman Fietze =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Telemotive AG B=FCro M=FChlha=
usen
> Breitwiesen =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A073=
347 M=FChlhausen
> Tel.: +49(0)7335/18493-45 =A0 =A0 =A0 =A0http://www.telemotive.de
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 11/13] powerpc/5200: LocalPlus driver: move RAM DMA address from request to driver
From: Grant Likely @ 2010-01-11 20:20 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220810.20622.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:10 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>

Please merge this change with the patch that adds the dma mapping

g.

> ---
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A01 -
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 13 +++++++------
> =A02 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/as=
m/mpc52xx.h
> index 043458e..91c65d0 100644
> --- a/arch/powerpc/include/asm/mpc52xx.h
> +++ b/arch/powerpc/include/asm/mpc52xx.h
> @@ -347,7 +347,6 @@ struct mpc52xx_lpbfifo_request {
>
> =A0 =A0 =A0 =A0/* Memory address */
> =A0 =A0 =A0 =A0void *data;
> - =A0 =A0 =A0 dma_addr_t data_dma;
>
> =A0 =A0 =A0 =A0/* Details of transfer */
> =A0 =A0 =A0 =A0size_t size;
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index cd8dc69..b2c92f5 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -41,6 +41,7 @@ struct mpc52xx_lpbfifo {
>
> =A0 =A0 =A0 =A0/* Current state data */
> =A0 =A0 =A0 =A0struct mpc52xx_lpbfifo_request *req;
> + =A0 =A0 =A0 dma_addr_t data_dma;
> =A0 =A0 =A0 =A0unsigned short irqs_pending;
> =A0 =A0 =A0 =A0int dma_irqs_enabled;
> =A0};
> @@ -49,7 +50,7 @@ struct mpc52xx_lpbfifo {
> =A0static struct mpc52xx_lpbfifo lpbfifo;
>
>
> -/* The order of the raised interrupts of SCLPC and BCOM cann not be
> +/* The order of the raised interrupts of SCLPC and BCOM cannot be
> =A0* predicted, because it depends on the individual BCOM and CPU
> =A0* loads. So in DMA mode we just wait for both until we finish the
> =A0* transaction. */
> @@ -160,7 +161,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 28);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(7));
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lpbfifo.bcom_cur_task =3D =
lpbfifo.bcom_tx_task;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->data_dma =3D dma_map_s=
ingle(lpbfifo.dev, req->data, req->size, DMA_TO_DEVICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.data_dma =3D dma_ma=
p_single(lpbfifo.dev, req->data, req->size, DMA_TO_DEVICE);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 1);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(0));
> @@ -177,7 +178,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0lpbfifo.dma_irqs_enabled =3D 1;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->data_dma =3D dma_map_s=
ingle(lpbfifo.dev, req->data, req->size, DMA_FROM_DEVICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.data_dma =3D dma_ma=
p_single(lpbfifo.dev, req->data, req->size, DMA_FROM_DEVICE);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* error irq & master enabled bit */
> @@ -187,7 +188,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd =3D bcom_prepare_next_buffer(lpbfifo.bc=
om_cur_task);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd->status =3D tc;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->data[0] =3D req->data_dma + req->pos;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->data[0] =3D lpbfifo.data_dma + req->pos=
;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bcom_submit_next_buffer(lpbfifo.bcom_cur_t=
ask, NULL);
> =A0 =A0 =A0 =A0}
>
> @@ -378,9 +379,9 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, =
void *dev_id)
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (req) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (mpc52xx_lpbfifo_is_wri=
te(lpbfifo->req->flags))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_s=
ingle(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_TO_DEVI=
CE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_s=
ingle(lpbfifo->dev, lpbfifo->data_dma, lpbfifo->req->size, DMA_TO_DEVICE);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_s=
ingle(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_FROM_DE=
VICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_s=
ingle(lpbfifo->dev, lpbfifo->data_dma, lpbfifo->req->size, DMA_FROM_DEVICE)=
;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lpbfifo->req =3D NULL;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo->regs->e=
nable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
> --
> 1.6.5.5
>
>
>
> --
> Roman Fietze =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Telemotive AG B=FCro M=FChlha=
usen
> Breitwiesen =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A073=
347 M=FChlhausen
> Tel.: +49(0)7335/18493-45 =A0 =A0 =A0 =A0http://www.telemotive.de
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order
From: Grant Likely @ 2010-01-11 20:19 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220809.09578.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:09 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> The order of the raised interrupts of SCLPC and BCOM cannot be
> predicted, because it depends on the individual BCOM and CPU loads. So
> in DMA mode we just wait for both until we finish the transaction.

I'm really not convinced.  It is true that the IRQ ordering may be
different, but by definition the BCOM *must* be finished before the
FIFO finishes on the TX path, and the FIFO definitely completes before
the BCOM completes on the RX path, regardless of the order IRQs are
actually processed.

g.

>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 94 +++++++++++++++=
++--------
> =A01 files changed, 64 insertions(+), 30 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 21b2a40..cd8dc69 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -32,7 +32,7 @@ struct mpc52xx_lpbfifo {
> =A0 =A0 =A0 =A0struct device *dev;
> =A0 =A0 =A0 =A0phys_addr_t regs_phys;
> =A0 =A0 =A0 =A0struct mpc52xx_sclpc __iomem *regs;
> - =A0 =A0 =A0 int irq;
> + =A0 =A0 =A0 int sclpc_irq;
> =A0 =A0 =A0 =A0spinlock_t lock;
>
> =A0 =A0 =A0 =A0struct bcom_task *bcom_tx_task;
> @@ -41,6 +41,7 @@ struct mpc52xx_lpbfifo {
>
> =A0 =A0 =A0 =A0/* Current state data */
> =A0 =A0 =A0 =A0struct mpc52xx_lpbfifo_request *req;
> + =A0 =A0 =A0 unsigned short irqs_pending;
> =A0 =A0 =A0 =A0int dma_irqs_enabled;
> =A0};
>
> @@ -48,6 +49,14 @@ struct mpc52xx_lpbfifo {
> =A0static struct mpc52xx_lpbfifo lpbfifo;
>
>
> +/* The order of the raised interrupts of SCLPC and BCOM cann not be
> + * predicted, because it depends on the individual BCOM and CPU
> + * loads. So in DMA mode we just wait for both until we finish the
> + * transaction. */
> +#define MPC52XX_LPBFIFO_PENDING_SCLPC =A0BIT(0)
> +#define MPC52XX_LPBFIFO_PENDING_BCOM =A0 BIT(1)
> +
> +
> =A0/**
> =A0* mpc52xx_lpbfifo_is_write - return true if it's a WRITE request
> =A0*/
> @@ -127,6 +136,8 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(r=
eg, *data++);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.irqs_pending =3D MPC52XX_LPBFIFO_PE=
NDING_SCLPC;
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Unmask both error and completion irqs *=
/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->enable, (MPC52xx_S=
CLPC_ENABLE_AIE |
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 MPC52xx_SCLPC_ENABLE_NIE |
> @@ -172,6 +183,8 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* error irq & master enabled bit */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->enable, MPC52xx_SC=
LPC_ENABLE_AIE | MPC52xx_SCLPC_ENABLE_NIE | MPC52xx_SCLPC_ENABLE_ME);
>
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.irqs_pending =3D MPC52XX_LPBFIFO_PE=
NDING_BCOM | MPC52XX_LPBFIFO_PENDING_SCLPC;
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd =3D bcom_prepare_next_buffer(lpbfifo.bc=
om_cur_task);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd->status =3D tc;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd->data[0] =3D req->data_dma + req->pos;
> @@ -188,6 +201,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bcom_enable(lpbfifo.bcom_cur_task);
> =A0}
>
> +
> =A0/**
> =A0* mpc52xx_lpbfifo_sclpc_irq - IRQ handler for LPB FIFO
> =A0*
> @@ -232,8 +246,9 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0*/
> =A0static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq, void *dev_id)
> =A0{
> + =A0 =A0 =A0 struct mpc52xx_lpbfifo *lpbfifo =3D dev_id;
> =A0 =A0 =A0 =A0struct mpc52xx_lpbfifo_request *req;
> - =A0 =A0 =A0 u32 status_count =3D in_be32(&lpbfifo.regs->bytes_done_stat=
us.bytes_done);
> + =A0 =A0 =A0 u32 status_count =3D in_be32(&lpbfifo->regs->bytes_done_sta=
tus.bytes_done);
> =A0 =A0 =A0 =A0void __iomem *reg;
> =A0 =A0 =A0 =A0u32 *data;
> =A0 =A0 =A0 =A0size_t i;
> @@ -242,18 +257,20 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int ir=
q, void *dev_id)
> =A0 =A0 =A0 =A0unsigned long flags;
> =A0 =A0 =A0 =A0int rflags;
>
> - =A0 =A0 =A0 spin_lock_irqsave(&lpbfifo.lock, flags);
> + =A0 =A0 =A0 spin_lock_irqsave(&lpbfifo->lock, flags);
> =A0 =A0 =A0 =A0ts =3D get_tbl();
>
> - =A0 =A0 =A0 req =3D lpbfifo.req;
> + =A0 =A0 =A0 req =3D lpbfifo->req;
> =A0 =A0 =A0 =A0if (!req) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo->lock, flag=
s);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pr_err("bogus SCLPC IRQ\n");
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return IRQ_HANDLED;
> =A0 =A0 =A0 =A0}
>
> + =A0 =A0 =A0 lpbfifo->irqs_pending &=3D ~MPC52XX_LPBFIFO_PENDING_SCLPC;
> +
> =A0 =A0 =A0 =A0rflags =3D req->flags;
> - =A0 =A0 =A0 status_count =3D in_be32(&lpbfifo.regs->bytes_done_status.b=
ytes_done);
> + =A0 =A0 =A0 status_count =3D in_be32(&lpbfifo->regs->bytes_done_status.=
bytes_done);
>
> =A0 =A0 =A0 =A0/* Check normal termination bit */
> =A0 =A0 =A0 =A0if (!(status_count & MPC52xx_SCLPC_STATUS_NT))
> @@ -261,19 +278,23 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int ir=
q, void *dev_id)
>
> =A0 =A0 =A0 =A0/* Check abort bit */
> =A0 =A0 =A0 =A0if (status_count & MPC52xx_SCLPC_STATUS_AT) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, MPC52xx_SCL=
PC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo->regs->enable, MPC52xx_SC=
LPC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0do_callback =3D 1;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out;
> =A0 =A0 =A0 =A0}
>
> - =A0 =A0 =A0 if (!mpc52xx_lpbfifo_is_dma(rflags)) {
> + =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_dma(rflags)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!lpbfifo->irqs_pending)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 do_callback =3D 1;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 else {
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Bytes done */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0status_count &=3D MPC52xx_SCLPC_STATUS_BYT=
ES_DONE_MASK;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!mpc52xx_lpbfifo_is_write(rflags)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* copy the data out of th=
e FIFO */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D &lpbfifo.regs->fifo=
_data;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D &lpbfifo->regs->fif=
o_data;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0data =3D req->data + req->=
pos;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0for (i =3D 0; i < status_c=
ount; i +=3D sizeof(u32))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*data++ =
=3D in_be32(reg);
> @@ -288,13 +309,10 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int ir=
q, void *dev_id)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0do_callback =3D 1;
> =A0 =A0 =A0 =A0}
> - =A0 =A0 =A0 else {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 do_callback =3D 1;
> - =A0 =A0 =A0 }
>
> =A0out:
> =A0 =A0 =A0 =A0/* Clear the IRQ */
> - =A0 =A0 =A0 out_8(&lpbfifo.regs->bytes_done_status.status, BIT(0));
> + =A0 =A0 =A0 out_8(&lpbfifo->regs->bytes_done_status.status, BIT(0));
>
> =A0 =A0 =A0 =A0req->last_byte =3D ((u8 *)req->data)[req->size - 1];
>
> @@ -304,11 +322,11 @@ out:
> =A0 =A0 =A0 =A0/* When the do_callback flag is set; it means the transfer=
 is finished
> =A0 =A0 =A0 =A0 * so set the FIFO as idle */
> =A0 =A0 =A0 =A0if (do_callback) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.req =3D NULL;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, MPC52xx_SCL=
PC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo->req =3D NULL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo->regs->enable, MPC52xx_SC=
LPC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0req->irq_ticks +=3D get_tbl() - ts;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo->lock, flag=
s);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Spinlock is released; it is now safe to=
 call the callback */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (req->callback)
> @@ -318,7 +336,7 @@ out:
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0req->irq_ticks +=3D get_tbl() - ts;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo->lock, flag=
s);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return IRQ_HANDLED;
> =A0 =A0 =A0 =A0}
> @@ -354,14 +372,30 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq=
, void *dev_id)
> =A0 =A0 =A0 =A0bcom_retrieve_buffer(lpbfifo->bcom_cur_task, NULL, NULL);
> =A0 =A0 =A0 =A0// req->irq_ticks +=3D get_tbl() - ts;
>
> - =A0 =A0 =A0 if (lpbfifo->req) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_write(lpbfifo->req->=
flags))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_single(lpbfifo->d=
ev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_TO_DEVICE);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_single(lpbfifo->d=
ev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_FROM_DEVICE);
> - =A0 =A0 =A0 } else
> - =A0 =A0 =A0 {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(lpbfifo->dev, "request is NULL\n");
> + =A0 =A0 =A0 lpbfifo->irqs_pending &=3D ~MPC52XX_LPBFIFO_PENDING_BCOM;
> + =A0 =A0 =A0 if (!lpbfifo->irqs_pending) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct mpc52xx_lpbfifo_request *req =3D lpb=
fifo->req;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (req) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_writ=
e(lpbfifo->req->flags))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_s=
ingle(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_TO_DEVI=
CE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_s=
ingle(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_FROM_DE=
VICE);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo->req =3D NULL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo->regs->en=
able, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpb=
fifo->lock, flags);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Spinlock is released; it=
 is now safe to call the callback */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (req->callback)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->callba=
ck(req);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return IRQ_HANDLED;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(lpbfifo->dev, "bogu=
s BCOM IRQ\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0spin_unlock_irqrestore(&lpbfifo->lock, flags);
> @@ -451,8 +485,8 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0if (lpbfifo.dev !=3D NULL)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENOSPC;
>
> - =A0 =A0 =A0 lpbfifo.irq =3D irq_of_parse_and_map(op->node, 0);
> - =A0 =A0 =A0 if (!lpbfifo.irq)
> + =A0 =A0 =A0 lpbfifo.sclpc_irq =3D irq_of_parse_and_map(op->node, 0);
> + =A0 =A0 =A0 if (!lpbfifo.sclpc_irq)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
>
> =A0 =A0 =A0 =A0if (of_address_to_resource(op->node, 0, &res))
> @@ -468,7 +502,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | =
MPC52xx_SCLPC_ENABLE_RF);
>
> =A0 =A0 =A0 =A0/* register the interrupt handler */
> - =A0 =A0 =A0 rc =3D request_irq(lpbfifo.irq, mpc52xx_lpbfifo_sclpc_irq, =
0,
> + =A0 =A0 =A0 rc =3D request_irq(lpbfifo.sclpc_irq, mpc52xx_lpbfifo_sclpc=
_irq, 0,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "mpc52xx-lpbfifo", &lpbfi=
fo);
> =A0 =A0 =A0 =A0if (rc)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_irq;
> @@ -539,7 +573,7 @@ static int __devexit mpc52xx_lpbfifo_remove(struct of=
_device *op)
> =A0 =A0 =A0 =A0free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo=
);
> =A0 =A0 =A0 =A0bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
>
> - =A0 =A0 =A0 free_irq(lpbfifo.irq, &lpbfifo);
> + =A0 =A0 =A0 free_irq(lpbfifo.sclpc_irq, &lpbfifo);
> =A0 =A0 =A0 =A0iounmap(lpbfifo.regs);
> =A0 =A0 =A0 =A0lpbfifo.regs =3D NULL;
> =A0 =A0 =A0 =A0lpbfifo.dev =3D NULL;
> @@ -547,7 +581,7 @@ static int __devexit mpc52xx_lpbfifo_remove(struct of=
_device *op)
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> -static struct of_device_id mpc52xx_lpbfifo_match[] __devinitconst =3D {
> +static const struct of_device_id mpc52xx_lpbfifo_match[] __devinitconst =
=3D {
> =A0 =A0 =A0 =A0{ .compatible =3D "fsl,mpc5200-lpbfifo", },
> =A0 =A0 =A0 =A0{},
> =A0};
> --
> 1.6.5.5
>
>
>
> --
> Roman Fietze =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Telemotive AG B=FCro M=FChlha=
usen
> Breitwiesen =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A073=
347 M=FChlhausen
> Tel.: +49(0)7335/18493-45 =A0 =A0 =A0 =A0http://www.telemotive.de
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 09/13] powerpc/5200: LocalPlus driver: smarter calculation of BPT, bytes per transfer
From: Grant Likely @ 2010-01-11 20:15 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220808.14759.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:08 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 33 +++++++++++++++=
----------
> =A01 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 48f2b4f..21b2a40 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -80,11 +80,11 @@ static inline int mpc52xx_lpbfifo_is_poll_dma(int fla=
gs)
> =A0*/
> =A0static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_request *req)
> =A0{
> - =A0 =A0 =A0 size_t transfer_size =3D req->size - req->pos;
> + =A0 =A0 =A0 size_t tc =3D req->size - req->pos;
> =A0 =A0 =A0 =A0struct bcom_bd *bd;
> =A0 =A0 =A0 =A0void __iomem *reg;
> =A0 =A0 =A0 =A0u32 *data;
> - =A0 =A0 =A0 u32 bit_fields;
> + =A0 =A0 =A0 u32 control;

Changing the name of these two variables makes it hard to review the
functional change.  A lot of unrelated housekeeping gets interwoven.

> =A0 =A0 =A0 =A0int rflags =3D req->flags;
>
> =A0 =A0 =A0 =A0/* Set and clear the reset bits; is good practice in User =
Manual */
> @@ -93,10 +93,10 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0/* Set width, chip select and READ mode */
> =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->start_address, req->offset + req->=
pos);
>
> - =A0 =A0 =A0 /* Set CS and BPT */
> - =A0 =A0 =A0 bit_fields =3D MPC52xx_SCLPC_CONTROL_CS(req->cs) | 0x8;
> + =A0 =A0 =A0 /* Setup CS */
> + =A0 =A0 =A0 control =3D MPC52xx_SCLPC_CONTROL_CS(req->cs);
> =A0 =A0 =A0 =A0if (!(mpc52xx_lpbfifo_is_write(rflags)))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCLPC_CONTROL_RWB_R=
ECEIVE; =A0 =A0 =A0 =A0/* read mode */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 control |=3D MPC52xx_SCLPC_CONTROL_RWB_RECE=
IVE; =A0 /* read mode */
>
> =A0 =A0 =A0 =A0if (!mpc52xx_lpbfifo_is_dma(rflags)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* While the FIFO can be setup for transfe=
r sizes as
> @@ -112,10 +112,10 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpb=
fifo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * The last block of data will be received=
 with the
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * flush bit set. This avoids stale read d=
ata.
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (transfer_size > 512)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 transfer_size =3D 512;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (tc > 512)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tc =3D 512;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else if (!(mpc52xx_lpbfifo_is_write(rflags=
)))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCL=
PC_CONTROL_FLUSH;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 control |=3D MPC52xx_SCLPC_=
CONTROL_FLUSH;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Load the FIFO with data */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (mpc52xx_lpbfifo_is_write(rflags)) {
> @@ -123,7 +123,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D &lpbfifo.regs->fif=
o_data;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0data =3D req->data + req->=
pos;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (i =3D 0; i < transfer_=
size; i +=3D 4)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (i =3D 0; i < tc; i +=
=3D 4)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(r=
eg, *data++);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> @@ -136,7 +136,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* In DMA mode we can always set the flush=
 bit to avoid
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * stale read data. */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!(mpc52xx_lpbfifo_is_write(rflags)))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCL=
PC_CONTROL_FLUSH;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 control |=3D MPC52xx_SCLPC_=
CONTROL_FLUSH;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Choose the correct direction
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *
> @@ -173,15 +173,17 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpb=
fifo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->enable, MPC52xx_SC=
LPC_ENABLE_AIE | MPC52xx_SCLPC_ENABLE_NIE | MPC52xx_SCLPC_ENABLE_ME);
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd =3D bcom_prepare_next_buffer(lpbfifo.bc=
om_cur_task);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->status =3D transfer_size;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->status =3D tc;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd->data[0] =3D req->data_dma + req->pos;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bcom_submit_next_buffer(lpbfifo.bcom_cur_t=
ask, NULL);
> =A0 =A0 =A0 =A0}
>
> - =A0 =A0 =A0 out_be32(&lpbfifo.regs->control, bit_fields);
> + =A0 =A0 =A0 /* Setup BPT. tc is already screened and a multiple of 4 */
> + =A0 =A0 =A0 control |=3D tc & 7 ? 4 : 8;
> + =A0 =A0 =A0 out_be32(&lpbfifo.regs->control, control);

The calculation looks correct.  However, why isn't the transfer size
calculated up by the /* Setup CS and BPT */ comment?  I don't think it
needs to be down here.

>
> =A0 =A0 =A0 =A0/* Set packet size and kick it off */
> - =A0 =A0 =A0 out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_SC=
LPC_PACKET_SIZE_RESTART | transfer_size);
> + =A0 =A0 =A0 out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_SC=
LPC_PACKET_SIZE_RESTART | tc);
> =A0 =A0 =A0 =A0if (mpc52xx_lpbfifo_is_dma(rflags))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bcom_enable(lpbfifo.bcom_cur_task);
> =A0}
> @@ -395,6 +397,11 @@ int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_re=
quest *req)
> =A0 =A0 =A0 =A0if (!lpbfifo.regs)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
>
> + =A0 =A0 =A0 /* The gen bd BestComm task currently only allows an increm=
ent
> + =A0 =A0 =A0 =A0* of 4 */
> + =A0 =A0 =A0 if (!req->size || req->size & 0x03)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> +
> =A0 =A0 =A0 =A0spin_lock_irqsave(&lpbfifo.lock, flags);
>
> =A0 =A0 =A0 =A0/* If the req pointer is already set, then a transfer is i=
n progress */
> --
> 1.6.5.5
>
>
>
> --
> Roman Fietze =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Telemotive AG B=FCro M=FChlha=
usen
> Breitwiesen =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A073=
347 M=FChlhausen
> Tel.: +49(0)7335/18493-45 =A0 =A0 =A0 =A0http://www.telemotive.de
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Sleep-capable GPIO slave-selects on MPC52xx?
From: Bill Gatliff @ 2010-01-11 20:13 UTC (permalink / raw)
  To: linuxppc-dev

Guys:


A platform I have inherited utilizes a GPIO on an I2C expander chip
(MAX7314) as a SPI slave-select.  I'm using the actual MPC52xx SPI
peripheral, not a PSC.

It looks like the current version of the MPC52xx SPI driver won't work
with sleep-capable GPIOs for slave-selects.  In particular, it looks
like mpc52xx_spi_fsmstate_transfer() is an interrupt handler that calls
mpc52xx_spi_chipsel(), which itself calls gpio_set_value().  Or, at
least my kernel thinks so, since I get a barrage of oops-type output
screaming at me whenever I hit the SPI device.  :)

Am I missing something, or is this a known (or at least now-identified)
limitation of the current mpc52xx_spi.c?


Thanks!


b.g.

-- 
Bill Gatliff
Embedded systems training and consulting
http://billgatliff.com
bgat@billgatliff.com

^ permalink raw reply

* Re: [PATCH 08/13] powerpc/5200: LocalPlus driver: smart flush of receive FIFO
From: Grant Likely @ 2010-01-11 20:06 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220806.33304.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:06 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>

Need patch description

> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 40 +++++++++++++++=
+---------
> =A01 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index a7cd585..48f2b4f 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -84,8 +84,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
> =A0 =A0 =A0 =A0struct bcom_bd *bd;
> =A0 =A0 =A0 =A0void __iomem *reg;
> =A0 =A0 =A0 =A0u32 *data;
> - =A0 =A0 =A0 int i;
> - =A0 =A0 =A0 int bit_fields;
> + =A0 =A0 =A0 u32 bit_fields;
> =A0 =A0 =A0 =A0int rflags =3D req->flags;
>
> =A0 =A0 =A0 =A0/* Set and clear the reset bits; is good practice in User =
Manual */
> @@ -96,27 +95,32 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
>
> =A0 =A0 =A0 =A0/* Set CS and BPT */
> =A0 =A0 =A0 =A0bit_fields =3D MPC52xx_SCLPC_CONTROL_CS(req->cs) | 0x8;
> - =A0 =A0 =A0 if (!(mpc52xx_lpbfifo_is_write(rflags))) {
> + =A0 =A0 =A0 if (!(mpc52xx_lpbfifo_is_write(rflags)))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bit_fields |=3D MPC52xx_SCLPC_CONTROL_RWB_=
RECEIVE; =A0 =A0 =A0 =A0/* read mode */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH=
;
> - =A0 =A0 =A0 }
> - =A0 =A0 =A0 out_be32(&lpbfifo.regs->control, bit_fields);

Writing the control register is being deferred to later.  I'm not
convinced this is correct (see comment on previous patch).

>
> =A0 =A0 =A0 =A0if (!mpc52xx_lpbfifo_is_dma(rflags)) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* While the FIFO can be setup for transfer=
 sizes as large as
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 16M-1, the FIFO itself is only 512 byt=
es deep and it does
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* not generate interrupts for FIFO full =
events (only transfer
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* complete will raise an IRQ). =A0Theref=
ore when not using
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Bestcomm to drive the FIFO it needs to=
 either be polled, or
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* transfers need to constrained to the s=
ize of the fifo.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* While the FIFO can be setup for transfer=
 sizes as
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* large as 16M-1, the FIFO itself is onl=
y 512 bytes
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* deep and it does not generate interrup=
ts for FIFO
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* full events (only transfer complete wi=
ll raise an
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* IRQ). Therefore when not using Bestcom=
m to drive the
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* FIFO it needs to either be polled, or =
transfers need
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* to constrained to the size of the fifo=
.

Drop formatting changes or spilt to separate patch.

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * This driver restricts the size of the t=
ransfer
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* The last block of data will be receive=
d with the
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* flush bit set. This avoids stale read =
data.
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (transfer_size > 512)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0transfer_size =3D 512;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else if (!(mpc52xx_lpbfifo_is_write(rflags)=
))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCL=
PC_CONTROL_FLUSH;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Load the FIFO with data */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (mpc52xx_lpbfifo_is_write(rflags)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 size_t i;
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D &lpbfifo.regs->fif=
o_data;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0data =3D req->data + req->=
pos;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0for (i =3D 0; i < transfer=
_size; i +=3D 4)
> @@ -128,6 +132,12 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbf=
ifo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 MPC52xx_SCLPC_ENABLE_NIE |
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 MPC52xx_SCLPC_ENABLE_ME));
> =A0 =A0 =A0 =A0} else {
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* In DMA mode we can always set the flush =
bit to avoid
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* stale read data. */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!(mpc52xx_lpbfifo_is_write(rflags)))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCL=
PC_CONTROL_FLUSH;
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Choose the correct direction
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Configure the watermarks so DMA will al=
ways complete correctly.
> @@ -168,6 +178,8 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bcom_submit_next_buffer(lpbfifo.bcom_cur_t=
ask, NULL);
> =A0 =A0 =A0 =A0}
>
> + =A0 =A0 =A0 out_be32(&lpbfifo.regs->control, bit_fields);
> +
> =A0 =A0 =A0 =A0/* Set packet size and kick it off */
> =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_S=
CLPC_PACKET_SIZE_RESTART | transfer_size);
> =A0 =A0 =A0 =A0if (mpc52xx_lpbfifo_is_dma(rflags))
> @@ -455,7 +467,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_irq;
>
> =A0 =A0 =A0 =A0/* Request the Bestcomm receive (fifo --> memory) task and=
 IRQ */
> - =A0 =A0 =A0 lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(4,
> + =A0 =A0 =A0 lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(2,

unrelated change (and this line was also changed in an earlier patch)

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 res.start + offsetof(struct mpc52xx_sclpc, fifo=
_data),
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 16 * 1024 * 1024);
> @@ -469,7 +481,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_bcom_rx_irq;
>
> =A0 =A0 =A0 =A0/* Request the Bestcomm transmit (memory --> fifo) task an=
d IRQ */
> - =A0 =A0 =A0 lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(4,
> + =A0 =A0 =A0 lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(2,

ditto

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 res.start + offsetof(struct mpc52xx_sclpc, fifo=
_data),
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_INITIATOR_SCLPC,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_IPR_SCLPC);
> --
> 1.6.5.5
>
>
>
> --
> Roman Fietze =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Telemotive AG B=FCro M=FChlha=
usen
> Breitwiesen =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A073=
347 M=FChlhausen
> Tel.: +49(0)7335/18493-45 =A0 =A0 =A0 =A0http://www.telemotive.de
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 07/13] powerpc/5200: LocalPlus driver: reset BestComm when committing new request
From: Grant Likely @ 2010-01-11 20:00 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220805.20200.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:05 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>

Again, need a description as to 'why?'

> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 =A04 +++-
> =A01 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 8d8a63a..a7cd585 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -398,6 +398,8 @@ int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_req=
uest *req)
> =A0 =A0 =A0 =A0req->buffer_not_done_cnt =3D 0;
> =A0 =A0 =A0 =A0req->pos =3D 0;
>
> + =A0 =A0 =A0 bcom_gen_bd_rx_reset(lpbfifo.bcom_rx_task);
> + =A0 =A0 =A0 bcom_gen_bd_tx_reset(lpbfifo.bcom_tx_task);
> =A0 =A0 =A0 =A0mpc52xx_lpbfifo_kick(req);
> =A0 =A0 =A0 =A0spin_unlock_irqrestore(&lpbfifo.lock, flags);
>
> @@ -456,7 +458,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(4,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 res.start + offsetof(struct mpc52xx_sclpc, fifo=
_data),
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A016*1024*1024);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A016 * 1024 * 1024);

Unrelated change.  Please drop.

g.


--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
From: Grant Likely @ 2010-01-11 19:59 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Roman Fietze
In-Reply-To: <4B4B7F13.3090809@freescale.com>

On Mon, Jan 11, 2010 at 12:42 PM, Scott Wood <scottwood@freescale.com> wrot=
e:
> Grant Likely wrote:
>>
>> Please don't. =A0I know that a lot of other 5200 code uses register map
>> structures in this way, but I consider it bad practice. =A0I coded this
>> driver without a structure for a reason. =A0The reason I haven't removed
>> the other 5200 register map structures is the code impact would be
>> huge, it would probably cause breakage, and it would break all
>> out-of-tree patches touching the same code for no measurable
>> advantage.
>
> FWIW, over on the U-Boot side patches are getting NACKed by Wolfgang if t=
hey
> don't use register structures. :-P
>
> They're nice from a type-safety and namespacing perspective, though they =
get
> ugly pretty quickly if there are gaps.

Regardless, I see no reason to change existing code in either direction.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 06/13] powerpc/5200: LocalPlus driver: map and unmap DMA areas
From: Grant Likely @ 2010-01-11 19:57 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220804.05543.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:04 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>

Yes, this is definitely needed.  Please respin this patch and move it
earlier in your series so I can apply it to mainline.

More comments below.

g.

> ---
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A02 +=
-
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 26 +++++++++++++++=
++++-----
> =A02 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/as=
m/mpc52xx.h
> index c659d1d..043458e 100644
> --- a/arch/powerpc/include/asm/mpc52xx.h
> +++ b/arch/powerpc/include/asm/mpc52xx.h
> @@ -347,7 +347,7 @@ struct mpc52xx_lpbfifo_request {
>
> =A0 =A0 =A0 =A0/* Memory address */
> =A0 =A0 =A0 =A0void *data;
> - =A0 =A0 =A0 phys_addr_t data_phys;
> + =A0 =A0 =A0 dma_addr_t data_dma;
>
> =A0 =A0 =A0 =A0/* Details of transfer */
> =A0 =A0 =A0 =A0size_t size;
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 1e4f725..8d8a63a 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -14,6 +14,7 @@
> =A0#include <linux/of.h>
> =A0#include <linux/of_platform.h>
> =A0#include <linux/spinlock.h>
> +#include <linux/dma-mapping.h>
> =A0#include <asm/io.h>
> =A0#include <asm/prom.h>
> =A0#include <asm/mpc52xx.h>
> @@ -138,6 +139,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 28);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(7));
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lpbfifo.bcom_cur_task =3D =
lpbfifo.bcom_tx_task;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->data_dma =3D dma_map_s=
ingle(lpbfifo.dev, req->data, req->size, DMA_TO_DEVICE);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 1);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->fi=
fo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(0));
> @@ -154,6 +156,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0lpbfifo.dma_irqs_enabled =3D 1;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->data_dma =3D dma_map_s=
ingle(lpbfifo.dev, req->data, req->size, DMA_FROM_DEVICE);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}

Need to ensure the return value !=3D NULL

>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* error irq & master enabled bit */
> @@ -161,7 +164,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd =3D bcom_prepare_next_buffer(lpbfifo.bc=
om_cur_task);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bd->status =3D transfer_size;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->data[0] =3D req->data_phys + req->pos;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->data[0] =3D req->data_dma + req->pos;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bcom_submit_next_buffer(lpbfifo.bcom_cur_t=
ask, NULL);
> =A0 =A0 =A0 =A0}
>
> @@ -236,12 +239,13 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int ir=
q, void *dev_id)
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0rflags =3D req->flags;
> + =A0 =A0 =A0 status_count =3D in_be32(&lpbfifo.regs->bytes_done_status.b=
ytes_done);
>
> - =A0 =A0 =A0 /* check normal termination bit */
> + =A0 =A0 =A0 /* Check normal termination bit */
> =A0 =A0 =A0 =A0if (!(status_count & MPC52xx_SCLPC_STATUS_NT))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out;
>
> - =A0 =A0 =A0 /* check abort bit */
> + =A0 =A0 =A0 /* Check abort bit */

unrelated changes

> =A0 =A0 =A0 =A0if (status_count & MPC52xx_SCLPC_STATUS_AT) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_be32(&lpbfifo.regs->enable, MPC52xx_SC=
LPC_ENABLE_RC | MPC52xx_SCLPC_ENABLE_RF);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0do_callback =3D 1;
> @@ -250,7 +254,7 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq,=
 void *dev_id)
>
> =A0 =A0 =A0 =A0if (!mpc52xx_lpbfifo_is_dma(rflags)) {
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* bytes done */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Bytes done */

ditto

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0status_count &=3D MPC52xx_SCLPC_STATUS_BYT=
ES_DONE_MASK;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!mpc52xx_lpbfifo_is_write(rflags)) {
> @@ -336,6 +340,16 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq,=
 void *dev_id)
> =A0 =A0 =A0 =A0bcom_retrieve_buffer(lpbfifo->bcom_cur_task, NULL, NULL);
> =A0 =A0 =A0 =A0// req->irq_ticks +=3D get_tbl() - ts;
>
> + =A0 =A0 =A0 if (lpbfifo->req) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_write(lpbfifo->req->=
flags))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_single(lpbfifo->d=
ev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_TO_DEVICE);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dma_unmap_single(lpbfifo->d=
ev, lpbfifo->req->data_dma, lpbfifo->req->size, DMA_FROM_DEVICE);
> + =A0 =A0 =A0 } else
> + =A0 =A0 =A0 {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(lpbfifo->dev, "request is NULL\n");
> + =A0 =A0 =A0 }
> +

The ->req pointer was verified earlier in this function.  It will
never be null here.

> =A0 =A0 =A0 =A0spin_unlock_irqrestore(&lpbfifo->lock, flags);
>
> =A0 =A0 =A0 =A0return IRQ_HANDLED;
> @@ -439,7 +453,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_irq;
>
> =A0 =A0 =A0 =A0/* Request the Bestcomm receive (fifo --> memory) task and=
 IRQ */
> - =A0 =A0 =A0 lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(16,
> + =A0 =A0 =A0 lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(4,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 res.start + offsetof(struct mpc52xx_sclpc, fifo=
_data),
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 16*1024*1024);

Why?

> @@ -453,7 +467,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_bcom_rx_irq;
>
> =A0 =A0 =A0 =A0/* Request the Bestcomm transmit (memory --> fifo) task an=
d IRQ */
> - =A0 =A0 =A0 lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(16,
> + =A0 =A0 =A0 lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(4,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 res.start + offsetof(struct mpc52xx_sclpc, fifo=
_data),
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_INITIATOR_SCLPC,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 BCOM_IPR_SCLPC);

Ditto.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 04/13] mpc52xx: LocalPlus driver: rewrite interrupt routines, fix errors
From: Grant Likely @ 2010-01-11 19:44 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220801.53720.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:01 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> Use SCLPC bit definitions from mpc52xx.h for better readability.

The changes of is_write etc. are intermingled with the functional
changes being made.  The functional behaviour of this thing is subtle,
and I'd prefer the stylistic stuff handled in a separate patch.

> Rewrite IRQ handlers, make them work for DMA.

Details please.  As far as my testing goes, dma irqs are working fine.

> Fix module unload error.

ditto here.

>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0306 ++++++++++++---=
----------
> =A01 files changed, 149 insertions(+), 157 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 2763d5e..2fd1f3f 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -46,6 +46,34 @@ struct mpc52xx_lpbfifo {
> =A0/* The MPC5200 has only one fifo, so only need one instance structure =
*/
> =A0static struct mpc52xx_lpbfifo lpbfifo;
>
> +
> +/**
> + * mpc52xx_lpbfifo_is_write - return true if it's a WRITE request
> + */
> +static inline int mpc52xx_lpbfifo_is_write(int flags)
> +{
> + =A0 =A0 =A0 return flags & MPC52XX_LPBFIFO_FLAG_WRITE;
> +}
> +
> +
> +/**
> + * mpc52xx_lpbfifo_is_dma - return true if it's a DMA request
> + */
> +static inline int mpc52xx_lpbfifo_is_dma(int flags)
> +{
> + =A0 =A0 =A0 return !(flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
> +}
> +
> +
> +/**
> + * mpc52xx_lpbfifo_is_poll_dma - return true if it's a polled DMA reques=
t
> + */
> +static inline int mpc52xx_lpbfifo_is_poll_dma(int flags)
> +{
> + =A0 =A0 =A0 return flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
> +}
> +
> +

I'm not (yet) convinced that adding these is a benefit.

> =A0/**
> =A0* mpc52xx_lpbfifo_kick - Trigger the next block of data to be transfer=
ed
> =A0*/
> @@ -57,16 +85,23 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
> =A0 =A0 =A0 =A0u32 *data;
> =A0 =A0 =A0 =A0int i;
> =A0 =A0 =A0 =A0int bit_fields;
> - =A0 =A0 =A0 int dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
> - =A0 =A0 =A0 int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
> - =A0 =A0 =A0 int poll_dma =3D req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA=
;
> + =A0 =A0 =A0 int rflags =3D req->flags;
>
> =A0 =A0 =A0 =A0/* Set and clear the reset bits; is good practice in User =
Manual */
> - =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, 0x01010000);
> + =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | M=
PC52xx_SCLPC_ENABLE_RF);
> +
> + =A0 =A0 =A0 /* Set width, chip select and READ mode */
> + =A0 =A0 =A0 out_be32(&lpbfifo.regs->start_address, req->offset + req->p=
os);
> +
> + =A0 =A0 =A0 /* Set CS and BPT */
> + =A0 =A0 =A0 bit_fields =3D MPC52xx_SCLPC_CONTROL_CS(req->cs) | 0x8;
> + =A0 =A0 =A0 if (!(mpc52xx_lpbfifo_is_write(rflags))) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCLPC_CONTROL_RWB_R=
ECEIVE; =A0 =A0 =A0 =A0/* read mode */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH=
;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 out_be32(&lpbfifo.regs->control, bit_fields);
>
> - =A0 =A0 =A0 /* set master enable bit */
> - =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, 0x00000001);

My experimenting has found that clearing the reset bits and setting
the master enable bit is needed before programming the FIFO.  It looks
to me like this patch drops the above line which does so.

> - =A0 =A0 =A0 if (!dma) {
> + =A0 =A0 =A0 if (!mpc52xx_lpbfifo_is_dma(rflags)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* While the FIFO can be setup for transfe=
r sizes as large as
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * 16M-1, the FIFO itself is only 512 byte=
s deep and it does
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * not generate interrupts for FIFO full e=
vents (only transfer
> @@ -80,7 +115,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfif=
o_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0transfer_size =3D 512;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Load the FIFO with data */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_write(rflags)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D &lpbfifo.regs->fif=
o_data;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0data =3D req->data + req->=
pos;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0for (i =3D 0; i < transfer=
_size; i +=3D 4)
> @@ -88,7 +123,9 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfif=
o_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Unmask both error and completion irqs *=
/
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, 0x00000301)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, (MPC52xx_SC=
LPC_ENABLE_AIE |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0MPC52xx_SCLPC_ENABLE_NIE |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0MPC52xx_SCLPC_ENABLE_ME));
> =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Choose the correct direction
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *
> @@ -97,16 +134,16 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbf=
ifo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * there is a performance impacit. =A0Howe=
ver, if it is wrong there
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * is a risk of DMA not transferring the l=
ast chunk of data
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->fif=
o_alarm, 0x1e4);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_8(&lpbfifo.regs->fifo_c=
ontrol, 7);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_write(rflags)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->fif=
o_alarm, MPC52xx_SCLPC_FIFO_SIZE - 28);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->fif=
o_control, MPC52xx_SLPC_FIFO_CONTROL_GR(7));
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lpbfifo.bcom_cur_task =3D =
lpbfifo.bcom_tx_task;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->fif=
o_alarm, 0x1ff);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_8(&lpbfifo.regs->fifo_c=
ontrol, 0);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->fif=
o_alarm, MPC52xx_SCLPC_FIFO_SIZE - 1);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->fif=
o_control, MPC52xx_SLPC_FIFO_CONTROL_GR(0));

More stylistic changes in a patch that also changes functional
behaviour.  Please split into separate patches.

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lpbfifo.bcom_cur_task =3D =
lpbfifo.bcom_rx_task;
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (poll_dma) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mpc52xx_lpbfifo_is_poll=
_dma(rflags)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (lpbfif=
o.dma_irqs_enabled) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0disable_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task));
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0lpbfifo.dma_irqs_enabled =3D 0;
> @@ -119,63 +156,34 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpb=
fifo_request *req)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* error irq & master enabled bit */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&lpbfifo.regs->enable, MPC52xx_SCL=
PC_ENABLE_AIE | MPC52xx_SCLPC_ENABLE_NIE | MPC52xx_SCLPC_ENABLE_ME);
> +

You'll need to explain this change it greater detail.  The old code
only enabled the NIE irq when in non-polled DMA mode.  You're changing
it and you need to explain why.  Not just for me but for future
readers.

I'm stopping here on this particular patch.  A number of comments have
been removed that describe the behaviour of the driver without being
replaced with comments describing the new behaviour.  There are also
several whitespace and style only change that should be done in a
separate patch.  Keeping them separate means I can merge
uncontroversial stuff while still debating the other points.

Please respin and describe not just what you've change, but how you
changed it, why the change is needed, and it should be clear what the
new behaviour model is.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
From: Scott Wood @ 2010-01-11 19:42 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Roman Fietze
In-Reply-To: <fa686aa41001111115g3451c9b9h4d6c551afd5698e1@mail.gmail.com>

Grant Likely wrote:
> Please don't.  I know that a lot of other 5200 code uses register map
> structures in this way, but I consider it bad practice.  I coded this
> driver without a structure for a reason.  The reason I haven't removed
> the other 5200 register map structures is the code impact would be
> huge, it would probably cause breakage, and it would break all
> out-of-tree patches touching the same code for no measurable
> advantage.

FWIW, over on the U-Boot side patches are getting NACKed by Wolfgang if 
they don't use register structures. :-P

They're nice from a type-safety and namespacing perspective, though they 
get ugly pretty quickly if there are gaps.

-Scott

^ permalink raw reply

* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
From: Grant Likely @ 2010-01-11 19:21 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220800.54373.roman.fietze@telemotive.de>

On Tue, Dec 22, 2009 at 12:00 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>

This should probably be merged with the first patch to actually use
the bit definitions.  More comments below.

> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/include/asm/mpc52xx.h | =A0 40 ++++++++++++++++++++++++++=
+--------
> =A01 files changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/as=
m/mpc52xx.h
> index 57f8335..c659d1d 100644
> --- a/arch/powerpc/include/asm/mpc52xx.h
> +++ b/arch/powerpc/include/asm/mpc52xx.h
> @@ -17,6 +17,7 @@
> =A0#include <asm/types.h>
> =A0#include <asm/prom.h>
> =A0#include <asm/mpc5xxx.h>
> +#include <linux/bitops.h>
> =A0#endif /* __ASSEMBLY__ */
>
> =A0#include <linux/suspend.h>
> @@ -212,11 +213,34 @@ struct mpc52xx_sclpc {
>
> =A0 =A0 =A0 =A0u32 fifo_data; =A0 =A0 =A0 =A0 =A0/* 0x40 FIFO data word r=
egister */
> =A0 =A0 =A0 =A0u32 fifo_status; =A0 =A0 =A0 =A0/* 0x44 FIFO status regist=
er */
> - =A0 =A0 =A0 u8 fifo_control; =A0 =A0 =A0 =A0/* 0x48 FIFO control regist=
er */
> - =A0 =A0 =A0 u8 reserved2[3];
> + =A0 =A0 =A0 u32 fifo_control; =A0 =A0 =A0 /* 0x48 FIFO control register=
 */
> =A0 =A0 =A0 =A0u32 fifo_alarm; =A0 =A0 =A0 =A0 /* 0x4C FIFO alarm registe=
r */
> =A0};
>
> +#define MPC52xx_SCLPC_FIFO_SIZE =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0(0x200) =A0 =A0 =A0 =A0 /* FIFO size 512 bytes */
> +
> +#define MPC52xx_SCLPC_CONTROL_CS(cs) =A0 =A0 =A0 =A0 =A0 ((uint32_t)(cs)=
 << 24) =A0/* CSX bits */
> +#define MPC52xx_SCLPC_CONTROL_FLUSH =A0 =A0 =A0 =A0 =A0 =A0BIT(17) =A0 =
=A0 =A0 =A0 /* flush, used in last packet =A0*/
> +#define MPC52xx_SCLPC_CONTROL_RWB_RECEIVE =A0 =A0 =A0BIT(16) =A0 =A0 =A0=
 =A0 /* RWb bit, 1 =3D receive */
> +#define MPC52xx_SCLPC_CONTROL_DAI =A0 =A0 =A0 =A0 =A0 =A0 =A0BIT(8)
> +
> +#define MPC52xx_SCLPC_ENABLE_RC =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0BIT(24) =A0 =A0 =A0 =A0 /* reset controller bit */
> +#define MPC52xx_SCLPC_ENABLE_RF =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0BIT(16) =A0 =A0 =A0 =A0 /* reset FIFO bit */
> +#define MPC52xx_SCLPC_ENABLE_AIE =A0 =A0 =A0 =A0 =A0 =A0 =A0 BIT(9) =A0 =
=A0 =A0 =A0 =A0/* abort interrupt enable bit */
> +#define MPC52xx_SCLPC_ENABLE_NIE =A0 =A0 =A0 =A0 =A0 =A0 =A0 BIT(8) =A0 =
=A0 =A0 =A0 =A0/* normal interrupt enable bit */
> +#define MPC52xx_SCLPC_ENABLE_ME =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0BIT(0) =A0 =A0 =A0 =A0 =A0/* master enable bit */
> +
> +#define MPC52xx_SCLPC_PACKET_SIZE_RESTART =A0 =A0 =A0BIT(24)
> +
> +#define MPC52xx_SCLPC_STATUS_AT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0BIT(28) =A0 =A0 =A0 =A0 /* abort termination */
> +#define MPC52xx_SCLPC_STATUS_NT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0BIT(24) =A0 =A0 =A0 =A0 /* normal termination */
> +#define MPC52xx_SCLPC_STATUS_BYTES_DONE_MASK =A0 (0x00FFFFFFU) =A0 /* by=
tes done bit mask */
> +
> +#define MPC52xx_SLPC_FIFO_STATUS_ERR =A0 =A0 =A0 =A0 =A0 BIT(22) /* erro=
r bit */
> +
> +#define MPC52xx_SLPC_FIFO_CONTROL_GR(gr) =A0 =A0 =A0 ((gr) << 24) =A0 =
=A0/* granularity bits */
> +
> +
> =A0/* Clock Distribution control */
> =A0struct mpc52xx_cdm {
> =A0 =A0 =A0 =A0u32 jtag_id; =A0 =A0 =A0 =A0 =A0 =A0/* CDM + 0x00 =A0reg0 =
read only */
> @@ -304,19 +328,18 @@ extern void mpc52xx_restart(char *cmd);
> =A0struct mpc52xx_gpt_priv;
> =A0extern struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq);
> =A0extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 p=
eriod,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int continuous);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int =
continuous);

Unrelated whitespace change?

> =A0extern u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt);
> =A0extern int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
>
> =A0/* mpc52xx_lpbfifo.c */
> =A0#define MPC52XX_LPBFIFO_FLAG_READ =A0 =A0 =A0 =A0 =A0 =A0 =A0(0)
> -#define MPC52XX_LPBFIFO_FLAG_WRITE =A0 =A0 =A0 =A0 =A0 =A0 (1<<0)
> -#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT =A0 =A0 =A0(1<<1)
> -#define MPC52XX_LPBFIFO_FLAG_NO_DMA =A0 =A0 =A0 =A0 =A0 =A0(1<<2)
> -#define MPC52XX_LPBFIFO_FLAG_POLL_DMA =A0 =A0 =A0 =A0 =A0(1<<3)
> +#define MPC52XX_LPBFIFO_FLAG_WRITE =A0 =A0 =A0 =A0 =A0 =A0 BIT(0)
> +#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT =A0 =A0 =A0BIT(1)
> +#define MPC52XX_LPBFIFO_FLAG_NO_DMA =A0 =A0 =A0 =A0 =A0 =A0BIT(2)
> +#define MPC52XX_LPBFIFO_FLAG_POLL_DMA =A0 =A0 =A0 =A0 =A0BIT(3)

I prefer the (1<<n) style myself.

>
> =A0struct mpc52xx_lpbfifo_request {
> - =A0 =A0 =A0 struct list_head list;

Why is the list head being removed?

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
From: Grant Likely @ 2010-01-11 19:15 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220759.23453.roman.fietze@telemotive.de>

On Mon, Dec 21, 2009 at 11:59 PM, Roman Fietze
<roman.fietze@telemotive.de> wrote:

No patch description?  Very few patches are sufficiently described by
the subject line alone.  Tell me what the problem is, what the patch
changes, and why.

> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0 =A0 =A0| =A0 24 +++=
+++++
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 79 +++++++++++----=
----------
> =A02 files changed, 59 insertions(+), 44 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/as=
m/mpc52xx.h
> index b664ce7..57f8335 100644
> --- a/arch/powerpc/include/asm/mpc52xx.h
> +++ b/arch/powerpc/include/asm/mpc52xx.h
> @@ -193,6 +193,30 @@ struct mpc52xx_xlb {
> =A0#define MPC52xx_XLB_CFG_PLDIS =A0 =A0 =A0 =A0 =A0(1 << 31)
> =A0#define MPC52xx_XLB_CFG_SNOOP =A0 =A0 =A0 =A0 =A0(1 << 15)
>
> +/* SCLPC */
> +struct mpc52xx_sclpc {
> + =A0 =A0 =A0 union {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 restart; =A0 =A0 /* 0x00 restart bit */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 packet_size; /* 0x00 packet size regist=
er */
> + =A0 =A0 =A0 } packet_size;
> + =A0 =A0 =A0 u32 start_address; =A0 =A0 =A0/* 0x04 start Address registe=
r */
> + =A0 =A0 =A0 u32 control; =A0 =A0 =A0 =A0 =A0 =A0/* 0x08 control registe=
r */
> + =A0 =A0 =A0 u32 enable; =A0 =A0 =A0 =A0 =A0 =A0 /* 0x0C enable register=
 */
> + =A0 =A0 =A0 u32 unused0; =A0 =A0 =A0 =A0 =A0 =A0/* 0x10 */
> + =A0 =A0 =A0 union {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 status; =A0 =A0 =A0/* 0x14 status regist=
er bits */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 bytes_done; /* 0x14 bytes done register=
 bits, read only */
> + =A0 =A0 =A0 } bytes_done_status;
> +
> + =A0 =A0 =A0 u32 reserved1[(0x40-0x18) / sizeof(u32)]; =A0 =A0 =A0 /* 0x=
18 .. 0x3c */
> +
> + =A0 =A0 =A0 u32 fifo_data; =A0 =A0 =A0 =A0 =A0/* 0x40 FIFO data word re=
gister */
> + =A0 =A0 =A0 u32 fifo_status; =A0 =A0 =A0 =A0/* 0x44 FIFO status registe=
r */
> + =A0 =A0 =A0 u8 fifo_control; =A0 =A0 =A0 =A0/* 0x48 FIFO control regist=
er */
> + =A0 =A0 =A0 u8 reserved2[3];
> + =A0 =A0 =A0 u32 fifo_alarm; =A0 =A0 =A0 =A0 /* 0x4C FIFO alarm register=
 */
> +};

Please don't.  I know that a lot of other 5200 code uses register map
structures in this way, but I consider it bad practice.  I coded this
driver without a structure for a reason.  The reason I haven't removed
the other 5200 register map structures is the code impact would be
huge, it would probably cause breakage, and it would break all
out-of-tree patches touching the same code for no measurable
advantage.

For this code, there is no advantage to changing the register access
method, and it generates more work in other areas.  Even if I
preferred register map structures I would resist applying this patch.

Please drop from your series.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 01/13] powerpc/5200: LocalPlus driver: fix indentation and white space
From: Grant Likely @ 2010-01-11 19:06 UTC (permalink / raw)
  To: Roman Fietze; +Cc: linuxppc-dev
In-Reply-To: <200912220757.44591.roman.fietze@telemotive.de>

Hi Roman.

I'm finally getting some time to look at these with a bit more detail.

On Mon, Dec 21, 2009 at 11:57 PM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
>
> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0 18 +++++++++------=
---
> =A01 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 929d017..4c84aa5 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -165,7 +165,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
>
> =A0 =A0 =A0 =A0bit_fields =3D req->cs << 24 | 0x000008;
> =A0 =A0 =A0 =A0if (!write)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D 0x010000; /* read mode */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D 0x010000; /* read mode */
> =A0 =A0 =A0 =A0out_be32(lpbfifo.regs + LPBFIFO_REG_CONTROL, bit_fields);
>
> =A0 =A0 =A0 =A0/* Kick it off */
> @@ -279,7 +279,7 @@ static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void =
*dev_id)
> =A0 =A0 =A0 =A0else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0do_callback =3D 1;
>
> - out:
> +out:

The label 1 space indentation is intentional so coax diff into
choosing the right line for the function name.  There are plenty of
examples of this in the kernel.

The rest of the changes are valid, but unimportant.  It's not worth
the effort.  In most cases I don't bother fixing whitespace unless
they style violations are so gratuitous  that it is hard to read the
code otherwise, or if I'm already touching the offending line.

g.


--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: PCI-PCI bridge scanning broken on 460EX
From: Felix Radensky @ 2010-01-11 16:46 UTC (permalink / raw)
  To: Stef van Os; +Cc: Stefan Roese, Feng Kan, linuxppc-dev
In-Reply-To: <4B4B1027.4020708@embedded-sol.com>

Hi Stef

Felix Radensky wrote:
> Hi Stef,
>
> Stef van Os wrote:
>> Hello Felix,
>>
>> I had a problem similar to this on the 440GX, the PCI code was not
>> sending type 1 transactions when scanning behind bridges. Perhaps you
>> could try this:
>>
>> Index: linux/arch/powerpc/sysdev/ppc4xx_pci.c
>> ===================================================================
>> --- linux/arch/powerpc/sysdev/ppc4xx_pci.c      (revision 26)
>> +++ linux/arch/powerpc/sysdev/ppc4xx_pci.c      (revision 27)
>> @@ -569,7 +569,7 @@
>>         hose->last_busno = bus_range ? bus_range[1] : 0xff;
>>
>>         /* Setup config space */
>> -       setup_indirect_pci(hose, rsrc_cfg.start, rsrc_cfg.start + 0x4,
>> 0);
>> +       setup_indirect_pci(hose, rsrc_cfg.start, rsrc_cfg.start + 0x4,
>> PPC_INDIRECT_TYPE_SET_CFG_TYPE);
>>
>>         /* Disable all windows */
>>         writel(0, reg + PCIX0_POM0SA);
>>
>>
>>
>> With kind regards / Met vriendelijke groet,
>>
>> Stef van Os
>>
>> Prodrive B.V.
>>
>>   
>
> I think you patch is a valid one, and should be applied, but 
> unfortunately it doesn't fix by problem.
> BTW, in u-boot transaction type bit is set correctly.
>
>

It seems I was wrong. I've manually applied the patch at the wrong 
place. After patching the correct function
I'm not getting hard resets any more, which is a great improvement ! 
Thanks a lot, I really appreciate your help !

Unfortunately not all problems are gone. PLX is now identified 
correctly, but device behind it is not detected,
although u-boot detects it correctly. See below.

PCI: Probing PCI hardware
pci_bus 0000:00: scanning bus
pci 0000:00:02.0: found [3388:0020] class 000604 header type 01
pci 0000:00:02.0: calling pcibios_fixup_resources+0x0/0xf4
pci 0000:00:02.0: calling fixup_ppc4xx_pci_bridge+0x0/0x154
pci 0000:00:02.0: calling quirk_resource_alignment+0x0/0x200
pci 0000:00:02.0: supports D1 D2
pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
pci 0000:00:02.0: PME# disabled
pci_bus 0000:00: fixups for bus
pci 0000:00:02.0: scanning behind bridge, config 010100, pass 0
pci_bus 0000:01: scanning bus
pci 0000:01:02.0: found [3388:0020] class 000604 header type 01
pci 0000:01:02.0: calling pcibios_fixup_resources+0x0/0xf4
pci 0000:01:02.0: calling fixup_ppc4xx_pci_bridge+0x0/0x154
pci 0000:01:02.0: calling quirk_resource_alignment+0x0/0x200
pci 0000:01:02.0: supports D1 D2
pci 0000:01:02.0: PME# supported from D0 D1 D2 D3hot
pci 0000:01:02.0: PME# disabled
pci_bus 0000:01: fixups for bus
pci 0000:00:02.0: PCI bridge to [bus 01-01]
pci 0000:00:02.0:   bridge window [mem 0x80000000-0x8c0fffff]
pci 0000:01:02.0: scanning behind bridge, config 010100, pass 0
pci 0000:01:02.0: bus configuration invalid, reconfiguring
pci 0000:01:02.0: scanning behind bridge, config 010100, pass 1
pci_bus 0000:01: bus scan returning with max=01
pci 0000:00:02.0: scanning behind bridge, config 010100, pass 1
pci_bus 0000:00: bus scan returning with max=01
pci 0000:00:02.0: disabling bridge window [mem 0xd80000000-0xd8c0fffff] 
to [bus 01-01] (unused)
pci 0000:00:02.0: PCI bridge to [bus 01-01]
pci 0000:00:02.0:   bridge window [io  disabled]
pci 0000:00:02.0:   bridge window [mem disabled]
pci 0000:00:02.0:   bridge window [mem pref disabled]
pci_bus 0000:00: resource 0 [io  0x0000-0xffff]
pci_bus 0000:00: resource 1 [mem 0xd80000000-0xdffffffff]
pci_bus 0000:01: resource 1 [??? 57982058496-58184433663 flags 0x0]

Any ideas what could be wrong now ?

Thanks a lot.

Felix.

^ permalink raw reply

* Re: fsldma: cleanup driver and fix async_tx compatibility
From: Ira W. Snyder @ 2010-01-11 16:29 UTC (permalink / raw)
  To: Dudhat Dipen-B09055
  Cc: herbert, Suresh Vishnu-B05022, Tabi Timur-B04825, linuxppc-dev,
	dan.j.williams, Gupta Maneesh-B18878, Li Yang-R58472
In-Reply-To: <0949C49693EF1A47A54B0F0113CDB4A6076CEE@zin33exm23.fsl.freescale.net>

On Mon, Jan 11, 2010 at 11:17:04AM +0530, Dudhat Dipen-B09055 wrote:
> 
> Hi Ira,
> 
> I have tested your patches with async DMA memcpy support. Though I
> haven't captured the improvement figures.
> It works fine for RAID5 memcpy offload as interrupts are coming for
> separate DMA channels while I have ran IOZONE onto RAID partition.
> 

Excellent, thanks for running these tests. I'm glad to hear that the
RAID offload is working now.

You shouldn't notice any difference in performance. On a 32MB memcpy
operation, broken into 32x 1MB memcpy(), 1x interrupt(), I noticed less
than 0.1% difference (approx 100,000 ns / 0.1ms). This is probably at or
near the limits of my measurement accuracy.

Ira


> Regards,
>  Dipen
>  
> 
> -----Original Message-----
> From: Dudhat Dipen-B09055 
> Sent: Tuesday, January 05, 2010 11:38 AM
> To: 'Ira W. Snyder'; dan.j.williams@intel.com
> Cc: galak@kernel.crashing.org; herbert@gondor.apana.org.au; Tabi
> Timur-B04825; linuxppc-dev@ozlabs.org; Suresh Vishnu-B05022; Gupta
> Maneesh-B18878; Li Yang-R58472
> Subject: RE: fsldma: cleanup driver and fix async_tx compatibility
> 
> 
> Hi Ira,
> 
> I will test it on 85xx hardware and let you know once done.
> 
> Thanks
> Dipen
>  
> 
> -----Original Message-----
> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]
> Sent: Friday, January 01, 2010 11:41 AM
> To: dan.j.williams@intel.com
> Cc: galak@kernel.crashing.org; herbert@gondor.apana.org.au; Tabi
> Timur-B04825; linuxppc-dev@ozlabs.org; Suresh Vishnu-B05022; Dudhat
> Dipen-B09055; Gupta Maneesh-B18878; Li Yang-R58472
> Subject: fsldma: cleanup driver and fix async_tx compatibility
> 
> This patch series cleans up the Freescale DMAEngine driver, including
> verifying the locking and making sure that all code paths are correct.
> There were a few places that seemed suspicious, and they have been
> fixed.
> 
> I have written a quick memory->memory DMAEngine test driver, and the
> performance is identical before and after my changes (<0.1% change). I
> measured both setting up the DMA operation (via
> device_prep_dma_interrupt() and device_prep_dma_memcpy()) and the actual
> DMA transfer itself.
> 
> As an added bonus, the interrupt load is measurably reduced. My test
> driver transfers 32MB as 32x 1MB chunks + 1 interrupt descriptor, using
> the functions noted above. Previous to this patch series, 31 interrupts
> were generated. After this patch series, only a single interrupt is
> generated for the whole transaction.
> 
> Some testing on 85xx/86xx hardware would be appreciated. Also, some
> testing by the users attempting to use async_tx and talitos to handle
> RAID offload would be great as well.
> 
>  Documentation/powerpc/dts-bindings/fsl/dma.txt |   17 +-
>  drivers/dma/fsldma.c                           | 1036
> ++++++++++++------------
>  drivers/dma/fsldma.h                           |   35 +-
>  3 files changed, 556 insertions(+), 532 deletions(-)
> 
> Thanks,
> Ira
> 

^ permalink raw reply

* [PATCH -tip tracing/kprobes v2] Powerpc port of the kprobe-based event tracer
From: Mahesh Salgaonkar @ 2010-01-11 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michael Neuling, Mahesh Salgaonkar, linuxppc-dev, systemtap,
	Ingo Molnar, Masami Hiramatsu
In-Reply-To: <20100111125952.604518521@mars.in.ibm.com>

This patch ports the kprobe-based event tracer to powerpc. This patch
is based in x86 port. This brings powerpc on par with x86.

ChangeLog - v2:
- Removed regs_get_argument_nth() API as function argument access syntax
  is dropped from kprobe-tracer. Please refer to patches below on Linux PPC
  mailing list:
  http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079331.html
  http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079332.html
- Rebased to commit f61d6b1dcb06d62bc20d40e51c7a1e80275a80ab of -tip

This patch is dependent on following patch:
  http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-January/079331.html

Port the following API's to ppc for accessing registers and stack entries
from pt_regs.

- regs_query_register_offset(const char *name)
   Query the offset of "name" register.

- regs_query_register_name(unsigned int offset)
   Query the name of register by its offset.

- regs_get_register(struct pt_regs *regs, unsigned int offset)
   Get the value of a register by its offset.

- regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
   Check the address is in the kernel stack.

- regs_get_kernel_stack_nth(struct pt_regs *reg, unsigned int nth)
   Get Nth entry of the kernel stack. (N >= 0)

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@redhat.com>
---

 arch/powerpc/include/asm/ptrace.h |   61 ++++++++++++++++++++++
 arch/powerpc/kernel/ptrace.c      |  102 +++++++++++++++++++++++++++++++++++++
 kernel/trace/Kconfig              |    2 -
 3 files changed, 164 insertions(+), 1 deletions(-)


diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index cbd759e..cb2eeda 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -83,6 +83,7 @@ struct pt_regs {
 
 #define instruction_pointer(regs) ((regs)->nip)
 #define user_stack_pointer(regs) ((regs)->gpr[1])
+#define kernel_stack_pointer(regs) ((regs)->gpr[1])
 #define regs_return_value(regs) ((regs)->gpr[3])
 
 #ifdef CONFIG_SMP
@@ -131,6 +132,66 @@ do {									      \
 } while (0)
 #endif /* __powerpc64__ */
 
+/* Query offset/name of register from its name/offset */
+#include <linux/stddef.h>
+#include <linux/thread_info.h>
+extern int regs_query_register_offset(const char *name);
+extern const char *regs_query_register_name(unsigned int offset);
+#define MAX_REG_OFFSET (offsetof(struct pt_regs, result))
+
+/**
+ * regs_get_register() - get register value from its offset
+ * @regs:	   pt_regs from which register value is gotten
+ * @offset:    offset number of the register.
+ *
+ * regs_get_register returns the value of a register whose offset from @regs.
+ * The @offset is the offset of the register in struct pt_regs.
+ * If @offset is bigger than MAX_REG_OFFSET, this returns 0.
+ */
+static inline unsigned long regs_get_register(struct pt_regs *regs,
+						unsigned int offset)
+{
+	if (unlikely(offset > MAX_REG_OFFSET))
+		return 0;
+	return *(unsigned long *)((unsigned long)regs + offset);
+}
+
+/**
+ * regs_within_kernel_stack() - check the address in the stack
+ * @regs:      pt_regs which contains kernel stack pointer.
+ * @addr:      address which is checked.
+ *
+ * regs_within_kernel_stack() checks @addr is within the kernel stack page(s).
+ * If @addr is within the kernel stack, it returns true. If not, returns false.
+ */
+
+static inline bool regs_within_kernel_stack(struct pt_regs *regs,
+						unsigned long addr)
+{
+	return ((addr & ~(THREAD_SIZE - 1))  ==
+		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1)));
+}
+
+/**
+ * regs_get_kernel_stack_nth() - get Nth entry of the stack
+ * @regs:	pt_regs which contains kernel stack pointer.
+ * @n:		stack entry number.
+ *
+ * regs_get_kernel_stack_nth() returns @n th entry of the kernel stack which
+ * is specified by @regs. If the @n th entry is NOT in the kernel stack,
+ * this returns 0.
+ */
+static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
+						      unsigned int n)
+{
+	unsigned long *addr = (unsigned long *)kernel_stack_pointer(regs);
+	addr += n;
+	if (regs_within_kernel_stack(regs, (unsigned long)addr))
+		return *addr;
+	else
+		return 0;
+}
+
 /*
  * These are defined as per linux/ptrace.h, which see.
  */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index ef14988..e816aba 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -39,6 +39,108 @@
 #include <asm/system.h>
 
 /*
+ * The parameter save area on the stack is used to store arguments being passed
+ * to callee function and is located at fixed offset from stack pointer.
+ */
+#ifdef CONFIG_PPC32
+#define PARAMETER_SAVE_AREA_OFFSET	24  /* bytes */
+#else /* CONFIG_PPC32 */
+#define PARAMETER_SAVE_AREA_OFFSET	48  /* bytes */
+#endif
+
+struct pt_regs_offset {
+	const char *name;
+	int offset;
+};
+
+#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs, r)}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+
+static const struct pt_regs_offset regoffset_table[] = {
+	REG_OFFSET_NAME(gpr[0]),
+	REG_OFFSET_NAME(gpr[1]),
+	REG_OFFSET_NAME(gpr[2]),
+	REG_OFFSET_NAME(gpr[3]),
+	REG_OFFSET_NAME(gpr[4]),
+	REG_OFFSET_NAME(gpr[5]),
+	REG_OFFSET_NAME(gpr[6]),
+	REG_OFFSET_NAME(gpr[7]),
+	REG_OFFSET_NAME(gpr[8]),
+	REG_OFFSET_NAME(gpr[9]),
+	REG_OFFSET_NAME(gpr[10]),
+	REG_OFFSET_NAME(gpr[11]),
+	REG_OFFSET_NAME(gpr[12]),
+	REG_OFFSET_NAME(gpr[13]),
+	REG_OFFSET_NAME(gpr[14]),
+	REG_OFFSET_NAME(gpr[15]),
+	REG_OFFSET_NAME(gpr[16]),
+	REG_OFFSET_NAME(gpr[17]),
+	REG_OFFSET_NAME(gpr[18]),
+	REG_OFFSET_NAME(gpr[19]),
+	REG_OFFSET_NAME(gpr[20]),
+	REG_OFFSET_NAME(gpr[21]),
+	REG_OFFSET_NAME(gpr[22]),
+	REG_OFFSET_NAME(gpr[23]),
+	REG_OFFSET_NAME(gpr[24]),
+	REG_OFFSET_NAME(gpr[25]),
+	REG_OFFSET_NAME(gpr[26]),
+	REG_OFFSET_NAME(gpr[27]),
+	REG_OFFSET_NAME(gpr[28]),
+	REG_OFFSET_NAME(gpr[29]),
+	REG_OFFSET_NAME(gpr[30]),
+	REG_OFFSET_NAME(gpr[31]),
+	REG_OFFSET_NAME(nip),
+	REG_OFFSET_NAME(msr),
+	REG_OFFSET_NAME(orig_gpr3),
+	REG_OFFSET_NAME(ctr),
+	REG_OFFSET_NAME(link),
+	REG_OFFSET_NAME(xer),
+	REG_OFFSET_NAME(ccr),
+#ifdef CONFIG_PPC64
+	REG_OFFSET_NAME(softe),
+#else
+	REG_OFFSET_NAME(mq),
+#endif
+	REG_OFFSET_NAME(trap),
+	REG_OFFSET_NAME(dar),
+	REG_OFFSET_NAME(dsisr),
+	REG_OFFSET_NAME(result),
+	REG_OFFSET_END,
+};
+
+/**
+ * regs_query_register_offset() - query register offset from its name
+ * @name:	the name of a register
+ *
+ * regs_query_register_offset() returns the offset of a register in struct
+ * pt_regs from its name. If the name is invalid, this returns -EINVAL;
+ */
+int regs_query_register_offset(const char *name)
+{
+	const struct pt_regs_offset *roff;
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (!strcmp(roff->name, name))
+			return roff->offset;
+	return -EINVAL;
+}
+
+/**
+ * regs_query_register_name() - query register name from its offset
+ * @offset:	the offset of a register in struct pt_regs.
+ *
+ * regs_query_register_name() returns the name of a register from its
+ * offset in struct pt_regs. If the @offset is invalid, this returns NULL;
+ */
+const char *regs_query_register_name(unsigned int offset)
+{
+	const struct pt_regs_offset *roff;
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (roff->offset == offset)
+			return roff->name;
+	return NULL;
+}
+
+/*
  * does not yet catch signals sent when the child dies.
  * in exit.c or in signal.c.
  */
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 6c22d8a..4d95966 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -451,7 +451,7 @@ config BLK_DEV_IO_TRACE
 
 config KPROBE_EVENT
 	depends on KPROBES
-	depends on X86
+	depends on X86 || PPC
 	bool "Enable kprobes-based dynamic events"
 	select TRACING
 	default y

^ permalink raw reply related

* [PATCH 8/8] powerpc/86xx: Enable VME driver on the GE SBC610
From: Martyn Welch @ 2010-01-11 12:23 UTC (permalink / raw)
  To: galak; +Cc: linuxppc-dev
In-Reply-To: <20100111121622.8306.87501.stgit@ES-J7S4D2J.amer.consind.ge.com>

Enable the VME driver (which is currently in staging) on the SBC610.

Signed-off-by: Martyn Welch <martyn.welch@gefanuc.com>
---

 arch/powerpc/configs/86xx/gef_sbc610_defconfig |   39 +++++++++++++++++++++++-
 1 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/configs/86xx/gef_sbc610_defconfig b/arch/powerpc/configs/86xx/gef_sbc610_defconfig
index 4912602..37adfb6 100644
--- a/arch/powerpc/configs/86xx/gef_sbc610_defconfig
+++ b/arch/powerpc/configs/86xx/gef_sbc610_defconfig
@@ -1600,7 +1600,44 @@ CONFIG_RTC_DRV_RX8581=y
 #
 # TI VLYNQ
 #
-# CONFIG_STAGING is not set
+CONFIG_STAGING=y
+# CONFIG_STAGING_EXCLUDE_BUILD is not set
+# CONFIG_ET131X is not set
+# CONFIG_ME4000 is not set
+# CONFIG_MEILHAUS is not set
+# CONFIG_USB_IP_COMMON is not set
+# CONFIG_ECHO is not set
+# CONFIG_COMEDI is not set
+# CONFIG_ASUS_OLED is not set
+# CONFIG_ALTERA_PCIE_CHDMA is not set
+# CONFIG_INPUT_MIMIO is not set
+# CONFIG_TRANZPORT is not set
+
+#
+# Android
+#
+# CONFIG_ANDROID is not set
+# CONFIG_DST is not set
+# CONFIG_POHMELFS is not set
+# CONFIG_B3DFG is not set
+# CONFIG_IDE_PHISON is not set
+# CONFIG_PLAN9AUTH is not set
+# CONFIG_HECI is not set
+# CONFIG_VT6655 is not set
+# CONFIG_USB_CPC is not set
+# CONFIG_RDC_17F3101X is not set
+CONFIG_VME_BUS=y
+
+#
+# VME Bridge Drivers
+#
+# CONFIG_VME_CA91CX42 is not set
+CONFIG_VME_TSI148=y
+
+#
+# VME Device Drivers
+#
+# CONFIG_VME_USER is not set
 
 #
 # File systems


--
Martyn Welch MEng MPhil MIET (Principal Software Engineer)   T:+44(0)1327322748
GE Fanuc Intelligent Platforms Ltd,        |Registered in England and Wales
Tove Valley Business Park, Towcester,      |(3828642) at 100 Barbirolli Square,
Northants, NN12 6PF, UK T:+44(0)1327359444 |Manchester,M2 3AB  VAT:GB 927559189

^ permalink raw reply related

* [PATCH 7/8] powerpc/86xx: Enable VME driver on the GE PPC9A
From: Martyn Welch @ 2010-01-11 12:23 UTC (permalink / raw)
  To: galak; +Cc: linuxppc-dev
In-Reply-To: <20100111121622.8306.87501.stgit@ES-J7S4D2J.amer.consind.ge.com>

Enable the VME driver (which is currently in staging) on the PPC9A

Signed-off-by: Martyn Welch <martyn.welch@gefanuc.com>
---

 arch/powerpc/configs/86xx/gef_ppc9a_defconfig |   47 ++++++++++++++++++++++++-
 1 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/configs/86xx/gef_ppc9a_defconfig b/arch/powerpc/configs/86xx/gef_ppc9a_defconfig
index 6cd2cd6..1a37b69 100644
--- a/arch/powerpc/configs/86xx/gef_ppc9a_defconfig
+++ b/arch/powerpc/configs/86xx/gef_ppc9a_defconfig
@@ -1499,7 +1499,52 @@ CONFIG_RTC_DRV_RX8581=y
 #
 # TI VLYNQ
 #
-# CONFIG_STAGING is not set
+CONFIG_STAGING=y
+# CONFIG_STAGING_EXCLUDE_BUILD is not set
+# CONFIG_ET131X is not set
+# CONFIG_ME4000 is not set
+# CONFIG_MEILHAUS is not set
+# CONFIG_USB_IP_COMMON is not set
+# CONFIG_ECHO is not set
+# CONFIG_COMEDI is not set
+# CONFIG_ASUS_OLED is not set
+# CONFIG_ALTERA_PCIE_CHDMA is not set
+# CONFIG_INPUT_MIMIO is not set
+# CONFIG_TRANZPORT is not set
+
+#
+# Android
+#
+# CONFIG_ANDROID is not set
+# CONFIG_DST is not set
+# CONFIG_POHMELFS is not set
+# CONFIG_B3DFG is not set
+# CONFIG_IDE_PHISON is not set
+# CONFIG_PLAN9AUTH is not set
+# CONFIG_HECI is not set
+# CONFIG_USB_CPC is not set
+
+#
+# Qualcomm MSM Camera And Video
+#
+
+#
+# Camera Sensor Selection
+#
+# CONFIG_HYPERV_STORAGE is not set
+# CONFIG_HYPERV_BLOCK is not set
+# CONFIG_HYPERV_NET is not set
+CONFIG_VME_BUS=y
+
+#
+# VME Bridge Drivers
+#
+CONFIG_VME_TSI148=y
+
+#
+# VME Device Drivers
+#
+# CONFIG_VME_USER is not set
 
 #
 # File systems


--
Martyn Welch MEng MPhil MIET (Principal Software Engineer)   T:+44(0)1327322748
GE Fanuc Intelligent Platforms Ltd,        |Registered in England and Wales
Tove Valley Business Park, Towcester,      |(3828642) at 100 Barbirolli Square,
Northants, NN12 6PF, UK T:+44(0)1327359444 |Manchester,M2 3AB  VAT:GB 927559189

^ permalink raw reply related

* [PATCH 6/8] powerpc/86xx: Add MSI section to GE PPC9A DTS
From: Martyn Welch @ 2010-01-11 12:23 UTC (permalink / raw)
  To: galak; +Cc: linuxppc-dev
In-Reply-To: <20100111121622.8306.87501.stgit@ES-J7S4D2J.amer.consind.ge.com>

From: Malcolm Crossley <malcolm.crossley2@gefanuc.com>

Add the MSI section to the DTS file for the GE PPC9A.
    
    Signed-off-by: Malcolm Crossley <malcolm.crossley2@gefanuc.com>
    Signed-off-by: Martyn Welch <martyn.welch@gefanuc.com>
---

 arch/powerpc/boot/dts/gef_ppc9a.dts |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/gef_ppc9a.dts b/arch/powerpc/boot/dts/gef_ppc9a.dts
index c86114e..977f260 100644
--- a/arch/powerpc/boot/dts/gef_ppc9a.dts
+++ b/arch/powerpc/boot/dts/gef_ppc9a.dts
@@ -341,6 +341,22 @@
 			device_type = "open-pic";
 		};
 
+		msi@41600 {
+			compatible = "fsl,mpc8641-msi", "fsl,mpic-msi";
+			reg = <0x41600 0x80>;
+			msi-available-ranges = <0 0x100>;
+			interrupts = <
+				0xe0 0
+				0xe1 0
+				0xe2 0
+				0xe3 0
+				0xe4 0
+				0xe5 0
+				0xe6 0
+				0xe7 0>;
+			interrupt-parent = <&mpic>;
+		};
+
 		global-utilities@e0000 {
 			compatible = "fsl,mpc8641-guts";
 			reg = <0xe0000 0x1000>;


--
Martyn Welch MEng MPhil MIET (Principal Software Engineer)   T:+44(0)1327322748
GE Fanuc Intelligent Platforms Ltd,        |Registered in England and Wales
Tove Valley Business Park, Towcester,      |(3828642) at 100 Barbirolli Square,
Northants, NN12 6PF, UK T:+44(0)1327359444 |Manchester,M2 3AB  VAT:GB 927559189

^ permalink raw reply related

* [PATCH 5/8] powerpc/86xx: Switch on highmem support on GE SBC610
From: Martyn Welch @ 2010-01-11 12:23 UTC (permalink / raw)
  To: galak; +Cc: linuxppc-dev
In-Reply-To: <20100111121622.8306.87501.stgit@ES-J7S4D2J.amer.consind.ge.com>

Signed-off-by: Martyn Welch <martyn.welch@gefanuc.com>
---

 arch/powerpc/configs/86xx/gef_sbc610_defconfig |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/configs/86xx/gef_sbc610_defconfig b/arch/powerpc/configs/86xx/gef_sbc610_defconfig
index 9284f04..4912602 100644
--- a/arch/powerpc/configs/86xx/gef_sbc610_defconfig
+++ b/arch/powerpc/configs/86xx/gef_sbc610_defconfig
@@ -234,7 +234,7 @@ CONFIG_MMIO_NVRAM=y
 #
 # Kernel options
 #
-# CONFIG_HIGHMEM is not set
+CONFIG_HIGHMEM=y
 CONFIG_TICK_ONESHOT=y
 # CONFIG_NO_HZ is not set
 CONFIG_HIGH_RES_TIMERS=y
@@ -1832,6 +1832,7 @@ CONFIG_DEBUG_PREEMPT=y
 # CONFIG_DEBUG_SPINLOCK_SLEEP is not set
 # CONFIG_DEBUG_LOCKING_API_SELFTESTS is not set
 # CONFIG_DEBUG_KOBJECT is not set
+# CONFIG_DEBUG_HIGHMEM is not set
 # CONFIG_DEBUG_BUGVERBOSE is not set
 CONFIG_DEBUG_INFO=y
 # CONFIG_DEBUG_VM is not set


--
Martyn Welch MEng MPhil MIET (Principal Software Engineer)   T:+44(0)1327322748
GE Fanuc Intelligent Platforms Ltd,        |Registered in England and Wales
Tove Valley Business Park, Towcester,      |(3828642) at 100 Barbirolli Square,
Northants, NN12 6PF, UK T:+44(0)1327359444 |Manchester,M2 3AB  VAT:GB 927559189

^ permalink raw reply related

* [PATCH 4/8] powerpc: Basic flash support for GE SBC610
From: Martyn Welch @ 2010-01-11 12:23 UTC (permalink / raw)
  To: galak; +Cc: linuxppc-dev
In-Reply-To: <20100111121622.8306.87501.stgit@ES-J7S4D2J.amer.consind.ge.com>

Support for the SBC610 VPX Single Board Computer from GE (PowerPC MPC8641D).

This patch adds basic support for the on-board flash.

Signed-off-by: Martyn Welch <martyn.welch@gefanuc.com>
---

 arch/powerpc/boot/dts/gef_sbc610.dts           |   50 ++++++++++++++++++++----
 arch/powerpc/configs/86xx/gef_sbc610_defconfig |   23 +++++++----
 2 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/boot/dts/gef_sbc610.dts b/arch/powerpc/boot/dts/gef_sbc610.dts
index 78c336f..bb70600 100644
--- a/arch/powerpc/boot/dts/gef_sbc610.dts
+++ b/arch/powerpc/boot/dts/gef_sbc610.dts
@@ -75,14 +75,48 @@
 		interrupts = <19 2>;
 		interrupt-parent = <&mpic>;
 
-		ranges = <0 0 0xff000000 0x01000000     // 16MB Boot flash
-			  1 0 0xe8000000 0x08000000     // Paged Flash 0
-			  2 0 0xe0000000 0x08000000     // Paged Flash 1
-			  3 0 0xfc100000 0x00020000     // NVRAM
-			  4 0 0xfc000000 0x00008000     // FPGA
-			  5 0 0xfc008000 0x00008000     // AFIX FPGA
-			  6 0 0xfd000000 0x00800000     // IO FPGA (8-bit)
-			  7 0 0xfd800000 0x00800000>;   // IO FPGA (32-bit)
+		ranges = <0 0 0xff000000 0x01000000	// 16MB Boot flash
+			  1 0 0xe8000000 0x08000000	// Paged Flash 0
+			  2 0 0xe0000000 0x08000000	// Paged Flash 1
+			  3 0 0xfc100000 0x00020000	// NVRAM
+			  4 0 0xfc000000 0x00008000	// FPGA
+			  5 0 0xfc008000 0x00008000	// AFIX FPGA
+			  6 0 0xfd000000 0x00800000	// IO FPGA (8-bit)
+			  7 0 0xfd800000 0x00800000>;	// IO FPGA (32-bit)
+
+		/* flash@0,0 is a mirror of part of the memory in flash@1,0
+		flash@0,0 {
+			compatible = "gef,sbc610-firmware-mirror", "cfi-flash";
+			reg = <0x0 0x0 0x1000000>;
+			bank-width = <4>;
+			device-width = <2>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition@0 {
+				label = "firmware";
+				reg = <0x0 0x1000000>;
+				read-only;
+			};
+		};
+		*/
+
+		flash@1,0 {
+			compatible = "gef,sbc610-paged-flash", "cfi-flash";
+			reg = <0x1 0x0 0x8000000>;
+			bank-width = <4>;
+			device-width = <2>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition@0 {
+				label = "user";
+				reg = <0x0 0x7800000>;
+			};
+			partition@7800000 {
+				label = "firmware";
+				reg = <0x7800000 0x800000>;
+				read-only;
+			};
+		};
 
 		nvram@3,0 {
 			device_type = "nvram";
diff --git a/arch/powerpc/configs/86xx/gef_sbc610_defconfig b/arch/powerpc/configs/86xx/gef_sbc610_defconfig
index 1975d41..9284f04 100644
--- a/arch/powerpc/configs/86xx/gef_sbc610_defconfig
+++ b/arch/powerpc/configs/86xx/gef_sbc610_defconfig
@@ -623,7 +623,7 @@ CONFIG_MTD_CONCAT=y
 CONFIG_MTD_PARTITIONS=y
 # CONFIG_MTD_REDBOOT_PARTS is not set
 # CONFIG_MTD_CMDLINE_PARTS is not set
-# CONFIG_MTD_OF_PARTS is not set
+CONFIG_MTD_OF_PARTS=y
 # CONFIG_MTD_AR7_PARTS is not set
 
 #
@@ -643,13 +643,9 @@ CONFIG_MTD_BLOCK=y
 # RAM/ROM/Flash chip drivers
 #
 CONFIG_MTD_CFI=y
-# CONFIG_MTD_JEDECPROBE is not set
+CONFIG_MTD_JEDECPROBE=y
 CONFIG_MTD_GEN_PROBE=y
-CONFIG_MTD_CFI_ADV_OPTIONS=y
-# CONFIG_MTD_CFI_NOSWAP is not set
-# CONFIG_MTD_CFI_BE_BYTE_SWAP is not set
-CONFIG_MTD_CFI_LE_BYTE_SWAP=y
-# CONFIG_MTD_CFI_GEOMETRY is not set
+# CONFIG_MTD_CFI_ADV_OPTIONS is not set
 CONFIG_MTD_MAP_BANK_WIDTH_1=y
 CONFIG_MTD_MAP_BANK_WIDTH_2=y
 CONFIG_MTD_MAP_BANK_WIDTH_4=y
@@ -660,7 +656,6 @@ CONFIG_MTD_CFI_I1=y
 CONFIG_MTD_CFI_I2=y
 # CONFIG_MTD_CFI_I4 is not set
 # CONFIG_MTD_CFI_I8 is not set
-# CONFIG_MTD_OTP is not set
 CONFIG_MTD_CFI_INTELEXT=y
 CONFIG_MTD_CFI_AMDSTD=y
 # CONFIG_MTD_CFI_STAA is not set
@@ -1682,7 +1677,17 @@ CONFIG_MISC_FILESYSTEMS=y
 # CONFIG_BEFS_FS is not set
 # CONFIG_BFS_FS is not set
 # CONFIG_EFS_FS is not set
-# CONFIG_JFFS2_FS is not set
+CONFIG_JFFS2_FS=y
+CONFIG_JFFS2_FS_DEBUG=0
+CONFIG_JFFS2_FS_WRITEBUFFER=y
+# CONFIG_JFFS2_FS_WBUF_VERIFY is not set
+# CONFIG_JFFS2_SUMMARY is not set
+# CONFIG_JFFS2_FS_XATTR is not set
+# CONFIG_JFFS2_COMPRESSION_OPTIONS is not set
+CONFIG_JFFS2_ZLIB=y
+# CONFIG_JFFS2_LZO is not set
+CONFIG_JFFS2_RTIME=y
+# CONFIG_JFFS2_RUBIN is not set
 # CONFIG_CRAMFS is not set
 # CONFIG_SQUASHFS is not set
 # CONFIG_VXFS_FS is not set


--
Martyn Welch MEng MPhil MIET (Principal Software Engineer)   T:+44(0)1327322748
GE Fanuc Intelligent Platforms Ltd,        |Registered in England and Wales
Tove Valley Business Park, Towcester,      |(3828642) at 100 Barbirolli Square,
Northants, NN12 6PF, UK T:+44(0)1327359444 |Manchester,M2 3AB  VAT:GB 927559189

^ permalink raw reply related


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