On 7/26/22 8:47 PM, Dan Carpenter wrote:
> Hello Vijendar Mukunda,
>
> The patch 4c33e5179ff1: "drm/amdgpu: create I2S platform devices for
> Jadeite platform" from Jun 30, 2022, leads to the following Smatch
> static checker warning:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:393 acp_hw_init()
> error: buffer overflow 'i2s_pdata' 3 <= 3
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:396 acp_hw_init()
> error: buffer overflow 'i2s_pdata' 3 <= 3
will fix it and provide a patch.
--
Vijendar
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> 225 static int acp_hw_init(void *handle)
> 226 {
> 227 int r;
> 228 u64 acp_base;
> 229 u32 val = 0;
> 230 u32 count = 0;
> 231 struct i2s_platform_data *i2s_pdata = NULL;
> 232
> 233 struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> 234
> 235 const struct amdgpu_ip_block *ip_block =
> 236 amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_ACP);
> 237
> 238 if (!ip_block)
> 239 return -EINVAL;
> 240
> 241 r = amd_acp_hw_init(adev->acp.cgs_device,
> 242 ip_block->version->major, ip_block->version->minor);
> 243 /* -ENODEV means board uses AZ rather than ACP */
> 244 if (r == -ENODEV) {
> 245 amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_ACP, true);
> 246 return 0;
> 247 } else if (r) {
> 248 return r;
> 249 }
> 250
> 251 if (adev->rmmio_size == 0 || adev->rmmio_size < 0x5289)
> 252 return -EINVAL;
> 253
> 254 acp_base = adev->rmmio_base;
> 255 adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL);
> 256 if (!adev->acp.acp_genpd)
> 257 return -ENOMEM;
> 258
> 259 adev->acp.acp_genpd->gpd.name = "ACP_AUDIO";
> 260 adev->acp.acp_genpd->gpd.power_off = acp_poweroff;
> 261 adev->acp.acp_genpd->gpd.power_on = acp_poweron;
> 262 adev->acp.acp_genpd->adev = adev;
> 263
> 264 pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
> 265 dmi_check_system(acp_quirk_table);
> 266 switch (acp_machine_id) {
> 267 case ST_JADEITE:
> 268 {
> 269 adev->acp.acp_cell = kcalloc(2, sizeof(struct mfd_cell),
> 270 GFP_KERNEL);
> 271 if (!adev->acp.acp_cell) {
> 272 r = -ENOMEM;
> 273 goto failure;
> 274 }
> 275
> 276 adev->acp.acp_res = kcalloc(3, sizeof(struct resource), GFP_KERNEL);
> 277 if (!adev->acp.acp_res) {
> 278 r = -ENOMEM;
> 279 goto failure;
> 280 }
> 281
> 282 i2s_pdata = kcalloc(1, sizeof(struct i2s_platform_data), GFP_KERNEL);
> 283 if (!i2s_pdata) {
> 284 r = -ENOMEM;
> 285 goto failure;
> 286 }
> 287
> 288 i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |
> 289 DW_I2S_QUIRK_16BIT_IDX_OVERRIDE;
> 290 i2s_pdata[0].cap = DWC_I2S_PLAY | DWC_I2S_RECORD;
> 291 i2s_pdata[0].snd_rates = SNDRV_PCM_RATE_8000_96000;
> 292 i2s_pdata[0].i2s_reg_comp1 = ACP_I2S_COMP1_CAP_REG_OFFSET;
> 293 i2s_pdata[0].i2s_reg_comp2 = ACP_I2S_COMP2_CAP_REG_OFFSET;
> 294
> 295 adev->acp.acp_res[0].name = "acp2x_dma";
> 296 adev->acp.acp_res[0].flags = IORESOURCE_MEM;
> 297 adev->acp.acp_res[0].start = acp_base;
> 298 adev->acp.acp_res[0].end = acp_base + ACP_DMA_REGS_END;
> 299
> 300 adev->acp.acp_res[1].name = "acp2x_dw_i2s_play_cap";
> 301 adev->acp.acp_res[1].flags = IORESOURCE_MEM;
> 302 adev->acp.acp_res[1].start = acp_base + ACP_I2S_CAP_REGS_START;
> 303 adev->acp.acp_res[1].end = acp_base + ACP_I2S_CAP_REGS_END;
> 304
> 305 adev->acp.acp_res[2].name = "acp2x_dma_irq";
> 306 adev->acp.acp_res[2].flags = IORESOURCE_IRQ;
> 307 adev->acp.acp_res[2].start = amdgpu_irq_create_mapping(adev, 162);
> 308 adev->acp.acp_res[2].end = adev->acp.acp_res[2].start;
> 309
> 310 adev->acp.acp_cell[0].name = "acp_audio_dma";
> 311 adev->acp.acp_cell[0].num_resources = 3;
> 312 adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
> 313 adev->acp.acp_cell[0].platform_data = &adev->asic_type;
> 314 adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
> 315
> 316 adev->acp.acp_cell[1].name = "designware-i2s";
> 317 adev->acp.acp_cell[1].num_resources = 1;
> 318 adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
> 319 adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
> 320 adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
> 321 r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, 2);
> 322 if (r)
> 323 goto failure;
> 324 r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
> 325 acp_genpd_add_device);
> 326 if (r)
> 327 goto failure;
> 328 break;
> 329 }
> 330 default:
> 331 adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> 332 GFP_KERNEL);
> 333
> 334 if (!adev->acp.acp_cell) {
> 335 r = -ENOMEM;
> 336 goto failure;
> 337 }
> 338
> 339 adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
> 340 if (!adev->acp.acp_res) {
> 341 r = -ENOMEM;
> 342 goto failure;
> 343 }
> 344
> 345 i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
> ^
> 3 elements
>
> 346 if (!i2s_pdata) {
> 347 r = -ENOMEM;
> 348 goto failure;
> 349 }
> 350
> 351 switch (adev->asic_type) {
> 352 case CHIP_STONEY:
> 353 i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |
> 354 DW_I2S_QUIRK_16BIT_IDX_OVERRIDE;
> 355 break;
> 356 default:
> 357 i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET;
> 358 }
> 359 i2s_pdata[0].cap = DWC_I2S_PLAY;
> 360 i2s_pdata[0].snd_rates = SNDRV_PCM_RATE_8000_96000;
> 361 i2s_pdata[0].i2s_reg_comp1 = ACP_I2S_COMP1_PLAY_REG_OFFSET;
> 362 i2s_pdata[0].i2s_reg_comp2 = ACP_I2S_COMP2_PLAY_REG_OFFSET;
> 363 switch (adev->asic_type) {
> 364 case CHIP_STONEY:
> 365 i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |
> 366 DW_I2S_QUIRK_COMP_PARAM1 |
> 367 DW_I2S_QUIRK_16BIT_IDX_OVERRIDE;
> 368 break;
> 369 default:
> 370 i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |
> 371 DW_I2S_QUIRK_COMP_PARAM1;
> 372 }
> 373
> 374 i2s_pdata[1].cap = DWC_I2S_RECORD;
> 375 i2s_pdata[1].snd_rates = SNDRV_PCM_RATE_8000_96000;
> 376 i2s_pdata[1].i2s_reg_comp1 = ACP_I2S_COMP1_CAP_REG_OFFSET;
> 377 i2s_pdata[1].i2s_reg_comp2 = ACP_I2S_COMP2_CAP_REG_OFFSET;
> 378
> 379 i2s_pdata[2].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET;
> 380 switch (adev->asic_type) {
> 381 case CHIP_STONEY:
> 382 i2s_pdata[2].quirks |= DW_I2S_QUIRK_16BIT_IDX_OVERRIDE;
> 383 break;
> 384 default:
> 385 break;
> 386 }
> 387
> 388 i2s_pdata[2].cap = DWC_I2S_PLAY | DWC_I2S_RECORD;
> 389 i2s_pdata[2].snd_rates = SNDRV_PCM_RATE_8000_96000;
> 390 i2s_pdata[2].i2s_reg_comp1 = ACP_BT_COMP1_REG_OFFSET;
> 391 i2s_pdata[2].i2s_reg_comp2 = ACP_BT_COMP2_REG_OFFSET;
> 392
> --> 393 i2s_pdata[3].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET;
> ^^^^^^^^^^^^
>
> 394 switch (adev->asic_type) {
> 395 case CHIP_STONEY:
> 396 i2s_pdata[3].quirks |= DW_I2S_QUIRK_16BIT_IDX_OVERRIDE;
> ^^^^^^^^^^^^
>
> Out of boundses.
>
> 397 break;
> 398 default:
> 399 break;
> 400 }
> 401 adev->acp.acp_res[0].name = "acp2x_dma";
> 402 adev->acp.acp_res[0].flags = IORESOURCE_MEM;
> 403 adev->acp.acp_res[0].start = acp_base;
> 404 adev->acp.acp_res[0].end = acp_base + ACP_DMA_REGS_END;
> 405
> 406 adev->acp.acp_res[1].name = "acp2x_dw_i2s_play";
> 407 adev->acp.acp_res[1].flags = IORESOURCE_MEM;
> 408 adev->acp.acp_res[1].start = acp_base + ACP_I2S_PLAY_REGS_START;
> 409 adev->acp.acp_res[1].end = acp_base + ACP_I2S_PLAY_REGS_END;
> 410
> 411 adev->acp.acp_res[2].name = "acp2x_dw_i2s_cap";
> 412 adev->acp.acp_res[2].flags = IORESOURCE_MEM;
> 413 adev->acp.acp_res[2].start = acp_base + ACP_I2S_CAP_REGS_START;
> 414 adev->acp.acp_res[2].end = acp_base + ACP_I2S_CAP_REGS_END;
> 415
> 416 adev->acp.acp_res[3].name = "acp2x_dw_bt_i2s_play_cap";
> 417 adev->acp.acp_res[3].flags = IORESOURCE_MEM;
> 418 adev->acp.acp_res[3].start = acp_base + ACP_BT_PLAY_REGS_START;
> 419 adev->acp.acp_res[3].end = acp_base + ACP_BT_PLAY_REGS_END;
> 420
> 421 adev->acp.acp_res[4].name = "acp2x_dma_irq";
> 422 adev->acp.acp_res[4].flags = IORESOURCE_IRQ;
> 423 adev->acp.acp_res[4].start = amdgpu_irq_create_mapping(adev, 162);
> 424 adev->acp.acp_res[4].end = adev->acp.acp_res[4].start;
> 425
> 426 adev->acp.acp_cell[0].name = "acp_audio_dma";
> 427 adev->acp.acp_cell[0].num_resources = 5;
> 428 adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0];
> 429 adev->acp.acp_cell[0].platform_data = &adev->asic_type;
> 430 adev->acp.acp_cell[0].pdata_size = sizeof(adev->asic_type);
> 431
> 432 adev->acp.acp_cell[1].name = "designware-i2s";
> 433 adev->acp.acp_cell[1].num_resources = 1;
> 434 adev->acp.acp_cell[1].resources = &adev->acp.acp_res[1];
> 435 adev->acp.acp_cell[1].platform_data = &i2s_pdata[0];
> 436 adev->acp.acp_cell[1].pdata_size = sizeof(struct i2s_platform_data);
> 437
> 438 adev->acp.acp_cell[2].name = "designware-i2s";
> 439 adev->acp.acp_cell[2].num_resources = 1;
> 440 adev->acp.acp_cell[2].resources = &adev->acp.acp_res[2];
> 441 adev->acp.acp_cell[2].platform_data = &i2s_pdata[1];
> 442 adev->acp.acp_cell[2].pdata_size = sizeof(struct i2s_platform_data);
> 443
> 444 adev->acp.acp_cell[3].name = "designware-i2s";
> 445 adev->acp.acp_cell[3].num_resources = 1;
> 446 adev->acp.acp_cell[3].resources = &adev->acp.acp_res[3];
> 447 adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
> 448 adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
> 449
> 450 r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, ACP_DEVS);
> 451 if (r)
> 452 goto failure;
> 453
> 454 r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
> 455 acp_genpd_add_device);
> 456 if (r)
> 457 goto failure;
> 458 }
> 459
> 460 /* Assert Soft reset of ACP */
> 461 val = cgs_read_register(adev->acp.cgs_device, mmACP_SOFT_RESET);
> 462
> 463 val |= ACP_SOFT_RESET__SoftResetAud_MASK;
> 464 cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
> 465
> 466 count = ACP_SOFT_RESET_DONE_TIME_OUT_VALUE;
> 467 while (true) {
> 468 val = cgs_read_register(adev->acp.cgs_device, mmACP_SOFT_RESET);
> 469 if (ACP_SOFT_RESET__SoftResetAudDone_MASK ==
> 470 (val & ACP_SOFT_RESET__SoftResetAudDone_MASK))
> 471 break;
> 472 if (--count == 0) {
> 473 dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
> 474 r = -ETIMEDOUT;
> 475 goto failure;
> 476 }
> 477 udelay(100);
> 478 }
> 479 /* Enable clock to ACP and wait until the clock is enabled */
> 480 val = cgs_read_register(adev->acp.cgs_device, mmACP_CONTROL);
> 481 val = val | ACP_CONTROL__ClkEn_MASK;
> 482 cgs_write_register(adev->acp.cgs_device, mmACP_CONTROL, val);
> 483
> 484 count = ACP_CLOCK_EN_TIME_OUT_VALUE;
> 485
> 486 while (true) {
> 487 val = cgs_read_register(adev->acp.cgs_device, mmACP_STATUS);
> 488 if (val & (u32) 0x1)
> 489 break;
> 490 if (--count == 0) {
> 491 dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
> 492 r = -ETIMEDOUT;
> 493 goto failure;
> 494 }
> 495 udelay(100);
> 496 }
> 497 /* Deassert the SOFT RESET flags */
> 498 val = cgs_read_register(adev->acp.cgs_device, mmACP_SOFT_RESET);
> 499 val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
> 500 cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
> 501 return 0;
> 502
> 503 failure:
> 504 kfree(i2s_pdata);
> 505 kfree(adev->acp.acp_res);
> 506 kfree(adev->acp.acp_cell);
> 507 kfree(adev->acp.acp_genpd);
> 508 return r;
> 509 }
>
> regards,
> dan carpenter
