On 24/05/2020 19:01, Dennis-YC Hsieh wrote: > Hi Matthias, > > Thanks for your comment. > > On Sat, 2020-05-16 at 19:59 +0200, Matthias Brugger wrote: >> >> On 08/03/2020 11:52, Dennis YC Hsieh wrote: >>> Add assign function in cmdq helper which assign constant value into >>> internal register by index. >>> >>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@xxxxxxxxxxxx> >>> Reviewed-by: CK Hu <ck.hu@xxxxxxxxxxxx> >>> --- >>> drivers/soc/mediatek/mtk-cmdq-helper.c | 24 +++++++++++++++++++++++- >>> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + >>> include/linux/soc/mediatek/mtk-cmdq.h | 14 ++++++++++++++ >>> 3 files changed, 38 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c >>> index 98f23ba3ba47..33153d17c9d9 100644 >>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c >>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c >>> @@ -12,6 +12,7 @@ >>> #define CMDQ_WRITE_ENABLE_MASK BIT(0) >>> #define CMDQ_POLL_ENABLE_MASK BIT(0) >>> #define CMDQ_EOC_IRQ_EN BIT(0) >>> +#define CMDQ_REG_TYPE 1 >>> >>> struct cmdq_instruction { >>> union { >>> @@ -21,8 +22,17 @@ struct cmdq_instruction { >>> union { >>> u16 offset; >>> u16 event; >>> + u16 reg_dst; >>> + }; >>> + union { >>> + u8 subsys; >>> + struct { >>> + u8 sop:5; >>> + u8 arg_c_t:1; >>> + u8 arg_b_t:1; >>> + u8 dst_t:1; >>> + }; >> >> This union seems without context in this patch. Please drop. >> > > The dst_t use in cmdq_pkt_assign function so how about merge other Ah didn't realize this. Then I think it's OK like it is. Regards, Matthias > variables to reserved and leave dst_t ? > > struct { > u8 reserved_t:7; > u8 dst_t:1; > }; > > > Regards, > Dennis > > >> Regards, >> Matthias >> >>> }; >>> - u8 subsys; >>> u8 op; >>> }; >>> >>> @@ -277,6 +287,18 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys, >>> } >>> EXPORT_SYMBOL(cmdq_pkt_poll_mask); >>> >>> +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value) >>> +{ >>> + struct cmdq_instruction inst = { {0} }; >>> + >>> + inst.op = CMDQ_CODE_LOGIC; >>> + inst.dst_t = CMDQ_REG_TYPE; >>> + inst.reg_dst = reg_idx; >>> + inst.value = value; >>> + return cmdq_pkt_append_command(pkt, inst); >>> +} >>> +EXPORT_SYMBOL(cmdq_pkt_assign); >>> + >>> static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) >>> { >>> struct cmdq_instruction inst = { {0} }; >>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h >>> index dfe5b2eb85cc..121c3bb6d3de 100644 >>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h >>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h >>> @@ -59,6 +59,7 @@ enum cmdq_code { >>> CMDQ_CODE_JUMP = 0x10, >>> CMDQ_CODE_WFE = 0x20, >>> CMDQ_CODE_EOC = 0x40, >>> + CMDQ_CODE_LOGIC = 0xa0, >>> }; >>> >>> enum cmdq_cb_status { >>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h >>> index a74c1d5acdf3..83340211e1d3 100644 >>> --- a/include/linux/soc/mediatek/mtk-cmdq.h >>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h >>> @@ -152,6 +152,20 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, >>> */ >>> int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys, >>> u16 offset, u32 value, u32 mask); >>> + >>> +/** >>> + * cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE >>> + * to execute an instruction that set a constant value into >>> + * internal register and use as value, mask or address in >>> + * read/write instruction. >>> + * @pkt: the CMDQ packet >>> + * @reg_idx: the CMDQ internal register ID >>> + * @value: the specified value >>> + * >>> + * Return: 0 for success; else the error code is returned >>> + */ >>> +int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value); >>> + >>> /** >>> * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ >>> * packet and call back at the end of done packet >>> >