Re: [PATCH 1/6] ASoC: Intel: catpt: Fix offset checks

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2025-11-24 3:31 PM, Andy Shevchenko wrote:
> On Fri, Nov 21, 2025 at 12:43:26PM +0100, Cezary Rojewski wrote:
>> Verify if the entire block is found within DRAM, not just the start of
>> it.
> 
> I would rewrap this to have less distant lengths.
> 
> E.g.,
> 
> Verify if the entire block is found within DRAM, not just
> the start of it.
> 

Sure, will do. Typically I just do 72-chars per line and whatever 
happens, happens.

>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> 
> ...
> 
>> static int catpt_restore_memdumps(struct catpt_dev *cdev, struct dma_chan *chan)
> 
>>   		off = catpt_to_host_offset(info->offset);
>> -		if (off < cdev->dram.start || off > cdev->dram.end)
>> +		if (off < cdev->dram.start || off + info->size >= cdev->dram.end)
>>   			continue;
> 
> Hmm... I would rather do something like
> 
> 		struct resource r;
> 		...
> 		resource_set_range(catpt_to_host_offset(info->offset), info->size);
> 		if (!resource_contains())
> 			continue;
> 
>>   		dev_dbg(cdev->dev, "restoring memdump: off 0x%08x size %d\n",
> 
> OTOH it seems more invasive change for kinda a fix.

Looks elegant though. My idea was to avoid additional operations (which 
resource_contains() does), perhaps unnecessarily as readability suffered 
because of that. Ack.

> 
> ...
> 
>> static int catpt_restore_fwimage(struct catpt_dev *cdev,
> 
>>   		off = catpt_to_host_offset(info->offset);
>> -		if (off < cdev->dram.start || off > cdev->dram.end)
>> -			continue;
>> -
>>   		r2.start = off;
>>   		r2.end = r2.start + info->size - 1;
>>   
>> +		if (r2.start < cdev->dram.start || r2.end > cdev->dram.end)
>> +			continue;
> 
> See above, but here we already good for use the respective resource_*() API.
> 
>>   		if (!resource_intersection(&r2, &r1, &common))
>>   			continue;
>