Received: by 2002:a05:6358:5282:b0:b5:90e7:25cb with SMTP id g2csp582937rwa; Sat, 20 Aug 2022 09:26:00 -0700 (PDT) X-Google-Smtp-Source: AA6agR5QEPxK7tKYcp62YMv9MM894XF7RKGke/DmYm7nlD8EaZpEhrvKP9p5l3Ir7bd6Va1Oi7GG X-Received: by 2002:a17:907:a427:b0:732:ea25:2d38 with SMTP id sg39-20020a170907a42700b00732ea252d38mr8053090ejc.87.1661012760555; Sat, 20 Aug 2022 09:26:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1661012760; cv=none; d=google.com; s=arc-20160816; b=JBdDRhAYLL6zFyYfHfWDjeHNgkN3EAdB18wE7D/Bn+405KMoMak0UDnSUVSti3LLTl ExPfQ8xzM6W+EQh5eGc3Fzp/DNFjMLbfRGn/jiAJcL4q0HAG9uNXoDJ5+gukQ9cVh1U8 W62Gv/2W+uR2WMojX/GFE+ZhQtnpVelkPnHOsoCjHaUieue0gqC0kpsvB8FvyZ7fSWI8 TrS6hltRlLf6W+lY1RePtN6x/936Ow1TmvwoMEDPIbJ5Fs8fsI5GS/+Syrc25TQLix1n iztc7Lb/qea+WpKngBVWy0e5IWYd8PpiKKFDFyMbHRw7tKyeLC1w0JQkZESGyexiygfs QeLw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=HdF47He1obOe/z7bMXQ34hAtRqKgVWrmpmNXRVq1I2Q=; b=Yrr5C70BlMA9Tm/7bFh8Klty3wx+UvOog26+cGe/UM7uF5tKwQrAtBLcxG+NUS/qLw 8F3nw7fjZIOfLpzjsVACrGcTvuO5gihR433J7wuWoKL41cs0Ir2FwcZ9lZZ5Of5wj5GP 97yhKSNJltOE/QVJvDv+xuL3O5E3JG0/7/lTv9CprfOF31VOjdjnmZ/g2Xwnrpq0SYWK PKDFQQOX45Wi1HxN7aaVN6MbOHo3lmIWgoTcghPTAVwwHo/LrRTY4b1ThXhUahrsBCGJ lkSTEoq4akvPb0v12XEq+cOx14YXc2UrujD+IFlzqvIgi01b/XSEeStCtDKypa0WvFeX MKMg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b="lnk0sD/A"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y4-20020a50eb04000000b0043d3a571d3fsi4947409edp.445.2022.08.20.09.25.35; Sat, 20 Aug 2022 09:26:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b="lnk0sD/A"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232648AbiHTQWT (ORCPT + 99 others); Sat, 20 Aug 2022 12:22:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58918 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229605AbiHTQWR (ORCPT ); Sat, 20 Aug 2022 12:22:17 -0400 Received: from mail-qt1-x82e.google.com (mail-qt1-x82e.google.com [IPv6:2607:f8b0:4864:20::82e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2205049B6E; Sat, 20 Aug 2022 09:22:16 -0700 (PDT) Received: by mail-qt1-x82e.google.com with SMTP id w28so5293080qtc.7; Sat, 20 Aug 2022 09:22:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=HdF47He1obOe/z7bMXQ34hAtRqKgVWrmpmNXRVq1I2Q=; b=lnk0sD/AncNwd1SYmUouALIuAEoEvIHTr0TQVTmUKeZQoyow3fZLEADMSPcp2Xg2TY yUEoTwD7Ig6pnmuqszVaavBxf1QbySDwnRqI7TVlU/OwKWjFYz+bQUVLV+AK1WVdC8/8 8hzzq+HZ1/QQmjiYGjiYjLyia7DPducVEpUBS663yU4KU5LuWdtS0bRywOBnWcRT4OSN HihJAr6tRGto0v6085YJAW9ql3BW3HmoOwLT0Qt6jQ7ORmSqacsV7O+WWQhKOd9shR8g WTfct6hTPv88t0wCwJIYzNq+0iuVj5Bbkt04GEaMHWuMxciALdhJdQf1kMVv/ZhT9o44 fvEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=HdF47He1obOe/z7bMXQ34hAtRqKgVWrmpmNXRVq1I2Q=; b=ctbRAmKGDL49II8LvIS0BWuGVvoUye++lOtG926agnh/03liG/NjfbB4k55P6CjZlI arYeyfa5G0MRY6xO0E0YOmRtvp0n9kFsHjPQLHscqPhl+ILAq76bVvPy7RIUhs72nbkK gvLWGovtpWBp/6veCLFtSGxFw2ui/3ntf2Cp+gQR/ZwuWyggdz6NTdmhJ9mS1Nv22mYJ IVPWznTLf3Cj6nthJT37iuDaxqcrQ7bTtZHG8S3Raqb9/L8GASnfTFbg+iseVvb2ezIA F6YMuDv8xlNHBxmHMgWElqPru/1FW1V7eMTEgHjQZzUmY3IrjFjUq3oOAU4NrrWJXKjn xZxQ== X-Gm-Message-State: ACgBeo2BD2hP9dGZLjAdkEHOJCZ9vhvYtWs3DusikEaEJ08uSWmIShkU vnH75NhKVZ6T9+B3nSZOS+FPK8tq8aptPksipe8= X-Received: by 2002:ac8:7dd0:0:b0:344:afc1:b11d with SMTP id c16-20020ac87dd0000000b00344afc1b11dmr2057287qte.195.1661012535127; Sat, 20 Aug 2022 09:22:15 -0700 (PDT) MIME-Version: 1.0 References: <3fd11489356b1c73a3d7b4bd9dec7e12c9fe8788.1660934107.git.mazziesaccount@gmail.com> <795d16f2-4dee-7492-4a87-e928020efebe@fi.rohmeurope.com> <0823a6e8-b325-78c5-d060-c5f9442e3df8@fi.rohmeurope.com> In-Reply-To: From: Andy Shevchenko Date: Sat, 20 Aug 2022 19:21:39 +0300 Message-ID: Subject: Re: [PATCH v3 08/14] iio: bmg160_core: Simplify using devm_regulator_*get_enable() To: Matti Vaittinen Cc: "Vaittinen, Matti" , Jonathan Cameron , Lars-Peter Clausen , Miaoqian Lin , Andy Shevchenko , Xiang wangx , linux-iio , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Aug 20, 2022 at 1:05 PM Matti Vaittinen wrote: > On 8/20/22 10:18, Andy Shevchenko wrote: > > On Sat, Aug 20, 2022 at 9:48 AM Vaittinen, Matti > > wrote: > >> On 8/20/22 09:25, Andy Shevchenko wrote: > >>> On Sat, Aug 20, 2022 at 9:19 AM Vaittinen, Matti > >>> wrote: > >>>> On 8/20/22 02:30, Andy Shevchenko wrote: > >>>>> On Fri, Aug 19, 2022 at 10:21 PM Matti Vaittinen > >>>>> wrote: > >>> > >>> What did I miss? > >> > >> >>>> struct bmg160_data *data; > >> >>>> struct iio_dev *indio_dev; > >> > >> This does already violate the rule. > > > > Indeed, I am reading this with an MTA that has True Type fonts, and I > > can't see it at the first glance. But this breaks that rule slightly > > while your added line breaks it significantly. > > Yes. As I said, I think the reverse xmas tree rule is not too well > justified. Bunch of the subsystems do not really follow it, nor did this > function. Yet, as I said, I can move the array to the first line in the > function when I respin the series.. You still can do better in _your_ series, right? > >>>>> this case you even can move it out of the function, so we will see > >>>>> clearly that this is (not a hidden) global variable. > >>>> > >>>> Here I do disagree with you. Moving the array out of the function makes > >>>> it _much_ less obvious it is not used outside this function. Reason for > >>>> making is "static const" is to allow the data be placed in read-only > >>>> area (thanks to Guenter who originally gave me this tip). > > Just wanted to correct - it was Sebastian Reichel, not Guenter who > explained me why doing local static const arrays is better than plain const. Did he suggest putting it inside the function? > >>> "static" in C language means two things (that's what come to my mind): > >>> - for functions this tells that a function is not used outside of the module; > >>> - for variables that it is a _global_ variable. > >>> > >>> Hiding static inside functions is not a good coding practice since it > >>> hides scope of the variable. > >> > >> For const arrays the static in function does make sense. Being able to > >> place the data in read-only areas do help with the memory on limited > >> systems. > > > > I'm not sure we are on the same page. I do not object to the "const" > > part and we are _not_ talking about that. > > Maybe the explanation by Sebastian here can put us on the same page: > https://lore.kernel.org/all/20190502073539.GB7864@localhost.localdomain/ > https://lore.kernel.org/all/322fa765ddd72972aba931c706657661ca685afa.camel@fi.rohmeurope.com/ Again, you are too focused on "const", I'm talking about "static". The above doesn't clear a bit regarding why you hide the global variable inside a function. I don't see either Sebastian's clear point on this. > >>> And if you look into the kernel code, I > >>> believe the use you are proposing is in minority. > >> > >> I don't know about the statistics. What I know is that we do have a > >> technical benefits when we use static const arrays instead of non static > >> ones in the functions. I do also believe placing the variables in blocks > >> is a good practice. > > > > Yes, and global variables are better to be seen as global variables. > > > >> I tend to agree with you that using local, non const statics has > >> pitfalls - but the pitfalls do not really apply with const ones. You > >> know the value and have no races. Benefit is that just by seeing that no > >> pointer is returned you can be sure that no "sane code" uses the data > >> outside the function it resides. > > > > Putting a global variable (const or non-const) to the function will > > hide its scope and it is prone to getting two variables with the same > > or very similar names with quite different semantics. > > I don't see how moving something from a local block to a global scope > does make conflicts less of an issue? You may add a static variable inside each function in the same module and name it "foo" and there will be no conflict, but when you read the code your brain will be spoiled. This is simply _bad code practice_. I don't know how else I can explain this to you. > On the contrary, it makes things > worse as then the moved variable will collide with any other variable in > any of the functions in the whole file. Having the array as function > local static makes the naming collisions to be issue only if another > global variable has the same name. Again, you missed my point. I'm talking about reading and analysing the code. Otherwise (good) compiler should spill a lot of warnings in case you have global vs. local naming collision. > And if that happened - the chances > are code would still be correct as the function here is clearly intended > to use the local one. If someone really later adds a global with the > same name - and uses the global in this function - then he should have > noted we have local variable with same name. Additionally - such user > would be using terribly bad name for a global variable. > > Please note that scope of the function local static variable is limited > to function even if the life-time is not just the life-time of a function. Nope. The RO section might be very well flashed into ROM, so... > > That's why it's > > really not good practice. I would rather see it outside of the > > function _esp_ because it's static const. > > Sorry, I really don't agree with your reasoning here. :( So, whom should we listen to here? Because bad code is bad code. And this is code above. -- With Best Regards, Andy Shevchenko