[PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[Abdelrahman Fekry]

Continuing the cleanup work that is being done on the AtomIsp driver,
This series:

- Process the error inside hmm_init().
- Unifies the HMM init tracking method.
- move hmm related function to hmm.c.

Previously, These patches were sent individualy but they build on
each other so i resent them as one patch series

Suggested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
Abdelrahman Fekry (3):
  staging: media: atomisp: return early on hmm_bo_device_init() failure
  staging: media: atomisp: unify HMM initialization tracking
  staging: media: atomisp: move hmm_get_mmu_base_addr()

 .../staging/media/atomisp/include/hmm/hmm.h   |  4 +--
 .../media/atomisp/include/hmm/hmm_bo.h        | 11 ++-----
 .../media/atomisp/pci/atomisp_compat_css20.c  | 12 -------
 drivers/staging/media/atomisp/pci/hmm/hmm.c   | 31 ++++++++++---------
 .../staging/media/atomisp/pci/hmm/hmm_bo.c    | 17 ++++------
 5 files changed, 27 insertions(+), 48 deletions(-)

-- 
2.25.1

[PATCH 1/3] staging: media: atomisp: return early on hmm_bo_device_init() failure

[Abdelrahman Fekry]

hmm_init() would continue execution even if hmm_bo_device_init() failed,
potentially leading to bad behaviour when calling hmm_alloc().

- returns the error immediately if hmm_bo_device_init() fails.

Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
 drivers/staging/media/atomisp/pci/hmm/hmm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index f998b57f90c4..97c7ce970aef 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -34,8 +34,10 @@ int hmm_init(void)
 
 	ret = hmm_bo_device_init(&bo_device, &sh_mmu_mrfld,
 				 ISP_VM_START, ISP_VM_SIZE);
-	if (ret)
+	if (ret) {
 		dev_err(atomisp_dev, "hmm_bo_device_init failed.\n");
+		return ret;
+	}
 
 	hmm_initialized = true;
 
@@ -48,7 +50,7 @@ int hmm_init(void)
 	 */
 	dummy_ptr = hmm_alloc(1);
 
-	return ret;
+	return 0;
 }
 
 void hmm_cleanup(void)
-- 
2.25.1

[PATCH 2/3] staging: media: atomisp: unify HMM initialization tracking

[Abdelrahman Fekry]

The HMM subsystem previously used two separate mechanisms to track
initialization state:
1. A global boolean `hmm_initialized` in hmm.c
2. A device-specific `flag` in struct hmm_bo_device with magic values

This dual approach was redundant and error-prone. Additionally, a
redundant hmm_init() call existed in the allocation path.

- Replaces the device-specific `flag` with a boolean `initialized` field
- Removes the global `hmm_initialized` variable
- Eliminates the hmm_bo_device_inited() helper function
- Removes the redundant hmm_init() call from __hmm_alloc()

Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
 .../staging/media/atomisp/include/hmm/hmm_bo.h  | 11 +++--------
 drivers/staging/media/atomisp/pci/hmm/hmm.c     | 11 -----------
 drivers/staging/media/atomisp/pci/hmm/hmm_bo.c  | 17 ++++++-----------
 3 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/media/atomisp/include/hmm/hmm_bo.h b/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
index e09ac29ac43d..ac556c1d71bb 100644
--- a/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
+++ b/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
@@ -58,8 +58,6 @@
 #define	ISP_VM_SIZE	(0x7FFFFFFF)	/* 2G address space */
 #define	ISP_PTR_NULL	NULL
 
