Re: [PATCH] staging: media: atomisp: Fix premature setting of HMM_BO_DEVICE_INITED flag

[Hans de Goede <hansg@kernel.org>]

Hi, Abdelrahman

On 1-Jul-25 5:45 PM, Abdelrahman Fekry wrote:
> Hi Hans,
> 
> On Tue, Jul 1, 2025 at 4:54 PM Hans de Goede <hansg@kernel.org> wrote:

...

>> Actually I'm pretty sure that there will be quite a few
>> error-handling paths with bugs in the atomisp code given
>> its overall quality. But lets clean things up first, that
>> should make addressing any such cases easier.
>>
> I totally agree with this , i have submitted a patch that cleans the
> custom sysfs atrributes
> as you suggested as a beginning , the patch got reviewed by andy and dan
> here is the link
> https://lore.kernel.org/all/20250627100604.29061-1-abdelrahmanfekry375@gmail.com/
> 
> What do you think I should work on next after these two patches, do
> you have any suggestions?

The hmm_alloc code can use some more cleanups:

* hmm_get_mmu_base_addr() should be moved to drivers/staging/media/atomisp/pci/hmm/hmm.c
  and then the "struct hmm_bo_device bo_device;" in hmm.c can be made static

* hmm_init() sets hmm_initialized = true even on errors. It should
  immediately exit (return ret) on errors instead of continue-ing
  with calling hmm_alloc() even though hmm_bo_device_init() failed.

* I've checked the code and hmm_init() is called before any hmm_alloc()
  calls are made so the extra hmm_init() call in __hmm_alloc() can be
  dropped.

* After dropping the extra hmm_init() call in __hmm_alloc() the
  hmm_initialized flag can be removed since it is now no longer read
  anywhere.

* And maybe you'll find more possible cleanups while working on this

Regards,

Hans