diff mbox series

[1/1] media: ipu6: Drop unused ipu6_dma_get_sgtable()

Message ID 20250225094532.40678-1-sakari.ailus@linux.intel.com
State New
Headers show
Series [1/1] media: ipu6: Drop unused ipu6_dma_get_sgtable() | expand

Commit Message

Sakari Ailus Feb. 25, 2025, 9:45 a.m. UTC 
ipu6_dma_get_sgtable() is now unused. Drop it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-dma.c | 33 -------------------------
 drivers/media/pci/intel/ipu6/ipu6-dma.h |  3 ---
 2 files changed, 36 deletions(-)

Comments

Stanislaw Gruszka Feb. 25, 2025, 4:47 p.m. UTC | #1 
On Tue, Feb 25, 2025 at 11:45:32AM +0200, Sakari Ailus wrote:
> ipu6_dma_get_sgtable() is now unused. Drop it.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Regards
Stanislaw

> ---
>  drivers/media/pci/intel/ipu6/ipu6-dma.c | 33 -------------------------
>  drivers/media/pci/intel/ipu6/ipu6-dma.h |  3 ---
>  2 files changed, 36 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
> index 44e24da33907..052e396a28af 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
> @@ -455,36 +455,3 @@ void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>  	ipu6_dma_unmap_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
>  }
>  EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sgtable, "INTEL_IPU6");
> -
> -/*
> - * Create scatter-list for the already allocated DMA buffer
> - */
> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> -			 void *cpu_addr, dma_addr_t handle, size_t size,
> -			 unsigned long attrs)
> -{
> -	struct device *dev = &sys->auxdev.dev;
> -	struct ipu6_mmu *mmu = sys->mmu;
> -	struct vm_info *info;
> -	int n_pages;
> -	int ret = 0;
> -
> -	info = get_vm_info(mmu, handle);
> -	if (!info)
> -		return -EFAULT;
> -
> -	if (!info->vaddr)
> -		return -EFAULT;
> -
> -	if (WARN_ON(!info->pages))
> -		return -ENOMEM;
> -
> -	n_pages = PFN_UP(size);
> -
> -	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
> -					GFP_KERNEL);
> -	if (ret)
> -		dev_warn(dev, "get sgt table failed\n");
> -
> -	return ret;
> -}
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
> index b51244add9e6..2882850d9366 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
> @@ -43,7 +43,4 @@ int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>  			 enum dma_data_direction dir, unsigned long attrs);
>  void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>  			    enum dma_data_direction dir, unsigned long attrs);
> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> -			 void *cpu_addr, dma_addr_t handle, size_t size,
> -			 unsigned long attrs);
>  #endif /* IPU6_DMA_H */
> -- 
> 2.39.5
>
Sakari Ailus Feb. 26, 2025, 7:58 a.m. UTC | #2 
Hi Bingbu,

On Wed, Feb 26, 2025 at 10:38:25AM +0800, Bingbu Cao wrote:
> Sakari,
> 
> On 2/25/25 5:45 PM, Sakari Ailus wrote:
> > ipu6_dma_get_sgtable() is now unused. Drop it.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/pci/intel/ipu6/ipu6-dma.c | 33 -------------------------
> >  drivers/media/pci/intel/ipu6/ipu6-dma.h |  3 ---
> >  2 files changed, 36 deletions(-)
> > 
> > diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
> > index 44e24da33907..052e396a28af 100644
> > --- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
> > +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
> > @@ -455,36 +455,3 @@ void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> >  	ipu6_dma_unmap_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
> >  }
> >  EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sgtable, "INTEL_IPU6");
> > -
> > -/*
> > - * Create scatter-list for the already allocated DMA buffer
> > - */
> > -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> > -			 void *cpu_addr, dma_addr_t handle, size_t size,
> > -			 unsigned long attrs)
> > -{
> > -	struct device *dev = &sys->auxdev.dev;
> > -	struct ipu6_mmu *mmu = sys->mmu;
> > -	struct vm_info *info;
> > -	int n_pages;
> > -	int ret = 0;
> > -
> > -	info = get_vm_info(mmu, handle);
> > -	if (!info)
> > -		return -EFAULT;
> > -
> > -	if (!info->vaddr)
> > -		return -EFAULT;
> > -
> > -	if (WARN_ON(!info->pages))
> > -		return -ENOMEM;
> > -
> > -	n_pages = PFN_UP(size);
> > -
> > -	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
> > -					GFP_KERNEL);
> > -	if (ret)
> > -		dev_warn(dev, "get sgt table failed\n");
> > -
> > -	return ret;
> > -}
> > diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
> > index b51244add9e6..2882850d9366 100644
> > --- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
> > +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
> > @@ -43,7 +43,4 @@ int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> >  			 enum dma_data_direction dir, unsigned long attrs);
> >  void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> >  			    enum dma_data_direction dir, unsigned long attrs);
> > -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> > -			 void *cpu_addr, dma_addr_t handle, size_t size,
> > -			 unsigned long attrs);
> 
> ipu6_dma_get_sgtable() is used in downstream PS driver, I don't think it
> is necessary to remove it as it will be hard to integrate with downstream PS
> driver.

