On Tue, Jul 18, 2017 at 12:22 AM, Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> wrote: > Hi Rob, > > On Mon, Jul 17, 2017 at 7:13 PM, Rob Herring <robh@xxxxxxxxxx> wrote: >> On Wed, Jul 12, 2017 at 12:49:38AM +0200, Martin Blumenstingl wrote: >>> The clock controller has a reset controller embedded. A large part of >>> the HHI_SYS_CPU_CLK_CNTL0 register contains reset bits. However, most of >>> them are only used by u-boot (as these are probably dangerous to use >>> when Linux is running). >> >> u-boot reads DT's. The DT should be defined for the h/w, not what you >> want for Linux today. > OK, that matches with your comment regarding splitting the patch below > >>> Bits 27:24 are interesting though: these are the CPUx core soft reset >>> bits (bit 24 = CPU0 soft reset, bit 25 = CPU1 ...). >>> >>> This patch implements a reset controller for these bits. The reset >>> controller itself is registered early (through CLK_OF_DECLARE_DRIVER) >>> because it is neede very early in the boot process (to start the >>> secondary CPU cores). >>> >>> Other reset bits in the HHI_SYS_CPU_CLK_CNTL0 register, which are not >>> implemented by this patch (as these may never be used from within the >>> Linux kernel - and I don't want to add dead code): >>> - bit 30: L2 cache soft reset >>> - bit 29: AXI64to128 bridge (A5-to-MMC) soft reset (A5 interface) >>> - bit 28: SCU soft reset >>> - bit 18: A5 Global Reset >>> - bit 17: A5 AXI Soft Reset >>> - bit 16: A5 APB Soft Reset >>> - bit 15: GEN_DIV_SOFT_RESET >>> - bit 14: SOFT_RESET >>> >>> All information was taken from the public S805 Datasheet and Amlogic's >>> vendor GPL kernel sources. This patch is based on an earlier version >>> submitted by Carlo Caione. >>> >>> Suggested-by: Carlo Caione <carlo@xxxxxxxxxxxx> >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> >>> --- >>> .../bindings/clock/amlogic,meson8b-clkc.txt | 7 +- >> >> It's preferred to split bindings to a separate patch. Given all the >> commentary about Linux, I'd suggest you do that here (so the Linux >> details are gone from the binding patch). > you are right - I'll split this into a dt-bindings and a clk driver > patch and keep the commit messages appropriate for each patch > >>> drivers/clk/meson/Kconfig | 1 + >>> drivers/clk/meson/meson8b.c | 109 ++++++++++++++++++--- >>> drivers/clk/meson/meson8b.h | 1 + >>> 4 files changed, 105 insertions(+), 13 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt >>> index 606da38c0959..6f444e3867a0 100644 >>> --- a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt >>> +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt >>> @@ -16,18 +16,23 @@ Required Properties: >>> mapped region. >>> >>> - #clock-cells: should be 1. >>> +- #reset-cells: should be 1. >>> >>> Each clock is assigned an identifier and client nodes can use this identifier >>> to specify the clock which they consume. All available clocks are defined as >>> preprocessor macros in the dt-bindings/clock/meson8b-clkc.h header and can be >>> used in device tree sources. >>> >>> +The clock controller provides a (soft) reset line for each CPU core. Valid >>> +reset lines are 0, 1, 2 and 3 (one for each CPU core). >> >> I suspect you would have different numbering if you enumerate all the >> possible resets. Is it just this one register that has reset bits? If >> so, I'd suggest using the bit position as the cell values. If not, well, >> just enumerate them all. > there are more resets in this register area: > - "AXI64to128 bridge (A5-to-MMC) soft reset (MMC interface)": bit 30 > in HHI_SYS_CPU_CLK_CNTL1 > - SOFT_RESET: bit 15 in HHI_VID_CLK_CNTL > - some more bits with "reset" in their name (CLK_DIV_RESET = bit 17 in > HHI_VID_CLK_DIV, and four more in HHI_VID_DIVIDER_CNTL: > SOFT_RESET_POST, SOFT_RESET_PRE, RESET_N_POST, RESET_N_PRE) > > Carlo's original patch added #defines (RST_CORE0...RST_CORE3) for each > CPU reset bit. > Arnd (I CC'ed him in this mail) suggested to skip the defines, see [0] > - which is what I did with this patch. > > by looking at Amlogic's u-boot sources I can see that the following > reset bits are used: > - bits 30:28 and 18:16 from HHI_SYS_CPU_CLK_CNTL0 and bit 30 from > HHI_SYS_CPU_CLK_CNTL1 are used during Meson8b system suspend/wakeup > from suspend > - the four resets from HHI_VID_DIVIDER_CNTL are used to setup the "vid PLL div" > > that leaves the following reset bits unused (hidden somewhere deep in the code): > - HHI_VID_CLK_CNTL bit 15 is used in Meson6's u-boot code - but only > in some #if 0'ed blocks > - SOFT_RESET: bit 15 in HHI_VID_CLK_CNTL I actually found two "consumers" in Amlogic's Linux kernel sources - they are writing 0x88001 and 0x80003 these nice magic numbers (found for example in arch/arm/mach-meson8b/include/mach/tvregs.h) assert and de-assert the reset line (bit 15 = 0x8000) it also seems that I listed this bit twice (no idea why though) anyway, this bit will be part of the next version of this patch > - bit 15:14 in HHI_SYS_CPU_CLK_CNTL0 these really seem to be unused, so my next patch version will not contain these > so if you want I can add more reset bits I found to the driver and > introduce a #define for each reset. > I guess this would be find for Arnd as well (please speak up if it isn't). > however, I would only do this for the reset bits for which I could > find a consumer in Amlogic's vendor GPL kernel/u-boot code (as there's > not a lot documentation available, and I don't like guessing register > bit purposes based on a 12-character description from a stripped down > version of the SoC datasheet). > >>> + >>> Example: Clock controller node: >>> >>> clkc: clock-controller@c1104000 { >>> - #clock-cells = <1>; >>> compatible = "amlogic,meson8b-clkc"; >>> reg = <0xc1108000 0x4>, <0xc1104000 0x460>; >>> + #clock-cells = <1>; >>> + #reset-cells = <1>; >>> }; >>> >>> > > Regards, > Martin > > [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-December/390397.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html