Re: [PATCH 2/4] usb: dwc3: gadget: check if dep->frame_number is still valid

2018-08-23 Thread Thinh Nguyen
On 8/23/2018 3:15 AM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen  writes:
>> On 8/20/2018 3:34 AM, Felipe Balbi wrote:
>>> Gadget driver may take an unbounded amount of time to queue requests
>>> after XferNotReady. This is important for isochronous endpoints which
>>> need to be started for a specific (micro-)frame.
>>>
>>> Before kicking the transfer, let's check if current frame number is
>>> still less than our aligned frame number that we got from the
>>> previous XferNotReady. If it isn't, then we'll increment
>>> dep->frame_number to make sure it's ahead of current frame number.
>>>
>>> Signed-off-by: Felipe Balbi 
>>> ---
>>>  drivers/usb/dwc3/core.h   |  2 ++
>>>  drivers/usb/dwc3/gadget.c | 11 +++
>>>  2 files changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 285ce0ef3b91..3acf8788a680 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -685,6 +685,8 @@ struct dwc3_ep {
>>> u8  type;
>>> u8  resource_index;
>>> u32 frame_number;
>>> +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff
>>> +
>>> u32 interval;
>>>  
>>> charname[20];
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index f61a4250c883..0bac9b02f28b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1257,6 +1257,9 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>>>  
>>>  static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>>>  {
>>> +   u16 current_frame_number;
>>> +   u16 frame_number;
>>> +
>>> if (list_empty(>pending_list)) {
>>> dev_info(dep->dwc->dev, "%s: ran out of requests\n",
>>> dep->name);
>>> @@ -1265,6 +1268,14 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep 
>>> *dep)
>>> }
>>>  
>>> dep->frame_number = DWC3_ALIGN_FRAME(dep);
>>> +   current_frame_number = __dwc3_gadget_get_frame(dep->dwc);
>>> +   frame_number = dep->frame_number & DWC3_EP_FRAME_NUMBER_MASK;
>>> +
>>> +   if (frame_number <= current_frame_number) {
>> This is not right. You're comparing (what should be a 16-bit but masked)
>> frame_number to a 14-bit current_frame_number.
>> If the current frame number is 2+ seconds ahead of dep->frame_number,
>> then this calculation is wrong.
> According to docs, the numbers are the same and the top 2 bits are used
> internally by the IP. I.E. they're not part of the frame number. This is
> described in table 6-51 on databook 2.60a.
>
> In any case, there's no other way to account for the overflow since the
> HW doesn't give me all the bits I need in DSTS. Any ideas?
>
You can check if there's pending/started request, then you can kick the
transfer. Otherwise don't kick transfer and send END_TRANSFER command so
that the controller can resend XferNotReady event to restart again.

Thinh



musb_hdrc HNP?

2018-08-23 Thread Takashi Matsuzawa
Hello.

I am trying HNP (host -> peripheral role change) using two PocketBeagles, but 
without success.
If there any idea on where I, where should I ask this, or how can I debug / 
fix, I really appreciate.

1) What I did

Two PocketBeagles (running the default Debian image).
Both added 2nd USB ports (musb_hdrc.1).
DTBs were modified so that the 2nd musb_hdrcs have dr_mode = "otg".
musb_dsps glue port seems to be used.
Tech reference manual says the SoC's musb core supports HNP.

One is A (by GNDing ID pin) and another B, connected with USB cable.
modprobe g_zero on both so that VBUS power is on, and A becomes a_host and b 
becomes b_peripheral (as read through debugfs mode value).

Then I did the following, expecting HNP happens and A bevcomes a_peripheral and 
B bcomes b_host:

i) Send b_hnp_enable request on A.
(This does set DEVCTL.HOSTREQ bits on B's musb_hdrc.1 and maybe doing some 
additinoal things in musb driver on B.)

ii) Set POWER.SUSPENDM bit on A's musb_hdrc.1
(According to TRM, this should tell B's musb core to initiate HNP.)

2) Observation

A: a_host -> a_wait_bcon -> a_idle -> a_wait_vrise -> a_host
B: b_peripheral -> b_idle -> b_peripheral

And I see "musb_hdrc.1 Babble" messages on both A and B console.

Looks like B disconnects once, but A = host/B = peripheral does not change.

Looking into musb driver source, there are code for HNP maybe to help musb core 
behavior.
(But I have not enabled driver debug message yet, which I may try next?)


Assalaam Alaikum,

