Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1457787

Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver

Show all headers | View raw

--- Original message ---
> Date:	Mon, 8 Aug 2016 14:28:39 +0200
> From: Wolfgang Grandegger <wg@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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-07-26 11:20 +0200
  Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Benjamin Poirier <benjamin.poirier@gmail.com> - 2016-08-08 06:00 +0200
    Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-08-08 09:40 +0200
      Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Benjamin Poirier <benjamin.poirier@gmail.com> - 2016-08-09 05:30 +0200
        Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-08-09 08:20 +0200
  Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Wolfgang Grandegger <wg@grandegger.com> - 2016-08-08 11:40 +0200
    Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-08-08 13:50 +0200
      Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Wolfgang Grandegger <wg@grandegger.com> - 2016-08-08 14:30 +0200
        Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be> - 2016-08-08 15:30 +0200
          Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-08-08 16:20 +0200
        Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-08-08 16:10 +0200
          Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Wolfgang Grandegger <wg@grandegger.com> - 2016-08-08 16:40 +0200
            Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Andreas Werner <andreas.werner@men.de> - 2016-08-09 08:20 +0200
              Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Wolfgang Grandegger <wg@grandegger.com> - 2016-08-09 14:00 +0200
              Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Oliver Hartkopp <socketcan@hartkopp.net> - 2016-08-10 22:30 +0200
  RE: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller  driver Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> - 2016-08-09 11:40 +0200

csiph-web