Hi, Bibby: On Thu, 2019-05-16 at 17:02 +0800, Bibby Hsieh wrote: > Define a instruction structure for gce driver to append command. > This structure can make the client's code more readability. > > Signed-off-by: Bibby Hsieh <bibby.hsieh@xxxxxxxxxxxx> > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 113 +++++++++++++++-------- > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 + > include/linux/soc/mediatek/mtk-cmdq.h | 14 +-- > 3 files changed, 84 insertions(+), 45 deletions(-) > [snip] > > /** > * cmdq_pkt_write_mask() - append write command with mask to the CMDQ packet > * @pkt: the CMDQ packet > - * @value: the specified target register value > * @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_write_mask(struct cmdq_pkt *pkt, u32 value, > - u32 subsys, u32 offset, u32 mask); > +int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys, u16 offset, > + u32 value, u32 mask); You have do two things for this interface: one is reordering the parameter, another one is changing type of subsys from u32 to u8. Define the instruction struct is not necessary to change the order and type. I would like you to separate these two things to another patches. So the patch sequence may be: 1. Reorder parameter of cmdq_pkt_write_mask() 2. Change subsys type to u8 3. define the instruction struct Regards, CK > > /** > * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet > @@ -88,7 +88,7 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 value, > * > * Return: 0 for success; else the error code is returned > */ > -int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u32 event); > +int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event); > > /** > * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet > @@ -97,7 +97,7 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u32 event); > * > * Return: 0 for success; else the error code is returned > */ > -int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u32 event); > +int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event); > > /** > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
