diff mbox series

[v2,2/9] dt-bindings: mailbox: Add property for CMDQ secure driver

Message ID 20231023043751.17114-3-jason-jh.lin@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add CMDQ secure driver for SVP | expand

Commit Message

Jason-JH Lin (林睿祥) Oct. 23, 2023, 4:37 a.m. UTC
Add mboxes to define a GCE loopping thread as a secure irq handler.
Add mediatek,gce-events to define a GCE event siganl sent from CMDQ
TA driver in secure world as a secure irq.

These 2 properties are required for CMDQ secure driver.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../bindings/mailbox/mediatek,gce-mailbox.yaml         | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Krzysztof Kozlowski Oct. 23, 2023, 7:49 a.m. UTC | #1
On 23/10/2023 06:37, Jason-JH.Lin wrote:
> Add mboxes to define a GCE loopping thread as a secure irq handler.
> Add mediatek,gce-events to define a GCE event siganl sent from CMDQ
> TA driver in secure world as a secure irq.
> 
> These 2 properties are required for CMDQ secure driver.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../bindings/mailbox/mediatek,gce-mailbox.yaml         | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> index cef9d7601398..65fb93bb53b6 100644
> --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> @@ -49,6 +49,16 @@ properties:
>      items:
>        - const: gce
>  
> +  mboxes:
> +    maxItems: 1
> +
> +  mediatek,gce-events:
> +    description:
> +      The event id which is mapping to the specific hardware event signal
> +      to gce. The event id is defined in the gce header
> +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

Nothing improved.

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof
Jason-JH Lin (林睿祥) Oct. 24, 2023, 4:37 p.m. UTC | #2
Hi Krzysztof,

Thanks for the reviews.

On Mon, 2023-10-23 at 09:49 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 23/10/2023 06:37, Jason-JH.Lin wrote:
> > Add mboxes to define a GCE loopping thread as a secure irq handler.
> > Add mediatek,gce-events to define a GCE event siganl sent from CMDQ
> > TA driver in secure world as a secure irq.
> > 
> > These 2 properties are required for CMDQ secure driver.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  .../bindings/mailbox/mediatek,gce-mailbox.yaml         | 10
> ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git
> a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml 
> b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> > index cef9d7601398..65fb93bb53b6 100644
> > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> > @@ -49,6 +49,16 @@ properties:
> >      items:
> >        - const: gce
> >  
> > +  mboxes:
> > +    maxItems: 1
> > +
> > +  mediatek,gce-events:
> > +    description:
> > +      The event id which is mapping to the specific hardware event
> signal
> > +      to gce. The event id is defined in the gce header
> > +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Nothing improved.
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my

Sorry, I didn't find your comments in the previous patch here:

https://patchwork.kernel.org/project/linux-mediatek/patch/20230918192204.32263-2-jason-jh.lin@mediatek.com/

Could you please comment again?
Thank you very much.


And I do see Rob's comment, so I sync the mediatek,gce-events property
definition from other yaml where used this property.

Should I also modify the definition of mediatek,gce-events property to
reference to mediatek,gce-mailbox.yaml for other yaml?

Regards,
Jason-JH.Lin

> feedback got lost between the quotes, maybe you just forgot to apply
> it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
> 
> Thank you.
> 
> Best regards,
> Krzysztof
> 
>
Krzysztof Kozlowski Oct. 28, 2023, 9:10 a.m. UTC | #3
On 24/10/2023 18:37, Jason-JH Lin (林睿祥) wrote:
> Hi Krzysztof,
> 
> Thanks for the reviews.
> 
> On Mon, 2023-10-23 at 09:49 +0200, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On 23/10/2023 06:37, Jason-JH.Lin wrote:
>>> Add mboxes to define a GCE loopping thread as a secure irq handler.
>>> Add mediatek,gce-events to define a GCE event siganl sent from CMDQ
>>> TA driver in secure world as a secure irq.
>>>
>>> These 2 properties are required for CMDQ secure driver.
>>>
>>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
>>> ---
>>>  .../bindings/mailbox/mediatek,gce-mailbox.yaml         | 10
>> ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml 
>> b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
>>> index cef9d7601398..65fb93bb53b6 100644
>>> --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
>> mailbox.yaml
>>> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
>> mailbox.yaml
>>> @@ -49,6 +49,16 @@ properties:
>>>      items:
>>>        - const: gce
>>>  
>>> +  mboxes:
>>> +    maxItems: 1
>>> +
>>> +  mediatek,gce-events:
>>> +    description:
>>> +      The event id which is mapping to the specific hardware event
>> signal
>>> +      to gce. The event id is defined in the gce header
>>> +      include/dt-bindings/gce/<chip>-gce.h of each chips.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> Nothing improved.
>>
>> This is a friendly reminder during the review process.
>>
>> It seems my previous comments were not fully addressed. Maybe my
> 
> Sorry, I didn't find your comments in the previous patch here:
> 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20230918192204.32263-2-jason-jh.lin@mediatek.com/
> 
> Could you please comment again?
> Thank you very much.

