On Mon, Aug 08, 2016 at 03:06:33PM +0200, Kurt Van Dijck wrote:
>
> --- Original message ---
> > Date: Mon, 8 Aug 2016 14:28:39 +0200
> > From: Wolfgang Grandegger <w...@grandegger.com>
> >
> [...]
> > >>>+
> > >>>+ if (!(cf->can_id & CAN_RTR_FLAG)) {
> > >>>+ writel(data[0], &cf_buf->data[0]);
> > >>>+ writel(data[1], &cf_buf->data[1]);
> > >>
> > >>Why do you not check cf->can_dlc here as well. And is the extra copy
> > >>necessary.
> > >>
> > >
> > >Yes, I agree with you. The extra copy could be also avoided.
> > >
> > >>>+
> > >>>+ stats->tx_bytes += cf->can_dlc;
> > >>>+ }
> > >>
> > >>If I look to other drivers, they write the data even in case of RTR.
> > >>
> > >
> > >But why?
> > >
> > >A RTR does not have any data, therefore there is no need to write the data.
> > >Only the length is required as the request size.
> >
> > Yes; I'm wondering as well.
> >
> > >
> > >If there is a reason behind writing the data of a RTR frame, I can
> > >change that, but for now there is no reason.
> >
> > Yep.
>
> I _think_ that copying the data without checking the RTR bit clearly
> avoids a condition and might produce faster code on some machines.
> In any case, it reads easier.
> I'm not sure how that interacts with caches etc etc.
>
> On the other hand, giving unused data is a bad habit that may reveal
> security information on some places, so better avoid it.
>
> Kurt
Hi Kurt,
thanks for your comment.
In my opinion, I really prever to NOT copying such data if the RTR flag ist set.
Regards
Andy