When will there be an upstream PSYS driver?

I suppose you can just as well apply a revert of this patch to your
downstream tree?
Bingbu Cao Feb. 26, 2025, 9:25 a.m. UTC | #3 
Sakari,

On 2/26/25 4:39 PM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> On Wed, Feb 26, 2025 at 04:01:16PM +0800, Bingbu Cao wrote:
>>
>> On 2/26/25 3:58 PM, Sakari Ailus wrote:
>>> Hi Bingbu,
>>>
>>> On Wed, Feb 26, 2025 at 10:38:25AM +0800, Bingbu Cao wrote:
>>>> Sakari,
>>>>
>>>> On 2/25/25 5:45 PM, Sakari Ailus wrote:
>>>>> ipu6_dma_get_sgtable() is now unused. Drop it.
>>>>>
>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>>> ---
>>>>>  drivers/media/pci/intel/ipu6/ipu6-dma.c | 33 -------------------------
>>>>>  drivers/media/pci/intel/ipu6/ipu6-dma.h |  3 ---
>>>>>  2 files changed, 36 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
>>>>> index 44e24da33907..052e396a28af 100644
>>>>> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
>>>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
>>>>> @@ -455,36 +455,3 @@ void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>>>  	ipu6_dma_unmap_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
>>>>>  }
>>>>>  EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sgtable, "INTEL_IPU6");
>>>>> -
>>>>> -/*
>>>>> - * Create scatter-list for the already allocated DMA buffer
>>>>> - */
>>>>> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>>> -			 void *cpu_addr, dma_addr_t handle, size_t size,
>>>>> -			 unsigned long attrs)
>>>>> -{
>>>>> -	struct device *dev = &sys->auxdev.dev;
>>>>> -	struct ipu6_mmu *mmu = sys->mmu;
>>>>> -	struct vm_info *info;
>>>>> -	int n_pages;
>>>>> -	int ret = 0;
>>>>> -
>>>>> -	info = get_vm_info(mmu, handle);
>>>>> -	if (!info)
>>>>> -		return -EFAULT;
>>>>> -
>>>>> -	if (!info->vaddr)
>>>>> -		return -EFAULT;
>>>>> -
>>>>> -	if (WARN_ON(!info->pages))
>>>>> -		return -ENOMEM;
>>>>> -
>>>>> -	n_pages = PFN_UP(size);
>>>>> -
>>>>> -	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
>>>>> -					GFP_KERNEL);
>>>>> -	if (ret)
>>>>> -		dev_warn(dev, "get sgt table failed\n");
>>>>> -
>>>>> -	return ret;
>>>>> -}
>>>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
>>>>> index b51244add9e6..2882850d9366 100644
>>>>> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
>>>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
>>>>> @@ -43,7 +43,4 @@ int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>>>  			 enum dma_data_direction dir, unsigned long attrs);
>>>>>  void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>>>  			    enum dma_data_direction dir, unsigned long attrs);
>>>>> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
>>>>> -			 void *cpu_addr, dma_addr_t handle, size_t size,
>>>>> -			 unsigned long attrs);
>>>>
>>>> ipu6_dma_get_sgtable() is used in downstream PS driver, I don't think it
>>>> is necessary to remove it as it will be hard to integrate with downstream PS
>>>> driver.
>>>
>>> When will there be an upstream PSYS driver?
>>>
>>> I suppose you can just as well apply a revert of this patch to your
>>> downstream tree?
>>>
>>
>> Sakari,
>>
>> I don't think all the OS vendors like this way, it make the downstream
>> development harder. It's a trade-off, I cannot see removing offer any
>> benefits that its side-effect.
> 
> I don't think the Linux distro maintainers like downstream drivers at all,
> independently of how many patches they consist of.
> 
> You could as well move the function to the PSYS driver and use
> find_vm_area(cpu_addr) instead to find the area.

