| Message ID | 20230918192204.32263-7-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add CMDQ secure driver for SVP | expand |
Hi, Jason: On Tue, 2023-09-19 at 03:21 +0800, Jason-JH.Lin wrote: > Add cmdq_mbox_stop to disable GCE thread. > > To support the error handling or the stop flow of the GCE loopping > thread, lopping thread user can call cmdq_mbox_stop to disable the > GCE HW thread. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > index 4d62b07c1411..8bd39fecbf00 100644 > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct mbox_chan > *chan) > spin_unlock_irqrestore(&thread->chan->lock, flags); > } > > +void cmdq_mbox_stop(struct mbox_chan *chan) > +{ > + cmdq_mbox_shutdown(chan); cmdq_mobx_stop() is equal to cmdq_mbox_shutdown(), so client driver could call mbox_free_channel() to do this and this function is redundant. Regards, CK > +} > +EXPORT_SYMBOL(cmdq_mbox_stop); > + > static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long > timeout) > { > struct cmdq_thread *thread = (struct cmdq_thread *)chan- > >con_priv; > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > b/include/linux/mailbox/mtk-cmdq-mailbox.h > index a8f0070c7aa9..f3e577335acb 100644 > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > @@ -79,5 +79,6 @@ struct cmdq_pkt { > }; > > u8 cmdq_get_shift_pa(struct mbox_chan *chan); > +void cmdq_mbox_stop(struct mbox_chan *chan); > > #endif /* __MTK_CMDQ_MAILBOX_H__ */
Hi CK, Thanks for the reviews. On Tue, 2023-09-19 at 01:24 +0000, CK Hu (胡俊光) wrote: > Hi, Jason: > > On Tue, 2023-09-19 at 03:21 +0800, Jason-JH.Lin wrote: > > Add cmdq_mbox_stop to disable GCE thread. > > > > To support the error handling or the stop flow of the GCE loopping > > thread, lopping thread user can call cmdq_mbox_stop to disable the > > GCE HW thread. > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ > > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > > 2 files changed, 7 insertions(+) > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > > b/drivers/mailbox/mtk-cmdq-mailbox.c > > index 4d62b07c1411..8bd39fecbf00 100644 > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > > @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct > > mbox_chan > > *chan) > > spin_unlock_irqrestore(&thread->chan->lock, flags); > > } > > > > +void cmdq_mbox_stop(struct mbox_chan *chan) > > +{ > > + cmdq_mbox_shutdown(chan); > > cmdq_mobx_stop() is equal to cmdq_mbox_shutdown(), so client driver > could call mbox_free_channel() to do this and this function is > redundant. > I'vd tried to use cmdq->mbox.ops->shutdown(cmdq->clt->chan) in mtk- cmdq-sec-mbox.c, but it'll call to cmdq_sec_mbox_shutdown(). If I want to call to the cmdq_mbox_shutdown in mtk-cmdq-sec-mailbox.c, I have to find the way to get the mbox of mtk-cmdq-mailbox.c. So I think open a API is easy solution for this. I have called mbox_free_channel() by calling cmdq_mbox_destroy() after cmdq_mbox_stop() in cmdq_sec_irq_notify_start() in mtk-cmdq-sec- mailbox.c. Regards, Jason-JH.Lin > Regards, > CK > > > +} > > +EXPORT_SYMBOL(cmdq_mbox_stop); > > + > > static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long > > timeout) > > { > > struct cmdq_thread *thread = (struct cmdq_thread *)chan- > > > con_priv; > > > > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > > b/include/linux/mailbox/mtk-cmdq-mailbox.h > > index a8f0070c7aa9..f3e577335acb 100644 > > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > > @@ -79,5 +79,6 @@ struct cmdq_pkt { > > }; > > > > u8 cmdq_get_shift_pa(struct mbox_chan *chan); > > +void cmdq_mbox_stop(struct mbox_chan *chan); > > > > #endif /* __MTK_CMDQ_MAILBOX_H__ */
On 18/09/2023 21:21, Jason-JH.Lin wrote: > Add cmdq_mbox_stop to disable GCE thread. > > To support the error handling or the stop flow of the GCE loopping > thread, lopping thread user can call cmdq_mbox_stop to disable the > GCE HW thread. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c > index 4d62b07c1411..8bd39fecbf00 100644 > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan) > spin_unlock_irqrestore(&thread->chan->lock, flags); > } > > +void cmdq_mbox_stop(struct mbox_chan *chan) > +{ > + cmdq_mbox_shutdown(chan); > +} > +EXPORT_SYMBOL(cmdq_mbox_stop); 1. EXPORT_SYMBOL_GPL 2. Missing kernel doc (full doc) Best regards, Krzysztof
On 18/09/2023 21:21, Jason-JH.Lin wrote: > Add cmdq_mbox_stop to disable GCE thread. > > To support the error handling or the stop flow of the GCE loopping > thread, lopping thread user can call cmdq_mbox_stop to disable the > GCE HW thread. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c > index 4d62b07c1411..8bd39fecbf00 100644 > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan) > spin_unlock_irqrestore(&thread->chan->lock, flags); > } > > +void cmdq_mbox_stop(struct mbox_chan *chan) > +{ > + cmdq_mbox_shutdown(chan); > +} > +EXPORT_SYMBOL(cmdq_mbox_stop); Plus there are no users. NAK. This is not code which should be posted upstream. Best regards, Krzysztof
Hi Krzysztof, Thanks for the review. On Sat, 2023-09-23 at 20:02 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 18/09/2023 21:21, Jason-JH.Lin wrote: > > Add cmdq_mbox_stop to disable GCE thread. > > > > To support the error handling or the stop flow of the GCE loopping > > thread, lopping thread user can call cmdq_mbox_stop to disable the > > GCE HW thread. > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ > > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > > 2 files changed, 7 insertions(+) > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > > index 4d62b07c1411..8bd39fecbf00 100644 > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > > @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct > mbox_chan *chan) > > spin_unlock_irqrestore(&thread->chan->lock, flags); > > } > > > > +void cmdq_mbox_stop(struct mbox_chan *chan) > > +{ > > +cmdq_mbox_shutdown(chan); > > +} > > +EXPORT_SYMBOL(cmdq_mbox_stop); > > 1. EXPORT_SYMBOL_GPL > 2. Missing kernel doc (full doc) > > After reviewing by CK, I think this patch should be dropped. Sorry for bothering you and thanks for the reviews. Regards, Jason-JH.Lin > > Best regards, > Krzysztof > >
Hi Krzysztof, Thanks for the reviews. On Sat, 2023-09-23 at 20:07 +0200, Krzysztof Kozlowski wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 18/09/2023 21:21, Jason-JH.Lin wrote: > > Add cmdq_mbox_stop to disable GCE thread. > > > > To support the error handling or the stop flow of the GCE loopping > > thread, lopping thread user can call cmdq_mbox_stop to disable the > > GCE HW thread. > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ > > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > > 2 files changed, 7 insertions(+) > > > > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > > index 4d62b07c1411..8bd39fecbf00 100644 > > --- a/drivers/mailbox/mtk-cmdq-mailbox.c > > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > > @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct > mbox_chan *chan) > > spin_unlock_irqrestore(&thread->chan->lock, flags); > > } > > > > +void cmdq_mbox_stop(struct mbox_chan *chan) > > +{ > > +cmdq_mbox_shutdown(chan); > > +} > > +EXPORT_SYMBOL(cmdq_mbox_stop); > > Plus there are no users. > > NAK. This is not code which should be posted upstream. > It'll be used in cmdq_sec_irq_notify_start() at [PATCH 10/15]. After reviewing by CK, I think I'll try to drop this patch. Regards, Jason-JH.Lin > Best regards, > Krzysztof > >
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c index 4d62b07c1411..8bd39fecbf00 100644 --- a/drivers/mailbox/mtk-cmdq-mailbox.c +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -469,6 +469,12 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan) spin_unlock_irqrestore(&thread->chan->lock, flags); } +void cmdq_mbox_stop(struct mbox_chan *chan) +{ + cmdq_mbox_shutdown(chan); +} +EXPORT_SYMBOL(cmdq_mbox_stop); + static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout) { struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv; diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index a8f0070c7aa9..f3e577335acb 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -79,5 +79,6 @@ struct cmdq_pkt { }; u8 cmdq_get_shift_pa(struct mbox_chan *chan); +void cmdq_mbox_stop(struct mbox_chan *chan); #endif /* __MTK_CMDQ_MAILBOX_H__ */
Add cmdq_mbox_stop to disable GCE thread. To support the error handling or the stop flow of the GCE loopping thread, lopping thread user can call cmdq_mbox_stop to disable the GCE HW thread. Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> --- drivers/mailbox/mtk-cmdq-mailbox.c | 6 ++++++ include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + 2 files changed, 7 insertions(+)