Possible usb_request leak in the function dwc2_gadget_complete_isoc_request_ddma

[Zengtao (B)]

Hi johnyoun:

I found a suspected bug, and I am writing to confirm with you.

In the function 
dwc2_gadget_complete_isoc_request_ddma(drivers/usb/dwc2/gadget.c).
Only the first request from the eq queue is processed while maybe there are 
more than one descriptors done by the HW.

1. Each usb request is associated with a DMA descriptor, but this is not 
reflect in the driver, so when one DMA descriptor is done, 
we don't know which usb request is done, but I think if only one DMA descriptor 
is done, we can know that the first USB request in 
eq queue is done, because the HW DMA descriptor and SW usb request are both in 
sequence.

2. In the function dwc2_gadget_complete_isoc_request_ddma, we may complete more 
than one DMA descriptor but only the first
Usb request is processed, but in fact, we should all the usb requests 
associated with all the done DMA descriptors.

3. I noticed that each DMA descriptor is configured to report an interrupt, and 
if each DMA descriptor generate an interrupt, the above
Flow should be ok, but the interrupts can merge and we have used the depdma to 
figure out the largest finished DMA descriptor index.

Looking forward your reply.

Thank you. 

Regards
Zengtao 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc3: Prevent indefinite sleep in _dwc3_set_mode during suspend/resume

[Roger Quadros]

Hi Baolin,

On 28/02/18 05:04, Baolin Wang wrote:
> Hi Roger,
> 
> On 27 February 2018 at 19:22, Roger Quadros  wrote:
>> In the following test we get stuck by sleeping forever in _dwc3_set_mode()
>> after which dual-role switching doesn't work.
>>
>> On dra7-evm's dual-role port,
>> - Load g_zero gadget driver and enumerate to host
>> - suspend to mem
>> - disconnect USB cable to host and connect otg cable with Pen drive in it.
>> - resume system
>> - we sleep indefinitely in _dwc3_set_mode due to.
>>   dwc3_gadget_exit()->usb_del_gadget_udc()->udc_stop()->
>> dwc3_gadget_stop()->wait_event_lock_irq()
>>
>> Let's clear the DWC3_EP_END_TRANSFER_PENDING flag on all endpoints
>> so we don't wait in dwc3_gadget_stop().
> 
> I am curious why the DWC3_DEPEVT_EPCMDCMPLT event was not triggered
> any more when you executed the DWC3_DEPCMD_ENDTRANSFER command?

In this particular case the USB gadget has been disconnected from the host so
we shouldn't be expecting any command completion events.

> 
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/dwc3/gadget.c | 14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 2bda4eb..0a360da 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -3273,6 +3273,20 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>
>>  void dwc3_gadget_exit(struct dwc3 *dwc)
>>  {
>> +   int epnum;
>> +   unsigned long flags;
>> +
>> +   spin_lock_irqsave(&dwc->lock, flags);
>> +   for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
>> +   struct dwc3_ep  *dep = dwc->eps[epnum];
>> +
>> +   if (!dep)
>> +   continue;
>> +
>> +   dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
>> +   }
>> +   spin_unlock_irqrestore(&dwc->lock, flags);
>> +
>> usb_del_gadget_udc(&dwc->gadget);
>> dwc3_gadget_free_endpoints(dwc);
>> dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
>> --
> 

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc3: Prevent indefinite sleep in _dwc3_set_mode during suspend/resume

[Roger Quadros]

Felipe,

On 28/02/18 09:53, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  writes:
>> In the following test we get stuck by sleeping forever in _dwc3_set_mode()
>> after which dual-role switching doesn't work.
>>
>> On dra7-evm's dual-role port,
>> - Load g_zero gadget driver and enumerate to host
>> - suspend to mem
>> - disconnect USB cable to host and connect otg cable with Pen drive in it.
>> - resume system
>> - we sleep indefinitely in _dwc3_set_mode due to.
>>   dwc3_gadget_exit()->usb_del_gadget_udc()->udc_stop()->
>>  dwc3_gadget_stop()->wait_event_lock_irq()
>>
>> Let's clear the DWC3_EP_END_TRANSFER_PENDING flag on all endpoints
>> so we don't wait in dwc3_gadget_stop().
>>
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/dwc3/gadget.c | 14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 2bda4eb..0a360da 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -3273,6 +3273,20 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>  
>>  void dwc3_gadget_exit(struct dwc3 *dwc)
>>  {
>> +int epnum;
>> +unsigned long flags;
>> +
>> +spin_lock_irqsave(&dwc->lock, flags);
>> +for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
>> +struct dwc3_ep  *dep = dwc->eps[epnum];
>> +
>> +if (!dep)
>> +continue;
>> +
>> +dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
>> +}
>> +spin_unlock_irqrestore(&dwc->lock, flags);
>> +
>>  usb_del_gadget_udc(&dwc->gadget);
>>  dwc3_gadget_free_endpoints(dwc);
> 
> free endpoints is a better place for this. It's already going to free
> the memory anyway. Might as well clear all flags to 0 there.
> 

But it won't solve the deadlock issue. Since dwc3_gadget_free_endpoints()
is called after usb_del_gadget_udc() and the deadlock happens when

usb_del_gadget_udc()->udc_stop()->dwc3_gadget_stop()->wait_event_lock_irq()

and DWC3_EP_END_TRANSFER_PENDING flag is set.

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Q: Does mass storage gadget use DMA ?

[Oliver Neukum]

Am Montag, den 26.02.2018, 18:09 +0200 schrieb Ran Shalit:
> On Mon, Feb 26, 2018 at 5:59 PM, Oliver Neukum  wrote:
> > 
> > Am Donnerstag, den 15.02.2018, 11:00 +0200 schrieb Ran Shalit:
> > > 
> > > I actually asked about DMA, because I see that it is possible to send
> > > urb using DMA allocated buffers or not (coherent and non-coherent)
> > > usb_buffer_alloc().
> > > So, How can we actually know if I should use this API with a device or
> > > not ? I mean, is it always possible to use the coherent buffer with
> > > the device ?
> > 
> > Hi,
> > 
> > it is always possible to use usb_alloc_coherent() on a host.
> > But it is generally not sensible. It is for buffers to be reused
> > many times. In general use kmalloc() [once per buffer. That is a rule.
> > You must not share them.]
> > 
> > HTH
> > Oliver
> > 
> Hi Oliver,
> 
> Is the dma engine which is responsible for the transaction is actually
> in the usb device (It's not dma controller in host) ?

OK, I think there is a fundamental misunderstanding here.
There are two sides: host and gadget
Either, both or no side may use DMA. The other side does
not learn of that.

In either case, however, you write a driver for a logical device
or a protocol, if you will. What we call a device driver on the host
(the gadget has its own equivalents) does not talk to hardware.
Essentially you allocate and manipulate URBs.

There is a driver for a host (respectively gadget) controller
which talks to hardware. It does DMA, not your driver.

> If I understand correctly in both ways (kmalloc or
> usb_alloc_coherent), then we are allocating buffers might be used for
> dma.

Yes. We must.

> Even if DMA is not used in the transaction, than is shall still be
> functional with the same DMA-allocated buffers.

Yes. It must.

HTH
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Possible usb_request leak in the function dwc2_gadget_complete_isoc_request_ddma

[Minas Harutyunyan]

Hi,

On 2/28/2018 1:00 PM, Zengtao (B) wrote:
> Hi johnyoun:
> 
> I found a suspected bug, and I am writing to confirm with you.
> 
> In the function 
> dwc2_gadget_complete_isoc_request_ddma(drivers/usb/dwc2/gadget.c).
> Only the first request from the eq queue is processed while maybe there are 
> more than one descriptors done by the HW.
> 
> 1. Each usb request is associated with a DMA descriptor, but this is not 
> reflect in the driver, so when one DMA descriptor is done,
> we don't know which usb request is done, but I think if only one DMA 
> descriptor is done, we can know that the first USB request in
> eq queue is done, because the HW DMA descriptor and SW usb request are both 
> in sequence.
> 
> 2. In the function dwc2_gadget_complete_isoc_request_ddma, we may complete 
> more than one DMA descriptor but only the first
> Usb request is processed, but in fact, we should all the usb requests 
> associated with all the done DMA descriptors.
> 
> 3. I noticed that each DMA descriptor is configured to report an interrupt, 
> and if each DMA descriptor generate an interrupt, the above
> Flow should be ok, but the interrupts can merge and we have used the depdma 
> to figure out the largest finished DMA descriptor index.
> 

Why you suspect that subsequent interrupts can be merged? Did you see 
this case? Can you provide a log?
Even in case of minimal interval=1, time between 2 subsequent interrupts 
should be about 125us. It's fully enough to process target descriptor, 
complete request, enqueue new request and prepare new descriptor.

Thanks,
Minas

> Looking forward your reply.
> 
> Thank you.
> 
> Regards
> Zengtao
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  
> https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIDAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=vbKt9jwY5FRUysFlZg1CB6HKRyFACygkwZBO0mvSQDc&s=7rLKr3g3UyOZT22GAer-w5wDtv-Bb5awlncAJQ1OICM&e=
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] usb: host: ehci-platform: add support for optional external vbus supply

[Roger Quadros]

Hi Amelie,

