From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.4 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 02280C2D0A3 for ; Mon, 9 Nov 2020 15:38:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 955802074F for ; Mon, 9 Nov 2020 15:38:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b="NLLux4EF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729914AbgKIPia (ORCPT ); Mon, 9 Nov 2020 10:38:30 -0500 Received: from mo4-p02-ob.smtp.rzone.de ([85.215.255.83]:21166 "EHLO mo4-p02-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727311AbgKIPia (ORCPT ); Mon, 9 Nov 2020 10:38:30 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1604936308; s=strato-dkim-0002; d=hartkopp.net; h=In-Reply-To:Date:Message-ID:From:References:Cc:To:Subject: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=S6fI1VVgo3BFqkCRhbg9Tu9n35Sv2Rz9Yq/+PsHLB0I=; b=NLLux4EFYzcguf8u4HCNV5NB376Fstv1MSblLpW5OTpW39WVnhtVf4S3i0uM2ajdhd X3+K1ngiSTBbnKFVp/fRW8sYu1eJaoAu2amApzdU7YUEjCly1N3HCI8p3+Ia0unTsnpL Qenh25XXbA59xz5Y5GIxPSg8GPqnPG9C3P/kVD51U3Y0hS3Feegxct96kwqI+sHrJweI sS+inuOKYTi8Lk6/HWCdaVQbA4oB3FYq9fCOCUg44KjiwGsk40n2+mleT64ePUW9ib88 7iBlCQnCspbgCR+VdoBB8FjE+l9gPnTUEm1oDdXDsk/1bTp8yo7zcJSZwVC9LJam44QD X2uw== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjG14FZxedJy6qgO1o3HMbEWKONeXTNI=" X-RZG-CLASS-ID: mo00 Received: from [192.168.50.177] by smtp.strato.de (RZmta 47.3.3 DYNA|AUTH) with ESMTPSA id V0298cwA9FcQ864 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Mon, 9 Nov 2020 16:38:26 +0100 (CET) Subject: Re: [PATCH v4 4/7] can: replace can_dlc as variable/element for payload length To: Vincent MAILHOL Cc: linux-can , Marc Kleine-Budde , netdev References: <20201109102618.2495-1-socketcan@hartkopp.net> <20201109102618.2495-5-socketcan@hartkopp.net> From: Oliver Hartkopp Message-ID: Date: Mon, 9 Nov 2020 16:38:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Vincent, On 09.11.20 13:59, Vincent MAILHOL wrote: > On Mon. 9 Nov 2020 at 19:26, Oliver Hartkopp wrote: >> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h >> index b2e8df8e4cb0..72671184a7a2 100644 >> --- a/include/linux/can/dev.h >> +++ b/include/linux/can/dev.h >> @@ -183,12 +183,12 @@ static inline void can_set_static_ctrlmode(struct net_device *dev, >> /* override MTU which was set by default in can_setup()? */ >> if (static_mode & CAN_CTRLMODE_FD) >> dev->mtu = CANFD_MTU; >> } >> >> -/* get data length from can_dlc with sanitized can_dlc */ >> -u8 can_dlc2len(u8 can_dlc); >> +/* get data length from raw data length code (DLC) */ > > /* > * convert a given data length code (dlc) of an FD CAN frame into a > * valid data length of max. 64 bytes. > */ > > I missed this point during my previous review: the can_dlc2len() function > is only valid for CAN FD frames. Comments should reflect this fact. > >> +u8 can_dlc2len(u8 dlc); > > Concerning the name: > * can_get_cc_len() converts a Classical CAN frame DLC into a data > length. > * can_dlc2len() converts an FD CAN frame DLC into a data length. > > Just realized that both macro/function do similar things so we could > think of a similar naming as well. > * Example 1: can_get_cc_len() and can_get_fd_len() > * Example 2: can_cc_dlc2len() and can_fd_dlc2len() I like! Patch set v5 is out now. Thanks, Oliver > > Or we could simply leave things as they are, this is not a big issue > as long as the comments clearly state which one is for classical > frames and which one is for FD frames. > >> >> /* map the sanitized data length to an appropriate data length code */ >> u8 can_len2dlc(u8 len); > > can_len2dlc() might be renamed (e.g. can_get_fd_dlc()) if Example 1 > solution is chosen. > >> struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max, > > Yours sincerely, > Vincent Mailhol >