| Message ID | 20231023043751.17114-2-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: > CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ > driver in the normal world that GCE secure thread has completed a task > in thee secure world. s/thee/the/ > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- This is a new patch, so you must mention it in the changelog. There is nothing in the changelog saying about this new patch. > include/dt-bindings/gce/mt8195-gce.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/dt-bindings/gce/mt8195-gce.h b/include/dt-bindings/gce/mt8195-gce.h > index dcfb302b8a5b..9f99da3363b9 100644 > --- a/include/dt-bindings/gce/mt8195-gce.h > +++ b/include/dt-bindings/gce/mt8195-gce.h > @@ -809,4 +809,10 @@ > /* end of hw event */ > #define CMDQ_MAX_HW_EVENT 1019 > > +/* > + * Notify normal CMDQ there are some secure task done, > + * this token sync with secure world. > + */ > +#define CMDQ_SYNC_TOKEN_SECURE_THR_EOF 980 Why is this below 1019? Your driver calls it also even, so is this an event or not? Your driver does not use this value, so does it mean FW uses it? Best regards, Krzysztof
On 23/10/2023 09:47, Krzysztof Kozlowski wrote: > On 23/10/2023 06:37, Jason-JH.Lin wrote: >> CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ >> driver in the normal world that GCE secure thread has completed a task >> in thee secure world. > > s/thee/the/ > >> >> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> >> --- > > This is a new patch, so you must mention it in the changelog. There is > nothing in the changelog saying about this new patch. Hm, now I found previous thread. I asked to squash the patch with the binding. Best regards, Krzysztof
On 23/10/2023 06:37, Jason-JH.Lin wrote: > CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ > driver in the normal world that GCE secure thread has completed a task > in thee secure world. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time, thus I will skip this patch entirely till you follow the process allowing the patch to be tested. Please kindly resend and include all necessary To/Cc entries. Best regards, Krzysztof
Dear 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 09:47, Krzysztof Kozlowski wrote: > > On 23/10/2023 06:37, Jason-JH.Lin wrote: > >> CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify > CMDQ > >> driver in the normal world that GCE secure thread has completed a > task > >> in thee secure world. > > > > s/thee/the/ I'll fix this typo, thanks! > > > >> > >> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > >> --- > > > > This is a new patch, so you must mention it in the changelog. There > is > > nothing in the changelog saying about this new patch. > > Hm, now I found previous thread. I asked to squash the patch with the > binding. OK, I thought I had to reverse the order of definition patch and the add property patch. I'll squash them in the next version. Regards, Jason-JH.Lin > > Best regards, > Krzysztof > >
Hi Krzysztof, Thanks for the reviews. On Mon, 2023-10-23 at 09:47 +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: > > CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ > > driver in the normal world that GCE secure thread has completed a > task > > in thee secure world. > > s/thee/the/ > > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > This is a new patch, so you must mention it in the changelog. There > is > nothing in the changelog saying about this new patch. > > > > include/dt-bindings/gce/mt8195-gce.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/include/dt-bindings/gce/mt8195-gce.h b/include/dt- > bindings/gce/mt8195-gce.h > > index dcfb302b8a5b..9f99da3363b9 100644 > > --- a/include/dt-bindings/gce/mt8195-gce.h > > +++ b/include/dt-bindings/gce/mt8195-gce.h > > @@ -809,4 +809,10 @@ > > /* end of hw event */ > > #define CMDQ_MAX_HW_EVENT1019 > > > > +/* > > + * Notify normal CMDQ there are some secure task done, > > + * this token sync with secure world. > > + */ > > +#define CMDQ_SYNC_TOKEN_SECURE_THR_EOF980 > > Why is this below 1019? Your driver calls it also even, so is this an > event or not? > > Your driver does not use this value, so does it mean FW uses it? I just want to separate this kind of event (sw token) from the HW event. So I define it after CMDQ_MAX_HW_EVENT. But from the perspective of GCE event signal, there is no difference. So I'll move it to the correct position. Regards, Jason-JH.Lin > > Best regards, > Krzysztof >
Hi Krzysztof, On Mon, 2023-10-23 at 10:08 +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: > > CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ > > driver in the normal world that GCE secure thread has completed a > task > > in thee secure world. > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > Please use scripts/get_maintainers.pl to get a list of necessary > people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > You missed at least devicetree list (maybe more), so this won't be > tested by automated tooling. Performing review on untested code might > be > a waste of time, thus I will skip this patch entirely till you follow > the process allowing the patch to be tested. > > Please kindly resend and include all necessary To/Cc entries. > Thanks for the reminder, I'll use scripts to check the CC list again next time! > Best regards, > Krzysztof >
On 25/10/2023 08:26, Jason-JH Lin (林睿祥) wrote: > Hi Krzysztof, > > Thanks for the reviews. > > On Mon, 2023-10-23 at 09:47 +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: >>> CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ >>> driver in the normal world that GCE secure thread has completed a >> task >>> in thee secure world. >> >> s/thee/the/ >> >>> >>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> >>> --- >> >> This is a new patch, so you must mention it in the changelog. There >> is >> nothing in the changelog saying about this new patch. >> >> >>> include/dt-bindings/gce/mt8195-gce.h | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/include/dt-bindings/gce/mt8195-gce.h b/include/dt- >> bindings/gce/mt8195-gce.h >>> index dcfb302b8a5b..9f99da3363b9 100644 >>> --- a/include/dt-bindings/gce/mt8195-gce.h >>> +++ b/include/dt-bindings/gce/mt8195-gce.h >>> @@ -809,4 +809,10 @@ >>> /* end of hw event */ >>> #define CMDQ_MAX_HW_EVENT1019 >>> >>> +/* >>> + * Notify normal CMDQ there are some secure task done, >>> + * this token sync with secure world. >>> + */ >>> +#define CMDQ_SYNC_TOKEN_SECURE_THR_EOF980 >> >> Why is this below 1019? Your driver calls it also even, so is this an >> event or not? >> >> Your driver does not use this value, so does it mean FW uses it? > > I just want to separate this kind of event (sw token) from the HW > event. So I define it after CMDQ_MAX_HW_EVENT. SW event? Then why is it in the bindings? Best regards, Krzysztof
On Wed, 2023-10-25 at 09:03 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 25/10/2023 08:26, Jason-JH Lin (林睿祥) wrote: > > Hi Krzysztof, > > > > Thanks for the reviews. > > > > On Mon, 2023-10-23 at 09:47 +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: > >>> CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify > CMDQ > >>> driver in the normal world that GCE secure thread has completed a > >> task > >>> in thee secure world. > >> > >> s/thee/the/ > >> > >>> > >>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > >>> --- > >> > >> This is a new patch, so you must mention it in the changelog. > There > >> is > >> nothing in the changelog saying about this new patch. > >> > >> > >>> include/dt-bindings/gce/mt8195-gce.h | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/include/dt-bindings/gce/mt8195-gce.h b/include/dt- > >> bindings/gce/mt8195-gce.h > >>> index dcfb302b8a5b..9f99da3363b9 100644 > >>> --- a/include/dt-bindings/gce/mt8195-gce.h > >>> +++ b/include/dt-bindings/gce/mt8195-gce.h > >>> @@ -809,4 +809,10 @@ > >>> /* end of hw event */ > >>> #define CMDQ_MAX_HW_EVENT1019 > >>> > >>> +/* > >>> + * Notify normal CMDQ there are some secure task done, > >>> + * this token sync with secure world. > >>> + */ > >>> +#define CMDQ_SYNC_TOKEN_SECURE_THR_EOF980 > >> > >> Why is this below 1019? Your driver calls it also even, so is this > an > >> event or not? > >> > >> Your driver does not use this value, so does it mean FW uses it? > > > > I just want to separate this kind of event (sw token) from the HW > > event. So I define it after CMDQ_MAX_HW_EVENT. > > SW event? Then why is it in the bindings? > Sorry, I didn't explain it clearly. There are 2 kind of GCE event signal: - The SW token means: a GCE HW event signal triggered by SW drivers. e.g. SW driver append a GCE cmd to set a GCE HW event after a specific GCE cmd. Or SW driver use CPU to write a event id to GCE register to trigger the GCE HW event corresponding to that event id. - The HW event means: a GCE HW event signal triggered by HW engines. e.g. When HW OVL fetches all the data in frame buffer, HW OVL will send a frame done irq and a frame done GCE HW event(via HW bus directly) in the same time. Regards, Jason-JH.Lin > Best regards, > Krzysztof > >
diff --git a/include/dt-bindings/gce/mt8195-gce.h b/include/dt-bindings/gce/mt8195-gce.h index dcfb302b8a5b..9f99da3363b9 100644 --- a/include/dt-bindings/gce/mt8195-gce.h +++ b/include/dt-bindings/gce/mt8195-gce.h @@ -809,4 +809,10 @@ /* end of hw event */ #define CMDQ_MAX_HW_EVENT 1019 +/* + * Notify normal CMDQ there are some secure task done, + * this token sync with secure world. + */ +#define CMDQ_SYNC_TOKEN_SECURE_THR_EOF 980 + #endif
CMDQ_SYNC_TOKEN_SECURE_THR_EOF is used as secure irq to notify CMDQ driver in the normal world that GCE secure thread has completed a task in thee secure world. Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> --- include/dt-bindings/gce/mt8195-gce.h | 6 ++++++ 1 file changed, 6 insertions(+)