Re: [PATCH v1 6/7] dt-bindings: clock: Describe MT6685 PM/Clock IC Clock Controller

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 24/10/25 18:25, Conor Dooley ha scritto:
> On Fri, Oct 24, 2025 at 10:33:00AM +0200, AngeloGioacchino Del Regno wrote:
>> Add bindings to describe the SCK_TOP clock controller embedded
>> in the MT6685 IC, reachable over the SPMI bus.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>
>> NOTE: This does not contain any example because the MT6685 RTC
>>        will be added to the mfd binding for MediaTek SPMI PMICs
>>        and examples will be there.
>>
>> ** For reviewing purposes, this is how the example will look like: **
>>
>>    - |
>>      #include <dt-bindings/interrupt-controller/irq.h>
>>      #include <dt-bindings/spmi/spmi.h>
>>
>>      spmi {
>>        #address-cells = <2>;
>>        #size-cells = <0>;
>>
>>        pmic@9 {
>>          compatible = "mediatek,mt6363";
>>          reg = <0x9 SPMI_USID>;
>>          interrupts = <9 1 IRQ_TYPE_LEVEL_HIGH>;
>>          interrupt-controller;
>>          #address-cells = <1>;
>>          #interrupt-cells = <3>;
>>          #size-cells = <0>;
>>
>>          clock-controller@514 {
>>            compatible = "mediatek,mt6685-sck-top";
>>            reg = <0x514>;
>>            #clock-cells = <1>;
>>          };
>>
>>          rtc@580 {
>>            compatible = "mediatek,mt6685-rtc";
>>            reg = <0x580>;
>>            interrupts = <9 0 IRQ_TYPE_LEVEL_HIGH>;
>>          };
>>        };
>>      };
>>
>>   .../bindings/clock/mediatek,mt6685-clock.yaml | 37 +++++++++++++++++++
>>   .../dt-bindings/clock/mediatek,mt6685-clock.h | 17 +++++++++
>>   2 files changed, 54 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt6685-clock.yaml
>>   create mode 100644 include/dt-bindings/clock/mediatek,mt6685-clock.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt6685-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt6685-clock.yaml
>> new file mode 100644
>> index 000000000000..5407ebf2f3b5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt6685-clock.yaml
>> @@ -0,0 +1,37 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/mediatek,mt6685-clock.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MediaTek Clock Controller for MT6685 SPMI PM/Clock IC
>> +
>> +maintainers:
>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> +
>> +description: |
>> +  The clock architecture in MediaTek PMICs+Clock ICs is structured like below:
>> +  Crystal(XO) or Internal ClockGen -->
>> +          dividers -->
>> +                      muxes
>> +                           -->
>> +                              clock gate
> 
> Is this the intended formatting? Looks weird with "dividers" being
> unaligned with the --> above it, but maybe you were just going for x
> number of spaces?
> 

Yeah I was just going for x number of spaces, otherwise that may become a bit
"too long"...

>> +
>> +  The device nodes provide clock gate control in different IP blocks.
> 
> I think this is more understandable as "This device provides clock gate
> control", if this sck-top is only doing gating. Otherwise, not clear if
> the dividers and muxes are here or elsewhere.

-> Datasheets are incomplete (sad-face-here) <-

Most of the information here is grabbed from more than one downstream kernel
for more than one SoC/device, and assembled together.

The XO/clockgen and dividers are not in SCKTOP - those should be partially in
the "TOP" portion (yeah, there's a top and a sck-top), and partially in another
block that controls only the clockgen.

I didn't want to implement those two, even though I almost precisely know how to
do that (and I did it in some local tests), because I could only gather partial
information and I didn't feel confident in upstreaming something that I'm not
entirely sure about.

Same goes for the MUX part: there's some here, some there, one in scktop as well
(but I didn't describe it because again incomplete info, and even downstream the
only mux in scktop seems to be unused).

So yeah - apart from one mux, anything before clock gate is elsewhere... I can
change that statement to the one you proposed, looks a bit better than what I
came up with, so thanks for that :-D

> 
>> +properties:
>> +  compatible:
>> +    const: mediatek,mt6685-sck-top
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - '#clock-cells'
>> +
>> +additionalProperties: false
>> diff --git a/include/dt-bindings/clock/mediatek,mt6685-clock.h b/include/dt-bindings/clock/mediatek,mt6685-clock.h
>> new file mode 100644
>> index 000000000000..acc5e2e15ce1
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/mediatek,mt6685-clock.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
>> +/*
>> + * Copyright (c) 2025 Collabora Ltd.
>> + *                    AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLK_MT6685_H
>> +#define _DT_BINDINGS_CLK_MT6685_H
>> +
>> +/* SCK_TOP_CKPDN */
>> +#define CLK_RTC_SEC_MCLK		0
>> +#define CLK_RTC_EOSC32			1
>> +#define CLK_RTC_SEC_32K			2
>> +#define CLK_RTC_MCLK			3
>> +#define CLK_RTC_32K			4
>> +
>> +#endif /* _DT_BINDINGS_CLK_MT6685_H */
>> -- 
>> 2.51.1
>>