| 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 |
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
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 > >
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
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 --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"
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(+)