<pre>
Hi Rob,

Thanks for the reviews.

On Tue, 2023-09-19 at 11:46 -0500, Rob Herring wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Tue, Sep 19, 2023 at 03:21:50AM +0800, Jason-JH.Lin wrote:
> > Add mboxes to define a GCE loopping thread as a secure irq handler.
> > Add mediatek,event to define a GCE software event siganl as a
> secure
> > irq.
> >
> > These 2 properties are required for CMDQ secure driver.
> >
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> > .../mailbox/mediatek,gce-mailbox.yaml | 30
> +++++++++++++++----
> > 1 file changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git
> a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> > index cef9d7601398..5c9aebe83d2d 100644
> > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> > @@ -49,6 +49,21 @@ properties:
> > items:
> > - const: gce
> >
> > + mboxes:
> > + description:
> > + A mailbox channel used as a secure irq handler in normal
> world.
> > + Using mailbox to communicate with GCE to setup looping
> thread,
> > + it should have this property and a phandle, mailbox
> specifiers.
>
> All cases of 'mboxes' have a phandle and specifiers. No need to
> repeat
> that here.

OK, I'll remove it.

>
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
>
> Already has a type definition too. You need to define how many
> entries
> and what each entry is if more than one. IOW, the same thing as
> clocks,
> resets, interrupts, etc.

OK, I'll add the maximum number to 1 for this.

>
> > +
> > + mediatek,gce-events:
>
> This is used all over. It really needs a single definition which is
> then
> referenced by the users.

OK, I think it would defined it here since it is a GCE HW event signal.
I'll try to reference to other modules and make a definition for it.

>
> > + description:
> > + The event id which is mapping to a software event signal to
> gce.
> > + It is used as a secure irq for every secure gce threads.
> > + The event id is defined in the gce header
> > + include/dt-bindings/mailbox/mediatek,<chip>-gce.h of each
> chips.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > required:
> > - compatible
> > - "#mbox-cells"
> > @@ -71,20 +86,23 @@ additionalProperties: false
> >
> > examples:
> > - |
> > - #include <dt-bindings/clock/mt8173-clk.h>
> > + #include <dt-bindings/clock/mediatek,mt8188-clk.h>
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/mailbox/mediatek,mt8188-gce.h>
> >
> > soc {
> > #address-cells = <2>;
> > #size-cells = <2>;
> >
> > - gce: mailbox@10212000 {
> > - compatible = "mediatek,mt8173-gce";
> > - reg = <0 0x10212000 0 0x1000>;
> > - interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_LOW>;
> > + gce0: mailbox@10320000 {
> > + compatible = "mediatek,mt8188-gce";
>
> Are these new properties only for mt8188? If so, then you need a
> schema
> saying that. If not, then this is an unnecessary change to the
> example.

No I think all SoC can add these properties if they have secure
requirement as mt8188. So I'll revert this unnecessary change to the
example.

>
> > + reg = <0 0x10320000 0 0x4000>;
> > + interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH 0>;
> > #mbox-cells = <2>;
> > - clocks = <&infracfg CLK_INFRA_GCE>;
> > + clocks = <&infracfg_ao CLK_INFRA_AO_GCE>;
> > clock-names = "gce";
> > + mboxes = <&gce0 15 CMDQ_THR_PRIO_1>;
>
> The provider is also a consumer?

We'll use a mbox channel for receiving GCE signal in the secure mailbox
driver. So I think it is a provider and also a consumer.

Regards,
Jason-JH.Lin

>
> > + mediatek,gce-events =
> <CMDQ_SYNC_TOKEN_SECURE_THR_EOF>;
> > };
> > };
> > --
> > 2.18.0
> >
>

</pre><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->