lkml.org 
[lkml]   [2019]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes
    On Tue 22 Jan 15:46 PST 2019, Doug Anderson wrote:

    > Hi,
    >
    > On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
    > <bjorn.andersson@linaro.org> wrote:
    > >
    > > Add the ADSP and CDSP nodes for PAS-based remoteproc, supporting booting
    > > these cores on e.g. the MTP, and enable the same for the MTP.
    > >
    > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
    > > ---
    > >
    > > Changes since v2:
    > > - New patch
    > >
    > > arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 8 ++++
    > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 58 +++++++++++++++++++++++++
    > > 2 files changed, 66 insertions(+)
    >
    > It's a bit of a nit of mine that if it's not totally obvious what
    > acronyms mean that they should be spelled out in places that use them.
    >
    > In this case I believe ADSP is the Audio DSP. Is CDSP the Camera DSP? ...or ?
    >

    C as in Compute. I'll spell these out as I respin the series.

    >
    > > + adsp_pas: remoteproc-adsp {
    > > + compatible = "qcom,sdm845-adsp-pas";
    > > +
    > > + interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>,
    > > + <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
    > > + <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
    > > + <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
    > > + <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
    > > + interrupt-names = "wdog", "fatal", "ready",
    > > + "handover", "stop-ack";
    > > +
    > > + clocks = <&xo_board>;
    > > + clock-names = "xo";
    >
    > I've found that nearly all the places that refer to xo_board are wrong
    > and should actually point to '<&rpmhcc RPMH_CXO_CLK>'. Maybe yours
    > should too?
    >

    Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
    cxo (or cxo2) pad of the SoC. So you're definitely right in that this
    should be referencing the actual 19.2MHz clock.

    We've kept referring to this as xo_board, as we don't handle probe
    deferral when gcc will probe earlier than rpmcc in the boot and for
    other non-clock drivers the fear of actually hitting 0 on the refcounter
    for this (you don't want to disable the cxo while running the system).

    I'll give it a spin with appropriate reference and see what happens, I
    think this should either be changed or documented in the commit message.

    Thanks,
    Bjorn

    \
     
     \ /
      Last update: 2019-01-23 01:26    [W:4.501 / U:25.420 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site