Re: [PATCH v4 1/3] usb: core: add dma-noncoherent buffer alloc and free API

[Xu Yang]

On Thu, Jul 03, 2025 at 04:46:57PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 03, 2025 at 06:38:09PM +0800, Xu Yang wrote:
> > This will add usb_alloc_noncoherent() and usb_free_noncoherent()
> > functions to support alloc and free buffer in a dma-noncoherent way.
> > 
> > To explicit manage the memory ownership for the kernel and device,
> > this will also add usb_dma_noncoherent_sync_for_cpu/device() functions
> > and call it at proper time.  The management requires the user save
> > sg_table returned by usb_alloc_noncoherent() to urb->sgt.
> 
> ...
> 
> >  	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> > -	if (urb->transfer_buffer_length != 0
> > -	    && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
> > +	if (!(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
> > +		if (!urb->transfer_buffer_length)
> > +			return ret;
> 
> This return ret out of a sudden are quite fragile. Please, move to use return 0
> directly where it is supposed to be 0.

Okay.

> 
> ...
> 
> > +	} else {
> > +		if (!urb->sgt)
> > +			return ret;
> 
> Ditto.

Okay.

> 
> > +		if (dir == DMA_TO_DEVICE)
> > +			flush_kernel_vmap_range(urb->transfer_buffer,
> > +						urb->transfer_buffer_length);
> > +		dma_sync_sgtable_for_device(hcd->self.sysdev, urb->sgt, dir);
> >  	}
> 
> >  	return ret;
> 
> Ditto?

With Alan's suggestion, this patch will not touch this line.

Thanks,
Xu Yang

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>