Re: [PATCH] [PATCH v2] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
On Mon, Apr 29, 2019 at 08:12:37PM +0800, Charles Yeh wrote: > Prolific has developed a new USB to UART chip: PL2303HXN > PL2303HXN : PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE/PL2303GB > The Vendor request used by the PL2303HXN (TYPE_HXN) is different from > the existing PL2303 series (TYPE_HX & TYPE_01). > Therefore, different Vendor requests are used to issue related commands. > > 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes >new Vendor request,new flow control and other related instructions >if TYPE_HXN is recognized. > > 2. Because the new PL2303HXN only accept the new Vendor request, >the old Vendor request cannot be accepted (the error message >will be returned) >So first determine the TYPE_HX or TYPE_HXN through >PL2303_READ_TYPE_HX_STATUS in pl2303_startup. > > 2.1 If the return message is "1", then the PL2303 is the existing > TYPE_HX/ TYPE_01 series. > The other settings in pl2303_startup are to continue execution. > 2.2 If the return message is "not 1", then the PL2303 is the new > TYPE_HXN series. > The other settings in pl2303_startup are ignored. > (PL2303HXN will directly use the default value in the hardware, >no need to add additional settings through the software) > > 3. In pl2303_open: Because TYPE_HXN is different from the instruction of reset >down/up stream used by TYPE_HX. >Therefore, we will also execute different instructions here. > > 4. In pl2303_set_termios: The UART flow control instructions used by >TYPE_HXN/TYPE_HX/TYPE_01 are different. >Therefore, we will also execute different instructions here. > > 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is different >from the vendor request instruction used by TYPE_HX/TYPE_01, >it will also execute different instructions here. > > Signed-off-by: Charles Yeh > --- What changed in v2? You forgot to add a changelog here. Looks like this one is not based on the current driver code (e.g. my usb-next branch as we discussed), and also does not address some of the issues raised so far (e.g. you're overwriting the entire flow control register on updates). I didn't have time to finish the prep work I promised to do, but don't worry, I haven't forgotten. Johan
Re: [PATCH] [PATCH v2] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
Johan Hovold 於 2019年5月3日 週五 下午3:11寫道： > What changed in v2? You forgot to add a changelog here. > > Looks like this one is not based on the current driver code (e.g. my > usb-next branch as we discussed), and also does not address some of the > issues raised so far (e.g. you're overwriting the entire flow control > register on updates). > I used the Kernel tree: git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git but the pl2303.c inside does not have the "pl2303_update_reg" you mentioned. Charles.
RE: [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries
Hi Claus, >-Original Message- >From: claus.stovga...@gmail.com [mailto:claus.stovga...@gmail.com] >Sent: Friday, May 03, 2019 3:06 AM >To: Anurag Kumar Vulisha ; Greg Kroah-Hartman >; Rob Herring ; Mark Rutland >; Felipe Balbi >Cc: linux-usb@vger.kernel.org; v.anuragku...@gmail.com; Rob Weber > >Subject: Re: [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 >entries > >Hi > >On tor, 2019-05-02 at 15:50 +0530, Anurag Kumar Vulisha wrote: >> Gadget applications may have a requirement to disable the U1 and U2 >> entry based on the usecase. For example, when performing performance >> benchmarking on mass storage gadget the U1 and U2 entries can be >> disabled. >> Another example is when periodic transfers like ISOC transfers are >> used >> with bInterval of 1 which doesn't require the link to enter into U1 >> or U2 >> state (since ping is issued from host for every uframe interval). In >> this >> case the U1 and U2 entry can be disabled. This can be done by setting >> U1DevExitLat and U2DevExitLat values to 0 in the BOS descriptor. Host >> on >> seeing 0 value for U1DevExitLat and U2DevExitLat, it doesn't send >> SET_SEL >> commands to the gadget. Thus entry of U1 and U2 states can be >> avioded. >> This patch updates the same. >> > >Will just vote for this feature, as I will also be able to use it for >solving Rob Webers and my issue [1] > Thanks for testing and voting for this patch. >Just today I was making another solution for this feature, using the >configfs instead of the devicetree. Though thinks your solution is >better, as it uses the U1DevExitLat and U2DevExitLat instead. I just >added my solution to the bottem of the mail for reference. > >[1] https://www.spinics.net/lists/linux-usb/msg179393.html > Your approach below is also good, but you are just avoiding the gadget dwc3 controller from entering into U1 and U2 states by disabling the ACCEPTU1ENA and ACCEPTU2ENA bits in DCTL but not preventing the host from sending the LG0_U1 and LGO_U2 link command signaling to the gadget. The host will keep on trying to get the link into U1 or U2 by sending LGO_U1 or LGO_U2 and the gadget rejects these signals by sending LXU link command. To avoid this extra overhead I thought that sending zero value in the BOS descriptor's U1DevExitLat and U2DevExitLat fields would be the best option. Host on seeing U 1 & U2 Exit Latencies doesn't initiate LPM U1 and U2 commands. Thanks, Anurag Kumar Vulisha >--- > >From 798ea2f5f365ecdf2dbcf436a2a0302e208c6c73 Mon Sep 17 00:00:00 2001 >From: "Claus H. Stovgaard" >Date: Thu, 2 May 2019 17:54:45 +0200 >Subject: [PATCH] usb: gadget: configfs: Add lpm_Ux_disable > >When combining dwc3 with an redriver for a USB Type-C device solution, >it >sometimes have problems with leaving U1/U2 for certain hosts, resulting >in >link training errors and reconnects. This patch create an interface via >configfs for disabling U1/U2, enabling a workaround for devices based >on >dwc3. > >Signed-off-by: Claus H. Stovgaard >--- > drivers/usb/dwc3/ep0.c| 9 ++- > drivers/usb/gadget/configfs.c | 56 >+++ > include/linux/usb/gadget.h| 6 - > 3 files changed, 69 insertions(+), 2 deletions(-) > >diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c >index 8efde17..5b2d26b 100644 >--- a/drivers/usb/dwc3/ep0.c >+++ b/drivers/usb/dwc3/ep0.c >@@ -379,6 +379,8 @@ static int dwc3_ep0_handle_u1(struct dwc3 *dwc, >enum usb_device_state state, > if ((dwc->speed != DWC3_DSTS_SUPERSPEED) && > (dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS)) > return -EINVAL; >+ if (dwc->gadget_driver->lpm_U1_disable) >+ return -EINVAL; > > reg = dwc3_readl(dwc->regs, DWC3_DCTL); > if (set) >@@ -401,6 +403,8 @@ static int dwc3_ep0_handle_u2(struct dwc3 *dwc, >enum usb_device_state state, > if ((dwc->speed != DWC3_DSTS_SUPERSPEED) && > (dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS)) > return -EINVAL; >+ if (dwc->gadget_driver->lpm_U2_disable) >+ return -EINVAL; > > reg = dwc3_readl(dwc->regs, DWC3_DCTL); > if (set) >@@ -626,7 +630,10 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, >struct usb_ctrlrequest *ctrl) >* nothing is pending from application. >*/ > reg = dwc3_readl(dwc->regs, DWC3_DCTL); >- reg |= (DWC3_DCTL_ACCEPTU1ENA | >DWC3_DCTL_ACCEPTU2ENA); >+ if (!dwc->gadget_driver->lpm_U1_disable) >+ reg |= DWC3_DCTL_ACCEPTU1ENA; >+ if (!dwc->gadget_driver->lpm_U2_disable) >+ reg |= DWC3_DCTL_ACCEPTU2ENA; > dwc3_writel(dwc->regs, DWC3_DCTL, reg); > } > break; >diff --git a/drivers/usb/gadget/configfs.c >b/drivers/usb/gadget/configfs.c >index 0251299..2ee9d10 100644 >---
Re: [PATCH] [PATCH v2] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
On Fri, May 03, 2019 at 03:28:04PM +0800, Charles Yeh wrote: > Johan Hovold 於 2019年5月3日 週五 下午3:11寫道： > > What changed in v2? You forgot to add a changelog here. > > > > Looks like this one is not based on the current driver code (e.g. my > > usb-next branch as we discussed), and also does not address some of the > > issues raised so far (e.g. you're overwriting the entire flow control > > register on updates). > > > > I used the Kernel tree: > git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git > > but the pl2303.c inside does not have the "pl2303_update_reg" you mentioned. Perhaps you did not use the usb-next branch? That branch has the following commit: https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-next&id=f64c3ab230682e8395a7fbd01f3eb5140c837d4e which adds the helper. Johan
Re: [PATCH v1 08/14] usb: dwc2: Add default param to control power optimization.
On 4/30/2019 19:23, Doug Anderson wrote: > Hi, > > On Tue, Apr 30, 2019 at 5:45 AM Artur Petrosyan > wrote: >> >>> If setting "power_down = 0" is wrong then please update your patch to >>> remove all the mainline code that sets power_down to 0. Presumably >>> this means you'd want to convert that code over to using "power_saving >>> = False". Perhaps then I can see your vision of how this works more >>> clearly. >> Yes this is a good idea. > > Actually, I have a feeling it won't work, at least not without more > changes. ...and that's part of the problem with your patch. I have lot of testes which prove that the patch is working. If there is any case that the patch is not working please mention. If possible provide the logs of the failure. > > Specifically dwc2 works by first filling in the "default" parameters. > Then the per-platform config function runs and overrides the defaults. > If the per-platform config function overrides the "power_saving" > parameter then it will be too late. > > >>> NOTE: I'm curious how you envision what someone would do if they had a >>> core that supported hibernation but they only wanted to enable partial >>> power down. I guess then they'd have to set "power_saving = True" and >>> then "power_down = DWC2_POWER_DOWN_PARAM_PARTIAL"? I guess your >>> vision of the world is: >> I have implemented everything based on programming guide and data book. >> Core can only support hibernation or partial power down based on the >> configuration parameters. There cannot be two modes simultaneously of >> power saving only one of them. > > Ah, this is new information to me. I assumed they were supersets of > each other, so if you supported hibernation you also supported partial > power down. Thanks for clearing that up! Welcome. All of that information is listed in the data book. > > >> The power_down flag is set to DWC2_POWER_DOWN_PARAM_PARTIAL , >> DWC2_POWER_DOWN_PARAM_HIBERNATION or DWC2_POWER_DOWN_PARAM_NONE while >> checking the hw parameters. So it just indicates which power down mode >> is supporting the core. > > I don't think this is what the params are about. The params are about > the currently configured parameters, not about what the core supports. > This is why all the other code checks the actual value of the params > to decide whether to do power savings. So for the power saving which should be currently configured parameter I have added power_saving flag. > > -Doug > -- Regards, Artur
Re: [PATCH 01/14] usb: dwc2: Fix dwc2_restore_device_registers() function.
On 4/30/2019 19:29, Doug Anderson wrote:
> Hi,
>
> On Mon, Apr 29, 2019 at 11:59 PM Artur Petrosyan
> wrote:
>>
>> On 4/29/2019 21:34, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Mon, Apr 29, 2019 at 3:51 AM Artur Petrosyan
>>> wrote:
On 4/27/2019 00:43, Doug Anderson wrote:
> Hi,
>
> On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan
> wrote:
>>
>> - Added backup of DCFG register.
>> - Added Set the Power-On Programming done bit.
>>
>> Signed-off-by: Artur Petrosyan
>> ---
>> drivers/usb/dwc2/gadget.c | 10 ++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 6812a8a3a98b..dcb0fbb8bc42 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct
>> dwc2_hsotg *hsotg, int remote_wakeup)
>> {
>>struct dwc2_dregs_backup *dr;
>>int i;
>> + u32 dctl;
>>
>>dev_dbg(hsotg->dev, "%s\n", __func__);
>>
>> @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct
>> dwc2_hsotg *hsotg, int remote_wakeup)
>>if (!remote_wakeup)
>>dwc2_writel(hsotg, dr->dctl, DCTL);
>>
>> + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
>> + dwc2_writel(hsotg, dr->dcfg, DCFG);
>> +
>> + /* Set the Power-On Programming done bit */
>> + dctl = dwc2_readl(hsotg, DCTL);
>> + dctl |= DCTL_PWRONPRGDONE;
>> + dwc2_writel(hsotg, dctl, DCTL);
>> + }
>
> I can't vouch one way or the other for the correctness of this change,
> but I will say that it's frustrating how asymmetric hibernate and
> partial power down are. It makes things really hard to reason about.
> Is there any way you could restructure this so it happens in the same
> way between hibernate and partial power down?
>
> Specifically looking at the similar sequence in
> dwc2_gadget_exit_hibernation() (which calls this function), I see:
>
> 1. There are some extra delays. Are those needed for partial power down?
Do you mean delays in dwc2_gadget_exit_hibernation() ? If yes they are
needed for hibernation flow. What relates to delays in hibernation
needed for partial power down. Anything that is implemented in the
hibernation delays or other things are part of hibernation and are not
used in partial power down because they are two different flows of power
saving.
>>>
>>> OK, if they aren't needed for partial power down then that's fine.
>>> When I see two functions doing nearly the same sets of writes but one
>>> has delays and the other doesn't it makes me wonder if that was on
>>> purpose or not.
>>>
>>>
> 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only
> happens if "not remote wakeup". Is it truly on purpose that you don't
> do that?
Currently partial power down doesn't support remote wakeup flow.
>>>
>>> Oh. What happens if you have partial power down enabled and try to
>>> enable remote wakeup? Does it give an error?
>> As far as I have been debugging I have not seen error in that case.
>>
>> Do you mean like it should print a message saying that current partial
>> power down code doesn't support remote wakeup?
>
> Not sure. ...why don't we just forget about this question? I don't
> have enough gadget knowledge nor any way to test.
Ok let's forget about that.
>
> -Doug
>
--
Regards,
Artur
Re: [PATCH v1 04/14] usb: dwc2: Fix suspend state in host mode for partial power down.
On 5/1/2019 05:55, Doug Anderson wrote: > Hi, > > On Tue, Apr 30, 2019 at 12:11 AM Artur Petrosyan > wrote: >> >> Hi, >> >> On 4/29/2019 21:35, Doug Anderson wrote: >>> Hi, >>> >>> On Mon, Apr 29, 2019 at 4:03 AM Artur Petrosyan >>> wrote: Hi, On 4/27/2019 00:45, Doug Anderson wrote: > Hi, > > On Fri, Apr 19, 2019 at 1:05 PM Artur Petrosyan > wrote: >> >> - In dwc2_port_suspend() function added waiting for the >> HPRT0.PrtSusp register field to be set. >> >> - In _dwc2_hcd_suspend() function added checking of >> "hsotg->flags.b.port_connect_status" port connection >> status if port connection status is 0 then skipping >> power saving (entering partial power down mode). >> Because if there is no device connected there would >> be no need to enter partial power down mode. >> >> - Added "hsotg->bus_suspended = true" beceuse after > > s/beceuse/because > >> entering partial power down in host mode the >> bus_suspended flag must be set. > > The above statement sounds to me like trying to win an argument by > saying "I'm right because I'm right." Can you give more details about > why "bus_suspended" must be set? See below also. > There is no intention of winning any argument. >>> >>> I was trying to say that your statement does not convey any >>> information about the "why". It just says: "I now set this variable >>> because it needs to be set". This tells me nothing useful because >>> presumably if it did't need to be set then you wouldn't have set it. >>> I want to know more information about how the code was broken before >>> you did this. What specifically requires this variable to be set? >> Specifically that variable is set when core enters to hibernation or >> partial power down. >>> >>> Are you familiar with "bus_suspended" flag ? have you looked at what is it used for ? * @bus_suspended: True if bus is suspended So when entering to hibernation is performed bus is suspended. To indicate that "hsotg->bus_suspended" is assigned to true. >>> >>> Perhaps you can help me understand what the difference is between >>> "port suspend" and "bus suspend" on dwc2. I think this is where my >>> confusion lies since there are functions for both and they do subtly >>> different things. ...but both functions set bus_suspended. >> dwc2_port_suspend() is a function which is called when set port feature >> "USB_PORT_FEAT_SUSPEND" is asserted. Yet, bus_suspended is a variable. >> That variable should be set any time that host enters to hibernation or >> partial power down. >> >>> >>> >> @@ -4514,6 +4519,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) >>goto skip_power_saving; >>} >> >> + hsotg->bus_suspended = true; >> + > > I'm a bit unsure about this. Specifically I note that > _dwc2_hcd_resume() has a special case "if (hsotg->bus_suspended)". > Does that now become dead code? No it doesn't became a dead code. >>> >>> Can you explain when it gets triggered, then? >> _dwc2_hcd_resume() is triggered by the system. >> As an example lets assume you have hibernated the PC and then you turn >> the PC on. When you turn the PC back on in that case _dwc2_hcd_resume() >> function is called to resume from suspended state (Hibernation/partial >> power down) > > I think you are still not understanding my question here. Please > re-read it again. Your question is "Can you explain when it gets triggered, then?" so I have explained one case when it is triggered. > > > -Doug > -- Regards, Artur
Re: [PATCH] [PATCH v2] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
Johan Hovold 於 2019年5月3日 週五 下午3:41寫道： > Perhaps you did not use the usb-next branch? That branch has the following > commit: > > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-next&id=f64c3ab230682e8395a7fbd01f3eb5140c837d4e > OK, I will do another patch witch "https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/snapshot/usb-serial-f64c3ab230682e8395a7fbd01f3eb5140c837d4e.tar.gz"; I already have seen pl2303_update_reg in the pl2303.c file. Charles.
Re: [PATCH] [PATCH v2] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
On Fri, May 03, 2019 at 04:22:01PM +0800, Charles Yeh wrote: > Johan Hovold 於 2019年5月3日 週五 下午3:41寫道： > > > Perhaps you did not use the usb-next branch? That branch has the following > > commit: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-next&id=f64c3ab230682e8395a7fbd01f3eb5140c837d4e > > > > OK, I will do another patch witch > "https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/snapshot/usb-serial-f64c3ab230682e8395a7fbd01f3eb5140c837d4e.tar.gz"; > > I already have seen pl2303_update_reg in the pl2303.c file. It's better if you use the full usb-next branch, e.g. git clone https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git cd usb-serial git checkout -b pl2303 usb-next or similar. That way you can easily fetch any future changes. Johan
[GIT PULL] USB changes for v5.2 merge window
Hi Greg,
here's my pull request for the next merge window. A bit later than
usuall this time around, due to internal high-priority tasks. Sorry
about that.
Let me know if you need anything to be changed.
cheers
___
< Busy as a bee >
---
\ ^__^
\ (oo)\___
(__)\ )\/\
||w |
|| ||
The following changes since commit 085b7755808aa11f78ab9377257e1dad2e6fa4bb:
Linux 5.1-rc6 (2019-04-21 10:45:57 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/usb-for-v5.2
for you to fetch changes up to 2e487d280525b91b03976203b15aba365ec5b4e6:
usb: dwc3: Rename DWC3_DCTL_LPM_ERRATA (2019-05-03 09:13:49 +0300)
USB: changes for v5.2 merge window
With a total of 50 non-merge commits, this is not a large pull
request. Most of the changes are, again, in dwc2 (37%) and dwc3 (32%)
with the rest of it scattered among other UDCs, function drivers and
device-tree bindings.
No really big feature this time around apart from support to Amlogic
being added to both dwc3 and dwc2 drivers.
Alan Stern (3):
USB: dummy-hcd: Fix failure to give back unlinked URBs
USB: UDC: net2280: Remove redundant "if" condition
USB: UDC: net22{80,72}: remove mistaken test of req->zero
Alexandre Belloni (5):
usb: gadget: udc: lpc32xx: simplify probe
usb: gadget: udc: lpc32xx: simplify vbus handling
usb: gadget: udc: lpc32xx: properly setup phy interrupts
usb: gadget: udc: lpc32xx: add support for stotg04 phy
usb: gadget: udc: lpc32xx: rework interrupt handling
Andy Shevchenko (1):
usb: dwc3: Free resource immediately after use
Arnd Bergmann (1):
usb: gadget: fsl: fix link error against usb-gadget module
Chunfeng Yun (2):
usb: dwc2: get optional clock by devm_clk_get_optional()
usb: introduce usb_ep_type_string() function
Douglas Anderson (6):
usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE
USB: Export usb_wakeup_enabled_descendants()
dt-bindings: usb: dwc2: Document quirk to reset PHY upon wakeup
usb: dwc2: optionally assert phy reset when waking up
ARM: dts: rockchip: Hook resets up to USB PHYs on rk3288.
ARM: dts: rockchip: Add quirk for resetting rk3288's dwc2 host on wakeup
Fei Yang (1):
usb: gadget: f_fs: don't free buffer prematurely
Jonas Bonn (3):
usb: gadget: atmel_usba_udc: simplify setting of interrupt-enabled mask
usb: gadget: atmel: support USB suspend
usb: gadget: atmel: tie wake lock to running clock
Jules Maselbas (5):
usb: dwc2: Move UTMI_PHY_DATA defines closer
usb: dwc2: gadget: Remove duplicated phy init
usb: dwc2: gadget: Replace phyif with phy_utmi_width
usb: dwc2: Move phy init into core
usb: dwc2: gadget: Move gadget phy init into core phy init
Marc Gonzalez (1):
usb: dwc3: Allow building USB_DWC3_QCOM without EXTCON
Marek Szyprowski (1):
usb: dwc3: move synchronize_irq() out of the spinlock protected block
Martin Blumenstingl (1):
dt-bindings: usb: dwc2: document the vbus-supply property
Minas Harutyunyan (7):
usb: dwc2: gadget: Reject LPM token during Control transfers
usb: dwc2: Delayed status support
usb: dwc2: gadget: Increase descriptors count for ISOC's
usb: dwc2: Set actual frame number for completed ISOC transfer
usb: dwc2: Fix channel disable flow
usb: dwc2: Set lpm mode parameters depend on HW configuration
dwc2: gadget: Fix completed transfer size calculation in DDMA
Neil Armstrong (4):
dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible
dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
usb: dwc2: Add Amlogic G12A DWC2 Params
usb: dwc3: Add Amlogic G12A DWC3 glue
Robin Murphy (1):
usb: dwc3: of-simple: Convert to bulk clk API
Romain Izard (2):
usb: gadget: f_ncm: Fix NTP-32 support
usb: gadget: f_ncm: Add OS descriptor support
Sergey Senozhatsky (1):
usb: gadget: do not use __constant_cpu_to_le16
Thinh Nguyen (5):
usb: dwc3: gadget: Set lpm_capable
usb: dwc3: Do core validation early on probe
usb: dwc3: debug: Print GET_STATUS(device) tracepoint
usb: dwc3: Fix default lpm_nyet_threshold value
usb: dwc3: Rename DWC3_DCTL_LPM_ERRATA
.../devicetree/bindings/usb/amlogic,dwc3.txt | 88 +++
Documentation/devicetree/bindings/usb/dwc2.txt | 7 +
arch/arm/boot/dts/rk3288.dtsi | 7 +
drivers/usb/common/common.c| 16 +
drivers/usb/core/hcd.c | 17 +-
drivers/usb/core/hub.c | 7 +-
drivers/usb/dwc2/core.c| 199 +++
drivers/us
Re: [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries
Hi Anurag On Fri, 2019-05-03 at 07:34 +, Anurag Kumar Vulisha wrote: > Hi Claus, > Thanks for testing and voting for this patch. I have first tested your patches today. My test setup is a ZynqMP device, running kernel 4.14 (Xilinx version) with your patches. I then create an overlay for the devicetree with the new parameters, and unbind/bind the dwc3 driver. Next I have a host running Windows 10 and a MacBook pro with Type-C ports. For logging the communication I use a Total Phase Beagle USB3 5000 V2 analyzer. The test showed that OS-X does as expected. When BOS descriptor (bU1DevExtLat and bU2DevExtLat) returns 0, it does not enable LPM. Windows 10 on the other hand does not, and even though it received 0 as bU1DevExtLat and bU2DevExtLat it send Set Sel with U1SEL 85 us, U1PEL 0 us, U2SEL 85 us and U2PEL 0 us. Next the Windows 10 host sends the U1 Enable and the U2 enable as Set Device Feature, resulting in the system entering U2. > > > > Just today I was making another solution for this feature, using > > the > > configfs instead of the devicetree. Though thinks your solution is > > better, as it uses the U1DevExitLat and U2DevExitLat instead. I > > just > > added my solution to the bottem of the mail for reference. > > > > [1] https://www.spinics.net/lists/linux-usb/msg179393.html > > > Your approach below is also good, but you are just avoiding the > gadget dwc3 > controller from entering into U1 and U2 states by disabling the > ACCEPTU1ENA > and ACCEPTU2ENA bits in DCTL but not preventing the host from sending > the > LG0_U1 and LGO_U2 link command signaling to the gadget. The host will > keep > on trying to get the link into U1 or U2 by sending LGO_U1 or LGO_U2 > and the > gadget rejects these signals by sending LXU link command. To avoid > this extra > overhead I thought that sending zero value in the BOS descriptor's > U1DevExitLat and U2DevExitLat fields would be the best option. Host > on seeing > U 1 & U2 Exit Latencies doesn't initiate LPM U1 and U2 commands. > > Thanks, > Anurag Kumar Vulisha Correct that it does not prevent the host from sending LG0_U1 and LG0_U2, and there is your solution better on hosts using the BOS descriptor for disabling LPM. So based on my test with Windows 10, I think we should combine the solutions. To prevent LG0_U1/LG0_U2 when possible and still being able to completely disable U1/U2. Regarding interface for controlling it. I am very novice regarding Linux kernel development, but would think the BOS descriptor control would be better from a configfs interface then devicetree as I don't see BOS descriptor as hardware specific. I am more in doubt about the forcing of U1/U2 as I did with setting hardware registers, as it control hardware registers. So will like to hear from other more experienced developers. Regards Claus
Re: [PATCH] USB: serial: io_edgeport: fix up switch fall-through comments
On 5/3/19 1:09 AM, Johan Hovold wrote: > On Thu, May 02, 2019 at 07:35:15PM +0200, Greg Kroah-Hartman wrote: >> Gustavo has been working to fix up all of the switch statements that >> "fall through" such that we can eventually turn on >> -Wimplicit-fallthrough. As part of that, the io_edgeport.c driver is a >> bit "messy" with the parsing logic of a data packet. Clean that logic >> up a bit by unindenting one level of the logic, and properly label >> /* Fall through */ to make gcc happy. >> >> Reported-by: Gustavo A. R. Silva >> Signed-off-by: Greg Kroah-Hartman > > Now applied, thanks. > > Gustavo, how many of these warnings are there left in the kernel now > that the last one in USB is gone? :) > Today, we woke up with 37 of these warnings left in linux-next. :) There were more than 2000 when I first started auditing them. Thanks -- Gustavo
Re: [PATCH V2 7/8] usb: gadget: Add UDC driver for tegra XUSB device mode controller
On Mon, Mar 11, 2019 at 04:41:55PM +0530, Nagarjuna Kristam wrote:
> This patch adds UDC driver for tegra XUSB 3.0 device mode controller.
s/tegra/Tegra/
> XUSB device mode controller support SS, HS and FS modes
s/support/supports/ and terminate the sentence with a full-stop.
>
> Based on work by:
> Mark Kuo
> Andrew Bresticker
>
> Signed-off-by: Nagarjuna Kristam
> ---
> drivers/usb/gadget/udc/Kconfig | 10 +
> drivers/usb/gadget/udc/Makefile |1 +
> drivers/usb/gadget/udc/tegra_xudc.c | 3702
> +++
> 3 files changed, 3713 insertions(+)
> create mode 100644 drivers/usb/gadget/udc/tegra_xudc.c
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 0a16cbd..f6f469c 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -439,6 +439,16 @@ config USB_GADGET_XILINX
> dynamically linked module called "udc-xilinx" and force all
> gadget drivers to also be dynamically linked.
>
> +config USB_TEGRA_XUDC
> + tristate "NVIDIA Superspeed USB 3.0 Device Controller"
NVIDIA Tegra? Not sure if this is available anywhere else.
> + depends on ARCH_TEGRA
> + help
> + Enables TEGRA USB 3.0 device mode controller driver.
NVIDIA Tegra here, too.
> +
> + Say "y" to link the driver statically, or "m" to build a
> + dynamically linked module called "tegra_xudc" and force all
> + gadget drivers to also be dynamically linked.
> +
> source "drivers/usb/gadget/udc/aspeed-vhub/Kconfig"
>
> #
> diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
> index 897f648..1c55c96 100644
> --- a/drivers/usb/gadget/udc/Makefile
> +++ b/drivers/usb/gadget/udc/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_BCM63XX_UDC) += bcm63xx_udc.o
> obj-$(CONFIG_USB_FSL_USB2) += fsl_usb2_udc.o
> fsl_usb2_udc-y := fsl_udc_core.o
> fsl_usb2_udc-$(CONFIG_ARCH_MXC) += fsl_mxc_udc.o
> +obj-$(CONFIG_USB_TEGRA_XUDC) += tegra_xudc.o
> obj-$(CONFIG_USB_M66592) += m66592-udc.o
> obj-$(CONFIG_USB_R8A66597) += r8a66597-udc.o
> obj-$(CONFIG_USB_RENESAS_USB3) += renesas_usb3.o
> diff --git a/drivers/usb/gadget/udc/tegra_xudc.c
> b/drivers/usb/gadget/udc/tegra_xudc.c
> new file mode 100644
> index 000..70beda0
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/tegra_xudc.c
> @@ -0,0 +1,3702 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NVIDIA XUSB device mode controller
Again, perhaps mention that this is for Tegra.
I didn't find anything that stood out in most of the driver. Below are a
couple of things towards the end that I think you should look at.
Generally I thought it was fairly difficult to read. You may want to see
if you can improve readability by adding a bit of whitespace where
appropriate. For example, try to leave a blank line above and below
block elements, such as conditionals or loops. I find that that helps a
lot in making the code easier to read.
See below for an example.
[...]
> +static int tegra_xudc_probe(struct platform_device *pdev)
> +{
> + struct tegra_xudc *xudc;
> + struct resource *res;
> + unsigned int i;
> + int err;
> +
> + xudc = devm_kzalloc(&pdev->dev, sizeof(*xudc), GFP_ATOMIC);
> + if (!xudc)
> + return -ENOMEM;
> + xudc->dev = &pdev->dev;
> + platform_set_drvdata(pdev, xudc);
This, for example, would be easier to read as:
xudc = devm_kzalloc(&pdev->dev, sizeof(*xudc), GFP_ATOMIC);
if (!xudc)
return -ENOMEM;
platform_set_drvdata(pdev, xudc);
xudc->dev = &pdev->dev;
> +
> + xudc->soc = of_device_get_match_data(&pdev->dev);
> + if (!xudc->soc)
> + return -ENODEV;
This situation can never happen. The driver is only ever instantiated
from device tree, in which case xudc->soc will be a valid pointer to the
correct SoC data structure.
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + xudc->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(xudc->base))
> + return PTR_ERR(xudc->base);
> + xudc->phys_base = res->start;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + xudc->fpci = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(xudc->fpci))
> + return PTR_ERR(xudc->fpci);
> +
> + if (xudc->soc->has_ipfs) {
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> + xudc->ipfs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(xudc->ipfs))
> + return PTR_ERR(xudc->ipfs);
> + }
> +
> + xudc->irq = platform_get_irq(pdev, 0);
> + if (xudc->irq < 0) {
> + dev_err(xudc->dev, "failed to get IRQ: %d\n",
> + xudc->irq);
> + return xudc->irq;
> + }
> + err = devm_request_irq(&pdev->dev, xudc->irq, tegra_xudc_irq, 0,
> +
Re: [PATCH V2 4/8] dt-bindings: usb: Add NVIDIA Tegra XUSB device mode controller binding
On Thu, Apr 25, 2019 at 05:14:01PM +0200, Thierry Reding wrote:
> On Mon, Mar 11, 2019 at 04:41:52PM +0530, Nagarjuna Kristam wrote:
> > Add device-tree binding documentation for the XUSB device mode controller
> > present on tegra210 SoC. This controller supports USB 3.0 specification
>
> Tegra210, please. "... supports the USB 3.0 ...". Also end sentences
> with a fullstop.
>
> >
> > Based on work by Andrew Bresticker .
> >
> > Signed-off-by: Nagarjuna Kristam
> > ---
> > .../devicetree/bindings/usb/nvidia,tegra-xudc.txt | 105
> > +
> > 1 file changed, 105 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> > b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> > new file mode 100644
> > index 000..990655d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra-xudc.txt
> > @@ -0,0 +1,105 @@
> > +Device tree binding for NVIDIA Tegra XUSB device mode controller (XUDC)
> > +===
> > +
> > +The Tegra XUDC controller supports both USB 2.0 HighSpeed/FullSpeed and
> > +USB 3.0 SuperSpeed protocols.
> > +
> > +Required properties:
> > +
> > +- compatible: For Tegra210, must contain "nvidia,tegra210-xudc".
> > +- reg: Must contain the base and length of the XUSB device registers, XUSB
> > device
> > + PCI Config registers and XUSB device controller registers.
> > +- interrupts: Must contain the XUSB device interrupt
> > +- clocks: Must contain an entry for ell clocks used.
>
> s/ell/all/
>
> > + See ../clock/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > + - xusb_device
> > + - xusb_ss
> > + - xusb_ss_src
> > + - xusb_hs_src
> > + - xusb_fs_src
>
> It'd be good to explain what each of these are.
Perhaps we should also drop the xusb_ prefix here. That's already
implied by this being the XUDC controller bindings.
>
> > +- nvidia,xusb-padctl: phandle to the XUSB pad controller that is used to
> > + configure the USB pads used by the XUDC controller
> > +- power-domains: A list of PM domain specifiers that reference each
> > power-domain
> > + used by the XUSB device mode controller. This list must comprise of a
> > specifier
> > + for the XUSBA and XUSBB power-domains. See ../power/power_domain.txt and
> > + ../arm/tegra/nvidia,tegra20-pmc.txt for details.
> > +- power-domain-names: A list of names that represent each of the
> > specifiers in
> > + the 'power-domains' property. Must include 'xusb_ss' and 'xusb_device'
Same here, I'd drop the xusb_ prefix. I think the "device" power domain
is also usually referred to as "XUSB_DEV", so perhaps make that "dev"
instead?
Thierry
> > +
> > +For Tegra210:
> > +- avddio-usb-supply: PCIe/USB3 analog logic power supply. Must supply 1.05
> > V.
> > +- hvdd-usb-supply: USB controller power supply. Must supply 3.3 V.
> > +- avdd-pll-utmip-supply: UTMI PLL power supply. Must supply 1.8 V.
>
> My understanding is that this last supply is really needed for the XUSB
> pad controller to bring up the PLL. In fact, I've just moved the same
> supply to the XUSB pad controller from the XUSB controller for all of
> the supported boards because having this in the XUSB controller would
> fail under some circumstances.
>
> > +
> > +- phys: Must contain an entry for each entry in phy-names.
> > + See ../phy/phy-bindings.txt for details.
> > +- extcon-usb: Must contains an extcon-usb entry which detects
>
> In the example below, this is simply "extcon".
>
> > + USB VBUS pin. See ../extcon/extcon-usb-gpio.txt for details.
> > +
> > +Optional properties:
> > +
> > +- phy-names: Should include an entry for each PHY used by the controller.
> > + Names must be "usb2", and "usb3" if support SuperSpeed device mode.
> > + - "usb3" phy, SuperSpeed (SSTX+/SSTX-/SSRX+/SSRX-) data lines
> > + - "usb2" phy, USB 2.0 (D+/D-) data lines
>
> Why are these optional? phys is required and references phy-names
> explicitly, so I think that effectively makes these phy-names required
> as well.
>
> > +
> > +Example:
> > +
> > + pmc: pmc@7000e400 {
> > + compatible = "nvidia,tegra210-pmc";
> > + reg = <0x0 0x7000e400 0x0 0x400>;
> > + clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
> > + clock-names = "pclk", "clk32k_in";
> > +
> > + powergates {
> > + pd_xusbss: xusba {
> > + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>;
> > + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>;
> > + #power-domain-cells = <0>;
> > + };
> > +
> > + pd_xusbdev: xusbb {
> > + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>;
> > +
[GIT PULL] USB-serial updates for 5.2-rc1
The following changes since commit 79a3aaa7b82e3106be97842dedfd8429248896e6: Linux 5.1-rc3 (2019-03-31 14:39:29 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git tags/usb-serial-5.2-rc1 for you to fetch changes up to 7f6fc50242d11d4fedab9cf6c5e8af368c076ccd: USB: serial: f81232: implement break control (2019-05-03 09:19:55 +0200) USB-serial updates for 5.2-rc1 Here are the USB-serial updates for 5.2-rc1, including: - flow-control related fixes for pl2303 - fix for an initial-termios issue - fix for a couple of unthrottle() races - fix for f81232 interrupt-handling issues - improved f81232 overrun handling - support for higher f81232 line speeds - support for f81232 break control Included are also various clean ups. All but the last four commits have been in linux-next and with no reported issues. Signed-off-by: Johan Hovold Greg Kroah-Hartman (1): USB: serial: io_edgeport: fix up switch fall-through comments Ji-Ze Hong (Peter Hong) (4): USB: serial: f81232: fix interrupt worker not stop USB: serial: f81232: clear overrun flag USB: serial: f81232: add high baud rate support USB: serial: f81232: implement break control Johan Hovold (17): USB: serial: pl2303: fix non-supported xon/xoff USB: serial: pl2303: fix tranceiver suspend mode USB: serial: digi_acceleport: clean up modem-control handling USB: serial: digi_acceleport: clean up set_termios USB: serial: fix initial-termios handling USB: serial: ark3116: drop redundant init_termios USB: serial: cypress_m8: drop unused driver data flag USB: serial: cypress_m8: drop unused termios USB: serial: cypress_m8: clean up initial-termios handling USB: serial: iuu_phoenix: drop bogus initial cflag USB: serial: iuu_phoenix: simplify init_termios USB: serial: oti6858: simplify init_termios USB: serial: spcp8x5: simplify init_termios USB: serial: fix unthrottle races USB: serial: clean up throttle handling USB: serial: drop unnecessary goto USB: serial: drop unused iflag macro drivers/usb/serial/ark3116.c | 11 -- drivers/usb/serial/cypress_m8.c | 49 ++--- drivers/usb/serial/digi_acceleport.c | 41 drivers/usb/serial/f81232.c | 198 --- drivers/usb/serial/generic.c | 76 +++--- drivers/usb/serial/io_edgeport.c | 37 +++ drivers/usb/serial/iuu_phoenix.c | 4 +- drivers/usb/serial/oti6858.c | 5 +- drivers/usb/serial/pl2303.c | 58 -- drivers/usb/serial/spcp8x5.c | 5 +- drivers/usb/serial/usb-serial.c | 11 +- include/linux/usb/serial.h | 8 +- 12 files changed, 324 insertions(+), 179 deletions(-)
Re: [PATCH] USB: serial: io_edgeport: fix up switch fall-through comments
On Fri, May 03, 2019 at 09:00:44AM -0500, Gustavo A. R. Silva wrote: > On 5/3/19 1:09 AM, Johan Hovold wrote: > > Gustavo, how many of these warnings are there left in the kernel now > > that the last one in USB is gone? :) > > > > Today, we woke up with 37 of these warnings left in linux-next. :) > > There were more than 2000 when I first started auditing them. Nice work! Hope the remaining ones aren't all super tricky to address. Johan
Re: [GIT PULL] USB-serial updates for 5.2-rc1
On Fri, May 03, 2019 at 05:36:50PM +0200, Johan Hovold wrote: > The following changes since commit 79a3aaa7b82e3106be97842dedfd8429248896e6: > > Linux 5.1-rc3 (2019-03-31 14:39:29 -0700) > > are available in the Git repository at: > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git > tags/usb-serial-5.2-rc1 Pulled and pushed out, thanks. greg k-h
Re: [GIT PULL] USB changes for v5.2 merge window
On Fri, May 03, 2019 at 12:15:18PM +0300, Felipe Balbi wrote: > > Hi Greg, > > here's my pull request for the next merge window. A bit later than > usuall this time around, due to internal high-priority tasks. Sorry > about that. > > Let me know if you need anything to be changed. > > cheers > > ___ > < Busy as a bee > > --- > \ ^__^ > \ (oo)\___ > (__)\ )\/\ > ||w | > || || > > The following changes since commit 085b7755808aa11f78ab9377257e1dad2e6fa4bb: > > Linux 5.1-rc6 (2019-04-21 10:45:57 -0700) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/usb-for-v5.2 Pulled and pushed out, thanks. greg k-h
[PATCH -next] usb: gadget: udc: lpc32xx: fix return value check in lpc32xx_udc_probe()
In case of error, the function devm_ioremap_resource() returns ERR_PTR()
and never returns NULL. The NULL test in the return value check should
be replaced with IS_ERR().
This issue was detected by using the Coccinelle software.
Fixes: 408b56ca5c8e ("usb: gadget: udc: lpc32xx: simplify probe")
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/udc/lpc32xx_udc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c
b/drivers/usb/gadget/udc/lpc32xx_udc.c
index d8f1c60793ed..00fb79c6d025 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -3070,9 +3070,9 @@ static int lpc32xx_udc_probe(struct platform_device *pdev)
}
udc->udp_baseaddr = devm_ioremap_resource(dev, res);
- if (!udc->udp_baseaddr) {
+ if (IS_ERR(udc->udp_baseaddr)) {
dev_err(udc->dev, "IO map failure\n");
- return -ENOMEM;
+ return PTR_ERR(udc->udp_baseaddr);
}
/* Get USB device clock */