My fault, I forgot I already removed the ipu6_dma_get_sgtable() from
downstream driver months ago, so,

Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>

;)

>
Sakari Ailus Feb. 26, 2025, 10:38 a.m. UTC | #4 
On Wed, Feb 26, 2025 at 05:25:06PM +0800, Bingbu Cao wrote:
> >>>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
> >>>>> index b51244add9e6..2882850d9366 100644
> >>>>> --- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
> >>>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
> >>>>> @@ -43,7 +43,4 @@ int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> >>>>>  			 enum dma_data_direction dir, unsigned long attrs);
> >>>>>  void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> >>>>>  			    enum dma_data_direction dir, unsigned long attrs);
> >>>>> -int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
> >>>>> -			 void *cpu_addr, dma_addr_t handle, size_t size,
> >>>>> -			 unsigned long attrs);
> >>>>
> >>>> ipu6_dma_get_sgtable() is used in downstream PS driver, I don't think it
> >>>> is necessary to remove it as it will be hard to integrate with downstream PS
> >>>> driver.
> >>>
> >>> When will there be an upstream PSYS driver?
> >>>
> >>> I suppose you can just as well apply a revert of this patch to your
> >>> downstream tree?
> >>>
> >>
> >> Sakari,
> >>
> >> I don't think all the OS vendors like this way, it make the downstream
> >> development harder. It's a trade-off, I cannot see removing offer any
> >> benefits that its side-effect.
> > 
> > I don't think the Linux distro maintainers like downstream drivers at all,
> > independently of how many patches they consist of.
> > 
> > You could as well move the function to the PSYS driver and use
> > find_vm_area(cpu_addr) instead to find the area.
> 
> My fault, I forgot I already removed the ipu6_dma_get_sgtable() from
> downstream driver months ago, so,
> 
> Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>

Thanks, Bingbu!
diff mbox series

Patch

diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
index 44e24da33907..052e396a28af 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
@@ -455,36 +455,3 @@  void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
 	ipu6_dma_unmap_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
 }
 EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sgtable, "INTEL_IPU6");
-
-/*
- * Create scatter-list for the already allocated DMA buffer
- */
-int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
-			 void *cpu_addr, dma_addr_t handle, size_t size,
-			 unsigned long attrs)
-{
-	struct device *dev = &sys->auxdev.dev;
-	struct ipu6_mmu *mmu = sys->mmu;
-	struct vm_info *info;
-	int n_pages;
-	int ret = 0;
-
-	info = get_vm_info(mmu, handle);
-	if (!info)
-		return -EFAULT;
-
-	if (!info->vaddr)
-		return -EFAULT;
-
-	if (WARN_ON(!info->pages))
-		return -ENOMEM;
-
-	n_pages = PFN_UP(size);
-
-	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
-					GFP_KERNEL);
-	if (ret)
-		dev_warn(dev, "get sgt table failed\n");
-
-	return ret;
-}
diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
index b51244add9e6..2882850d9366 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
@@ -43,7 +43,4 @@  int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
 			 enum dma_data_direction dir, unsigned long attrs);
 void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
 			    enum dma_data_direction dir, unsigned long attrs);
-int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
-			 void *cpu_addr, dma_addr_t handle, size_t size,
-			 unsigned long attrs);
 #endif /* IPU6_DMA_H */