Message ID | 20241205080633.2623142-2-quic_srichara@quicinc.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | mailbox: tmelite-qmp: Introduce QCOM TMEL QMP mailbox driver | expand |
On 05/12/2024 09:06, Sricharan R wrote: > From: Sricharan Ramabadhran <quic_srichara@quicinc.com> > > This binding describes the component responsible for communication > between the TME-L server based subsystems (Q6) and the TME-L client > (APPSS/BTSS/AUDIOSS), used for security services like secure image > authentication, enable/disable efuses, crypto services. Each client > in the SoC has its own block of message RAM and IRQ for communication > with the TME-L SS. The protocol used to communicate in the message RAM > is known as Qualcomm Messaging Protocol (QMP). This is RFC, so only limited review follows. I will review more once this is ready for review. > > Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com> > --- > .../bindings/mailbox/qcom,tmelite-qmp.yaml | 70 +++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml > > diff --git a/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml > new file mode 100644 > index 000000000000..1f2b3e02b894 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mailbox/qcom,tmelite-qmp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm TMELITE IPCC channel > + > +maintainers: > + - Sricharan Ramabadhran <quic_srichara@quicinc.com> > + > +description: > + This binding describes the component responsible for communication Describe the hardware, not the binding. > + between the TME-L server based subsystems (Q6) and the TME-L client > + (APPSS/BTSS/AUDIOSS), used for security services like secure image > + authentication, enable/disable efuses, crypto services. Each client > + in the SoC has its own block of message RAM and IRQ for communication > + with the TME-L SS. The protocol used to communicate in the message RAM > + is known as Qualcomm Messaging Protocol (QMP). > + > +properties: > + compatible: > + items: > + - enum: > + - qcom,ipq5424-tmelite-qmp > + - const: qcom,tmelite-qmp Drop generic compatible. > + > + reg: > + maxItems: 1 > + description: > + The base address and size of the message RAM for this client's > + communication with the TMELITE core Drop obvious description. Same everywhere else. > + > + interrupts: > + maxItems: 1 > + description: > + Should specify the TMELITE message IRQ for this client > + > + mboxes: > + maxItems: 1 > + description: > + Reference to the mailbox representing the outgoing doorbell in APCS for > + this client, as described in mailbox/mailbox.txt > + > + "#mbox-cells": > + const: 2 > + description: > + The first cell is the client-id, and the second cell is the signal-id. I guess that's the only description not stating obvious. > + > +required: > + - compatible > + - reg > + - interrupts > + - mboxes > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + tmel_qmp: qmp@32090000 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "qcom,ipq5424-tmelite-qmp", "qcom,tmelite-qmp"; Use 4 spaces for example indentation. > + reg = <0x32090000 0x2000>; > +... Best regards, Krzysztof
On 12/5/2024 1:42 PM, Krzysztof Kozlowski wrote: > On 05/12/2024 09:06, Sricharan R wrote: >> From: Sricharan Ramabadhran <quic_srichara@quicinc.com> >> >> This binding describes the component responsible for communication >> between the TME-L server based subsystems (Q6) and the TME-L client >> (APPSS/BTSS/AUDIOSS), used for security services like secure image >> authentication, enable/disable efuses, crypto services. Each client >> in the SoC has its own block of message RAM and IRQ for communication >> with the TME-L SS. The protocol used to communicate in the message RAM >> is known as Qualcomm Messaging Protocol (QMP). > > This is RFC, so only limited review follows. I will review more once > this is ready for review. > Thanks. Once i get the design/approach confirmed, will post the V1. >> >> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com> >> --- >> .../bindings/mailbox/qcom,tmelite-qmp.yaml | 70 +++++++++++++++++++ >> 1 file changed, 70 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml >> >> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml >> new file mode 100644 >> index 000000000000..1f2b3e02b894 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml >> @@ -0,0 +1,70 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mailbox/qcom,tmelite-qmp.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm TMELITE IPCC channel >> + >> +maintainers: >> + - Sricharan Ramabadhran <quic_srichara@quicinc.com> >> + >> +description: >> + This binding describes the component responsible for communication > > > Describe the hardware, not the binding. ho ok, will fix and move the hardware description here. > >> + between the TME-L server based subsystems (Q6) and the TME-L client >> + (APPSS/BTSS/AUDIOSS), used for security services like secure image >> + authentication, enable/disable efuses, crypto services. Each client >> + in the SoC has its own block of message RAM and IRQ for communication >> + with the TME-L SS. The protocol used to communicate in the message RAM >> + is known as Qualcomm Messaging Protocol (QMP). >> + >> +properties: >> + compatible: >> + items: >> + - enum: >> + - qcom,ipq5424-tmelite-qmp >> + - const: qcom,tmelite-qmp > > Drop generic compatible. ok > > >> + >> + reg: >> + maxItems: 1 >> + description: >> + The base address and size of the message RAM for this client's >> + communication with the TMELITE core > > Drop obvious description. Same everywhere else. ok > >> + >> + interrupts: >> + maxItems: 1 >> + description: >> + Should specify the TMELITE message IRQ for this client >> + >> + mboxes: >> + maxItems: 1 >> + description: >> + Reference to the mailbox representing the outgoing doorbell in APCS for >> + this client, as described in mailbox/mailbox.txt >> + >> + "#mbox-cells": >> + const: 2 >> + description: >> + The first cell is the client-id, and the second cell is the signal-id. > > I guess that's the only description not stating obvious. > ok >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - mboxes >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + tmel_qmp: qmp@32090000 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > ho ok, will fix > >> + compatible = "qcom,ipq5424-tmelite-qmp", "qcom,tmelite-qmp"; > > Use 4 spaces for example indentation. > ok Regards, Sricharan
On 05/12/2024 10:17, Sricharan Ramabadhran wrote: > > > On 12/5/2024 1:42 PM, Krzysztof Kozlowski wrote: >> On 05/12/2024 09:06, Sricharan R wrote: >>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com> >>> >>> This binding describes the component responsible for communication >>> between the TME-L server based subsystems (Q6) and the TME-L client >>> (APPSS/BTSS/AUDIOSS), used for security services like secure image >>> authentication, enable/disable efuses, crypto services. Each client >>> in the SoC has its own block of message RAM and IRQ for communication >>> with the TME-L SS. The protocol used to communicate in the message RAM >>> is known as Qualcomm Messaging Protocol (QMP). >> >> This is RFC, so only limited review follows. I will review more once >> this is ready for review. >> > Thanks. Once i get the design/approach confirmed, will post the V1. Not a v1, but next version. This was v1 already, because we do not count from 0. Please use b4 to avoid such mistakes. Best regards, Krzysztof
On Thu, Dec 05, 2024 at 01:36:32PM +0530, Sricharan R wrote: > From: Sricharan Ramabadhran <quic_srichara@quicinc.com> > > This binding describes the component responsible for communication > between the TME-L server based subsystems (Q6) and the TME-L client This should start by explaining what is TME-L. > (APPSS/BTSS/AUDIOSS), used for security services like secure image > authentication, enable/disable efuses, crypto services. Each client > in the SoC has its own block of message RAM and IRQ for communication > with the TME-L SS. The protocol used to communicate in the message RAM > is known as Qualcomm Messaging Protocol (QMP). > > Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com> > --- > .../bindings/mailbox/qcom,tmelite-qmp.yaml | 70 +++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml > > diff --git a/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml > new file mode 100644 > index 000000000000..1f2b3e02b894 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mailbox/qcom,tmelite-qmp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm TMELITE IPCC channel So, TME-L or TMELITE or TME-LITE? > + > +maintainers: > + - Sricharan Ramabadhran <quic_srichara@quicinc.com> > + > +description: > + This binding describes the component responsible for communication It's already a description of the binding, no need to repeat the obvious. > + between the TME-L server based subsystems (Q6) and the TME-L client > + (APPSS/BTSS/AUDIOSS), used for security services like secure image > + authentication, enable/disable efuses, crypto services. Each client > + in the SoC has its own block of message RAM and IRQ for communication > + with the TME-L SS. The protocol used to communicate in the message RAM > + is known as Qualcomm Messaging Protocol (QMP). > + > +properties: > + compatible: > + items: > + - enum: > + - qcom,ipq5424-tmelite-qmp > + - const: qcom,tmelite-qmp > + > + reg: > + maxItems: 1 > + description: > + The base address and size of the message RAM for this client's > + communication with the TMELITE core > + > + interrupts: > + maxItems: 1 > + description: > + Should specify the TMELITE message IRQ for this client Just should? This is a very relaxed constraint. Just "the message IRQ for the client" sounds better. > + > + mboxes: > + maxItems: 1 > + description: > + Reference to the mailbox representing the outgoing doorbell in APCS for > + this client, as described in mailbox/mailbox.txt > + > + "#mbox-cells": > + const: 2 > + description: > + The first cell is the client-id, and the second cell is the signal-id. > + > +required: > + - compatible > + - reg > + - interrupts > + - mboxes > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + tmel_qmp: qmp@32090000 { > + compatible = "qcom,ipq5424-tmelite-qmp", "qcom,tmelite-qmp"; > + reg = <0x32090000 0x2000>; > + interrupts = <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>; > + mboxes = <&apcs_glb 20>; > + #mbox-cells = <2>; > + }; > + > +... > -- > 2.34.1 >
On 12/5/2024 5:13 PM, Krzysztof Kozlowski wrote: > On 05/12/2024 10:17, Sricharan Ramabadhran wrote: >> >> >> On 12/5/2024 1:42 PM, Krzysztof Kozlowski wrote: >>> On 05/12/2024 09:06, Sricharan R wrote: >>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com> >>>> >>>> This binding describes the component responsible for communication >>>> between the TME-L server based subsystems (Q6) and the TME-L client >>>> (APPSS/BTSS/AUDIOSS), used for security services like secure image >>>> authentication, enable/disable efuses, crypto services. Each client >>>> in the SoC has its own block of message RAM and IRQ for communication >>>> with the TME-L SS. The protocol used to communicate in the message RAM >>>> is known as Qualcomm Messaging Protocol (QMP). >>> >>> This is RFC, so only limited review follows. I will review more once >>> this is ready for review. >>> >> Thanks. Once i get the design/approach confirmed, will post the V1. > > Not a v1, but next version. This was v1 already, because we do not count > from 0. Please use b4 to avoid such mistakes. Ok, understand. Regards, Sricharan
On 12/5/2024 5:54 PM, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 01:36:32PM +0530, Sricharan R wrote: >> From: Sricharan Ramabadhran <quic_srichara@quicinc.com> >> >> This binding describes the component responsible for communication >> between the TME-L server based subsystems (Q6) and the TME-L client > > This should start by explaining what is TME-L. > ok, will change. >> (APPSS/BTSS/AUDIOSS), used for security services like secure image >> authentication, enable/disable efuses, crypto services. Each client >> in the SoC has its own block of message RAM and IRQ for communication >> with the TME-L SS. The protocol used to communicate in the message RAM >> is known as Qualcomm Messaging Protocol (QMP). >> >> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com> >> --- >> .../bindings/mailbox/qcom,tmelite-qmp.yaml | 70 +++++++++++++++++++ >> 1 file changed, 70 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml >> >> diff --git a/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml >> new file mode 100644 >> index 000000000000..1f2b3e02b894 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml >> @@ -0,0 +1,70 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mailbox/qcom,tmelite-qmp.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm TMELITE IPCC channel > > So, TME-L or TMELITE or TME-LITE? > ok, will use TME-L in all places. >> + >> +maintainers: >> + - Sricharan Ramabadhran <quic_srichara@quicinc.com> >> + >> +description: >> + This binding describes the component responsible for communication > > It's already a description of the binding, no need to repeat the obvious. > ok >> + between the TME-L server based subsystems (Q6) and the TME-L client >> + (APPSS/BTSS/AUDIOSS), used for security services like secure image >> + authentication, enable/disable efuses, crypto services. Each client >> + in the SoC has its own block of message RAM and IRQ for communication >> + with the TME-L SS. The protocol used to communicate in the message RAM >> + is known as Qualcomm Messaging Protocol (QMP). >> + >> +properties: >> + compatible: >> + items: >> + - enum: >> + - qcom,ipq5424-tmelite-qmp >> + - const: qcom,tmelite-qmp >> + >> + reg: >> + maxItems: 1 >> + description: >> + The base address and size of the message RAM for this client's >> + communication with the TMELITE core >> + >> + interrupts: >> + maxItems: 1 >> + description: >> + Should specify the TMELITE message IRQ for this client > > Just should? This is a very relaxed constraint. Just "the message IRQ > for the client" sounds better. > ok, will reword. Regards, Sricharan
diff --git a/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml new file mode 100644 index 000000000000..1f2b3e02b894 --- /dev/null +++ b/Documentation/devicetree/bindings/mailbox/qcom,tmelite-qmp.yaml @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mailbox/qcom,tmelite-qmp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm TMELITE IPCC channel + +maintainers: + - Sricharan Ramabadhran <quic_srichara@quicinc.com> + +description: + This binding describes the component responsible for communication + between the TME-L server based subsystems (Q6) and the TME-L client + (APPSS/BTSS/AUDIOSS), used for security services like secure image + authentication, enable/disable efuses, crypto services. Each client + in the SoC has its own block of message RAM and IRQ for communication + with the TME-L SS. The protocol used to communicate in the message RAM + is known as Qualcomm Messaging Protocol (QMP). + +properties: + compatible: + items: + - enum: + - qcom,ipq5424-tmelite-qmp + - const: qcom,tmelite-qmp + + reg: + maxItems: 1 + description: + The base address and size of the message RAM for this client's + communication with the TMELITE core + + interrupts: + maxItems: 1 + description: + Should specify the TMELITE message IRQ for this client + + mboxes: + maxItems: 1 + description: + Reference to the mailbox representing the outgoing doorbell in APCS for + this client, as described in mailbox/mailbox.txt + + "#mbox-cells": + const: 2 + description: + The first cell is the client-id, and the second cell is the signal-id. + +required: + - compatible + - reg + - interrupts + - mboxes + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + tmel_qmp: qmp@32090000 { + compatible = "qcom,ipq5424-tmelite-qmp", "qcom,tmelite-qmp"; + reg = <0x32090000 0x2000>; + interrupts = <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>; + mboxes = <&apcs_glb 20>; + #mbox-cells = <2>; + }; + +...