linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: AngeloGioacchino Del Regno  <angelogioacchino.delregno@somainline.org>
Cc: Jassi Brar <jassisinghbrar@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Martin Botka <martin.botka@somainline.org>,
	~postmarketos/upstreaming@lists.sr.ht,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	jamipkettunen@somainline.org, Andy Gross <agross@kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V3 3/3] mailbox: qcom-apcs: Add SM6125 compatible
Date: Thu, 24 Jun 2021 19:02:44 -0500	[thread overview]
Message-ID: <YNUdJC1y8lWHg114@yoga> (raw)
In-Reply-To: <b0ed9294-e4e1-3185-d81f-63e6e8d45692@somainline.org>

On Thu 24 Jun 17:59 CDT 2021, AngeloGioacchino Del Regno wrote:

> Il 24/06/21 23:07, Bjorn Andersson ha scritto:
> > On Tue 22 Jun 09:36 CDT 2021, AngeloGioacchino Del Regno wrote:
[..]
> > > Hello Jassi, Bjorn
> > > 
> > > I've read the entire thread and I can't say that Jassi is entirely wrong
> > > but I also agree with Bjorn on this matter.
> > > 
> > > This driver is here to "simply" manage the register offset in the APCS
> > > IP, which is a pretty straightforward operation.
> > > If you check in this driver, you will see that there's not much
> > > duplication between the various qcom_apcs_ipc_data that we have for
> > > all the different SoCs.
> > > 
> > > Checking further, we can effectively reduce the amount of compatibles
> > > in this driver by simply removing some "duplicated" instances and in
> > > particular:
> > > ipq6018, ipq8074, msm8916, msm8994, msm8998, sdm660
> > > 
> > > and eventually replacing them with either of:
> > > - 8bits_apcs_data    qcom,apcs-apps-global-8bit
> > >                       qcom,apcs-kpss-global-8bit
> > 
> > I don't like those compatibles, simply because the binding is supposed
> > to describe the hardware block, not the fact that Linux _currently_ only
> > pokes this one register.
> > 
> 
> Since you've immediately misunderstood my naming, yeah, that wouldn't be
> the best thing to use as a compatible.
> 

Sorry, that was certainly not my intention.

> > We could probably "qcom,apss-global" as a catch-all for at least sc7180,
> > sc7280, sdm845, sm8150, sm8250 and sm8350.
> > 
> 
> Doesn't look like a bad idea, but if we want to *enforce* writing also
> the platform-specific compatible, I can see patch series going back
> and forth and getting refused because this will not be really understood
> by everyone, I think.
> In this case, if writing the platform compatible is something mandatory,
> the only way to really make sure to avoid losing time with reviews like
> "[...] here you have to write also the platform compatible", is to just
> keep the thing as it is.
> 

My understanding from the DT maintainers is that the dts would be
checked by the schema, but I suspect that you're right in that we might
have some back and forth on the DT binding, but I don't think that's a
big deal.

> > But look at 8996 and 8998, both named "something-hmss-something", with
> > different register layout. And a quick glance seems to indicate that
> > sdm660 isn't a hmss after all :/
> > 
> 
> Starting from the fact that I don't clearly remember what-when-why of
> my research done more than one year ago, I do remember that conclusion
> was that, in this regard, SDM630/660 were "mostly the same" as MSM8998.
> In any case, this is something that, at this point, is better get
> verified, maybe.
> 

Yeah, I presume that someone with the documentation would need to go
through each one of these and see what kind of grouping there might be.

And I also presume that there might be cases where the CPU clocks has
moved into the secure world, so that even though the hardware block is
the same the presence of a in-kernel clock driver in the implementation
might differ.


But just to clarify, I find it annoying having to sprinkle compatibles
all over the place every time I try to get a new board up. So I am not
against us trying to figure this out.

Regards,
Bjorn

  reply	other threads:[~2021-06-25  0:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-12  9:46 [PATCH V3 1/3] socinfo: Add missing SoC ID for SM6125 Martin Botka
2021-06-12  9:46 ` [PATCH V3 2/3] dt-bindings: mailbox: Add binding for sm6125 Martin Botka
2021-06-24 19:26   ` Rob Herring
2021-06-12  9:46 ` [PATCH V3 3/3] mailbox: qcom-apcs: Add SM6125 compatible Martin Botka
2021-06-21  4:03   ` Jassi Brar
2021-06-21 17:02     ` Martin Botka
2021-06-21 19:46     ` Rob Herring
2021-06-21 23:10       ` Jassi Brar
2021-06-21 23:19         ` Rob Herring
2021-06-21 23:35           ` Bjorn Andersson
2021-06-22  1:00             ` Jassi Brar
2021-06-22  2:27               ` Bjorn Andersson
2021-06-22  3:34                 ` Jassi Brar
2021-06-22  3:52                   ` Bjorn Andersson
2021-06-22 14:36                     ` AngeloGioacchino Del Regno
2021-06-22 14:43                       ` Jassi Brar
2021-06-24 21:07                       ` Bjorn Andersson
2021-06-24 22:59                         ` AngeloGioacchino Del Regno
2021-06-25  0:02                           ` Bjorn Andersson [this message]
2021-06-26 16:50                             ` Jassi Brar
2021-06-22 14:37                     ` Jassi Brar

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=YNUdJC1y8lWHg114@yoga \
    --to=bjorn.andersson@linaro.org \
    --cc=agross@kernel.org \
    --cc=angelogioacchino.delregno@somainline.org \
    --cc=jamipkettunen@somainline.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=konrad.dybcio@somainline.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=martin.botka@somainline.org \
    --cc=robh+dt@kernel.org \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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).