[PATCH 1/2] clk: meson: meson8b: register the built-in reset controller

Sat Jul 22 11:30:44 PDT 2017

On Tue, Jul 18, 2017 at 12:22 AM, Martin Blumenstingl
<martin.blumenstingl at googlemail.com> wrote:
> Hi Rob,
>
> On Mon, Jul 17, 2017 at 7:13 PM, Rob Herring <robh at kernel.org> 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 at endlessm.com>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl at googlemail.com>
>>> ---
>>>  .../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 at 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