linux-kernel - Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <wjdbgsasdvocglz3qbqnp3rc6tedlxuxlqsv3rhlad2u2y56ll@5irs2bu2647k>
Date: Thu, 24 Apr 2025 15:18:23 +0200
From: Jacopo Mondi <jacopo.mondi@...asonboard.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Jacopo Mondi <jacopo.mondi+renesas@...asonboard.com>, 
	Kieran Bingham <kieran.bingham+renesas@...asonboard.com>, 
	Niklas Söderlund <niklas.soderlund@...natech.se>, linux-kernel@...r.kernel.org, linux-media@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH v7 5/5] media: vsp1: Add VSPX support

Hi Laurent

On Thu, Apr 24, 2025 at 12:10:35AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Apr 01, 2025 at 04:22:05PM +0200, Jacopo Mondi wrote:
> > Add support for VSPX, a specialized version of the VSP2 that
> > transfer data to the ISP. The VSPX is composed by two RPF units
>
> It seems you forgot to take comments from v2 into account.
>

Are you referring to the commit message alone ? Or is there anything
else ?

> > to read data from external memory and an IIF instance that performs
> > transfer towards the ISP.
> >

[snip]

>
> > + *		 buffer CPU-mapped address and the bus address
> > + *
> > + * Return: %0 on success or a negative error code on failure
> > + */
> > +int vsp1_isp_alloc_buffers(struct device *dev, size_t size,
> > +			   struct vsp1_isp_buffer_desc *buffer_desc)
> > +{
> > +	struct device *bus_master = vsp1_isp_get_bus_master(dev);
> > +
> > +	if (IS_ERR_OR_NULL(bus_master))
> > +		return -ENODEV;
> > +
> > +	buffer_desc->cpu_addr = dma_alloc_coherent(bus_master, size,
> > +						   &buffer_desc->dma_addr,
> > +						   GFP_KERNEL);
> > +	if (IS_ERR_OR_NULL(buffer_desc->cpu_addr))
> > +		return -EINVAL;
>
> As commented by Alok, this should be
>
> 	if (!buffer_desc->cpu_addr)
> 		return -ENOMEM;

Done, thanks Alok for the comment

>
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(vsp1_isp_alloc_buffers);
>
> Where is the buffer freed ?
>

Right, I presume a call to dma_free_coherent() is needed in the
vb2 buf_cleanup() operation call path ?