On 23/02/18 15:46, Amelie Delaunay wrote:
> On some boards, especially when vbus supply requires large current,
> and the charge pump on the PHY isn't enough, an external vbus power switch
> may be used.
> Add support for optional external vbus supply per port in ehci-platform.
> 
> Signed-off-by: Amelie Delaunay 
> 
> ---
> Changes in v3:
>  * Address Felipe Balbi comments: reduce indentation in
>ehci_platform_port_power.
>  * Address Roger Quadros and Alan Stern comments: platforms can have one
>external vbus supply per port, so add support to get as many optional
>regulator as implemented ports on the host controller.
> 
> Changes in v2:
>  * Address Roger Quadros comments: move regulator_enable/disable from
>ehci_platform_power_on/off to ehci_platform_port_power.
> ---
>  Documentation/devicetree/bindings/usb/usb-ehci.txt |  1 +
>  drivers/usb/host/ehci-platform.c   | 52 
> +-
>  2 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt 
> b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> index 3efde12..cd576db 100644
> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> @@ -19,6 +19,7 @@ Optional properties:
>   - phys : phandle + phy specifier pair
>   - phy-names : "usb"
>   - resets : phandle + reset specifier pair
> + - portN_vbus-supply : phandle of regulator supplying vbus for port N
>  
>  Example (Sequoia 440EPx):
>  ehci@e300 {

Sorry for not pointing this out earlier but I think patch to DT bindings
should come separately (before the driver changes) with the following subject.

"dt-bindings: usb: ehci: "


> diff --git a/drivers/usb/host/ehci-platform.c 
> b/drivers/usb/host/ehci-platform.c
> index b065a96..8e9f201 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -46,6 +47,7 @@ struct ehci_platform_priv {
>   struct reset_control *rsts;
>   struct phy **phys;
>   int num_phys;
> + struct regulator **vbus_supplies;
>   bool reset_on_resume;
>  };
>  
> @@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>   struct platform_device *pdev = to_platform_device(hcd->self.controller);
>   struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev);
>   struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> - int retval;
> + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
> + int portnum, n_ports, retval;
>  
>   ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
>  
> @@ -71,11 +74,57 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>   if (retval)
>   return retval;
>  
> + n_ports = HCS_N_PORTS(ehci->hcs_params);
> + priv->vbus_supplies = devm_kcalloc(&pdev->dev, n_ports,
> +sizeof(struct regulator *),
> +GFP_KERNEL);
> + if (!priv->vbus_supplies)
> + return -ENOMEM;
> +
> + for (portnum = 0; portnum < n_ports; portnum++) {
> + struct regulator *vbus_supply;
> + char id[20];
> +
> + sprintf(id, "port%d_vbus", portnum);
> +
> + vbus_supply = devm_regulator_get_optional(&pdev->dev, id);
> + if (IS_ERR(vbus_supply)) {
> + retval = PTR_ERR(vbus_supply);
> + if (retval == -ENODEV)
> + priv->vbus_supplies[portnum] = NULL;
> + else
> + return retval;
> + } else {
> + priv->vbus_supplies[portnum] = vbus_supply;
> + }
> + }
> +
>   if (pdata->no_io_watchdog)
>   ehci->need_io_watchdog = 0;
>   return 0;
>  }
>  
> +static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum,
> + bool enable)
> +{
> + struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
> + int ret;
> +
> + if (!priv->vbus_supplies[portnum])
> + return 0;
> +
> + if (enable)
> + ret = regulator_enable(priv->vbus_supplies[portnum]);
> + else
> + ret = regulator_disable(priv->vbus_supplies[portnum]);

A newline could be used here.

> + if (ret)
> + dev_err(hcd->self.controller,
> + "failed to %s vbus supply for port %d: %d\n",
> + enable ? "enable" : "disable", portnum, ret);
> +
> + return ret;
> +}
> +
>  static int ehci_platform_power_on(struct platform_device *dev)
>  {
>   struct usb_hcd *hcd = platform_get_drvdata(dev);
> @@ -134,6 +183,7 @@ static struct hc_driver __read_mostly 
> ehci_platform_hc_driver;
>  static const struct ehci_driver_overrides platform_overrides __initconst = {
> 

Re: [PATCH v3] usb: host: ehci-platform: add support for optional external vbus supply

[Robin Murphy]

Hi Amelie,

Just a couple of drive-by coding style comments...

On 23/02/18 13:46, Amelie Delaunay wrote:

On some boards, especially when vbus supply requires large current,
and the charge pump on the PHY isn't enough, an external vbus power switch
may be used.
Add support for optional external vbus supply per port in ehci-platform.

Signed-off-by: Amelie Delaunay 

---
Changes in v3:
  * Address Felipe Balbi comments: reduce indentation in
ehci_platform_port_power.
  * Address Roger Quadros and Alan Stern comments: platforms can have one
external vbus supply per port, so add support to get as many optional
regulator as implemented ports on the host controller.

Changes in v2:
  * Address Roger Quadros comments: move regulator_enable/disable from
ehci_platform_power_on/off to ehci_platform_port_power.
---
  Documentation/devicetree/bindings/usb/usb-ehci.txt |  1 +
  drivers/usb/host/ehci-platform.c   | 52 +-
  2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt 
b/Documentation/devicetree/bindings/usb/usb-ehci.txt
index 3efde12..cd576db 100644
--- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
@@ -19,6 +19,7 @@ Optional properties:
   - phys : phandle + phy specifier pair
   - phy-names : "usb"
   - resets : phandle + reset specifier pair
+ - portN_vbus-supply : phandle of regulator supplying vbus for port N
  
  Example (Sequoia 440EPx):

  ehci@e300 {
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index b065a96..8e9f201 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -46,6 +47,7 @@ struct ehci_platform_priv {
struct reset_control *rsts;
struct phy **phys;
int num_phys;
+   struct regulator **vbus_supplies;
bool reset_on_resume;
  };
  
@@ -56,7 +58,8 @@ static int ehci_platform_reset(struct usb_hcd *hcd)

struct platform_device *pdev = to_platform_device(hcd->self.controller);
struct usb_ehci_pdata *pdata = dev_get_platdata(&pdev->dev);
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-   int retval;
+   struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   int portnum, n_ports, retval;
  
  	ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
  
@@ -71,11 +74,57 @@ static int ehci_platform_reset(struct usb_hcd *hcd)

if (retval)
return retval;
  
+	n_ports = HCS_N_PORTS(ehci->hcs_params);

+   priv->vbus_supplies = devm_kcalloc(&pdev->dev, n_ports,
+  sizeof(struct regulator *),


Using "sizeof(*priv->vbus_supplies)" here will prevent people sending 
annoying cleanup patches later.



+  GFP_KERNEL);
+   if (!priv->vbus_supplies)
+   return -ENOMEM;
+
+   for (portnum = 0; portnum < n_ports; portnum++) {
+   struct regulator *vbus_supply;
+   char id[20];
+
+   sprintf(id, "port%d_vbus", portnum);
+
+   vbus_supply = devm_regulator_get_optional(&pdev->dev, id);
+   if (IS_ERR(vbus_supply)) {
+   retval = PTR_ERR(vbus_supply);
+   if (retval == -ENODEV)
+   priv->vbus_supplies[portnum] = NULL;


The array element here hasn't yet been assigned to since kcalloc() 
initially zeroed it, so this is entirely redundant - you can simply make 
the comparison a "!=" and remove the "else" case.


Robin.


+   else
+   return retval;
+   } else {
+   priv->vbus_supplies[portnum] = vbus_supply;
+   }
+   }
+
if (pdata->no_io_watchdog)
ehci->need_io_watchdog = 0;
return 0;
  }
  
+static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum,

+   bool enable)
+{
+   struct ehci_platform_priv *priv = hcd_to_ehci_priv(hcd);
+   int ret;
+
+   if (!priv->vbus_supplies[portnum])
+   return 0;
+
+   if (enable)
+   ret = regulator_enable(priv->vbus_supplies[portnum]);
+   else
+   ret = regulator_disable(priv->vbus_supplies[portnum]);
+   if (ret)
+   dev_err(hcd->self.controller,
+   "failed to %s vbus supply for port %d: %d\n",
+   enable ? "enable" : "disable", portnum, ret);
+
+   return ret;
+}
+
  static int ehci_platform_power_on(struct platform_device *dev)
  {
struct usb_hcd *hcd = platform_get_drvdata(dev);
@@ -134,6 +183,7 @@ static struct hc_driver __read_mostly 
ehci_platform_hc_driver;
  static const struct ehci_driver_overrides platform_over

Re: [PATCH v5 6/6] drm/bridge/sii8620: use micro-USB cable detection logic to detect MHL

[Andrzej Hajda]

On 27.02.2018 23:26, Chanwoo Choi wrote:
> Hi,
>
> On 2018년 02월 27일 21:05, Andrzej Hajda wrote:
>> On 27.02.2018 12:08, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> On 2018년 02월 27일 16:11, Andrzej Hajda wrote:
 From: Maciej Purski 

 Currently MHL chip must be turned on permanently to detect MHL cable. It
 duplicates micro-USB controller's (MUIC) functionality and consumes
 unnecessary power. Lets use extcon attached to MUIC to enable MHL chip
 only if it detects MHL cable.

 Signed-off-by: Maciej Purski 
 Signed-off-by: Andrzej Hajda 
 ---
 v5: updated extcon API

 This is rework of the patch by Maciej with following changes:
 - use micro-USB port bindings to get extcon, instead of extcon property,
 - fixed remove sequence,
 - fixed extcon get state logic.

 Code finding extcon node is hacky IMO, I guess ultimately it should be done
 via some framework (maybe even extcon), or at least via helper, I hope it
 can stay as is until the proper solution will be merged.

 Signed-off-by: Andrzej Hajda 
 ---
  drivers/gpu/drm/bridge/sil-sii8620.c | 97 
 ++--
  1 file changed, 94 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
 b/drivers/gpu/drm/bridge/sil-sii8620.c
 index 9e785b8e0ea2..62b0adabcac2 100644
 --- a/drivers/gpu/drm/bridge/sil-sii8620.c
 +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
 @@ -17,6 +17,7 @@
  
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 @@ -25,6 +26,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  
 @@ -81,6 +83,10 @@ struct sii8620 {
struct edid *edid;
unsigned int gen2_write_burst:1;
enum sii8620_mt_state mt_state;
 +  struct extcon_dev *extcon;
 +  struct notifier_block extcon_nb;
 +  struct work_struct extcon_wq;
 +  int cable_state;
struct list_head mt_queue;
struct {
int r_size;
 @@ -2175,6 +2181,77 @@ static void sii8620_init_rcp_input_dev(struct 
 sii8620 *ctx)
ctx->rc_dev = rc_dev;
  }
  
 +static void sii8620_cable_out(struct sii8620 *ctx)
 +{
 +  disable_irq(to_i2c_client(ctx->dev)->irq);
 +  sii8620_hw_off(ctx);
 +}
 +
 +static void sii8620_extcon_work(struct work_struct *work)
 +{
 +  struct sii8620 *ctx =
 +  container_of(work, struct sii8620, extcon_wq);
 +  int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
 +
 +  if (state == ctx->cable_state)
 +  return;
 +
 +  ctx->cable_state = state;
 +
 +  if (state > 0)
 +  sii8620_cable_in(ctx);
 +  else
 +  sii8620_cable_out(ctx);
 +}
 +
 +static int sii8620_extcon_notifier(struct notifier_block *self,
 +  unsigned long event, void *ptr)
 +{
 +  struct sii8620 *ctx =
 +  container_of(self, struct sii8620, extcon_nb);
 +
 +  schedule_work(&ctx->extcon_wq);
 +
 +  return NOTIFY_DONE;
 +}
 +
 +static int sii8620_extcon_init(struct sii8620 *ctx)
 +{
 +  struct extcon_dev *edev;
 +  struct device_node *musb, *muic;
 +  int ret;
 +
 +  /* get micro-USB connector node */
 +  musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
 +  /* next get micro-USB Interface Controller node */
 +  muic = of_get_next_parent(musb);
 +
 +  if (!muic) {
 +  dev_info(ctx->dev, "no extcon found, switching to 'always on' 
 mode\n");
 +  return 0;
 +  }
 +
 +  edev = extcon_find_edev_by_node(muic);
 +  of_node_put(muic);
 +  if (IS_ERR(edev)) {
 +  if (PTR_ERR(edev) == -EPROBE_DEFER)
 +  return -EPROBE_DEFER;
 +  dev_err(ctx->dev, "Invalid or missing extcon\n");
 +  return PTR_ERR(edev);
 +  }
 +
 +  ctx->extcon = edev;
 +  ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
 +  INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
 +  ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
>>> You better to use devm_extcon_register_notifier().
>> With devm version I risk that in case of device unbind notification will
>> be called after .remove callback, it seems to me quite dangerous. Or am
>> I missing something?
> If you use the cancel_work_sync() in remove() instead of flush_work(),
> you can use the 'devm_extcon_*'.

cancel_work_sync() does not prevent works scheduled later from execution
[1] and this scenario is possible with devm_extcon_register_notifier()
and cancel_work_sync().
So we end up with:
    sii8620_remove() calls cancel_work_sync()
...
    notifier(called asynchronously) schedules sii8620_extcon_work()
...
    notifier is removed by devm framework
    s

Re: [Regression] xhci: some hard drives cannot be seen using a JMicron JMS56x enclosure

[Mathias Nyman]

Hi

On 23.02.2018 04:55, Cyril Roelandt wrote:

Hello,

On 02/21/18 17:04, Mathias Nyman wrote:


could you change one flag in the xhci driver and take the same
set of logs? It will add more details about the URB and xHC hw position check.

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index cc368ad..f1b73e6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -772,7 +772,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
 hw_deq &= ~0xf;
 if (trb_in_td(xhci, cur_td->start_seg, cur_td->first_trb,
- cur_td->last_trb, hw_deq, false)) {
+ cur_td->last_trb, hw_deq, true)) {
 xhci_find_new_dequeue_state(xhci, slot_id, ep_index,
 cur_td->urb->stream_id,
 cur_td, &deq_state);





OK, so I tried this patch on top of 4.13. I'll post the logs at the end of this
message.


Another thing not related to xhci is that the serial number seems to be changing
for this device after reset.
This causes the "usb 4-2: device firmware changed" messages, and usb core will
try to logically disconnect the device.

Can you also plug in and out the device a few times without changing the
drives in the enclosure, and check if the serial number really is changing.


This behaviour can be seen on a machine running 4.11 (a version of the kernel
with which the enclosure works well):

# dmesg -T | grep "SerialNumber:"
[Fri Feb 23 02:43:20 2018] usb 2-2: SerialNumber: RANDOM__7758C4668BE7
[Fri Feb 23 02:43:38 2018] usb 2-2: SerialNumber: RANDOM__D6BA993A16EF
[Fri Feb 23 02:43:57 2018] usb 2-2: SerialNumber: RANDOM__DA02D6BA993A
[Fri Feb 23 02:55:45 2018] usb 2-2: SerialNumber: RANDOM__BB3BB6A08401


Thanks for your help, I'm leaving the (huge) logs below.
Cyril Roelandt.



Thanks for the logs, They clearly show that there is a issue in getting the
position where the second stream stopped.

When reading the stopped position for URB2 on stream 2 we get stream1 position,
in fact stream 2 never starts again after this, and when re-reading the stopped
position of stream 2 it keeps giving stream 1 position.

One possible cause is that a xhci command accidentally writes a "0" to "Max Primary 
Streams"
while configuring some other endpoint value, this would limit streams to 1.
Current tracing has a bug in displaying "Max Primaty streams" and always showed 
zero.

I have a series of even more custom debugging patches.
attached patches apply on 4.13, but seris for both 4.13 and 4.15 can be found 
in the
streams-debug-4.13 and streams-debug-4.15 branches at

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git

Can I ask you to do take logs with these?
no need for the previous diff, it's included

Thanks
Mathias
>From 1b6dc2f3d4a2d6a4b110858da27b69ae1182bc05 Mon Sep 17 00:00:00 2001
From: Mathias Nyman 
Date: Tue, 27 Feb 2018 16:01:48 +0200
Subject: [PATCH 1/3] xhci: fix endpoint context tracer output

Fix incorrent values showed for max Primary stream and
Linear stream array (LSA) values in the endpoint context
decoder.

Fixes: 19a7d0d65c4a ("usb: host: xhci: add Slot and EP Context tracers")
Cc:  # v4.12+
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e9352..77b3fa1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -721,11 +721,12 @@ struct xhci_ep_ctx {
 /* bits 10:14 are Max Primary Streams */
 /* bit 15 is Linear Stream Array */
 /* Interval - period between requests to an endpoint - 125u increments. */
-#define EP_INTERVAL(p)		(((p) & 0xff) << 16)
-#define EP_INTERVAL_TO_UFRAMES(p)		(1 << (((p) >> 16) & 0xff))
-#define CTX_TO_EP_INTERVAL(p)	(((p) >> 16) & 0xff)
-#define EP_MAXPSTREAMS_MASK	(0x1f << 10)
-#define EP_MAXPSTREAMS(p)	(((p) << 10) & EP_MAXPSTREAMS_MASK)
+#define EP_INTERVAL(p)			(((p) & 0xff) << 16)
+#define EP_INTERVAL_TO_UFRAMES(p)	(1 << (((p) >> 16) & 0xff))
+#define CTX_TO_EP_INTERVAL(p)		(((p) >> 16) & 0xff)
+#define EP_MAXPSTREAMS_MASK		(0x1f << 10)
+#define EP_MAXPSTREAMS(p)		(((p) << 10) & EP_MAXPSTREAMS_MASK)
+#define CTX_TO_EP_MAXPSTREAMS(p)	(((p) & EP_MAXPSTREAMS_MASK) >> 10)
 /* Endpoint is set up with a Linear Stream Array (vs. Secondary Stream Array) */
 #define	EP_HAS_LSA		(1 << 15)
 
@@ -2449,21 +2450,22 @@ static inline const char *xhci_decode_ep_context(u32 info, u32 info2, u64 deq,
 	u8 burst;
 	u8 cerr;
 	u8 mult;
-	u8 lsa;
-	u8 hid;
+
+	bool lsa;
+	bool hid;
 
 	esit = EP_MAX_ESIT_PAYLOAD_HI(info) << 16 |
 		EP_MAX_ESIT_PAYLOAD_LO(tx_info);
 
 	ep_state = info & EP_STATE_MASK;
-	max_pstr = info & EP_MAXPSTREAMS_MASK;
+	max_pstr = CTX_TO_EP_MAXPSTREAMS(info);
 	interval 

Re: [PATCH 0/2] Allow xhci-plat using a second clock

[Gregory CLEMENT]

Hi Mathias,
 
 On mer., févr. 14 2018, Gregory CLEMENT  wrote:

> Hello,
>
> The purpose of this series is to allow xhci-plat using a second
> clock. It is needed on the Armada 7K/8K but could be used by other
> SoCs.

Do you have some comments on this series and especially on the second
patch?

Thanks,

Gregory

>
> The first patch is just a fix found while I was working on this
> feature.
>
> Thanks,
>
> Gregory
>
>
> Gregory CLEMENT (2):
>   usb: host: xhci-plat: Remove useless test before clk_disable_unprepare
>   usb: host: xhci-plat: Fix clock resource by adding a register clock
>
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |  5 ++-
>  drivers/usb/host/xhci-plat.c   | 39 
> --
>  drivers/usb/host/xhci.h|  3 +-
>  3 files changed, 35 insertions(+), 12 deletions(-)
>
> -- 
> 2.15.1
>

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore

[Matthew Wilcox]

On Mon, Feb 26, 2018 at 11:04:57PM +0800, Kai-Heng Feng wrote:
> +static char quirks_param[128];
> +module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
> +MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying 
> quirks=vendorID:productID:quirks");
> +
> +static char quirks_param_orig[128];

> +static u32 usb_detect_dynamic_quirks(struct usb_device *udev)
> +{
> + u16 vid = le16_to_cpu(udev->descriptor.idVendor);
> + u16 pid = le16_to_cpu(udev->descriptor.idProduct);
> + struct quirk_entry *quirk;
> +
> + mutex_lock(&quirk_mutex);
> + if (strcmp(quirks_param, quirks_param_orig) != 0) {
> + strcpy(quirks_param_orig, quirks_param);

What happens if the user is writing to quirks_param at the same time
that you memcpy it?

I think you're going about this wrong by trying to use the
module_param_string machinery.  You should be using module_param_cb()
to build the quirks list when the user writes it (and then translate
back into a string when the user wants to read from it.
Also, you won't need to use a linked list for this; you can just allocate
an array of quirks.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 12/12] extcon: axp288: Set USB role where necessary

[Hans de Goede]

The AXP288 BC1.2 charger detection / extcon code may seem like a strange
place to add code to control the USB role-switch on devices with an AXP288,
but there are 2 reasons to do this inside the axp288 extcon code:

1) On many devices the USB role is controlled by ACPI AML code, but the AML
   code only switches between the host and none roles, because of Windows
   not really using device mode. To make device mode work we need to toggle
   between the none/device roles based on Vbus presence, and the axp288
   extcon gets interrupts on Vbus insertion / removal.

2) In order for our BC1.2 charger detection to work properly the role
   mux must be properly set to device mode before we do the detection.

Also note the Kconfig help-text / obsolete depends on USB_PHY which are
remnants from older never upstreamed code also controlling the mux from
the axp288 extcon code.

This commit also adds code to get notifications from the INT3496 extcon
device, which is used on some devices to notify the kernel about id-pin
changes instead of them being handled through AML code.

This fixes:
-Device mode not working on most CHT devices with an AXP288
-Host mode not working on devices with an INT3496 ACPI device
-Charger-type misdetection (always SDP) on devices with an INT3496 when the
 USB role (always) gets initialized as host

Reviewed-by: Heikki Krogerus 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v2:
-Add depends on X86 to Kconfig (the AXP288 PMIC is only used on X86)
-Use new acpi_dev_get_first_match_name() helper to get the INT3496 device-name
-Add Heikki's Reviewed-by
---
 drivers/extcon/Kconfig |   3 +-
 drivers/extcon/extcon-axp288.c | 177 +++--
 2 files changed, 171 insertions(+), 9 deletions(-)

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index a7bca4207f44..de15bf55895b 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -30,7 +30,8 @@ config EXTCON_ARIZONA
 
 config EXTCON_AXP288
tristate "X-Power AXP288 EXTCON support"
-   depends on MFD_AXP20X && USB_PHY
+   depends on MFD_AXP20X && USB_SUPPORT && X86
+   select USB_ROLE_SWITCH
help
  Say Y here to enable support for USB peripheral detection
  and USB MUX switching by X-Power AXP288 PMIC.
diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index 3ec4c715e240..51e77c7a32c2 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -1,6 +1,7 @@
 /*
  * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver
  *
+ * Copyright (c) 2017-2018 Hans de Goede 
  * Copyright (C) 2015 Intel Corporation
  * Author: Ramakrishna Pallala 
  *
@@ -14,6 +15,8 @@
  * GNU General Public License for more details.
  */
 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -25,6 +28,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+#include 
+#include 
 
 /* Power source status register */
 #define PS_STAT_VBUS_TRIGGER   BIT(0)
@@ -97,9 +105,19 @@ struct axp288_extcon_info {
struct device *dev;
struct regmap *regmap;
struct regmap_irq_chip_data *regmap_irqc;
+   struct usb_role_switch *role_sw;
+   struct work_struct role_work;
int irq[EXTCON_IRQ_END];
struct extcon_dev *edev;
+   struct extcon_dev *id_extcon;
+   struct notifier_block id_nb;
unsigned int previous_cable;
+   bool vbus_attach;
+};
+
+static const struct x86_cpu_id cherry_trail_cpu_ids[] = {
+   { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT, X86_FEATURE_ANY },
+   {}
 };
 
 /* Power up/down reason string array */
@@ -137,20 +155,74 @@ static void axp288_extcon_log_rsi(struct 
axp288_extcon_info *info)
regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask);
 }
 
-static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
+/*
+ * The below code to control the USB role-switch on devices with an AXP288
+ * may seem out of place, but there are 2 reasons why this is the best place
+ * to control the USB role-switch on such devices:
+ * 1) On many devices the USB role is controlled by AML code, but the AML code
+ *only switches between the host and none roles, because of Windows not
+ *really using device mode. To make device mode work we need to toggle
+ *between the none/device roles based on Vbus presence, and this driver
+ *gets interrupts on Vbus insertion / removal.
+ * 2) In order for our BC1.2 charger detection to work properly the role
+ *mux must be properly set to device mode before we do the detection.
+ */
+
+/* Returns the id-pin value, note pulled low / false == host-mode */
+static bool axp288_get_id_pin(struct axp288_extcon_info *info)
 {
-   int ret, stat, cfg, pwr_stat;
-   u8 chrg_type;
-   unsigned int cable = info->previous_cable;
-   bool vbus_attach = false;
+   enum usb

[PATCH v5 08/12] xhci: Add Intel extended cap / otg phy mux handling

[Hans de Goede]

The xHCI controller on various Intel SoCs 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 role-sw driver included in the xHCI code (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 role-sw driver for
the role-switch.

Note this commit adds a call to the new xhci_ext_cap_init() function
to xhci_pci_probe(), it is added here because xhci_ext_cap_init() must
be called only once. If in the future we also want to handle ext-caps
on non pci xHCI HCDs from xhci_ext_cap_init() a call to it should also
be added to other bus probe paths.

Acked-by: Mathias Nyman 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v2:
-Use SPDX license header
-Various small style cleanups / changes
-Add Heikki's Reviewed-by

Changes from some time ago when this patch was part of another patch-set:
-Check xHCI controller PCI device-id instead of only checking for the
 Intel Extended capability ID, as the Extended capability ID is used on
 other model Intel xHCI controllers too
-Add a new generic xhci_ext_cap_init() function and handle the new
 XHCI_INTEL_CHT_USB_MUX quirk there.
-Stop using Cherry Trail / CHT in various places as other Intel SoCs
 (e.g. Broxton / Apollo Lake) also have this
---
 drivers/usb/host/Makefile|  2 +-
 drivers/usb/host/xhci-ext-caps.c | 90 
 drivers/usb/host/xhci-ext-caps.h |  2 +
 drivers/usb/host/xhci-pci.c  |  5 +++
 drivers/usb/host/xhci.h  |  2 +
 5 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/host/xhci-ext-caps.c

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 4ede4ce12366..8a8cffe0b445 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -11,7 +11,7 @@ fhci-y += fhci-mem.o fhci-tds.o fhci-sched.o
 
 fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
 
-xhci-hcd-y := xhci.o xhci-mem.o
+xhci-hcd-y := xhci.o xhci-mem.o xhci-ext-caps.o
 xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
 xhci-hcd-y += xhci-trace.o
 
diff --git a/drivers/usb/host/xhci-ext-caps.c b/drivers/usb/host/xhci-ext-caps.c
new file mode 100644
index ..399113f9fc5c
--- /dev/null
+++ b/drivers/usb/host/xhci-ext-caps.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * XHCI extended capability handling
+ *
+ * Copyright (c) 2017 Hans de Goede 
+ */
+
+#include 
+#include "xhci.h"
+
+#define USB_SW_DRV_NAME"intel_xhci_usb_sw"
+#define USB_SW_RESOURCE_SIZE   0x400
+
+static void xhci_intel_unregister_pdev(void *arg)
+{
+   platform_device_unregister(arg);
+}
+
+static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd *xhci, u32 
cap_offset)
+{
+   struct usb_hcd *hcd = xhci_to_hcd(xhci);
+   struct device *dev = hcd->self.controller;
+   struct platform_device *pdev;
+   struct resource res = { 0, };
+   int ret;
+
+   pdev = platform_device_alloc(USB_SW_DRV_NAME, PLATFORM_DEVID_NONE);
+   if (!pdev) {
+   xhci_err(xhci, "couldn't allocate %s platform device\n",
+USB_SW_DRV_NAME);
+   return -ENOMEM;
+   }
+
+   res.start = hcd->rsrc_start + cap_offset;
+   res.end   = res.start + USB_SW_RESOURCE_SIZE - 1;
+   res.name  = USB_SW_DRV_NAME;
+   res.flags = IORESOURCE_MEM;
+
+   ret = platform_device_add_resources(pdev, &res, 1);
+   if (ret) {
+   dev_err(dev, "couldn't add resources to intel_xhci_usb_sw 
pdev\n");
+   platform_device_put(pdev);
+   return ret;
+   }
+
+   pdev->dev.parent = dev;
+
+   ret = platform_device_add(pdev);
+   if (ret) {
+   dev_err(dev, "couldn't register intel_xhci_usb_sw pdev\n");
+   platform_device_put(pdev);
+   return ret;
+   }
+
+   ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
+   if (ret) {
+   dev_err(dev, "couldn't add unregister action for 
intel_xhci_usb_sw pdev\n");
+   return ret;
+   }
+
+   return 0;
+}
+
+int xhci_ext_cap_init(struct xhci_hcd *xhci)
+{
+   void __iomem *base = &xhci->cap_regs->hc_capbase;
+   u32 offset, val;
+   int ret;
+
+   offset = xhci_find_next_ext_cap(base, 0, 0);
+
+   while (offset) {
+   val = readl(base + offset);
+
+   switch (XHCI_EXT_CAPS_ID(val)) {
+   case XHCI_EXT_CAPS_VENDOR_INTEL:
+   if (xhci->quirks & XHCI_INTEL_USB_ROLE_SW) {
+   ret = xhci_create_intel_xhci_sw_pdev(xhci,
+  

[PATCH v5 05/12] usb: typec: tcpm: Set USB role switch to device mode when configured as such

[Hans de Goede]

Setting the mux to MUX_NONE and the switch to USB_SWITCH_DISCONNECT when
the data-role is device is not correct. Plenty of devices support
operating as USB device through a (separate) USB device controller.

We really need 2 different versions of USB_SWITCH_CONNECT,
USB_SWITCH_CONNECT_HOST and USB_SWITCH_DEVICE. Rather then modifying the
tcpc_usb_switch enum for this, simply remove it and switch to the
usb_role enum which provides exactly this, this will save use needing to
convert betweent the 2 enums when calling an usb-role-switch driver later.

Besides switching to the usb_role type, this commit also actually sets the
mux to TYPEC_MUX_USB and the switch to USB_ROLE_DEVICE instead of setting
both to none when the data-role is device.

This commit also makes tcpm_reset_port() call tcpm_mux_set(port,
TYPEC_MUX_NONE, USB_ROLE_NONE) so that the mux and switch
do _not_ stay in their last mode after a detach.

Reviewed-by: Heikki Krogerus 
Reviewed-by: Guenter Roeck 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v3:
-Add Guenter's Reviewed-by

Changes in v2:
-Added Heikki's Reviewed-by
---
 drivers/usb/typec/tcpm.c | 22 +++---
 include/linux/usb/tcpm.h |  8 ++--
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 7cd28b700a7f..00ca2822432f 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -618,15 +618,15 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
 EXPORT_SYMBOL_GPL(tcpm_pd_transmit_complete);
 
 static int tcpm_mux_set(struct tcpm_port *port, enum tcpc_mux_mode mode,
-   enum tcpc_usb_switch config)
+   enum usb_role usb_role)
 {
int ret = 0;
 
-   tcpm_log(port, "Requesting mux mode %d, config %d, polarity %d",
-mode, config, port->polarity);
+   tcpm_log(port, "Requesting mux mode %d, usb-role %d, polarity %d",
+mode, usb_role, port->polarity);
 
if (port->tcpc->mux)
-   ret = port->tcpc->mux->set(port->tcpc->mux, mode, config,
+   ret = port->tcpc->mux->set(port->tcpc->mux, mode, usb_role,
   port->polarity);
 
return ret;
@@ -742,14 +742,15 @@ static int tcpm_set_attached_state(struct tcpm_port 
*port, bool attached)
 static int tcpm_set_roles(struct tcpm_port *port, bool attached,
  enum typec_role role, enum typec_data_role data)
 {
+   enum usb_role usb_role;
int ret;
 
if (data == TYPEC_HOST)
-   ret = tcpm_mux_set(port, TYPEC_MUX_USB,
-  TCPC_USB_SWITCH_CONNECT);
+   usb_role = USB_ROLE_HOST;
else
-   ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
-  TCPC_USB_SWITCH_DISCONNECT);
+   usb_role = USB_ROLE_DEVICE;
+
+   ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role);
if (ret < 0)
return ret;
 
@@ -2096,7 +2097,7 @@ static int tcpm_src_attach(struct tcpm_port *port)
 out_disable_pd:
port->tcpc->set_pd_rx(port->tcpc, false);
 out_disable_mux:
-   tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
+   tcpm_mux_set(port, TYPEC_MUX_NONE, USB_ROLE_NONE);
return ret;
 }
 
@@ -2140,6 +2141,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
tcpm_init_vconn(port);
tcpm_set_current_limit(port, 0, 0);
tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
+   tcpm_mux_set(port, TYPEC_MUX_NONE, USB_ROLE_NONE);
tcpm_set_attached_state(port, false);
port->try_src_count = 0;
port->try_snk_count = 0;
@@ -2190,8 +2192,6 @@ static int tcpm_snk_attach(struct tcpm_port *port)
 static void tcpm_snk_detach(struct tcpm_port *port)
 {
tcpm_detach(port);
-
-   /* XXX: (Dis)connect SuperSpeed mux? */
 }
 
 static int tcpm_acc_attach(struct tcpm_port *port)
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index ca1c0b57f03f..268721bff2c1 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -16,6 +16,7 @@
 #define __LINUX_USB_TCPM_H
 
 #include 
+#include 
 #include 
 #include "pd.h"
 
@@ -97,11 +98,6 @@ struct tcpc_config {
const struct typec_altmode_desc *alt_modes;
 };
 
-enum tcpc_usb_switch {
-   TCPC_USB_SWITCH_CONNECT,
-   TCPC_USB_SWITCH_DISCONNECT,
-};
-
 /* Mux state attributes */
 #define TCPC_MUX_USB_ENABLED   BIT(0)  /* USB enabled */
 #define TCPC_MUX_DP_ENABLEDBIT(1)  /* DP enabled */
@@ -118,7 +114,7 @@ enum tcpc_mux_mode {
 
 struct tcpc_mux_dev {
int (*set)(struct tcpc_mux_dev *dev, enum tcpc_mux_mode mux_mode,
-  enum tcpc_usb_switch usb_config,
+  enum usb_role usb_role,
   enum typec_cc_polarity polarity);
bool dfp_only;
void *priv_

[PATCH v5 10/12] usb: typec: driver for Pericom PI3USB30532 Type-C cross switch

[Hans de Goede]

Add a driver for the Pericom PI3USB30532 Type-C cross switch /
mux chip found on some devices with a Type-C port.

Reviewed-by: Heikki Krogerus 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v2:
-Cleanup pi3usb30532_set_conf() error handling
-Add Heikki's Reviewed-by

Changes in v1:
-This is a rewrite of my previous driver which was using the
 drivers/mux framework to use the new drivers/usb/typec/mux framework
---
 MAINTAINERS |   6 ++
 drivers/usb/typec/Kconfig   |   2 +
 drivers/usb/typec/Makefile  |   1 +
 drivers/usb/typec/mux/Kconfig   |  10 ++
 drivers/usb/typec/mux/Makefile  |   3 +
 drivers/usb/typec/mux/pi3usb30532.c | 178 
 6 files changed, 200 insertions(+)
 create mode 100644 drivers/usb/typec/mux/Kconfig
 create mode 100644 drivers/usb/typec/mux/Makefile
 create mode 100644 drivers/usb/typec/mux/pi3usb30532.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 523f14b4216d..cc66ac1b9ee0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14530,6 +14530,12 @@ F: drivers/usb/
 F: include/linux/usb.h
 F: include/linux/usb/
 
+USB TYPEC PI3USB30532 MUX DRIVER
+M: Hans de Goede 
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/typec/mux/pi3usb30532.c
+
 USB TYPEC SUBSYSTEM
 M: Heikki Krogerus 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index a2a0684e7c82..030f88cb0c3f 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -85,4 +85,6 @@ config TYPEC_TPS6598X
  If you choose to build this driver as a dynamically linked module, the
  module will be called tps6598x.ko.
 
+source "drivers/usb/typec/mux/Kconfig"
+
 endif # TYPEC
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 56b2e9516ec1..1f599a6c30cc 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -6,3 +6,4 @@ obj-y   += fusb302/
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tps6598x.o
+obj-$(CONFIG_TYPEC)+= mux/
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
new file mode 100644
index ..9a954d2b8d8f
--- /dev/null
+++ b/drivers/usb/typec/mux/Kconfig
@@ -0,0 +1,10 @@
+menu "USB Type-C Multiplexer/DeMultiplexer Switch support"
+
+config TYPEC_MUX_PI3USB30532
+   tristate "Pericom PI3USB30532 Type-C cross switch driver"
+   depends on I2C
+   help
+ Say Y or M if your system has a Pericom PI3USB30532 Type-C cross
+ switch / mux chip found on some devices with a Type-C port.
+
+endmenu
diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
new file mode 100644
index ..1332e469b8a0
--- /dev/null
+++ b/drivers/usb/typec/mux/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_TYPEC_MUX_PI3USB30532)+= pi3usb30532.o
diff --git a/drivers/usb/typec/mux/pi3usb30532.c 
b/drivers/usb/typec/mux/pi3usb30532.c
new file mode 100644
index ..86cda9f388f3
--- /dev/null
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Pericom PI3USB30532 Type-C cross switch / mux driver
+ *
+ * Copyright (c) 2017-2018 Hans de Goede 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define PI3USB30532_CONF   0x00
+
+#define PI3USB30532_CONF_OPEN  0x00
+#define PI3USB30532_CONF_SWAP  0x01
+#define PI3USB30532_CONF_4LANE_DP  0x02
+#define PI3USB30532_CONF_USB3  0x04
+#define PI3USB30532_CONF_USB3_AND_2LANE_DP 0x06
+
+struct pi3usb30532 {
+   struct i2c_client *client;
+   struct mutex lock; /* protects the cached conf register */
+   struct typec_switch sw;
+   struct typec_mux mux;
+   u8 conf;
+};
+
+static int pi3usb30532_set_conf(struct pi3usb30532 *pi, u8 new_conf)
+{
+   int ret = 0;
+
+   if (pi->conf == new_conf)
+   return 0;
+
+   ret = i2c_smbus_write_byte_data(pi->client, PI3USB30532_CONF, new_conf);
+   if (ret) {
+   dev_err(&pi->client->dev, "Error writing conf: %d\n", ret);
+   return ret;
+   }
+
+   pi->conf = new_conf;
+   return 0;
+}
+
+static int pi3usb30532_sw_set(struct typec_switch *sw,
+ enum typec_orientation orientation)
+{
+   struct pi3usb30532 *pi = container_of(sw, struct pi3usb30532, sw);
+   u8 new_conf;
+   int ret;
+
+   mutex_lock(&pi->lock);
+   new_conf = pi->conf;
+
+   switch (orientation) {
+   case TYPEC_ORIENTATION_NONE:
+   new_conf = PI3USB30532_CONF_OPEN;
+   break;
+   case TYPEC_ORIENTATION_NORMAL:
+   new_conf &= ~PI3USB

[PATCH v5 07/12] xhci: Add option to get next extended capability in list by passing id = 0

[Hans de Goede]

From: Mathias Nyman 

Modify xhci_find_next_ext_cap(base, offset, id) to return the next
capability offset if 0 is passed for id. Otherwise it will behave as
previously and return the offset of the next capability with matching id

capability id 0 is not used by xHCI (reserved)

This is useful when we want to loop through all capabilities.

Signed-off-by: Mathias Nyman 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v2:
-Added Heikki's Reviewed-by
---
 drivers/usb/host/xhci-ext-caps.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index bf7316e130d3..631e7cc62604 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -84,7 +84,8 @@
  * @base   PCI MMIO registers base address.
  * @start  address at which to start looking, (0 or HCC_PARAMS to start at
  * beginning of list)
- * @id Extended capability ID to search for.
+ * @id Extended capability ID to search for, or 0 for the next
+ * capability
  *
  * Returns the offset of the next matching extended capability structure.
  * Some capabilities can occur several times, e.g., the XHCI_EXT_CAPS_PROTOCOL,
@@ -110,7 +111,7 @@ static inline int xhci_find_next_ext_cap(void __iomem 
*base, u32 start, int id)
val = readl(base + offset);
if (val == ~0)
return 0;
-   if (XHCI_EXT_CAPS_ID(val) == id && offset != start)
+   if (offset != start && (id == 0 || XHCI_EXT_CAPS_ID(val) == id))
return offset;
 
next = XHCI_EXT_CAPS_NEXT(val);
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 04/12] usb: common: Small class for USB role switches

[Hans de Goede]

From: Heikki Krogerus 

USB role switch is a device that can be used to choose the
data role for USB connector. With dual-role capable USB
controllers, the controller itself will be the switch, but
on some platforms the USB host and device controllers are
separate IPs and there is a mux between them and the
connector. On those platforms the mux driver will need to
register the switch.

With USB Type-C connectors, the host-to-device relationship
is negotiated over the Configuration Channel (CC). That
means the USB Type-C drivers need to be in control of the
role switch. The class provides a simple API for the USB
Type-C drivers for the control.

For other types of USB connectors (mainly microAB) the class
provides user space control via sysfs attribute file that
can be used to request role swapping from the switch.

Signed-off-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Consistently use IS_ERR_OR_NULL where applicable
-Add Andy's Reviewed-by

Changes in v2:
-Minor style fixes from review of v1
---
 Documentation/ABI/testing/sysfs-class-usb_role |  21 ++
 drivers/usb/Kconfig|   3 +
 drivers/usb/common/Makefile|   1 +
 drivers/usb/common/roles.c | 305 +
 include/linux/usb/role.h   |  51 +
 5 files changed, 381 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-usb_role
 create mode 100644 drivers/usb/common/roles.c
 create mode 100644 include/linux/usb/role.h

diff --git a/Documentation/ABI/testing/sysfs-class-usb_role 
b/Documentation/ABI/testing/sysfs-class-usb_role
new file mode 100644
index ..3b810a425a52
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-usb_role
@@ -0,0 +1,21 @@
+What:  /sys/class/usb_role/
+Date:  Jan 2018
+Contact:   Heikki Krogerus 
+Description:
+   Place in sysfs for USB Role Switches. USB Role Switch is a
+   device that can select the data role (host or device) for USB
+   port.
+
+What:  /sys/class/usb_role//role
+Date:  Jan 2018
+Contact:   Heikki Krogerus 
+Description:
+   The current role of the switch. This attribute can be used for
+   requesting role swapping with non-USB Type-C ports. With USB
+   Type-C ports, the ABI defined for USB Type-C connector class
+   must be used.
+
+   Valid values:
+   - none
+   - host
+   - device
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 148f3ee70286..f278958e04ca 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -203,4 +203,7 @@ config USB_ULPI_BUS
  To compile this driver as a module, choose M here: the module will
  be called ulpi.
 
+config USB_ROLE_SWITCH
+   tristate
+
 endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 0a7c45e85481..fb4d5ef4165c 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -9,3 +9,4 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
 obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
+obj-$(CONFIG_USB_ROLE_SWITCH)  += roles.o
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
new file mode 100644
index ..bd616042afca
--- /dev/null
+++ b/drivers/usb/common/roles.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Role Switch Support
+ *
+ * Copyright (C) 2018 Intel Corporation
+ * Author: Heikki Krogerus 
+ * Hans de Goede 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static struct class *role_class;
+
+struct usb_role_switch {
+   struct device dev;
+   struct mutex lock; /* device lock*/
+   enum usb_role role;
+
+   /* From descriptor */
+   struct device *usb2_port;
+   struct device *usb3_port;
+   struct device *udc;
+   usb_role_switch_set_t set;
+   usb_role_switch_get_t get;
+   bool allow_userspace_control;
+};
+
+#define to_role_switch(d)  container_of(d, struct usb_role_switch, dev)
+
+/**
+ * usb_role_switch_set_role - Set USB role for a switch
+ * @sw: USB role switch
+ * @role: USB role to be switched to
+ *
+ * Set USB role @role for @sw.
+ */
+int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
+{
+   int ret;
+
+   if (IS_ERR_OR_NULL(sw))
+   return 0;
+
+   mutex_lock(&sw->lock);
+
+   ret = sw->set(sw->dev.parent, role);
+   if (!ret)
+   sw->role = role;
+
+   mutex_unlock(&sw->lock);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_set_role);
+
+/**
+ * usb_role_switch_get_role - Get the USB role for a switch
+ * @sw: USB role switch
+ *
+ * Depending on the role-switch-driver this function returns either a cached
+ * val

[PATCH v5 11/12] platform/x86: intel_cht_int33fe: Add device connections for the Type-C port

[Hans de Goede]

We need to add device-connections for the Type-C mux/switch and usb-role
code to be able to find the PI3USB30532 Type-C cross-switch and the
device/host role-switch integrated in the CHT SoC.

Reviewed-by: Heikki Krogerus 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Replace Andy's Acked-by with his Reviewed-by

Changes in v2:
-Add Andy's Acked-by
-Add Heikki's Reviewed-by
---
 drivers/platform/x86/intel_cht_int33fe.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 380ef7ec094f..a3f8674f14da 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -21,6 +21,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -33,6 +34,8 @@ struct cht_int33fe_data {
struct i2c_client *max17047;
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
+   /* Contain a list-head must be per device */
+   struct devcon connections[3];
 };
 
 /*
@@ -172,6 +175,20 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
+   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[0].id = "typec-switch";
+   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[1].id = "typec-mux";
+   data->connections[2].endpoint[0] = "i2c-fusb302";
+   data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
+   data->connections[2].id = "usb-role-switch";
+
+   add_device_connection(&data->connections[0]);
+   add_device_connection(&data->connections[1]);
+   add_device_connection(&data->connections[2]);
+
memset(&board_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
board_info.dev_name = "fusb302";
@@ -201,6 +218,10 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
+   remove_device_connection(&data->connections[2]);
+   remove_device_connection(&data->connections[1]);
+   remove_device_connection(&data->connections[0]);
+
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
 
@@ -213,6 +234,10 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
+   remove_device_connection(&data->connections[2]);
+   remove_device_connection(&data->connections[1]);
+   remove_device_connection(&data->connections[0]);
+
return 0;
 }
 
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 09/12] usb: roles: Add Intel xHCI USB role switch driver

[Hans de Goede]

Various Intel SoCs (Cherry Trail, Broxton and others) have an internal USB
role switch for swiching the OTG USB data lines between the xHCI host
controller and the dwc3 gadget controller.

Note on some Cherry Trail systems there is ACPI/AML code listening to
edge interrupts on the id-pin (through an _AIE ACPI method) and switching
the role between ROLE_HOST and ROLE_NONE based on the id-pin. Note it does
not set the role to ROLE_DEVICE, because device-mode is usually not used
under Windows.

The presence of AML code which modifies the cfg0 reg (on some systems)
means that our read/write/modify of cfg0 may race with the AML code
doing the same to avoid this we take the global ACPI lock while doing
the read/write/modify.

Reviewed-by: Andy Shevchenko 
Reviewed-by: Heikki Krogerus 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by
-Add Heikki's Reviewed-by

Changes in v2:
-Drop unnecessary depends on EXTCON from Kconfig
-Use BIT(), resource_size()
-Various other small style fixes
---
 MAINTAINERS|   6 +
 drivers/usb/Kconfig|   2 +
 drivers/usb/Makefile   |   2 +
 drivers/usb/roles/Kconfig  |  14 ++
 drivers/usb/roles/Makefile |   1 +
 drivers/usb/roles/intel-xhci-usb-role-switch.c | 192 +
 6 files changed, 217 insertions(+)
 create mode 100644 drivers/usb/roles/Kconfig
 create mode 100644 drivers/usb/roles/Makefile
 create mode 100644 drivers/usb/roles/intel-xhci-usb-role-switch.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0f66f044f988..523f14b4216d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14394,6 +14394,12 @@ S: Maintained
 F: Documentation/hid/hiddev.txt
 F: drivers/hid/usbhid/
 
+USB INTEL XHCI ROLE MUX DRIVER
+M: Hans de Goede 
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/roles/intel-xhci-usb-role-switch.c
+
 USB ISP116X DRIVER
 M: Olav Kongas 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index f278958e04ca..75f7fb151f71 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -171,6 +171,8 @@ source "drivers/usb/gadget/Kconfig"
 
 source "drivers/usb/typec/Kconfig"
 
+source "drivers/usb/roles/Kconfig"
+
 config USB_LED_TRIG
bool "USB LED Triggers"
depends on LEDS_CLASS && LEDS_TRIGGERS
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 060643a1b5c8..7d1b8c82b208 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -65,3 +65,5 @@ obj-$(CONFIG_USB_COMMON)  += common/
 obj-$(CONFIG_USBIP_CORE)   += usbip/
 
 obj-$(CONFIG_TYPEC)+= typec/
+
+obj-$(CONFIG_USB_ROLE_SWITCH)  += roles/
diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig
new file mode 100644
index ..f5a5e6f79f1b
--- /dev/null
+++ b/drivers/usb/roles/Kconfig
@@ -0,0 +1,14 @@
+if USB_ROLE_SWITCH
+
+config USB_ROLES_INTEL_XHCI
+   tristate "Intel XHCI USB Role Switch"
+   depends on ACPI && X86
+   help
+ Driver for the internal USB role switch for switching the USB data
+ lines between the xHCI host controller and the dwc3 gadget controller
+ found on various Intel SoCs.
+
+ To compile the driver as a module, choose M here: the module will
+ be called intel-xhci-usb-role-switch.
+
+endif # USB_ROLE_SWITCH
diff --git a/drivers/usb/roles/Makefile b/drivers/usb/roles/Makefile
new file mode 100644
index ..e44b179ba275
--- /dev/null
+++ b/drivers/usb/roles/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_USB_ROLES_INTEL_XHCI) += intel-xhci-usb-role-switch.o
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
new file mode 100644
index ..3d7be5eace49
--- /dev/null
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Intel XHCI (Cherry Trail, Broxton and others) USB OTG role switch driver
+ *
+ * Copyright (c) 2016-2017 Hans de Goede 
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Author: Wu, Hao
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* register definition */
+#define DUAL_ROLE_CFG0 0x68
+#define SW_VBUS_VALID  BIT(24)
+#define SW_IDPIN_ENBIT(21)
+#define SW_IDPIN   BIT(20)
+
+#define DUAL_ROLE_CFG1 0x6c
+#define HOST_MODE  BIT(29)
+
+#define DUAL_ROLE_CFG1_POLL_TIMEOUT1000
+
+#define DRV_NAME   "intel_xhci_usb_sw"
+
+struct intel_xhci_usb_data {
+   struct usb_role_switch *role_sw;
+   void __iomem *base;
+};
+
+struct intel_xhci_acpi_match {
+   const char *hid;
+   int hrv;
+};
+
+/*
+ * ACPI IDs for PMICs which do not support separate data and pow

Re: [PATCH 0/3] console: Expand dummy functions for CFI

[Bartlomiej Zolnierkiewicz]

On Monday, February 26, 2018 04:04:17 PM Kees Cook wrote:
> This is a small series that cleans up struct consw a bit and
> prepares it for Control Flow Integrity checking (i.e. Clang's
> -fsanitize=cfi).

for drivers/video/ parts:

Acked-by: Bartlomiej Zolnierkiewicz 

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 03/12] usb: typec: API for controlling USB Type-C Multiplexers

[Hans de Goede]

From: Heikki Krogerus 

USB Type-C connectors consist of various muxes and switches
that route the pins on the connector to the right locations.
The USB Type-C drivers need to be able to control the muxes,
as they are the ones that know things like the cable plug
orientation, and the current mode that was negotiated with
the partner.

This introduces a small API for registering and controlling
cable plug orientation switches, and separate small API for
registering and controlling pin multiplexer/demultiplexer
switches that are needed with Accessory/Alternate Modes.

Signed-off-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v3:
-Add #include-s for a few missing headers to drivers/usb/typec/mux.c
-Various spelling and gramar fixes in the docs pointed out by Randy Dunlap
---
 Documentation/driver-api/usb/typec.rst |  73 +++--
 drivers/usb/typec/Makefile |   1 +
 drivers/usb/typec/{typec.c => class.c} |  70 
 drivers/usb/typec/mux.c| 190 +
 include/linux/usb/typec.h  |  14 +++
 include/linux/usb/typec_mux.h  |  55 ++
 6 files changed, 392 insertions(+), 11 deletions(-)
 rename drivers/usb/typec/{typec.c => class.c} (95%)
 create mode 100644 drivers/usb/typec/mux.c
 create mode 100644 include/linux/usb/typec_mux.h

diff --git a/Documentation/driver-api/usb/typec.rst 
b/Documentation/driver-api/usb/typec.rst
index 8a7249f2ff04..feb31946490b 100644
--- a/Documentation/driver-api/usb/typec.rst
+++ b/Documentation/driver-api/usb/typec.rst
@@ -61,7 +61,7 @@ Registering the ports
 The port drivers will describe every Type-C port they control with struct
 typec_capability data structure, and register them with the following API:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_register_port typec_unregister_port
 
 When registering the ports, the prefer_role member in struct typec_capability
@@ -80,7 +80,7 @@ typec_partner_desc. The class copies the details of the 
partner during
 registration. The class offers the following API for registering/unregistering
 partners.
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_register_partner typec_unregister_partner
 
 The class will provide a handle to struct typec_partner if the registration was
@@ -92,7 +92,7 @@ should include handle to struct usb_pd_identity instance. The 
class will then
 create a sysfs directory for the identity under the partner device. The result
 of Discover Identity command can then be reported with the following API:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_partner_set_identity
 
 Registering Cables
@@ -113,7 +113,7 @@ typec_cable_desc and about a plug in struct 
typec_plug_desc. The class copies
 the details during registration. The class offers the following API for
 registering/unregistering cables and their plugs:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_register_cable typec_unregister_cable typec_register_plug 
typec_unregister_plug
 
 The class will provide a handle to struct typec_cable and struct typec_plug if
@@ -125,7 +125,7 @@ include handle to struct usb_pd_identity instance. The 
class will then create a
 sysfs directory for the identity under the cable device. The result of Discover
 Identity command can then be reported with the following API:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_cable_set_identity
 
 Notifications
@@ -135,7 +135,7 @@ When the partner has executed a role change, or when the 
default roles change
 during connection of a partner or cable, the port driver must use the following
 APIs to report it to the class:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_set_data_role typec_set_pwr_role typec_set_vconn_role 
typec_set_pwr_opmode
 
 Alternate Modes
@@ -150,7 +150,7 @@ and struct typec_altmode_desc which is a container for all 
the supported modes.
 Ports that support Alternate Modes need to register each SVID they support with
 the following API:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_port_register_altmode
 
 If a partner or cable plug provides a list of SVIDs as response to USB Power
@@ -159,12 +159,12 @@ registered.
 
 API for the partners:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_partner_register_altmode
 
 API for the Cable Plugs:
 
-.. kernel-doc:: drivers/usb/typec/typec.c
+.. kernel-doc:: drivers/usb/typec/class.c
:functions: typec_plug_register_altmode
 
 So po

[PATCH v5 06/12] usb: typec: tcpm: Use new Type-C switch/mux and usb-role-switch functions

[Hans de Goede]

Remove the unused (not implemented anywhere) tcpc_mux_dev abstraction
and replace it with calling the new typec_set_orientation,
usb_role_switch_set and typec_set_mode functions.

Reviewed-by: Heikki Krogerus 
Reviewed-by: Guenter Roeck 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v3:
-Add Guenter's Reviewed-by

Changes in v2:
-Added Heikki's Reviewed-by
---
 drivers/usb/typec/Kconfig   |  1 +
 drivers/usb/typec/fusb302/fusb302.c |  1 -
 drivers/usb/typec/tcpm.c| 46 -
 include/linux/usb/tcpm.h| 10 
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index bcb2744c5977..a2a0684e7c82 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -48,6 +48,7 @@ if TYPEC
 config TYPEC_TCPM
tristate "USB Type-C Port Controller Manager"
depends on USB
+   select USB_ROLE_SWITCH
help
  The Type-C Port Controller Manager provides a USB PD and USB Type-C
  state machine for use with Type-C Port Controllers.
diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index dcd8ef085b30..a7b06053a538 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1249,7 +1249,6 @@ static void init_tcpc_dev(struct tcpc_dev 
*fusb302_tcpc_dev)
fusb302_tcpc_dev->set_roles = tcpm_set_roles;
fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
-   fusb302_tcpc_dev->mux = NULL;
 }
 
 static const char * const cc_polarity_name[] = {
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 00ca2822432f..bfcaf6618a1f 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -176,6 +177,7 @@ struct tcpm_port {
struct typec_port *typec_port;
 
struct tcpc_dev *tcpc;
+   struct usb_role_switch *role_sw;
 
enum typec_role vconn_role;
enum typec_role pwr_role;
@@ -618,18 +620,25 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
 EXPORT_SYMBOL_GPL(tcpm_pd_transmit_complete);
 
 static int tcpm_mux_set(struct tcpm_port *port, enum tcpc_mux_mode mode,
-   enum usb_role usb_role)
+   enum usb_role usb_role,
+   enum typec_orientation orientation)
 {
-   int ret = 0;
+   int ret;
 
-   tcpm_log(port, "Requesting mux mode %d, usb-role %d, polarity %d",
-mode, usb_role, port->polarity);
+   tcpm_log(port, "Requesting mux mode %d, usb-role %d, orientation %d",
+mode, usb_role, orientation);
 
-   if (port->tcpc->mux)
-   ret = port->tcpc->mux->set(port->tcpc->mux, mode, usb_role,
-  port->polarity);
+   ret = typec_set_orientation(port->typec_port, orientation);
+   if (ret)
+   return ret;
 
-   return ret;
+   if (port->role_sw) {
+   ret = usb_role_switch_set_role(port->role_sw, usb_role);
+   if (ret)
+   return ret;
+   }
+
+   return typec_set_mode(port->typec_port, mode);
 }
 
 static int tcpm_set_polarity(struct tcpm_port *port,
@@ -742,15 +751,21 @@ static int tcpm_set_attached_state(struct tcpm_port 
*port, bool attached)
 static int tcpm_set_roles(struct tcpm_port *port, bool attached,
  enum typec_role role, enum typec_data_role data)
 {
+   enum typec_orientation orientation;
enum usb_role usb_role;
int ret;
 
+   if (port->polarity == TYPEC_POLARITY_CC1)
+   orientation = TYPEC_ORIENTATION_NORMAL;
+   else
+   orientation = TYPEC_ORIENTATION_REVERSE;
+
if (data == TYPEC_HOST)
usb_role = USB_ROLE_HOST;
else
usb_role = USB_ROLE_DEVICE;
 
-   ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role);
+   ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role, orientation);
if (ret < 0)
return ret;
 
@@ -2097,7 +2112,8 @@ static int tcpm_src_attach(struct tcpm_port *port)
 out_disable_pd:
port->tcpc->set_pd_rx(port->tcpc, false);
 out_disable_mux:
-   tcpm_mux_set(port, TYPEC_MUX_NONE, USB_ROLE_NONE);
+   tcpm_mux_set(port, TYPEC_MUX_NONE, USB_ROLE_NONE,
+TYPEC_ORIENTATION_NONE);
return ret;
 }
 
@@ -2141,7 +2157,8 @@ static void tcpm_reset_port(struct tcpm_port *port)
tcpm_init_vconn(port);
tcpm_set_current_limit(port, 0, 0);
tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
-   tcpm_mux_set(port, TYPEC_MUX_NONE, USB_ROLE_NONE);
+   tcpm_mux_set(port, TYPEC_MUX_NONE, USB_ROLE_NONE,
+TYP

[PATCH v5 02/12] usb: typec: Start using ERR_PTR

[Hans de Goede]

From: Heikki Krogerus 

In order to allow the USB Type-C Class driver take care of
things like muxes and other possible dependencies for the
port drivers, returning ERR_PTR instead of NULL from the
registration functions in case of failure.

The reason for taking over control of the muxes for example
is because handling them in the port drivers would be just
boilerplate.

Signed-off-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Reviewed-by: Guenter Roeck 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Add Andy's Reviewed-by

Changes in v3:
-Add Guenter's Reviewed-by

Changes in v2:
-Add IS_ERR_OR_NULL() checks to the unregister functions
---
 drivers/usb/typec/tcpm.c  | 16 +---
 drivers/usb/typec/tps6598x.c  | 15 ---
 drivers/usb/typec/typec.c | 44 +--
 drivers/usb/typec/ucsi/ucsi.c | 31 ++
 4 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index f4d563ee7690..7cd28b700a7f 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1044,7 +1044,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
break;
case CMDT_RSP_ACK:
/* silently drop message if we are not connected */
-   if (!port->partner)
+   if (IS_ERR_OR_NULL(port->partner))
break;
 
switch (cmd) {
@@ -3743,8 +3743,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
port->port_type = tcpc->config->type;
 
port->typec_port = typec_register_port(port->dev, &port->typec_caps);
-   if (!port->typec_port) {
-   err = -ENOMEM;
+   if (IS_ERR(port->typec_port)) {
+   err = PTR_ERR(port->typec_port);
goto out_destroy_wq;
}
 
@@ -3753,15 +3753,17 @@ struct tcpm_port *tcpm_register_port(struct device 
*dev, struct tcpc_dev *tcpc)
 
i = 0;
while (paltmode->svid && i < ARRAY_SIZE(port->port_altmode)) {
-   port->port_altmode[i] =
- typec_port_register_altmode(port->typec_port,
- paltmode);
-   if (!port->port_altmode[i]) {
+   struct typec_altmode *alt;
+
+   alt = typec_port_register_altmode(port->typec_port,
+ paltmode);
+   if (IS_ERR(alt)) {
tcpm_log(port,
 "%s: failed to register port alternate 
mode 0x%x",
 dev_name(dev), paltmode->svid);
break;
}
+   port->port_altmode[i] = alt;
i++;
paltmode++;
}
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 2719f5d382f7..37a15c14a6c6 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -158,15 +158,15 @@ static int tps6598x_connect(struct tps6598x *tps, u32 
status)
desc.identity = &tps->partner_identity;
}
 
-   tps->partner = typec_register_partner(tps->port, &desc);
-   if (!tps->partner)
-   return -ENODEV;
-
typec_set_pwr_opmode(tps->port, mode);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
 
+   tps->partner = typec_register_partner(tps->port, &desc);
+   if (IS_ERR(tps->partner))
+   return PTR_ERR(tps->partner);
+
if (desc.identity)
typec_partner_set_identity(tps->partner);
 
@@ -175,7 +175,8 @@ static int tps6598x_connect(struct tps6598x *tps, u32 
status)
 
 static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
 {
-   typec_unregister_partner(tps->partner);
+   if (!IS_ERR(tps->partner))
+   typec_unregister_partner(tps->partner);
tps->partner = NULL;
typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
@@ -418,8 +419,8 @@ static int tps6598x_probe(struct i2c_client *client)
tps->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
 
tps->port = typec_register_port(&client->dev, &tps->typec_cap);
-   if (!tps->port)
-   return -ENODEV;
+   if (IS_ERR(tps->port))
+   return PTR_ERR(tps->port);
 
if (status & TPS_STATUS_PLUG_PRESENT) {
ret = tps6598x_connect(tps, status);
diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 735726ced602..dc28a

[PATCH v5 00/12] USB Type-C device-connection, mux and switch support

[Hans de Goede]

Hi All,

Here is version 5 of Heikki's and my USB Type-C device-connection, mux and
switch support series. Versions 2 - 5 bring various small code and style
fixes based on review (no major changes).

Here is the original cover-letter of v1:

Some devices with an USB Type-C connector have a bunch of muxes
behind that connector which need to be controlled by the kernel (rather
then having them controlled by firmware as on most devices).

Quite a while back I submitted a patch-series to tie together these muxes
and the Type-C Port Manager (tcpm) code, using the then new drivers/mux
framework. But the way I used the mux framework went against what it was
designed for, so in the end that series got nowhere.

Heikki Krogerus from Intel, who maintains the USB TYPEC subsystem, has
recently been working on solving the same problem for some boards he is
doing hardware-enablement for.

Heikki has come up with a number of infrastructure patches for this.
The first one is a new device-connection framework. This solves the
problem of describing non bus device-links on x86 in what in my experience
with this problematic area is a really nice simple, clean and *generic*
way. This could for example in the near future also replace the custom
lookup code in the pwm subsys and the custom pwm_add_table() /
pwm_remove_table() functions.

The other 3 patches add a framework for the different type of Type-C /
USB "muxes".

Heikki and I have gone through a number of iterations of these patches
together and we believe these are now ready for merging. Since merging
infrastructure patches without users is not done and Heikki's own use-case
for these is not yet ready for merging, the rest of this series consists
of patches by me to make the Type-C connector found on some Cherry Trail
devices (finally) be able to actually work as an USB port and not just
a charge port.

The last patch uses the new usb-role-switch framework to also do proper
devcie / host switching on CHT devices with a USB micro AB connector.
This is also a big feature for CHT users, because before this they had
to do a reboot to get an OTG-host cable recognized (on some devices).

Part of this series is an usb-role-switch driver for the role-switch
found inside the xhci controller on e.g. CHT devices, this is currently
implemented as the generic xhci controller instantiating a platform
child-device for this, since this really is a separate chunk of HW
which happens to sit in the XHCI mmio space. This approach may not be
universally liked, given that in this new series the role-switch driver
is much smaller and does not have any external deps anymore we could
just integrate it into the xhci code if that is preferred.

About merging this series (once everything is reviewed, etc.), there are
quite some interdependencies in it esp. a lot of the patches depend on
the first patch. Luckily patches 1-10 all apply to subsystems which are
maintained by Greg (most to the USB subsys). Which just leaves patches
11 and 12 once 1-10 are merged. Greg, can you create an immutable branch
for the platform/x86 and extcon maintainers to merge once this is done?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 01/12] drivers: base: Unified device connection lookup

[Hans de Goede]

From: Heikki Krogerus 

Several frameworks - clk, gpio, phy, pmw, etc. - maintain
lookup tables for describing connections and provide custom
API for handling them. This introduces a single generic
lookup table and API for the connections.

The motivation for this commit is centralizing the
connection lookup, but the goal is to ultimately extract the
connection descriptions also from firmware by using the
fwnode_graph_* functions and other mechanisms that are
available.

Signed-off-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Reviewed-by: Andy Shevchenko 
Signed-off-by: Hans de Goede 
---
Changes in v5:
-Add missing documentation for @list struct devcon member

Changes in v4:
-Add Andy's Reviewed-by

Changes in v3:
-Various spelling and gramar fixes in the docs pointed out by Randy Dunlap

Changes in v2:
-Add a (struct devcon) cast to the DEVCON() macro
---
 Documentation/driver-api/device_connection.rst |  43 
 drivers/base/Makefile  |   3 +-
 drivers/base/devcon.c  | 139 +
 include/linux/connection.h |  34 ++
 4 files changed, 218 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/driver-api/device_connection.rst
 create mode 100644 drivers/base/devcon.c
 create mode 100644 include/linux/connection.h

diff --git a/Documentation/driver-api/device_connection.rst 
b/Documentation/driver-api/device_connection.rst
new file mode 100644
index ..64a3e5e9bb7c
--- /dev/null
+++ b/Documentation/driver-api/device_connection.rst
@@ -0,0 +1,43 @@
+==
+Device connections
+==
+
+Introduction
+
+
+Devices often have connections to other devices that are outside of the direct
+child/parent relationship. A serial or network communication controller, which
+could be a PCI device, may need to be able to get a reference to its PHY
+component, which could be attached for example to the I2C bus. Some device
+drivers need to be able to control the clocks or the GPIOs for their devices,
+and so on.
+
+Device connections are generic descriptions of any type of connection between
+two separate devices.
+
+Device connections alone do not create a dependency between the two devices.
+They are only descriptions which are not tied to either of the devices 
directly.
+A dependency between the two devices exists only if one of the two endpoint
+devices requests a reference to the other. The descriptions themselves can be
+defined in firmware (not yet supported) or they can be built-in.
+
+Usage
+-
+
+Device connections should exist before device ``->probe`` callback is called 
for
+either endpoint device in the description. If the connections are defined in
+firmware, this is not a problem. It should be considered if the connection
+descriptions are "built-in", and need to be added separately.
+
+The connection description consists of the names of the two devices with the
+connection, i.e. the endpoints, and unique identifier for the connection which
+is needed if there are multiple connections between the two devices.
+
+After a description exists, the devices in it can request reference to the 
other
+endpoint device, or they can request the description itself.
+
+API
+---
+
+.. kernel-doc:: drivers/base/devcon.c
+   : functions: __device_find_connection device_find_connection 
add_device_connection remove_device_connection
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index e32a52490051..12a7f64d35a9 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -5,7 +5,8 @@ obj-y   := component.o core.o bus.o dd.o 
syscore.o \
   driver.o class.o platform.o \
   cpu.o firmware.o init.o map.o devres.o \
   attribute_container.o transport_class.o \
-  topology.o container.o property.o cacheinfo.o
+  topology.o container.o property.o cacheinfo.o \
+  devcon.o
 obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
 obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
 obj-y  += power/
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
new file mode 100644
index ..1628dcb34709
--- /dev/null
+++ b/drivers/base/devcon.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * Device connections
+ *
+ * Copyright (C) 2018 Intel Corporation
+ * Author: Heikki Krogerus 
+ */
+
+#include 
+#include 
+
+static DEFINE_MUTEX(devcon_lock);
+static LIST_HEAD(devcon_list);
+
+/**
+ * __device_find_connection - Find physical connection to a device
+ * @dev: Device with the connection
+ * @con_id: Identifier for the connection
+ * @data: Data for the match function
+ * @match: Function to check and convert the connection description
+ *
+ * Find a connection with unique identifier @con_id between @dev and another
+ * device. @match will be used to convert the connection description to dat

Re: [PATCH v4 01/12] drivers: base: Unified device connection lookup

[Hans de Goede]

Hi,

On 26-02-18 20:34, Randy Dunlap wrote:

On 02/26/2018 10:39 AM, Hans de Goede wrote:

diff --git a/include/linux/connection.h b/include/linux/connection.h
new file mode 100644
index ..0b4430eae53a
--- /dev/null
+++ b/include/linux/connection.h
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#ifndef _LINUX_CONNECTION_H_
+#define _LINUX_CONNECTION_H_
+
+#include 
+
+struct device;
+
+/**
+ * struct devcon - Device Connection Descriptor
+ * @endpoint: The names of the two devices connected together
+ * @id: Unique identifier for the connection


Hi,
  * @list: 
is missing above.


Thanks, fixed in v5, which I'm going to send out right
away since there have been no other remarks for the last 24h.

Regards,

Hans






+ */
+struct devcon {
+   const char  *endpoint[2];
+   const char  *id;
+   struct list_headlist;
+};




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] console: Expand dummy functions for CFI

[Bartlomiej Zolnierkiewicz]

On Wednesday, February 28, 2018 04:09:47 PM Bartlomiej Zolnierkiewicz wrote:
> On Monday, February 26, 2018 04:04:17 PM Kees Cook wrote:
> > This is a small series that cleans up struct consw a bit and
> > prepares it for Control Flow Integrity checking (i.e. Clang's
> > -fsanitize=cfi).
> 
> for drivers/video/ parts:
> 
> Acked-by: Bartlomiej Zolnierkiewicz 

also I'll be happy to merge all patches through fbdev tree
(if Greg is fine with it and ACKs drivers/usb/ parts)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 08/12] xhci: Add Intel extended cap / otg phy mux handling

[Heikki Krogerus]

On Wed, Feb 28, 2018 at 04:07:45PM +0100, Hans de Goede wrote:
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 96099a245c69..5917e3095e2a 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1825,6 +1825,7 @@ struct xhci_hcd {
>  /* Reserved. It was XHCI_U2_DISABLE_WAKE */
>  #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL  (1 << 28)
>  #define XHCI_HW_LPM_DISABLE  (1 << 29)
> +#define XHCI_INTEL_USB_ROLE_SW   (1 << 30)

Did you rebased these on tope of the latest usb-next? This does not
apply cleanly on top of linux-next.


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] console: Expand dummy functions for CFI

[Greg Kroah-Hartman]

On Wed, Feb 28, 2018 at 04:14:38PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday, February 28, 2018 04:09:47 PM Bartlomiej Zolnierkiewicz wrote:
> > On Monday, February 26, 2018 04:04:17 PM Kees Cook wrote:
> > > This is a small series that cleans up struct consw a bit and
> > > prepares it for Control Flow Integrity checking (i.e. Clang's
> > > -fsanitize=cfi).
> > 
> > for drivers/video/ parts:
> > 
> > Acked-by: Bartlomiej Zolnierkiewicz 
> 
> also I'll be happy to merge all patches through fbdev tree
> (if Greg is fine with it and ACKs drivers/usb/ parts)

I've already taken this through the tty tree, sorry about that.
Shouldn't cause any merge issues.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 08/12] xhci: Add Intel extended cap / otg phy mux handling

[Hans de Goede]

Hi,

On 28-02-18 16:15, Heikki Krogerus wrote:

On Wed, Feb 28, 2018 at 04:07:45PM +0100, Hans de Goede wrote:

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 96099a245c69..5917e3095e2a 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1825,6 +1825,7 @@ struct xhci_hcd {
  /* Reserved. It was XHCI_U2_DISABLE_WAKE */
  #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL   (1 << 28)
  #define XHCI_HW_LPM_DISABLE   (1 << 29)
+#define XHCI_INTEL_USB_ROLE_SW (1 << 30)


Did you rebased these on tope of the latest usb-next?


No I did not expect that to be necessary, but I see now that it is.
I've just done a rebase locally, any other remarks before I send
out a v6?


This does not
apply cleanly on top of linux-next.


Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock

[Mathias Nyman]

On 14.02.2018 18:16, Gregory CLEMENT wrote:

On Armada 7K/8K we need to explicitly enable the register clock. This
clock is optional because not all the SoCs using this IP need it but at
least for Armada 7K/8K it is actually mandatory.

The change was done at xhci-plat level and not at a xhci-mvebu.c because,
it is expected that other SoC would have this kind of constraint.

The binding documentation is updating accordingly.

Signed-off-by: Gregory CLEMENT 
---
  Documentation/devicetree/bindings/usb/usb-xhci.txt |  5 +++-
  drivers/usb/host/xhci-plat.c   | 33 ++
  drivers/usb/host/xhci.h|  3 +-
  3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index e2ea59bbca93..e4b14511f4f8 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -27,7 +27,10 @@ Required properties:
- interrupts: one XHCI interrupt should be described here.
  
  Optional properties:

-  - clocks: reference to a clock
+  - clocks: reference to the clocks
+  - clock-names: mandatory if there is a second clock, in this case
+the name must be "core" for the first clock and "reg" for the
+second one
- usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
- usb3-lpm-capable: determines if platform is USB3 LPM capable
- quirk-broken-port-ped: set if the controller has broken port disable 
mechanism


Would be good to get a Ack or review by Rob Herring for the above


diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 79afaac57ef6..fd0c399013a2 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
struct resource *res;
struct usb_hcd  *hcd;
struct clk  *clk;
+   struct clk  *reg_clk;
int ret;
int irq;
  
@@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device *pdev)

hcd->rsrc_len = resource_size(res);
  
  	/*

-* Not all platforms have a clk so it is not an error if the
-* clock does not exists.
+* Not all platforms have clks so it is not an error if the
+* clock do not exist.
 */
+   reg_clk = devm_clk_get(&pdev->dev, "reg");
+   if (!IS_ERR(reg_clk)) {
+   ret = clk_prepare_enable(reg_clk);
+   if (ret)
+   goto put_hcd;
+   } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) {
+   ret = -EPROBE_DEFER;
+   goto put_hcd;
+   }
+
clk = devm_clk_get(&pdev->dev, NULL);
if (!IS_ERR(clk)) {
ret = clk_prepare_enable(clk);
if (ret)
-   goto put_hcd;
+   goto disable_reg_clk;
} else if (PTR_ERR(clk) == -EPROBE_DEFER) {
ret = -EPROBE_DEFER;
-   goto put_hcd;
+   goto disable_reg_clk;
}
  
  	xhci = hcd_to_xhci(hcd);

@@ -252,6 +263,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
device_wakeup_enable(hcd->self.controller);
  
  	xhci->clk = clk;

+   xhci->reg_clk = reg_clk;
xhci->main_hcd = hcd;
xhci->shared_hcd = __usb_create_


hcd(driver, sysdev, &pdev->dev,

dev_name(&pdev->dev), hcd);
@@ -321,6 +333,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
  disable_clk:
clk_disable_unprepare(clk);
  
+disable_reg_clk:

+   clk_disable_unprepare(reg_clk);
+
  put_hcd:
usb_put_hcd(hcd);
  
@@ -336,6 +351,7 @@ static int xhci_plat_remove(struct platform_device *dev)

struct usb_hcd  *hcd = platform_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
+   struct clk *reg_clk = xhci->reg_clk;
  
  	xhci->xhc_state |= XHCI_STATE_REMOVING;
  
@@ -346,6 +362,7 @@ static int xhci_plat_remove(struct platform_device *dev)

usb_put_hcd(xhci->shared_hcd);
  
  	clk_disable_unprepare(clk);

+   clk_disable_unprepare(reg_clk);
usb_put_hcd(hcd);
  
  	pm_runtime_set_suspended(&dev->dev);

@@ -370,8 +387,10 @@ static int __maybe_unused xhci_plat_suspend(struct device 
*dev)
 */
ret = xhci_suspend(xhci, device_may_wakeup(dev));
  
-	if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk))

+   if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk)) {
clk_disable_unprepare(xhci->clk);
+   clk_disable_unprepare(xhci->reg_clk);
+   }
  
  	return ret;

  }
@@ -382,8 +401,10 @@ static int __maybe_unused xhci_plat_resume(struct device 
*dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
int ret;
  
-	if (!device_may_wakeup(dev) &

Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore

[Kai Heng Feng]

On 28 Feb 2018, at 10:47 PM, Matthew Wilcox  wrote:

On Mon, Feb 26, 2018 at 11:04:57PM +0800, Kai-Heng Feng wrote:

+static char quirks_param[128];
+module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
+MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying  
quirks=vendorID:productID:quirks");

+
+static char quirks_param_orig[128];



+static u32 usb_detect_dynamic_quirks(struct usb_device *udev)
+{
+   u16 vid = le16_to_cpu(udev->descriptor.idVendor);
+   u16 pid = le16_to_cpu(udev->descriptor.idProduct);
+   struct quirk_entry *quirk;
+
+   mutex_lock(&quirk_mutex);
+   if (strcmp(quirks_param, quirks_param_orig) != 0) {
+   strcpy(quirks_param_orig, quirks_param);


What happens if the user is writing to quirks_param at the same time
that you memcpy it?

I think you're going about this wrong by trying to use the
module_param_string machinery.  You should be using module_param_cb()
to build the quirks list when the user writes it (and then translate
back into a string when the user wants to read from it.


Thanks! module_param_cb() is exactly what I want.
I’ll use it in next version.


Also, you won't need to use a linked list for this; you can just allocate
an array of quirks.



I use linked list because the total quirks number is known after the entire  
string gets parsed.
Do you suggest that I should just alloc a predefined number (like 16) quirk  
entries, instead of doing it dynamically?


Kai-Heng
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock

[Manu Gautam]

Hi,


On 2/14/2018 9:46 PM, Gregory CLEMENT wrote:
> On Armada 7K/8K we need to explicitly enable the register clock. This
> clock is optional because not all the SoCs using this IP need it but at
> least for Armada 7K/8K it is actually mandatory.
>
> The change was done at xhci-plat level and not at a xhci-mvebu.c because,
> it is expected that other SoC would have this kind of constraint.
>
> The binding documentation is updating accordingly.
>
> Signed-off-by: Gregory CLEMENT 
> ---
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |  5 +++-
>  drivers/usb/host/xhci-plat.c   | 33 
> ++
>  drivers/usb/host/xhci.h|  3 +-
>  3 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
> b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index e2ea59bbca93..e4b14511f4f8 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -27,7 +27,10 @@ Required properties:
>- interrupts: one XHCI interrupt should be described here.
>  
>  Optional properties:
> -  - clocks: reference to a clock
> +  - clocks: reference to the clocks
> +  - clock-names: mandatory if there is a second clock, in this case
> +the name must be "core" for the first clock and "reg" for the
> +second one
>- usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
>- usb3-lpm-capable: determines if platform is USB3 LPM capable
>- quirk-broken-port-ped: set if the controller has broken port disable 
> mechanism
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 79afaac57ef6..fd0c399013a2 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   struct resource *res;
>   struct usb_hcd  *hcd;
>   struct clk  *clk;
> + struct clk  *reg_clk;
>   int ret;
>   int irq;
>  
> @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   hcd->rsrc_len = resource_size(res);
>  
>   /*
> -  * Not all platforms have a clk so it is not an error if the
> -  * clock does not exists.
> +  * Not all platforms have clks so it is not an error if the
> +  * clock do not exist.
>*/
> + reg_clk = devm_clk_get(&pdev->dev, "reg");
> + if (!IS_ERR(reg_clk)) {
> + ret = clk_prepare_enable(reg_clk);
> + if (ret)
> + goto put_hcd;
> + } else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto put_hcd;
> + }
> +

How about using clk_bulk_ APIs?

>   clk = devm_clk_get(&pdev->dev, NULL);
>   if (!IS_ERR(clk)) {
>   ret = clk_prepare_enable(clk);
>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] usb: host: xhci-plat: Fix clock resource by adding a register clock

[Gregory CLEMENT]

Hi Manu,
 
 On mer., févr. 28 2018, Manu Gautam  wrote:

> Hi,
>
>
> On 2/14/2018 9:46 PM, Gregory CLEMENT wrote:
>> On Armada 7K/8K we need to explicitly enable the register clock. This
>> clock is optional because not all the SoCs using this IP need it but at
>> least for Armada 7K/8K it is actually mandatory.
>>
>> The change was done at xhci-plat level and not at a xhci-mvebu.c because,
>> it is expected that other SoC would have this kind of constraint.
>>
>> The binding documentation is updating accordingly.
>>
>> Signed-off-by: Gregory CLEMENT 
>> ---
>>  Documentation/devicetree/bindings/usb/usb-xhci.txt |  5 +++-
>>  drivers/usb/host/xhci-plat.c   | 33 
>> ++
>>  drivers/usb/host/xhci.h|  3 +-
>>  3 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
>> b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> index e2ea59bbca93..e4b14511f4f8 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> @@ -27,7 +27,10 @@ Required properties:
>>- interrupts: one XHCI interrupt should be described here.
>>  
>>  Optional properties:
>> -  - clocks: reference to a clock
>> +  - clocks: reference to the clocks
>> +  - clock-names: mandatory if there is a second clock, in this case
>> +the name must be "core" for the first clock and "reg" for the
>> +second one
>>- usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
>>- usb3-lpm-capable: determines if platform is USB3 LPM capable
>>- quirk-broken-port-ped: set if the controller has broken port disable 
>> mechanism
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 79afaac57ef6..fd0c399013a2 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -157,6 +157,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  struct resource *res;
>>  struct usb_hcd  *hcd;
>>  struct clk  *clk;
>> +struct clk  *reg_clk;
>>  int ret;
>>  int irq;
>>  
>> @@ -226,17 +227,27 @@ static int xhci_plat_probe(struct platform_device 
>> *pdev)
>>  hcd->rsrc_len = resource_size(res);
>>  
>>  /*
>> - * Not all platforms have a clk so it is not an error if the
>> - * clock does not exists.
>> + * Not all platforms have clks so it is not an error if the
>> + * clock do not exist.
>>   */
>> +reg_clk = devm_clk_get(&pdev->dev, "reg");
>> +if (!IS_ERR(reg_clk)) {
>> +ret = clk_prepare_enable(reg_clk);
>> +if (ret)
>> +goto put_hcd;
>> +} else if (PTR_ERR(reg_clk) == -EPROBE_DEFER) {
>> +ret = -EPROBE_DEFER;
>> +goto put_hcd;
>> +}
>> +
>
> How about using clk_bulk_ APIs?

I didn't know this API, but after having a look on it, it didn't match
what I need. Indeed the second clock is "optional" to handle the
backward compatibility. With the clk_bulk_ APIS all the clocks are
mandatory.

Thanks,

Gregory

>
>>  clk = devm_clk_get(&pdev->dev, NULL);
>>  if (!IS_ERR(clk)) {
>>  ret = clk_prepare_enable(clk);
>>
>
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore

[Matthew Wilcox]

On Thu, Mar 01, 2018 at 12:01:40AM +0800, Kai Heng Feng wrote:
> > Also, you won't need to use a linked list for this; you can just allocate
> > an array of quirks.
> 
> I use linked list because the total quirks number is known after the entire
> string gets parsed.
> Do you suggest that I should just alloc a predefined number (like 16) quirk
> entries, instead of doing it dynamically?

I'd just do:

unsigned int nr = 1;
for (i = 0; i < len; i++)
if (str[i] == ',')
nr++;
kcalloc(nr, sizeof(struct), GFP_KERNEL);

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Continuous USB resets on NXP i.MX 6ULL device

[Stefan Wahren]

Hi,

i'm not sure this is related, but i noticed something ugly in
driver/usb/chipidea/usbmisc_imx.c.

The compatible "fsl,imx6ul-usbmisc" uses imx6sx_usbmisc_ops, which uses
usbmisc_imx6sx_init (which also calls usbmisc_imx6q_init).
Within usbmisc_imx6q_init the flag MX6_BM_NON_BURST_SETTING is enabled.

But according to the i.MX6 ULL reference manual this bit is marked as
reserved and set to zero at reset.

So i think it would be better to have separate imx6ul_usbmisc_ops.

Regards
Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Wakeup from USB on i.MX6S

[Ralf.4MailingLists]

Hi,

On 28 February 2018 4:43 AM, Peter Chen  wrote:
> You mean you measure it after echo "mem" > /sys/power/state, right?

Yes, I've put my multimeter to some grounded part of the board and the other 
measuring head(or is it called test prod in English?) to the right pin of the 
usb port (if watched from above).
It stayed on after echo mem > /sys/power/state.



> You mean disabled "1-1" wakeup entry, but other wakeup entries are
> enabled, still wakes up system for "mem"?

Yes.


 
> Would you please try below changes:
> 1. By connecting external HUB at USBOTG0, and only enable its wakeup
> to see if the abnormal wakeup occurs?
> 2184200.usb -> 2184000.usb
> ci\_hdrc.1 ->ci\_hdrc.0
> 
> 2. Unbind wired USB HUB at host port to see if the abnormal wakeup
> occurs?
> echo -n "1-1:1.0" > /sys/bus/usb/drivers/hub/unbind

I'm afraid I'm not able to do this at the moment. I only have a HUB with a 
Mini-USB-Input and a cable Mini-USB-B to (usual) USB-A. The connector on the 
board also is a Mini-A/B-Port, so i would need a 
Mini-USB-(AorB)--to--Mini-USB-B--cable. Maybe with OTG capability and an 
USB-HUB with OTG capability.
If necessary, I could try to order one or solder one by 2 old Mini-USB-cables.



> 3. Search schematic to see if Power of USB HUB is controlled by
> software, and be off at mem-sleep.

I don't have a schematic drawing of the board, but I have the manuals of cpu- 
and carrier-board.
I could ask our supplier for that drawing if necessary.

But maybe these informations might help:
"The VBUS supplies of Host interfaces at the USB connectors are controlled by 
the common enable signal USBH_PEN# of the emCON interface. Also a common over 
current flag USBH_OC# is fed back." [2]
It looks like there are 4 ports connected to the Hub (2xUSB2.0, 1xUSB3.0, 
1xminiPCIe(There seem to be additional USB Pins on the miniPCIe-Socket))
and 1 OTG Port that has nothing to do with the hub.


About the 2.0 Ports on the Hub: "The VBUS outputs of both ports are driven by a 
dual channel power switch which is controlled by the signal USBH_PEN# of the 
emCON interface." [2]

About the 3.0 Ports on the Hub: "The USB 3.0 interface is directly driven by 
the emCON connector."; "The USB 2.0 interface is driven by the USB Hub of the 
Avari."; "The VBUS output of this connector is controlled by an own USB power 
switch which can source 2 A. The power switch is controlled by the signal 
USBH_PEN# of the emCON interface." [2]

About the OTG Port: "The interface signals are directly connected to the emCON 
connector. Therefore all characteristics of the interface depend on the used 
CPU module.
The VBUS power switch is controlled by the signal USBOTG_PEN# of the CPU module 
and by the ID pin of the USB connector. Only if the ID pin is driven low by a 
connected cable the VBUS supply can be driven by the CPU module. The ID pin 
also controls the signal USBOTG_VBUS at the emCON connector. If the ID pin is 
left open the VBUS switch of the Avari is disabled and the level of the VBUS 
signal at pin 1 of the USB connector is driven to the signal USBOTG_VBUS. Thus 
plugging of a USB Host can is detected while the interface is operated in 
Device mode." [2]


While the cpu board manual says about the "USB Host": "To switch the bus power 
the control line USBH_PEN# is connected to the emCON connector. A logical “0” 
at the processor GPIO3-31 switches the power on; a logical “1” turns the power 
off."; "The USBH_VBUS signal on the emCON connector is only an input to detect 
the VBUS voltage on the baseboard." [3]

And about the "USB OTG": "To switch the bus power in USB Host mode the control 
line USBOTG_PEN# is connected to the emCON connector. A logical “0” at the 
processor GPIO1-8 switches the power on; a logical “1” turns the power off."; 
"The USBOTG_VBUS signal on the emCON connector is only an input to detect the 
VBUS voltage on the baseboard." [3]

To clearify some names:
- emCON connector := large connector between cpu board and carrier board
- Avari := carrier board

It looks like you can turn off the hub by GPIO3-31 and the power of the OTG 
Port by GPIO1-8, so I think it is really controlled by software.
If you want to take a look on those manuals by your own, all I've quoted is in 
[2] chapter 7 and [3] chapters 4.5 and 4.6.



> To see if function imx6q_finalize_wakeup_setting is existed at
> drivers/usb/chipidea/usbmisc_imx.c, if existed, it the kernel from
> fsl/nxp.

There ale only these functions in usbmisc_imx.c that contain wakeup in their 
name:
- static int usbmisc_imx6q_set_wakeup
- static int usbmisc_imx7d_set_wakeup
- int imx_usbmisc_set_wakeup


Best regards,
Ralf


[1] i.MX 6Solo/6DualLite Applications Processor Reference Manual, Rev. 3, 
09/2017 (IMX6SDLRM.pdf) (https://imxdev.gitlab.io/imxdoc/reference-manual-imx6/)
[2] emCON_Avari, emCON compliant baseboard, Hardware Description, Rev3 / 
25.10.2016 (emCON_Avari_HW_v003en.pdf) 
(https://www.emtrion.de/de/details_products-accessoires/emcon-mx6-57.html)
[3

Re: [PATCH 3/7] usb: dwc3: pci: Store device properties dynamically

[John Youn]

On 02/22/2018 07:20 AM, Andy Shevchenko wrote:

On Thu, Feb 22, 2018 at 1:57 AM, Thinh Nguyen  wrote:

On 2/21/2018 6:46 AM, Andy Shevchenko wrote:

On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen
 wrote:

On 2/17/2018 7:29 AM, Andy Shevchenko wrote:

On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen
 wrote:

Add the ability to add device properties dynamically. Currently, device
properties are added to platform device using
platform_device_add_properties(). However, this function does not allow
adding properties incrementally. It is useful to have this ability when
the driver needs to set common device properties across different HW


I'm not sure it's useful anyhow.


or
if IP and FPGA validation test different configurations for different
HW.


Shouldn't be a separate stuff for FPGA exclusively?


Can you clarify/expand your question?


FPGA is the one which might have different properties at run time for
the same device.
So, why do we care on driver / generic level of it?

Shouldn't be FPGA manager take care of it (via DT overlays, for example)?


Normally it is handled using DT overlays. But for using FPGA as PCI
device, I'm not aware of a better solution.


If it's a PCI device, it may utilize PCI (hot plug if you would like
to change IP at run time) with appropriate IDs and stuff, right?


Introduce two new functions to do so:
* dwc3_pci_add_one_property() - this function adds one property to
  dwc->properties array and increase its size as needed
* dwc3_pci_add_properties() - this function takes a null terminated
  array of device properties and add them to dwc->properties


So, why you can't use ACPI / DT here?



dwc3_pci_add_properties() is a convenient function that takes statically
allocated array of (quirks) properties for different HW and store them
to dwc->properties. The idea is to add more properties on top of these
required quirks.


Yes, I understand that. What's wrong with DT? The built-in device
properties have the same nature as usual properties for DT.
Whenever driver calls device_property_read_uXX() or alike it would
check all property provides for asked one.


I may not have explained fully in my commit message the purpose of this
change. That's why I think I misinterpreted your previous question.



With this new debugging feature, we want to delay adding device
properties until the user initiates it.


So, see above.
When user wants to enable this IP at run time it will be a PCI hot
plug event which makes device appear behind PCI switch.
When device appears it would have it's own VndrID/DevID + sub pair.

Based on that IDs you may apply hard coded quirks (though I am against
quirks in new hardware) as it's done right now.



Hi Andy,

Using VID/PID is not really feasible for our use case. If we only had
a few concrete devices then it would be ok.

But understand that this an IP running on an FPGA validation
platform. The IP is very large and flexible with *many* parameters
that we must test against.  It is also deployed by our customers with
widely varying configurations. So we are constantly testing these as
well.

One of the last pieces for moving our FPGA validation completely to
the in-kernel Linux stack is the ability to configure the driver to
set parameters that are not visible to the driver, or with parameters
that we want to constrain for testing.

For example, consider the maximum_speed property. We could create a
PID for each speed. But what if we want test a configuration with
maximum_speed in combination with some other parameters, say nump,
burst size, fifo size, fifo thresholding, etc.

The amount of PIDs we would need for our validation would get out of
hand quickly and would be a pain to maintain. And it wouldn't help for
testing with fine variations, like increasing the thresholding value
by 1.

The other part of this is that we don't necessarily need an entirely
new device to perform all testing with different parameters. So we
wouldn't even have an opportunity to change the VID/PID.

Regards,
John

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Continuous USB resets on NXP i.MX 6ULL device

[Fabio Estevam]

Hi Stefan,

On Wed, Feb 28, 2018 at 2:08 PM, Stefan Wahren  wrote:
> Hi,
>
> i'm not sure this is related, but i noticed something ugly in
> driver/usb/chipidea/usbmisc_imx.c.
>
> The compatible "fsl,imx6ul-usbmisc" uses imx6sx_usbmisc_ops, which uses
> usbmisc_imx6sx_init (which also calls usbmisc_imx6q_init).
> Within usbmisc_imx6q_init the flag MX6_BM_NON_BURST_SETTING is enabled.
>
> But according to the i.MX6 ULL reference manual this bit is marked as
> reserved and set to zero at reset.
>
> So i think it would be better to have separate imx6ul_usbmisc_ops.

Yes, good catch.

Peter, do you agree with this proposed change?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] net: usb: asix88179_178a: de-duplicate code

[Alexander Kurz]

Remove the duplicated code for asix88179_178a bind and reset methods.

Signed-off-by: Alexander Kurz 
---
 drivers/net/usb/ax88179_178a.c | 117 +++--
 1 file changed, 31 insertions(+), 86 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index a6ef75907ae9..b1d8c2043d17 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1223,7 +1223,7 @@ static int ax88179_led_setting(struct usbnet *dev)
return 0;
 }
 
-static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
+static int ax88179_link_bind_or_reset(struct usbnet *dev, int do_reset)
 {
u8 buf[5];
u16 *tmp16;
@@ -1231,12 +1231,11 @@ static int ax88179_bind(struct usbnet *dev, struct 
usb_interface *intf)
struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
struct ethtool_eee eee_data;
 
-   usbnet_get_endpoints(dev, intf);
-
tmp16 = (u16 *)buf;
tmp = (u8 *)buf;
 
-   memset(ax179_data, 0, sizeof(*ax179_data));
+   if (!do_reset)
+   memset(ax179_data, 0, sizeof(*ax179_data));
 
/* Power up ethernet PHY */
*tmp16 = 0;
@@ -1249,9 +1248,13 @@ static int ax88179_bind(struct usbnet *dev, struct 
usb_interface *intf)
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_CLK_SELECT, 1, 1, tmp);
msleep(100);
 
+   if (do_reset)
+   ax88179_auto_detach(dev, 0);
+
ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
 ETH_ALEN, dev->net->dev_addr);
-   memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
+   if (!do_reset)
+   memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
 
/* RX bulk configuration */
memcpy(tmp, &AX88179_BULKIN_SIZE[0], 5);
@@ -1266,19 +1269,21 @@ static int ax88179_bind(struct usbnet *dev, struct 
usb_interface *intf)
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PAUSE_WATERLVL_HIGH,
  1, 1, tmp);
 
-   dev->net->netdev_ops = &ax88179_netdev_ops;
-   dev->net->ethtool_ops = &ax88179_ethtool_ops;
-   dev->net->needed_headroom = 8;
-   dev->net->max_mtu = 4088;
-
-   /* Initialize MII structure */
-   dev->mii.dev = dev->net;
-   dev->mii.mdio_read = ax88179_mdio_read;
-   dev->mii.mdio_write = ax88179_mdio_write;
-   dev->mii.phy_id_mask = 0xff;
-   dev->mii.reg_num_mask = 0xff;
-   dev->mii.phy_id = 0x03;
-   dev->mii.supports_gmii = 1;
+   if (!do_reset) {
+   dev->net->netdev_ops = &ax88179_netdev_ops;
+   dev->net->ethtool_ops = &ax88179_ethtool_ops;
+   dev->net->needed_headroom = 8;
+   dev->net->max_mtu = 4088;
+
+   /* Initialize MII structure */
+   dev->mii.dev = dev->net;
+   dev->mii.mdio_read = ax88179_mdio_read;
+   dev->mii.mdio_write = ax88179_mdio_write;
+   dev->mii.phy_id_mask = 0xff;
+   dev->mii.reg_num_mask = 0xff;
+   dev->mii.phy_id = 0x03;
+   dev->mii.supports_gmii = 1;
+   }
 
dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
  NETIF_F_RXCSUM;
@@ -1330,6 +1335,13 @@ static int ax88179_bind(struct usbnet *dev, struct 
usb_interface *intf)
return 0;
 }
 
+static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+   usbnet_get_endpoints(dev, intf);
+
+   return ax88179_link_bind_or_reset(dev, 0);
+}
+
 static void ax88179_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
u16 tmp16;
@@ -1458,74 +1470,7 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff 
*skb, gfp_t flags)
 
 static int ax88179_link_reset(struct usbnet *dev)
 {
-   struct ax88179_data *ax179_data = (struct ax88179_data *)dev->data;
-   u8 tmp[5], link_sts;
-   u16 mode, tmp16, delay = HZ / 10;
-   u32 tmp32 = 0x4000;
-   unsigned long jtimeout;
-
-   jtimeout = jiffies + delay;
-   while (tmp32 & 0x4000) {
-   mode = 0;
-   ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, &mode);
-   ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2,
- &ax179_data->rxctl);
-
-   /*link up, check the usb device control TX FIFO full or empty*/
-   ax88179_read_cmd(dev, 0x81, 0x8c, 0, 4, &tmp32);
-
-   if (time_after(jiffies, jtimeout))
-   return 0;
-   }
-
-   mode = AX_MEDIUM_RECEIVE_EN | AX_MEDIUM_TXFLOW_CTRLEN |
-  AX_MEDIUM_RXFLOW_CTRLEN;
-
-   ax88179_read_cmd(dev, AX_ACCESS_MAC, PHYSICAL_LINK_STATUS,
-1, 1, &link_sts);
-
-   ax88179_read_cmd(dev, AX_ACCESS_PHY, AX88179_PHY_ID,
-GMII_PHY_PHYSR, 2, &tmp16);
-
-   if (!(tmp16 & GMII_PHY_PHYSR_LINK)) {
-   ret

[PATCH 1/2] net: usb: asix88179_178a: set permanent address once only

[Alexander Kurz]

The permanent address of asix88179_178a devices is read at probe time
and should not be overwritten later. Otherwise it may be overwritten
unintentionally with a configured address.

Signed-off-by: Alexander Kurz 
---
 drivers/net/usb/ax88179_178a.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index f32261ecd215..a6ef75907ae9 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1556,7 +1556,6 @@ static int ax88179_reset(struct usbnet *dev)
 
ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, ETH_ALEN,
 dev->net->dev_addr);
-   memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
 
/* RX bulk configuration */
memcpy(tmp, &AX88179_BULKIN_SIZE[0], 5);
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] usb: musb: call pm_runtime_{get,put}_sync before reading vbus registers

[Tony Lindgren]

* Merlijn Wajer  [180227 22:29]:
> Without pm_runtime_{get,put}_sync calls in place, reading
> vbus status via /sys causes the following error:
> 
> Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa0ab060
> pgd = b333e822
> [fa0ab060] *pgd=48011452(bad)
> 
> [] (musb_default_readb) from [] (musb_vbus_show+0x58/0xe4)
> [] (musb_vbus_show) from [] (dev_attr_show+0x20/0x44)
> [] (dev_attr_show) from [] (sysfs_kf_seq_show+0x80/0xdc)
> [] (sysfs_kf_seq_show) from [] (seq_read+0x250/0x448)
> [] (seq_read) from [] (__vfs_read+0x1c/0x118)
> [] (__vfs_read) from [] (vfs_read+0x90/0x144)
> [] (vfs_read) from [] (SyS_read+0x3c/0x74)
> [] (SyS_read) from [] (ret_fast_syscall+0x0/0x54)
> 
> Solution was suggested by Tony Lindgren .
> 
> Signed-off-by: Merlijn Wajer 

Thanks for fixing this. Assuming it passes Bin's tests:

Acked-by: Tony Lindgren 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore

[kbuild test robot]

Hi Kai-Heng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.16-rc3 next-20180228]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kai-Heng-Feng/usb-core-Add-quirks-parameter-for-usbcore/20180227-211635
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: tile-tilegx_defconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile 

All errors (new ones prefixed by >>):

>> drivers/usb/core/quirks.c:15:43: error: expected ')' before 'sizeof'
module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
  ^~
   drivers/usb/core/quirks.c:16:26: error: expected ')' before string constant
MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying 
quirks=vendorID:productID:quirks");
 
^~

vim +15 drivers/usb/core/quirks.c

13  
14  static char quirks_param[128];
  > 15  module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
16  MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying 
quirks=vendorID:productID:quirks");
17  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v4] usb: core: Add "quirks" parameter for usbcore

[kbuild test robot]

Hi Kai-Heng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.16-rc3 next-20180228]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Kai-Heng-Feng/usb-core-Add-quirks-parameter-for-usbcore/20180227-211635
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: mips-sb1250_swarm_defconfig (attached as .config)
compiler: mips64-linux-gnuabi64-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   drivers/usb/core/quirks.c:15:43: error: expected ')' before 'sizeof'
module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
  ^~
>> drivers/usb/core/quirks.c:16:26: error: expected ')' before string constant
MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying 
quirks=vendorID:productID:quirks");
 
^~

vim +16 drivers/usb/core/quirks.c

13  
14  static char quirks_param[128];
  > 15  module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
  > 16  MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying 
quirks=vendorID:productID:quirks");
17  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

RE: [PATCH v5 01/12] drivers: base: Unified device connection lookup

[Jun Li]

Hi,
> -Original Message-

> +struct device *device_find_connection(struct device *dev, const char
> +*con_id) {
> + return __device_find_connection(dev, con_id, generic_match, NULL); }

-   return __device_find_connection(dev, con_id, generic_match, NULL);
+   return __device_find_connection(dev, con_id, NULL, generic_match);

Jun Li
> +EXPORT_SYMBOL_GPL(device_find_connection);

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Continuous USB resets on NXP i.MX 6ULL device

[Peter Chen]

On Wed, Feb 28, 2018 at 06:08:20PM +0100, Stefan Wahren wrote:
> Hi,
> 
> i'm not sure this is related, but i noticed something ugly in
> driver/usb/chipidea/usbmisc_imx.c.
> 
> The compatible "fsl,imx6ul-usbmisc" uses imx6sx_usbmisc_ops, which uses
> usbmisc_imx6sx_init (which also calls usbmisc_imx6q_init).
> Within usbmisc_imx6q_init the flag MX6_BM_NON_BURST_SETTING is enabled.
> 
> But according to the i.MX6 ULL reference manual this bit is marked as
> reserved and set to zero at reset.
> 

For non-core registers, i.mx6ull can use i.mx6q as its base setting, the
bits you mentioned may also be reserved at imx6qdl RM.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

[Tony Lindgren]

* Rob Herring  [180219 20:41]:
> On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote:
> > The GPIOs controlling MDM6600 are used to power MDM660 on and off, to
> 
> MDM660 a typo?

Thanks fixing.

> > +Required properties:
> > +- compatible   Must be "motorola,mapphone-mdm6600"
> > +- enable-gpios GPIO to enable the USB PHY
> > +- power-gpios  GPIO to power on the device
> > +- reset-gpios  GPIO to reset the device
> 
> The are pretty standard, but...
> 
> > +- mode-gpios   Two GPIOs to configure MDM6600 USB start-up mode for
> > +   normal mode versus USB flashing mode
> > +- status-gpios Three GPIOs to read the power state of the MDM6600
> > +- cmd-gpiosThree GPIOs to control the power state of the MDM6600
> 
> These 3 should have vendor a prefix.

OK

> > +
> > +Example:
> > +
> > +fsusb1_phy: fsusb1_phy {
> 
> usb-phy {

Thanks will send out an updated version shortly.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

[Tony Lindgren]

* kbuild test robot  [180219 07:47]:
>  > 354if (reset_gpio >= 0)
>355gpiod_set_value_cansleep(reset_gpio, 1);

Thanks this test can be just dropped. Will send out an updated
version shortly.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] staging: typec: handle vendor defined part and modify drp toggling flow

[ShuFanLee]

From: ShuFanLee 

Handle vendor defined behavior in tcpci_init, tcpci_set_vconn and export 
tcpci_irq.
More operations can be extended in tcpci_data if needed.
According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
TCPC shall not start DRP toggling until subsequently the TCPM
writes to the COMMAND register to start DRP toggling.
DRP toggling flow is chagned as following:
  - Write DRP = 0 & Rd/Rd
  - Write DRP = 1
  - Set LOOK4CONNECTION command

Signed-off-by: ShuFanLee 
---
 drivers/staging/typec/tcpci.c | 127 +-
 drivers/staging/typec/tcpci.h |  13 +
 2 files changed, 113 insertions(+), 27 deletions(-)

 patch changelogs between v1 & v2
 - Remove unnecessary i2c_client in the structure of tcpci
 - Rename structure of tcpci_vendor_data to tcpci_data
 - Not exporting tcpci read/write wrappers but register i2c regmap in glue 
driver
 - Add set_vconn ops in tcpci_data
   (It is necessary for RT1711H to enable/disable idle mode before 
disabling/enabling vconn)
 - Export tcpci_irq so that vendor can call it in their own IRQ handler

 patch changelogs between v2 & v3
 - Change the return type of tcpci_irq from int to irqreturn_t

 patch changelogs between v3 & v4
 - Directly return regmap_write at the end of drp_toggling function
 - Because tcpci_irq is called in _tcpci_irq, move _tcpci_irq to the place 
after tcpci_irq

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 9bd4412..5bafcc4 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -21,7 +21,6 @@
 
 struct tcpci {
struct device *dev;
-   struct i2c_client *client;
 
struct tcpm_port *port;
 
@@ -30,6 +29,12 @@ struct tcpci {
bool controls_vbus;
 
struct tcpc_dev tcpc;
+   struct tcpci_data *data;
+};
+
+struct tcpci_chip {
+   struct tcpci *tcpci;
+   struct tcpci_data data;
 };
 
 static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
@@ -37,8 +42,7 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev 
*tcpc)
return container_of(tcpc, struct tcpci, tcpc);
 }
 
-static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
-   u16 *val)
+static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
 {
return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
 }
@@ -98,8 +102,10 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum 
typec_cc_status cc)
 static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
enum typec_cc_status cc)
 {
+   int ret;
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
-   unsigned int reg = TCPC_ROLE_CTRL_DRP;
+   unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
+  (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
 
switch (cc) {
default:
@@ -117,7 +123,16 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
break;
}
 
-   return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+   usleep_range(500, 1000);
+   reg |= TCPC_ROLE_CTRL_DRP;
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+   return regmap_write(tcpci->regmap, TCPC_COMMAND,
+   TCPC_CMD_LOOK4CONNECTION);
 }
 
 static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
@@ -178,6 +193,16 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool 
enable)
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
int ret;
 
