Re: [PATCH v7 10/20] soc: mediatek: mtk-cmdq: Add new APIs to replace cmdq_pkt_write() and cmdq_pkt_write_mask()

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 13/10/25 11:50, Jason-JH Lin (林睿祥) ha scritto:
> On Fri, 2025-09-05 at 09:41 +0000, CK Hu (胡俊光) wrote:
>> On Wed, 2025-08-27 at 19:37 +0800, Jason-JH Lin wrote:
>>> To support generating GCE write instructions using both pa_base and
>>> subsys, the original cmdq_pkt_write() and cmdq_pkt_write_mask()
>>> have
>>> been expanded into four new APIs:
>>> - Replaced cmdq_pkt_write() to cmdq_pkt_write_pa() and
>>>    cmdq_pkt_write_subsys().
>>> - Replaced cmdq_pkt_write_mask() to cmdq_pkt_write_mask_pa() and
>>>    cmdq_pkt_write_mask_subsys().
>>>
>>> The original cmdq_pkt_write() and cmdq_pkt_write_mask() will be
>>> removed
>>> after all CMDQ users have migrated to the new APIs.
>>>
>>> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
>>> ---
>>>   drivers/soc/mediatek/mtk-cmdq-helper.c | 41 +++++++++++++
>>>   include/linux/soc/mediatek/mtk-cmdq.h  | 79
>>> ++++++++++++++++++++++++++
>>>   2 files changed, 120 insertions(+)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index 41e1997cdd53..7e86299213d8 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -213,6 +213,26 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8
>>> subsys, u16 offset, u32 value)
>>>   }
>>>   EXPORT_SYMBOL(cmdq_pkt_write);
>>>   
>>> +int cmdq_pkt_write_pa(struct cmdq_pkt *pkt, u8 subsys /*unused*/,
>>> u32 pa_base,
>>> +		      u16 offset, u32 value)
>>
>> subsys is useless. Drop it.
>>
>>> +{
>>> +	int err;
>>> +
>>> +	err = cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX0,
>>> CMDQ_ADDR_HIGH(pa_base));
>>> +	if (err < 0)
>>> +		return err;
>>> +
>>> +	return cmdq_pkt_write_s_value(pkt, CMDQ_THR_SPR_IDX0,
>>> CMDQ_ADDR_LOW(offset), value);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_write_pa);
>>> +
>>> +int cmdq_pkt_write_subsys(struct cmdq_pkt *pkt, u8 subsys, u32
>>> pa_base /*unused*/,
>>> +			  u16 offset, u32 value)
>>
>> pa_base is useless. Drop it.
>>
>>> +{
>>> +	return cmdq_pkt_write(pkt, subsys, offset, value);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_write_subsys);
>>> +
>>>   int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
>>>   			u16 offset, u32 value, u32 mask)
>>>   {
>>> @@ -230,6 +250,27 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt,
>>> u8 subsys,
>>>   }
>>>   EXPORT_SYMBOL(cmdq_pkt_write_mask);
>>>   
>>> +int cmdq_pkt_write_mask_pa(struct cmdq_pkt *pkt, u8 subsys
>>> /*unused*/, u32 pa_base,
>>> +			   u16 offset, u32 value, u32 mask)
>>
>> subsys is useless. Drop it.
>>
>>> +{
>>> +	int err;
>>> +
>>> +	err = cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX0,
>>> CMDQ_ADDR_HIGH(pa_base));
>>> +	if (err < 0)
>>> +		return err;
>>> +
>>> +	return cmdq_pkt_write_s_mask_value(pkt, CMDQ_THR_SPR_IDX0,
>>> +					   CMDQ_ADDR_LOW(offset),
>>> value, mask);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_write_mask_pa);
>>> +
>>> +int cmdq_pkt_write_mask_subsys(struct cmdq_pkt *pkt, u8 subsys,
>>> u32 pa_base /*unused*/,
>>> +			       u16 offset, u32 value, u32 mask)
>>
>> pa_base is useless. Drop it.
>>
>>> +{
>>> +	return cmdq_pkt_write_mask(pkt, subsys, offset, value,
>>> mask);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_write_mask_subsys);
>>> +
> 
> Hi CK,
> 
> I'll drop the unused parameters.
> Thanks for the reviews.

It's unused, but if we want to use function pointers we do need those.

Unless you want to use one variable for both things, which then becomes
kind of janky and unreadable.

Cheers,
Angelo

> 
> Regards,
> Jason-JH Lin