[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAOprWoucfBm_BZOwU+qzo3YrpDE+f-x4YKNDS6phtOD2hvjsGg@mail.gmail.com>
Date: Sun, 28 Sep 2025 00:26:59 +0800
From: Andrea Daoud <andreadaoud6@...il.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Heiko Stuebner <heiko@...ech.de>, Elaine Zhang <zhangqing@...k-chips.com>, kernel@...gutronix.de,
linux-can@...r.kernel.org, netdev@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org, Alexander Shiyan <eagle.alexander923@...il.com>
Subject: Re: Possible race condition of the rockchip_canfd driver
Hi Marc,
On Mon, Sep 22, 2025 at 4:50 PM Marc Kleine-Budde <mkl@...gutronix.de> wrote:
>
> On 20.09.2025 18:08:03, Andrea Daoud wrote:
> > > On 18.09.2025 20:58:33, Andrea Daoud wrote:
> > > > I'm using the rockchip_canfd driver on an RK3568. When under high bus
> > > > load, I get
> > > > the following logs [1] in rkcanfd_tx_tail_is_eff, and the CAN bus is unable to
> > > > communicate properly under this condition. The exact cause is currently not
> > > > entirely clear, and it's not reliably reproducible.
> > >
> > > Our customer is using a v3 silicon revision of the chip, which doesn't
> > > this workaround.
> >
> > Could you please let me know how to check whether my RK3568 is v2 or v3?
>
> Alexander Shiyan (Cc'ed) reads the information from an nvmem cell:
>
> | https://github.com/MacroGroup/barebox/blob/macro/arch/arm/boards/diasom-rk3568/board.c#L239-L257
>
> The idea is to fixup the device tree in the bootloader depending on the
> SoC revision, so that the CAN driver uses only the needed workarounds.
>
Thanks, it is not easy to correlate this because I am currently not using
barebox. I'll try this later.
> > > > In the logs we can spot some strange points:
> > > >
> > > > 1. Line 24, tx_head == tx_tail. This should have been rejected by the if
> > > > (!rkcanfd_get_tx_pending) clause.
> > > >
> > > > 2. Line 26, the last bit of priv->tx_tail (0x0185dbb3) is 1. This means that the
> > > > tx_tail should be 1, because rkcanfd_get_tx_tail is essentially mod the
> > > > priv->tx_tail by two. But the printed tx_tail is 0.
> > > >
> > > > I believe these problems could mean that the code is suffering from some race
> > > > condition. It seems that, in the whole IRQ processing chain of the driver,
> > > > there's no lock protection. Maybe some IRQ happens within the execution of
> > > > rkcanfd_tx_tail_is_eff, and touches the state of the tx_head and tx_tail?
> > > >
> > > > Could you please have a look at the code, and check if some locking is needed?
> > >
> > > My time for community support is currently a bit limited. I think this
> > > has to wait a bit, apologies :/
> >
> > No worries, I will debug myself, and hopefully send a PR if I found
> > something out.
>
> Great, I have a both a v2 and a v3 SoC here to test.
It turns out there are two issues:
1. The race condition (between TX interrupt and TX queue netif xmit) is indeed
an issue. Fixed by adding a spinlock around TX logic, and no warning
occurs after
adding the lock.
2. The CLK_CAN0 was clocked by GPLL, which makes it 148.5MHz. This frequency
will lead to various errors quickly when dealing with EFF IDs. Fixed
by constraining it
to use CPLL and 250MHz freq.
Regarding problem 1, I will send a patch.
Regarding problem 2, I suggest adding some extra sanity checks to
guard against this.
>
> 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 |
Regards,
Andrea
Powered by blists - more mailing lists