Please implement Rob's feedback.
> 
> 
> And I do see Rob's comment, so I sync the mediatek,gce-events property
> definition from other yaml where used this property.
> 
> Should I also modify the definition of mediatek,gce-events property to
> reference to mediatek,gce-mailbox.yaml for other yaml?

You need single definition in one of the bindings or in shared bindings.
Depends on the users of this property.


Best regards,
Krzysztof
Jason-JH Lin (林睿祥) Oct. 31, 2023, 2:34 a.m. UTC | #4
Hi Krzysztof,

On Sat, 2023-10-28 at 11:10 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 24/10/2023 18:37, Jason-JH Lin (林睿祥) wrote:
> > Hi Krzysztof,
> > 
> > Thanks for the reviews.
> > 
> > On Mon, 2023-10-23 at 09:49 +0200, Krzysztof Kozlowski wrote:
> >>   
> >> External email : Please do not click links or open attachments
> until
> >> you have verified the sender or the content.
> >>  On 23/10/2023 06:37, Jason-JH.Lin wrote:
> >>> Add mboxes to define a GCE loopping thread as a secure irq
> handler.
> >>> Add mediatek,gce-events to define a GCE event siganl sent from
> CMDQ
> >>> TA driver in secure world as a secure irq.
> >>>
> >>> These 2 properties are required for CMDQ secure driver.
> >>>
> >>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> >>> ---
> >>>  .../bindings/mailbox/mediatek,gce-mailbox.yaml         | 10
> >> ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>>
> >>> diff --git
> >> a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml 
> >> b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> >>> index cef9d7601398..65fb93bb53b6 100644
> >>> --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> >> mailbox.yaml
> >>> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> >> mailbox.yaml
> >>> @@ -49,6 +49,16 @@ properties:
> >>>      items:
> >>>        - const: gce
> >>>  
> >>> +  mboxes:
> >>> +    maxItems: 1
> >>> +
> >>> +  mediatek,gce-events:
> >>> +    description:
> >>> +      The event id which is mapping to the specific hardware
> event
> >> signal
> >>> +      to gce. The event id is defined in the gce header
> >>> +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >>
> >> Nothing improved.
> >>
> >> This is a friendly reminder during the review process.
> >>
> >> It seems my previous comments were not fully addressed. Maybe my
> > 
> > Sorry, I didn't find your comments in the previous patch here:
> > 
> > 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20230918192204.32263-2-jason-jh.lin@mediatek.com/
> > 
> > Could you please comment again?
> > Thank you very much.
> 
> Please implement Rob's feedback.
> > 
> > 
> > And I do see Rob's comment, so I sync the mediatek,gce-events
> property
> > definition from other yaml where used this property.
> > 
> > Should I also modify the definition of mediatek,gce-events property
> to
> > reference to mediatek,gce-mailbox.yaml for other yaml?
> 
> You need single definition in one of the bindings or in shared
> bindings.
> Depends on the users of this property.
> 

Because this property is related to GCE HW.
I think the definition should be defined in mediatek,gce-mailbox.yaml.

So I should have a single definition in mediatek,gce-mailbox.yaml:
mediatek,gce-events:
  description:
    The event id which is mapping to the specific hardware event signal
    to gce. The event id is defined in the gce header
    include/dt-bindings/gce/<chip>-gce.h of each chips.
  $ref: /schemas/types.yaml#/definitions/uint32-array


And other bindings:
Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml#L56
D
ocumentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml#L41
Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml#L37
Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml#L37
should modify the definition to:
mediatek,gce-events:
  description:
    Reference to
Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.


Is that right?

Regards,
Jason-JH.Lin

> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
index cef9d7601398..65fb93bb53b6 100644
--- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
@@ -49,6 +49,16 @@  properties:
     items:
       - const: gce
 
+  mboxes:
+    maxItems: 1
+
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 required:
   - compatible
   - "#mbox-cells"