Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191008234505.222991-1-swboyd@chromium.org> <20191008235504.GN63675@minitux>
 <5d9d3ed4.1c69fb81.5a936.2b18@mx.google.com>
In-Reply-To: <5d9d3ed4.1c69fb81.5a936.2b18@mx.google.com>
From:   Evan Green <evgreen@chromium.org>
Date:   Wed, 9 Oct 2019 09:01:22 -0700
Message-ID: <CAE=gft6SmWH3-Td-mZZPn-3=EzwexEdYTR00z5NCP-X1sspihA@mail.gmail.com>
Subject: Re: [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver
To:     Stephen Boyd <swboyd@chromium.org>
Cc:     Bjorn Andersson <bjorn.andersson@linaro.org>,
        Andy Gross <agross@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Venkata Narendra Kumar Gutta <vnkgutta@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Oct 8, 2019 at 6:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Bjorn Andersson (2019-10-08 16:55:04)
> > On Tue 08 Oct 16:45 PDT 2019, Stephen Boyd wrote:
> > >     @@ drivers/soc/qcom/llcc-slice.c
> > >
> > >       static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
> > >
> > >     --static const struct regmap_config llcc_regmap_config = {
> > >     +-static struct regmap_config llcc_regmap_config = {
> > >      -        .reg_bits = 32,
> > >      -        .reg_stride = 4,
> > >      -        .val_bits = 32,
> > >     @@ drivers/soc/qcom/llcc-slice.c: static struct regmap *qcom_llcc_init_mmio(struct
> > >       {
> > >               struct resource *res;
> > >               void __iomem *base;
> > >     -+        static struct regmap_config llcc_regmap_config = {
> > >     ++        struct regmap_config llcc_regmap_config = {
> >
> > Now that this isn't static I like the end result better. Not sure about
> > the need for splitting it in two patches, but if Evan is happy I'll take
> > it.
> >
>
> Well I split it into bug fix and micro-optimization so backport choices
> can be made. But yeah, I hope Evan is happy enough to provide a
> reviewed-by tag!

It's definitely better without the static local since it no longer has
the cognitive trap, but I still don't really get why we're messing
with the global v. local aspect of it. We're now inconsistent with
every other caller of this function, and for what exactly? We've
traded some data space for a call to memset() and some instructions. I
would have thought anecdotally that memory was the cheaper thing (ie
cpu speeds stopped increasing awhile ago, but memory is still getting
cheaper).

But either way it's correct, so really it's fine if you ignore me :)
-Evan