Il 01/09/22 11:42, Krzysztof Kozlowski ha scritto:
On 01/09/2022 11:04, AngeloGioacchino Del Regno wrote:
Il 31/08/22 15:19, Krzysztof Kozlowski ha scritto:
On 31/08/2022 15:48, Johnson Wang wrote:
Add the new binding documentation for MediaTek frequency hopping
and spread spectrum clocking control.
Co-developed-by: Edward-JW Yang <edward-jw.yang@xxxxxxxxxxxx>
Signed-off-by: Edward-JW Yang <edward-jw.yang@xxxxxxxxxxxx>
Signed-off-by: Johnson Wang <johnson.wang@xxxxxxxxxxxx>
---
.../bindings/arm/mediatek/mediatek,fhctl.yaml | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml
new file mode 100644
index 000000000000..c5d76410538b
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/mediatek/mediatek,fhctl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek frequency hopping and spread spectrum clocking control
+
+maintainers:
+ - Edward-JW Yang <edward-jw.yang@xxxxxxxxxxxx>
+
+description: |
+ Frequency hopping control (FHCTL) is a piece of hardware that control
+ some PLLs to adopt "hopping" mechanism to adjust their frequency.
+ Spread spectrum clocking (SSC) is another function provided by this hardware.
+
+properties:
+ compatible:
+ const: mediatek,fhctl
You need SoC/device specific compatibles. Preferably only SoC specific,
without generic fallback, unless you can guarantee (while representing
MediaTek), that generic fallback will cover all of their SoCs?
+
+ reg:
+ maxItems: 1
+
+ mediatek,hopping-ssc-percents:
+ description: |
+ Determine the enablement of frequency hopping feature and the percentage
+ of spread spectrum clocking for PLLs.
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ items:
+ items:
+ - description: PLL id that is expected to enable frequency hopping.
So the clocks are indices from some specific, yet unnamed
clock-controller? This feels hacky. You should rather take here clock
phandles (1) or integrate it into specific clock controller (2). The
reason is that either your device does something on top of existing
clocks (option 1, thus it takes clock as inputs) or it modifies existing
clocks (option 2, thus it is integral part of clock-controller).
FHCTL is a MCU that handles (some, or all, depending on what's supported on the
SoC and what's needed by the board) PLL frequency setting, doing it in steps and
avoiding overshooting and other issues.
We had a pretty big conversation about this a while ago and the indices instead
of phandles is actually my fault, that happened because I initially proposed your
option 2 but then for a number of reasons we went with this kind of solution.
I know it's going to be a long read, but the entire conversation is on the list [1]
Sorry, but it's a hacky architecture where one device (which is a clock
provider) and second device have no relationship in hardware description
but both play with each other resources.
Yes, that's exactly how it is hardware-side. Except, just to be as clear as
possible, FHCTL plays with the clock provider, but *not* vice-versa.
That's simply not a proper
hardware description, so again:
1. If this is separate device (as you indicated), then it needs
expressing the dependencies and uses of other device resources.
Agreed. In this case, what about...
mediatek,hopping-ssc-percents = <&provider CLK_SOMEPLL 3>;
or would it be better to specify the clocks in a separated property?
clocks = <&provider CLK_SOMEPLL>, <&provider CLK_SOME_OTHER_PLL>;
mediatek,hopping-ssc-percents = <3>, <5>;
Thanks,
Angelo
2. If this is not a separate device, but integral part of clock
controller, then it would be fine but then probably should be child of
that device.
Best regards,
Krzysztof