-#define	HMM_BO_DEVICE_INITED	0x1
-
 enum hmm_bo_type {
 	HMM_BO_PRIVATE,
 	HMM_BO_VMALLOC,
@@ -86,7 +84,9 @@ struct hmm_bo_device {
 
 	/* list lock is used to protect the entire_bo_list */
 	spinlock_t	list_lock;
-	int flag;
+
+	/* boolean to indicate whether the bo device is inited or not*/
+	bool initialized;
 
 	/* linked list for entire buffer object */
 	struct list_head entire_bo_list;
@@ -142,11 +142,6 @@ int hmm_bo_device_init(struct hmm_bo_device *bdev,
  */
 void hmm_bo_device_exit(struct hmm_bo_device *bdev);
 
-/*
- * whether the bo device is inited or not.
- */
-int hmm_bo_device_inited(struct hmm_bo_device *bdev);
-
 /*
  * increase buffer object reference.
  */
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 97c7ce970aef..8d64e5fd812c 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -26,7 +26,6 @@
 
 struct hmm_bo_device bo_device;
 static ia_css_ptr dummy_ptr = mmgr_EXCEPTION;
-static bool hmm_initialized;
 
 int hmm_init(void)
 {
@@ -39,8 +38,6 @@ int hmm_init(void)
 		return ret;
 	}
 
-	hmm_initialized = true;
-
 	/*
 	 * As hmm use NULL to indicate invalid ISP virtual address,
 	 * and ISP_VM_START is defined to 0 too, so we allocate
@@ -63,7 +60,6 @@ void hmm_cleanup(void)
 	dummy_ptr = 0;
 
 	hmm_bo_device_exit(&bo_device);
-	hmm_initialized = false;
 }
 
 static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
@@ -73,13 +69,6 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
 	struct hmm_buffer_object *bo;
 	int ret;
 
-	/*
-	 * Check if we are initialized. In the ideal world we wouldn't need
-	 * this but we can tackle it once the driver is a lot cleaner
-	 */
-
-	if (!hmm_initialized)
-		hmm_init();
 	/* Get page number from size */
 	pgnr = size_to_pgnr_ceil(bytes);
 
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
index 5d0cd5260d3a..aabb2a126caa 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
@@ -373,7 +373,7 @@ int hmm_bo_device_init(struct hmm_bo_device *bdev,
 
 	__bo_insert_to_free_rbtree(&bdev->free_rbtree, bo);
 
-	bdev->flag = HMM_BO_DEVICE_INITED;
+	bdev->initialized = true;
 
 	return 0;
 }
@@ -385,9 +385,10 @@ struct hmm_buffer_object *hmm_bo_alloc(struct hmm_bo_device *bdev,
 	struct rb_root *root = &bdev->free_rbtree;
 
 	check_bodev_null_return(bdev, NULL);
-	var_equal_return(hmm_bo_device_inited(bdev), 0, NULL,
-			 "hmm_bo_device not inited yet.\n");
-
+	if (!bdev->initialized) {
+		dev_err(atomisp_dev, "hmm_bo_device is not initialized!\n");
+		return NULL;
+	}
 	if (pgnr == 0) {
 		dev_err(atomisp_dev, "0 size buffer is not allowed.\n");
 		return NULL;
@@ -522,13 +523,7 @@ void hmm_bo_device_exit(struct hmm_bo_device *bdev)
 	kmem_cache_destroy(bdev->bo_cache);
 
 	isp_mmu_exit(&bdev->mmu);
-}
-
-int hmm_bo_device_inited(struct hmm_bo_device *bdev)
-{
-	check_bodev_null_return(bdev, -EINVAL);
-
-	return bdev->flag == HMM_BO_DEVICE_INITED;
+	bdev->initialized = false;
 }
 
 int hmm_bo_allocated(struct hmm_buffer_object *bo)
-- 
2.25.1

[PATCH 3/3] staging: media: atomisp: move hmm_get_mmu_base_addr()

[Abdelrahman Fekry]

The function hmm_get_mmu_base_addr() is related to the functionality
of hmm so its better to be declared and defined in this scope.

- Move hmm_get_mmu_base_addr() to hmm.c.
- Change hmm_bo_device struct to be static.

Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
---
 drivers/staging/media/atomisp/include/hmm/hmm.h    |  4 +---
 .../media/atomisp/pci/atomisp_compat_css20.c       | 12 ------------
 drivers/staging/media/atomisp/pci/hmm/hmm.c        | 14 +++++++++++++-
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/atomisp/include/hmm/hmm.h b/drivers/staging/media/atomisp/include/hmm/hmm.h
index a7aef27f54de..da85d835ec10 100644
--- a/drivers/staging/media/atomisp/include/hmm/hmm.h
+++ b/drivers/staging/media/atomisp/include/hmm/hmm.h
@@ -33,7 +33,7 @@ int hmm_load(ia_css_ptr virt, void *data, unsigned int bytes);
 int hmm_store(ia_css_ptr virt, const void *data, unsigned int bytes);
 int hmm_set(ia_css_ptr virt, int c, unsigned int bytes);
 int hmm_flush(ia_css_ptr virt, unsigned int bytes);
-
+int hmm_get_mmu_base_addr(struct device *dev, unsigned int *mmu_base_addr);
 /*
  * get kernel memory physical address from ISP virtual address.
  */
@@ -65,6 +65,4 @@ void hmm_flush_vmap(ia_css_ptr virt);
  */
 int hmm_mmap(struct vm_area_struct *vma, ia_css_ptr virt);
 
-extern struct hmm_bo_device bo_device;
-
 #endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index be5f37f4a6fd..b4913cb32a64 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -157,18 +157,6 @@ void atomisp_load_uint32(hrt_address addr, uint32_t *data)
 	*data = atomisp_css2_hw_load_32(addr);
 }
 
-static int hmm_get_mmu_base_addr(struct device *dev, unsigned int *mmu_base_addr)
-{
-	if (!sh_mmu_mrfld.get_pd_base) {
-		dev_err(dev, "get mmu base address failed.\n");
-		return -EINVAL;
-	}
-
-	*mmu_base_addr = sh_mmu_mrfld.get_pd_base(&bo_device.mmu,
-			 bo_device.mmu.base_address);
-	return 0;
-}
-
 static void __dump_pipe_config(struct atomisp_sub_device *asd,
 			       struct atomisp_stream_env *stream_env,
 			       unsigned int pipe_id)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 8d64e5fd812c..321166e58bb5 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -24,7 +24,7 @@
 #include "mmu/isp_mmu.h"
 #include "mmu/sh_mmu_mrfld.h"
 
-struct hmm_bo_device bo_device;
+static struct hmm_bo_device bo_device;
 static ia_css_ptr dummy_ptr = mmgr_EXCEPTION;
 
 int hmm_init(void)
@@ -488,3 +488,15 @@ void hmm_vunmap(ia_css_ptr virt)
 
 	hmm_bo_vunmap(bo);
 }
+
+int hmm_get_mmu_base_addr(struct device *dev, unsigned int *mmu_base_addr)
+{
+	if (!sh_mmu_mrfld.get_pd_base) {
+		dev_err(dev, "get mmu base address failed.\n");
+		return -EINVAL;
+	}
+
+	*mmu_base_addr = sh_mmu_mrfld.get_pd_base(&bo_device.mmu,
+			 bo_device.mmu.base_address);
+	return 0;
+}
-- 
2.25.1

Re: [PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[Dan Carpenter]

On Sat, Jul 12, 2025 at 10:13:22PM +0300, Abdelrahman Fekry wrote:
> Continuing the cleanup work that is being done on the AtomIsp driver,
> This series:
> 
> - Process the error inside hmm_init().
> - Unifies the HMM init tracking method.
> - move hmm related function to hmm.c.
> 
> Previously, These patches were sent individualy but they build on
> each other so i resent them as one patch series
> 
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>

These seem reasonable enough to me.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

But it would be better if someone could test them as well.

regards,
dan carpenter

Re: [PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[Abdelrahman Fekry]

Hi Dan,
Thanks for your feedback

On Wed, Jul 16, 2025 at 9:21 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Sat, Jul 12, 2025 at 10:13:22PM +0300, Abdelrahman Fekry wrote:
> > Continuing the cleanup work that is being done on the AtomIsp driver,
> > This series:
> >
> > - Process the error inside hmm_init().
> > - Unifies the HMM init tracking method.
> > - move hmm related function to hmm.c.
> >
> > Previously, These patches were sent individualy but they build on
> > each other so i resent them as one patch series
> >
> > Suggested-by: Hans de Goede <hansg@kernel.org>
> > Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
>
> These seem reasonable enough to me.
>
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
>
> But it would be better if someone could test them as well.
I tested this by building the driver and i really don't think further
testing is needed
since this series doesn't introduce any behavioral or functional change.
>
> regards,
> dan carpenter

Best Regards,
Abdelrahman Fekry

Re: [PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[Abdelrahman Fekry]

Hi Maintainers,
I'm just bringing attention to this patch series as I have a lot of
other patches that build on it .
Thank you!

On Thu, Jul 17, 2025 at 4:35 AM Abdelrahman Fekry
<abdelrahmanfekry375@gmail.com> wrote:
>
> Hi Dan,
> Thanks for your feedback
>
> On Wed, Jul 16, 2025 at 9:21 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >
> > On Sat, Jul 12, 2025 at 10:13:22PM +0300, Abdelrahman Fekry wrote:
> > > Continuing the cleanup work that is being done on the AtomIsp driver,
> > > This series:
> > >
> > > - Process the error inside hmm_init().
> > > - Unifies the HMM init tracking method.
> > > - move hmm related function to hmm.c.
> > >
> > > Previously, These patches were sent individualy but they build on
> > > each other so i resent them as one patch series
> > >
> > > Suggested-by: Hans de Goede <hansg@kernel.org>
> > > Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@gmail.com>
> >
> > These seem reasonable enough to me.
> >
> > Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> >
> > But it would be better if someone could test them as well.
> I tested this by building the driver and i really don't think further
> testing is needed
> since this series doesn't introduce any behavioral or functional change.
> >
> > regards,
> > dan carpenter
>
> Best Regards,
> Abdelrahman Fekry

Re: [PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[Greg KH]

On Thu, Jul 31, 2025 at 06:24:09AM +0300, Abdelrahman Fekry wrote:
> Hi Maintainers,
> I'm just bringing attention to this patch series as I have a lot of
> other patches that build on it .

It is the middle of the merge window.  Please wait for -rc1 to be
released, there is no rush.

thanks,

greg k-h

Re: [PATCH 0/3] staging: media: atomisp: More Cleanup on driver AtomIsp

[Abdelrahman Fekry]

On Thu, Jul 31, 2025 at 7:35 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 31, 2025 at 06:24:09AM +0300, Abdelrahman Fekry wrote:
> > Hi Maintainers,
> > I'm just bringing attention to this patch series as I have a lot of
> > other patches that build on it .
>
> It is the middle of the merge window.  Please wait for -rc1 to be
> released, there is no rush.
noted, thank you!
>
> thanks,
>
> greg k-h