Hi, Angelo:
On Thu, 2024-12-05 at 12:45 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>
>
> In preparation for adding a new driver for the HDMI TX v2 IP,
> split out the functions that will be common between the already
> present mtk_hdmi (v1) driver and the new one.
>
> Since the probe flow for both drivers is 90% similar, add a common
> probe function that will be called from each driver's .probe()
> callback, avoiding lots of code duplication.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> ---
[snip]
> +static void mtk_hdmi_unregister_audio_driver(void *data)
> +{
> + platform_device_unregister(data);
> +}
> +
> +static int mtk_hdmi_register_audio_driver(struct device *dev)
> +{
> + struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> + struct hdmi_audio_param *aud_param = &hdmi->aud_param;
> + struct hdmi_codec_pdata codec_data = {
> + .i2s = 1,
> + .max_i2s_channels = 2,
> + .data = hdmi,
> + .ops = hdmi->conf->ver_conf->codec_ops
> + };
> + int ret;
> +
> + aud_param->aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
> + aud_param->aud_sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
> + aud_param->aud_input_type = HDMI_AUD_INPUT_I2S;
> + aud_param->aud_i2s_fmt = HDMI_I2S_MODE_I2S_24BIT;
> + aud_param->aud_mclk = HDMI_AUD_MCLK_128FS;
> + aud_param->aud_input_chan_type = HDMI_AUD_CHAN_TYPE_2_0;
You squash mtk_hdmi_outout_init() into this function.
I'm not sure this is necessary or not.
If it's necessary, I would like this modification to be a separate patch and it's easier to review.
I want this patch is simply moving common code.
And this patch remove hdmi->csp and it is really not necessary.
Let removing hdmi->csp be a clean up patch.
> +
> + hdmi->audio_pdev = platform_device_register_data(dev,
> + HDMI_CODEC_DRV_NAME,
> + PLATFORM_DEVID_AUTO,
> + &codec_data,
> + sizeof(codec_data));
> + if (IS_ERR(hdmi->audio_pdev))
> + return PTR_ERR(hdmi->audio_pdev);
> +
> + ret = devm_add_action_or_reset(dev, mtk_hdmi_unregister_audio_driver,
> + hdmi->audio_pdev);
This adding action or reset is other than moving common function.
So separate this to a refinement patch.
Regards,
CK
> + if (ret) {
> + platform_device_unregister(hdmi->audio_pdev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
