Re: [PATCH 1/1] soc: mediatek: Add command for APU SMC call

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

Il 24/10/24 11:27, Karl.Li ha scritto:
> From: Karl Li <karl.li@mediatek.com>
> 
> Add command for APU SMC call.
> The APU microprocess's start and stop sequence will process in ATF.
> 
> Signed-off-by: Karl Li <karl.li@mediatek.com>
> ---
>   include/linux/soc/mediatek/mtk_apu_secure.h | 32 +++++++++++++++++++++

This is firmware stuff!

Please move this file to
include/linux/firmware/mediatek/apu.h or apusys.h

If you expect that the APU will *never* have any non-secure commands, then you
can add the "-secure" suffix (apu-secure.h) - but if there will ever be any
commands that aren't secured, you should not call it xxxx-secure.h

>   1 file changed, 32 insertions(+)
>   create mode 100644 include/linux/soc/mediatek/mtk_apu_secure.h
> 
> diff --git a/include/linux/soc/mediatek/mtk_apu_secure.h b/include/linux/soc/mediatek/mtk_apu_secure.h
> new file mode 100644
> index 000000000000..ea491bc24adb
> --- /dev/null
> +++ b/include/linux/soc/mediatek/mtk_apu_secure.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2024 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_APU_SECURE_H__
> +#define __MTK_APU_SECURE_H__

Please, __MTK --> __MEDIATEK (also because the folder is called "mediatek", anyway)

> +
> +enum mtk_apusys_kernel_op {

here you can keep the prefix as mtk_

> +	MTK_APUSYS_KERNEL_OP_APUSYS_PWR_TOP_ON,			/*  0 */

and MTK_ is ok for those definitions too.

Cheers,
Angelo

> +	MTK_APUSYS_KERNEL_OP_APUSYS_PWR_TOP_OFF,		/*  1 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_SETUP_REVISER,		/*  2 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_RESET_MP,		/*  3 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_SETUP_BOOT,		/*  4 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_START_MP,		/*  5 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_STOP_MP,			/*  6 */
> +	MTK_APUSYS_KERNEL_OP_DEVAPC_INIT_RCX,			/*  7 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_SETUP_SEC_MEM,		/*  8 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_DISABLE_WDT_ISR,		/*  9 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_CLEAR_WDT_ISR,		/* 10 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_CG_GATING,		/* 11 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_CG_UNGATING,		/* 12 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_RV_SETUP_APUMMU,		/* 13 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_LOGTOP_REG_DUMP,		/* 14 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_LOGTOP_REG_WRITE,		/* 15 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_LOGTOP_REG_W1C,		/* 16 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_COLD_BOOT_CLR_MBOX_DUMMY,	/* 17 */
> +	MTK_APUSYS_KERNEL_OP_APUSYS_SETUP_CE_BIN,		/* 18 */
> +	MTK_APUSYS_KERNEL_OP_NUM,
> +};
> +
> +#endif