| Message ID | 20220914124552.16964-3-johnson.wang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce MediaTek frequency hopping driver | expand |
Il 14/09/22 14:45, Johnson Wang ha scritto: > Add the new binding documentation for MediaTek frequency hopping > and spread spectrum clocking control. > > Co-developed-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > Signed-off-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > --- > .../bindings/arm/mediatek/mediatek,fhctl.yaml | 47 +++++++++++++++++++ > 1 file changed, 47 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..7b0fd0889bb6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml > @@ -0,0 +1,47 @@ > +# 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@mediatek.com> > + > +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,mt8186-fhctl > + > + reg: > + maxItems: 1 There are still a few issues in this binding that I can immediately see... > + > + clocks: MT8195 has 23 PLLs, MT8186 has 14, but perhaps in the future we may see something more than that on some newer SoC, so... clocks: maxItems: 30 > + description: Phandles of the PLL with FHCTL hardware capability. > + > + mediatek,hopping-ssc-percents: > + description: The percentage of spread spectrum clocking for one PLL. > + $ref: /schemas/types.yaml#/definitions/uint32 This is an array, so... $ref: /schemas/types.yaml#/definitions/uint32-array ...also, maxItems? and you should also specify: default: 0 <- because, by default, SSC is disabled minimum: 0 <- because this is the minimum accepted value Regards, Angelo > + maximum: 8 > + > +required: > + - compatible > + - reg > + - clocks > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/mt8186-clk.h> > + fhctl: fhctl@1000ce00 { > + compatible = "mediatek,mt8186-fhctl"; > + reg = <0x1000c000 0xe00>; > + clocks = <&apmixedsys CLK_APMIXED_MSDCPLL>; > + mediatek,hopping-ssc-percents = <3>; > + };
Hi Angelo, Thanks for your review. On Wed, 2022-09-14 at 15:46 +0200, AngeloGioacchino Del Regno wrote: > Il 14/09/22 14:45, Johnson Wang ha scritto: > > Add the new binding documentation for MediaTek frequency hopping > > and spread spectrum clocking control. > > > > Co-developed-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > > Signed-off-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > --- > > .../bindings/arm/mediatek/mediatek,fhctl.yaml | 47 > > +++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam > > l > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam > > l > > new file mode 100644 > > index 000000000000..7b0fd0889bb6 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam > > l > > @@ -0,0 +1,47 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > https://urldefense.com/v3/__http://devicetree.org/schemas/arm/mediatek/mediatek,fhctl.yaml*__;Iw!!CTRNKA9wMg0ARbw!3sumdhtrK5Ah5_rfIilgm4UUmnwkkqMpc3r_ZfkLfsXsLn-_AKm9ZokhJGD1Fl-gJpckAKHZh-jNVW64KRU8Duv1kg$ > > > > +$schema: > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!3sumdhtrK5Ah5_rfIilgm4UUmnwkkqMpc3r_ZfkLfsXsLn-_AKm9ZokhJGD1Fl-gJpckAKHZh-jNVW64KRWMb8jIsw$ > > > > + > > +title: MediaTek frequency hopping and spread spectrum clocking > > control > > + > > +maintainers: > > + - Edward-JW Yang <edward-jw.yang@mediatek.com> > > + > > +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,mt8186-fhctl > > + > > + reg: > > + maxItems: 1 > > There are still a few issues in this binding that I can immediately > see... > > > + > > + clocks: > > MT8195 has 23 PLLs, MT8186 has 14, but perhaps in the future we may > see > something more than that on some newer SoC, so... > > clocks: > maxItems: 30 May I add "minItems: 1" to clocks property? Without this, dt_binding_check will fail because we don't have enough clocks in the example. (Both MT8195 and MT8186 don't have 30 PLLs) > > > + description: Phandles of the PLL with FHCTL hardware > > capability. > > + > > + mediatek,hopping-ssc-percents: > > + description: The percentage of spread spectrum clocking for > > one PLL. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > This is an array, so... > $ref: /schemas/types.yaml#/definitions/uint32-array > > ...also, maxItems? As you know, mediatek,hopping-ssc-percents property is used to specify ssc rate for matching clocks. If we have to add maxItems, I think we should specify the same value as clocks property. Is my understanding wrong? Thanks! BRs, Johnson Wang > > and you should also specify: > > default: 0 <- because, by default, SSC is disabled > minimum: 0 <- because this is the minimum accepted value > > > Regards, > Angelo > > > + maximum: 8 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/mt8186-clk.h> > > + fhctl: fhctl@1000ce00 { > > + compatible = "mediatek,mt8186-fhctl"; > > + reg = <0x1000c000 0xe00>; > > + clocks = <&apmixedsys CLK_APMIXED_MSDCPLL>; > > + mediatek,hopping-ssc-percents = <3>; > > + }; > >
Il 15/09/22 06:00, Johnson Wang ha scritto: > Hi Angelo, > > Thanks for your review. > > On Wed, 2022-09-14 at 15:46 +0200, AngeloGioacchino Del Regno wrote: >> Il 14/09/22 14:45, Johnson Wang ha scritto: >>> Add the new binding documentation for MediaTek frequency hopping >>> and spread spectrum clocking control. >>> >>> Co-developed-by: Edward-JW Yang <edward-jw.yang@mediatek.com> >>> Signed-off-by: Edward-JW Yang <edward-jw.yang@mediatek.com> >>> Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> >>> --- >>> .../bindings/arm/mediatek/mediatek,fhctl.yaml | 47 >>> +++++++++++++++++++ >>> 1 file changed, 47 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam >>> l >>> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam >>> l >>> new file mode 100644 >>> index 000000000000..7b0fd0889bb6 >>> --- /dev/null >>> +++ >>> b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam >>> l >>> @@ -0,0 +1,47 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: >>> https://urldefense.com/v3/__http://devicetree.org/schemas/arm/mediatek/mediatek,fhctl.yaml*__;Iw!!CTRNKA9wMg0ARbw!3sumdhtrK5Ah5_rfIilgm4UUmnwkkqMpc3r_ZfkLfsXsLn-_AKm9ZokhJGD1Fl-gJpckAKHZh-jNVW64KRU8Duv1kg$ >>> >>> +$schema: >>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!3sumdhtrK5Ah5_rfIilgm4UUmnwkkqMpc3r_ZfkLfsXsLn-_AKm9ZokhJGD1Fl-gJpckAKHZh-jNVW64KRWMb8jIsw$ >>> >>> + >>> +title: MediaTek frequency hopping and spread spectrum clocking >>> control >>> + >>> +maintainers: >>> + - Edward-JW Yang <edward-jw.yang@mediatek.com> >>> + >>> +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,mt8186-fhctl >>> + >>> + reg: >>> + maxItems: 1 >> >> There are still a few issues in this binding that I can immediately >> see... >> >>> + >>> + clocks: >> >> MT8195 has 23 PLLs, MT8186 has 14, but perhaps in the future we may >> see >> something more than that on some newer SoC, so... >> >> clocks: >> maxItems: 30 > > May I add "minItems: 1" to clocks property? > Of course you can! Sorry for the incomplete advice here :-) > Without this, dt_binding_check will fail because we don't have enough > clocks in the example. (Both MT8195 and MT8186 don't have 30 PLLs) > >> >>> + description: Phandles of the PLL with FHCTL hardware >>> capability. >>> + >>> + mediatek,hopping-ssc-percents: >>> + description: The percentage of spread spectrum clocking for >>> one PLL. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >> >> This is an array, so... >> $ref: /schemas/types.yaml#/definitions/uint32-array >> >> ...also, maxItems? > > As you know, mediatek,hopping-ssc-percents property is used to specify > ssc rate for matching clocks. > > If we have to add maxItems, I think we should specify the same value > as clocks property. Is my understanding wrong? > Your understanding is right. The number of min/max items on the ssc percents will be the same as clocks. Regards, Angelo > > Thanks! > > BRs, > Johnson Wang >> >> and you should also specify: >> >> default: 0 <- because, by default, SSC is disabled >> minimum: 0 <- because this is the minimum accepted value >> >> >> Regards, >> Angelo >> >>> + maximum: 8 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - clocks >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/clock/mt8186-clk.h> >>> + fhctl: fhctl@1000ce00 { >>> + compatible = "mediatek,mt8186-fhctl"; >>> + reg = <0x1000c000 0xe00>; >>> + clocks = <&apmixedsys CLK_APMIXED_MSDCPLL>; >>> + mediatek,hopping-ssc-percents = <3>; >>> + }; >> >> >
On 14/09/2022 13:45, 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@mediatek.com> > Signed-off-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > --- > .../bindings/arm/mediatek/mediatek,fhctl.yaml | 47 +++++++++++++++++++ > 1 file changed, 47 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..7b0fd0889bb6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml Name of file matching compatible. > @@ -0,0 +1,47 @@ > +# 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@mediatek.com> > + > +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,mt8186-fhctl > + > + reg: > + maxItems: 1 > + > + clocks: > + description: Phandles of the PLL with FHCTL hardware capability. You need constraints here. > + > + mediatek,hopping-ssc-percents: That's not the correct unit suffix name. https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml > + description: The percentage of spread spectrum clocking for one PLL. > + $ref: /schemas/types.yaml#/definitions/uint32 Best regards, Krzysztof
Hi Krzysztof, Sorry for the late reply. Thanks for your review. On Sun, 2022-09-18 at 10:38 +0100, Krzysztof Kozlowski wrote: > On 14/09/2022 13:45, 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@mediatek.com> > > Signed-off-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > --- > > .../bindings/arm/mediatek/mediatek,fhctl.yaml | 47 > > +++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam > > l > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam > > l > > new file mode 100644 > > index 000000000000..7b0fd0889bb6 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yam > > l > > Name of file matching compatible. I will change file name in the next version. > > > @@ -0,0 +1,47 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > https://urldefense.com/v3/__http://devicetree.org/schemas/arm/mediatek/mediatek,fhctl.yaml*__;Iw!!CTRNKA9wMg0ARbw!3e-QEzJcF8ER6bi9pBinXakWUMDsurZPy-q69INHqalP_evj50meSBC7SN0ukZhfEmO-$ > > > > +$schema: > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!3e-QEzJcF8ER6bi9pBinXakWUMDsurZPy-q69INHqalP_evj50meSBC7SN0ukZLSg9mh$ > > > > + > > +title: MediaTek frequency hopping and spread spectrum clocking > > control > > + > > +maintainers: > > + - Edward-JW Yang <edward-jw.yang@mediatek.com> > > + > > +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,mt8186-fhctl > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + description: Phandles of the PLL with FHCTL hardware > > capability. > > You need constraints here. I will improve this in the next version. > > > + > > + mediatek,hopping-ssc-percents: > > That's not the correct unit suffix name. > https://urldefense.com/v3/__https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml__;!!CTRNKA9wMg0ARbw!3e-QEzJcF8ER6bi9pBinXakWUMDsurZPy-q69INHqalP_evj50meSBC7SN0ukaKEhFnN$ I will go with "mediatek,hopping-ssc-percent" in the next version. Thanks! BRs, Johnson Wang > > > > > + description: The percentage of spread spectrum clocking for > > one PLL. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > Best regards, > Krzysztof
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..7b0fd0889bb6 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,fhctl.yaml @@ -0,0 +1,47 @@ +# 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@mediatek.com> + +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,mt8186-fhctl + + reg: + maxItems: 1 + + clocks: + description: Phandles of the PLL with FHCTL hardware capability. + + mediatek,hopping-ssc-percents: + description: The percentage of spread spectrum clocking for one PLL. + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 8 + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/mt8186-clk.h> + fhctl: fhctl@1000ce00 { + compatible = "mediatek,mt8186-fhctl"; + reg = <0x1000c000 0xe00>; + clocks = <&apmixedsys CLK_APMIXED_MSDCPLL>; + mediatek,hopping-ssc-percents = <3>; + };