Re: [PATCH 3/3] media: staging/ipu7: cleanup the MMU correctly in IPU7 driver release

[Bingbu Cao <bingbu.cao@linux.intel.com>]

Sakari,

On 8/16/25 5:26 AM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> Thanks for the set.
> 
> On Fri, Aug 15, 2025 at 05:20:37PM +0800, bingbu.cao@intel.com wrote:
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> IPU7 ISYS and PSYS auxiliary devices are released after
>> ipu7_bus_del_devices(), so driver can not reference the MMU devices
>> from ISYS and PSYS auxiliary devices, so move the MMUs cleanup before
>> releasing the auxiliary devices.
>>
>> Fixes: b7fe4c0019b1 ("media: staging/ipu7: add Intel IPU7 PCI device driver")
> 
> Please also add Cc: stable tag if you have a Fixes: tag.
> 
>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>> ---
>>  drivers/staging/media/ipu7/ipu7.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/media/ipu7/ipu7.c b/drivers/staging/media/ipu7/ipu7.c
>> index aef931d23510..3de41b3d1ae1 100644
>> --- a/drivers/staging/media/ipu7/ipu7.c
>> +++ b/drivers/staging/media/ipu7/ipu7.c
>> @@ -2644,6 +2644,9 @@ static void ipu7_pci_remove(struct pci_dev *pdev)
>>  	if (!IS_ERR_OR_NULL(isp->fw_code_region))
>>  		vfree(isp->fw_code_region);
>>  
>> +	ipu7_mmu_cleanup(isp->isys->mmu);
>> +	ipu7_mmu_cleanup(isp->psys->mmu);
>> +
>>  	ipu7_bus_del_devices(pdev);
>>  
>>  	pm_runtime_forbid(&pdev->dev);
>> @@ -2653,8 +2656,6 @@ static void ipu7_pci_remove(struct pci_dev *pdev)
>>  
>>  	release_firmware(isp->cpd_fw);
>>  
> 
> This newline will be extra once the two lines below have been removed.
> 
> Does the same issue exist in the IPU6 driver? These calls are located in
> the same location but I haven't checked if that's the right place for them.
>

IPU6 driver fix this problem in another way.

	struct ipu6_mmu *isys_mmu = isp->isys->mmu;
	struct ipu6_mmu *psys_mmu = isp->psys->mmu;
...
	release_firmware(isp->cpd_fw);

	ipu6_mmu_cleanup(psys_mmu);
	ipu6_mmu_cleanup(isys_mmu);
	
I think cleanup MMU before releasing auxdevs make more sense.

>> -	ipu7_mmu_cleanup(isp->psys->mmu);
>> -	ipu7_mmu_cleanup(isp->isys->mmu);
>>  }
>>  
>>  static void ipu7_pci_reset_prepare(struct pci_dev *pdev)
> 

-- 
Best regards,
Bingbu Cao