Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1734874 > unrolled thread

Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap /  otg phy mux handling Mathias Nyman <mathias.nyman@linux.intel.com> - 2017-09-19 14:40 +0200
    Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap /  otg phy mux handling Hans de Goede <hdegoede@redhat.com> - 2017-09-21 14:00 +0200

#1734874 — Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

[Multipart message — attachments visible in raw view] — view raw

Hi,

sorry about the long delay

On 07.09.2017 18:49, Hans de Goede wrote:
> Hi,
>
> On 07-09-17 15:14, Mathias Nyman wrote:
>> On 05.09.2017 19:42, Hans de Goede wrote:
>>> The Intel cherrytrail xhci controller has an extended cap mmio-range
>>> which contains registers to control the muxing to the xhci (host mode)
>>> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>>>
>>> Having a mux driver included in the xhci code (or under drivers/usb/host)
>>> is not desirable. So this commit adds a simple handler for this extended
>>> capability, which creates a platform device with the caps mmio region as
>>> resource, this allows us to write a separate platform mux driver for the
>>> mux.
>>>
>> I think it would be better to have one place where we add handlers for
>> vendor specific extended capabilities.
>>
>> Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
>> there's a xhci-ext-caps.h header already
>>
>> We could walk through the capability list once and add the needed handlers.
>> Something like:
>>
>> +int xhci_ext_cap_init(void __iomem *base)
>
> This will need to take a struct xhci_hcd *xhci param instead
> as some of the ext_cap handling (including the cht mux code)
> will need access to this.
>

yes, sample code added in second patch for reference/testing.

>
> So I see 2 options here (without making this function PCI specific)
> 1) Add an u32 product_id field to struct xhci_hcd; or
> 2) Use a quirk flag as my current code is doing.
>
> I'm fine with doing this either way, please let me know your preference.

Lets go with the quirk for now, I'll sort that out later

>
> Can you do a "git format-patch" of that and send it to me? If you
> can give me that + your preference for how to check if we're
> dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
> with your suggestions applied.

Ended up modifying xhci_find_next_ext_cap() using id = 0 for
the next capability in list. Patch attached,

Second patch is just for reference how to use it.

Thanks
-Mathias

[toc] | [next] | [standalone]

#1736601

Hi,

On 19-09-17 14:40, Mathias Nyman wrote:
> Hi,
> 
> sorry about the long delay
> 
> On 07.09.2017 18:49, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-17 15:14, Mathias Nyman wrote:
>>> On 05.09.2017 19:42, Hans de Goede wrote:
>>>> The Intel cherrytrail xhci controller has an extended cap mmio-range
>>>> which contains registers to control the muxing to the xhci (host mode)
>>>> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>>>>
>>>> Having a mux driver included in the xhci code (or under drivers/usb/host)
>>>> is not desirable. So this commit adds a simple handler for this extended
>>>> capability, which creates a platform device with the caps mmio region as
>>>> resource, this allows us to write a separate platform mux driver for the
>>>> mux.
>>>>
>>> I think it would be better to have one place where we add handlers for
>>> vendor specific extended capabilities.
>>>
>>> Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
>>> there's a xhci-ext-caps.h header already
>>>
>>> We could walk through the capability list once and add the needed handlers.
>>> Something like:
>>>
>>> +int xhci_ext_cap_init(void __iomem *base)
>>
>> This will need to take a struct xhci_hcd *xhci param instead
>> as some of the ext_cap handling (including the cht mux code)
>> will need access to this.
>>
> 
> yes, sample code added in second patch for reference/testing.
> 
>>
>> So I see 2 options here (without making this function PCI specific)
>> 1) Add an u32 product_id field to struct xhci_hcd; or
>> 2) Use a quirk flag as my current code is doing.
>>
>> I'm fine with doing this either way, please let me know your preference.
> 
> Lets go with the quirk for now, I'll sort that out later
> 
>>
>> Can you do a "git format-patch" of that and send it to me? If you
>> can give me that + your preference for how to check if we're
>> dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
>> with your suggestions applied.
> 
> Ended up modifying xhci_find_next_ext_cap() using id = 0 for
> the next capability in list. Patch attached,
> 
> Second patch is just for reference how to use it.

Thank you for the patches, I'm working on prepping a v3 of
this series which includes and uses the first patch.

Regards,

Hans

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web