2018-08-23 Thread Hajia Fadia Mohammed .
Assalaam Alaikum,

ATTN:


May Allah the most graceful and the most merciful be with you. I have not been 
able to contact you because of my ill-health conditions since August 2012, but 
by the mercy of Allah I am still alive today.

I want to inform you that the transaction I proposed to you since August, 2012, 
concerning the funds my family ear-marked for charity  has still not been 
accomplished and I need your re-assurances, honesty and trust in order to 
commence with you as my reliable partner, now that my condition has fairly 
improved health-wise.

I instinctively do believe you that you are a trustworthy and sincere person, 
that is why I am still asking for your partnership, especially in nominating an 
account to move this funds.

Please respond quickly as time is not so much on our side, to enable me know 
the next step to follow.

Thank you once again, while I will anticipate your immediate response in this 
regards.



N:B YOU REPLY ME BACK ON THIS EMAIL BOX hajiafad...@outlook.com


May Allah be with you.




Hajia Fadia Mohammed.


[no subject]

2018-08-23 Thread Mavis W



-- 

Hallo Am Mrs Mavis Wancyzk, Sie haben eine Spende von 4,800,000.00EUR Ich 
gewann die America Lottery im Wert von $ 758.7 Millionen und ich spende einen 
Teil davon an fnf glckliche Menschen und 
Wohlttigkeits-Huser in Erinnerung an meinen verstorbenen Ehemann, 
der an Krebs gestorben ist. Kontaktieren Sie mich fr weitere Details 
unter: 
[maviswanczy...@hotmail.com]


