Re: [PATCH V8 4/5] ASoC: codecs: Aw883xx chip register file, data type file and Kconfig Makefile

From: Dan Carpenter
Date: Fri Jan 06 2023 - 02:10:29 EST


Hi,

url: https://github.com/intel-lab-lkp/linux/commits/wangweidong-a-awinic-com/ASoC-codecs-Add-i2c-and-codec-registration-for-aw883xx-and-their-associated-operation-functions/20221230-173723
base: bff687b3dad6e0e56b27f4d3ed8a9695f35c7b1a
patch link: https://lore.kernel.org/r/20221230093454.190579-5-wangweidong.a%40awinic.com
patch subject: [PATCH V8 4/5] ASoC: codecs: Aw883xx chip register file, data type file and Kconfig Makefile
config: loongarch-randconfig-m031-20230103
compiler: loongarch64-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Reported-by: Dan Carpenter <error27@xxxxxxxxx>

New smatch warnings:
sound/soc/codecs/aw883xx/aw883xx_device.c:1163 aw_dev_dsp_update_container() warn: inconsistent returns '&aw_dev->dsp_lock'.
sound/soc/codecs/aw883xx/aw883xx_device.c:1303 aw_dev_check_sram() warn: inconsistent returns '&aw_dev->dsp_lock'.

Old smatch warnings:
sound/soc/codecs/aw883xx/aw883xx_device.c:1078 aw_dev_update_reg_container() error: uninitialized symbol 'ret'.
sound/soc/codecs/aw883xx/aw883xx_device.c:1271 aw_dev_check_sram() warn: missing unwind goto?

vim +1163 sound/soc/codecs/aw883xx/aw883xx_device.c

acf2ebfd20ae60 Weidong Wang 2022-12-30 1120 static int aw_dev_dsp_update_container(struct aw_device *aw_dev,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1121 unsigned char *data, unsigned int len, unsigned short base)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1122 {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1123 #ifdef AW_DSP_I2C_WRITES

These ifdefs are not ideal.

acf2ebfd20ae60 Weidong Wang 2022-12-30 1124 u32 tmp_len;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1125 #else
acf2ebfd20ae60 Weidong Wang 2022-12-30 1126 u16 reg_val;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1127 #endif
acf2ebfd20ae60 Weidong Wang 2022-12-30 1128 int i, ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1129
acf2ebfd20ae60 Weidong Wang 2022-12-30 1130 mutex_lock(&aw_dev->dsp_lock);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1131 #ifdef AW_DSP_I2C_WRITES
acf2ebfd20ae60 Weidong Wang 2022-12-30 1132 ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, base);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1133 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1134 return ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1135
acf2ebfd20ae60 Weidong Wang 2022-12-30 1136 for (i = 0; i < len; i += AW_MAX_RAM_WRITE_BYTE_SIZE) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1137 if ((len - i) < AW_MAX_RAM_WRITE_BYTE_SIZE)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1138 tmp_len = len - i;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1139 else
acf2ebfd20ae60 Weidong Wang 2022-12-30 1140 tmp_len = AW_MAX_RAM_WRITE_BYTE_SIZE;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1141
acf2ebfd20ae60 Weidong Wang 2022-12-30 1142 ret = regmap_raw_write(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1143 &data[i], tmp_len);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1144 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1145 return ret;

Needs unlock before returning.

acf2ebfd20ae60 Weidong Wang 2022-12-30 1146 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1147
acf2ebfd20ae60 Weidong Wang 2022-12-30 1148 #else
acf2ebfd20ae60 Weidong Wang 2022-12-30 1149 /* i2c write */
acf2ebfd20ae60 Weidong Wang 2022-12-30 1150 ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, base);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1151 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1152 return ret;

Here too

acf2ebfd20ae60 Weidong Wang 2022-12-30 1153 for (i = 0; i < len; i += 2) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1154 reg_val = (data[i] << 8) + data[i + 1];
acf2ebfd20ae60 Weidong Wang 2022-12-30 1155 ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMDAT_REG,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1156 reg_val);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1157 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1158 return ret;

Here.

