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

2022-07-26 Thread Mukunda,Vijendar 
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

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

2022-07-26 Thread Dan Carpenter 
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

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 =