linux-kernel - Re: [PATCH v14 09/10] drm/mediatek: DP audio support for MT8195

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <32ae9ba6-55aa-1663-05f6-20a637604f70@collabora.com>
Date:   Thu, 14 Jul 2022 13:43:01 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Bo-Chen Chen <rex-bc.chen@...iatek.com>, chunkuang.hu@...nel.org,
        p.zabel@...gutronix.de, daniel@...ll.ch, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, mripard@...nel.org,
        tzimmermann@...e.de, matthias.bgg@...il.com, deller@....de,
        airlied@...ux.ie
Cc:     msp@...libre.com, granquet@...libre.com, jitao.shi@...iatek.com,
        wenst@...omium.org, ck.hu@...iatek.com, liangxu.xu@...iatek.com,
        dri-devel@...ts.freedesktop.org,
        linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-fbdev@...r.kernel.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v14 09/10] drm/mediatek: DP audio support for MT8195

Il 12/07/22 13:12, Bo-Chen Chen ha scritto:
> From: Guillaume Ranquet <granquet@...libre.com>
> 
> This patch adds audio support to the DP driver for MT8195 with up to 8
> channels.
> 
> Signed-off-by: Guillaume Ranquet <granquet@...libre.com>
> Signed-off-by: Bo-Chen Chen <rex-bc.chen@...iatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_dp.c     | 723 ++++++++++++++++++++++++++
>   drivers/gpu/drm/mediatek/mtk_dp_reg.h |   2 +
>   2 files changed, 725 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 5ab646bd2bc4..fa7bb102a105 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c

..snip..

> @@ -2082,6 +2693,104 @@ static void mtk_dp_debounce_timer(struct timer_list *t)
>   	mtk_dp->need_debounce = true;
>   }
>   
> +/*
> + * HDMI audio codec callbacks
> + */
> +static int mtk_dp_audio_hw_params(struct device *dev, void *data,
> +				  struct hdmi_codec_daifmt *daifmt,
> +				  struct hdmi_codec_params *params)
> +{
> +	struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
> +	struct mtk_dp_audio_cfg cfg;
> +
> +	if (!mtk_dp->enabled) {
> +		pr_err("%s, DP is not ready!\n", __func__);

drm_err() here please.

> +		return -ENODEV;
> +	}
> +
> +	cfg.channels = params->cea.channels;
> +	cfg.sample_rate = params->sample_rate;
> +	cfg.word_length_bits = 24;
> +
> +	mtk_dp_audio_setup(mtk_dp, &cfg);
> +
> +	return 0;
> +}
> +

..snip..

> +
> +static int mtk_dp_register_audio_driver(struct device *dev)
> +{
> +	struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
> +	struct hdmi_codec_pdata codec_data = {
> +		.ops = &mtk_dp_audio_codec_ops,
> +		.max_i2s_channels = 8,
> +		.i2s = 1,
> +		.data = mtk_dp,
> +	};
> +	struct platform_device *pdev;
> +
> +	pdev = platform_device_register_data(dev, HDMI_CODEC_DRV_NAME,
> +					     PLATFORM_DEVID_AUTO, &codec_data,
> +					     sizeof(codec_data));
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);

You're never unregistering this device, which is unacceptable.

Please add a platform device pointer in struct mtk_dp, so that this function
becomes, simply:

	mtk_dp->audio_pdev = platform_device_register_data(dev,
						HDMI_CODEC_DRV_NAME,
						PLATFORM_DEVID_AUTO, &codec_data,
						sizeof(codec_data));
	return PTR_ERR_OR_ZERO(mtk_dp->audio_pdev);

> +
> +	return 0;
> +}
> +
>   static int mtk_dp_probe(struct platform_device *pdev)
>   {
>   	struct mtk_dp *mtk_dp;
> @@ -2127,8 +2836,21 @@ static int mtk_dp_probe(struct platform_device *pdev)
>   		return dev_err_probe(dev, -EPROBE_DEFER,
>   				     "failed to request mediatek dptx irq\n");
>   
> +	mutex_init(&mtk_dp->edid_lock);
> +	mutex_init(&mtk_dp->eld_lock);
> +	mutex_init(&mtk_dp->update_plugged_status_lock);
> +
>   	platform_set_drvdata(pdev, mtk_dp);
>   
> +	if (!mtk_dp_is_edp(mtk_dp)) {
> +		ret = mtk_dp_register_audio_driver(dev);
> +		if (ret) {
> +			dev_err(dev, "Failed to register audio driver: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
>   	mtk_dp->phy_dev = platform_device_register_data(dev, "mediatek-dp-phy",
>   							PLATFORM_DEVID_AUTO,
>   							&mtk_dp->regs,
> @@ -2174,6 +2896,7 @@ static int mtk_dp_remove(struct platform_device *pdev)
>   
>   	platform_device_unregister(mtk_dp->phy_dev);

... and unregister it here:

	if (mtk_dp->audio_pdev)
		platform_device_unregister(mtk_dp->audio_pdev);

>   	mtk_dp_video_mute(mtk_dp, true);
> +	mtk_dp_audio_mute(mtk_dp, true);
>   	del_timer_sync(&mtk_dp->debounce_timer);
>   
>   	pm_runtime_disable(&pdev->dev);

Regards,
Angelo