From: Joakim Zhang <qiangqing.zhang@nxp.com>
To: Stefan-gabriel Mirea <stefan-gabriel.mirea@nxp.com>,
Marc Kleine-Budde <mkl@pengutronix.de>
Cc: "linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH V2 2/5] can: flexcan: add CAN FD mode support
Date: Tue, 9 Apr 2019 07:36:29 +0000 [thread overview]
Message-ID: <DB7PR04MB46185B04DA6F714FF120A3A8E62D0@DB7PR04MB4618.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <VI1PR0402MB286355821F5C81122A7C86E2DF2D0@VI1PR0402MB2863.eurprd04.prod.outlook.com>
> -----Original Message-----
> From: Stefan-gabriel Mirea
> Sent: 2019年4月9日 15:05
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Marc Kleine-Budde
> <mkl@pengutronix.de>
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: RE: [PATCH V2 2/5] can: flexcan: add CAN FD mode support
>
> > From: Joakim Zhang
> > Sent: Tuesday, April 9, 2019 5:07 AM
> > Hi Stefan,
> >
> > Thanks for your validation! Could you add your test tag if you can
> > successfully validated?
>
> Sure, no problem. Please note that I needed to replace "flexcan_read" and
> "flexcan_write" with "priv->read" and "priv->write" respectively, in PATCH V2
> 4/5.
Hi Stefan,
I made a mistake here, thank you for pointing out. I will fix it in V3.
> > [Joakim Zhang] We added can fd support in 4.9 kernel which let
> > mailbox_read call alloc_can(fd)_skb in the past. Now the driver
> > allocate skb before mailbox_read in rx_offload and also read the overflow
> frames.
> > I add the "is_canfd" since I don't want to change the
> > rx_offload framework too much. @mkl@pengutronix.de, could you give
> > some advice, which solution is better?
>
> This is more of a functionality issue. For example, candump from canutils 4.0.6
> never shows CAN FD frames, but should still be able to show the CAN 2.0 ones
> regardless of whether "fd on" was supplied. I suggest the following separation:
>
> can_rx_offload_offload_one:
> struct sk_buff *skb = NULL;
> ...
> bool drop = unlikely(skb_queue_len(&offload->skb_queue) >
> offload->skb_queue_len_max);
>
> if (offload->mailbox_read(offload, drop, &skb, ×tamp, n) && !skb)
> offload->dev->stats.rx_dropped++;
>
> if (skb) {
> struct can_rx_offload_cb *cb = can_rx_offload_get_cb(skb);
>
> cb->timestamp = timestamp;
> }
>
> return skb;
>
> flexcan_mailbox_read:
> ...
> if (!drop) {
> if (reg_ctrl & FLEXCAN_MB_CNT_EDL)
> *skb = alloc_canfd_skb(offload->dev, &cf);
> else
> *skb = alloc_can_skb(offload->dev,
> (struct can_frame **)&cf);
> }
> if (*skb) {
> /* use cf */
> ...
> }
> /* mark as read */
>
> Although not visible in the FlexCAN case, the socket buffers would be also
> freed from inside mailbox_read if errors were encountered after allocation.
This is a good solution, I will add it in V3. Thank you very much.
Best Regards,
Joakim Zhang
> Regards,
> Stefan
next prev parent reply other threads:[~2019-04-09 7:36 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-19 5:17 [PATCH V2 0/5] can: flexcan: add CAN FD support on i.MX8QM Joakim Zhang
2019-03-19 5:17 ` [PATCH V2 1/5] can: rx-offload: add CANFD support based on offload Joakim Zhang
2019-03-19 5:17 ` [PATCH V2 2/5] can: flexcan: add CAN FD mode support Joakim Zhang
2019-04-08 17:46 ` Stefan-gabriel Mirea
2019-04-09 2:07 ` Joakim Zhang
2019-04-09 7:04 ` Stefan-gabriel Mirea
2019-04-09 7:36 ` Joakim Zhang [this message]
2019-03-19 5:17 ` [PATCH V2 3/5] can: flexcan: add CANFD BRS support and improve bittiming setting Joakim Zhang
2019-03-19 5:17 ` [PATCH V2 4/5] can: flexcan: add ISO CAN FD feature support Joakim Zhang
2019-03-19 5:17 ` [PATCH V2 5/5] can: flexcan: add imx8qm support Joakim Zhang
2019-04-04 2:11 ` [PATCH V2 0/5] can: flexcan: add CAN FD support on i.MX8QM Joakim Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB7PR04MB46185B04DA6F714FF120A3A8E62D0@DB7PR04MB4618.eurprd04.prod.outlook.com \
--to=qiangqing.zhang@nxp.com \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
--cc=stefan-gabriel.mirea@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).