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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 263B2C433EF for ; Sat, 7 May 2022 14:33:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1383480AbiEGOh2 (ORCPT ); Sat, 7 May 2022 10:37:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39638 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347909AbiEGOh1 (ORCPT ); Sat, 7 May 2022 10:37:27 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B6F781580B; Sat, 7 May 2022 07:33:40 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 402A960B3D; Sat, 7 May 2022 14:33:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9B8A3C385A9; Sat, 7 May 2022 14:33:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1651934019; bh=QTQsvBXAXq+u552u7h6IkbyqINQY/MTcAhV9CyC5H2w=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GzlWpqVpg2p5jIuObrLhrKSOypv6RjDK7ZulDZ7jfkMvZEDa7Zs8hykQeqF1/Cl9h RYNvslJFfV+TFq4bmUhbTLIFG2g5HX6KMAqJ4OiEdg7RCQ8pMI6qHj/SRhWv0dz/hE j4Qu4C9rYj5/saYBgjvTz+EmHx9Nc2cuSzwdgxrSQ7KQqxlDKE9UVYTnfyMONxDTbq HnG4zvJeNBdqobtbokkaUMUc8S+wVWPxnTMP5ROMW4/hE/6738HgqLh/YW5mzfkKNI xaK2BV805J08yvxEVtVdQ8P4C5DI7/HTwqfU5og+aUVewHEVXQuv9VUGeIzcyBUuiD QriFlCg/yDJhQ== Date: Sat, 7 May 2022 15:42:06 +0100 From: Jonathan Cameron To: Andy Shevchenko Cc: LI Qingwu , Lars-Peter Clausen , Rob Herring , Tomas Melin , devicetree , Linux Kernel Mailing List , linux-iio , Rob Herring Subject: Re: [PATCH V3 3/5] iio: accel: sca3300: modified to support multi chips Message-ID: <20220507154206.328e2877@jic23-huawei> In-Reply-To: References: <20220504133612.604304-1-Qing-wu.Li@leica-geosystems.com.cn> <20220504133612.604304-4-Qing-wu.Li@leica-geosystems.com.cn> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Thu, 5 May 2022 20:01:22 +0200 Andy Shevchenko wrote: > On Thu, May 5, 2022 at 4:12 PM LI Qingwu > wrote: > > > -----Original Message----- > > > From: Andy Shevchenko > > > Sent: Wednesday, May 4, 2022 10:39 PM > > > On Wed, May 4, 2022 at 4:35 PM LI Qingwu > > > wrote: =20 > > > > > From: Andy Shevchenko > > > > > Sent: Wednesday, May 4, 2022 10:20 PM On Wed, May 4, 2022 at 3:36= PM > > > > > LI Qingwu wrote: =20 >=20 > ... >=20 > > > > > > +struct sca3300_chip_info { > > > > > > + const struct iio_chan_spec *channels; > > > > > > + const int (*accel_scale_table)[2]; > > > > > > + const int *accel_scale_modes_map; > > > > > > + const unsigned long *scan_masks; > > > > > > + const int *avail_modes_table; > > > > > > + const int *freq_modes_map; > > > > > > + const int *freq_table; > > > > > > + const u8 num_accel_scales; > > > > > > + const u8 num_avail_modes; > > > > > > + const u8 num_channels; > > > > > > + const u8 num_freqs; > > > > > > + const u8 chip_id; =20 > > > > > > > > > > Why do you have const qualifier on all members? The last one is > > > > > understandable, but the rest, esp. pointers should be justified. = =20 > > > > Because I thought it was static and has fix value for each chip, un= acceptable =20 > > > for you? > > > > > > But why const qualifier? What is the point of it for example for u8 m= embers if > > > the entire object is qualified as const below in the same patch? > > > > > > On top of that, please explain what in your opinion the "const ... > > > *foo" gives us, and what we will lose if we remove the "const" part o= ut of them. =20 > > > > Ah, you are right, those const are unnecessary for nonpointer members. > > for the pointers, the contexts that the pointer points to are still wri= table. > > what about if I remove all the const from nonpointer and keep it for th= e pointers? > > Like=EF=BC=9A > > const struct iio_chan_spec *channels; > > const int (*accel_scale_table)[2]; > > const int (*incli_scale_table)[2]; > > const int *accel_scale_modes_map; > > const int *incli_scale_modes_map; > > const unsigned long *scan_masks; > > const int *avail_modes_table; > > const int *freq_modes_map; > > const int *freq_table; > > const char *name; > > u8 num_accel_scales; > > u8 num_incli_scales; > > u8 num_avail_modes; > > u8 num_channels; > > u8 num_freqs; > > u8 chip_id; > > bool angle; =20 >=20 > It's better, but you still need to justify the rest with explanation > in the commit message. > And I leave this to maintainers to say if the const:s are needed or not. Where they are being set to point to static const int array[]=20 then to my mind it makes sense to have them as const as makes it nice and o= bvious what is going on. Jonathan >=20 > > > > > > + const char *name; > > > > > > +}; =20 >=20