* [PATCH] can: mcp251x: avoid repeated frame bug
@ 2012-09-03 12:17 Marc Kleine-Budde
2012-09-03 17:13 ` Benoît
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-09-03 12:17 UTC (permalink / raw)
To: linux-can; +Cc: Benoit, Marc Kleine-Budde
From: Benoit <benoit@pouic.eu>
The MCP2515 has a silicon bug causing repeated frame transmission, see section
5 of MCP2515 Rev. B Silicon Errata Revision G (March 2007).
Basically, setting TXBnCTRL.TXREQ in either SPI mode (00 or 11) will eventually
cause the bug. The workaround proposed by Microchip is to use mode 00 and send
an RTS command on the SPI bus to initiate the transmission.
TODO: get S-o-b from Benoit <benoit@pouic.eu>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hello Benoit,
can you please test this patch, if it still fixes your problem. In order to
integrate this patch into the kernel, I need you Signed-off-by [1] with you
real name.
regards, Marc
[1] http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L298
drivers/net/can/mcp251x.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index a580db2..26e7129 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -83,6 +83,11 @@
#define INSTRUCTION_LOAD_TXB(n) (0x40 + 2 * (n))
#define INSTRUCTION_READ_RXB(n) (((n) == 0) ? 0x90 : 0x94)
#define INSTRUCTION_RESET 0xC0
+#define RTS_TXB0 0x01
+#define RTS_TXB1 0x02
+#define RTS_TXB2 0x04
+#define INSTRUCTION_RTS(n) (0x80 | ((n) & 0x07))
+
/* MPC251x registers */
#define CANSTAT 0x0e
@@ -397,6 +402,7 @@ static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
int tx_buf_idx)
{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
u32 sid, eid, exide, rtr;
u8 buf[SPI_TRANSFER_BUF_LEN];
@@ -418,7 +424,10 @@ static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
memcpy(buf + TXBDAT_OFF, frame->data, frame->can_dlc);
mcp251x_hw_tx_frame(spi, buf, frame->can_dlc, tx_buf_idx);
- mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx), TXBCTRL_TXREQ);
+
+ /* use INSTRUCTION_RTS, to avoid "repeated frame problem" */
+ priv->spi_tx_buf[0] = INSTRUCTION_RTS(1 << tx_buf_idx);
+ mcp251x_spi_trans(priv->spi, 1);
}
static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
--
1.7.10
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] can: mcp251x: avoid repeated frame bug
2012-09-03 12:17 [PATCH] can: mcp251x: avoid repeated frame bug Marc Kleine-Budde
@ 2012-09-03 17:13 ` Benoît
2012-09-03 17:59 ` Marc Kleine-Budde
2012-09-05 19:59 ` Marc Kleine-Budde
0 siblings, 2 replies; 6+ messages in thread
From: Benoît @ 2012-09-03 17:13 UTC (permalink / raw)
To: linux-can; +Cc: Marc Kleine-Budde
Hello Marc,
First, thank you for your attention on this bug.
On 09/03/2012 02:17 PM, Marc Kleine-Budde wrote:
> From: Benoit <benoit@pouic.eu>
>
> The MCP2515 has a silicon bug causing repeated frame transmission, see section
> 5 of MCP2515 Rev. B Silicon Errata Revision G (March 2007).
>
> Basically, setting TXBnCTRL.TXREQ in either SPI mode (00 or 11) will eventually
> cause the bug. The workaround proposed by Microchip is to use mode 00 and send
> an RTS command on the SPI bus to initiate the transmission.
>
> TODO: get S-o-b from Benoit <benoit@pouic.eu>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Hello Benoit,
>
> can you please test this patch, if it still fixes your problem. In order to
> integrate this patch into the kernel, I need you Signed-off-by [1] with you
> real name.
I just carefully ran some before/after tests with your updated patch and I confirm the problem is still fixed with it.
As for my "Signed-off-by", here it goes:
Signed-off-by: Benoît Locher <Benoit.Locher@skf.com>
By the way, about using spi_write()instead of mcp251x_spi_trans(): I innocently thought it would make less function call nesting,
trying to improve response time of the driver particularly on my Raspberry Pi.
Best Regards,
Benoît.
>
> regards, Marc
>
> [1] http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L298
>
> drivers/net/can/mcp251x.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
> index a580db2..26e7129 100644
> --- a/drivers/net/can/mcp251x.c
> +++ b/drivers/net/can/mcp251x.c
> @@ -83,6 +83,11 @@
> #define INSTRUCTION_LOAD_TXB(n) (0x40 + 2 * (n))
> #define INSTRUCTION_READ_RXB(n) (((n) == 0) ? 0x90 : 0x94)
> #define INSTRUCTION_RESET 0xC0
> +#define RTS_TXB0 0x01
> +#define RTS_TXB1 0x02
> +#define RTS_TXB2 0x04
> +#define INSTRUCTION_RTS(n) (0x80 | ((n) & 0x07))
> +
>
> /* MPC251x registers */
> #define CANSTAT 0x0e
> @@ -397,6 +402,7 @@ static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
> static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
> int tx_buf_idx)
> {
> + struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
> u32 sid, eid, exide, rtr;
> u8 buf[SPI_TRANSFER_BUF_LEN];
>
> @@ -418,7 +424,10 @@ static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
> buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
> memcpy(buf + TXBDAT_OFF, frame->data, frame->can_dlc);
> mcp251x_hw_tx_frame(spi, buf, frame->can_dlc, tx_buf_idx);
> - mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx), TXBCTRL_TXREQ);
> +
> + /* use INSTRUCTION_RTS, to avoid "repeated frame problem" */
> + priv->spi_tx_buf[0] = INSTRUCTION_RTS(1 << tx_buf_idx);
> + mcp251x_spi_trans(priv->spi, 1);
> }
>
> static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] can: mcp251x: avoid repeated frame bug
2012-09-03 17:13 ` Benoît
@ 2012-09-03 17:59 ` Marc Kleine-Budde
2012-09-03 18:09 ` Benoît
2012-09-05 19:59 ` Marc Kleine-Budde
1 sibling, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-09-03 17:59 UTC (permalink / raw)
To: Benoît; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
On 09/03/2012 07:13 PM, Benoît wrote:
[...]
>> Hello Benoit,
>>
>> can you please test this patch, if it still fixes your problem. In
>> order to
>> integrate this patch into the kernel, I need you Signed-off-by [1]
>> with you
>> real name.
>
> I just carefully ran some before/after tests with your updated patch and
> I confirm the problem is still fixed with it.
Thanks.
>
> As for my "Signed-off-by", here it goes:
>
> Signed-off-by: Benoît Locher <Benoit.Locher@skf.com>
I'll change the author of the patch to that address then.
> By the way, about using spi_write()instead of mcp251x_spi_trans(): I
> innocently thought it would make less function call nesting,
> trying to improve response time of the driver particularly on my
> Raspberry Pi.
Ahh, I see. but the mcp251x_spi_trans() function takes does the setup
thats needed for DMA.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: mcp251x: avoid repeated frame bug
2012-09-03 17:59 ` Marc Kleine-Budde
@ 2012-09-03 18:09 ` Benoît
0 siblings, 0 replies; 6+ messages in thread
From: Benoît @ 2012-09-03 18:09 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
On 09/03/2012 07:59 PM, Marc Kleine-Budde wrote:
>> By the way, about using spi_write()instead of mcp251x_spi_trans(): I
>> innocently thought it would make less function call nesting,
>> trying to improve response time of the driver particularly on my
>> Raspberry Pi.
> Ahh, I see. but the mcp251x_spi_trans() function takes does the setup
> thats needed for DMA.
Very good argument indeed.
Best Regards,
Benoît.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] can: mcp251x: avoid repeated frame bug
2012-09-03 17:13 ` Benoît
2012-09-03 17:59 ` Marc Kleine-Budde
@ 2012-09-05 19:59 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-09-05 19:59 UTC (permalink / raw)
To: Benoît; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]
On 09/03/2012 07:13 PM, Benoît wrote:
[...]
> I just carefully ran some before/after tests with your updated patch and
> I confirm the problem is still fixed with it.
>
> As for my "Signed-off-by", here it goes:
>
> Signed-off-by: Benoît Locher <Benoit.Locher@skf.com>
Congrats, the patch will be part of v3.6. :)
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* pull-request: can 2012-09-04
@ 2012-09-04 20:55 Marc Kleine-Budde
2012-09-04 20:55 ` [PATCH] can: mcp251x: avoid repeated frame bug Marc Kleine-Budde
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-09-04 20:55 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-can
Hello David,
this patch is for the v3.6 release cycle. Benoît Locher fixed a repeated frame
bug in the mcp251x driver. He implemented the workaround suggested by the
errata sheet.
regards, Marc
--
The following changes since commit 5002200599429e83fc13e0d9a2d4788b79515b0c:
net: qmi_wwan: add several new Gobi devices (2012-09-01 22:49:34 -0400)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can.git fixes-for-3.6
for you to fetch changes up to cab32f39dcc5b35db96497dc0a026b5dea76e4e7:
can: mcp251x: avoid repeated frame bug (2012-09-03 20:12:06 +0200)
----------------------------------------------------------------
Benoît Locher (1):
can: mcp251x: avoid repeated frame bug
drivers/net/can/mcp251x.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] can: mcp251x: avoid repeated frame bug
2012-09-04 20:55 pull-request: can 2012-09-04 Marc Kleine-Budde
@ 2012-09-04 20:55 ` Marc Kleine-Budde
0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2012-09-04 20:55 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-can, Benoît Locher, stable, Marc Kleine-Budde
From: Benoît Locher <Benoit.Locher@skf.com>
The MCP2515 has a silicon bug causing repeated frame transmission, see section
5 of MCP2515 Rev. B Silicon Errata Revision G (March 2007).
Basically, setting TXBnCTRL.TXREQ in either SPI mode (00 or 11) will eventually
cause the bug. The workaround proposed by Microchip is to use mode 00 and send
a RTS command on the SPI bus to initiate the transmission.
Cc: <stable@vger.kernel.org>
Signed-off-by: Benoît Locher <Benoit.Locher@skf.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/mcp251x.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
index a580db2..26e7129 100644
--- a/drivers/net/can/mcp251x.c
+++ b/drivers/net/can/mcp251x.c
@@ -83,6 +83,11 @@
#define INSTRUCTION_LOAD_TXB(n) (0x40 + 2 * (n))
#define INSTRUCTION_READ_RXB(n) (((n) == 0) ? 0x90 : 0x94)
#define INSTRUCTION_RESET 0xC0
+#define RTS_TXB0 0x01
+#define RTS_TXB1 0x02
+#define RTS_TXB2 0x04
+#define INSTRUCTION_RTS(n) (0x80 | ((n) & 0x07))
+
/* MPC251x registers */
#define CANSTAT 0x0e
@@ -397,6 +402,7 @@ static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
int tx_buf_idx)
{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
u32 sid, eid, exide, rtr;
u8 buf[SPI_TRANSFER_BUF_LEN];
@@ -418,7 +424,10 @@ static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
memcpy(buf + TXBDAT_OFF, frame->data, frame->can_dlc);
mcp251x_hw_tx_frame(spi, buf, frame->can_dlc, tx_buf_idx);
- mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx), TXBCTRL_TXREQ);
+
+ /* use INSTRUCTION_RTS, to avoid "repeated frame problem" */
+ priv->spi_tx_buf[0] = INSTRUCTION_RTS(1 << tx_buf_idx);
+ mcp251x_spi_trans(priv->spi, 1);
}
static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
--
1.7.10
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-05 19:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-03 12:17 [PATCH] can: mcp251x: avoid repeated frame bug Marc Kleine-Budde
2012-09-03 17:13 ` Benoît
2012-09-03 17:59 ` Marc Kleine-Budde
2012-09-03 18:09 ` Benoît
2012-09-05 19:59 ` Marc Kleine-Budde
-- strict thread matches above, loose matches on Subject: below --
2012-09-04 20:55 pull-request: can 2012-09-04 Marc Kleine-Budde
2012-09-04 20:55 ` [PATCH] can: mcp251x: avoid repeated frame bug Marc Kleine-Budde
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.