* [PATCH 01/13] powerpc/5200: LocalPlus driver: fix indentation and white space
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
@ 2009-12-22 6:57 ` Roman Fietze
2010-01-11 19:06 ` Grant Likely
2009-12-22 6:59 ` [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure Roman Fietze
` (11 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 6:57 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 929d017..4c84aa5 100644
=2D-- 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_lpbfifo=
_request *req)
=20
bit_fields =3D req->cs << 24 | 0x000008;
if (!write)
=2D bit_fields |=3D 0x010000; /* read mode */
+ bit_fields |=3D 0x010000; /* read mode */
out_be32(lpbfifo.regs + LPBFIFO_REG_CONTROL, bit_fields);
=20
/* Kick it off */
@@ -279,7 +279,7 @@ static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *d=
ev_id)
else
do_callback =3D 1;
=20
=2D out:
+out:
/* Clear the IRQ */
out_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS, 0x01);
=20
@@ -379,11 +379,11 @@ void mpc52xx_lpbfifo_poll(void)
int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
=20
/*
=2D * For more information, see comments on the "Fat Lady"=20
+ * For more information, see comments on the "Fat Lady"
*/
if (dma && write)
mpc52xx_lpbfifo_irq(0, NULL);
=2D else=20
+ else
mpc52xx_lpbfifo_bcom_irq(0, NULL);
}
EXPORT_SYMBOL(mpc52xx_lpbfifo_poll);
@@ -491,12 +491,12 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
lpbfifo.dev =3D &op->dev;
return 0;
=20
=2D err_bcom_tx:
+err_bcom_tx:
free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
=2D err_bcom_rx_irq:
+err_bcom_rx_irq:
bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
=2D err_bcom_rx:
=2D err_irq:
+err_bcom_rx:
+err_irq:
iounmap(lpbfifo.regs);
lpbfifo.regs =3D NULL;
=20
@@ -516,7 +516,7 @@ static int __devexit mpc52xx_lpbfifo_remove(struct of_d=
evice *op)
/* Release the bestcomm transmit task */
free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
bcom_gen_bd_tx_release(lpbfifo.bcom_tx_task);
=2D=09
+
/* Release the bestcomm receive task */
free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 01/13] powerpc/5200: LocalPlus driver: fix indentation and white space
2009-12-22 6:57 ` [PATCH 01/13] powerpc/5200: LocalPlus driver: fix indentation and white space Roman Fietze
@ 2010-01-11 19:06 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 19:06 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
2009-12-22 6:57 ` [PATCH 01/13] powerpc/5200: LocalPlus driver: fix indentation and white space Roman Fietze
@ 2009-12-22 6:59 ` Roman Fietze
2010-01-11 19:15 ` Grant Likely
2009-12-22 7:00 ` [PATCH 03/13] mpc52xx: add SCLPC register bit definitions Roman Fietze
` (10 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 6:59 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/include/asm/mpc52xx.h | 24 ++++++++
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 79 +++++++++++----------=
=2D---
2 files changed, 59 insertions(+), 44 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index b664ce7..57f8335 100644
=2D-- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -193,6 +193,30 @@ struct mpc52xx_xlb {
#define MPC52xx_XLB_CFG_PLDIS (1 << 31)
#define MPC52xx_XLB_CFG_SNOOP (1 << 15)
=20
+/* SCLPC */
+struct mpc52xx_sclpc {
+ union {
+ u8 restart; /* 0x00 restart bit */
+ u32 packet_size; /* 0x00 packet size register */
+ } packet_size;
+ u32 start_address; /* 0x04 start Address register */
+ u32 control; /* 0x08 control register */
+ u32 enable; /* 0x0C enable register */
+ u32 unused0; /* 0x10 */
+ union {
+ u8 status; /* 0x14 status register bits */
+ u32 bytes_done; /* 0x14 bytes done register bits, read only */
+ } bytes_done_status;
+
+ u32 reserved1[(0x40-0x18) / sizeof(u32)]; /* 0x18 .. 0x3c */
+
+ u32 fifo_data; /* 0x40 FIFO data word register */
+ u32 fifo_status; /* 0x44 FIFO status register */
+ u8 fifo_control; /* 0x48 FIFO control register */
+ u8 reserved2[3];
+ u32 fifo_alarm; /* 0x4C FIFO alarm register */
+};
+
/* Clock Distribution control */
struct mpc52xx_cdm {
u32 jtag_id; /* CDM + 0x00 reg0 read only */
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 4c84aa5..2763d5e 100644
=2D-- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -27,20 +27,10 @@ MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>=
");
MODULE_DESCRIPTION("MPC5200 LocalPlus FIFO device driver");
MODULE_LICENSE("GPL");
=20
=2D#define LPBFIFO_REG_PACKET_SIZE (0x00)
=2D#define LPBFIFO_REG_START_ADDRESS (0x04)
=2D#define LPBFIFO_REG_CONTROL (0x08)
=2D#define LPBFIFO_REG_ENABLE (0x0C)
=2D#define LPBFIFO_REG_BYTES_DONE_STATUS (0x14)
=2D#define LPBFIFO_REG_FIFO_DATA (0x40)
=2D#define LPBFIFO_REG_FIFO_STATUS (0x44)
=2D#define LPBFIFO_REG_FIFO_CONTROL (0x48)
=2D#define LPBFIFO_REG_FIFO_ALARM (0x4C)
=2D
struct mpc52xx_lpbfifo {
struct device *dev;
phys_addr_t regs_phys;
=2D void __iomem *regs;
+ struct mpc52xx_sclpc __iomem *regs;
int irq;
spinlock_t lock;
=20
@@ -72,10 +62,10 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
int poll_dma =3D req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
=20
/* Set and clear the reset bits; is good practice in User Manual */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, 0x01010000);
=20
/* set master enable bit */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x00000001);
+ out_be32(&lpbfifo.regs->enable, 0x00000001);
if (!dma) {
/* While the FIFO can be setup for transfer sizes as large as
* 16M-1, the FIFO itself is only 512 bytes deep and it does
@@ -91,14 +81,14 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
=20
/* Load the FIFO with data */
if (write) {
=2D reg =3D lpbfifo.regs + LPBFIFO_REG_FIFO_DATA;
+ reg =3D &lpbfifo.regs->fifo_data;
data =3D req->data + req->pos;
for (i =3D 0; i < transfer_size; i +=3D 4)
out_be32(reg, *data++);
}
=20
/* Unmask both error and completion irqs */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x00000301);
+ out_be32(&lpbfifo.regs->enable, 0x00000301);
} else {
/* Choose the correct direction
*
@@ -108,12 +98,12 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfif=
o_request *req)
* is a risk of DMA not transferring the last chunk of data
*/
if (write) {
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_FIFO_ALARM, 0x1e4);
=2D out_8(lpbfifo.regs + LPBFIFO_REG_FIFO_CONTROL, 7);
+ out_be32(&lpbfifo.regs->fifo_alarm, 0x1e4);
+ out_8(&lpbfifo.regs->fifo_control, 7);
lpbfifo.bcom_cur_task =3D lpbfifo.bcom_tx_task;
} else {
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_FIFO_ALARM, 0x1ff);
=2D out_8(lpbfifo.regs + LPBFIFO_REG_FIFO_CONTROL, 0);
+ out_be32(&lpbfifo.regs->fifo_alarm, 0x1ff);
+ out_8(&lpbfifo.regs->fifo_control, 0);
lpbfifo.bcom_cur_task =3D lpbfifo.bcom_rx_task;
=20
if (poll_dma) {
@@ -155,21 +145,21 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
/* Unmask irqs */
if (write && (!poll_dma))
bit_fields |=3D 0x00000100; /* completion irq too */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, bit_fields);
+ out_be32(&lpbfifo.regs->enable, bit_fields);
}
=20
/* Set transfer size, width, chip select and READ mode */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_START_ADDRESS,
+ out_be32(&lpbfifo.regs->start_address,
req->offset + req->pos);
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_PACKET_SIZE, transfer_size);
+ out_be32(&lpbfifo.regs->packet_size.packet_size, transfer_size);
=20
bit_fields =3D req->cs << 24 | 0x000008;
if (!write)
bit_fields |=3D 0x010000; /* read mode */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_CONTROL, bit_fields);
+ out_be32(&lpbfifo.regs->control, bit_fields);
=20
/* Kick it off */
=2D out_8(lpbfifo.regs + LPBFIFO_REG_PACKET_SIZE, 0x01);
+ out_8(&lpbfifo.regs->packet_size.restart, 0x01);
if (dma)
bcom_enable(lpbfifo.bcom_cur_task);
}
@@ -218,7 +208,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *dev_id)
{
struct mpc52xx_lpbfifo_request *req;
=2D u32 status =3D in_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS);
+ u32 status =3D in_8(&lpbfifo.regs->bytes_done_status.status);
void __iomem *reg;
u32 *data;
int count, i;
@@ -253,18 +243,18 @@ static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void =
*dev_id)
=20
/* check abort bit */
if (status & 0x10) {
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, 0x01010000);
do_callback =3D 1;
goto out;
}
=20
/* Read result from hardware */
=2D count =3D in_be32(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS);
+ count =3D in_be32(&lpbfifo.regs->bytes_done_status.bytes_done);
count &=3D 0x00ffffff;
=20
if (!dma && !write) {
/* copy the data out of the FIFO */
=2D reg =3D lpbfifo.regs + LPBFIFO_REG_FIFO_DATA;
+ reg =3D &lpbfifo.regs->fifo_data;
data =3D req->data + req->pos;
for (i =3D 0; i < count; i +=3D 4)
*data++ =3D in_be32(reg);
@@ -281,7 +271,7 @@ static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *d=
ev_id)
=20
out:
/* Clear the IRQ */
=2D out_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS, 0x01);
+ out_8(&lpbfifo.regs->bytes_done_status.status, 0x01);
=20
if (dma && (status & 0x11)) {
/*
@@ -379,11 +369,11 @@ void mpc52xx_lpbfifo_poll(void)
int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
=20
/*
=2D * For more information, see comments on the "Fat Lady"
+ * For more information, see comments on the "Fat Lady"=20
*/
if (dma && write)
mpc52xx_lpbfifo_irq(0, NULL);
=2D else
+ else=20
mpc52xx_lpbfifo_bcom_irq(0, NULL);
}
EXPORT_SYMBOL(mpc52xx_lpbfifo_poll);
@@ -429,7 +419,7 @@ void mpc52xx_lpbfifo_abort(struct mpc52xx_lpbfifo_reque=
st *req)
/* Put it into reset and clear the state */
bcom_gen_bd_rx_reset(lpbfifo.bcom_rx_task);
bcom_gen_bd_tx_reset(lpbfifo.bcom_tx_task);
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, 0x01010000);
lpbfifo.req =3D NULL;
}
spin_unlock_irqrestore(&lpbfifo.lock, flags);
@@ -459,19 +449,19 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
spin_lock_init(&lpbfifo.lock);
=20
/* Put FIFO into reset */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, 0x01010000);
=20
=2D /* Register the interrupt handler */
+ /* register the interrupt handler */
rc =3D request_irq(lpbfifo.irq, mpc52xx_lpbfifo_irq, 0,
"mpc52xx-lpbfifo", &lpbfifo);
if (rc)
goto err_irq;
=20
/* Request the Bestcomm receive (fifo --> memory) task and IRQ */
=2D lpbfifo.bcom_rx_task =3D
=2D bcom_gen_bd_rx_init(2, res.start + LPBFIFO_REG_FIFO_DATA,
=2D BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
=2D 16*1024*1024);
+ lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(2,
+ res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
+ BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
+ 16*1024*1024);
if (!lpbfifo.bcom_rx_task)
goto err_bcom_rx;
=20
@@ -482,9 +472,10 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const stru=
ct of_device_id *match)
goto err_bcom_rx_irq;
=20
/* Request the Bestcomm transmit (memory --> fifo) task and IRQ */
=2D lpbfifo.bcom_tx_task =3D
=2D bcom_gen_bd_tx_init(2, res.start + LPBFIFO_REG_FIFO_DATA,
=2D BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC);
+ lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(2,
+ res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
+ BCOM_INITIATOR_SCLPC,
+ BCOM_IPR_SCLPC);
if (!lpbfifo.bcom_tx_task)
goto err_bcom_tx;
=20
@@ -511,12 +502,12 @@ static int __devexit mpc52xx_lpbfifo_remove(struct of=
_device *op)
return 0;
=20
/* Put FIFO in reset */
=2D out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, 0x01010000);
=20
=2D /* Release the bestcomm transmit task */
+ /* release the bestcomm transmit task */
free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
bcom_gen_bd_tx_release(lpbfifo.bcom_tx_task);
=2D
+=09
/* Release the bestcomm receive task */
free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2009-12-22 6:59 ` [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure Roman Fietze
@ 2010-01-11 19:15 ` Grant Likely
2010-01-11 19:42 ` Scott Wood
` (2 more replies)
0 siblings, 3 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 19:15 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2010-01-11 19:15 ` Grant Likely
@ 2010-01-11 19:42 ` Scott Wood
2010-01-11 19:59 ` Grant Likely
2010-01-11 20:43 ` Wolfgang Denk
2010-01-12 7:06 ` Roman Fietze
2 siblings, 1 reply; 46+ messages in thread
From: Scott Wood @ 2010-01-11 19:42 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, Roman Fietze
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 [flat|nested] 46+ messages in thread
* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2010-01-11 19:42 ` Scott Wood
@ 2010-01-11 19:59 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 19:59 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Roman Fietze
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 [flat|nested] 46+ messages in thread
* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2010-01-11 19:15 ` Grant Likely
2010-01-11 19:42 ` Scott Wood
@ 2010-01-11 20:43 ` Wolfgang Denk
2010-01-11 21:20 ` Grant Likely
2010-01-12 7:06 ` Roman Fietze
2 siblings, 1 reply; 46+ messages in thread
From: Wolfgang Denk @ 2010-01-11 20:43 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, Roman Fietze
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 [flat|nested] 46+ messages in thread
* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2010-01-11 20:43 ` Wolfgang Denk
@ 2010-01-11 21:20 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 21:20 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, Roman Fietze
On Mon, Jan 11, 2010 at 1:43 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Grant,
>
> In message <fa686aa41001111115g3451c9b9h4d6c551afd5698e1@mail.gmail.com> =
you 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
>
> May I ask _why_ you consider this bad practice?
Many reasons. First off, while C structures somewhat represent the
layout of a hardware register set, I still find them a poor fit.
Registers do not data structures, and trying to describe them as such
causes problems. Not all devices get mapped onto the bus in the same
way. ie. a single device can get wired up with 8-bit wide addressing
on one system and 32 wide on another. A struct cannot encode this. I
also find I often need to access registers at "none-native" widths due
to implementation details of the device which is made messy when the
layout is encoded in a C struct. Finally, we're talking about a
hardware interface here. Driver authors must understand exactly what
they are doing when writing to registers and it is my opinion (though
others may disagree with me) that using structs to describe register
maps encourages a glosses over details that are best left explicit.
I used to prefer C structs for register definitions, but my opinion
changed as I gained more experience.
> 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? =A0Using lists of register offsets
> (without any type information) as for example ARM is doing?
Yes, that is what I prefer. That and, when needed, device-specific
accessor functions that can be adapted for different bus attachements.
>> driver without a structure for a reason. =A0The reason I haven't removed
>
> Could you please explain this reason?
As described above.
> 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?
I've stated what I prefer. I don't think I've rejected any code that
uses structs over register offsets, but I do apply friction on any
patch that changes a current driver from one to the other without
need.
> And: is this just your personal preferences, or generally agreed on?
Others will need to answer that.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2010-01-11 19:15 ` Grant Likely
2010-01-11 19:42 ` Scott Wood
2010-01-11 20:43 ` Wolfgang Denk
@ 2010-01-12 7:06 ` Roman Fietze
2010-01-12 14:33 ` Grant Likely
2 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2010-01-12 7:06 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: Text/Plain, Size: 1053 bytes --]
Hello Grant,
On Monday 11 January 2010 20:15:58 Grant Likely 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.
And a lot of other information. Thank you very much for the effort you
put into those patches. I almost knew that you will criticize e.g.
patches that mix functional and "optical" changes, patches that are
not describing more exactly what I did.
As soon as I have some time left I will try to go once more from the
original driver to the final driver using clear and separate, well
bescribed patches, of course taking care of your and other people
comments.
Again, thanks
Roman
--
Roman Fietze Telemotive AG Büro Mühlhausen
Breitwiesen 73347 Mühlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
Amtsgericht Ulm HRB 541321
Vorstand:
Peter Kersten, Markus Fischer, Franz Diller, Markus Stolz
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure
2010-01-12 7:06 ` Roman Fietze
@ 2010-01-12 14:33 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-12 14:33 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
On Tue, Jan 12, 2010 at 12:06 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
> Hello Grant,
>
> On Monday 11 January 2010 20:15:58 Grant Likely wrote:
>
>> No patch description? =A0Very few patches are sufficiently described by
>> the subject line alone. =A0Tell me what the problem is, what the patch
>> changes, and why.
>
> And a lot of other information. Thank you very much for the effort you
> put into those patches. I almost knew that you will criticize e.g.
> patches that mix functional and "optical" changes, patches that are
> not describing more exactly what I did.
>
> As soon as I have some time left I will try to go once more from the
> original driver to the final driver using clear and separate, well
> bescribed patches, of course taking care of your and other people
> comments.
Sounds good. I look forward to seeing them.
g.
>
> Again, thanks
>
>
> Roman
>
> --
> 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
>
> Amtsgericht Ulm =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0HRB 541321
> Vorstand:
> Peter Kersten, Markus Fischer, Franz Diller, Markus Stolz
>
> _______________________________________________
> 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 [flat|nested] 46+ messages in thread
* [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
2009-12-22 6:57 ` [PATCH 01/13] powerpc/5200: LocalPlus driver: fix indentation and white space Roman Fietze
2009-12-22 6:59 ` [PATCH 02/13] powerpc/5200: LocalPlus driver: use SCLPC register structure Roman Fietze
@ 2009-12-22 7:00 ` Roman Fietze
2010-01-11 19:21 ` Grant Likely
2009-12-22 7:01 ` [PATCH 04/13] mpc52xx: LocalPlus driver: rewrite interrupt routines, fix errors Roman Fietze
` (9 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:00 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/include/asm/mpc52xx.h | 40 +++++++++++++++++++++++++++-----=
=2D--
1 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index 57f8335..c659d1d 100644
=2D-- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -17,6 +17,7 @@
#include <asm/types.h>
#include <asm/prom.h>
#include <asm/mpc5xxx.h>
+#include <linux/bitops.h>
#endif /* __ASSEMBLY__ */
=20
#include <linux/suspend.h>
@@ -212,11 +213,34 @@ struct mpc52xx_sclpc {
=20
u32 fifo_data; /* 0x40 FIFO data word register */
u32 fifo_status; /* 0x44 FIFO status register */
=2D u8 fifo_control; /* 0x48 FIFO control register */
=2D u8 reserved2[3];
+ u32 fifo_control; /* 0x48 FIFO control register */
u32 fifo_alarm; /* 0x4C FIFO alarm register */
};
=20
+#define MPC52xx_SCLPC_FIFO_SIZE (0x200) /* FIFO size 512 bytes */
+
+#define MPC52xx_SCLPC_CONTROL_CS(cs) ((uint32_t)(cs) << 24) /* CSX bits */
+#define MPC52xx_SCLPC_CONTROL_FLUSH BIT(17) /* flush, used in last packe=
t */
+#define MPC52xx_SCLPC_CONTROL_RWB_RECEIVE BIT(16) /* RWb bit, 1 =3D recei=
ve */
+#define MPC52xx_SCLPC_CONTROL_DAI BIT(8)
+
+#define MPC52xx_SCLPC_ENABLE_RC BIT(24) /* reset controller bit */
+#define MPC52xx_SCLPC_ENABLE_RF BIT(16) /* reset FIFO bit */
+#define MPC52xx_SCLPC_ENABLE_AIE BIT(9) /* abort interrupt enable bit */
+#define MPC52xx_SCLPC_ENABLE_NIE BIT(8) /* normal interrupt enable bit */
+#define MPC52xx_SCLPC_ENABLE_ME BIT(0) /* master enable bit */
+
+#define MPC52xx_SCLPC_PACKET_SIZE_RESTART BIT(24)
+
+#define MPC52xx_SCLPC_STATUS_AT BIT(28) /* abort termination */
+#define MPC52xx_SCLPC_STATUS_NT BIT(24) /* normal termination */
+#define MPC52xx_SCLPC_STATUS_BYTES_DONE_MASK (0x00FFFFFFU) /* bytes done b=
it mask */
+
+#define MPC52xx_SLPC_FIFO_STATUS_ERR BIT(22) /* error bit */
+
+#define MPC52xx_SLPC_FIFO_CONTROL_GR(gr) ((gr) << 24) /* granularity bits =
*/
+
+
/* Clock Distribution control */
struct mpc52xx_cdm {
u32 jtag_id; /* CDM + 0x00 reg0 read only */
@@ -304,19 +328,18 @@ extern void mpc52xx_restart(char *cmd);
struct mpc52xx_gpt_priv;
extern struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq);
extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 perio=
d,
=2D int continuous);
+ int continuous);
extern u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt);
extern int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
=20
/* mpc52xx_lpbfifo.c */
#define MPC52XX_LPBFIFO_FLAG_READ (0)
=2D#define MPC52XX_LPBFIFO_FLAG_WRITE (1<<0)
=2D#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT (1<<1)
=2D#define MPC52XX_LPBFIFO_FLAG_NO_DMA (1<<2)
=2D#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)
=20
struct mpc52xx_lpbfifo_request {
=2D struct list_head list;
=20
/* localplus bus address */
unsigned int cs;
@@ -383,4 +406,3 @@ extern char saved_sram[0x4000]; /* reuse buffer from mp=
c52xx suspend */
#endif /* CONFIG_PM */
=20
#endif /* __ASM_POWERPC_MPC52xx_H__ */
=2D
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
2009-12-22 7:00 ` [PATCH 03/13] mpc52xx: add SCLPC register bit definitions Roman Fietze
@ 2010-01-11 19:21 ` Grant Likely
2010-01-11 20:50 ` Wolfgang Denk
2010-01-12 7:55 ` Roman Fietze
0 siblings, 2 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 19:21 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
2010-01-11 19:21 ` Grant Likely
@ 2010-01-11 20:50 ` Wolfgang Denk
2010-01-12 7:55 ` Roman Fietze
1 sibling, 0 replies; 46+ messages in thread
From: Wolfgang Denk @ 2010-01-11 20:50 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, Roman Fietze
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 [flat|nested] 46+ messages in thread
* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
2010-01-11 19:21 ` Grant Likely
2010-01-11 20:50 ` Wolfgang Denk
@ 2010-01-12 7:55 ` Roman Fietze
2010-01-12 14:07 ` Grant Likely
2010-01-12 14:29 ` Grant Likely
1 sibling, 2 replies; 46+ messages in thread
From: Roman Fietze @ 2010-01-12 7:55 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
[-- Attachment #1: Type: Text/Plain, Size: 1229 bytes --]
Hello Grant,
On Monday 11 January 2010 20:21:22 Grant Likely wrote:
> Unrelated whitespace change?
My fault, as with some other white space changes. My Emacs is
configured using some older setup found in a 2.4
Documentation/CodingStyle. Before saving a file my spine is used to
reformat the whole source file, which then causes those artefacts.
I'll fix the Emacs setup and try to fix my spine, and double check
before commits.
BTW: no Emacs users amongst the kernel coders any more? At least I
have to read "So, you can either get rid of GNU emacs, or ...".
> I prefer the (1<<n) style myself.
Ok. Looking at it a second time you and Wolfgang are definitively
right. And I prefer the (1<<n) style a lot more than the 0x0080 style.
> Why is the list head being removed?
Not used at all, except in initialization?
Of course a seperate patch would have been needed.
Roman
--
Roman Fietze Telemotive AG Büro Mühlhausen
Breitwiesen 73347 Mühlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
Amtsgericht Ulm HRB 541321
Vorstand:
Peter Kersten, Markus Fischer, Franz Diller, Markus Stolz
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
2010-01-12 7:55 ` Roman Fietze
@ 2010-01-12 14:07 ` Grant Likely
2010-01-12 14:29 ` Grant Likely
1 sibling, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-12 14:07 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
On Tue, Jan 12, 2010 at 12:55 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
> Hello Grant,
>
> On Monday 11 January 2010 20:21:22 Grant Likely wrote:
>
>> Unrelated whitespace change?
>
> My fault, as with some other white space changes. My Emacs is
> configured using some older setup found in a 2.4
> Documentation/CodingStyle. Before saving a file my spine is used to
> reformat the whole source file, which then causes those artefacts.
> I'll fix the Emacs setup and try to fix my spine, and double check
> before commits.
>
> BTW: no Emacs users amongst the kernel coders any more? At least I
> have to read "So, you can either get rid of GNU emacs, or ...".
I believe there are lots of emacs using kernel hackers. emacs is fine
and most of the style changes you were making were valid, but they
were unrelated and unimportant. Unless emacs is making automatic
changes to the files, it sounds like your emacs config is just fine.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 03/13] mpc52xx: add SCLPC register bit definitions
2010-01-12 7:55 ` Roman Fietze
2010-01-12 14:07 ` Grant Likely
@ 2010-01-12 14:29 ` Grant Likely
1 sibling, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-12 14:29 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
On Tue, Jan 12, 2010 at 12:55 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
> On Monday 11 January 2010 20:21:22 Grant Likely wrote:
>> Why is the list head being removed?
>
> Not used at all, except in initialization?
>
> Of course a seperate patch would have been needed.
For the record, I put the list_head in when writing the driver so that
requests can get queued up instead of the one-at-a-time model
currently used. It is still my intention to implement request queuing
but as with so many things I haven't got to it yet. :-/
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 04/13] mpc52xx: LocalPlus driver: rewrite interrupt routines, fix errors
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (2 preceding siblings ...)
2009-12-22 7:00 ` [PATCH 03/13] mpc52xx: add SCLPC register bit definitions Roman Fietze
@ 2009-12-22 7:01 ` Roman Fietze
2010-01-11 19:44 ` Grant Likely
2009-12-22 7:02 ` [PATCH 05/13] powerpc/5200: LocalPlus driver: fix DMA TX interrupt request Roman Fietze
` (8 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:01 UTC (permalink / raw)
To: linuxppc-dev
Use SCLPC bit definitions from mpc52xx.h for better readability.
Rewrite IRQ handlers, make them work for DMA.
=46ix module unload error.
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 306 ++++++++++++---------=
=2D---
1 files changed, 149 insertions(+), 157 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 2763d5e..2fd1f3f 100644
=2D-- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -46,6 +46,34 @@ struct mpc52xx_lpbfifo {
/* The MPC5200 has only one fifo, so only need one instance structure */
static struct mpc52xx_lpbfifo lpbfifo;
=20
+
+/**
+ * mpc52xx_lpbfifo_is_write - return true if it's a WRITE request
+ */
+static inline int mpc52xx_lpbfifo_is_write(int flags)
+{
+ 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)
+{
+ return !(flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
+}
+
+
+/**
+ * mpc52xx_lpbfifo_is_poll_dma - return true if it's a polled DMA request
+ */
+static inline int mpc52xx_lpbfifo_is_poll_dma(int flags)
+{
+ return flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
+}
+
+
/**
* mpc52xx_lpbfifo_kick - Trigger the next block of data to be transfered
*/
@@ -57,16 +85,23 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
u32 *data;
int i;
int bit_fields;
=2D int dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
=2D int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
=2D int poll_dma =3D req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
+ int rflags =3D req->flags;
=20
/* Set and clear the reset bits; is good practice in User Manual */
=2D out_be32(&lpbfifo.regs->enable, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_E=
NABLE_RF);
+
+ /* Set width, chip select and READ mode */
+ out_be32(&lpbfifo.regs->start_address, req->offset + req->pos);
+
+ /* Set CS and BPT */
+ bit_fields =3D MPC52xx_SCLPC_CONTROL_CS(req->cs) | 0x8;
+ if (!(mpc52xx_lpbfifo_is_write(rflags))) {
+ bit_fields |=3D MPC52xx_SCLPC_CONTROL_RWB_RECEIVE; /* read mode */
+ bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
+ }
+ out_be32(&lpbfifo.regs->control, bit_fields);
=20
=2D /* set master enable bit */
=2D out_be32(&lpbfifo.regs->enable, 0x00000001);
=2D if (!dma) {
+ if (!mpc52xx_lpbfifo_is_dma(rflags)) {
/* While the FIFO can be setup for transfer sizes as large as
* 16M-1, the FIFO itself is only 512 bytes deep and it does
* not generate interrupts for FIFO full events (only transfer
@@ -80,7 +115,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_=
request *req)
transfer_size =3D 512;
=20
/* Load the FIFO with data */
=2D if (write) {
+ if (mpc52xx_lpbfifo_is_write(rflags)) {
reg =3D &lpbfifo.regs->fifo_data;
data =3D req->data + req->pos;
for (i =3D 0; i < transfer_size; i +=3D 4)
@@ -88,7 +123,9 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_=
request *req)
}
=20
/* Unmask both error and completion irqs */
=2D out_be32(&lpbfifo.regs->enable, 0x00000301);
+ out_be32(&lpbfifo.regs->enable, (MPC52xx_SCLPC_ENABLE_AIE |
+ MPC52xx_SCLPC_ENABLE_NIE |
+ MPC52xx_SCLPC_ENABLE_ME));
} else {
/* Choose the correct direction
*
@@ -97,16 +134,16 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfif=
o_request *req)
* there is a performance impacit. However, if it is wrong there
* is a risk of DMA not transferring the last chunk of data
*/
=2D if (write) {
=2D out_be32(&lpbfifo.regs->fifo_alarm, 0x1e4);
=2D out_8(&lpbfifo.regs->fifo_control, 7);
+ if (mpc52xx_lpbfifo_is_write(rflags)) {
+ out_be32(&lpbfifo.regs->fifo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 28);
+ out_be32(&lpbfifo.regs->fifo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(7));
lpbfifo.bcom_cur_task =3D lpbfifo.bcom_tx_task;
} else {
=2D out_be32(&lpbfifo.regs->fifo_alarm, 0x1ff);
=2D out_8(&lpbfifo.regs->fifo_control, 0);
+ out_be32(&lpbfifo.regs->fifo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 1);
+ out_be32(&lpbfifo.regs->fifo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(0));
lpbfifo.bcom_cur_task =3D lpbfifo.bcom_rx_task;
=20
=2D if (poll_dma) {
+ if (mpc52xx_lpbfifo_is_poll_dma(rflags)) {
if (lpbfifo.dma_irqs_enabled) {
disable_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task));
lpbfifo.dma_irqs_enabled =3D 0;
@@ -119,63 +156,34 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
}
}
=20
+ /* error irq & master enabled bit */
+ out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_AIE | MPC52xx_SCLPC=
_ENABLE_NIE | MPC52xx_SCLPC_ENABLE_ME);
+
bd =3D bcom_prepare_next_buffer(lpbfifo.bcom_cur_task);
bd->status =3D transfer_size;
=2D if (!write) {
=2D /*
=2D * In the DMA read case, the DMA doesn't complete,
=2D * possibly due to incorrect watermarks in the ALARM
=2D * and CONTROL regs. For now instead of trying to
=2D * determine the right watermarks that will make this
=2D * work, just increase the number of bytes the FIFO is
=2D * expecting.
=2D *
=2D * When submitting another operation, the FIFO will get
=2D * reset, so the condition of the FIFO waiting for a
=2D * non-existent 4 bytes will get cleared.
=2D */
=2D transfer_size +=3D 4; /* BLECH! */
=2D }
bd->data[0] =3D req->data_phys + req->pos;
bcom_submit_next_buffer(lpbfifo.bcom_cur_task, NULL);
=2D
=2D /* error irq & master enabled bit */
=2D bit_fields =3D 0x00000201;
=2D
=2D /* Unmask irqs */
=2D if (write && (!poll_dma))
=2D bit_fields |=3D 0x00000100; /* completion irq too */
=2D out_be32(&lpbfifo.regs->enable, bit_fields);
}
=20
=2D /* Set transfer size, width, chip select and READ mode */
=2D out_be32(&lpbfifo.regs->start_address,
=2D req->offset + req->pos);
=2D out_be32(&lpbfifo.regs->packet_size.packet_size, transfer_size);
=2D
=2D bit_fields =3D req->cs << 24 | 0x000008;
=2D if (!write)
=2D bit_fields |=3D 0x010000; /* read mode */
=2D out_be32(&lpbfifo.regs->control, bit_fields);
=2D
=2D /* Kick it off */
=2D out_8(&lpbfifo.regs->packet_size.restart, 0x01);
=2D if (dma)
+ /* Set packet size and kick it off */
+ out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_SCLPC_PACKET_SIZ=
E_RESTART | transfer_size);
+ if (mpc52xx_lpbfifo_is_dma(rflags))
bcom_enable(lpbfifo.bcom_cur_task);
}
=20
/**
=2D * mpc52xx_lpbfifo_irq - IRQ handler for LPB FIFO
+ * mpc52xx_lpbfifo_sclpc_irq - IRQ handler for LPB FIFO
*
=2D * On transmit, the dma completion irq triggers before the fifo completi=
on
=2D * triggers. Handle the dma completion here instead of the LPB FIFO Bes=
tcomm
=2D * task completion irq becuase everyting is not really done until the LP=
B FIFO
=2D * completion irq triggers.
+ * On transmit, the dma completion irq triggers before the fifo
+ * completion triggers. Handle 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
=2D * transmit, the fifo completion irq is the "Fat Lady". The opera (or in=
this
=2D * case the DMA/FIFO operation) is not finished until the "Fat Lady" sin=
gs.
+ * 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.
*
* Reasons for entering this routine:
* 1) PIO mode rx and tx completion irq
@@ -205,17 +213,17 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
* extra fiddling is done to make sure all paths lead to the same
* outbound code.
*/
=2Dstatic irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *dev_id)
+static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq, void *dev_id)
{
struct mpc52xx_lpbfifo_request *req;
=2D u32 status =3D in_8(&lpbfifo.regs->bytes_done_status.status);
+ u32 status_count =3D in_be32(&lpbfifo.regs->bytes_done_status.bytes_done);
void __iomem *reg;
u32 *data;
=2D int count, i;
+ size_t i;
int do_callback =3D 0;
u32 ts;
unsigned long flags;
=2D int dma, write, poll_dma;
+ int rflags;
=20
spin_lock_irqsave(&lpbfifo.lock, flags);
ts =3D get_tbl();
@@ -223,87 +231,79 @@ static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void =
*dev_id)
req =3D lpbfifo.req;
if (!req) {
spin_unlock_irqrestore(&lpbfifo.lock, flags);
=2D pr_err("bogus LPBFIFO IRQ\n");
+ pr_err("bogus SCLPC IRQ\n");
return IRQ_HANDLED;
}
=20
=2D dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
=2D write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
=2D poll_dma =3D req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
+ rflags =3D req->flags;
=20
=2D if (dma && !write) {
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
=2D pr_err("bogus LPBFIFO IRQ (dma and not writting)\n");
=2D return IRQ_HANDLED;
=2D }
=2D
=2D if ((status & 0x01) =3D=3D 0) {
+ /* check normal termination bit */
+ if (!(status_count & MPC52xx_SCLPC_STATUS_NT))
goto out;
=2D }
=20
/* check abort bit */
=2D if (status & 0x10) {
=2D out_be32(&lpbfifo.regs->enable, 0x01010000);
+ if (status_count & MPC52xx_SCLPC_STATUS_AT) {
+ out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_=
ENABLE_RF);
do_callback =3D 1;
goto out;
}
=20
=2D /* Read result from hardware */
=2D count =3D in_be32(&lpbfifo.regs->bytes_done_status.bytes_done);
=2D count &=3D 0x00ffffff;
+ if (!mpc52xx_lpbfifo_is_dma(rflags)) {
=20
=2D if (!dma && !write) {
=2D /* copy the data out of the FIFO */
=2D reg =3D &lpbfifo.regs->fifo_data;
=2D data =3D req->data + req->pos;
=2D for (i =3D 0; i < count; i +=3D 4)
=2D *data++ =3D in_be32(reg);
=2D }
+ /* bytes done */
+ status_count &=3D MPC52xx_SCLPC_STATUS_BYTES_DONE_MASK;
=20
=2D /* Update transfer position and count */
=2D req->pos +=3D count;
+ if (!mpc52xx_lpbfifo_is_write(rflags)) {
+ /* copy the data out of the FIFO */
+ reg =3D &lpbfifo.regs->fifo_data;
+ data =3D req->data + req->pos;
+ for (i =3D 0; i < status_count; i +=3D sizeof(u32))
+ *data++ =3D in_be32(reg);
+ }
=20
=2D /* Decide what to do next */
=2D if (req->size - req->pos)
=2D mpc52xx_lpbfifo_kick(req); /* more work to do */
=2D else
+ /* Update transfer position and count */
+ req->pos +=3D status_count;
+
+ /* Decide what to do next */
+ if (req->size - req->pos)
+ mpc52xx_lpbfifo_kick(req); /* more work to do */
+ else
+ do_callback =3D 1;
+ }
+ else {
do_callback =3D 1;
+ }
=20
out:
/* Clear the IRQ */
=2D out_8(&lpbfifo.regs->bytes_done_status.status, 0x01);
+ out_8(&lpbfifo.regs->bytes_done_status.status, BIT(0));
=20
=2D if (dma && (status & 0x11)) {
=2D /*
=2D * Count the DMA as complete only when the FIFO completion
=2D * status or abort bits are set.
=2D *
=2D * (status & 0x01) should always be the case except sometimes
=2D * when using polled DMA.
=2D *
=2D * (status & 0x10) {transfer aborted}: This case needs more
=2D * testing.
=2D */
=2D bcom_retrieve_buffer(lpbfifo.bcom_cur_task, &status, NULL);
=2D }
req->last_byte =3D ((u8 *)req->data)[req->size - 1];
=20
+ if (irq !=3D 0) /* don't increment on polled case */
+ req->irq_count++;
+
/* When the do_callback flag is set; it means the transfer is finished
* so set the FIFO as idle */
=2D if (do_callback)
+ if (do_callback) {
lpbfifo.req =3D NULL;
+ out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_=
ENABLE_RF);
=20
=2D if (irq !=3D 0) /* don't increment on polled case */
=2D req->irq_count++;
+ req->irq_ticks +=3D get_tbl() - ts;
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
=20
=2D req->irq_ticks +=3D get_tbl() - ts;
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ /* Spinlock is released; it is now safe to call the callback */
+ if (req->callback)
+ req->callback(req);
=20
=2D /* Spinlock is released; it is now safe to call the callback */
=2D if (do_callback && req->callback)
=2D req->callback(req);
+ return IRQ_HANDLED;
+ }
+ else {
+ req->irq_ticks +=3D get_tbl() - ts;
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
=20
=2D return IRQ_HANDLED;
+ return IRQ_HANDLED;
+ }
}
=20
/**
@@ -313,48 +313,30 @@ out:
*/
static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, void *dev_id)
{
=2D struct mpc52xx_lpbfifo_request *req;
+ struct mpc52xx_lpbfifo *lpbfifo =3D dev_id;
unsigned long flags;
=2D u32 status;
=2D u32 ts;
=2D
=2D spin_lock_irqsave(&lpbfifo.lock, flags);
=2D ts =3D get_tbl();
=2D
=2D req =3D lpbfifo.req;
=2D if (!req || (req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA)) {
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
=2D return IRQ_HANDLED;
=2D }
=20
=2D if (irq !=3D 0) /* don't increment on polled case */
=2D req->irq_count++;
+ spin_lock_irqsave(&lpbfifo->lock, flags);
+ // ts =3D get_tbl();
=20
=2D if (!bcom_buffer_done(lpbfifo.bcom_cur_task)) {
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ if (!bcom_buffer_done(lpbfifo->bcom_cur_task)) {
=20
=2D req->buffer_not_done_cnt++;
=2D if ((req->buffer_not_done_cnt % 1000) =3D=3D 0)
=2D pr_err("transfer stalled\n");
+ if (bcom_queue_empty(lpbfifo->bcom_cur_task)) {
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
+ dev_err(lpbfifo->dev, "DMA queue empty\n");
+ }
+ else {
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
+ dev_err(lpbfifo->dev, "DMA buffer not done\n");
+ }
=20
return IRQ_HANDLED;
}
=20
=2D bcom_retrieve_buffer(lpbfifo.bcom_cur_task, &status, NULL);
=2D
=2D req->last_byte =3D ((u8 *)req->data)[req->size - 1];
=2D
=2D req->pos =3D status & 0x00ffffff;
=2D
=2D /* Mark the FIFO as idle */
=2D lpbfifo.req =3D NULL;
+ bcom_retrieve_buffer(lpbfifo->bcom_cur_task, NULL, NULL);
+ // req->irq_ticks +=3D get_tbl() - ts;
=20
=2D /* Release the lock before calling out to the callback. */
=2D req->irq_ticks +=3D get_tbl() - ts;
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
=2D
=2D if (req->callback)
=2D req->callback(req);
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
=20
return IRQ_HANDLED;
}
@@ -365,14 +347,12 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, =
void *dev_id)
void mpc52xx_lpbfifo_poll(void)
{
struct mpc52xx_lpbfifo_request *req =3D lpbfifo.req;
=2D int dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
=2D int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
=20
/*
* For more information, see comments on the "Fat Lady"=20
*/
=2D if (dma && write)
=2D mpc52xx_lpbfifo_irq(0, NULL);
+ if (mpc52xx_lpbfifo_is_dma(req->flags) && (req->flags & MPC52XX_LPBFIFO_F=
LAG_WRITE))
+ mpc52xx_lpbfifo_sclpc_irq(0, NULL);
else=20
mpc52xx_lpbfifo_bcom_irq(0, NULL);
}
@@ -406,6 +386,7 @@ int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_reque=
st *req)
=20
mpc52xx_lpbfifo_kick(req);
spin_unlock_irqrestore(&lpbfifo.lock, flags);
+
return 0;
}
EXPORT_SYMBOL(mpc52xx_lpbfifo_submit);
@@ -419,7 +400,7 @@ void mpc52xx_lpbfifo_abort(struct mpc52xx_lpbfifo_reque=
st *req)
/* Put it into reset and clear the state */
bcom_gen_bd_rx_reset(lpbfifo.bcom_rx_task);
bcom_gen_bd_tx_reset(lpbfifo.bcom_tx_task);
=2D out_be32(&lpbfifo.regs->enable, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_=
ENABLE_RF);
lpbfifo.req =3D NULL;
}
spin_unlock_irqrestore(&lpbfifo.lock, flags);
@@ -449,16 +430,16 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
spin_lock_init(&lpbfifo.lock);
=20
/* Put FIFO into reset */
=2D out_be32(&lpbfifo.regs->enable, 0x01010000);
+ out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_E=
NABLE_RF);
=20
/* register the interrupt handler */
=2D rc =3D request_irq(lpbfifo.irq, mpc52xx_lpbfifo_irq, 0,
+ rc =3D request_irq(lpbfifo.irq, mpc52xx_lpbfifo_sclpc_irq, 0,
"mpc52xx-lpbfifo", &lpbfifo);
if (rc)
goto err_irq;
=20
/* Request the Bestcomm receive (fifo --> memory) task and IRQ */
=2D lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(2,
+ lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(16,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
16*1024*1024);
@@ -472,16 +453,27 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
goto err_bcom_rx_irq;
=20
/* Request the Bestcomm transmit (memory --> fifo) task and IRQ */
=2D lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(2,
+ lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(16,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC,
BCOM_IPR_SCLPC);
if (!lpbfifo.bcom_tx_task)
goto err_bcom_tx;
=20
+ rc =3D request_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task),
+ mpc52xx_lpbfifo_bcom_irq, 0,
+ "mpc52xx-lpbfifo-rx", &lpbfifo);
+ if (rc)
+ goto err_bcom_tx_irq;
+
+ lpbfifo.dma_irqs_enabled =3D 1;
+
lpbfifo.dev =3D &op->dev;
+
return 0;
=20
+err_bcom_tx_irq:
+ free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
err_bcom_tx:
free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
err_bcom_rx_irq:
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 04/13] mpc52xx: LocalPlus driver: rewrite interrupt routines, fix errors
2009-12-22 7:01 ` [PATCH 04/13] mpc52xx: LocalPlus driver: rewrite interrupt routines, fix errors Roman Fietze
@ 2010-01-11 19:44 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 19:44 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 05/13] powerpc/5200: LocalPlus driver: fix DMA TX interrupt request
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (3 preceding siblings ...)
2009-12-22 7:01 ` [PATCH 04/13] mpc52xx: LocalPlus driver: rewrite interrupt routines, fix errors Roman Fietze
@ 2009-12-22 7:02 ` Roman Fietze
2009-12-22 7:20 ` Grant Likely
2009-12-22 7:04 ` [PATCH 06/13] powerpc/5200: LocalPlus driver: map and unmap DMA areas Roman Fietze
` (7 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:02 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 2fd1f3f..1e4f725 100644
=2D-- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -460,9 +460,9 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
if (!lpbfifo.bcom_tx_task)
goto err_bcom_tx;
=20
=2D rc =3D request_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task),
+ rc =3D request_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task),
mpc52xx_lpbfifo_bcom_irq, 0,
=2D "mpc52xx-lpbfifo-rx", &lpbfifo);
+ "mpc52xx-lpbfifo-tx", &lpbfifo);
if (rc)
goto err_bcom_tx_irq;
=20
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 05/13] powerpc/5200: LocalPlus driver: fix DMA TX interrupt request
2009-12-22 7:02 ` [PATCH 05/13] powerpc/5200: LocalPlus driver: fix DMA TX interrupt request Roman Fietze
@ 2009-12-22 7:20 ` Grant Likely
2009-12-22 7:42 ` Roman Fietze
0 siblings, 1 reply; 46+ messages in thread
From: Grant Likely @ 2009-12-22 7:20 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
On Tue, Dec 22, 2009 at 12:02 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 =A04 ++--
> =A01 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> index 2fd1f3f..1e4f725 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -460,9 +460,9 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const str=
uct of_device_id *match)
> =A0 =A0 =A0 =A0if (!lpbfifo.bcom_tx_task)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_bcom_tx;
>
> - =A0 =A0 =A0 rc =3D request_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task),
> + =A0 =A0 =A0 rc =3D request_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task),
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_lpbfifo_bcom_irq,=
0,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"mpc52xx-lpbfifo-rx", &l=
pbfifo);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"mpc52xx-lpbfifo-tx", &l=
pbfifo);
Is this really what you want? In the TX path, it is the FIFO irq that
indicates the transfer is complete (ie, the DMA fills the FIFO and
finishes before the FIFO has drained). If both irqs were enabled,
then the DMA irq would fire first, followed by the FIFO irq. For the
use cases I was dealing with, the next transfer cannot be kicked off
until the FIFO has drained.
In the RX path, the DMA irq indicates that the transfer is complete
(ie, the localbus fills the FIFO and finishes before the DMA drains
the FIFO.
For my use cases, the RX DMA irq was always required, but the TX DMA
irq was never needed.
Does your use case differ?
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 05/13] powerpc/5200: LocalPlus driver: fix DMA TX interrupt request
2009-12-22 7:20 ` Grant Likely
@ 2009-12-22 7:42 ` Roman Fietze
0 siblings, 0 replies; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:42 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
Hello Grant,
On Tuesday 22 December 2009 08:20:16 Grant Likely wrote:
> Is this really what you want? In the TX path, it is the FIFO irq
> that indicates the transfer is complete (ie, the DMA fills the FIFO
> and finishes before the FIFO has drained). If both irqs were
> enabled, then the DMA irq would fire first, followed by the FIFO
> irq. For the use cases I was dealing with, the next transfer cannot
> be kicked off until the FIFO has drained.
I added another patch (PATCH 10/13) that takes care of this problem.
Debugging output (some tiny printk's when I detected the "abnormal"
case) showed, that when receiving data one cannot predict the order of
the the SDMA and SCLPC interrupt, esp. when e.g. the BestComm has
other tasks with higher prio running, in my case ATA+FEC. I hope I
didn't kill the TX side with this change, because, as I said, could
not yet test it.
Two possible solutions came into my mind:
1) let the SDMA interrupts run freely in parallel and just use the
SCLPC interrupt to detect a finished transaction.
2) Always wait for both, the second one defined the end of the
transaction. IMHO some waste of time.
> In the RX path, the DMA irq indicates that the transfer is complete
> (ie, the localbus fills the FIFO and finishes before the DMA drains
> the FIFO.
See above. That's not always true as stress tests showed. I had cases
where the BestComm was second. The SDMA beeing first is almost always
true for non BD tasks (there is one available in the 2.4.25, but not
in the 2.6 yet), and is mostly true even with BD tasks, but not 100%.
> For my use cases, the RX DMA irq was always required, but the TX DMA
> irq was never needed.
Unloading the module caused an oops when I did not request the TX SDMA
IRQ. Have to check that again.
> Does your use case differ?
I'm currently using your modified lpc test driver and I am currently
in the process of porting a 2.4.25 MOST150 FPGA driver to 2.6 using
the platform LPC driver. As soon as I have results using the FPGA
driver (very high load) I will come back with the results or probably
further patches.
One of them might be increasing the number of BDs again, because the
BestComm seems to take some time until it gives them back to the
world, and then sometimes in batches. I had a similar problem on a
high speed SPI link using the BestComm.
Roman
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 06/13] powerpc/5200: LocalPlus driver: map and unmap DMA areas
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (4 preceding siblings ...)
2009-12-22 7:02 ` [PATCH 05/13] powerpc/5200: LocalPlus driver: fix DMA TX interrupt request Roman Fietze
@ 2009-12-22 7:04 ` Roman Fietze
2010-01-11 19:57 ` Grant Likely
2009-12-22 7:05 ` [PATCH 07/13] powerpc/5200: LocalPlus driver: reset BestComm when committing new request Roman Fietze
` (6 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:04 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/include/asm/mpc52xx.h | 2 +-
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 26 +++++++++++++++++++--=
=2D--
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index c659d1d..043458e 100644
=2D-- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -347,7 +347,7 @@ struct mpc52xx_lpbfifo_request {
=20
/* Memory address */
void *data;
=2D phys_addr_t data_phys;
+ dma_addr_t data_dma;
=20
/* Details of transfer */
size_t size;
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 1e4f725..8d8a63a 100644
=2D-- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -14,6 +14,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/spinlock.h>
+#include <linux/dma-mapping.h>
#include <asm/io.h>
#include <asm/prom.h>
#include <asm/mpc52xx.h>
@@ -138,6 +139,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
out_be32(&lpbfifo.regs->fifo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 28);
out_be32(&lpbfifo.regs->fifo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(7));
lpbfifo.bcom_cur_task =3D lpbfifo.bcom_tx_task;
+ req->data_dma =3D dma_map_single(lpbfifo.dev, req->data, req->size, DMA=
_TO_DEVICE);
} else {
out_be32(&lpbfifo.regs->fifo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 1);
out_be32(&lpbfifo.regs->fifo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(0));
@@ -154,6 +156,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
lpbfifo.dma_irqs_enabled =3D 1;
}
}
+ req->data_dma =3D dma_map_single(lpbfifo.dev, req->data, req->size, DMA=
_FROM_DEVICE);
}
=20
/* error irq & master enabled bit */
@@ -161,7 +164,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
=20
bd =3D bcom_prepare_next_buffer(lpbfifo.bcom_cur_task);
bd->status =3D transfer_size;
=2D bd->data[0] =3D req->data_phys + req->pos;
+ bd->data[0] =3D req->data_dma + req->pos;
bcom_submit_next_buffer(lpbfifo.bcom_cur_task, NULL);
}
=20
@@ -236,12 +239,13 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq,=
void *dev_id)
}
=20
rflags =3D req->flags;
+ status_count =3D in_be32(&lpbfifo.regs->bytes_done_status.bytes_done);
=20
=2D /* check normal termination bit */
+ /* Check normal termination bit */
if (!(status_count & MPC52xx_SCLPC_STATUS_NT))
goto out;
=20
=2D /* check abort bit */
+ /* Check abort bit */
if (status_count & MPC52xx_SCLPC_STATUS_AT) {
out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_=
ENABLE_RF);
do_callback =3D 1;
@@ -250,7 +254,7 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq, v=
oid *dev_id)
=20
if (!mpc52xx_lpbfifo_is_dma(rflags)) {
=20
=2D /* bytes done */
+ /* Bytes done */
status_count &=3D MPC52xx_SCLPC_STATUS_BYTES_DONE_MASK;
=20
if (!mpc52xx_lpbfifo_is_write(rflags)) {
@@ -336,6 +340,16 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, v=
oid *dev_id)
bcom_retrieve_buffer(lpbfifo->bcom_cur_task, NULL, NULL);
// req->irq_ticks +=3D get_tbl() - ts;
=20
+ if (lpbfifo->req) {
+ if (mpc52xx_lpbfifo_is_write(lpbfifo->req->flags))
+ dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->si=
ze, DMA_TO_DEVICE);
+ else
+ dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->si=
ze, DMA_FROM_DEVICE);
+ } else
+ {
+ dev_err(lpbfifo->dev, "request is NULL\n");
+ }
+
spin_unlock_irqrestore(&lpbfifo->lock, flags);
=20
return IRQ_HANDLED;
@@ -439,7 +453,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
goto err_irq;
=20
/* Request the Bestcomm receive (fifo --> memory) task and IRQ */
=2D lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(16,
+ lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(4,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
16*1024*1024);
@@ -453,7 +467,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
goto err_bcom_rx_irq;
=20
/* Request the Bestcomm transmit (memory --> fifo) task and IRQ */
=2D lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(16,
+ lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(4,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC,
BCOM_IPR_SCLPC);
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 06/13] powerpc/5200: LocalPlus driver: map and unmap DMA areas
2009-12-22 7:04 ` [PATCH 06/13] powerpc/5200: LocalPlus driver: map and unmap DMA areas Roman Fietze
@ 2010-01-11 19:57 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 19:57 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 07/13] powerpc/5200: LocalPlus driver: reset BestComm when committing new request
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (5 preceding siblings ...)
2009-12-22 7:04 ` [PATCH 06/13] powerpc/5200: LocalPlus driver: map and unmap DMA areas Roman Fietze
@ 2009-12-22 7:05 ` Roman Fietze
2010-01-11 20:00 ` Grant Likely
2009-12-22 7:06 ` [PATCH 08/13] powerpc/5200: LocalPlus driver: smart flush of receive FIFO Roman Fietze
` (5 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:05 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 8d8a63a..a7cd585 100644
=2D-- 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_reque=
st *req)
req->buffer_not_done_cnt =3D 0;
req->pos =3D 0;
=20
+ bcom_gen_bd_rx_reset(lpbfifo.bcom_rx_task);
+ bcom_gen_bd_tx_reset(lpbfifo.bcom_tx_task);
mpc52xx_lpbfifo_kick(req);
spin_unlock_irqrestore(&lpbfifo.lock, flags);
=20
@@ -456,7 +458,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(4,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
=2D 16*1024*1024);
+ 16 * 1024 * 1024);
if (!lpbfifo.bcom_rx_task)
goto err_bcom_rx;
=20
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 07/13] powerpc/5200: LocalPlus driver: reset BestComm when committing new request
2009-12-22 7:05 ` [PATCH 07/13] powerpc/5200: LocalPlus driver: reset BestComm when committing new request Roman Fietze
@ 2010-01-11 20:00 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 20:00 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 08/13] powerpc/5200: LocalPlus driver: smart flush of receive FIFO
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (6 preceding siblings ...)
2009-12-22 7:05 ` [PATCH 07/13] powerpc/5200: LocalPlus driver: reset BestComm when committing new request Roman Fietze
@ 2009-12-22 7:06 ` Roman Fietze
2010-01-11 20:06 ` Grant Likely
2009-12-22 7:08 ` [PATCH 09/13] powerpc/5200: LocalPlus driver: smarter calculation of BPT, bytes per transfer Roman Fietze
` (4 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:06 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 40 ++++++++++++++++-----=
=2D---
1 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index a7cd585..48f2b4f 100644
=2D-- 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_r=
equest *req)
struct bcom_bd *bd;
void __iomem *reg;
u32 *data;
=2D int i;
=2D int bit_fields;
+ u32 bit_fields;
int rflags =3D req->flags;
=20
/* Set and clear the reset bits; is good practice in User Manual */
@@ -96,27 +95,32 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
=20
/* Set CS and BPT */
bit_fields =3D MPC52xx_SCLPC_CONTROL_CS(req->cs) | 0x8;
=2D if (!(mpc52xx_lpbfifo_is_write(rflags))) {
+ if (!(mpc52xx_lpbfifo_is_write(rflags)))
bit_fields |=3D MPC52xx_SCLPC_CONTROL_RWB_RECEIVE; /* read mode */
=2D bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
=2D }
=2D out_be32(&lpbfifo.regs->control, bit_fields);
=20
if (!mpc52xx_lpbfifo_is_dma(rflags)) {
=2D /* While the FIFO can be setup for transfer sizes as large as
=2D * 16M-1, the FIFO itself is only 512 bytes deep and it does
=2D * not generate interrupts for FIFO full events (only transfer
=2D * complete will raise an IRQ). Therefore when not using
=2D * Bestcomm to drive the FIFO it needs to either be polled, or
=2D * transfers need to constrained to the size of the fifo.
+ /* While the FIFO can be setup for transfer sizes as
+ * large as 16M-1, the FIFO itself is only 512 bytes
+ * deep and it does not generate interrupts for FIFO
+ * full events (only transfer complete will raise an
+ * IRQ). Therefore when not using Bestcomm to drive the
+ * FIFO it needs to either be polled, or transfers need
+ * to constrained to the size of the fifo.
*
* This driver restricts the size of the transfer
+ *
+ * The last block of data will be received with the
+ * flush bit set. This avoids stale read data.
*/
if (transfer_size > 512)
transfer_size =3D 512;
+ else if (!(mpc52xx_lpbfifo_is_write(rflags)))
+ bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
=20
/* Load the FIFO with data */
if (mpc52xx_lpbfifo_is_write(rflags)) {
+ size_t i;
+
reg =3D &lpbfifo.regs->fifo_data;
data =3D req->data + req->pos;
for (i =3D 0; i < transfer_size; i +=3D 4)
@@ -128,6 +132,12 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfif=
o_request *req)
MPC52xx_SCLPC_ENABLE_NIE |
MPC52xx_SCLPC_ENABLE_ME));
} else {
+
+ /* In DMA mode we can always set the flush bit to avoid
+ * stale read data. */
+ if (!(mpc52xx_lpbfifo_is_write(rflags)))
+ bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
+
/* Choose the correct direction
*
* Configure the watermarks so DMA will always complete correctly.
@@ -168,6 +178,8 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
bcom_submit_next_buffer(lpbfifo.bcom_cur_task, NULL);
}
=20
+ out_be32(&lpbfifo.regs->control, bit_fields);
+
/* Set packet size and kick it off */
out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_SCLPC_PACKET_SIZ=
E_RESTART | transfer_size);
if (mpc52xx_lpbfifo_is_dma(rflags))
@@ -455,7 +467,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
goto err_irq;
=20
/* Request the Bestcomm receive (fifo --> memory) task and IRQ */
=2D lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(4,
+ lpbfifo.bcom_rx_task =3D bcom_gen_bd_rx_init(2,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
16 * 1024 * 1024);
@@ -469,7 +481,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
goto err_bcom_rx_irq;
=20
/* Request the Bestcomm transmit (memory --> fifo) task and IRQ */
=2D lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(4,
+ lpbfifo.bcom_tx_task =3D bcom_gen_bd_tx_init(2,
res.start + offsetof(struct mpc52xx_sclpc, fifo_data),
BCOM_INITIATOR_SCLPC,
BCOM_IPR_SCLPC);
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 08/13] powerpc/5200: LocalPlus driver: smart flush of receive FIFO
2009-12-22 7:06 ` [PATCH 08/13] powerpc/5200: LocalPlus driver: smart flush of receive FIFO Roman Fietze
@ 2010-01-11 20:06 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 20:06 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 09/13] powerpc/5200: LocalPlus driver: smarter calculation of BPT, bytes per transfer
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (7 preceding siblings ...)
2009-12-22 7:06 ` [PATCH 08/13] powerpc/5200: LocalPlus driver: smart flush of receive FIFO Roman Fietze
@ 2009-12-22 7:08 ` Roman Fietze
2010-01-11 20:15 ` Grant Likely
2009-12-22 7:09 ` [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order Roman Fietze
` (3 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:08 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 33 +++++++++++++++------=
=2D---
1 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 48f2b4f..21b2a40 100644
=2D-- 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 flags)
*/
static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_request *req)
{
=2D size_t transfer_size =3D req->size - req->pos;
+ size_t tc =3D req->size - req->pos;
struct bcom_bd *bd;
void __iomem *reg;
u32 *data;
=2D u32 bit_fields;
+ u32 control;
int rflags =3D req->flags;
=20
/* Set and clear the reset bits; is good practice in User Manual */
@@ -93,10 +93,10 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
/* Set width, chip select and READ mode */
out_be32(&lpbfifo.regs->start_address, req->offset + req->pos);
=20
=2D /* Set CS and BPT */
=2D bit_fields =3D MPC52xx_SCLPC_CONTROL_CS(req->cs) | 0x8;
+ /* Setup CS */
+ control =3D MPC52xx_SCLPC_CONTROL_CS(req->cs);
if (!(mpc52xx_lpbfifo_is_write(rflags)))
=2D bit_fields |=3D MPC52xx_SCLPC_CONTROL_RWB_RECEIVE; /* read mode */
+ control |=3D MPC52xx_SCLPC_CONTROL_RWB_RECEIVE; /* read mode */
=20
if (!mpc52xx_lpbfifo_is_dma(rflags)) {
/* While the FIFO can be setup for transfer sizes as
@@ -112,10 +112,10 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
* The last block of data will be received with the
* flush bit set. This avoids stale read data.
*/
=2D if (transfer_size > 512)
=2D transfer_size =3D 512;
+ if (tc > 512)
+ tc =3D 512;
else if (!(mpc52xx_lpbfifo_is_write(rflags)))
=2D bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
+ control |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
=20
/* Load the FIFO with data */
if (mpc52xx_lpbfifo_is_write(rflags)) {
@@ -123,7 +123,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
=20
reg =3D &lpbfifo.regs->fifo_data;
data =3D req->data + req->pos;
=2D for (i =3D 0; i < transfer_size; i +=3D 4)
+ for (i =3D 0; i < tc; i +=3D 4)
out_be32(reg, *data++);
}
=20
@@ -136,7 +136,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
/* In DMA mode we can always set the flush bit to avoid
* stale read data. */
if (!(mpc52xx_lpbfifo_is_write(rflags)))
=2D bit_fields |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
+ control |=3D MPC52xx_SCLPC_CONTROL_FLUSH;
=20
/* Choose the correct direction
*
@@ -173,15 +173,17 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfi=
fo_request *req)
out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_AIE | MPC52xx_SCLPC=
_ENABLE_NIE | MPC52xx_SCLPC_ENABLE_ME);
=20
bd =3D bcom_prepare_next_buffer(lpbfifo.bcom_cur_task);
=2D bd->status =3D transfer_size;
+ bd->status =3D tc;
bd->data[0] =3D req->data_dma + req->pos;
bcom_submit_next_buffer(lpbfifo.bcom_cur_task, NULL);
}
=20
=2D out_be32(&lpbfifo.regs->control, bit_fields);
+ /* Setup BPT. tc is already screened and a multiple of 4 */
+ control |=3D tc & 7 ? 4 : 8;
+ out_be32(&lpbfifo.regs->control, control);
=20
/* Set packet size and kick it off */
=2D out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_SCLPC_PACKET_S=
IZE_RESTART | transfer_size);
+ out_be32(&lpbfifo.regs->packet_size.packet_size, MPC52xx_SCLPC_PACKET_SIZ=
E_RESTART | tc);
if (mpc52xx_lpbfifo_is_dma(rflags))
bcom_enable(lpbfifo.bcom_cur_task);
}
@@ -395,6 +397,11 @@ int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_requ=
est *req)
if (!lpbfifo.regs)
return -ENODEV;
=20
+ /* The gen bd BestComm task currently only allows an increment
+ * of 4 */
+ if (!req->size || req->size & 0x03)
+ return -EINVAL;
+
spin_lock_irqsave(&lpbfifo.lock, flags);
=20
/* If the req pointer is already set, then a transfer is in progress */
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 09/13] powerpc/5200: LocalPlus driver: smarter calculation of BPT, bytes per transfer
2009-12-22 7:08 ` [PATCH 09/13] powerpc/5200: LocalPlus driver: smarter calculation of BPT, bytes per transfer Roman Fietze
@ 2010-01-11 20:15 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 20:15 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (8 preceding siblings ...)
2009-12-22 7:08 ` [PATCH 09/13] powerpc/5200: LocalPlus driver: smarter calculation of BPT, bytes per transfer Roman Fietze
@ 2009-12-22 7:09 ` Roman Fietze
2010-01-11 20:19 ` Grant Likely
2009-12-22 7:10 ` [PATCH 11/13] powerpc/5200: LocalPlus driver: move RAM DMA address from request to driver Roman Fietze
` (2 subsequent siblings)
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:09 UTC (permalink / raw)
To: linuxppc-dev
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.
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 94 +++++++++++++++++----=
=2D---
1 files changed, 64 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index 21b2a40..cd8dc69 100644
=2D-- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -32,7 +32,7 @@ struct mpc52xx_lpbfifo {
struct device *dev;
phys_addr_t regs_phys;
struct mpc52xx_sclpc __iomem *regs;
=2D int irq;
+ int sclpc_irq;
spinlock_t lock;
=20
struct bcom_task *bcom_tx_task;
@@ -41,6 +41,7 @@ struct mpc52xx_lpbfifo {
=20
/* Current state data */
struct mpc52xx_lpbfifo_request *req;
+ unsigned short irqs_pending;
int dma_irqs_enabled;
};
=20
@@ -48,6 +49,14 @@ struct mpc52xx_lpbfifo {
static struct mpc52xx_lpbfifo lpbfifo;
=20
=20
+/* 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 BIT(0)
+#define MPC52XX_LPBFIFO_PENDING_BCOM BIT(1)
+
+
/**
* mpc52xx_lpbfifo_is_write - return true if it's a WRITE request
*/
@@ -127,6 +136,8 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
out_be32(reg, *data++);
}
=20
+ lpbfifo.irqs_pending =3D MPC52XX_LPBFIFO_PENDING_SCLPC;
+
/* Unmask both error and completion irqs */
out_be32(&lpbfifo.regs->enable, (MPC52xx_SCLPC_ENABLE_AIE |
MPC52xx_SCLPC_ENABLE_NIE |
@@ -172,6 +183,8 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
/* error irq & master enabled bit */
out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_AIE | MPC52xx_SCLPC=
_ENABLE_NIE | MPC52xx_SCLPC_ENABLE_ME);
=20
+ lpbfifo.irqs_pending =3D MPC52XX_LPBFIFO_PENDING_BCOM | MPC52XX_LPBFIFO_=
PENDING_SCLPC;
+
bd =3D bcom_prepare_next_buffer(lpbfifo.bcom_cur_task);
bd->status =3D tc;
bd->data[0] =3D req->data_dma + req->pos;
@@ -188,6 +201,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
bcom_enable(lpbfifo.bcom_cur_task);
}
=20
+
/**
* mpc52xx_lpbfifo_sclpc_irq - IRQ handler for LPB FIFO
*
@@ -232,8 +246,9 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
*/
static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq, void *dev_id)
{
+ struct mpc52xx_lpbfifo *lpbfifo =3D dev_id;
struct mpc52xx_lpbfifo_request *req;
=2D u32 status_count =3D in_be32(&lpbfifo.regs->bytes_done_status.bytes_don=
e);
+ u32 status_count =3D in_be32(&lpbfifo->regs->bytes_done_status.bytes_done=
);
void __iomem *reg;
u32 *data;
size_t i;
@@ -242,18 +257,20 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq,=
void *dev_id)
unsigned long flags;
int rflags;
=20
=2D spin_lock_irqsave(&lpbfifo.lock, flags);
+ spin_lock_irqsave(&lpbfifo->lock, flags);
ts =3D get_tbl();
=20
=2D req =3D lpbfifo.req;
+ req =3D lpbfifo->req;
if (!req) {
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
pr_err("bogus SCLPC IRQ\n");
return IRQ_HANDLED;
}
=20
+ lpbfifo->irqs_pending &=3D ~MPC52XX_LPBFIFO_PENDING_SCLPC;
+
rflags =3D req->flags;
=2D status_count =3D in_be32(&lpbfifo.regs->bytes_done_status.bytes_done);
+ status_count =3D in_be32(&lpbfifo->regs->bytes_done_status.bytes_done);
=20
/* Check normal termination bit */
if (!(status_count & MPC52xx_SCLPC_STATUS_NT))
@@ -261,19 +278,23 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq,=
void *dev_id)
=20
/* Check abort bit */
if (status_count & MPC52xx_SCLPC_STATUS_AT) {
=2D out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLP=
C_ENABLE_RF);
+ out_be32(&lpbfifo->regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC=
_ENABLE_RF);
do_callback =3D 1;
goto out;
}
=20
=2D if (!mpc52xx_lpbfifo_is_dma(rflags)) {
+ if (mpc52xx_lpbfifo_is_dma(rflags)) {
+ if (!lpbfifo->irqs_pending)
+ do_callback =3D 1;
+ }
+ else {
=20
/* Bytes done */
status_count &=3D MPC52xx_SCLPC_STATUS_BYTES_DONE_MASK;
=20
if (!mpc52xx_lpbfifo_is_write(rflags)) {
/* copy the data out of the FIFO */
=2D reg =3D &lpbfifo.regs->fifo_data;
+ reg =3D &lpbfifo->regs->fifo_data;
data =3D req->data + req->pos;
for (i =3D 0; i < status_count; i +=3D sizeof(u32))
*data++ =3D in_be32(reg);
@@ -288,13 +309,10 @@ static irqreturn_t mpc52xx_lpbfifo_sclpc_irq(int irq,=
void *dev_id)
else
do_callback =3D 1;
}
=2D else {
=2D do_callback =3D 1;
=2D }
=20
out:
/* Clear the IRQ */
=2D out_8(&lpbfifo.regs->bytes_done_status.status, BIT(0));
+ out_8(&lpbfifo->regs->bytes_done_status.status, BIT(0));
=20
req->last_byte =3D ((u8 *)req->data)[req->size - 1];
=20
@@ -304,11 +322,11 @@ out:
/* When the do_callback flag is set; it means the transfer is finished
* so set the FIFO as idle */
if (do_callback) {
=2D lpbfifo.req =3D NULL;
=2D out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLP=
C_ENABLE_RF);
+ lpbfifo->req =3D NULL;
+ out_be32(&lpbfifo->regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC=
_ENABLE_RF);
=20
req->irq_ticks +=3D get_tbl() - ts;
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
=20
/* Spinlock is released; it is now safe to call the callback */
if (req->callback)
@@ -318,7 +336,7 @@ out:
}
else {
req->irq_ticks +=3D get_tbl() - ts;
=2D spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
=20
return IRQ_HANDLED;
}
@@ -354,14 +372,30 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, =
void *dev_id)
bcom_retrieve_buffer(lpbfifo->bcom_cur_task, NULL, NULL);
// req->irq_ticks +=3D get_tbl() - ts;
=20
=2D if (lpbfifo->req) {
=2D if (mpc52xx_lpbfifo_is_write(lpbfifo->req->flags))
=2D dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->=
size, DMA_TO_DEVICE);
=2D else
=2D dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->=
size, DMA_FROM_DEVICE);
=2D } else
=2D {
=2D dev_err(lpbfifo->dev, "request is NULL\n");
+ lpbfifo->irqs_pending &=3D ~MPC52XX_LPBFIFO_PENDING_BCOM;
+ if (!lpbfifo->irqs_pending) {
+ struct mpc52xx_lpbfifo_request *req =3D lpbfifo->req;
+
+ if (req) {
+ if (mpc52xx_lpbfifo_is_write(lpbfifo->req->flags))
+ dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->s=
ize, DMA_TO_DEVICE);
+ else
+ dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req->s=
ize, DMA_FROM_DEVICE);
+
+ lpbfifo->req =3D NULL;
+ out_be32(&lpbfifo->regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLP=
C_ENABLE_RF);
+
+ spin_unlock_irqrestore(&lpbfifo->lock, flags);
+
+ /* Spinlock is released; it is now safe to call the callback */
+ if (req->callback)
+ req->callback(req);
+
+ return IRQ_HANDLED;
+ }
+ else {
+ dev_err(lpbfifo->dev, "bogus BCOM IRQ\n");
+ }
}
=20
spin_unlock_irqrestore(&lpbfifo->lock, flags);
@@ -451,8 +485,8 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
if (lpbfifo.dev !=3D NULL)
return -ENOSPC;
=20
=2D lpbfifo.irq =3D irq_of_parse_and_map(op->node, 0);
=2D if (!lpbfifo.irq)
+ lpbfifo.sclpc_irq =3D irq_of_parse_and_map(op->node, 0);
+ if (!lpbfifo.sclpc_irq)
return -ENODEV;
=20
if (of_address_to_resource(op->node, 0, &res))
@@ -468,7 +502,7 @@ mpc52xx_lpbfifo_probe(struct of_device *op, const struc=
t of_device_id *match)
out_be32(&lpbfifo.regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLPC_E=
NABLE_RF);
=20
/* register the interrupt handler */
=2D rc =3D request_irq(lpbfifo.irq, mpc52xx_lpbfifo_sclpc_irq, 0,
+ rc =3D request_irq(lpbfifo.sclpc_irq, mpc52xx_lpbfifo_sclpc_irq, 0,
"mpc52xx-lpbfifo", &lpbfifo);
if (rc)
goto err_irq;
@@ -539,7 +573,7 @@ static int __devexit mpc52xx_lpbfifo_remove(struct of_d=
evice *op)
free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
=20
=2D free_irq(lpbfifo.irq, &lpbfifo);
+ free_irq(lpbfifo.sclpc_irq, &lpbfifo);
iounmap(lpbfifo.regs);
lpbfifo.regs =3D NULL;
lpbfifo.dev =3D NULL;
@@ -547,7 +581,7 @@ static int __devexit mpc52xx_lpbfifo_remove(struct of_d=
evice *op)
return 0;
}
=20
=2Dstatic struct of_device_id mpc52xx_lpbfifo_match[] __devinitconst =3D {
+static const struct of_device_id mpc52xx_lpbfifo_match[] __devinitconst =
=3D {
{ .compatible =3D "fsl,mpc5200-lpbfifo", },
{},
};
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order
2009-12-22 7:09 ` [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order Roman Fietze
@ 2010-01-11 20:19 ` Grant Likely
2010-01-12 7:43 ` Roman Fietze
0 siblings, 1 reply; 46+ messages in thread
From: Grant Likely @ 2010-01-11 20:19 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread* Re: [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order
2010-01-11 20:19 ` Grant Likely
@ 2010-01-12 7:43 ` Roman Fietze
0 siblings, 0 replies; 46+ messages in thread
From: Roman Fietze @ 2010-01-12 7:43 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
[-- Attachment #1: Type: Text/Plain, Size: 1021 bytes --]
Hello Grant,
On Monday 11 January 2010 21:19:14 Grant Likely wrote:
> I'm really not convinced.
At least you made me think about that some more. And of course, you
are right, the order must be fixed, one way using TX, the other way
using RX.
I think delayed BCOM IRQs in TX direction, caused by a high FEC or ATA
BCOM load, could cause starting the next job with a not yet cleand up
BD ring.
Please give me another change to come up with a hopefully much cleaner
design, esp. after some tests under low and high load with alternating
transfer directions. Probably we can stay with the original design,
and only take some extra effort when the transfer directions changes.
Roman
--
Roman Fietze Telemotive AG Büro Mühlhausen
Breitwiesen 73347 Mühlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
Amtsgericht Ulm HRB 541321
Vorstand:
Peter Kersten, Markus Fischer, Franz Diller, Markus Stolz
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH 11/13] powerpc/5200: LocalPlus driver: move RAM DMA address from request to driver
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (9 preceding siblings ...)
2009-12-22 7:09 ` [PATCH 10/13] powerpc/5200: LocalPlus driver: fix problem caused by unpredictable IRQ order Roman Fietze
@ 2009-12-22 7:10 ` Roman Fietze
2010-01-11 20:20 ` Grant Likely
2009-12-22 7:12 ` [PATCH 12/13] mpc52xx: add mpc5200-localplus-test LocalPlus test driver Roman Fietze
2009-12-22 7:13 ` [PATCH 13/13] powerpc/5200: LocalPlus driver: clean up comments Roman Fietze
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:10 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/include/asm/mpc52xx.h | 1 -
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 13 +++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index 043458e..91c65d0 100644
=2D-- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -347,7 +347,6 @@ struct mpc52xx_lpbfifo_request {
=20
/* Memory address */
void *data;
=2D dma_addr_t data_dma;
=20
/* Details of transfer */
size_t size;
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index cd8dc69..b2c92f5 100644
=2D-- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -41,6 +41,7 @@ struct mpc52xx_lpbfifo {
=20
/* Current state data */
struct mpc52xx_lpbfifo_request *req;
+ dma_addr_t data_dma;
unsigned short irqs_pending;
int dma_irqs_enabled;
};
@@ -49,7 +50,7 @@ struct mpc52xx_lpbfifo {
static struct mpc52xx_lpbfifo lpbfifo;
=20
=20
=2D/* 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
* 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. */
@@ -160,7 +161,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
out_be32(&lpbfifo.regs->fifo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 28);
out_be32(&lpbfifo.regs->fifo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(7));
lpbfifo.bcom_cur_task =3D lpbfifo.bcom_tx_task;
=2D req->data_dma =3D dma_map_single(lpbfifo.dev, req->data, req->size, D=
MA_TO_DEVICE);
+ lpbfifo.data_dma =3D dma_map_single(lpbfifo.dev, req->data, req->size, =
DMA_TO_DEVICE);
} else {
out_be32(&lpbfifo.regs->fifo_alarm, MPC52xx_SCLPC_FIFO_SIZE - 1);
out_be32(&lpbfifo.regs->fifo_control, MPC52xx_SLPC_FIFO_CONTROL_GR(0));
@@ -177,7 +178,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
lpbfifo.dma_irqs_enabled =3D 1;
}
}
=2D req->data_dma =3D dma_map_single(lpbfifo.dev, req->data, req->size, D=
MA_FROM_DEVICE);
+ lpbfifo.data_dma =3D dma_map_single(lpbfifo.dev, req->data, req->size, =
DMA_FROM_DEVICE);
}
=20
/* error irq & master enabled bit */
@@ -187,7 +188,7 @@ static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo=
_request *req)
=20
bd =3D bcom_prepare_next_buffer(lpbfifo.bcom_cur_task);
bd->status =3D tc;
=2D bd->data[0] =3D req->data_dma + req->pos;
+ bd->data[0] =3D lpbfifo.data_dma + req->pos;
bcom_submit_next_buffer(lpbfifo.bcom_cur_task, NULL);
}
=20
@@ -378,9 +379,9 @@ static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, vo=
id *dev_id)
=20
if (req) {
if (mpc52xx_lpbfifo_is_write(lpbfifo->req->flags))
=2D dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req-=
>size, DMA_TO_DEVICE);
+ dma_unmap_single(lpbfifo->dev, lpbfifo->data_dma, lpbfifo->req->size, =
DMA_TO_DEVICE);
else
=2D dma_unmap_single(lpbfifo->dev, lpbfifo->req->data_dma, lpbfifo->req-=
>size, DMA_FROM_DEVICE);
+ dma_unmap_single(lpbfifo->dev, lpbfifo->data_dma, lpbfifo->req->size, =
DMA_FROM_DEVICE);
=20
lpbfifo->req =3D NULL;
out_be32(&lpbfifo->regs->enable, MPC52xx_SCLPC_ENABLE_RC | MPC52xx_SCLP=
C_ENABLE_RF);
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 11/13] powerpc/5200: LocalPlus driver: move RAM DMA address from request to driver
2009-12-22 7:10 ` [PATCH 11/13] powerpc/5200: LocalPlus driver: move RAM DMA address from request to driver Roman Fietze
@ 2010-01-11 20:20 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 20:20 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread
* [PATCH 12/13] mpc52xx: add mpc5200-localplus-test LocalPlus test driver
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (10 preceding siblings ...)
2009-12-22 7:10 ` [PATCH 11/13] powerpc/5200: LocalPlus driver: move RAM DMA address from request to driver Roman Fietze
@ 2009-12-22 7:12 ` Roman Fietze
2009-12-22 7:13 ` [PATCH 13/13] powerpc/5200: LocalPlus driver: clean up comments Roman Fietze
12 siblings, 0 replies; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:12 UTC (permalink / raw)
To: linuxppc-dev
The original version was done by Grant Likely. This is a modified
version using the platform LocalPlus driver.
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
drivers/misc/mpc5200-localplus-test.c | 879 +++++++++++++++++++++++++++++=
++++
1 files changed, 879 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/mpc5200-localplus-test.c
diff --git a/drivers/misc/mpc5200-localplus-test.c b/drivers/misc/mpc5200-l=
ocalplus-test.c
new file mode 100644
index 0000000..dcfb282
=2D-- /dev/null
+++ b/drivers/misc/mpc5200-localplus-test.c
@@ -0,0 +1,879 @@
+/*
+ * LocalPlusBus performance tests.
+ *
+ * Copyright (C) Secret Lab Technologies Ltd. 2008-2009
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ *
+ * This file implements a set of LocalPlus bus performance tests when using
+ * direct Programmed IO (PIO), the LocalPlus FIFO, and when using the
+ * Bestcomm DMA engine to transfer data. It can be compiled into the
+ * kernel or loaded as a module.
+ *
+ * The test module is controlled via files in the sysfs filesystem.
+ * Special control files are created in
+ * /sys/devices/platform/lpbtest.0 which control the tests and report
+ * the results. Test parameters are set by writing values into the
+ * parameter files (blocksize, blockcount, bpt, period, and type).
+ * The test is started and stopped with the 'action' file. Results
+ * are retrieved by reading the contents of the 'results' file.
+ *
+ * The following parameters can be modified:
+ * blocksize: number of bytes to transfer in each block.
+ * blockcount: number of blocks to transfer per timer tick.
+ * period: period of timer or distance (leading '+') in jiffies
+ * ("[1-9]{1,}j") or microseconds. Every timer tick will
+ * start a new transfer of data blocks. If this value is 0,
+ * new transfers will be started as quickly as possible using
+ * a tasklet.
+ * type: type of test; may be 'ram', 'fifo' or 'bcom'.
+ * chipselect: chipselect to use for transfer
+ *
+ * The first test copies contents of an LPB address range using a memcpy.
+ * Usage:
+ * $ echo ram > /sys/devices/platform/lpbtest.0/type
+ * $ echo start > /sys/devices/platform/lpbtest.0/action
+ * $ sleep 5s
+ * $ echo stop > /sys/devices/platform/lpbtest.0/action
+ *
+ * The second test copies contents of an LPB range to RAM using the
+ * LocalPlus FIFO. The FIFO ISR copies each packet from the FIFO to RAM.
+ * Usage:
+ * $ echo fifo > /sys/devices/platform/lpbtest.0/type
+ * $ echo start > /sys/devices/platform/lpbtest.0/action
+ * $ sleep 5s
+ * $ echo stop > /sys/devices/platform/lpbtest.0/action
+ *
+ * The third test copies contents of an LPB range to RAM using both the FI=
=46O
+ * and the Bestcomm DMA engine.
+ *
+ * Usage:
+ * $ echo bcom > /sys/devices/platform/lpbtest.0/type
+ * $ echo start > /sys/devices/platform/lpbtest.0/action
+ * $ sleep 5s
+ * $ echo stop > /sys/devices/platform/lpbtest.0/action
+ *
+ * All sysfs entries can be read by using cat <parameter>
+ * e.g. cat /sys/devices/platform/lpbtest.0/type will show the test type
+ *
+ * The following is a useful command to dump out all the state of the modu=
le:
+ * $ grep '' *
+ *
+ */
+
+// #define DEBUG
+
+#include <linux/ctype.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/string.h>
+#include <linux/mempool.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <sysdev/bestcomm/bestcomm.h>
+#include <sysdev/bestcomm/gen_bd.h>
+#include <sysdev/bestcomm/bestcomm_priv.h>
+#include <asm/page.h>
+#include <asm/time.h>
+#include <asm/cacheflush.h>
+#include <asm/div64.h>
+
+MODULE_AUTHOR("Steven Cavanagh <scavan...@secretlab.ca>");
+MODULE_LICENSE("GPL");
+
+
+#define DRVNAME "lpbtest"
+
+#define LPBTEST_FLASH_BASE_ADDR (0xff000000)
+#define LPBTEST_FLASH_SIZE (0x01000000) /* 16 MiB */
+
+#define LPBTEST_BLOCK_SIZE_MIN 4
+#define LPBTEST_BLOCK_SIZE_MAX (0x20000) /* 128 KiB */
+
+#define LPBTEST_JIFFIES_MIN 0
+
+/* Address of SCLPC relative to MBAR
+ *
+ * TODO: move to OF tree?
+ */
+#define MPC52xx_SCLPC_OFFSET 0x3C00
+
+
+/**
+ * lpbtest - Private driver data
+ * @lpb_regs_base: pointer to the LPB's registers
+ * @irq: IRQ of this LPB FIFO
+ * @dev: struct device pointer
+ */
+struct lpbtest {
+ struct device *dev;
+
+ /* sysfs attributes */
+ int action;
+ size_t type;
+ int write; /* true for write test */
+ size_t blockcount;
+ size_t blocksize;
+ unsigned long period; /* in jiffies */
+ int period_us; /* true: period is in us, false: in jiffies */
+ int period_delta; /* true: period is time between the
+ * transactions, false: tme between
+ * start of two transactions */
+ unsigned int chipselect;
+ int verify;
+
+ spinlock_t lock;
+
+ void *ram_virt;
+
+ void *dev_virt;
+ phys_addr_t dev_phys;
+
+ struct mpc52xx_lpbfifo_request req;
+
+ /* Timeslice timer */
+ struct timer_list timer;
+ unsigned long tnext; /* next deadline; in jiffies */
+
+ /* Statistics */
+ unsigned long irq_time;
+ unsigned long timer_time;
+ unsigned long bcom_time;
+ unsigned long start_time;
+ unsigned long stop_time;
+ unsigned long long data_read;
+ int overrun_count;
+
+ /* state variables */
+ size_t next_block; /* Number of next block to send. If
+ * this is >=3D blockcount, then all the
+ * transfers are finished */
+
+ size_t rcvd_blocks; /* Number of blocks received. */
+};
+
+/* Helper functions to test selected behaviour */
+static inline int lpbtest_isfifo(struct lpbtest *priv)
+{
+ return priv->type =3D=3D 1;
+}
+
+
+static void lpbtest_stop(struct lpbtest *priv)
+{
+ priv->stop_time =3D get_tbl();
+ dev_dbg(priv->dev, "%s() stop_time=3D%lu\n", __FUNCTION__, priv->stop_tim=
e);
+}
+
+
+static inline unsigned long lpbtest_period_in_jiffies(struct lpbtest *priv)
+{
+ return priv->period_us ? usecs_to_jiffies(priv->period) : priv->period;
+}
+
+
+/* Start timer. In case finished is false this routine is called at
+ * the start of a transfer, if finished is true at the end of a
+ * transfer.
+ *
+ * In the first case we only start the timer if a period >
+ * LPBTEST_PERIOD_ISO_LIMIT is set, in the second case we only start
+ * the timer if a period <=3D LPBTEST_PERIOD_ISO_LIMIT is set.
+ */
+static void lpbtest_start_timer(struct lpbtest *priv, int finished)
+{
+ int ret;
+ if (finished) {
+ if (priv->period_delta) {
+ priv->tnext =3D lpbtest_period_in_jiffies(priv);
+#if LPBTEST_JIFFIES_MIN
+ if (priv->tnext < LPBTEST_JIFFIES_MIN)
+ priv->tnext =3D LPBTEST_JIFFIES_MIN;
+#endif
+ priv->tnext +=3D jiffies;
+ ret =3D mod_timer(&priv->timer, priv->tnext);
+ dev_dbg(priv->dev, "%s(%d): j=3D%lu tnext=3D%lu r=3D%d\n", __FUNCTION__=
, finished, jiffies, priv->tnext, ret);
+ return;
+ }
+ }
+ else {
+ if (!priv->period_delta) {
+ priv->tnext +=3D lpbtest_period_in_jiffies(priv);
+ /* dev_dbg(dev, "%s: jiffies=3D%lu time=3D%lu\n", __FUNCTION__, jiffies=
, priv->tnext); */
+ if (time_is_before_jiffies(priv->tnext)) {
+ dev_err(priv->dev, "Timeslice overrun by %i us; aborting\n",
+ jiffies_to_usecs(jiffies - priv->tnext));
+ lpbtest_stop(priv);
+ return;
+ }
+
+ ret =3D mod_timer(&priv->timer, priv->tnext);
+ dev_dbg(priv->dev, "%s(%d): j=3D%lu tnext=3D%lu r=3D%d\n", __FUNCTION__=
, finished, jiffies, priv->tnext, ret);
+ return;
+ }
+ }
+
+ dev_dbg(priv->dev, "%s(%d): j=3D%lu no action\n", __FUNCTION__, finished,=
jiffies);
+}
+
+static inline int lpbtest_check_stop(struct device *dev)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ dev_dbg(dev, "%s()\n", __FUNCTION__);
+
+ if (!priv->action) {
+ lpbtest_stop(priv);
+ return 1;
+ }
+
+ return 0;
+}
+
+
+static void lpbtest_read_channels_to_ram(unsigned long _dev)
+{
+ struct device *dev =3D (struct device *)_dev;
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ int i;
+
+ dev_dbg(dev, "%lu: %s()\n", jiffies, __FUNCTION__);
+
+ if (!lpbtest_check_stop(dev)) {
+ const unsigned long tstart =3D get_tbl();
+
+ lpbtest_start_timer(priv, 0);
+
+ /* Assume, that all channels have data available */
+ for (i =3D 0; i < priv->blockcount; i++) {
+ memcpy(priv->ram_virt, priv->dev_virt, priv->blocksize);
+ priv->data_read +=3D priv->blocksize;
+ }
+
+ priv->timer_time +=3D get_tbl() - tstart;
+
+ lpbtest_start_timer(priv, 1);
+ }
+}
+
+
+static void lpbtest_fifo_done(struct mpc52xx_lpbfifo_request *req)
+{
+ struct lpbtest *priv =3D req->priv;
+
+ // dev_info(priv->dev, "%s()\n", __FUNCTION__);
+
+ if (priv->verify) {
+ if (memcmp(priv->ram_virt, priv->dev_virt, priv->blocksize)) {
+ print_hex_dump(KERN_INFO, "RAM: ", DUMP_PREFIX_OFFSET,
+ 16, sizeof(uint32_t),
+ priv->ram_virt, priv->blocksize, 0);
+ print_hex_dump(KERN_INFO, "DEV: ", DUMP_PREFIX_OFFSET,
+ 16, sizeof(uint32_t),
+ priv->dev_virt, priv->blocksize, 0);
+ }
+ else {
+ /* dev_info(priv->dev, "memcmp OK\n"); */
+ }
+ }
+
+ lpbtest_start_timer(priv, 1);
+}
+
+
+static void lpbtest_do_next_transfer(struct lpbtest *priv)
+{
+ dev_dbg(priv->dev, "%s()\n", __FUNCTION__);
+
+ if (priv->next_block < priv->blockcount) {
+
+ int err;
+ struct mpc52xx_lpbfifo_request *req =3D &priv->req;
+
+ memset(req, 0, sizeof(*req));
+ req->cs =3D priv->chipselect;
+ req->offset =3D priv->dev_phys - LPBTEST_FLASH_BASE_ADDR;
+ req->data =3D priv->ram_virt;
+ req->size =3D priv->blocksize;
+ req->pos =3D 0;
+ req->flags =3D MPC52XX_LPBFIFO_FLAG_NO_INCREMENT;
+ req->flags |=3D priv->write ? MPC52XX_LPBFIFO_FLAG_WRITE : MPC52XX_LPBFI=
=46O_FLAG_READ;
+ if (lpbtest_isfifo(priv))
+ req->flags |=3D MPC52XX_LPBFIFO_FLAG_NO_DMA;
+ req->callback =3D lpbtest_fifo_done;
+ req->priv =3D priv;
+
+ priv->next_block++;
+ dev_dbg(priv->dev, "%s: next_block=3D%zu\n", __FUNCTION__, priv->next_bl=
ock);
+
+ memset(priv->ram_virt, 0x55, priv->blocksize);
+ err =3D mpc52xx_lpbfifo_submit(req);
+ if (err) {
+ dev_err(priv->dev, "cannot submit FIFO request: %d\n", err);
+ lpbtest_stop(priv);
+ }
+ }
+}
+
+static void lpbtest_read_channels(unsigned long _dev)
+{
+ struct device *dev =3D (struct device *)_dev;
+
+ dev_dbg(dev, "%lu: %s()\n", jiffies, __FUNCTION__);
+
+ if (!lpbtest_check_stop(dev)) {
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ const unsigned long tstart =3D get_tbl();
+
+ lpbtest_start_timer(priv, 0);
+
+ /* This line is the FIFO throttle, the faster the next packet
+ * is cleared, the faster the FIFO can be read and filled by
+ * the IRQ. The ISR will stop handling the FIFO, when all the
+ * channels have been read.
+ */
+ priv->next_block =3D priv->rcvd_blocks =3D 0;
+ // dev_dbg(dev, "%s: next_block=3D%zu\n", __FUNCTION__, priv->next_block=
);
+ lpbtest_do_next_transfer(priv);
+
+ priv->timer_time +=3D get_tbl() - tstart;
+ }
+}
+
+
+static const struct lpbtest_type {
+ char *name;
+ void (*test_timer)(unsigned long);
+} lpbtest_type[] =3D {
+ {
+ .name =3D "ram",
+ .test_timer =3D lpbtest_read_channels_to_ram,
+ },
+ {
+ .name =3D "fifo",
+ .test_timer =3D lpbtest_read_channels,
+ },
+ {
+ .name =3D "bcom",
+ .test_timer =3D lpbtest_read_channels,
+ }
+};
+
+/* ---------------------------------------------------------------------
+ * sysfs interfaces
+ * --------------------------------------------------------------------- */
+static ssize_t lpbtest_set_type(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ const char *name;
+ size_t i;
+
+ for (i =3D 0; i < ARRAY_SIZE(lpbtest_type); i++) {
+ name =3D lpbtest_type[i].name;
+
+ if (count < strlen(name))
+ continue;
+
+ if (strncmp(buf, name, strlen(name)) =3D=3D 0) {
+ priv->type =3D i;
+ return count;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static ssize_t lpbtest_show_type(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%s\n", lpbtest_type[priv->type].name);
+}
+
+static ssize_t lpbtest_set_dir(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ if (strncmp(buf, "read", strlen("read")) =3D=3D 0) {
+ priv->write =3D 0;
+ }
+ else if (strncmp(buf, "write", strlen("write")) =3D=3D 0) {
+ priv->write =3D 1;
+ }
+ else {
+ dev_err(dev, "Usage: echo [read,write] > dir\n");
+ return -EINVAL;
+ }
+
+ return count;
+}
+
+static ssize_t lpbtest_show_dir(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%s\n", priv->write ? "write" : "read");
+}
+
+static ssize_t lpbtest_set_action(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ const struct lpbtest_type *type =3D &lpbtest_type[priv->type];
+ int oldaction =3D priv->action;
+
+ if (strncmp(buf, "start", strlen("start")) =3D=3D 0) {
+ priv->action =3D 1;
+ }
+ else if (strncmp(buf, "stop", strlen("stop")) =3D=3D 0) {
+ priv->action =3D 0;
+ }
+ else {
+ dev_err(dev, "Usage: echo [start,stop] > action\n");
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "%s: action=3D%d->%d\n", __FUNCTION__, oldaction, priv->acti=
on);
+
+ if (oldaction !=3D priv->action) {
+ oldaction =3D priv->action;
+
+ if (priv->action) {
+ init_timer(&priv->timer);
+ priv->timer.function =3D type->test_timer;
+ priv->timer.data =3D (unsigned long)dev;
+
+ priv->next_block =3D priv->rcvd_blocks =3D 0;
+ priv->irq_time =3D 0;
+ priv->timer_time =3D 0;
+ priv->bcom_time =3D 0;
+ priv->stop_time =3D priv->start_time =3D get_tbl();
+ priv->data_read =3D 0;
+ priv->overrun_count =3D 0;
+
+ /* Map the device */
+ priv->dev_virt =3D ioremap(priv->dev_phys, LPBTEST_BLOCK_SIZE_MAX);
+ if (!priv->dev_virt) {
+ dev_err(dev, "Error mapping device\n");
+ return -ENOMEM;
+ }
+
+ printk(KERN_INFO "Started %s test, blocksize=3D0x%zx\n", type->name, pr=
iv->blocksize);
+
+ /* Set the expiration time for the timer. */
+ priv->tnext =3D jiffies + 1;
+ mod_timer(&priv->timer, priv->tnext);
+
+ }
+ else {
+ del_timer_sync(&priv->timer);
+ lpbtest_stop(priv);
+
+ printk(KERN_INFO "Stopped %s test, blocksize=3D0x%zx\n", type->name, pr=
iv->blocksize);
+ }
+ }
+
+ return count;
+}
+
+static ssize_t lpbtest_show_action(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ char *action;
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ action =3D (priv->action =3D=3D 1) ? "start" : "stop";
+ return sprintf(buf, "%s\n", action);
+}
+
+/*
+ * Export a blockcount attr
+ */
+static ssize_t lpbtest_set_blockcount(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ unsigned long temp;
+
+ if (strict_strtoul(buf, 10, &temp))
+ return -EINVAL;
+ priv->blockcount =3D temp;
+
+ return count;
+}
+
+static ssize_t lpbtest_show_blockcount(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%zu\n", priv->blockcount);
+}
+
+/*
+ * Export a blocksize attr
+ */
+static ssize_t lpbtest_set_blocksize(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ unsigned long temp;
+
+ if (strict_strtoul(buf, 0, &temp))
+ return -EINVAL;
+
+ if (temp < LPBTEST_BLOCK_SIZE_MIN || temp > LPBTEST_BLOCK_SIZE_MAX)
+ return -EINVAL;
+
+ priv->blocksize =3D temp;
+ return count;
+}
+
+static ssize_t lpbtest_show_blocksize(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%zu\n", priv->blocksize);
+}
+
+static ssize_t lpbtest_set_period(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned long temp;
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ const char *pb =3D buf;
+ char *pe;
+
+ priv->period_delta =3D 0;
+ if (*pb =3D=3D '+') {
+ priv->period_delta =3D 1;
+ pb++;
+ }
+ temp =3D simple_strtoul(pb, &pe, 10);
+ if (buf =3D=3D pe) {
+ dev_err(dev, "period \"%s\" empty?\n", buf);
+ return -EINVAL;
+ }
+
+ if (*pe =3D=3D 'j') {
+ priv->period_us =3D 0;
+ }
+ else if (isspace(*pe) || *pe =3D=3D '\0') {
+ priv->period_us =3D 1;
+ }
+ else {
+ dev_err(dev, "period \"%s\" invalid\n", buf);
+ return -EINVAL;
+ }
+
+ priv->period =3D temp;
+ dev_dbg(dev, "period \"%s\" =3D %s%lu %s\n",
+ buf,
+ priv->period_delta ? "+" : "", temp,
+ priv->period_us ? "us" : "j");
+
+ return count;
+}
+
+static ssize_t lpbtest_show_period(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%s%lu%s\n",
+ priv->period_delta ? "+" : "",
+ priv->period,
+ priv->period_us ? "" : "j");
+}
+
+static ssize_t lpbtest_set_cs(struct device *dev, struct device_attribute =
*attr,
+ const char *buf, size_t count)
+{
+ unsigned long temp;
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ if (strict_strtoul(buf, 0, &temp))
+ return -EINVAL;
+
+ if (temp > 7)
+ return -EINVAL;
+
+ priv->chipselect =3D temp;
+ return count;
+}
+
+static ssize_t lpbtest_show_cs(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%u\n", priv->chipselect);
+}
+
+static ssize_t lpbtest_set_baseaddr(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned long temp;
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ if (strict_strtoul(buf, 0, &temp))
+ return -EINVAL;
+
+ priv->dev_phys =3D temp;
+ return count;
+}
+
+static ssize_t lpbtest_show_baseaddr(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%llx\n", (unsigned long long) priv->dev_phys);
+}
+
+static ssize_t lpbtest_set_verify(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ unsigned long temp;
+
+ if (strict_strtoul(buf, 10, &temp))
+ return -EINVAL;
+
+ priv->verify =3D temp;
+ return count;
+}
+
+static ssize_t lpbtest_show_verify(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ return sprintf(buf, "%d\n", priv->verify);
+}
+
+static ssize_t lpbtest_show_results(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+ int c;
+ unsigned long systime;
+ unsigned long realtime;
+ unsigned int utilization;
+ unsigned long rate;
+ unsigned long netrate;
+ unsigned long long tmp;
+
+ realtime =3D priv->stop_time - priv->start_time;
+ systime =3D priv->timer_time + priv->irq_time + priv->bcom_time;
+
+ if (!realtime) {
+ realtime =3D get_tbl() - priv->start_time;
+ }
+
+ if (realtime) {
+ tmp =3D systime * 10000ULL;
+ do_div(tmp, realtime);
+ utilization =3D tmp;
+
+ tmp =3D priv->data_read;
+ tmp *=3D tb_ticks_per_usec;
+ tmp *=3D 100;
+ do_div(tmp, realtime);
+ rate =3D tmp;
+ }
+ else {
+ utilization =3D 0;
+ rate =3D 0;
+ }
+
+ if (systime) {
+ tmp =3D priv->data_read;
+ tmp *=3D tb_ticks_per_usec;
+ tmp *=3D 100;
+ do_div(tmp, systime);
+ netrate =3D tmp;
+ }
+ else {
+ netrate =3D 0;
+ }
+
+ c =3D sprintf(buf, "real\t\t:\t%10lu ticks\t%9lu us\n",
+ realtime, realtime / tb_ticks_per_usec);
+ c +=3D sprintf(buf + c, "sys\t\t:\t%10lu ticks\t%9lu us\n",
+ systime, systime / tb_ticks_per_usec);
+ c +=3D sprintf(buf + c, "timer\t\t:\t%10lu ticks\t%9lu us\n",
+ priv->timer_time, priv->timer_time / tb_ticks_per_usec);
+ c +=3D sprintf(buf + c, "fifo irq\t:\t%10lu ticks\t%9lu us\n",
+ priv->irq_time, priv->irq_time / tb_ticks_per_usec);
+ c +=3D sprintf(buf + c, "bcom irq\t:\t%10lu ticks\t%9lu us\n",
+ priv->bcom_time, priv->bcom_time / tb_ticks_per_usec);
+ c +=3D sprintf(buf + c, "overruns\t:\t%10u\n", priv->overrun_count);
+ c +=3D sprintf(buf + c, "%%CPU\t\t:\t%10u.%.2u %%\n",
+ utilization / 100, utilization % 100);
+ c +=3D sprintf(buf + c, "byte count\t:\t%10llu\n", priv->data_read);
+ c +=3D sprintf(buf + c, "net data rate\t:\t%10lu.%.2lu MB/s\n",
+ rate / 100, rate % 100);
+ c +=3D sprintf(buf + c, "max data rate\t:\t%10lu.%.2lu MB/s\n",
+ netrate / 100, netrate % 100);
+
+ return c;
+}
+
+static struct device_attribute lpbtest_attrib[] =3D {
+ __ATTR(action, S_IWUSR | S_IRUGO,
+ lpbtest_show_action, lpbtest_set_action),
+ __ATTR(blockcount, S_IWUSR | S_IRUGO,
+ lpbtest_show_blockcount, lpbtest_set_blockcount),
+ __ATTR(blocksize, S_IWUSR | S_IRUGO,
+ lpbtest_show_blocksize, lpbtest_set_blocksize),
+ __ATTR(period, S_IWUSR | S_IRUGO,
+ lpbtest_show_period, lpbtest_set_period),
+ __ATTR(chipselect, S_IWUSR | S_IRUGO,
+ lpbtest_show_cs, lpbtest_set_cs),
+ __ATTR(baseaddr, S_IWUSR | S_IRUGO,
+ lpbtest_show_baseaddr, lpbtest_set_baseaddr),
+ __ATTR(verify, S_IWUSR | S_IRUGO,
+ lpbtest_show_verify, lpbtest_set_verify),
+ __ATTR(type, S_IWUSR | S_IRUGO,
+ lpbtest_show_type, lpbtest_set_type),
+ __ATTR(dir, S_IWUSR | S_IRUGO,
+ lpbtest_show_dir, lpbtest_set_dir),
+ __ATTR(results, S_IWUSR | S_IRUGO,
+ lpbtest_show_results, NULL),
+};
+
+static void lpbtest_cleanup_sysfs(struct platform_device *pdev)
+{
+ struct device *dev =3D &pdev->dev;
+ int i;
+
+ for (i =3D 0; i < ARRAY_SIZE(lpbtest_attrib); i++)
+ device_remove_file(dev, &lpbtest_attrib[i]);
+}
+
+static int lpbtest_setup(struct device *dev)
+{
+ struct lpbtest *priv =3D dev_get_drvdata(dev);
+
+ /* Allocate a destination buffer */
+ priv->ram_virt =3D kzalloc(LPBTEST_BLOCK_SIZE_MAX, GFP_KERNEL);
+ if (!priv->ram_virt) {
+ dev_err(dev, "Error allocating test buffer\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int __devinit lpbtest_probe(struct platform_device *pdev)
+{
+ struct device *dev =3D &pdev->dev;
+ struct lpbtest *priv;
+
+ int ret =3D 0, i;
+
+ /* Allocate and initialize the driver private data */
+ priv =3D kzalloc(sizeof *priv, GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev =3D dev;
+ priv->blockcount =3D 16;
+ priv->blocksize =3D MPC52xx_SCLPC_FIFO_SIZE;
+ priv->period =3D 20000;
+ priv->period_us =3D 1;
+ priv->period_delta =3D 0;
+ priv->dev_phys =3D LPBTEST_FLASH_BASE_ADDR;
+
+ spin_lock_init(&priv->lock);
+ platform_set_drvdata(pdev, priv);
+
+ ret =3D lpbtest_setup(dev);
+ if (ret) {
+ dev_err(dev, "lpbtest_setup() error\n");
+ return ret;
+ }
+
+ /* Register the SYSFS files */
+ for (i =3D 0; i < ARRAY_SIZE(lpbtest_attrib); i++) {
+ ret =3D device_create_file(dev, &lpbtest_attrib[i]);
+ if (ret) {
+ dev_err(dev, "error creating sysfs files (%d)\n", ret);
+ lpbtest_cleanup_sysfs(pdev);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+static int __devexit lpbtest_remove(struct platform_device *pdev)
+{
+ struct lpbtest *priv =3D platform_get_drvdata(pdev);
+
+ lpbtest_cleanup_sysfs(pdev);
+
+ del_timer(&priv->timer);
+
+ kfree(priv->ram_virt);
+ kfree(priv);
+
+ return 0;
+}
+
+static struct platform_driver lpbtest_driver =3D {
+ .driver =3D {
+ .owner =3D THIS_MODULE,
+ .name =3D DRVNAME,
+ },
+ .probe =3D lpbtest_probe,
+ .remove =3D __devexit_p(lpbtest_remove),
+};
+
+static struct platform_device *lpbtest_pdev;
+
+
+static int __init lpbtest_init(void)
+{
+ int rc;
+
+ printk(KERN_INFO "%s init\n", DRVNAME);
+
+ lpbtest_pdev =3D platform_device_register_simple(DRVNAME, 0, NULL, 0);
+ if (!lpbtest_pdev) {
+ pr_err("%s: error registering test device\n", DRVNAME);
+ return -ENOMEM;
+ }
+
+ rc =3D platform_driver_register(&lpbtest_driver);
+ if (rc)
+ platform_device_unregister(lpbtest_pdev);
+ return rc;
+}
+
+static void lpbtest_exit(void)
+{
+ platform_device_unregister(lpbtest_pdev);
+ platform_driver_unregister(&lpbtest_driver);
+}
+
+module_init(lpbtest_init);
+module_exit(lpbtest_exit);
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH 13/13] powerpc/5200: LocalPlus driver: clean up comments
2009-12-22 6:55 ` [PATCH 0/13] MPC5200B LocalPlus Platform Driver Changes Roman Fietze
` (11 preceding siblings ...)
2009-12-22 7:12 ` [PATCH 12/13] mpc52xx: add mpc5200-localplus-test LocalPlus test driver Roman Fietze
@ 2009-12-22 7:13 ` Roman Fietze
2010-01-11 20:24 ` Grant Likely
12 siblings, 1 reply; 46+ messages in thread
From: Roman Fietze @ 2009-12-22 7:13 UTC (permalink / raw)
To: linuxppc-dev
Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
=2D--
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 14 --------------
1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/p=
latforms/52xx/mpc52xx_lpbfifo.c
index b2c92f5..a89072a 100644
=2D-- 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_lpbfif=
o_request *req)
/**
* mpc52xx_lpbfifo_sclpc_irq - IRQ handler for LPB FIFO
*
=2D * On transmit, the dma completion irq triggers before the fifo
=2D * completion triggers. Handle the dma completion here instead of the
=2D * LPB FIFO Bestcomm task completion irq because everything is not
=2D * really done until the LPB FIFO completion irq triggers.
=2D *
=2D * In other words:
=2D * For DMA, on receive, the "Fat Lady" is the bestcom completion irq. on
=2D * transmit, the fifo completion irq is the "Fat Lady". The opera (or in
=2D * this case the DMA/FIFO operation) is not finished until the "Fat
=2D * Lady" sings.
=2D *
* Reasons for entering this routine:
* 1) PIO mode rx and tx completion irq
* 2) DMA interrupt mode tx completion irq
@@ -411,9 +400,6 @@ void mpc52xx_lpbfifo_poll(void)
{
struct mpc52xx_lpbfifo_request *req =3D lpbfifo.req;
=20
=2D /*
=2D * For more information, see comments on the "Fat Lady"=20
=2D */
if (mpc52xx_lpbfifo_is_dma(req->flags) && (req->flags & MPC52XX_LPBFIFO_F=
LAG_WRITE))
mpc52xx_lpbfifo_sclpc_irq(0, NULL);
else=20
=2D-=20
1.6.5.5
=2D-=20
Roman Fietze Telemotive AG B=FCro M=FChlhausen
Breitwiesen 73347 M=FChlhausen
Tel.: +49(0)7335/18493-45 http://www.telemotive.de
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH 13/13] powerpc/5200: LocalPlus driver: clean up comments
2009-12-22 7:13 ` [PATCH 13/13] powerpc/5200: LocalPlus driver: clean up comments Roman Fietze
@ 2010-01-11 20:24 ` Grant Likely
0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2010-01-11 20:24 UTC (permalink / raw)
To: Roman Fietze; +Cc: linuxppc-dev
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 [flat|nested] 46+ messages in thread