Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 21 May 2019 17:27:43 +0200
From:   Fredrik Noring <noring@nocrew.org>
To:     laurentiu.tudor@nxp.com
Cc:     hch@lst.de, stern@rowland.harvard.edu, gregkh@linuxfoundation.org,
        linux-usb@vger.kernel.org, marex@denx.de, leoyang.li@nxp.com,
        linux-kernel@vger.kernel.org, robin.murphy@arm.com,
        JuergenUrban@gmx.de
Subject: Re: [PATCH v5 2/5] USB: use genalloc for USB HCs with local memory
Message-ID: <20190521152743.GA4693@sx9>
References: <20190521140748.20012-1-laurentiu.tudor@nxp.com>
 <20190521140748.20012-3-laurentiu.tudor@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20190521140748.20012-3-laurentiu.tudor@nxp.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Thanks Laurentiu!

> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -216,6 +216,9 @@ struct usb_hcd {
>  #define	HC_IS_RUNNING(state) ((state) & __ACTIVE)
>  #define	HC_IS_SUSPENDED(state) ((state) & __SUSPEND)
>  
> +	/* allocator for HCs having local memory */
> +	struct gen_pool         *localmem_pool;
> +

I have tested patches 1, 2 and 5, and they seem to work. Nice! May I
suggest clarifying the NULL pointers? I think that may help someone
debugging or maintaining this in the future. Something like this:

--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -385,6 +385,8 @@ struct ohci_hcd {
 
 	/*
 	 * memory management for queue data structures
+	 *
+	 * @td_cache and @ed_cache are %NULL if &usb_hcd.localmem_pool is used.
 	 */
 	struct dma_pool		*td_cache;
 	struct dma_pool		*ed_cache;
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -211,7 +211,7 @@ struct usb_hcd {
 #define	HC_IS_RUNNING(state) ((state) & __ACTIVE)
 #define	HC_IS_SUSPENDED(state) ((state) & __SUSPEND)
 
-	/* allocator for HCs having local memory */
+	/* allocator for HCs having local memory, or %NULL */
 	struct gen_pool         *localmem_pool;
 
 	/* more shared queuing code would be good; it should support

Fredrik