Re: [PATCH v5 13/13] soc: mediatek: cmdq: add set event function

Matthias Brugger
2020-05-24T18:15:55Z

On 24/05/2020 19:39, Dennis-YC Hsieh wrote:
> Hi Matthias,
>
> Thanks for your comment.
>
>
> On Sat, 2020-05-16 at 20:32 +0200, Matthias Brugger wrote:
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> Add set event function in cmdq helper functions to set specific event.
>>>
>>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@mediatek.com>
>>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
>>> include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
>>> 3 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index ec5637d43254..3294c9285994 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_clear_event);
>>>
>>> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> +
>>> + if (event >= CMDQ_MAX_EVENT)
>>> + return -EINVAL;
>>> +
>>> + inst.op = CMDQ_CODE_WFE;
>>> + inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
>>> + inst.event = event;
>>> +
>>> + return cmdq_pkt_append_command(pkt, inst);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_set_event);
>>> +
>>> int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>>> u16 offset, u32 value)
>>> {
>>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> index 42d2a30e6a70..ba2d811183a9 100644
>>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> @@ -17,6 +17,7 @@
>>> #define CMDQ_JUMP_PASS CMDQ_INST_SIZE
>>>
>>> #define CMDQ_WFE_UPDATE BIT(31)
>>> +#define CMDQ_WFE_UPDATE_VALUE BIT(16)
>>> #define CMDQ_WFE_WAIT BIT(15)
>>> #define CMDQ_WFE_WAIT_VALUE 0x1
>>>
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index d63749440697..ca70296ae120 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
>>> */
>>> int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
>>>
>>> +/**
>>> + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
>>> + * @pkt: the CMDQ packet
>>> + * @event: the desired event to be set
>>
>> Can we add the events and their code, so that later on, when a consumer calls
>> cmdq_pkt_set_event() we don't have any magic values that are hard to understand?
>
> Please see patch 02/13:
> http://lists.infradead.org/pipermail/linux-mediatek/2020-March/027801.html
>
> Definitions begin with CMDQ_EVENT_ is the event id to this function.
> Since the event id is different between platform, client must parse it
> from device tree. So no magic values require when call this function.
>
>

Got it, thanks for clarification.
Matthias