Re: [RESEND, PATCH v13 10/12] soc: mediatek: cmdq: add polling function
On Fri, 2019-08-23 at 16:05 +0200, Matthias Brugger wrote: > > On 20/08/2019 10:49, Bibby Hsieh wrote: > > add polling function in cmdq helper functions > > > > Signed-off-by: Bibby Hsieh > > Reviewed-by: CK Hu > > --- > > drivers/soc/mediatek/mtk-cmdq-helper.c | 28 > > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > > include/linux/soc/mediatek/mtk-cmdq.h| 15 + > > 3 files changed, 44 insertions(+) > > > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > > b/drivers/soc/mediatek/mtk-cmdq-helper.c > > index e3d5b0be8e79..c53f8476c68d 100644 > > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > > @@ -221,6 +221,34 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 > > event) > > } > > EXPORT_SYMBOL(cmdq_pkt_clear_event); > > > > +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, > > + u16 offset, u32 value, u32 mask) > > +{ > > + struct cmdq_instruction *inst; > > + > > + if (mask != 0x) { > > Is this necessary? Can't we just always set the mask, even if it's 0x? > > Regarding interfaces, depending on how often you expect the mask being ~0 we > might think of adding a cmdq_pkt_poll_mask call. > What I want to say, if in the end most of the callers will use the mask with > 0x, then we should add a call cmdq_pkt_poll_mask which actually allows > to set the mask and let cmdq_pkt_poll set the mask in it's function body. > As I already said, this depends on how often you think a caller will > use/not-use > the mask. > Does this make sense? It's better to have two function: cmdq_pkt_poll_mask and cmdq_pkt_poll, client can choose which they need by themselves. Thanks for the comments. Bibby > > + inst = cmdq_pkt_append_command(pkt); > > + if (!inst) > > + return -ENOMEM; > > + > > + inst->op = CMDQ_CODE_MASK; > > + inst->value = ~mask; > > + offset = offset | 0x1; > > + } > > + > > + inst = cmdq_pkt_append_command(pkt); > > + if (!inst) > > + return -ENOMEM; > > + > > + inst->op = CMDQ_CODE_POLL; > > + inst->value = value; > > + inst->offset = offset; > > + inst->subsys = subsys; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(cmdq_pkt_poll); > > + > > static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > > { > > struct cmdq_instruction *inst; > > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > > b/include/linux/mailbox/mtk-cmdq-mailbox.h > > index c8adedefaf42..9e3502945bc1 100644 > > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > > @@ -46,6 +46,7 @@ > > enum cmdq_code { > > CMDQ_CODE_MASK = 0x02, > > CMDQ_CODE_WRITE = 0x04, > > + CMDQ_CODE_POLL = 0x08, > > CMDQ_CODE_JUMP = 0x10, > > CMDQ_CODE_WFE = 0x20, > > CMDQ_CODE_EOC = 0x40, > > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h > > b/include/linux/soc/mediatek/mtk-cmdq.h > > index 9618debb9ceb..a345870a6d10 100644 > > --- a/include/linux/soc/mediatek/mtk-cmdq.h > > +++ b/include/linux/soc/mediatek/mtk-cmdq.h > > @@ -99,6 +99,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event); > > */ > > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event); > > > > +/** > > + * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to > > + * execute an instruction that wait for a specified hardware > > + * register to check for the value. All GCE hardware > > + * threads will be blocked by this instruction. > > + * @pkt: the CMDQ packet > > + * @subsys:the CMDQ sub system code > > + * @offset:register offset from CMDQ sub system > > + * @value: the specified target register value > > + * @mask: the specified target register mask > > + * > > + * Return: 0 for success; else the error code is returned > > + */ > > +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, > > + u16 offset, u32 value, u32 mask); > > /** > > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ > > * packet and call back at the end of done packet > >
Re: [RESEND, PATCH v13 10/12] soc: mediatek: cmdq: add polling function
On 20/08/2019 10:49, Bibby Hsieh wrote: > add polling function in cmdq helper functions > > Signed-off-by: Bibby Hsieh > Reviewed-by: CK Hu > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 28 > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > include/linux/soc/mediatek/mtk-cmdq.h| 15 + > 3 files changed, 44 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > b/drivers/soc/mediatek/mtk-cmdq-helper.c > index e3d5b0be8e79..c53f8476c68d 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -221,6 +221,34 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event) > } > EXPORT_SYMBOL(cmdq_pkt_clear_event); > > +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, > + u16 offset, u32 value, u32 mask) > +{ > + struct cmdq_instruction *inst; > + > + if (mask != 0x) { Is this necessary? Can't we just always set the mask, even if it's 0x? Regarding interfaces, depending on how often you expect the mask being ~0 we might think of adding a cmdq_pkt_poll_mask call. What I want to say, if in the end most of the callers will use the mask with 0x, then we should add a call cmdq_pkt_poll_mask which actually allows to set the mask and let cmdq_pkt_poll set the mask in it's function body. As I already said, this depends on how often you think a caller will use/not-use the mask. Does this make sense? > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_MASK; > + inst->value = ~mask; > + offset = offset | 0x1; > + } > + > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_POLL; > + inst->value = value; > + inst->offset = offset; > + inst->subsys = subsys; > + > + return 0; > +} > +EXPORT_SYMBOL(cmdq_pkt_poll); > + > static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > { > struct cmdq_instruction *inst; > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > b/include/linux/mailbox/mtk-cmdq-mailbox.h > index c8adedefaf42..9e3502945bc1 100644 > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > @@ -46,6 +46,7 @@ > enum cmdq_code { > CMDQ_CODE_MASK = 0x02, > CMDQ_CODE_WRITE = 0x04, > + CMDQ_CODE_POLL = 0x08, > CMDQ_CODE_JUMP = 0x10, > CMDQ_CODE_WFE = 0x20, > CMDQ_CODE_EOC = 0x40, > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h > b/include/linux/soc/mediatek/mtk-cmdq.h > index 9618debb9ceb..a345870a6d10 100644 > --- a/include/linux/soc/mediatek/mtk-cmdq.h > +++ b/include/linux/soc/mediatek/mtk-cmdq.h > @@ -99,6 +99,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event); > */ > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event); > > +/** > + * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to > + *execute an instruction that wait for a specified hardware > + *register to check for the value. All GCE hardware > + *threads will be blocked by this instruction. > + * @pkt: the CMDQ packet > + * @subsys: the CMDQ sub system code > + * @offset: register offset from CMDQ sub system > + * @value: the specified target register value > + * @mask:the specified target register mask > + * > + * Return: 0 for success; else the error code is returned > + */ > +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, > + u16 offset, u32 value, u32 mask); > /** > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ > * packet and call back at the end of done packet >
Re: [RESEND, PATCH v13 10/12] soc: mediatek: cmdq: add polling function
On Tue, 2019-08-20 at 16:49 +0800, Bibby Hsieh wrote: > add polling function in cmdq helper functions > > Signed-off-by: Bibby Hsieh > Reviewed-by: CK Hu > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 28 > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + > include/linux/soc/mediatek/mtk-cmdq.h| 15 + > 3 files changed, 44 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > b/drivers/soc/mediatek/mtk-cmdq-helper.c > index e3d5b0be8e79..c53f8476c68d 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -221,6 +221,34 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event) > } > EXPORT_SYMBOL(cmdq_pkt_clear_event); > > +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, > + u16 offset, u32 value, u32 mask) > +{ > + struct cmdq_instruction *inst; > + > + if (mask != 0x) { > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_MASK; > + inst->value = ~mask; > + offset = offset | 0x1; > + } > + > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_POLL; > + inst->value = value; > + inst->offset = offset; > + inst->subsys = subsys; > + > + return 0; > +} > +EXPORT_SYMBOL(cmdq_pkt_poll); > + > static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > { > struct cmdq_instruction *inst; > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h > b/include/linux/mailbox/mtk-cmdq-mailbox.h > index c8adedefaf42..9e3502945bc1 100644 > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > @@ -46,6 +46,7 @@ > enum cmdq_code { > CMDQ_CODE_MASK = 0x02, > CMDQ_CODE_WRITE = 0x04, > + CMDQ_CODE_POLL = 0x08, > CMDQ_CODE_JUMP = 0x10, > CMDQ_CODE_WFE = 0x20, > CMDQ_CODE_EOC = 0x40, > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h > b/include/linux/soc/mediatek/mtk-cmdq.h > index 9618debb9ceb..a345870a6d10 100644 > --- a/include/linux/soc/mediatek/mtk-cmdq.h > +++ b/include/linux/soc/mediatek/mtk-cmdq.h > @@ -99,6 +99,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event); > */ > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event); > > +/** > + * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to > + *execute an instruction that wait for a specified hardware > + *register to check for the value. All GCE hardware > + *threads will be blocked by this instruction. > + * @pkt: the CMDQ packet > + * @subsys: the CMDQ sub system code > + * @offset: register offset from CMDQ sub system > + * @value: the specified target register value > + * @mask:the specified target register mask > + * > + * Return: 0 for success; else the error code is returned > + */ > +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, > + u16 offset, u32 value, u32 mask); > /** > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ > * packet and call back at the end of done packet Reviewed-by: Houlong Wei
[RESEND, PATCH v13 10/12] soc: mediatek: cmdq: add polling function
add polling function in cmdq helper functions Signed-off-by: Bibby Hsieh Reviewed-by: CK Hu --- drivers/soc/mediatek/mtk-cmdq-helper.c | 28 include/linux/mailbox/mtk-cmdq-mailbox.h | 1 + include/linux/soc/mediatek/mtk-cmdq.h| 15 + 3 files changed, 44 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index e3d5b0be8e79..c53f8476c68d 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -221,6 +221,34 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event) } EXPORT_SYMBOL(cmdq_pkt_clear_event); +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, + u16 offset, u32 value, u32 mask) +{ + struct cmdq_instruction *inst; + + if (mask != 0x) { + inst = cmdq_pkt_append_command(pkt); + if (!inst) + return -ENOMEM; + + inst->op = CMDQ_CODE_MASK; + inst->value = ~mask; + offset = offset | 0x1; + } + + inst = cmdq_pkt_append_command(pkt); + if (!inst) + return -ENOMEM; + + inst->op = CMDQ_CODE_POLL; + inst->value = value; + inst->offset = offset; + inst->subsys = subsys; + + return 0; +} +EXPORT_SYMBOL(cmdq_pkt_poll); + static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) { struct cmdq_instruction *inst; diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h index c8adedefaf42..9e3502945bc1 100644 --- a/include/linux/mailbox/mtk-cmdq-mailbox.h +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h @@ -46,6 +46,7 @@ enum cmdq_code { CMDQ_CODE_MASK = 0x02, CMDQ_CODE_WRITE = 0x04, + CMDQ_CODE_POLL = 0x08, CMDQ_CODE_JUMP = 0x10, CMDQ_CODE_WFE = 0x20, CMDQ_CODE_EOC = 0x40, diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 9618debb9ceb..a345870a6d10 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -99,6 +99,21 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event); */ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event); +/** + * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to + * execute an instruction that wait for a specified hardware + * register to check for the value. All GCE hardware + * threads will be blocked by this instruction. + * @pkt: the CMDQ packet + * @subsys:the CMDQ sub system code + * @offset:register offset from CMDQ sub system + * @value: the specified target register value + * @mask: the specified target register mask + * + * Return: 0 for success; else the error code is returned + */ +int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys, + u16 offset, u32 value, u32 mask); /** * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ * packet and call back at the end of done packet -- 2.18.0