On 17.07.2023 12:08:15, Marc Kleine-Budde wrote:
> From: Fedor Ross <fedor.ross@xxxxxxx>
>
> The mcp251xfd controller needs an idle bus to enter 'Normal CAN 2.0
> mode' or . The maximum length of a CAN frame is 736 bits (64 data
> bytes, CAN-FD, EFF mode, worst case bit stuffing and interframe
> spacing). For low bit rates like 10 kbit/s the arbitrarily chosen
> MCP251XFD_POLL_TIMEOUT_US of 1 ms is too small.
>
> Otherwise during polling for the CAN controller to enter 'Normal CAN
> 2.0 mode' the timeout limit is exceeded and the configuration fails
> with:
>
> | $ ip link set dev can1 up type can bitrate 10000
> | [ 731.911072] mcp251xfd spi2.1 can1: Controller failed to enter mode CAN 2.0 Mode (6) and stays in Configuration Mode (4) (con=0x068b0760, osc=0x00000468).
> | [ 731.927192] mcp251xfd spi2.1 can1: CRC read error at address 0x0e0c (length=4, data=00 00 00 00, CRC=0x0000) retrying.
> | [ 731.938101] A link change request failed with some changes committed already. Interface can1 may have been left with an inconsistent configuration, please check.
> | RTNETLINK answers: Connection timed out
>
> Make MCP251XFD_POLL_TIMEOUT_US timeout calculation dynamic. Use
> maximum of 1ms and bit time of 1 full 64 data bytes CAN-FD frame in
> EFF mode, worst case bit stuffing and interframe spacing at the
> current bit rate.
>
> For easier backporting define the macro MCP251XFD_FRAME_LEN_MAX_BITS
> that holds the max frame length in bits, which is 736. This can be
> replaced by can_frame_bits(true, true, true, true, CANFD_MAX_DLEN) in
> a cleanup patch later.
>
> Fixes: 55e5b97f003e8 ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN")
> Signed-off-by: Fedor Ross <fedor.ross@xxxxxxx>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> Cc: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> Cc: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> Cc: Thomas Kopp <thomas.kopp@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
> Hello,
>
> picking up Fedor's and Marek's work. I decided to make it a minimal
> patch and add stable on Cc. The mentioned cleanup patch that replaces
> 736 by can_frame_bits() can be done later and will go upstream via
> can-next.
>
> regards,
> Marc
>
> v3:
> - use 736 as max CAN frame length, calculated by Vincent Mailhol's
> 80a2fbce456e ("can: length: refactor frame lengths definition to add size in bits")
> - update commit message
> - drop patch 2/2
>
> v2: https://lore.kernel.org/all/20230505222820.126441-1-marex@xxxxxxx
> - Add macros for CAN_BIT_STUFFING_OVERHEAD and CAN_IDLE_CONDITION_SAMPLES
> (thanks Thomas, but please double check the comments)
> - Update commit message
>
> v1: https://lore.kernel.org/all/20230504195059.4706-1-marex@xxxxxxx
>
> drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 4 +++-
> drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 1 +
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index 68df6d4641b5..876e8e3cbb0b 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> @@ -227,6 +227,7 @@ static int
> __mcp251xfd_chip_set_mode(const struct mcp251xfd_priv *priv,
> const u8 mode_req, bool nowait)
> {
> + const struct can_bittiming *bt = &priv->can.bittiming;
> u32 con = 0, con_reqop, osc = 0;
> u8 mode;
> int err;
> @@ -251,7 +252,8 @@ __mcp251xfd_chip_set_mode(const struct mcp251xfd_priv *priv,
> FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK,
> con) == mode_req,
> MCP251XFD_POLL_SLEEP_US,
> - MCP251XFD_POLL_TIMEOUT_US);
> + max_t(unsigned long, MCP251XFD_POLL_TIMEOUT_US,
> + MCP251XFD_FRAME_LEN_MAX_BITS * USEC_PER_SEC / bt->bitrate));
This segfaults with a div by zero if bitrate is not set, yet. Fixed in
v4.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