acf2ebfd20ae60 Weidong Wang 2022-12-30 1159 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1160 #endif
acf2ebfd20ae60 Weidong Wang 2022-12-30 1161 mutex_unlock(&aw_dev->dsp_lock);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1162
acf2ebfd20ae60 Weidong Wang 2022-12-30 @1163 return 0;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1164 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1165
acf2ebfd20ae60 Weidong Wang 2022-12-30 1166 static int aw_dev_dsp_update_fw(struct aw_device *aw_dev,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1167 unsigned char *data, unsigned int len)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1168 {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1169
acf2ebfd20ae60 Weidong Wang 2022-12-30 1170 dev_dbg(aw_dev->dev, "dsp firmware len:%d", len);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1171
acf2ebfd20ae60 Weidong Wang 2022-12-30 1172 if (len && (data != NULL)) {

Flip this around.

if (!len || !data)
return -EINVAL;

Always do error handling, not success handling.

acf2ebfd20ae60 Weidong Wang 2022-12-30 1173 aw_dev_dsp_update_container(aw_dev,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1174 data, len, AW_PID_2049_DSP_FW_ADDR);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1175 aw_dev->dsp_fw_len = len;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1176 } else {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1177 dev_err(aw_dev->dev, "dsp firmware data is null or len is 0");
acf2ebfd20ae60 Weidong Wang 2022-12-30 1178 return -EINVAL;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1179 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1180
acf2ebfd20ae60 Weidong Wang 2022-12-30 1181 return 0;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1182 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1183
acf2ebfd20ae60 Weidong Wang 2022-12-30 1184 static int aw_dev_copy_to_crc_dsp_cfg(struct aw_device *aw_dev,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1185 unsigned char *data, unsigned int size)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1186 {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1187 struct aw_sec_data_desc *crc_dsp_cfg = &aw_dev->crc_dsp_cfg;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1188
acf2ebfd20ae60 Weidong Wang 2022-12-30 1189 if (!crc_dsp_cfg->data) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1190 crc_dsp_cfg->data = devm_kzalloc(aw_dev->dev, size, GFP_KERNEL);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1191 if (!crc_dsp_cfg->data)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1192 return -ENOMEM;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1193 crc_dsp_cfg->len = size;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1194 } else if (crc_dsp_cfg->len < size) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1195 devm_kfree(aw_dev->dev, crc_dsp_cfg->data);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1196 crc_dsp_cfg->data = devm_kzalloc(aw_dev->dev, size, GFP_KERNEL);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1197 if (!crc_dsp_cfg->data) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1198 dev_err(aw_dev->dev, "error allocating memory");

I am surprised this error message does not generate a checkpatch
warning. kmalloc() has its own better warnings. Delete this one.

acf2ebfd20ae60 Weidong Wang 2022-12-30 1199 return -ENOMEM;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1200 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1201 crc_dsp_cfg->len = size;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1202 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1203 memcpy(crc_dsp_cfg->data, data, size);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1204 swab16_array((u16 *)crc_dsp_cfg->data, size >> 1);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1205
acf2ebfd20ae60 Weidong Wang 2022-12-30 1206 return 0;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1207 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1208
acf2ebfd20ae60 Weidong Wang 2022-12-30 1209 static int aw_dev_dsp_update_cfg(struct aw_device *aw_dev,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1210 unsigned char *data, unsigned int len)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1211 {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1212 int ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1213
acf2ebfd20ae60 Weidong Wang 2022-12-30 1214 dev_dbg(aw_dev->dev, "dsp config len:%d", len);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1215
acf2ebfd20ae60 Weidong Wang 2022-12-30 1216 if (len && (data != NULL)) {

Flip this around.

if (!len || !data)
return -EINVAL;

aw_dev_dsp_update_container(aw_dev, data, len,
AW_PID_2049_DSP_CFG_ADDR);

acf2ebfd20ae60 Weidong Wang 2022-12-30 1217 aw_dev_dsp_update_container(aw_dev,
acf2ebfd20ae60 Weidong Wang 2022-12-30 1218 data, len, AW_PID_2049_DSP_CFG_ADDR);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1219 aw_dev->dsp_cfg_len = len;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1220
acf2ebfd20ae60 Weidong Wang 2022-12-30 1221 ret = aw_dev_copy_to_crc_dsp_cfg(aw_dev, data, len);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1222 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1223 return ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1224
acf2ebfd20ae60 Weidong Wang 2022-12-30 1225 ret = aw_dev_set_vcalb(aw_dev);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1226 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1227 return ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1228 ret = aw_dev_get_ra(&aw_dev->cali_desc);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1229 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1230 return ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1231 ret = aw_dev_get_cali_f0_delay(aw_dev);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1232 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1233 return ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1234
acf2ebfd20ae60 Weidong Wang 2022-12-30 1235 ret = aw_dev_get_vmax(aw_dev, &aw_dev->vmax_desc.init_vmax);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1236 if (ret) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1237 dev_err(aw_dev->dev, "get vmax failed");
acf2ebfd20ae60 Weidong Wang 2022-12-30 1238 return ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1239 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1240 dev_dbg(aw_dev->dev, "get init vmax:0x%x",
acf2ebfd20ae60 Weidong Wang 2022-12-30 1241 aw_dev->vmax_desc.init_vmax);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1242 aw_dev->dsp_crc_st = AW_DSP_CRC_NA;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1243 } else {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1244 dev_err(aw_dev->dev, "dsp config data is null or len is 0");
acf2ebfd20ae60 Weidong Wang 2022-12-30 1245 return -EINVAL;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1246 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1247
acf2ebfd20ae60 Weidong Wang 2022-12-30 1248 return 0;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1249 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1250
acf2ebfd20ae60 Weidong Wang 2022-12-30 1251 static int aw_dev_check_sram(struct aw_device *aw_dev)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1252 {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1253 unsigned int reg_val;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1254 int ret;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1255
acf2ebfd20ae60 Weidong Wang 2022-12-30 1256 mutex_lock(&aw_dev->dsp_lock);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1257 /* check the odd bits of reg 0x40 */
acf2ebfd20ae60 Weidong Wang 2022-12-30 1258 ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, AW_DSP_ODD_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1259 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1260 return ret;

goto error;

acf2ebfd20ae60 Weidong Wang 2022-12-30 1261 ret = regmap_read(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, &reg_val);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1262 if (reg_val != AW_DSP_ODD_NUM_BIT_TEST || ret) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1263 dev_err(aw_dev->dev, "check reg 0x40 odd bit failed, read[0x%x] != write[0x%x]",
acf2ebfd20ae60 Weidong Wang 2022-12-30 1264 reg_val, AW_DSP_ODD_NUM_BIT_TEST);

This does not set the error code correctly. Technically, it reg_val is
unintialized if ret is negative so it's an uninitialized variable bug
as well. Write it like so:

ret = regmap_read(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, &reg_val);
if (ret)
goto error;
if (reg_val != AW_DSP_ODD_NUM_BIT_TEST) {
ret = -EINVAL;
goto error;
}

acf2ebfd20ae60 Weidong Wang 2022-12-30 1265 goto error;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1266 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1267
acf2ebfd20ae60 Weidong Wang 2022-12-30 1268 /* check the even bits of reg 0x40 */
acf2ebfd20ae60 Weidong Wang 2022-12-30 1269 ret = regmap_write(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, AW_DSP_EVEN_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1270 if (ret)
acf2ebfd20ae60 Weidong Wang 2022-12-30 1271 return ret;

goto error;

acf2ebfd20ae60 Weidong Wang 2022-12-30 1272 ret = regmap_read(aw_dev->regmap, AW_PID_2049_DSPMADD_REG, &reg_val);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1273 if (reg_val != AW_DSP_EVEN_NUM_BIT_TEST || ret) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1274 dev_err(aw_dev->dev, "check reg 0x40 even bit failed, read[0x%x] != write[0x%x]",
acf2ebfd20ae60 Weidong Wang 2022-12-30 1275 reg_val, AW_DSP_EVEN_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1276 goto error;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1277 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1278
acf2ebfd20ae60 Weidong Wang 2022-12-30 1279 /* check dsp_fw_base_addr */
acf2ebfd20ae60 Weidong Wang 2022-12-30 1280 aw_dev_dsp_write_16bit(aw_dev, AW_PID_2049_DSP_FW_ADDR, AW_DSP_EVEN_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1281 aw_dev_dsp_read_16bit(aw_dev, AW_PID_2049_DSP_FW_ADDR, &reg_val);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1282 if (reg_val != AW_DSP_EVEN_NUM_BIT_TEST) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1283 dev_err(aw_dev->dev, "check dsp fw addr failed, read[0x%x] != write[0x%x]",
acf2ebfd20ae60 Weidong Wang 2022-12-30 1284 reg_val, AW_DSP_EVEN_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1285 goto error;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1286 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1287
acf2ebfd20ae60 Weidong Wang 2022-12-30 1288 /* check dsp_cfg_base_addr */
acf2ebfd20ae60 Weidong Wang 2022-12-30 1289 aw_dev_dsp_write_16bit(aw_dev, AW_PID_2049_DSP_CFG_ADDR, AW_DSP_ODD_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1290
acf2ebfd20ae60 Weidong Wang 2022-12-30 1291 aw_dev_dsp_read_16bit(aw_dev, AW_PID_2049_DSP_CFG_ADDR, &reg_val);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1292 if (reg_val != AW_DSP_ODD_NUM_BIT_TEST) {
acf2ebfd20ae60 Weidong Wang 2022-12-30 1293 dev_err(aw_dev->dev, "check dsp cfg failed, read[0x%x] != write[0x%x]",
acf2ebfd20ae60 Weidong Wang 2022-12-30 1294 reg_val, AW_DSP_ODD_NUM_BIT_TEST);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1295 goto error;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1296 }
acf2ebfd20ae60 Weidong Wang 2022-12-30 1297
acf2ebfd20ae60 Weidong Wang 2022-12-30 1298 mutex_unlock(&aw_dev->dsp_lock);
acf2ebfd20ae60 Weidong Wang 2022-12-30 1299 return 0;
acf2ebfd20ae60 Weidong Wang 2022-12-30 1300
acf2ebfd20ae60 Weidong Wang 2022-12-30 1301 error:
acf2ebfd20ae60 Weidong Wang 2022-12-30 1302 mutex_unlock(&aw_dev->dsp_lock);
acf2ebfd20ae60 Weidong Wang 2022-12-30 @1303 return -EPERM;

Oh. Huh. Change this to "return ret;"

acf2ebfd20ae60 Weidong Wang 2022-12-30 1304 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests