Hi Dan,
On Tue, Oct 24, 2023 at 5:50 PM Dan Carpenter dan.carpenter@linaro.org wrote:
Hello xiazhengqiao,
The patch d88c43383101: "ASoC: mediatek: mt8186_mt6366_rt1019_rt5682s: add rt5650 support" from Oct 19, 2023 (linux-next), leads to the following Smatch static checker warning:
sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c:198mt8186_rt5682s_init() warn: does endianness matter for 'type'?
sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c 161 static int mt8186_rt5682s_init(struct snd_soc_pcm_runtime *rtd) 162 { 163 struct snd_soc_component *cmpnt_afe = 164 snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME); 165 struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt_afe); 166 struct mtk_soc_card_data *soc_card_data = 167 snd_soc_card_get_drvdata(rtd->card); 168 struct mt8186_mt6366_rt1019_rt5682s_priv *priv = soc_card_data->mach_priv; 169 struct snd_soc_jack *jack = &priv->headset_jack; 170 struct snd_soc_component *cmpnt_codec = 171 snd_soc_rtd_to_codec(rtd, 0)->component; 172 int ret; 173 int type; 174 175 ret = mt8186_dai_i2s_set_share(afe, "I2S1", "I2S0"); 176 if (ret) { 177 dev_err(rtd->dev, "Failed to set up shared clocks\n"); 178 return ret; 179 } 180 181 ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack", 182 SND_JACK_HEADSET | SND_JACK_BTN_0 | 183 SND_JACK_BTN_1 | SND_JACK_BTN_2 | 184 SND_JACK_BTN_3, 185 jack, mt8186_jack_pins, 186 ARRAY_SIZE(mt8186_jack_pins)); 187 if (ret) { 188 dev_err(rtd->dev, "Headset Jack creation failed: %d\n", ret); 189 return ret; 190 } 191 192 snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE); 193 snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOICECOMMAND); 194 snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEUP); 195 snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN); 196 197 type = SND_JACK_HEADSET | SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2 | SND_JACK_BTN_3; --> 198 return snd_soc_component_set_jack(cmpnt_codec, jack, (void *)&type);
This is an unpublished Smatch check where I manually review casts to see if they are correct. Quite often they aren't because of an endian bug or a 64 bit vs 32 bit issue.
Here it's not clear to me what's happening. Normally with this sort of pass a void pointer code, you can tie it very easily to the same driver. But in this case it's much more difficult.
There are two functions which use the void *data pointer, rt5640_set_jack() and rt5645_component_set_jack(). One takes an int and the other takes a struct rt5640_set_jack_data pointer. So presumably we know that the cmpnt_codec->driver->set_jack points to --> rt5645_component_set_jack(). But how do we know that?
--> Is there a trick for me as a reviewer to use?
static const struct of_device_id mt8186_mt6366_rt1019_rt5682s_dt_match[] = { { .compatible = "mediatek,mt8186-mt6366-rt1019-rt5682s-sound", .data = &mt8186_mt6366_rt1019_rt5682s_soc_card, }, { .compatible = "mediatek,mt8186-mt6366-rt5682s-max98360-sound", .data = &mt8186_mt6366_rt5682s_max98360_soc_card, }, { .compatible = "mediatek,mt8186-mt6366-rt5650-sound", .data = &mt8186_mt6366_rt5650_soc_card, }, {} };
This driver uses only two codecs: rt5650, rt5682 in mt8186-mt6366-rt1019-rt5682s.c, rt5650 uses rt5645_component_set_jack to set jack, '(*int* *)data http://172.31.93.46:8888/source/s?defs=data&project=v5.15' is used in the function, RT5682S does not use 'void * data' in rt5682s_set_jack_detect.
As for rt5640, we are not using it in this driver, so it will not run rt5640_set_jack.
if we want to know what function ''snd_soc_component_set_jack'' calls, we can see what codecs this driver uses in kconfig like:
config SND_SOC_MT8186_MT6366_RT1019_RT5682S tristate "ASoC Audio driver for MT8186 with RT1019 RT5682S MAX98357A/MAX98360 codec" depends on I2C && GPIOLIB depends on SND_SOC_MT8186 && MTK_PMIC_WRAP select SND_SOC_MAX98357A select SND_SOC_MT6358 select SND_SOC_MAX98357A select SND_SOC_RT1015P select SND_SOC_RT5682S select SND_SOC_RT5645 select SND_SOC_BT_SCO select SND_SOC_DMIC select SND_SOC_HDMI_CODEC help
199 }
regards, dan carpenter