Message-ID: <CAD=FV=WwTN_fswRcMLR3_=3_Sju50eV+LpDZsi+x5-7bu87Qew@mail.gmail.com>
Date:   Mon, 18 Jun 2018 10:42:25 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Raju P L S S S N <rplsssn@...eaurora.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andy Gross <andy.gross@...aro.org>,
        Lina Iyer <ilina@...eaurora.org>
Cc:     David Brown <david.brown@...aro.org>,
        linux-arm-msm@...r.kernel.org,
        "open list:ARM/QUALCOMM SUPPORT" <linux-soc@...r.kernel.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Evan Green <evgreen@...omium.org>,
        Matthias Kaehlcke <mka@...omium.org>
Subject: Re: [PATCH v11 00/10] drivers/qcom: add RPMH communication support

Hi,

On Mon, Jun 18, 2018 at 6:37 AM, Raju P L S S S N
<rplsssn@...eaurora.org> wrote:
> From: "Raju P.L.S.S.S.N" <rplsssn@...eaurora.org>

[ ... ]

> This set of patches add the ability for platform drivers to make use of shared
> resources in newer Qualcomm SoCs like SDM845. Resources that are shared between
> multiple processors in a SoC are generally controlled by a dedicated remote
> processor. The remote processor (Resource Power Manager or RPM in previous QCOM
> SoCs) receives requests for resource state from other processors using the
> shared resource, aggregates the request and applies the result on the shared
> resource. SDM845 advances this concept and uses h/w (hardened I/P) blocks for
> aggregating requests and applying the result on the resource. The resources
> could be clocks, regulators or bandwidth requests for buses. This new
> architecture is called RPM-hardened or RPMH in short.
>
> Since this communication mechanism is completely hardware driven without a
> processor intervention on the remote end, existing mechanisms like RPM-SMD are
> no longer useful. Also, there is no serialization of data or is data is written
> to a shared memory in this new format. The data used is different, unsigned 32
> bits are used for representing an address, data and header. Each resource's
> property is a unique u32 address and have pre-defined set of property specific
> valid values. A request that comprises of <header, addr, data> is sent by
> writing to a set of registers from Linux and transmitted to the remote slave
> through an internal bus. The remote end aggregates this request along with
> requests from other processors for the <addr> and applies the result.
>
> The hardware block that houses this functionality is called Resource State
> Coordinator or RSC. Inside the RSC are set of slots for sending RPMH requests
> called Trigger Commands Sets (TCS). The set of patches are for writing the
> requests into these TCSes and sending them to hardened IP blocks.
>
> The driver design is split into two components. The RSC driver housed in
> rpmh-rsc.c and the set of library functions in rpmh.c that frame the request and
> transmit it using the controller. This first set of patches allow a simple
> synchronous request to be made by the platform drivers. Future patches will add
> more functionality that cater to complex drivers and use cases.
>
> Please consider reviewing this patchset.
>
> v1: https://www.spinics.net/lists/devicetree/msg210980.html
> v2: https://lkml.org/lkml/2018/2/15/852
> v3: https://lkml.org/lkml/2018/3/2/801
> v4: https://lkml.org/lkml/2018/3/9/979
> v5: https://lkml.org/lkml/2018/4/5/480
> v6: https://lkml.org/lkml/2018/4/19/914
> v7: https://lkml.org/lkml/2018/5/2/779
> v8: https://lkml.org/lkml/2018/5/9/729
> v9: https://lkml.org/lkml/2018/5/24/530
> v10: https://lkml.org/lkml/2018/6/11/542
>
> Lina Iyer (10):
>   drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs
>   dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs
>   drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE
>   drivers: qcom: rpmh: add RPMH helper functions
>   drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS
>   drivers: qcom: rpmh-rsc: allow invalidation of sleep/wake TCS
>   drivers: qcom: rpmh: cache sleep/wake state requests
>   drivers: qcom: rpmh: allow requests to be sent asynchronously
>   drivers: qcom: rpmh: add support for batch RPMH request
>   drivers: qcom: rpmh-rsc: allow active requests from wake TCS
>
>  .../devicetree/bindings/soc/qcom/rpmh-rsc.txt      | 137 +++++
>  drivers/soc/qcom/Kconfig                           |  10 +
>  drivers/soc/qcom/Makefile                          |   4 +
>  drivers/soc/qcom/rpmh-internal.h                   | 114 ++++
>  drivers/soc/qcom/rpmh-rsc.c                        | 682 +++++++++++++++++++++
>  drivers/soc/qcom/rpmh.c                            | 512 ++++++++++++++++
>  drivers/soc/qcom/trace-rpmh.h                      |  82 +++
>  include/dt-bindings/soc/qcom,rpmh-rsc.h            |  14 +
>  include/soc/qcom/rpmh.h                            |  51 ++
>  include/soc/qcom/tcs.h                             |  56 ++
>  10 files changed, 1662 insertions(+)

I haven't reviewed every last line of this patch series but IMHO it's
been floating around for quite some time and it seems like it might be
wise to land it and then continue to make incremental improvements
atop it as needed.  What do others think of this plan?  This would
allow us to have functioning regulators / clocks on sdm845 atop this
driver which would unblock a lot of other stuff.

Note that to address a piece of review feedback from Stephen Boyd I've
posted <https://patchwork.kernel.org/patch/10472383/> atop this
series.  Assuming others like that, we should probably pick that too,
then we can land future patches that remove the boilerplate code from
the regulator and clock driver.

-Doug

Powered by blists - more mailing lists