Re: [PATCH v19 03/10] soc: mediatek: add mtk-mmsys support for mt8195 vdosys0

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

Il 15/04/22 10:39, jason-jh.lin ha scritto:
> 1. Add mt8195 mmsys compatible for 2 vdosys.
> 2. Add io_start into each driver data of mt8195 vdosys.
> 3. Add get match data function to identify mmsys by io_start.
> 4. Add mt8195 routing table settings of vdosys0.
> 
> Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>

Unless anyone wants this commit to be split in two, one for the match data
and one for the mt8195 addition, it looks almost good to me, apart one small
change that has to be done, check below:


> --- >   drivers/soc/mediatek/mt8195-mmsys.h    | 370 +++++++++++++++++++++++++
>   drivers/soc/mediatek/mtk-mmsys.c       | 152 +++++++++-
>   drivers/soc/mediatek/mtk-mmsys.h       |   6 +
>   include/linux/soc/mediatek/mtk-mmsys.h |  11 +
>   4 files changed, 528 insertions(+), 11 deletions(-)
>   create mode 100644 drivers/soc/mediatek/mt8195-mmsys.h
> 

..snip..

> diff --git a/drivers/soc/mediatek/mtk-mmsys.h b/drivers/soc/mediatek/mtk-mmsys.h
> index 77f37f8c715b..21cf300ba864 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.h
> +++ b/drivers/soc/mediatek/mtk-mmsys.h
> @@ -87,12 +87,18 @@ struct mtk_mmsys_routes {
>   };
>   
>   struct mtk_mmsys_driver_data {
> +	const u32 io_start;

I agree with you that this iostart is in 32bits boundaries, and that this will
practically never change... and I was in doubt whether this would be acceptable
or not, because of saving some memory.

Even though I would really love to have the savings, in order to avoid having any
"surprises" in the future (any future breakage for "reasons"), we shall comply
with the kernel types, so, this has to be...

	const resource_size_t io_start;

...and this is the same for both this file and mtk_drm_drv.h, which you modify in
patch 07/10.

After fixing that:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


Cheers,
Angelo