Re: [PATCH 5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration

2018-08-23 Thread Bjorn Andersson
On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote:

> Phy power on/off cycle can happen several times during device life.
> We then need to balance the extcon notifier registration accordingly.
> 
> Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
> Signed-off-by: Loic Poulain 
> ---
>  drivers/phy/qualcomm/phy-qcom-usb-hs.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c 
> b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> index 2d0c70b..92e9d94 100644
> --- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> @@ -181,6 +181,12 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
>  {
>   struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
>  
> + if (uphy->vbus_edev) {
> + devm_extcon_unregister_notifier(>ulpi->dev,
> + uphy->vbus_edev, EXTCON_USB,
> + >vbus_notify);

As I presume the power_on should always be matched with a power_off I
wonder if it really is appropriate to use the devres version of this
API.

Should we drop the devm_ from registration in power_on as well?

> + }
> +
>   regulator_disable(uphy->v3p3);
>   regulator_disable(uphy->v1p8);
>   clk_disable_unprepare(uphy->sleep_clk);

Regards,
Bjorn


RE: USB Type-C hub detection is flaky

2018-08-23 Thread Mario.Limonciello
> -Original Message-
> From: Timur Kristóf [mailto:timur.kris...@gmail.com]
> Sent: Thursday, August 23, 2018 11:03 AM
> To: Heikki Krogerus; Limonciello, Mario
> Cc: linux-usb@vger.kernel.org; Mika Westerberg
> Subject: Re: USB Type-C hub detection is flaky
> 
> On Thu, 2018-07-05 at 15:58 +0300, Heikki Krogerus wrote:
> 
> > > >
> > > > Did you have a change to test if the problem can be reproduced in
> > > > Windows too?
> > >
> > > Sorry, not yet but I will get around to it next week.
> > > (The XPS is my primary work machine so I will have to back up
> > > everything before trying my luck with Windows.)
> >
> > Oh, ic. In that case, perhaps somebody else could test this with
> > Windows?
> >
> > Mario! Can you guys test if self powered USB hubs get enumerated
> > normally in Windows on XPS 9370?
> >
> 
> Hi Heikki & Mario,
> 
> Have you guys managed to test this under Windows?
> If not, I can still do it.
> 
> Thanks & best regards,
> Tim

Timur,

Sorry I haven't gotten a chance.  I didn't have a self powered USB hub
readily available to me and got pre-occupied with other things .  If you do
have one and can test you can probably do it quicker than me right now.


Re: USB Type-C hub detection is flaky

2018-08-23 Thread Timur Kristóf
On Thu, 2018-07-05 at 15:58 +0300, Heikki Krogerus wrote:

> > > 
> > > Did you have a change to test if the problem can be reproduced in
> > > Windows too?
> > 
> > Sorry, not yet but I will get around to it next week.
> > (The XPS is my primary work machine so I will have to back up
> > everything before trying my luck with Windows.)
> 
> Oh, ic. In that case, perhaps somebody else could test this with
> Windows?
> 
> Mario! Can you guys test if self powered USB hubs get enumerated
> normally in Windows on XPS 9370?
> 

Hi Heikki & Mario,

Have you guys managed to test this under Windows?
If not, I can still do it.

Thanks & best regards,
Tim



Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection

2018-08-23 Thread Loic Poulain
Hi Bjorn,

On 23 August 2018 at 16:53, Bjorn Andersson  wrote:
> On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote:
>
>> Some hardware implementations require to configure pins differently
>> according to the USB role (host/device), this can be an update of the
>> pins routing or a simple GPIO value change.
>>
>> This patch introduces new optional "host" and "device" pinctrls.
>> If these pinctrls are defined by the device, they are respectively
>> selected on host/device role start.
>>
>> If a default pinctrl exist, it is restored on host/device role stop.
>>
>
> The implementation looks reasonable, but we're actually just toggling a
> gpio using pinctrl states. Do you see any reason not to control this mux
> through the gpio api?
>

Actually, two gpios (including hub reset one), but you're right.
Point is that these gpios are very specific to the Dragonboard layout and
not related to USB controller itself (external mux), so I'm not sure it makes
sense to control a 'mux' GPIO from the chipidea driver. That's why I used
pinctrl which is more generic and hides the underlying operations (a simple
GPIO toggling in our case).

But let me know if there is a better way.


Regards,
Loic


Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection

2018-08-23 Thread Bjorn Andersson
On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote:

> Some hardware implementations require to configure pins differently
> according to the USB role (host/device), this can be an update of the
> pins routing or a simple GPIO value change.
> 
> This patch introduces new optional "host" and "device" pinctrls.
> If these pinctrls are defined by the device, they are respectively
> selected on host/device role start.
> 
> If a default pinctrl exist, it is restored on host/device role stop.
> 

The implementation looks reasonable, but we're actually just toggling a
gpio using pinctrl states. Do you see any reason not to control this mux
through the gpio api?

Regards,
Bjorn

> Signed-off-by: Loic Poulain 
> ---
>  drivers/usb/chipidea/core.c  | 19 +++
>  drivers/usb/chipidea/host.c  |  9 +
>  drivers/usb/chipidea/udc.c   |  9 +
>  include/linux/usb/chipidea.h |  6 ++
>  4 files changed, 43 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 85fc6db..03e52fc 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -46,6 +46,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -723,6 +724,24 @@ static int ci_get_platdata(struct device *dev,
>   else
>   cable->connected = false;
>   }
> +
> + platdata->pctl = devm_pinctrl_get(dev);
> + if (!IS_ERR(platdata->pctl)) {
> + struct pinctrl_state *p;
> +
> + p = pinctrl_lookup_state(platdata->pctl, "default");
> + if (!IS_ERR(p))
> + platdata->pins_default = p;
> +
> + p = pinctrl_lookup_state(platdata->pctl, "host");
> + if (!IS_ERR(p))
> + platdata->pins_host = p;
> +
> + p = pinctrl_lookup_state(platdata->pctl, "device");
> + if (!IS_ERR(p))
> + platdata->pins_device = p;
> + }
> +
>   return 0;
>  }
>  
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index af45aa32..55dbd49 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "../host/ehci.h"
>  
> @@ -150,6 +151,10 @@ static int host_start(struct ci_hdrc *ci)
>   }
>   }
>  
> + if (ci->platdata->pins_host)
> + pinctrl_select_state(ci->platdata->pctl,
> +  ci->platdata->pins_host);
> +
>   ret = usb_add_hcd(hcd, 0, 0);
>   if (ret) {
>   goto disable_reg;
> @@ -194,6 +199,10 @@ static void host_stop(struct ci_hdrc *ci)
>   }
>   ci->hcd = NULL;
>   ci->otg.host = NULL;
> +
> + if (ci->platdata->pins_host && ci->platdata->pins_default)
> + pinctrl_select_state(ci->platdata->pctl,
> +  ci->platdata->pins_default);
>  }
>  
>  
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 9852ec5..c04384e 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "ci.h"
>  #include "udc.h"
> @@ -1965,6 +1966,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
>  
>  static int udc_id_switch_for_device(struct ci_hdrc *ci)
>  {
> + if (ci->platdata->pins_device)
> + pinctrl_select_state(ci->platdata->pctl,
> +  ci->platdata->pins_device);
> +
>   if (ci->is_otg)
>   /* Clear and enable BSV irq */
>   hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
> @@ -1983,6 +1988,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
>   hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
>  
>   ci->vbus_active = 0;
> +
> + if (ci->platdata->pins_device && ci->platdata->pins_default)
> + pinctrl_select_state(ci->platdata->pctl,
> +  ci->platdata->pins_default);
>  }
>  
>  /**
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index 07f9936..63758c3 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -77,6 +77,12 @@ struct ci_hdrc_platform_data {
>   struct ci_hdrc_cablevbus_extcon;
>   struct ci_hdrc_cableid_extcon;
>   u32 phy_clkgate_delay_us;
> +
> + /* pins */
> + struct pinctrl *pctl;
> + struct pinctrl_state *pins_default;
> + struct pinctrl_state *pins_host;
> + struct pinctrl_state *pins_device;
>  };
>  
>  /* Default offset of capability registers */
> -- 
> 2.7.4
> 


