Re: [PATCH v5 3/5] media: platform: mediatek: isp_30: add mediatek ISP3.0 sensor interface

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

Hi, Julien:

On Thu, 2024-07-04 at 15:36 +0200, Julien Stephan wrote:
>  	 
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>  From: Louis Kuo <louis.kuo@mediatek.com>
> 
> This will add the mediatek ISP3.0 seninf (sensor interface) driver found
> on several Mediatek SoCs such as the mt8365.
> 
> Then seninf module has 4 physical CSI-2 inputs. Depending on the soc they
> may not be all connected.
> 
> Signed-off-by: Louis Kuo <louis.kuo@mediatek.com>
> Signed-off-by: Phi-bang Nguyen <pnguyen@baylibre.com>
> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Co-developed-by: Julien Stephan <jstephan@baylibre.com>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---

[snip]

> +static const struct mtk_seninf_conf seninf_8365_conf = {
> +.model = "mtk-camsys-3.0",
> +.nb_inputs = 4,
> +.nb_muxes = 6,
> +.nb_outputs = 4,
> +};
> +

I think you should directly define these value as symbols because now
only support one SoC.

#define MODEL     "mtk-camsys-3.0"
#define INPUT_NR  4
#define MUTEX_NR  6
#define OUTPUT_NR 4

Because we don't know which SoC would be upstream later, maybe the next
SoC would be

static const struct mtk_seninf_conf seninf_83xx_conf = {
        .model = "mtk-camsys-3.0",
        .nb_inputs = 4,
        .nb_muxes = 6,
        .nb_outputs = 4,
        .support_xxx = true;
};

then model, nb_inputs, nb_muxes, and nb_outputs has no difference, so
it's not necessary to define them as variable. So define them as
constant now, and when next SoC upstream, then we know which one would
be variable.

Regards,
CK