+   /* Handle vendor set vconn */
+   if (tcpci->data) {
+   if (tcpci->data->set_vconn) {
+   ret = tcpci->data->set_vconn(tcpci, tcpci->data,
+enable);
+   if (ret < 0)
+   return ret;
+   }
+   }
+
ret = regmap_write(tcpci->regmap, TCPC_POWER_CTRL,
   enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
if (ret < 0)
@@ -323,6 +348,15 @@ static int tcpci_init(struct tcpc_dev *tcpc)
if (time_after(jiffies, timeout))
return -ETIMEDOUT;
 
+   /* Handle vendor init */
+   if (tcpci->data) {
+   if (tcpci->data->init) {
+   ret = tcpci->data->init(tcpci, tcpci->data);
+   if (ret < 0)
+   return ret;
+   }
+   }
+
/* Clear all events */
ret = tcpci_write16(tcpci, TCPC_ALERT, 0x);
if (ret < 0)
@@ -344,9 +378,8 @@ static int tcpci_init(struct tcpc_dev *tcpc)
return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
 }
 
-static irqreturn_t tcpci_irq(int irq, void *dev_id)
+irqreturn_t tcpci_irq(struct tcpci *tcpci)
 {

[PATCHv2] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

[Tony Lindgren]

Let's add support for the GPIO controlled USB PHY on the MDM6600 modem.
It is used on some Motorola Mapphone series of phones and tablets such
as Droid 4.

The MDM6600 is hardwired to the first OHCI port in the Droid 4 case, and
is controlled by several GPIOs. The USB PHY is integrated into the MDM6600
device it seems. We know this as we get L3 errors from omap-usb-host if
trying to use the PHY before MDM6600 is configured.

The GPIOs controlling MDM6600 are used to power device on and off, to
configure the USB start-up mode (normal mode versus USB flashing), and
they also tell the state of the MDM6600 device.

The two start-up mode GPIOs are dual-purposed and used for out of band
(OOB) wake-up for USB and TS 27.010 serial mux. But we need to configure
the USB start-up mode first to get MDM6600 booted in the right mode to
be usable in the first place.

Note that the Motorola Mapphone Linux kernel tree has a "radio-ctrl"
driver for modems. But it really does not control the radio at all, it
just controls the modem power and start-up mode for USB. So I came to
the conclusion that we're better off having this done in the USB PHY
driver. For adding support for USB flashing mode, we can later on add
a kernel module option for flash_mode=1 or something similar.

Also note that currently there is no PM runtime support for the OHCI
on omap variant SoCs. So for low(er) power idle states, currenty both
ohci-platform and phy-mapphone-mdm6600 must be unloaded or unbound.

For reference here is what I measured for total power consumption on
an idle Droid 4 with and without USB related MDM6600 modules:

idle lcd offphy-mapphone-mdm6600ohci-platform
153mW   284mW   344mW

So it seems that MDM6600 is currently not yet idling even with it's
radio turned off, but that's something that is beyond the control of
this USB PHY driver. This patch does get us to the point where modem
data and GPS are usable with libqmi and ModemManager for example.
Voice calls need more audio driver work.

Cc: devicet...@vger.kernel.org
Cc: Mark Rutland 
Cc: Marcel Partap 
Cc: Michael Scott 
Cc: Rob Herring 
Cc: Sebastian Reichel 
Signed-off-by: Tony Lindgren 
---

Changes since v1:
- Fixed up issues noticed by Rob and Sebastian
- Implemented wake irq handler (for debug use only for now)
- Improved error handling based on more testing

---
 .../bindings/phy/phy-mapphone-mdm6600.txt  |  30 ++
 drivers/phy/motorola/Kconfig   |   9 +
 drivers/phy/motorola/Makefile  |   1 +
 drivers/phy/motorola/phy-mapphone-mdm6600.c| 556 +
 4 files changed, 596 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
 create mode 100644 drivers/phy/motorola/phy-mapphone-mdm6600.c

diff --git a/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt 
b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
@@ -0,0 +1,30 @@
+Device tree binding documentation for Motorola Mapphone MDM6600 USB PHY
+
+Required properties:
+- compatible   Must be "motorola,mapphone-mdm6600"
+- enable-gpios GPIO to enable the USB PHY
+- power-gpios  GPIO to power on the device
+- reset-gpios  GPIO to reset the device
+- motorola,mode-gpios  Two GPIOs to configure MDM6600 USB start-up mode for
+   normal mode versus USB flashing mode
+- motorola,cmd-gpios   Three GPIOs to control the power state of the MDM6600
+- motorola,status-gpiosThree GPIOs to read the power state of the 
MDM6600
+
+Example:
+
+usb-phy {
+   compatible = "motorola,mapphone-mdm6600";
+   enable-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
+   power-gpios = <&gpio2 22 GPIO_ACTIVE_HIGH>;
+   reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>;
+   motorola,mode-gpios = <&gpio5 20 GPIO_ACTIVE_HIGH>,
+ <&gpio5 21 GPIO_ACTIVE_HIGH>;
+   motorola,cmd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>,
+<&gpio4 8 GPIO_ACTIVE_HIGH>,
+<&gpio5 14 GPIO_ACTIVE_HIGH>;
+   motorola,status-gpios = <&gpio2 20 GPIO_ACTIVE_HIGH>,
+   <&gpio2 21 GPIO_ACTIVE_HIGH>,
+   <&gpio2 23 GPIO_ACTIVE_HIGH>;
+   #phy-cells = <0>;
+};
+
diff --git a/drivers/phy/motorola/Kconfig b/drivers/phy/motorola/Kconfig
--- a/drivers/phy/motorola/Kconfig
+++ b/drivers/phy/motorola/Kconfig
@@ -10,3 +10,12 @@ config PHY_CPCAP_USB
help
  Enable this for USB to work on Motorola phones and tablets
  such as Droid 4.
+
+config PHY_MAPPHONE_MDM6600
+   tristate "Motorola Mapphone MDM6600 modem USB PHY driver"
+   depends on OF && USB_SUPPORT
+   select GENERIC_PHY
+   select USB_PHY
+   help
+ Enable this for MDM6600 USB modem to work on Motorola phones
+

[PATCH v5] staging: typec: handle vendor defined part and modify drp toggling flow

[ShuFan Lee]

From: ShuFan Lee 

Handle vendor defined behavior in tcpci_init, tcpci_set_vconn and export 
tcpci_irq.
More operations can be extended in tcpci_data if needed.
According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
TCPC shall not start DRP toggling until subsequently the TCPM
writes to the COMMAND register to start DRP toggling.
DRP toggling flow is chagned as following:
  - Write DRP = 0 & Rd/Rd
  - Write DRP = 1
  - Set LOOK4CONNECTION command

Signed-off-by: ShuFan Lee 
---
 drivers/staging/typec/tcpci.c | 127 +-
 drivers/staging/typec/tcpci.h |  13 +
 2 files changed, 113 insertions(+), 27 deletions(-)

 patch changelogs between v1 & v2
 - Remove unnecessary i2c_client in the structure of tcpci
 - Rename structure of tcpci_vendor_data to tcpci_data
 - Not exporting tcpci read/write wrappers but register i2c regmap in glue 
driver
 - Add set_vconn ops in tcpci_data
   (It is necessary for RT1711H to enable/disable idle mode before 
disabling/enabling vconn)
 - Export tcpci_irq so that vendor can call it in their own IRQ handler

 patch changelogs between v2 & v3
 - Change the return type of tcpci_irq from int to irqreturn_t

 patch changelogs between v3 & v4
 - Directly return regmap_write at the end of drp_toggling function
 - Because tcpci_irq is called in _tcpci_irq, move _tcpci_irq to the place 
after tcpci_irq

 patch changelogs between v4 & v5
 - Add a space between my first & last name, from ShuFanLee to ShuFan Lee.

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 9bd4412..5bafcc4 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -21,7 +21,6 @@
 
 struct tcpci {
struct device *dev;
-   struct i2c_client *client;
 
struct tcpm_port *port;
 
@@ -30,6 +29,12 @@ struct tcpci {
bool controls_vbus;
 
struct tcpc_dev tcpc;
+   struct tcpci_data *data;
+};
+
+struct tcpci_chip {
+   struct tcpci *tcpci;
+   struct tcpci_data data;
 };
 
 static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
@@ -37,8 +42,7 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev 
*tcpc)
return container_of(tcpc, struct tcpci, tcpc);
 }
 
