Message-ID: <552644DF.6030908@ti.com>
Date: Thu, 9 Apr 2015 12:22:39 +0300
From: Roger Quadros <rogerq@ti.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
To: Mathias Nyman <mathias.nyman@linux.intel.com>, <mathias.nyman@intel.com>,
        Alan Stern <stern@rowland.harvard.edu>
CC: <gregkh@linuxfoundation.org>, <linux-usb@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/5] usb: xhci: cleanup xhci_hcd allocation
References: <1427977409-7671-1-git-send-email-rogerq@ti.com> <1427977409-7671-2-git-send-email-rogerq@ti.com> <5523E861.7060807@linux.intel.com>
In-Reply-To: <5523E861.7060807@linux.intel.com>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2496
Lines: 66

Hi,

On 07/04/15 17:23, Mathias Nyman wrote:
> Hi
> 
> On 02.04.2015 15:23, Roger Quadros wrote:
>> HCD core allocates memory for HCD private data in
>> usb_create_[shared_]hcd() so make use of that
>> mechanism to allocate the struct xhci_hcd.
>>
>> Introduce struct xhci_driver_overrides to provide
>> the size of HCD private data and hc_driver operation
>> overrides. As of now we only need to override the
>> reset and start methods.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> I'm not sure I fully understand the what's going on, or what the
> intention of this patch is.

The main intention is to have both primary and shared HCDs allocated
before calling usb_add_hcd() for the primary hcd.
This is so that at the first usb_add_hcd() the OTG core knows the HCD topology
(i.e. whether it uses a shared HCD or not).

>From the OTG perspective we have to prevent the actual usb_add_hcd() till the
OTG state machine says so.
This means that xhci_gen_setup() won't be necessarily called immediately and
so we need to allocate for xhci somewhere else.

> 
> So currently xhci driver manages the allocation and freeing of
> the xhci_hcd structure. We store a pointer to the xhci_hcd structure in
> the content of both the primary and shared usb_hcds structures hcd_priv
> field.
> 
> With this patch xhci would be part of the usb_hcd structure,
> starting at hcd_priv[0]. (Like EHCI I think) It allocates enough space to include
> the xhci_hcd in both the primary and shared usb_hcd, but always only use the one
> in the primary hcd.

precisely.

> I'm not sure what to do with the space allocated for the shared hcd's
> hcd_priv field.

we just ignore the space allocated for the shared hcd.

> 
> This also means that xhci goes away together with the primary hcd. It's possible
> this has some impact as the xhci driver expects xhci to always exists.

Can you please point out where this impact is.

I've been testing add/remove HCD extensively and didn't observe any issues after applying
these 5 patches. Well there is one issue that comes up but it has nothing to do with xhci
not being allocated. It has more to do with command being queued after the HCD has gone away
and so getting stuck forever without timing out.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/