| 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
| ||
|
Message-ID: <9f8c2da3-8cce-4fb2-a863-4d79f524f58b@collabora.com>
Date: Wed, 8 Jan 2025 10:23:06 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.com>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
Cc: "robh@...nel.org" <robh@...nel.org>,
"jie.qiu@...iatek.com" <jie.qiu@...iatek.com>,
"tzimmermann@...e.de" <tzimmermann@...e.de>,
"simona@...ll.ch" <simona@...ll.ch>, "mripard@...nel.org"
<mripard@...nel.org>, Jitao Shi (石记涛)
<jitao.shi@...iatek.com>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"kernel@...labora.com" <kernel@...labora.com>,
"dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"airlied@...il.com" <airlied@...il.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"junzhi.zhao@...iatek.com" <junzhi.zhao@...iatek.com>
Subject: Re: [PATCH v3 14/33] drm/mediatek: mtk_hdmi: Move audio params
selection to new function
Il 20/12/24 04:47, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Tue, 2024-12-17 at 16:43 +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 splitting common bits of this driver, move the
>> audio params (codec, sample rate/size, input type, i2s format, etc)
>> selection to a new function called mtk_hdmi_audio_params().
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> ---
>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 46 +++++++++++++++++------------
>> 1 file changed, 27 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> index d2f1d6286fbc..8f5ab97a0261 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> @@ -1479,12 +1479,11 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
>> * HDMI audio codec callbacks
>> */
>>
>> -static int mtk_hdmi_audio_hw_params(struct device *dev, void *data,
>> - struct hdmi_codec_daifmt *daifmt,
>> - struct hdmi_codec_params *params)
>> +static int mtk_hdmi_audio_params(struct mtk_hdmi *hdmi,
>> + struct hdmi_codec_daifmt *daifmt,
>> + struct hdmi_codec_params *params)
>> {
>> - struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
>> - struct hdmi_audio_param hdmi_params;
>> + struct hdmi_audio_param aud_params = { 0 };
>> unsigned int chan = params->cea.channels;
>>
>> dev_dbg(hdmi->dev, "%s: %u Hz, %d bit, %d channels\n", __func__,
>> @@ -1495,16 +1494,16 @@ static int mtk_hdmi_audio_hw_params(struct device *dev, void *data,
>>
>> switch (chan) {
>> case 2:
>> - hdmi_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_2_0;
>> + aud_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_2_0;
>> break;
>> case 4:
>> - hdmi_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_4_0;
>> + aud_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_4_0;
>> break;
>> case 6:
>> - hdmi_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_5_1;
>> + aud_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_5_1;
>> break;
>> case 8:
>> - hdmi_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_7_1;
>> + aud_params.aud_input_chan_type = HDMI_AUD_CHAN_TYPE_7_1;
>> break;
>> default:
>> dev_err(hdmi->dev, "channel[%d] not supported!\n", chan);
>> @@ -1528,26 +1527,35 @@ static int mtk_hdmi_audio_hw_params(struct device *dev, void *data,
>>
>> switch (daifmt->fmt) {
>> case HDMI_I2S:
>> - hdmi_params.aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
>> - hdmi_params.aud_sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
>> - hdmi_params.aud_input_type = HDMI_AUD_INPUT_I2S;
>> - hdmi_params.aud_i2s_fmt = HDMI_I2S_MODE_I2S_24BIT;
>> - hdmi_params.aud_mclk = HDMI_AUD_MCLK_128FS;
>> + aud_params.aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
>> + aud_params.aud_sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
>> + aud_params.aud_input_type = HDMI_AUD_INPUT_I2S;
>> + aud_params.aud_i2s_fmt = HDMI_I2S_MODE_I2S_24BIT;
>> + aud_params.aud_mclk = HDMI_AUD_MCLK_128FS;
>> break;
>> case HDMI_SPDIF:
>> - hdmi_params.aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
>> - hdmi_params.aud_sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
>> - hdmi_params.aud_input_type = HDMI_AUD_INPUT_SPDIF;
>> + aud_params.aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
>> + aud_params.aud_sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
>> + aud_params.aud_input_type = HDMI_AUD_INPUT_SPDIF;
>> break;
>> default:
>> dev_err(hdmi->dev, "%s: Invalid DAI format %d\n", __func__,
>> daifmt->fmt);
>> return -EINVAL;
>> }
>> + memcpy(&aud_params.codec_params, params, sizeof(aud_params.codec_params));
>
> You copy to local variable, so this function does nothing.
>
>> +
>> + return 0;
>> +}
>>
>> - memcpy(&hdmi_params.codec_params, params,
>> - sizeof(hdmi_params.codec_params));
>> +static int mtk_hdmi_audio_hw_params(struct device *dev, void *data,
>> + struct hdmi_codec_daifmt *daifmt,
>> + struct hdmi_codec_params *params)
>> +{
>> + struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
>> + struct hdmi_audio_param hdmi_params;
>>
>> + mtk_hdmi_audio_params(hdmi, daifmt, params);
>> mtk_hdmi_audio_set_param(hdmi, &hdmi_params);
>
> hdmi_params has not been initialized.
That was done on purpose: we are anyway rewriting all fields of that structure
with the call to mtk_hdmi_audio_set_param(), so if that was stack-initialized
to zero, that would be a double initialization.
Cheers,
Angelo
Powered by blists - more mailing lists