Re: [PATCH v1 1/2] coresight: bindings for debug module
From: Leo Yan
Date: Mon Feb 27 2017 - 10:35:18 EST
Hi Mike,
On Mon, Feb 27, 2017 at 12:37:45PM +0000, Mike Leach wrote:
> Hi Leo
>
> On 23 February 2017 at 01:57, Leo Yan <leo.yan@xxxxxxxxxx> wrote:
> > According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
> > Chapter 'Part H: External debug', the CPU can integrate debug module
> > and it can support self-hosted debug and external debug. Especially
> > for supporting self-hosted debug, this means the program can access
> > the debug module from mmio region; and usually the mmio region is
> > integrated with coresight.
> >
> > So add document for binding debug component, includes binding to two
> > clocks, one is apb clock for bus and another is debug clock for debug
> > module self; and also need specify the CPU node which the debug module
> > is dedicated to specific CPU.
> >
> > Signed-off-by: Leo Yan <leo.yan@xxxxxxxxxx>
> > ---
> > .../devicetree/bindings/arm/coresight-debug.txt | 39 ++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt
> >
> > diff --git a/Documentation/devicetree/bindings/arm/coresight-debug.txt b/Documentation/devicetree/bindings/arm/coresight-debug.txt
> > new file mode 100644
> > index 0000000..6e03e9b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/coresight-debug.txt
> > @@ -0,0 +1,39 @@
> > +* CoreSight Debug Component:
> > +
> > +CoreSight debug component are compliant with the ARMv8 architecture reference
> > +manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The external debug
> > +module is mainly used for two modes: self-hosted debug and external debug, and
> > +it can be accessed from mmio region from Coresight and eventually the debug
> > +module connects with CPU for debugging. And the debug module provides
> > +sample-based profiling extension, which can be used to sample CPU program
> > +counter, secure state and exception level, etc; usually every CPU has one
> > +dedicated debug module to be connected.
> > +
> > +Required properties:
> > +
> > +- compatible : should be
> > + * "arm,coresight-debug", "arm,primecell"; supplemented with
> > + "arm,primecell" as driver is using the AMBA bus interface.
> > +
> > +- reg : physical base address and length of the register set.
> > +
> > +- clocks : the clocks associated to this component.
> > +
> > +- clock-names : the name of the clocks referenced by the code. Since we are
> > + using the AMBA framework, the name of the clock providing
> > + the interconnect should be "apb_pclk", and the debug module
> > + has an additional clock "dbg_clk", which is used to provide
> > + clock for debug module itself. Both clocks are mandatory.
> > +
>
> Perhaps I am misunderstanding the nature of the .dts files, but I'm
> puzzled about the dbg_clk. I cannot see anything in the architecture
> or normal A53 implementation that mentions this.
> To access external debug from the core/external debug agent then we do
> need the APB clock, but the interface between the debug logic and the
> processor core is clocked by the internal CPU clocks.
You are right and sorry I may introduce confusion at here.
When I wrote this doc for binding, I assumed every debug logic has
its own clock source. This is because there have one register
ACPU_SC_PDBGUP_MBIST with eight bits, every bit is used to control
every CPU's DBGPWRDUP signal. I wrongly understood this bit is used
for debug logic clock gating.
I read CA53 TRM, the DBGPWRDUP signal actually is used between power
controller and CPU so can ensure CPU can be safely enter and exit low
power states; and "The EDPRSR.PU bit reflects the value of this
DBGPWRDUP signal". So I made mistake here and the bits in
ACPU_SC_PDBGUP_MBIST is no matter with debug logic clocks.
I will remove the "dbg_clk" from binding doc. Please correct me if my
understanding is still wrong or blur.
> > +- cpu : the cpu phandle the debug module is affined to. When omitted
> > + the source is considered to belong to CPU0.
> > +
> > +Example:
> > +
> > + debug@f6590000 {
> > + compatible = "arm,coresight-debug","arm,primecell";
> > + reg = <0 0xf6590000 0 0x1000>;
> > + clocks = <&sys_ctrl HI6220_CS_ATB>, <&acpu_ctrl HI6220_ACPU_DBG_CLK0>;
> > + clock-names = "apb_pclk", "dbg_clk";
> > + cpu = <&cpu0>;
> > + };
> > --
> > 2.7.4
> >
>
> When I was looking at clocks for trace on the HiKey board I noted:
> HI6220_CS_DAPB -- which I assumed was the debug APB clock.
> HI6220_CS_ATB - which I assumed was the ATB (trace bus) clock.
The clock HI6220_CS_DAPB is a mux; and after went through the spec for
register definition, the clock driver misses the dapb gate clock in
its system controller 'sysctrl'; And I checked this bit is luckily
enabled by bootloaders (I have not checked it's enabled by ARM-TF or
UEFI) so the driver can access debug module registers.
diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index 553d083..a4ac75e 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c
@@ -134,6 +134,7 @@ static struct hisi_gate_clock hi6220_separated_gate_clks_sys[] __initdata = {
{ HI6220_UART4_PCLK, "uart4_pclk", "uart4_src", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 8, 0, },
{ HI6220_SPI_CLK, "spi_clk", "clk_150m", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 9, 0, },
{ HI6220_TSENSOR_CLK, "tsensor_clk", "clk_bus", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 12, 0, },
+ { HI6220_DAPB_CLK, "dapb_clk", "cs_dapb", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 18, 0, },
{ HI6220_MMU_CLK, "mmu_clk", "ddrc_axi1", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x240, 11, 0, },
{ HI6220_HIFI_SEL, "hifi_sel", "hifi_src", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x270, 0, 0, },
{ HI6220_MMC0_SYSPLL, "mmc0_syspll", "syspll", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x270, 1, 0, },
Thanks,
Leo Yan