-static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
-   u16 *val)
+static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
 {
return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
 }
@@ -98,8 +102,10 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum 
typec_cc_status cc)
 static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
enum typec_cc_status cc)
 {
+   int ret;
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
-   unsigned int reg = TCPC_ROLE_CTRL_DRP;
+   unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
+  (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
 
switch (cc) {
default:
@@ -117,7 +123,16 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
break;
}
 
-   return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+   usleep_range(500, 1000);
+   reg |= TCPC_ROLE_CTRL_DRP;
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+   return regmap_write(tcpci->regmap, TCPC_COMMAND,
+   TCPC_CMD_LOOK4CONNECTION);
 }
 
 static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
@@ -178,6 +193,16 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool 
enable)
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
int ret;
 
+   /* Handle vendor set vconn */
+   if (tcpci->data) {
+   if (tcpci->data->set_vconn) {
+   ret = tcpci->data->set_vconn(tcpci, tcpci->data,
+enable);
+   if (ret < 0)
+   return ret;
+   }
+   }
+
ret = regmap_write(tcpci->regmap, TCPC_POWER_CTRL,
   enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
if (ret < 0)
@@ -323,6 +348,15 @@ static int tcpci_init(struct tcpc_dev *tcpc)
if (time_after(jiffies, timeout))
return -ETIMEDOUT;
 
+   /* Handle vendor init */
+   if (tcpci->data) {
+   if (tcpci->data->init) {
+   ret = tcpci->data->init(tcpci, tcpci->data);
+   if (ret < 0)
+   return ret;
+   }
+   }
+
/* Clear all events */
ret = tcpci_write16(tcpci, TCPC_ALERT, 0x);
if (ret < 0)
@@ -344,9 +378,8 @@ static int tcpci_init(struct tcpc_dev *tcpc)
return tcpci_write16(tcpci, TCPC_ALERT_MASK

RE: [PATCH] usb: gadget: fsl_udc_core: fix ep valid checks

[Leo Li]

> -Original Message-
> From: Stefan Agner [mailto:ste...@agner.ch]
> Sent: Monday, February 12, 2018 7:15 AM
> To: Leo Li ; ba...@kernel.org
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; Stefan Agner
> 
> Subject: [PATCH] usb: gadget: fsl_udc_core: fix ep valid checks
> 
> Clang reports the following warning:
>   drivers/usb/gadget/udc/fsl_udc_core.c:1312:10: warning: address of array
>   'ep->name' will always evaluate to 'true' [-Wpointer-bool-conversion]
> if (ep->name)
> ~~  ^~~~
> 
> It seems that the authors intention was to check if the ep has been
> configured through struct_ep_setup. Check whether struct usb_ep name
> pointer has been set instead.
> 
> Signed-off-by: Stefan Agner 

Acked-by: Li Yang 

> ---
>  drivers/usb/gadget/udc/fsl_udc_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
> b/drivers/usb/gadget/udc/fsl_udc_core.c
> index e5b4ee96c4bf..56b517a38865 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -1305,7 +1305,7 @@ static void udc_reset_ep_queue(struct fsl_udc
> *udc, u8 pipe)  {
>   struct fsl_ep *ep = get_ep_by_pipe(udc, pipe);
> 
> - if (ep->name)
> + if (ep->ep.name)
>   nuke(ep, -ESHUTDOWN);
>  }
> 
> @@ -1693,7 +1693,7 @@ static void dtd_complete_irq(struct fsl_udc *udc)
>   curr_ep = get_ep_by_pipe(udc, i);
> 
>   /* If the ep is configured */
> - if (curr_ep->name == NULL) {
> + if (!curr_ep->ep.name) {
>   WARNING("Invalid EP?");
>   continue;
>   }
> --
> 2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] staging: typec: handle vendor defined part and modify drp toggling flow

[Jun Li]

Hi
> -Original Message-
> From: shufan_lee(李��帆) [mailto:shufan_...@richtek.com]
> Sent: 2018年2月28日 11:41
> To: Jun Li ; ShuFanLee ;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net; g...@kroah.com
> Cc: cy_huang(�S�⒃�) ;
> linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> 
> Subject: 回覆: [PATCH] staging: typec: handle vendor defined part and
> modify drp toggling flow
> 
> Hi Jun,
> 
>   For the questions of drp_toggling, our test is as following:
> 
>   According to TCPCI 4.4.5.2
> It is recommended the TCPM write ROLE_CONTROL.DRP=0 before writing to
> POWER_CONTROL.AutoDischargeDisconnect and starting the DRP toggling
> using COMMAND.Look4Connection.
> 
>   We've encounter a situation while testing with RT1711H as following:
>   We repeatedly plug in/out a device (with Rd), and not to provide VBUS(5V)
> while plugging in.
>   If we plug out the device right after TCPC detects it and stops toggling,
> TCPM will try to restart drp_toggling.
>   Here, we re-plug in the device before TCPM calls drp_toggling. Under this
> circumstance, RT1711H reports open/open after drp_toggling is called.

Why RT1711H reports open/open after drp_toggling is enabled?
This open/open is for you plug out the device, right?
Why RT1711H can't report new cc change events after you plug in the device?

Please use tcpm log to show all the events and state transitions for your
above corner case.
cat /sys/kernel/debug/tcpm/x

>   For current TCPM, it stops if open/open is reported at drp_toggling state.
> 
>   The detailed flow that causes RT1711H reporting open/open is described
> as following:
>   1. RT1711H detects the device, stops toggling and reports to TCPM.
>   2. Rd is plugged out before set_cc is called. So, ROLE_CONTROL.DRP is
> still 1.

So open/open cc change will generate.
 
>   3. Plug in the device before TCPM restarts drp_toggling
>   4. The device is detected by RT1711H's internal firmware again(TCPC
> chip's internal firmware).

What cc change event tcpc will report on step 4?

>   5. TCPM calls drp_toggling before cc_change triggered by step 4 is
> handled.
>   6. TCPM sets ROLE_CONTROL.DRP = 1, Rd/Rd and start drp_toggling.
> According to TCPCI 4.4.5.2
> The TCPM shall write B6 (DRP) = 0b and B3..0 (CC1/CC2) if it wishes to
> control the Rp/Rd directly instead of having the TCPC perform DRP toggling
> autonomousl.
> So, Rd/Rd set in step 6 will not work. (Because ROLE_CONTROL.DRP is 1
> since the first step.)
>   7. RT1711H will stop toggling immediately (Because it's internal firmware
> already detects device at step 4) and report open/open (Because CC1/CC2
> will be changed to Rd by RT1711H after LOOK4CONNECTION is set. RT1711H
> sets to Rd and device is Rd so open is reported)
>   8. TCPCI reports open/open to TCPM at drp_toggling state
> 
>   That's why we always set ROLE_CONTROL.DRP to 0 while setting Rd/Rd.
>   If this circumstance is not a general case, we can also use a vendor ops to
> replace it.

Thanks for the detailed description, the tcpm full log is required to know
what's going on here, you can apply my drp_toggling change patch[1] and
run your problem case again, then post the full tcpm log. 

Generally, I think you need check further this is a problem on the current
tcpm _or_ your tcpc chip, if the problem on the tcpm, you should resolve
this issue by improve tcpm, if the problem on your tcpc chip, you can add
a vendor ops.

I tested my tcpc HW with my drp_toggling change patch[1] (w/o your change)
by quickly pulg-in & plug-out a sink, no problem found.
Did you verify your change can pass the typec compliance test?

[1] https://www.spinics.net/lists/devicetree/msg216851.html
> ==
> ==
> 
> I don't catch the point of how you handle private events by above change,
> maybe post your RT1711H part as a user in one series can make it clear, I
> assume this can be done in existing tcpci_irq like above vender specific
> handling as well:
> 
> For every glue driver, it registers its own irq handler and calls tcpci_irq 
> in the
> handler.
> 
> Take RT1711H as an example:
> It registers its own irq handler in probe function and handle vendor defined
> interrupts before calling general tcpci_irq:
> 
> static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
> struct rt1711h_chip *chip = dev_id;
> 
> /* handle vendor defined interrupts here */
> 
> /* call general tcpci_irq */
> return tcpci_irq(chip->tcpci);
> }
> 
> static int rt1711h_probe(struct i2c_client *client, const struct i2c_device_id
> *i2c_id) {
> 
> ret = devm_request_threaded_irq(chip->dev, client->irq, NULL,
>   _tcpci_irq,
>   IRQF_ONESHOT |
> IRQF_TRIGGER_LOW,
>   dev_name(&client->dev),
> chip); }
> 
> 
Get it, thanks.

> Best Regards,
> **

Re: [PATCH v5 08/12] xhci: Add Intel extended cap / otg phy mux handling

[Heikki Krogerus]

On Wed, Feb 28, 2018 at 04:42:32PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 28-02-18 16:15, Heikki Krogerus wrote:
> > On Wed, Feb 28, 2018 at 04:07:45PM +0100, Hans de Goede wrote:
> > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > > index 96099a245c69..5917e3095e2a 100644
> > > --- a/drivers/usb/host/xhci.h
> > > +++ b/drivers/usb/host/xhci.h
> > > @@ -1825,6 +1825,7 @@ struct xhci_hcd {
> > >   /* Reserved. It was XHCI_U2_DISABLE_WAKE */
> > >   #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
> > >   #define XHCI_HW_LPM_DISABLE (1 << 29)
> > > +#define XHCI_INTEL_USB_ROLE_SW   (1 << 30)
> > 
> > Did you rebased these on tope of the latest usb-next?
> 
> No I did not expect that to be necessary, but I see now that it is.
> I've just done a rebase locally, any other remarks before I send
> out a v6?

Nothing from me.


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 01/12] drivers: base: Unified device connection lookup

[Heikki Krogerus]

Hi,

On Thu, Mar 01, 2018 at 12:56:57AM +, Jun Li wrote:
> > +struct device *device_find_connection(struct device *dev, const char
> > +*con_id) {
> > +   return __device_find_connection(dev, con_id, generic_match, NULL); }
> 
> -   return __device_find_connection(dev, con_id, generic_match, NULL);
> +   return __device_find_connection(dev, con_id, NULL, generic_match);

Good catch!

Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: usb-skeleton: make MODULE_LICENSE and SPDX tag match

[Marcus Folkesson]

GPL v2 is the original license according to the old license text.
See f64cdd0e94f1faf3b7f2b03af71f70dc6d8da0c2.

Signed-off-by: Marcus Folkesson 
---
 drivers/usb/usb-skeleton.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c
index 26ca0ec01fd5..c3ddd0f1f449 100644
--- a/drivers/usb/usb-skeleton.c
+++ b/drivers/usb/usb-skeleton.c
@@ -640,4 +640,4 @@ static struct usb_driver skel_driver = {
 
 module_usb_driver(skel_driver);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
-- 
2.16.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html