Re: [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local memory

[Laurentiu Tudor <laurentiu.tudor@nxp.com>]

On 14.05.2019 17:42, Christoph Hellwig wrote:
>> @@ -136,6 +137,10 @@ void *hcd_buffer_alloc(
>>   		if (size <= pool_max[i])
>>   			return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
>>   	}
>> +
>> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
>> +		return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);
> 
> I think this check needs to be before the above code to use the dma
> pools, as we should always use the HCD local memory.  Probably all the
> way up just below the size == 0 check, that way we can also remove the
> other HCD_LOCAL_MEM check.

Alright.

>> @@ -165,5 +170,10 @@ void hcd_buffer_free(
>>   			return;
>>   		}
>>   	}
>> -	dma_free_coherent(hcd->self.sysdev, size, addr, dma);
>> +
>> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
>> +		gen_pool_free(hcd->localmem_pool, (unsigned long)addr,
>> +			      size);
>> +	else
>> +		dma_free_coherent(hcd->self.sysdev, size, addr, dma);
> 
> Same here.

Ok.

>> @@ -505,8 +506,15 @@ static int ohci_init (struct ohci_hcd *ohci)
>>   	timer_setup(&ohci->io_watchdog, io_watchdog_func, 0);
>>   	ohci->prev_frame_no = IO_WATCHDOG_OFF;
>>   
>> -	ohci->hcca = dma_alloc_coherent (hcd->self.controller,
>> -			sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL);
>> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
>> +		ohci->hcca = gen_pool_dma_alloc(hcd->localmem_pool,
>> +						sizeof(*ohci->hcca),
>> +						&ohci->hcca_dma);
>> +	else
>> +		ohci->hcca = dma_alloc_coherent(hcd->self.controller,
>> +						sizeof(*ohci->hcca),
>> +						&ohci->hcca_dma,
>> +						GFP_KERNEL);
> 
> I wonder if we could just use hcd_buffer_alloc/free here, althought
> that would require them to be exported.  I'll leave that decision to
> the relevant maintainers, though.
> 
> Except for this the series looks exactly what I had envisioned to
> get rid of the device local dma_declare_coherent use case, thanks!

Glad I could help. On the remoteproc_virtio.c case, I had a cursory look 
and found out that the dma_declare_coherent_memory() usage was 
introduced quite recently, by this patch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=086d08725d34c6b3333db710344ae9c4fdafb2d5

---
Best Regards, Laurentiu