On 2020-10-10 10:15 PM, Dan Carpenter wrote:
Hello Cezary Rojewski,
The patch a9aa6fb3eb6c: "ASoC: Intel: catpt: Firmware loading and context restore" from Sep 29, 2020, leads to the following static checker warning:
sound/soc/intel/catpt/loader.c:654 catpt_first_boot_firmware() warn: consider using resource_size() here
sound/soc/intel/catpt/loader.c 638 int catpt_first_boot_firmware(struct catpt_dev *cdev) 639 { 640 struct resource *res; 641 int ret; 642 643 ret = catpt_boot_firmware(cdev, false); 644 if (ret) { 645 dev_err(cdev->dev, "basefw boot failed: %d\n", ret); 646 return ret; 647 } 648 649 /* restrict FW Core dump area */ 650 __request_region(&cdev->dram, 0, 0x200, NULL, 0); 651 /* restrict entire area following BASE_FW - highest offset in DRAM */ 652 for (res = cdev->dram.child; res->sibling; res = res->sibling) 653 ; 654 __request_region(&cdev->dram, res->end + 1, 655 cdev->dram.end - res->end, NULL, 0); ^^^^^^^^^^^^^^^^^^^^^^^^^ It's been years since I have seen one of these warnings. Back in the day we used have have a lot of off by one warnings because resource_size() is supposed to be calculated as "end - start + 1". But here we are calculating "dram.end - res->end" so I'm not sure if the math is correct or not. This is very new code so hopefully you know the answer off the top of your head?
Hello Dan,
Thanks for your report. However, I do not see any problem with above code.
Let me elaborate the context so both parties are aware of what's going on: Host (kernel driver) is tasked with reserving entire region of DRAM past the module (aka resource) with highest offset. Offset is unknown upfront. It needs to be searched for. So, after all modules have their adequate DRAM space assigned, we search for the very last module. Its 'end + 1' marks the beginning of DSP-reserved region, that is one that DSP firmware will be making use of internally. Host needs to reserve it early so further reservations (done e.g.: on audio stream open) won't target said region. 'dram.end' on the other hand marks the end of the entire block - past that point IRAM begins. Area in-between is what we need to flag as already in-use.
Some basic math/example: dram.start = 0x0 dram.end = 0xAFFFF resource_size(dram) = 0xAFFFF - 0x0 + 1 = 0xB0000
found 'res': res.end = 0x9FFFF
area to be reserved: target.start = res.end + 1 = 0xA0000 target.end = dram.end = 0xAFFFF resource_size(target) = 0xAFFFF - 0xA0000 + 1 = 0x10000
and so: dram.end - res.end = 0xAFFFF - 0x9FFFF = 0x10000 what equals resource_size(target).
Please, let me know if I'm missing something here.
Thanks, Czarek