From: Evan Green <evgreen@chromium.org>
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>
Subject: Re: [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver
Date: Wed, 9 Oct 2019 09:01:22 -0700 [thread overview]
Message-ID: <CAE=gft6SmWH3-Td-mZZPn-3=EzwexEdYTR00z5NCP-X1sspihA@mail.gmail.com> (raw)
In-Reply-To: <5d9d3ed4.1c69fb81.5a936.2b18@mx.google.com>
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
next prev parent reply other threads:[~2019-10-09 16:02 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-08 23:45 [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver Stephen Boyd
2019-10-08 23:45 ` [PATCH v2 1/2] soc: qcom: llcc: Name regmaps to avoid collisions Stephen Boyd
2019-10-09 15:25 ` Evan Green
2019-10-08 23:45 ` [PATCH v2 2/2] soc: qcom: llcc: Move regmap config to local variable Stephen Boyd
2019-10-08 23:55 ` [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver Bjorn Andersson
2019-10-09 1:58 ` Stephen Boyd
2019-10-09 16:01 ` Evan Green [this message]
2019-10-09 17:46 ` Bjorn Andersson
2019-10-09 17:59 ` Evan Green
2019-10-10 3:57 ` Bjorn Andersson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAE=gft6SmWH3-Td-mZZPn-3=EzwexEdYTR00z5NCP-X1sspihA@mail.gmail.com' \
--to=evgreen@chromium.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=swboyd@chromium.org \
--cc=vnkgutta@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).