[bug report] ASoC: mediatek: mt8173: Enable IRQ when pdata is ready
Ricardo Ribalda
ribalda at chromium.org
Mon Jun 12 00:42:04 PDT 2023
Hi Dan
Thanks for the heads-up
On Mon, 12 Jun 2023 at 09:06, Dan Carpenter <dan.carpenter at linaro.org> wrote:
>
> Hello Ricardo Ribalda,
>
> The patch 4cbb264d4e91: "ASoC: mediatek: mt8173: Enable IRQ when
> pdata is ready" from Nov 28, 2022, leads to the following Smatch
> static checker warning:
>
> sound/soc/mediatek/mt8173/mt8173-afe-pcm.c:1180 mt8173_afe_pcm_dev_probe()
> warn: missing unwind goto?
>
> sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
> 1049 static int mt8173_afe_pcm_dev_probe(struct platform_device *pdev)
> 1050 {
> 1051 int ret, i;
> 1052 int irq_id;
> 1053 struct mtk_base_afe *afe;
> 1054 struct mt8173_afe_private *afe_priv;
> 1055 struct snd_soc_component *comp_pcm, *comp_hdmi;
> 1056
> 1057 ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(33));
> 1058 if (ret)
> 1059 return ret;
> 1060
> 1061 afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL);
> 1062 if (!afe)
> 1063 return -ENOMEM;
> 1064
> 1065 afe->platform_priv = devm_kzalloc(&pdev->dev, sizeof(*afe_priv),
> 1066 GFP_KERNEL);
> 1067 afe_priv = afe->platform_priv;
> 1068 if (!afe_priv)
> 1069 return -ENOMEM;
> 1070
> 1071 afe->dev = &pdev->dev;
> 1072
> 1073 afe->base_addr = devm_platform_ioremap_resource(pdev, 0);
> 1074 if (IS_ERR(afe->base_addr))
> 1075 return PTR_ERR(afe->base_addr);
> 1076
> 1077 afe->regmap = devm_regmap_init_mmio(&pdev->dev, afe->base_addr,
> 1078 &mt8173_afe_regmap_config);
> 1079 if (IS_ERR(afe->regmap))
> 1080 return PTR_ERR(afe->regmap);
> 1081
> 1082 /* initial audio related clock */
> 1083 ret = mt8173_afe_init_audio_clk(afe);
> 1084 if (ret) {
> 1085 dev_err(afe->dev, "mt8173_afe_init_audio_clk fail\n");
> 1086 return ret;
> 1087 }
> 1088
> 1089 /* memif % irq initialize*/
> 1090 afe->memif_size = MT8173_AFE_MEMIF_NUM;
> 1091 afe->memif = devm_kcalloc(afe->dev, afe->memif_size,
> 1092 sizeof(*afe->memif), GFP_KERNEL);
> 1093 if (!afe->memif)
> 1094 return -ENOMEM;
> 1095
> 1096 afe->irqs_size = MT8173_AFE_IRQ_NUM;
> 1097 afe->irqs = devm_kcalloc(afe->dev, afe->irqs_size,
> 1098 sizeof(*afe->irqs), GFP_KERNEL);
> 1099 if (!afe->irqs)
> 1100 return -ENOMEM;
> 1101
> 1102 for (i = 0; i < afe->irqs_size; i++) {
> 1103 afe->memif[i].data = &memif_data[i];
> 1104 afe->irqs[i].irq_data = &irq_data[i];
> 1105 afe->irqs[i].irq_occupyed = true;
> 1106 afe->memif[i].irq_usage = i;
> 1107 afe->memif[i].const_irq = 1;
> 1108 }
> 1109
> 1110 afe->mtk_afe_hardware = &mt8173_afe_hardware;
> 1111 afe->memif_fs = mt8173_memif_fs;
> 1112 afe->irq_fs = mt8173_irq_fs;
> 1113
> 1114 platform_set_drvdata(pdev, afe);
> 1115
> 1116 pm_runtime_enable(&pdev->dev);
> 1117 if (!pm_runtime_enabled(&pdev->dev)) {
> 1118 ret = mt8173_afe_runtime_resume(&pdev->dev);
> 1119 if (ret)
> 1120 goto err_pm_disable;
> 1121 }
> 1122
> 1123 afe->reg_back_up_list = mt8173_afe_backup_list;
> 1124 afe->reg_back_up_list_num = ARRAY_SIZE(mt8173_afe_backup_list);
> 1125 afe->runtime_resume = mt8173_afe_runtime_resume;
> 1126 afe->runtime_suspend = mt8173_afe_runtime_suspend;
> 1127
> 1128 ret = devm_snd_soc_register_component(&pdev->dev,
> 1129 &mtk_afe_pcm_platform,
> 1130 NULL, 0);
> 1131 if (ret)
> 1132 goto err_pm_disable;
> 1133
> 1134 comp_pcm = devm_kzalloc(&pdev->dev, sizeof(*comp_pcm), GFP_KERNEL);
> 1135 if (!comp_pcm) {
> 1136 ret = -ENOMEM;
> 1137 goto err_pm_disable;
> 1138 }
> 1139
> 1140 ret = snd_soc_component_initialize(comp_pcm,
> 1141 &mt8173_afe_pcm_dai_component,
> 1142 &pdev->dev);
> 1143 if (ret)
> 1144 goto err_pm_disable;
> 1145
> 1146 #ifdef CONFIG_DEBUG_FS
> 1147 comp_pcm->debugfs_prefix = "pcm";
> 1148 #endif
> 1149
> 1150 ret = snd_soc_add_component(comp_pcm,
> 1151 mt8173_afe_pcm_dais,
> 1152 ARRAY_SIZE(mt8173_afe_pcm_dais));
> 1153 if (ret)
> 1154 goto err_pm_disable;
> 1155
> 1156 comp_hdmi = devm_kzalloc(&pdev->dev, sizeof(*comp_hdmi), GFP_KERNEL);
> 1157 if (!comp_hdmi) {
> 1158 ret = -ENOMEM;
> 1159 goto err_pm_disable;
> 1160 }
> 1161
> 1162 ret = snd_soc_component_initialize(comp_hdmi,
> 1163 &mt8173_afe_hdmi_dai_component,
> 1164 &pdev->dev);
> 1165 if (ret)
> 1166 goto err_pm_disable;
> 1167
> 1168 #ifdef CONFIG_DEBUG_FS
> 1169 comp_hdmi->debugfs_prefix = "hdmi";
> 1170 #endif
> 1171
> 1172 ret = snd_soc_add_component(comp_hdmi,
> 1173 mt8173_afe_hdmi_dais,
> 1174 ARRAY_SIZE(mt8173_afe_hdmi_dais));
> 1175 if (ret)
> 1176 goto err_cleanup_components;
> 1177
> 1178 irq_id = platform_get_irq(pdev, 0);
> 1179 if (irq_id <= 0)
> --> 1180 return irq_id < 0 ? irq_id : -ENXIO;
>
> The comments say platform_get_irq() can't return zero. I believe that
> the situation there is that official policy is that zero is an invalid
> IRQ but there are some out of tree ARM stuff where zero is a valid IRQ.
Not sure what you mean here. It looks to me that platform_get_irq can
indeed return a negative number:
https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L259
>
> Anyway, missing clean up as well.
Sending a patch right away.
Thanks!
>
> 1181 ret = devm_request_irq(afe->dev, irq_id, mt8173_afe_irq_handler,
> 1182 0, "Afe_ISR_Handle", (void *)afe);
> 1183 if (ret) {
> 1184 dev_err(afe->dev, "could not request_irq\n");
> 1185 goto err_pm_disable;
> 1186 }
> 1187
> 1188 dev_info(&pdev->dev, "MT8173 AFE driver initialized.\n");
> 1189 return 0;
> 1190
> 1191 err_cleanup_components:
> 1192 snd_soc_unregister_component(&pdev->dev);
> 1193 err_pm_disable:
> 1194 pm_runtime_disable(&pdev->dev);
> 1195 return ret;
> 1196 }
>
> regards,
> dan carpenter
--
Ricardo Ribalda
More information about the Linux-mediatek
mailing list