| Message ID | 20240424-smsm-mbox-v1-1-555f3f442841@z3ntu.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | Support mailbox interface in qcom,smsm driver | expand |
On Wed, Apr 24, 2024 at 07:21:51PM +0200, Luca Weiss wrote: > The qcom,ipc-N properties are essentially providing a reference to a > mailbox, so allow using the mboxes property to do the same in a more > structured way. Can we mark qcom,ipc-N as deprecated then? > Since multiple SMSM hosts are supported, we need to be able to provide > the correct mailbox for each host. The old qcom,ipc-N properties map to > the mboxes property by index, starting at 0 since that's a valid SMSM > host also. > > The new example shows how an smsm node with just qcom,ipc-3 should be > specified with the mboxes property. > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > .../devicetree/bindings/soc/qcom/qcom,smsm.yaml | 48 ++++++++++++++++++---- > 1 file changed, 40 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > index db67cf043256..b12589171169 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > @@ -33,6 +33,13 @@ properties: > specifier of the column in the subscription matrix representing the local > processor. > > + mboxes: > + minItems: 1 > + maxItems: 5 Need to define what each entry is. > + description: > + Reference to the mailbox representing the outgoing doorbell in APCS for > + this client. > + > '#size-cells': > const: 0 > > @@ -98,15 +105,18 @@ required: > - '#address-cells' > - '#size-cells' > > -anyOf: > +oneOf: > - required: > - - qcom,ipc-1 > - - required: > - - qcom,ipc-2 > - - required: > - - qcom,ipc-3 > - - required: > - - qcom,ipc-4 > + - mboxes > + - anyOf: > + - required: > + - qcom,ipc-1 > + - required: > + - qcom,ipc-2 > + - required: > + - qcom,ipc-3 > + - required: > + - qcom,ipc-4 > > additionalProperties: false > > @@ -136,3 +146,25 @@ examples: > #interrupt-cells = <2>; > }; > }; > + # Example using mboxes property > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + shared-memory { > + compatible = "qcom,smsm"; > + #address-cells = <1>; > + #size-cells = <0>; > + mboxes = <0>, <0>, <0>, <&apcs 19>; > + > + apps@0 { > + reg = <0>; > + #qcom,smem-state-cells = <1>; > + }; > + > + wcnss@7 { > + reg = <7>; > + interrupts = <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>; > + interrupt-controller; > + #interrupt-cells = <2>; > + }; > + }; > > -- > 2.44.0 >
On Donnerstag, 25. April 2024 18:17:15 MESZ Rob Herring wrote: > On Wed, Apr 24, 2024 at 07:21:51PM +0200, Luca Weiss wrote: > > The qcom,ipc-N properties are essentially providing a reference to a > > mailbox, so allow using the mboxes property to do the same in a more > > structured way. > > Can we mark qcom,ipc-N as deprecated then? Yes, that should be ok. Will also send a similar change to the other bindings that support both qcom,ipc and mboxes. > > > Since multiple SMSM hosts are supported, we need to be able to provide > > the correct mailbox for each host. The old qcom,ipc-N properties map to > > the mboxes property by index, starting at 0 since that's a valid SMSM > > host also. > > > > The new example shows how an smsm node with just qcom,ipc-3 should be > > specified with the mboxes property. > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > --- > > .../devicetree/bindings/soc/qcom/qcom,smsm.yaml | 48 ++++++++++++++++++---- > > 1 file changed, 40 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > > index db67cf043256..b12589171169 100644 > > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > > @@ -33,6 +33,13 @@ properties: > > specifier of the column in the subscription matrix representing the local > > processor. > > > > + mboxes: > > + minItems: 1 > > + maxItems: 5 > > Need to define what each entry is. The entry is (description from qcom,ipc-N) "the outgoing ipc bit used for signaling the N:th remote processor." So you want me to add 5 times e.g. - the IPC mailbox used for signaling the 0th remote processor - the IPC mailbox used for signaling the 1st remote processor etc? I don't really have any extra knowledge on smsm to be able to write something better there.. Also what are your thoughts on this binding vs the alternative I wrote in the cover letter? I'm not really happy about how the properties are represented. Regards Luca > > > + description: > > + Reference to the mailbox representing the outgoing doorbell in APCS for > > + this client. > > + > > '#size-cells': > > const: 0 > > > > @@ -98,15 +105,18 @@ required: > > - '#address-cells' > > - '#size-cells' > > > > -anyOf: > > +oneOf: > > - required: > > - - qcom,ipc-1 > > - - required: > > - - qcom,ipc-2 > > - - required: > > - - qcom,ipc-3 > > - - required: > > - - qcom,ipc-4 > > + - mboxes > > + - anyOf: > > + - required: > > + - qcom,ipc-1 > > + - required: > > + - qcom,ipc-2 > > + - required: > > + - qcom,ipc-3 > > + - required: > > + - qcom,ipc-4 > > > > additionalProperties: false > > > > @@ -136,3 +146,25 @@ examples: > > #interrupt-cells = <2>; > > }; > > }; > > + # Example using mboxes property > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + shared-memory { > > + compatible = "qcom,smsm"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + mboxes = <0>, <0>, <0>, <&apcs 19>; > > + > > + apps@0 { > > + reg = <0>; > > + #qcom,smem-state-cells = <1>; > > + }; > > + > > + wcnss@7 { > > + reg = <7>; > > + interrupts = <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + }; > > >
Hi Rob, Any feedback on the below topic? Regards Luca On Donnerstag, 25. April 2024 20:54:40 MESZ Luca Weiss wrote: > On Donnerstag, 25. April 2024 18:17:15 MESZ Rob Herring wrote: > > On Wed, Apr 24, 2024 at 07:21:51PM +0200, Luca Weiss wrote: > > > The qcom,ipc-N properties are essentially providing a reference to a > > > mailbox, so allow using the mboxes property to do the same in a more > > > structured way. > > > > Can we mark qcom,ipc-N as deprecated then? > > Yes, that should be ok. Will also send a similar change to the other bindings > that support both qcom,ipc and mboxes. > > > > > > Since multiple SMSM hosts are supported, we need to be able to provide > > > the correct mailbox for each host. The old qcom,ipc-N properties map to > > > the mboxes property by index, starting at 0 since that's a valid SMSM > > > host also. > > > > > > The new example shows how an smsm node with just qcom,ipc-3 should be > > > specified with the mboxes property. > > > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > > --- > > > .../devicetree/bindings/soc/qcom/qcom,smsm.yaml | 48 ++++++++++++++++++---- > > > 1 file changed, 40 insertions(+), 8 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > > > index db67cf043256..b12589171169 100644 > > > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml > > > @@ -33,6 +33,13 @@ properties: > > > specifier of the column in the subscription matrix representing the local > > > processor. > > > > > > + mboxes: > > > + minItems: 1 > > > + maxItems: 5 > > > > Need to define what each entry is. > > The entry is (description from qcom,ipc-N) > > "the outgoing ipc bit used for signaling the N:th remote processor." > > So you want me to add 5 times e.g. > > - the IPC mailbox used for signaling the 0th remote processor > - the IPC mailbox used for signaling the 1st remote processor > > etc? I don't really have any extra knowledge on smsm to be able to write > something better there.. > > Also what are your thoughts on this binding vs the alternative I wrote > in the cover letter? I'm not really happy about how the properties are > represented. > > Regards > Luca > > > > > > > + description: > > > + Reference to the mailbox representing the outgoing doorbell in APCS for > > > + this client. > > > + > > > '#size-cells': > > > const: 0 > > > > > > @@ -98,15 +105,18 @@ required: > > > - '#address-cells' > > > - '#size-cells' > > > > > > -anyOf: > > > +oneOf: > > > - required: > > > - - qcom,ipc-1 > > > - - required: > > > - - qcom,ipc-2 > > > - - required: > > > - - qcom,ipc-3 > > > - - required: > > > - - qcom,ipc-4 > > > + - mboxes > > > + - anyOf: > > > + - required: > > > + - qcom,ipc-1 > > > + - required: > > > + - qcom,ipc-2 > > > + - required: > > > + - qcom,ipc-3 > > > + - required: > > > + - qcom,ipc-4 > > > > > > additionalProperties: false > > > > > > @@ -136,3 +146,25 @@ examples: > > > #interrupt-cells = <2>; > > > }; > > > }; > > > + # Example using mboxes property > > > + - | > > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > > + > > > + shared-memory { > > > + compatible = "qcom,smsm"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + mboxes = <0>, <0>, <0>, <&apcs 19>; > > > + > > > + apps@0 { > > > + reg = <0>; > > > + #qcom,smem-state-cells = <1>; > > > + }; > > > + > > > + wcnss@7 { > > > + reg = <7>; > > > + interrupts = <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>; > > > + interrupt-controller; > > > + #interrupt-cells = <2>; > > > + }; > > > + }; > > > > > > >
On 15/05/2024 17:06, Luca Weiss wrote: > Hi Rob, > > Any feedback on the below topic? Can be explained in description, like mboxes: description: Each entry corresponds to one remote processor maxItems: 5 Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml index db67cf043256..b12589171169 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smsm.yaml @@ -33,6 +33,13 @@ properties: specifier of the column in the subscription matrix representing the local processor. + mboxes: + minItems: 1 + maxItems: 5 + description: + Reference to the mailbox representing the outgoing doorbell in APCS for + this client. + '#size-cells': const: 0 @@ -98,15 +105,18 @@ required: - '#address-cells' - '#size-cells' -anyOf: +oneOf: - required: - - qcom,ipc-1 - - required: - - qcom,ipc-2 - - required: - - qcom,ipc-3 - - required: - - qcom,ipc-4 + - mboxes + - anyOf: + - required: + - qcom,ipc-1 + - required: + - qcom,ipc-2 + - required: + - qcom,ipc-3 + - required: + - qcom,ipc-4 additionalProperties: false @@ -136,3 +146,25 @@ examples: #interrupt-cells = <2>; }; }; + # Example using mboxes property + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + shared-memory { + compatible = "qcom,smsm"; + #address-cells = <1>; + #size-cells = <0>; + mboxes = <0>, <0>, <0>, <&apcs 19>; + + apps@0 { + reg = <0>; + #qcom,smem-state-cells = <1>; + }; + + wcnss@7 { + reg = <7>; + interrupts = <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>; + interrupt-controller; + #interrupt-cells = <2>; + }; + };
The qcom,ipc-N properties are essentially providing a reference to a mailbox, so allow using the mboxes property to do the same in a more structured way. Since multiple SMSM hosts are supported, we need to be able to provide the correct mailbox for each host. The old qcom,ipc-N properties map to the mboxes property by index, starting at 0 since that's a valid SMSM host also. The new example shows how an smsm node with just qcom,ipc-3 should be specified with the mboxes property. Signed-off-by: Luca Weiss <luca@z3ntu.xyz> --- .../devicetree/bindings/soc/qcom/qcom,smsm.yaml | 48 ++++++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-)