[bug report] drm/amdgpu: create I2S platform devices for Jadeite platform

Mukunda,Vijendar vijendar.mukunda at amd.com
Wed Jul 27 06:36:29 UTC 2022


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



More information about the amd-gfx mailing list