On 08/01/2018 02:56 PM, Pankaj Bansal wrote:
-----Original Message-----
From: Marc Kleine-Budde [mailto:mkl@xxxxxxxxxxxxxx]
Sent: Wednesday, August 1, 2018 3:00 PM
To: Pankaj Bansal <pankaj.bansal@xxxxxxx>; linux-can@xxxxxxxxxxxxxxx
Subject: Re: [PATCH v3 7/7] net: can: flexcan: use CAN FD frames for Tx/Rx
On 08/01/2018 04:06 PM, Pankaj Bansal wrote:
Use can FD frames for Tx/Rx operations. This would be needed in
upcoming SOC LX2160A, which supports CAN FD protocol
Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx>
I think the code can be simplified if you introduce a union for can_fram and
canfd_frame.
Can you be more specific in which structure/function?
I was thinking of passing a skb to mailbox_read function instead of canfd_frame or can_frame.
That way the code in rx_overload.c would become cleaner.
IMO there's no need to have a union of struct can_frame and struct
canfd_frame as the latter has been defined to have a binary compatible
layout. See at:
https://can-newsletter.org/engineering/standardization/nr_stand_can-fd_linux3.6_120703
So you can put either type of CAN frame into a struct canfd_frame. But
whether your content is CAN or CAN FD has to be known somewhere else!!
At least there should be a comment that the struct canfd_frame is used
for both types of CAN frames.
Regards,
Oliver
Also for overflow cases (when we can't allocate skb for mailbox_read) I think a reserved skb should be
placed in rx_overload structure when rx_overload is initialized.
As for xmit and mailbox_read functions in flexcan.c, I think these are fine.
More review on Friday.
I will wait for your comments, before I send any new version of these patches.
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 |
N�����r��y���b�X��ǧv�^�){.n�+����{�q���^n�r���z���h����&�� �G���h�(�階�ݢj"���m�����z�ޖ���f���h���~�mml==
--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
