From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752433AbcHHNYM (ORCPT ); Mon, 8 Aug 2016 09:24:12 -0400 Received: from relay-b03.edpnet.be ([212.71.1.220]:50003 "EHLO relay-b03.edpnet.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752107AbcHHNYK (ORCPT ); Mon, 8 Aug 2016 09:24:10 -0400 X-Greylist: delayed 1051 seconds by postgrey-1.27 at vger.kernel.org; Mon, 08 Aug 2016 09:24:09 EDT X-ASG-Debug-ID: 1470661595-0a88181ce6227dd30001-xx1T2L X-Barracuda-Envelope-From: dev.kurt@vandijck-laurijssen.be X-Barracuda-Effective-Source-IP: 109.236.138.82.wls.msr91gkk3.adsl.dyn.edpnet.net[109.236.138.82] X-Barracuda-Apparent-Source-IP: 109.236.138.82 Date: Mon, 8 Aug 2016 15:06:33 +0200 From: Kurt Van Dijck To: Wolfgang Grandegger Cc: Andreas Werner , mkl@pengutronix.de, linux-can@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, davem@davemloft.net, jthumshirn@suse.de, andy@wernerandy.de, michael.miehling@men.de Subject: Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver Message-ID: <20160808130633.GC12298@airbook.newtec.eu> X-ASG-Orig-Subj: Re: [PATCH RESEND] net: can: Introduce MEN 16Z192-00 CAN controller driver Mail-Followup-To: Wolfgang Grandegger , Andreas Werner , mkl@pengutronix.de, linux-can@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, davem@davemloft.net, jthumshirn@suse.de, andy@wernerandy.de, michael.miehling@men.de References: <20160726091555.GA26227@awelinux> <9487fe77-dbd0-3a64-b648-cf76b897feda@grandegger.com> <20160808113938.GA17459@awelinux> <456c4f82-6a53-e821-3396-63d413db9eb7@grandegger.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <456c4f82-6a53-e821-3396-63d413db9eb7@grandegger.com> User-Agent: Mutt/1.5.22 (2013-10-16) X-Barracuda-Connect: 109.236.138.82.wls.msr91gkk3.adsl.dyn.edpnet.net[109.236.138.82] X-Barracuda-Start-Time: 1470661595 X-Barracuda-URL: https://212.71.1.220:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 1285 X-Barracuda-BRTS-Status: 1 X-Barracuda-Bayes: INNOCENT GLOBAL 0.5023 1.0000 0.7500 X-Barracuda-Spam-Score: 0.75 X-Barracuda-Spam-Status: No, SCORE=0.75 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=6.0 tests=BSF_SC0_MISMATCH_TO X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.31838 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.00 BSF_SC0_MISMATCH_TO Envelope rcpt doesn't match header Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --- Original message --- > Date: Mon, 8 Aug 2016 14:28:39 +0200 > From: Wolfgang Grandegger > [...] > >>>+ > >>>+ 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