[PATCH v2] usb: musb: dsps: do not disable CPPI41 irq in driver teardown

2018-08-23 Thread Bin Liu
TI AM335x CPPI 4.1 module uses a single register bit for CPPI interrupts
in both musb controllers. So disabling the CPPI irq in one musb driver
breaks the other musb module.

Since musb is already disabled before tearing down dma controller in
musb_remove(), it is safe to not disable CPPI irq in
musb_dma_controller_destroy().

Fixes: 255348289f71 ("usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in DSPS")
Cc: sta...@vger.kernel.org
Signed-off-by: Bin Liu 
---
v2: revise commit message

 drivers/usb/musb/musb_dsps.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index df827ff57b0d..23a0df79ef21 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -658,16 +658,6 @@ dsps_dma_controller_create(struct musb *musb, void __iomem 
*base)
return controller;
 }
 
-static void dsps_dma_controller_destroy(struct dma_controller *c)
-{
-   struct musb *musb = c->musb;
-   struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
-   void __iomem *usbss_base = glue->usbss_base;
-
-   musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
-   cppi41_dma_controller_destroy(c);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static void dsps_dma_controller_suspend(struct dsps_glue *glue)
 {
@@ -697,7 +687,7 @@ static struct musb_platform_ops dsps_ops = {
 
 #ifdef CONFIG_USB_TI_CPPI41_DMA
.dma_init   = dsps_dma_controller_create,
-   .dma_exit   = dsps_dma_controller_destroy,
+   .dma_exit   = cppi41_dma_controller_destroy,
 #endif
.enable = dsps_musb_enable,
.disable= dsps_musb_disable,
-- 
2.17.1



Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex

2018-08-23 Thread Laurent Pinchart
Hi Andy,

On Thursday, 23 August 2018 12:57:57 EEST Andy Shevchenko wrote:
> On Mon, Aug 20, 2018 at 3:21 PM Laurent Pinchart wrote:
> > On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote:
> >> On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote:
>  - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status:
>  %s",
>  + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status:
>  %s",
> >>> 
> >>> How about 0x%x ?
> >> 
> >> Side note: # is one character less for the same.
> > 
> > Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I
> > find that always printing the 0x is more consistent. I'll leave that up
> > to Felipe, I'm OK with both options.
> 
>#  The value should be converted to an "alternate form".
> For o conversions, the first character  of
>  the  output  string is made zero (by prefixing a 0 if it
> was not zero already).  For x and X con‐
>  versions, a nonzero result has the string "0x" (or "0X"
> for X conversions) prepended to it.

That's exactly my point, it will only prepend 0x when the value is not zero. 
Small inconsistency, but I don't mind too much.

-- 
Regards,

Laurent Pinchart





Re: [PATCH 2/4] usb: dwc3: gadget: check if dep->frame_number is still valid

2018-08-23 Thread Felipe Balbi

Hi,

Thinh Nguyen  writes:
> On 8/20/2018 3:34 AM, Felipe Balbi wrote:
>> Gadget driver may take an unbounded amount of time to queue requests
>> after XferNotReady. This is important for isochronous endpoints which
>> need to be started for a specific (micro-)frame.
>>
>> Before kicking the transfer, let's check if current frame number is
>> still less than our aligned frame number that we got from the
>> previous XferNotReady. If it isn't, then we'll increment
>> dep->frame_number to make sure it's ahead of current frame number.
>>
>> Signed-off-by: Felipe Balbi 
>> ---
>>  drivers/usb/dwc3/core.h   |  2 ++
>>  drivers/usb/dwc3/gadget.c | 11 +++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 285ce0ef3b91..3acf8788a680 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -685,6 +685,8 @@ struct dwc3_ep {
>>  u8  type;
>>  u8  resource_index;
>>  u32 frame_number;
>> +#define DWC3_EP_FRAME_NUMBER_MASK 0x3fff
>> +
>>  u32 interval;
>>  
>>  charname[20];
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index f61a4250c883..0bac9b02f28b 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1257,6 +1257,9 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>>  
>>  static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>>  {
>> +u16 current_frame_number;
>> +u16 frame_number;
>> +
>>  if (list_empty(>pending_list)) {
>>  dev_info(dep->dwc->dev, "%s: ran out of requests\n",
>>  dep->name);
>> @@ -1265,6 +1268,14 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep 
>> *dep)
>>  }
>>  
>>  dep->frame_number = DWC3_ALIGN_FRAME(dep);
>> +current_frame_number = __dwc3_gadget_get_frame(dep->dwc);
>> +frame_number = dep->frame_number & DWC3_EP_FRAME_NUMBER_MASK;
>> +
>> +if (frame_number <= current_frame_number) {
>
> This is not right. You're comparing (what should be a 16-bit but masked)
> frame_number to a 14-bit current_frame_number.
> If the current frame number is 2+ seconds ahead of dep->frame_number,
> then this calculation is wrong.

According to docs, the numbers are the same and the top 2 bits are used
internally by the IP. I.E. they're not part of the frame number. This is
described in table 6-51 on databook 2.60a.

In any case, there's no other way to account for the overflow since the
HW doesn't give me all the bits I need in DSTS. Any ideas?

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection

2018-08-23 Thread Andy Shevchenko
On Tue, Aug 21, 2018 at 4:57 PM Loic Poulain  wrote:
>
> Some hardware implementations require to configure pins differently
> according to the USB role (host/device), this can be an update of the
> pins routing or a simple GPIO value change.
>
> This patch introduces new optional "host" and "device" pinctrls.
> If these pinctrls are defined by the device, they are respectively
> selected on host/device role start.
>
> If a default pinctrl exist, it is restored on host/device role stop.

>  #include 
>  #include 
>  #include 
> +#include 

A nit: Even in this context it would be nice to keep *some* order.

>  #include 
>  #include 
>  #include 

> +   p = pinctrl_lookup_state(platdata->pctl, "default");
> +   p = pinctrl_lookup_state(platdata->pctl, "host");
> +   p = pinctrl_lookup_state(platdata->pctl, "device");

I'm wondering if we have any rules applied to these names.

>  #include 
>  #include 
>  #include 
> +#include 

Ditto about ordering.

> +   if (ci->platdata->pins_host)
> +   pinctrl_select_state(ci->platdata->pctl,
> +ci->platdata->pins_host);

Hmm... Do we need to have a condition above?

> +   if (ci->platdata->pins_host && ci->platdata->pins_default)
> +   pinctrl_select_state(ci->platdata->pctl,
> +ci->platdata->pins_default);

Ditto about conditional.

>  #include 
>  #include 
>  #include 
> +#include 

Ditto about ordering.

> +   if (ci->platdata->pins_device)
> +   pinctrl_select_state(ci->platdata->pctl,
> +ci->platdata->pins_device);


> +   if (ci->platdata->pins_device && ci->platdata->pins_default)
> +   pinctrl_select_state(ci->platdata->pctl,
> +ci->platdata->pins_default);

Ditto about conditional.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH 3/4] usb: dwc3: trace: log ep commands in hex

2018-08-23 Thread Andy Shevchenko
On Mon, Aug 20, 2018 at 3:21 PM Laurent Pinchart
 wrote:
>
> Hi Andy,
>
> On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote:
> > On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote:
> > >> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s",
> > >> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s",
> > >
> > > How about 0x%x ?
> >
> > Side note: # is one character less for the same.
>
> Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I find
> that always printing the 0x is more consistent. I'll leave that up to Felipe,
> I'm OK with both options.

   #  The value should be converted to an "alternate form".
For o conversions, the first character  of
 the  output  string is made zero (by prefixing a 0 if it
was not zero already).  For x and X con‐
 versions, a nonzero result has the string "0x" (or "0X"
for X conversions) prepended to it.


-- 
With Best Regards,
Andy Shevchenko