LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Maulik Shah <mkshah@codeaurora.org>
Cc: Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Rajendra Nayak <rnayak@codeaurora.org>,
Matthias Kaehlcke <mka@chromium.org>,
Evan Green <evgreen@chromium.org>,
Stephen Boyd <swboyd@chromium.org>,
Lina Iyer <ilina@codeaurora.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFT PATCH 2/9] drivers: qcom: rpmh-rsc: Document the register layout better
Date: Wed, 11 Mar 2020 08:27:44 -0700 [thread overview]
Message-ID: <CAD=FV=W5RGwu=ywtM7aCv3H-EpJ1i23S0awgVc8QtOsXtige4w@mail.gmail.com> (raw)
In-Reply-To: <285d3315-7558-d9f6-fe65-24d8ad07949d@codeaurora.org>
Hi,
On Wed, Mar 11, 2020 at 2:35 AM Maulik Shah <mkshah@codeaurora.org> wrote:
>
> Hi Doug,
>
> On 3/7/2020 5:29 AM, Douglas Anderson wrote:
> > Perhaps it's just me, it took a really long time to understand what
> > the register layout of rpmh-rsc was just from the #defines.
> i don't understand why register layout is so important for you to understand?
>
> besides, i think all required registers are properly named with #define
>
> for e.g.
> /* Register offsets */
> #define RSC_DRV_IRQ_ENABLE 0x00
> #define RSC_DRV_IRQ_STATUS 0x04
> #define RSC_DRV_IRQ_CLEAR 0x08
>
> now when you want to enable/disable irq in driver code, its pretty simple to figure out
> that we need to read/write at RSC_DRV_IRQ_ENABLE offset.
It's not the specific layout that's important to me. It's the
relationships between everything. In other words:
a) one rpmh-rsc contains some registers and then a whole bunch of TCS blocks
b) one TCS block contains some registers and space for up to 16 commands.
c) each command has a handful of registers
Nothing describes this in the existing code--you've gotta read through
all the code and figure out how it's reading/writing registers to
figure it out.
> this seems unnecessary change to me, can you please drop this when you spin v2?
In v2 I'll replace this with the prose below. Personally I find this
inferior to the struct definitions which are easier to read
at-a-glance, but it seems like it would be less controversial...
/*
* Here's a high level overview of how all the registers in RPMH work
* together:
*
* - The main rpmh-rsc address is the base of a register space that can
* be used to find overall configuration of the hardware
* (DRV_PRNT_CHLD_CONFIG). Also found within the rpmh-rsc register
* space are all the TCS blocks. The offset of the TCS blocks is
* specified in the device tree by "qcom,tcs-offset" and used to
* compute tcs_base.
* - TCS blocks come one after another. Type, count, and order are
* specified by the device tree as "qcom,tcs-config".
* - Each TCS block has some registers, then space for up to 16 commands.
* Note that though address space is reserved for 16 commands, fewer
* might be present. See ncpt (num cmds per TCS).
* - The first TCS block is special in that it has registers to control
* interrupts (RSC_DRV_IRQ_XXX). Space for these registers is reserved
* in all TCS blocks to make the math easier, but only the first one
* matters.
*/
I'll also move the documentation of "irq_clear", "cmd_wait_for_cmpl",
"status", and "cmd_enable" next to the respective #defines.
next prev parent reply other threads:[~2020-03-11 15:28 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-06 23:59 [RFT PATCH 0/9] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Douglas Anderson
2020-03-06 23:59 ` [RFT PATCH 1/9] drivers: qcom: rpmh-rsc: Clean code reading/writing regs/cmds Douglas Anderson
2020-03-11 8:47 ` Maulik Shah
2020-03-11 15:03 ` Doug Anderson
2020-03-11 16:17 ` Matthias Kaehlcke
2020-03-11 19:30 ` Stephen Boyd
2020-03-06 23:59 ` [RFT PATCH 2/9] drivers: qcom: rpmh-rsc: Document the register layout better Douglas Anderson
2020-03-11 9:35 ` Maulik Shah
2020-03-11 15:27 ` Doug Anderson [this message]
2020-03-11 18:49 ` Evan Green
2020-03-11 20:08 ` Stephen Boyd
2020-03-11 22:35 ` Doug Anderson
2020-03-06 23:59 ` [RFT PATCH 3/9] drivers: qcom: rpmh-rsc: Fold tcs_ctrl_write() into its single caller Douglas Anderson
2020-03-11 9:50 ` Maulik Shah
2020-03-06 23:59 ` [RFT PATCH 4/9] drivers: qcom: rpmh-rsc: Remove get_tcs_of_type() abstraction Douglas Anderson
2020-03-11 12:02 ` Maulik Shah
2020-03-06 23:59 ` [RFT PATCH 5/9] drivers: qcom: rpmh-rsc: A lot of comments Douglas Anderson
2020-03-06 23:59 ` [RFT PATCH 6/9] drivers: qcom: rpmh-rsc: Only use "tcs_in_use" for ACTIVE_ONLY Douglas Anderson
2020-03-11 0:33 ` Doug Anderson
2020-03-06 23:59 ` [RFT PATCH 7/9] drivers: qcom: rpmh-rsc: Warning if tcs_write() used for non-active Douglas Anderson
2020-03-06 23:59 ` [RFT PATCH 8/9] drivers: qcom: rpmh-rsc: spin_lock_irqsave() for tcs_invalidate() Douglas Anderson
2020-03-06 23:59 ` [RFT PATCH 9/9] drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire Douglas Anderson
2020-03-11 0:35 ` Doug Anderson
2020-03-11 9:48 ` [RFT PATCH 0/9] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Maulik Shah
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='CAD=FV=W5RGwu=ywtM7aCv3H-EpJ1i23S0awgVc8QtOsXtige4w@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=evgreen@chromium.org \
--cc=ilina@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mka@chromium.org \
--cc=mkshah@codeaurora.org \
--cc=rnayak@codeaurora.org \
--cc=swboyd@chromium.org \
--subject='Re: [RFT PATCH 2/9] drivers: qcom: rpmh-rsc: Document the register layout better' \
/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
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).