Messages in this thread | | | Date | Sun, 24 Apr 2022 15:52:37 +0200 | Subject | Re: [PATCH] dt-bindings: soc: qcom: convert GLINK binding to yaml | From | David Heidelberg <> |
| |
Yeah, it seems there are several mistakes, I thought it was in better shape (4 months passed.. :( ).
About testing, currently I have limited ability since broken libfdt, which dt-schema use, doesn't work on new py3.10.
I'll send partly fixed version and get back to it, when I get more time and fixed environment for testing.
David
On 24/04/2022 15:36, Krzysztof Kozlowski wrote: > On 24/04/2022 12:16, David Heidelberg wrote: >> Convert Qualcomm GLINK binding to the yaml format. >> >> Signed-off-by: David Heidelberg <david@ixit.cz> >> --- >> v1: >> - remove quotes around qcom,intent >> - use additionalProperties > Thank you for your patch. There is something to discuss/improve. > >> .../bindings/soc/qcom/qcom,glink.txt | 94 ---------------- >> .../bindings/soc/qcom/qcom,glink.yaml | 103 ++++++++++++++++++ >> 2 files changed, 103 insertions(+), 94 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt >> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,glink.yaml >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt >> deleted file mode 100644 >> index 1214192847ac..000000000000 >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt >> +++ /dev/null >> @@ -1,94 +0,0 @@ >> -Qualcomm GLINK edge binding >> - >> -This binding describes a Qualcomm GLINK edge, a fifo based mechanism for >> -communication between subsystem-pairs on various Qualcomm platforms. Two types >> -of edges can be described by the binding; the GLINK RPM edge and a SMEM based >> -edge. >> - >> -- compatible: >> - Usage: required for glink-rpm >> - Value type: <stringlist> >> - Definition: must be "qcom,glink-rpm" >> - >> -- label: >> - Usage: optional >> - Value type: <string> >> - Definition: should specify the subsystem name this edge corresponds to. >> - >> -- interrupts: >> - Usage: required >> - Value type: <prop-encoded-array> >> - Definition: should specify the IRQ used by the remote processor to >> - signal this processor about communication related events >> - >> -- qcom,remote-pid: >> - Usage: required for glink-smem >> - Value type: <u32> >> - Definition: specifies the identifier of the remote endpoint of this edge >> - >> -- qcom,rpm-msg-ram: >> - Usage: required for glink-rpm >> - Value type: <prop-encoded-array> >> - Definition: handle to RPM message memory resource >> - >> -- mboxes: >> - Usage: required >> - Value type: <prop-encoded-array> >> - Definition: reference to the "rpm_hlos" mailbox in APCS, as described >> - in mailbox/mailbox.txt >> - >> -= GLINK DEVICES >> -Each subnode of the GLINK node represent function tied to a virtual >> -communication channel. The name of the nodes are not important. The properties >> -of these nodes are defined by the individual bindings for the specific function >> -- but must contain the following property: >> - >> -- qcom,glink-channels: >> - Usage: required >> - Value type: <stringlist> >> - Definition: a list of channels tied to this function, used for matching >> - the function to a set of virtual channels >> - >> -- qcom,intents: >> - Usage: optional >> - Value type: <prop-encoded-array> >> - Definition: a list of size,amount pairs describing what intents should >> - be preallocated for this virtual channel. This can be used >> - to tweak the default intents available for the channel to >> - meet expectations of the remote. >> - >> -= EXAMPLE >> -The following example represents the GLINK RPM node on a MSM8996 device, with >> -the function for the "rpm_request" channel defined, which is used for >> -regulators and root clocks. >> - >> - apcs_glb: mailbox@9820000 { >> - compatible = "qcom,msm8996-apcs-hmss-global"; >> - reg = <0x9820000 0x1000>; >> - >> - #mbox-cells = <1>; >> - }; >> - >> - rpm_msg_ram: memory@68000 { >> - compatible = "qcom,rpm-msg-ram"; >> - reg = <0x68000 0x6000>; >> - }; >> - >> - rpm-glink { >> - compatible = "qcom,glink-rpm"; >> - >> - interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>; >> - >> - qcom,rpm-msg-ram = <&rpm_msg_ram>; >> - >> - mboxes = <&apcs_glb 0>; >> - >> - rpm-requests { >> - compatible = "qcom,rpm-msm8996"; >> - qcom,glink-channels = "rpm_requests"; >> - >> - qcom,intents = <0x400 5 >> - 0x800 1>; >> - ... >> - }; >> - }; >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.yaml >> new file mode 100644 >> index 000000000000..12ccc875ff0f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.yaml >> @@ -0,0 +1,103 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/soc/qcom/qcom,glink.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: Qualcomm GLINK edge >> + >> +description: | >> + Qualcomm GLINK edge, a fifo based mechanism for communication between >> + subsystem-pairs on various Qualcomm platforms. Two types of edges can be >> + described by the binding; the GLINK RPM edge and a SMEM based. >> + >> +maintainers: >> + - Bjorn Andersson <bjorn.andersson@linaro.org> >> + >> +properties: >> + $nodename: >> + pattern: "^(rpm-)?glink(-edge)?$" >> + >> + compatible: >> + const: qcom,glink-rpm >> + >> + label: >> + $ref: /schemas/types.yaml#/definitions/string >> + description: should specify the subsystem name this edge corresponds to > Drop all this "should specify" and similar. It's making description > unnecessary long and complicated. This is therefore "Name of subsystem > this edge corresponds to". > >> + >> + interrupts: >> + description: > > No need for '>'. > >> + should specify the IRQ used by the remote processor to >> + signal this processor about communication related events >> + maxItems: 1 >> + >> + mboxes: >> + description: > > The same. > >> + reference to the "rpm_hlos" mailbox in APCS, as described >> + in mailbox/mailbox.txt > Skip the path (entire last part of sentence). > >> + >> + qcom,remote-pid: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: specifies the identifier of the remote endpoint of this edge >> + >> + qcom,rpm-msg-ram: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: RPM message memory resource >> + >> +additionalProperties: > This is a bit confusing... why it's not part of properties? Looks like > mix-up schema and actually you miss proper additionalProperties. > >> + type: object > Eh? Object here? I don't think you tested your bindings. If you test, > you will notice several errors. > >> + properties: >> + qcom,glink-channels: > These go to children. > >> + $ref: /schemas/types.yaml#/definitions/string > Isn't this a list of strings? > >> + description: > >> + a list of channels tied to this function, used for matching >> + the function to a set of virtual channels >> + >> + qcom,intents: >> + $ref: /schemas/types.yaml#/definitions/uint32-array > Should > >> + description: > >> + a list of size, amount pairs describing what intents should >> + be preallocated for this virtual channel. This can be used >> + to tweak the default intents available for the channel to >> + meet expectations of the remote > min/max items. This looks like matrix, but it seems is not used that way. :( > >> + >> + required: >> + - qcom,glink-channels >> + >> + additionalProperties: false >> + >> +required: > compatible > >> + - interrupts >> + - mboxes >> + - qcom,smem > What's this? > >> + - qcom,local-pid > This is also something new. This does not look like proper bindings > conversion, or you copied some other bindings as template. You need to > really go through it and clean it up. > >> + - qcom,remote-pid > Isn't this conflicting with part below? > >> + >> +anyOf: >> + - required: >> + - qcom,remote-pid >> + - required: >> + - qcom,rpm-msg-ram >> + >> +examples: >> + # The following example represents the GLINK RPM node on a MSM8996 device, >> + # with the function for the "rpm_request" channel defined, which >> + # is used for regulators and root clocks. >> + - | >> + rpm-glink { >> + compatible = "qcom,glink-rpm"; >> + >> + interrupts = <0 168 1>; >> + >> + qcom,rpm-msg-ram = <&rpm_msg_ram>; >> + > Remove unneeded blank lines between properties. > >> + mboxes = <&apcs_glb 0>; >> + >> + rpm-requests { >> + compatible = "qcom,rpm-msm8996"; >> + qcom,glink-channels = "rpm_requests"; >> + >> + qcom,intents = <0x400 5 >> + 0x800 1>; >> + }; >> + }; > > Best regards, > Krzysztof
-- David Heidelberg Consultant Software Engineer
Signal: +420 608 26 33 76 Matrix: @okias:matrix.org
| |