RE: [PATCH v3] usb: core: verify devicetree nodes for USB devices
>
> On 2019-05-10 05:10, Peter Chen wrote:
> >
> >> Marek Szyprowski writes:
> >>> Commit 69bec7259853 ("USB: core: let USB device know device node")
> >>> added support for attaching devicetree node for USB devices. The
> >>> mentioned commit however identifies the given USB device node only by the
> 'reg'
> >>> property in the host controller children nodes. The USB device node
> >>> however also has to have a 'compatible' property as described in
> >>> Documentation/devicetree/bindings/usb/usb-device.txt. Lack for the
> >>> 'compatible' property check might result in assigning a devicetree
> >>> node, which is not intended to be the proper node for the given USB
> >>> device.
> >>>
> >>> This is important especially when USB host controller has
> >>> child-nodes for other purposes. For example, Exynos EHCI and OHCI
> >>> drivers already define child-nodes for each physical root hub port
> >>> and assigns respective PHY controller and parameters for them. Those
> >>> binding predates support for USB devicetree nodes.
> >>>
> >>> Checking for the proper compatibility string allows to mitigate the
> >>> conflict between USB device devicetree nodes and the bindings for
> >>> USB controllers with child nodes. It also fixes the side-effect of
> >>> the other commits, like 01fdf179f4b0 ("usb: core: skip interfaces
> >>> disabled in devicetree"), which incorrectly disables some devices on
> >>> Exynos based boards.
> > Hi Marek,
> >
> > The purpose of your patch is do not set of_node for device under USB
> > controller,
> right?
>
> Right.
>
Do you mind doing it at function exynos_ehci_get_phy of ehci-exynos.c?
Peter
Re: [PATCH v3] usb: core: verify devicetree nodes for USB devices
Hi Peter,
On 2019-05-13 11:00, Peter Chen wrote:
>> On 2019-05-10 05:10, Peter Chen wrote:
Marek Szyprowski writes:
> Commit 69bec7259853 ("USB: core: let USB device know device node")
> added support for attaching devicetree node for USB devices. The
> mentioned commit however identifies the given USB device node only by the
>> 'reg'
> property in the host controller children nodes. The USB device node
> however also has to have a 'compatible' property as described in
> Documentation/devicetree/bindings/usb/usb-device.txt. Lack for the
> 'compatible' property check might result in assigning a devicetree
> node, which is not intended to be the proper node for the given USB
> device.
>
> This is important especially when USB host controller has
> child-nodes for other purposes. For example, Exynos EHCI and OHCI
> drivers already define child-nodes for each physical root hub port
> and assigns respective PHY controller and parameters for them. Those
> binding predates support for USB devicetree nodes.
>
> Checking for the proper compatibility string allows to mitigate the
> conflict between USB device devicetree nodes and the bindings for
> USB controllers with child nodes. It also fixes the side-effect of
> the other commits, like 01fdf179f4b0 ("usb: core: skip interfaces
> disabled in devicetree"), which incorrectly disables some devices on
> Exynos based boards.
>>> Hi Marek,
>>>
>>> The purpose of your patch is do not set of_node for device under USB
>>> controller,
>> right?
>>
>> Right.
>>
> Do you mind doing it at function exynos_ehci_get_phy of ehci-exynos.c?
I don't mind fixing it in ehci-exynos, but frankly so far I have no idea
how to do it. The problem is that newly created USB devices get of-node
pointer pointing to a node which if not intended for them. How this can
be fixed in ehci-exynos?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
RE: [PATCH v3] usb: core: verify devicetree nodes for USB devices
> On 2019-05-13 11:00, Peter Chen wrote:
> >> On 2019-05-10 05:10, Peter Chen wrote:
> Marek Szyprowski writes:
> > Commit 69bec7259853 ("USB: core: let USB device know device node")
> > added support for attaching devicetree node for USB devices. The
> > mentioned commit however identifies the given USB device node only
> > by the
> >> 'reg'
> > property in the host controller children nodes. The USB device
> > node however also has to have a 'compatible' property as described
> > in Documentation/devicetree/bindings/usb/usb-device.txt. Lack for
> > the 'compatible' property check might result in assigning a
> > devicetree node, which is not intended to be the proper node for the
> > given
> USB device.
> >
> > This is important especially when USB host controller has
> > child-nodes for other purposes. For example, Exynos EHCI and OHCI
> > drivers already define child-nodes for each physical root hub port
> > and assigns respective PHY controller and parameters for them.
> > Those binding predates support for USB devicetree nodes.
> >
> > Checking for the proper compatibility string allows to mitigate
> > the conflict between USB device devicetree nodes and the bindings
> > for USB controllers with child nodes. It also fixes the
> > side-effect of the other commits, like 01fdf179f4b0 ("usb: core:
> > skip interfaces disabled in devicetree"), which incorrectly
> > disables some devices on Exynos based boards.
> >>> Hi Marek,
> >>>
> >>> The purpose of your patch is do not set of_node for device under USB
> >>> controller,
> >> right?
> >>
> >> Right.
> >>
> > Do you mind doing it at function exynos_ehci_get_phy of ehci-exynos.c?
>
> I don't mind fixing it in ehci-exynos, but frankly so far I have no idea how
> to do it.
> The problem is that newly created USB devices get of-node pointer pointing to
> a
> node which if not intended for them. How this can be fixed in ehci-exynos?
>
Can't be workaround by setting of_node as NULL for EHCI controller or for PHY
node at
exynos_ehci_get_phy?
Peter
Re: [PATCH v3] usb: core: verify devicetree nodes for USB devices
Hi Peter,
On 2019-05-13 11:23, Peter Chen wrote:
>> On 2019-05-13 11:00, Peter Chen wrote:
On 2019-05-10 05:10, Peter Chen wrote:
>> Marek Szyprowski writes:
>>> Commit 69bec7259853 ("USB: core: let USB device know device node")
>>> added support for attaching devicetree node for USB devices. The
>>> mentioned commit however identifies the given USB device node only
>>> by the
'reg'
>>> property in the host controller children nodes. The USB device
>>> node however also has to have a 'compatible' property as described
>>> in Documentation/devicetree/bindings/usb/usb-device.txt. Lack for
>>> the 'compatible' property check might result in assigning a
>>> devicetree node, which is not intended to be the proper node for the
>>> given
>> USB device.
>>> This is important especially when USB host controller has
>>> child-nodes for other purposes. For example, Exynos EHCI and OHCI
>>> drivers already define child-nodes for each physical root hub port
>>> and assigns respective PHY controller and parameters for them.
>>> Those binding predates support for USB devicetree nodes.
>>>
>>> Checking for the proper compatibility string allows to mitigate
>>> the conflict between USB device devicetree nodes and the bindings
>>> for USB controllers with child nodes. It also fixes the
>>> side-effect of the other commits, like 01fdf179f4b0 ("usb: core:
>>> skip interfaces disabled in devicetree"), which incorrectly
>>> disables some devices on Exynos based boards.
> Hi Marek,
>
> The purpose of your patch is do not set of_node for device under USB
> controller,
right?
Right.
>>> Do you mind doing it at function exynos_ehci_get_phy of ehci-exynos.c?
>> I don't mind fixing it in ehci-exynos, but frankly so far I have no idea how
>> to do it.
>> The problem is that newly created USB devices get of-node pointer pointing
>> to a
>> node which if not intended for them. How this can be fixed in ehci-exynos?
>>
>
> Can't be workaround by setting of_node as NULL for EHCI controller or for PHY
> node at
> exynos_ehci_get_phy?
Ah, such workaround? I will check, but this will need to be done with
care, because have a side effect for other subsystems like regulators or
clocks.
BTW, What's wrong with proper, full verification of USB device nodes?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Re: [PATCH v3] usb: core: verify devicetree nodes for USB devices
Marek Szyprowski writes:
> Hi Peter,
>
> On 2019-05-10 05:10, Peter Chen wrote:
>>
>>> Marek Szyprowski writes:
Commit 69bec7259853 ("USB: core: let USB device know device node")
added support for attaching devicetree node for USB devices. The
mentioned commit however identifies the given USB device node only by the
'reg'
property in the host controller children nodes. The USB device node
however also has to have a 'compatible' property as described in
Documentation/devicetree/bindings/usb/usb-device.txt. Lack for the
'compatible' property check might result in assigning a devicetree
node, which is not intended to be the proper node for the given USB device.
This is important especially when USB host controller has child-nodes
for other purposes. For example, Exynos EHCI and OHCI drivers already
define child-nodes for each physical root hub port and assigns
respective PHY controller and parameters for them. Those binding
predates support for USB devicetree nodes.
Checking for the proper compatibility string allows to mitigate the
conflict between USB device devicetree nodes and the bindings for USB
controllers with child nodes. It also fixes the side-effect of the
other commits, like 01fdf179f4b0 ("usb: core: skip interfaces disabled
in devicetree"), which incorrectly disables some devices on Exynos
based boards.
>> Hi Marek,
>>
>> The purpose of your patch is do not set of_node for device under USB
>> controller, right?
>
> Right.
>
>> I do not understand how 01fdf179f4b0 affect your boards, some nodes
>> under the USB controller with status is not "okay", but still want to
>> be enumerated?
>
> Please look at the ehci node in arch/arm/boot/dts/exynos4.dtsi and then
> at the changes to that node in arch/arm/boot/dts/exynos4412-odroidx.dts.
> Exynos EHCI controller has 3 subnodes, which matches to the physical
> ports of it and allows the driver to enable given PHY ports depending on
> which physical port is used on the particular board. All ports cannot
> not be enabled by default, because PHY controller has limited resources
> and shares them between USB host and USB device ports.
It seems like what's happening is that the Exynos port/phy nodes are
mistaken for standard USB device nodes attached to the root hub. The
problem is that hub port numbering starts at 1 while the Exynos nodes
start from 0. This causes attached devices to be associated with the
wrong DT node.
Ignoring backwards compatibility, I can see a few ways of fixing this:
- Add another child node, along side the port@N nodes, of the host
controller to represent the root hub. Nodes for attached devices
would then be descendants of this new node.
- Change the Exynos HCD binding to use a more standard "phys" property
and get rid of the child nodes for this purpose.
- Move the port@N nodes below a new dedicated child node of the HCD.
The first is probably the easiest to implement since it doesn't require
any nasty hacks to avoid breaking existing device trees.
--
Måns Rullgård
Re: [PATCH v3] usb: core: verify devicetree nodes for USB devices
Marek Szyprowski writes:
> Hi Peter,
>
> On 2019-05-13 11:23, Peter Chen wrote:
>>> On 2019-05-13 11:00, Peter Chen wrote:
> On 2019-05-10 05:10, Peter Chen wrote:
>>> Marek Szyprowski writes:
Commit 69bec7259853 ("USB: core: let USB device know device node")
added support for attaching devicetree node for USB devices. The
mentioned commit however identifies the given USB device node only
by the
> 'reg'
property in the host controller children nodes. The USB device
node however also has to have a 'compatible' property as described
in Documentation/devicetree/bindings/usb/usb-device.txt. Lack for
the 'compatible' property check might result in assigning a
devicetree node, which is not intended to be the proper node for the
given
>>> USB device.
This is important especially when USB host controller has
child-nodes for other purposes. For example, Exynos EHCI and OHCI
drivers already define child-nodes for each physical root hub port
and assigns respective PHY controller and parameters for them.
Those binding predates support for USB devicetree nodes.
Checking for the proper compatibility string allows to mitigate
the conflict between USB device devicetree nodes and the bindings
for USB controllers with child nodes. It also fixes the
side-effect of the other commits, like 01fdf179f4b0 ("usb: core:
skip interfaces disabled in devicetree"), which incorrectly
disables some devices on Exynos based boards.
>> Hi Marek,
>>
>> The purpose of your patch is do not set of_node for device under USB
>> controller,
> right?
>
> Right.
>
Do you mind doing it at function exynos_ehci_get_phy of ehci-exynos.c?
>>> I don't mind fixing it in ehci-exynos, but frankly so far I have no
>>> idea how to do it. The problem is that newly created USB devices
>>> get of-node pointer pointing to a node which if not intended for
>>> them. How this can be fixed in ehci-exynos?
>>>
>>
>> Can't be workaround by setting of_node as NULL for EHCI controller or
>> for PHY node at exynos_ehci_get_phy?
>
> Ah, such workaround? I will check, but this will need to be done with
> care, because have a side effect for other subsystems like regulators or
> clocks.
>
> BTW, What's wrong with proper, full verification of USB device nodes?
Your approach so far doesn't address the actual problem of a conflict
between the generic USB DT bindings and those for the Exynos host
controller. If you fix that, the validation issue goes away.
--
Måns Rullgård
Re: [PATCH 1/5] USB: serial: fix unthrottle races
On Thu, Apr 25, 2019 at 06:05:36PM +0200, Johan Hovold wrote:
> Fix two long-standing bugs which could potentially lead to memory
> corruption or leave the port throttled until it is reopened (on weakly
> ordered systems), respectively, when read-URB completion races with
> unthrottle().
>
> First, the URB must not be marked as free before processing is complete
> to prevent it from being submitted by unthrottle() on another CPU.
>
> CPU 1 CPU 2
>
> complete() unthrottle()
> process_urb();
> smp_mb__before_atomic();
> set_bit(i, free); if (test_and_clear_bit(i, free))
> submit_urb();
>
> Second, the URB must be marked as free before checking the throttled
> flag to prevent unthrottle() on another CPU from failing to observe that
> the URB needs to be submitted if complete() sees that the throttled flag
> is set.
>
> CPU 1 CPU 2
>
> complete() unthrottle()
> set_bit(i, free); throttled = 0;
> smp_mb__after_atomic(); smp_mb();
> if (throttled) if (test_and_clear_bit(i, free))
> return; submit_urb();
>
> Note that test_and_clear_bit() only implies barriers when the test is
> successful. To handle the case where the URB is still in use an explicit
> barrier needs to be added to unthrottle() for the second race condition.
>
> Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs")
> Signed-off-by: Johan Hovold
Greg, I noticed you added a stable tag to the corresponding cdc-acm fix
and think I should have added on one from the start to this one as well.
Would you mind queuing this one up for stable?
Upstream commit 3f5edd58d040bfa4b74fb89bc02f0bc6b9cd06ab.
Thanks,
Johan
Re: [PATCH 1/5] USB: serial: fix unthrottle races
On Mon, May 13, 2019 at 12:43:39PM +0200, Johan Hovold wrote:
> On Thu, Apr 25, 2019 at 06:05:36PM +0200, Johan Hovold wrote:
> > Fix two long-standing bugs which could potentially lead to memory
> > corruption or leave the port throttled until it is reopened (on weakly
> > ordered systems), respectively, when read-URB completion races with
> > unthrottle().
> >
> > First, the URB must not be marked as free before processing is complete
> > to prevent it from being submitted by unthrottle() on another CPU.
> >
> > CPU 1 CPU 2
> >
> > complete() unthrottle()
> > process_urb();
> > smp_mb__before_atomic();
> > set_bit(i, free); if (test_and_clear_bit(i, free))
> > submit_urb();
> >
> > Second, the URB must be marked as free before checking the throttled
> > flag to prevent unthrottle() on another CPU from failing to observe that
> > the URB needs to be submitted if complete() sees that the throttled flag
> > is set.
> >
> > CPU 1 CPU 2
> >
> > complete() unthrottle()
> > set_bit(i, free); throttled = 0;
> > smp_mb__after_atomic(); smp_mb();
> > if (throttled) if (test_and_clear_bit(i, free))
> > return; submit_urb();
> >
> > Note that test_and_clear_bit() only implies barriers when the test is
> > successful. To handle the case where the URB is still in use an explicit
> > barrier needs to be added to unthrottle() for the second race condition.
> >
> > Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs")
> > Signed-off-by: Johan Hovold
>
> Greg, I noticed you added a stable tag to the corresponding cdc-acm fix
> and think I should have added on one from the start to this one as well.
>
> Would you mind queuing this one up for stable?
>
> Upstream commit 3f5edd58d040bfa4b74fb89bc02f0bc6b9cd06ab.
Sure, now queued up for 4.9+
thanks,
greg k-h
Re: [PATCH 0/5] usb: gadget: udc: lpc32xx: add stotg04 phy support
Tested with a board containing LPC3250 SOC and STOTG04 PHY by using serial gadget. Needed patch "[PATCH] usb: gadget: udc: lpc32xx: allocate descriptor with GFP_ATOMIC" also. Tested-by: James Grant Regards, James Grant. On 09/04/2019 14:09, Alexandre Belloni Wrote: Hi, This series starts to clean up the lpc32xx udc driver and also repairs interrupt handling so hotplugging actually works. The design I tested that on uses an stotg04 instead of the usual isp1301 so support for that is also added. A bit more work is needed to get the RNDIS gadget driver to work. 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 drivers/usb/gadget/udc/lpc32xx_udc.c | 167 +++ 1 file changed, 66 insertions(+), 101 deletions(-)
Re: [PATCH] usb: gadget: udc: lpc32xx: allocate descriptor with GFP_ATOMIC
Tested with a board containing LPC3250 SOC and STOTG04 PHY by using serial gadget. Needed patch series starting with "[PATCH 0/5] usb: gadget: udc: lpc32xx: add stotg04 phy support" also. Tested-by: James Grant Regards, James Grant. On 11/05/2019 00:42, Alexandre Belloni wrote: Gadget drivers may queue request in interrupt context. This would lead to a descriptor allocation in that context. In that case we would hit BUG_ON(in_interrupt()) in __get_vm_area_node. Signed-off-by: Alexandre Belloni --- drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c index d8f1c60793ed..b706d9c85a35 100644 --- a/drivers/usb/gadget/udc/lpc32xx_udc.c +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c @@ -938,7 +938,7 @@ static struct lpc32xx_usbd_dd_gad *udc_dd_alloc(struct lpc32xx_udc *udc) struct lpc32xx_usbd_dd_gad *dd; dd = (struct lpc32xx_usbd_dd_gad *) dma_pool_alloc( - udc->dd_cache, (GFP_KERNEL | GFP_DMA), &dma); + udc->dd_cache, (GFP_ATOMIC | GFP_DMA), &dma); if (dd) dd->this_dma = dma;
Re: [PATCH 1/5] USB: serial: fix unthrottle races
On Mon, May 13, 2019 at 12:56:06PM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 13, 2019 at 12:43:39PM +0200, Johan Hovold wrote:
> > On Thu, Apr 25, 2019 at 06:05:36PM +0200, Johan Hovold wrote:
> > > Fix two long-standing bugs which could potentially lead to memory
> > > corruption or leave the port throttled until it is reopened (on weakly
> > > ordered systems), respectively, when read-URB completion races with
> > > unthrottle().
> > > Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs")
> > > Signed-off-by: Johan Hovold
> >
> > Greg, I noticed you added a stable tag to the corresponding cdc-acm fix
> > and think I should have added on one from the start to this one as well.
> >
> > Would you mind queuing this one up for stable?
> >
> > Upstream commit 3f5edd58d040bfa4b74fb89bc02f0bc6b9cd06ab.
>
> Sure, now queued up for 4.9+
Thanks. The issue has been there since v3.3 so I guess you could queue
it for all stable trees.
Johan
Re: [PATCH v2] usb: renesas_usbhs: Use specific struct instead of USBHS_TYPE_* enums
On Mon, May 13, 2019 at 11:40:29AM +0900, Yoshihiro Shimoda wrote:
> This patch adds a specific struct "usbhs_of_data" to add a new SoC
> data easily instead of code basis in the future.
>
> Signed-off-by: Yoshihiro Shimoda
> Reviewed-by: Geert Uytterhoeven
Hi Shimoda-san,
the minor suggestion below not withstanding this looks good to me.
Reviewed-by: Simon Horman
> ---
> Changes from v1 [1]:
> - Use sizeof(variable) instead of sizeof(type).
> - Add Geert-san's reviewed-by (thanks!).
>
> [1]
> https://patchwork.kernel.org/patch/10938575/
>
> drivers/usb/renesas_usbhs/common.c | 112
> +
> drivers/usb/renesas_usbhs/common.h | 5 ++
> 2 files changed, 70 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/usb/renesas_usbhs/common.c
> b/drivers/usb/renesas_usbhs/common.c
> index 249fbee..0ca89de 100644
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -535,53 +535,92 @@ static int usbhsc_drvcllbck_notify_hotplug(struct
> platform_device *pdev)
> return 0;
> }
>
> +static const struct usbhs_of_data rcar_gen2_data = {
> + .platform_callback = &usbhs_rcar2_ops,
> + .param = {
> + .type = USBHS_TYPE_RCAR_GEN2,
> + .has_usb_dmac = 1,
> + .pipe_configs = usbhsc_new_pipe,
> + .pipe_size = ARRAY_SIZE(usbhsc_new_pipe),
> + }
> +};
> +
> +static const struct usbhs_of_data rcar_gen3_data = {
> + .platform_callback = &usbhs_rcar3_ops,
> + .param = {
> + .type = USBHS_TYPE_RCAR_GEN3,
> + .has_usb_dmac = 1,
> + .pipe_configs = usbhsc_new_pipe,
> + .pipe_size = ARRAY_SIZE(usbhsc_new_pipe),
> + }
> +};
> +
> +static const struct usbhs_of_data rcar_gen3_with_pll_data = {
> + .platform_callback = &usbhs_rcar3_with_pll_ops,
> + .param = {
> + .type = USBHS_TYPE_RCAR_GEN3_WITH_PLL,
> + .has_usb_dmac = 1,
> + .pipe_configs = usbhsc_new_pipe,
> + .pipe_size = ARRAY_SIZE(usbhsc_new_pipe),
> + }
> +};
> +
> +static const struct usbhs_of_data rza1_data = {
> + .platform_callback = &usbhs_rza1_ops,
> + .param = {
> + .type = USBHS_TYPE_RZA1,
> + .pipe_configs = usbhsc_new_pipe,
> + .pipe_size = ARRAY_SIZE(usbhsc_new_pipe),
> + }
> +};
> +
> /*
> * platform functions
> */
> static const struct of_device_id usbhs_of_match[] = {
> {
> .compatible = "renesas,usbhs-r8a774c0",
> - .data = (void *)USBHS_TYPE_RCAR_GEN3_WITH_PLL,
> + .data = &rcar_gen3_with_pll_data,
> },
> {
> .compatible = "renesas,usbhs-r8a7790",
> - .data = (void *)USBHS_TYPE_RCAR_GEN2,
> + .data = &rcar_gen2_data,
> },
> {
> .compatible = "renesas,usbhs-r8a7791",
> - .data = (void *)USBHS_TYPE_RCAR_GEN2,
> + .data = &rcar_gen2_data,
> },
> {
> .compatible = "renesas,usbhs-r8a7794",
> - .data = (void *)USBHS_TYPE_RCAR_GEN2,
> + .data = &rcar_gen2_data,
> },
> {
> .compatible = "renesas,usbhs-r8a7795",
> - .data = (void *)USBHS_TYPE_RCAR_GEN3,
> + .data = &rcar_gen3_data,
> },
> {
> .compatible = "renesas,usbhs-r8a7796",
> - .data = (void *)USBHS_TYPE_RCAR_GEN3,
> + .data = &rcar_gen3_data,
> },
> {
> .compatible = "renesas,usbhs-r8a77990",
> - .data = (void *)USBHS_TYPE_RCAR_GEN3_WITH_PLL,
> + .data = &rcar_gen3_with_pll_data,
> },
> {
> .compatible = "renesas,usbhs-r8a77995",
> - .data = (void *)USBHS_TYPE_RCAR_GEN3_WITH_PLL,
> + .data = &rcar_gen3_with_pll_data,
> },
> {
> .compatible = "renesas,rcar-gen2-usbhs",
> - .data = (void *)USBHS_TYPE_RCAR_GEN2,
> + .data = &rcar_gen2_data,
> },
> {
> .compatible = "renesas,rcar-gen3-usbhs",
> - .data = (void *)USBHS_TYPE_RCAR_GEN3,
> + .data = &rcar_gen3_data,
> },
> {
> .compatible = "renesas,rza1-usbhs",
> - .data = (void *)USBHS_TYPE_RZA1,
> + .data = &rza1_data,
> },
> { },
> };
> @@ -591,6 +630,7 @@ static struct renesas_usbhs_platform_info
> *usbhs_parse_dt(struct device *dev)
> {
> struct renesas_usbhs_platform_info *info;
> struct renesas_usbhs_driver_param *dparam;
> + const struct usbhs_of_data *data;
> u32 tmp;
> int gpio;
>
> @@ -598,8 +638,15 @@ static struct renesas_usbhs_platform_info
> *usbhs_parse_dt(struct device *dev)
> if (!info)
> return NULL;
>
> + data = of_device_get_match_data(dev);
> + if (!data)
> + return NULL;
> +
> dparam = &
Re: [PATCH v2 02/15] ARM: dts: rza2mevb: Add 48MHz USB clock
On Thu, May 09, 2019 at 03:11:29PM -0500, Chris Brandt wrote: > The RZ/A2M EVB has a 48MHz clock attached to USB_X1. > > Signed-off-by: Chris Brandt Thanks, This looks fine to me but I will wait to see if there are other reviews before applying. Reviewed-by: Simon Horman
Re: [PATCH v2 01/15] ARM: dts: r7s9210: Add USB clock
On Thu, May 09, 2019 at 03:11:28PM -0500, Chris Brandt wrote: > Add USB clock node. If present, this clock input must be 48MHz. > > Signed-off-by: Chris Brandt Thanks, This looks fine to me but I will wait to see if there are other reviews before applying. Reviewed-by: Simon Horman
Re: [PATCH 1/5] USB: serial: fix unthrottle races
On Mon, May 13, 2019 at 01:46:01PM +0200, Johan Hovold wrote:
> On Mon, May 13, 2019 at 12:56:06PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, May 13, 2019 at 12:43:39PM +0200, Johan Hovold wrote:
> > > On Thu, Apr 25, 2019 at 06:05:36PM +0200, Johan Hovold wrote:
> > > > Fix two long-standing bugs which could potentially lead to memory
> > > > corruption or leave the port throttled until it is reopened (on weakly
> > > > ordered systems), respectively, when read-URB completion races with
> > > > unthrottle().
>
> > > > Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs")
> > > > Signed-off-by: Johan Hovold
> > >
> > > Greg, I noticed you added a stable tag to the corresponding cdc-acm fix
> > > and think I should have added on one from the start to this one as well.
> > >
> > > Would you mind queuing this one up for stable?
> > >
> > > Upstream commit 3f5edd58d040bfa4b74fb89bc02f0bc6b9cd06ab.
> >
> > Sure, now queued up for 4.9+
>
> Thanks. The issue has been there since v3.3 so I guess you could queue
> it for all stable trees.
Doesn't apply cleanly for 4.4.y or 3.18.y, so if it's really worth
adding there (and I kind of doubt it), I would need a backport :)
thanks,
greg k-h
Re: [PATCH 1/5] USB: serial: fix unthrottle races
On Mon, May 13, 2019 at 02:51:31PM +0200, Greg Kroah-Hartman wrote: > On Mon, May 13, 2019 at 01:46:01PM +0200, Johan Hovold wrote: > > Thanks. The issue has been there since v3.3 so I guess you could queue > > it for all stable trees. > > Doesn't apply cleanly for 4.4.y or 3.18.y, so if it's really worth > adding there (and I kind of doubt it), I would need a backport :) Ah ok, just wasn't sure why you chose 4.9+. Johan
Re: [REGRESSION] usb: gadget: f_fs: Allow scatter-gather buffers
Hi John, W dniu 09.05.2019 o 23:23, John Stultz pisze: On Thu, May 9, 2019 at 11:25 AM John Stultz wrote: On Thu, May 9, 2019 at 7:02 AM Andrzej Pietrasiewicz wrote: Ok. Apologies for earlier confusion. So the kzalloc/memset fix you sent for f_fs.c does seem to avoid the crash on bootup I was seeing w/ HiKey/dwc2 (previously I had only tested it on HiKey960/dwc3). However with that patch, I still see tranfer problems with adb, unless I comment out setting sg_supported in dwc2/gadget.c (in the same fashion I have to with HiKey960/dwc3). The dwc2 zlp patch doesn't seem to affect things much either way in my testing. But maybe I'm just not tripping on that issue yet. So yes, the kzalloc/memset patch is a clear improvement, as it avoids the bootup crash on dwc2, and seems like it should go in. However, there is still the outstanding issue w/ functionfs sg support stalling on larger transfers. Do you get "functionfs read size 512 > requested size 24, splitting request into multiple reads" message when problems happen? Is there anything in the kernel log? I'm unable to reproduce your problems. I thought I was able, but it was another problem, which is fixed with: 5acb4b970184d189d901192d075997c933b82260 dwc2: gadget: Fix completed transfer size calculation in DDMA (or you can simply take upstream drivers/usb/dwc2). Do your problems happen on dwc2 or dwc3? Is there a way to try your adb without building and running the whole Android? Andrzej
RE: [PATCH v2 3/3] usb: dwc3: gadget: Add support for disabling U1 and U2 entries
Hi Thinh,
>-Original Message-
>From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com]
>Sent: Saturday, May 11, 2019 7:18 AM
>To: Anurag Kumar Vulisha ; Thinh Nguyen
>; Greg Kroah-Hartman
>; Rob Herring ; Mark Rutland
>; Felipe Balbi ; Claus H. Stovgaard
>
>Cc: linux-usb@vger.kernel.org; devicet...@vger.kernel.org; linux-
>ker...@vger.kernel.org; v.anuragku...@gmail.com
>Subject: Re: [PATCH v2 3/3] usb: dwc3: gadget: Add support for disabling U1
>and U2
>entries
>
>Hi,
>
>Anurag Kumar Vulisha wrote:
>> Hi Thinh,
>>
>>> -Original Message-
>>> From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com]
>>> Sent: Friday, May 10, 2019 5:30 AM
>>> To: Anurag Kumar Vulisha ; Thinh Nguyen
>>> ; Greg Kroah-Hartman
>>> ; Rob Herring ; Mark Rutland
>>> ; Felipe Balbi ; Claus H. Stovgaard
>>>
>>> Cc: linux-usb@vger.kernel.org; devicet...@vger.kernel.org; linux-
>>> ker...@vger.kernel.org; v.anuragku...@gmail.com
>>> Subject: Re: [PATCH v2 3/3] usb: dwc3: gadget: Add support for disabling U1
>>> and
>U2
>>> entries
>>>
>>> Hi Anurag,
>>>
>>> Anurag Kumar Vulisha wrote:
>> +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
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index e293400..f2d3112 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2073,6 +2073,24 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>> return 0;
>> }
>>
>> +static void dwc3_gadget_config_params(struct usb_gadget *g,
>> + struct usb_dcd_config_params
>> *params)
>> +{
>> +struct dwc3 *dwc = gadget_to_dwc(g);
>> +
>> +/* U1 Device exit Latency */
>> +if (dwc->dis_u1_entry_quirk)
>> +params->bU1devExitLat = 0;
> It doesn't make sense to have exit latency of 0. Rejecting
> SET_FEATURE(enable U1/U2) should already let the host know that the
> device doesn't support U1/U2.
>
I am okay to remove this, but I feel that it is better to report zero
value instead
of a non-zero value in exit latency of BOS when U1 or U2 entries are not
>supported.
Advantage of reporting 0 is that some hosts doesn't even send
>>> SET_FEATURE(U1/U2)
requests on seeing zero value in BOS descriptor. Also there can be cases
where
>U1 is
disabled and U2 entry is allowed or vice versa, for these kind of cases
the driver
>can
set zero exit latency value for U1 and non-zero exit latency value for U2
. Based
>on
>>> this
I think it would be better to report 0 when U1/U2 states are not enabled.
Please
>>> provide
your opinion on this.
>>> Hm... I assume you're testing against linux usb stack and xhci host. If
>>> that's the case, it looks like host will still request the device to
>>> enter U1/U2 despite the device rejecting SET_FEATURE(enable U1/U2). This
>>> needs to be fixed. I think what you have is fine to workaround this issue.
>> Thanks . Will send the next series with the other fixes that you have
>> suggested
>>
>> Best Regards,
>> Anurag Kumar Vulisha
>>
>
>I want to try something. Can you see if this helps with your performance
>test without setting the U1/U2 exit latency to 0?
>(No need to change what you have in your patch. This is just for testing).
>
With your patch , the link doesn't enter into U1/U2 and I am also getting
better performance
Thanks,
Anurag Kumar Vulisha
>diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>index 2f94568ba385..619351c581cf 100644
>--- a/drivers/usb/core/hub.c
>+++ b/drivers/usb/core/hub.c
>@@ -4057,8 +4057,18 @@ static void usb_enable_link_state(struct usb_hcd
>*hcd, struct usb_device *udev,
>/* Only a configured device will accept the Set Feature
> * U1/U2_ENABLE
> */
>- if (udev->actconfig)
>- usb_set_device_initiated_lpm(udev, state, true);
>+ if (udev->actconfig) {
>+ if (usb_set_device_initiated_lpm(udev, state,
>true)) {
>+ /*
>+* Don't request U1/U2 entry if the device
>+* cannot enable U1/U2.
>+*/
>+ usb_set_lpm_timeout(udev, state, 0);
>+
>hcd->driver->disable_usb3_lpm_timeout(hcd, udev,
>+
>state);
>+ return;
>+ }
>+ }
>
>/* As soon as usb_set_lpm_timeout(timeout) returns 0, the
> * hub-initiated LPM is enabled. Thus, LPM is enabled no
>
>
Re: KASAN: slab-out-of-bounds Write in usb_get_bos_descriptor
On Fri, 10 May 2019, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:43151d6c usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=124794d8a0
> kernel config: https://syzkaller.appspot.com/x/.config?x=4183eeef650d1234
> dashboard link: https://syzkaller.appspot.com/bug?extid=71f1e64501a309fcc012
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=176a53d0a0
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+71f1e64501a309fcc...@syzkaller.appspotmail.com
>
> usb 1-1: Using ep0 maxpacket: 8
> ==
> BUG: KASAN: slab-out-of-bounds in usb_get_bos_descriptor+0x8be/0x8fb
> drivers/usb/core/config.c:976
> Write of size 1 at addr 8880a48e38ec by task kworker/0:2/533
Insufficient descriptor size check.
Alan Stern
#syz test: https://github.com/google/kasan.git usb-fuzzer
drivers/usb/core/config.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: usb-devel/drivers/usb/core/config.c
===
--- usb-devel.orig/drivers/usb/core/config.c
+++ usb-devel/drivers/usb/core/config.c
@@ -932,8 +932,8 @@ int usb_get_bos_descriptor(struct usb_de
/* Get BOS descriptor */
ret = usb_get_descriptor(dev, USB_DT_BOS, 0, bos, USB_DT_BOS_SIZE);
- if (ret < USB_DT_BOS_SIZE) {
- dev_err(ddev, "unable to get BOS descriptor\n");
+ if (ret < USB_DT_BOS_SIZE || bos->bLength < USB_DT_BOS_SIZE) {
+ dev_err(ddev, "unable to get BOS descriptor or descriptor too
short\n");
if (ret >= 0)
ret = -ENOMSG;
kfree(bos);
Unitialisation of SOC USB pads
Dear Thibaut and others, can you please explain who must configure AT91SAM9G20 SOC pads to operate as USB Host port - AT91Bootstrap, U-Boot bootloader, Linux kernel or this is not required at all? I ask, because during connection of USB disk, my board complains usb 1-1: device descriptor read/64, error -62 usb 1-1: device descriptor read/64, error -62 usb 1-1: device descriptor read/64, error -62 usb 1-1: device descriptor read/64, error -62 usb 1-1: device not accepting address 4, error -62 usb 1-1: device not accepting address 5, error -62 usb usb1-port1: unable to enumerate USB device Looks like there is no connectivity between USB Host module of SOC and USB device. Or am I wrong? My setup is: * AT91SAM9G20 based custom board; * Linux kernel 4.9.36, from LINUX4SAM project. * USB disk connected to USB Host port 0 (HDPA, HDMA pins of SOC). 39 Ohm series resistors and 15 Kohm pull-down resistors added at these lines. Connectivity between SOC and USB device confirmed by Ohmmeter. * USB_VBUS voltage measured at USB connector = 4.96 VDC. Best wishes -- Igor Plyatov
Re: KASAN: slab-out-of-bounds Write in usb_get_bos_descriptor
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+71f1e64501a309fcc...@syzkaller.appspotmail.com Tested on: commit: 43151d6c usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer kernel config: https://syzkaller.appspot.com/x/.config?x=4183eeef650d1234 compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=15912100a0 Note: testing is done by a robot and is best-effort only.
Initialisation of SOC USB pads
Dear developers, can you please explain who must configure AT91SAM9G20 SOC pads to operate as USB Host port? Is it AT91Bootstrap, U-Boot bootloader, Linux kernel or this is not required at all? I ask, because during connection of USB disk, my board complains usb 1-1: device descriptor read/64, error -62 usb 1-1: device descriptor read/64, error -62 usb 1-1: device descriptor read/64, error -62 usb 1-1: device descriptor read/64, error -62 usb 1-1: device not accepting address 4, error -62 usb 1-1: device not accepting address 5, error -62 usb usb1-port1: unable to enumerate USB device Looks like there is no connectivity between USB Host module of SOC and USB device. Or am I wrong? My setup is: * AT91SAM9G20 based custom board; * Linux kernel 4.9.36, from LINUX4SAM project. * USB disk connected to USB Host port 0 (HDPA, HDMA pins of SOC). 39 Ohm series resistors and 15 Kohm pull-down resistors added at these lines. Connectivity between SOC and USB device confirmed by Ohmmeter. * USB_VBUS voltage measured at USB connector = 4.96 VDC. Best wishes -- Igor Plyatov
Re: Initialisation of SOC USB pads
On Mon, 13 May 2019, Igor Plyatov wrote: > Dear developers, > > can you please explain who must configure AT91SAM9G20 SOC pads to > operate as USB Host port? Is it AT91Bootstrap, U-Boot bootloader, Linux > kernel or this is not required at all? > > I ask, because during connection of USB disk, my board complains > > usb 1-1: device descriptor read/64, error -62 > usb 1-1: device descriptor read/64, error -62 > usb 1-1: device descriptor read/64, error -62 > usb 1-1: device descriptor read/64, error -62 > usb 1-1: device not accepting address 4, error -62 > usb 1-1: device not accepting address 5, error -62 > usb usb1-port1: unable to enumerate USB device > > > Looks like there is no connectivity between USB Host module of SOC and > USB device. Or am I wrong? > > > My setup is: > > > * AT91SAM9G20 based custom board; > * Linux kernel 4.9.36, from LINUX4SAM project. > * USB disk connected to USB Host port 0 (HDPA, HDMA pins of SOC). 39 Ohm > series resistors and 15 Kohm pull-down resistors added at these lines. > Connectivity between SOC and USB device confirmed by Ohmmeter. > * USB_VBUS voltage measured at USB connector = 4.96 VDC. You probably should ask somebody at the LINUX4SAM project. Alan Stern
[PATCH] USB: serial: option: add support for Simcom SIM7500/SIM7600 RNDIS mode
Added IDs for Simcom SIM7500/SIM7600 series cellular module in RNDIS
mode. Reserved the interface for ADB.
T: Bus=03 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 7 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1e0e ProdID=9011 Rev=03.18
S: Manufacturer=SimTech, Incorporated
S: Product=SimTech, Incorporated
S: SerialNumber=0123456789ABCDEF
C: #Ifs= 8 Cfg#= 1 Atr=a0 MxPwr=500mA
I: If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=02 Prot=ff
Driver=rndis_host
I: If#=0x1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00
Driver=rndis_host
I: If#=0x2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
I: If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#=0x5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#=0x6 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#=0x7 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)
Signed-off-by: Jörgen Storvist
---
drivers/usb/serial/option.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 83869065b802..3fa253984fe7 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1772,6 +1772,8 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(ALINK_VENDOR_ID, SIMCOM_PRODUCT_SIM7100E),
.driver_info = RSVD(5) | RSVD(6) },
{ USB_DEVICE_INTERFACE_CLASS(0x1e0e, 0x9003, 0xff) }, /* Simcom
SIM7500/SIM7600 MBIM mode */
+ { USB_DEVICE_INTERFACE_CLASS(0x1e0e, 0x9011, 0xff),
+ .driver_info = RSVD(7) }, /* Simcom SIM7500/SIM7600 RNDIS mode */
{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_X060S_X200),
.driver_info = NCTRL(0) | NCTRL(1) | RSVD(4) },
{ USB_DEVICE(ALCATEL_VENDOR_ID, ALCATEL_PRODUCT_X220_X500D),
--
2.21.0
[PATCH] USB: Fix slab-out-of-bounds write in usb_get_bos_descriptor
The syzkaller USB fuzzer found a slab-out-of-bounds write bug in the
USB core, caused by a failure to check the actual size of a BOS
descriptor. This patch adds a check to make sure the descriptor is at
least as large as it is supposed to be, so that the code doesn't
inadvertently access memory beyond the end of the allocated region
when assigning to dev->bos->desc->bNumDeviceCaps later on.
Signed-off-by: Alan Stern
Reported-and-tested-by: syzbot+71f1e64501a309fcc...@syzkaller.appspotmail.com
CC:
---
[as1898]
drivers/usb/core/config.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: usb-devel/drivers/usb/core/config.c
===
--- usb-devel.orig/drivers/usb/core/config.c
+++ usb-devel/drivers/usb/core/config.c
@@ -932,8 +932,8 @@ int usb_get_bos_descriptor(struct usb_de
/* Get BOS descriptor */
ret = usb_get_descriptor(dev, USB_DT_BOS, 0, bos, USB_DT_BOS_SIZE);
- if (ret < USB_DT_BOS_SIZE) {
- dev_err(ddev, "unable to get BOS descriptor\n");
+ if (ret < USB_DT_BOS_SIZE || bos->bLength < USB_DT_BOS_SIZE) {
+ dev_err(ddev, "unable to get BOS descriptor or descriptor too
short\n");
if (ret >= 0)
ret = -ENOMSG;
kfree(bos);
Re: [v3 PATCH] dt-binding: usb: add usb-role-switch property
On Wed, 8 May 2019 17:17:44 +0800, Chunfeng Yun wrote: > Add a property usb-role-switch to tell the driver that use > USB Role Switch framework to handle the role switch, > it's useful when the driver has already supported other ways, > such as extcon framework etc. > > Cc: Biju Das > Cc: Yu Chen > Signed-off-by: Chunfeng Yun > --- > v3: > add property type, modify description suggested by Heikki > > v2: > describe it in terms of h/w functionality suggested by Rob > > v1: > the property is discussed in: > [v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property > https://patchwork.kernel.org/patch/10852497/ > > Mediatek and Hisilicon also try to use it: > [v4,3/6] dt-bindings: usb: mtu3: add properties about USB Role Switch > https://patchwork.kernel.org/patch/10918385/ > [v4,6/6] usb: mtu3: register a USB Role Switch for dual role mode > https://patchwork.kernel.org/patch/10918367/ > > [v6,10/13] usb: dwc3: Registering a role switch in the DRD code > https://patchwork.kernel.org/patch/10909981/ > --- > Documentation/devicetree/bindings/usb/generic.txt | 4 > 1 file changed, 4 insertions(+) > Reviewed-by: Rob Herring
Re: [PATCH v3 1/3] doc: dt: bindings: usb: dwc3: Update entries for disabling U1 and U2
On Fri, 10 May 2019 12:37:26 +0530, Anurag Kumar Vulisha wrote: > This patch updates the documentation with the information related > to the quirks that needs to be added for disabling the link entering > into the U1 and U2 states > > Signed-off-by: Anurag Kumar Vulisha > --- > Changes in v3: > -None > > Changes in v2 > 1. As suggested by Thinh Nguyen changed the "snps,dis_u1_entry_quirk" > to "snps,dis-u1-entry-quirk" > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Rob Herring
Re: [REGRESSION] usb: gadget: f_fs: Allow scatter-gather buffers
On Mon, May 13, 2019 at 7:08 AM Andrzej Pietrasiewicz wrote: > W dniu 09.05.2019 o 23:23, John Stultz pisze: > > So yes, the kzalloc/memset patch is a clear improvement, as it avoids > > the bootup crash on dwc2, and seems like it should go in. > > > > However, there is still the outstanding issue w/ functionfs sg > > support stalling on larger transfers. > > Do you get "functionfs read size 512 > requested size 24, splitting > request into multiple reads" message when problems happen? Unfortunately no. > Is there anything in the kernel log? Nope. Just the transfers stall/hang and further attempts at adb connections fail until the USB cable is unplugged and replugged. > I'm unable to reproduce your problems. I thought I was able, but > it was another problem, which is fixed with: > > 5acb4b970184d189d901192d075997c933b82260 > dwc2: gadget: Fix completed transfer size calculation in DDMA > > (or you can simply take upstream drivers/usb/dwc2). Yea, I'm able to test against mainline. I can give this a whirl, but since it affects multiple drivers, I suspect the trouble is in the f_fs code, not just dwc2. > Do your problems happen on dwc2 or dwc3? The transfer hangs happen on *both* dwc2 and dwc3. And on both we can use a similar workaround of disabling sg_supported to get by. https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey-mainline-WIP&id=21dfaac615f1f287377897804cbfe69450d489e3 https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=5b70ec4ae1c06ae700fcca7141130c71e205fa1c > Is there a way to try your adb without building and running the > whole Android? Maybe? I have heard of folks doing it in the past, though I don't really know a quick path to get there. Is there anything else I can try for you? thanks -john
Re: [PATCH] USB: Fix slab-out-of-bounds write in usb_get_bos_descriptor
On Mon, May 13, 2019 at 01:14:29PM -0400, Alan Stern wrote:
> The syzkaller USB fuzzer found a slab-out-of-bounds write bug in the
> USB core, caused by a failure to check the actual size of a BOS
> descriptor. This patch adds a check to make sure the descriptor is at
> least as large as it is supposed to be, so that the code doesn't
> inadvertently access memory beyond the end of the allocated region
> when assigning to dev->bos->desc->bNumDeviceCaps later on.
>
> Signed-off-by: Alan Stern
> Reported-and-tested-by: syzbot+71f1e64501a309fcc...@syzkaller.appspotmail.com
> CC:
>
> ---
>
>
> [as1898]
>
>
> drivers/usb/core/config.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: usb-devel/drivers/usb/core/config.c
> ===
> --- usb-devel.orig/drivers/usb/core/config.c
> +++ usb-devel/drivers/usb/core/config.c
> @@ -932,8 +932,8 @@ int usb_get_bos_descriptor(struct usb_de
>
> /* Get BOS descriptor */
> ret = usb_get_descriptor(dev, USB_DT_BOS, 0, bos, USB_DT_BOS_SIZE);
> - if (ret < USB_DT_BOS_SIZE) {
> - dev_err(ddev, "unable to get BOS descriptor\n");
> + if (ret < USB_DT_BOS_SIZE || bos->bLength < USB_DT_BOS_SIZE) {
> + dev_err(ddev, "unable to get BOS descriptor or descriptor too
> short\n");
Nice fix, I thought we had found all of these the last time we fuzzed
this area :)
I'll queue this up once 5.2-rc1 is out, thanks.
greg k-h
RE: [PATCH v2 04/15] dt-bindings: rcar-gen3-phy-usb2: Document use of usb_x1
Hi Geert and Shimoda-san,
On Fri, May 10, 2019, Geert Uytterhoeven wrote:
> > I think we can reuse it like below:
> >
> > - clock-names: Name of the clocks. This property is model-dependent.
> > - R-Car Gen3 SoCs use a single functional clock. The clock doesn't
> need to be
> > named.
> > - RZ/A2 uses a single functional clock as a separate dedicated 48MHz
>
> and a separate?
>
> > USB_X1 input. So, the functional clock must be named "???" and
> > the USB_X1 input must be named as "usb_x1".
> >
> > What do you think? I'm not sure how to be named the functional clock so
> that
> > the sample is named as "???".
>
> We typically use "fclk" for the functional clock's name.
Just to make sure I'm following this, here is what you are asking for:
[r7s9210.dtsi]
usb2_phy1: usb-phy@e821a200 {
compatible = "renesas,usb2-phy-r7s9210",
"renesas,rcar-gen3-usb2-phy";
reg = <0xe821a200 0x10>;
interrupts = ;
+ clocks = <&cpg CPG_MOD 60>, <&usb_x1_clk>;
+ clock-names = "fclk", "usb_x1";
power-domains = <&cpg>;
#phy-cells = <0>;
status = "disabled";
[phy-rcar-gen3-usb2.c]
usb_x1_clk = devm_clk_get(dev, "usb_x1");
if (!IS_ERR(usb_x1_clk))
if (clk_get_rate(usb_x1_clk))
channel->uses_usb_x1 = true;
And then document this in the bindings, saying that clock-names is
option if there is only 1 clock (to be backward compatible with existing
Device Trees.
Is this correct?
Thanks,
Chris
Re: [PATCH v2 04/15] dt-bindings: rcar-gen3-phy-usb2: Document use of usb_x1
Hi Chris,
On Mon, May 13, 2019 at 11:07 PM Chris Brandt wrote:
> On Fri, May 10, 2019, Geert Uytterhoeven wrote:
> > > I think we can reuse it like below:
> > >
> > > - clock-names: Name of the clocks. This property is model-dependent.
> > > - R-Car Gen3 SoCs use a single functional clock. The clock doesn't
> > need to be
> > > named.
> > > - RZ/A2 uses a single functional clock as a separate dedicated 48MHz
> >
> > and a separate?
> >
> > > USB_X1 input. So, the functional clock must be named "???" and
> > > the USB_X1 input must be named as "usb_x1".
> > >
> > > What do you think? I'm not sure how to be named the functional clock so
> > that
> > > the sample is named as "???".
> >
> > We typically use "fclk" for the functional clock's name.
>
>
> Just to make sure I'm following this, here is what you are asking for:
>
> [r7s9210.dtsi]
>
> usb2_phy1: usb-phy@e821a200 {
> compatible = "renesas,usb2-phy-r7s9210",
> "renesas,rcar-gen3-usb2-phy";
> reg = <0xe821a200 0x10>;
> interrupts = ;
> + clocks = <&cpg CPG_MOD 60>, <&usb_x1_clk>;
> + clock-names = "fclk", "usb_x1";
> power-domains = <&cpg>;
> #phy-cells = <0>;
> status = "disabled";
>
>
> [phy-rcar-gen3-usb2.c]
> usb_x1_clk = devm_clk_get(dev, "usb_x1");
> if (!IS_ERR(usb_x1_clk)))
> if (clk_get_rate(usb_x1_clk))
if (!IS_ERR(usb_x1_clk) && clk_get_rate(usb_x1_clk))
> channel->uses_usb_x1 = true;
>
>
> And then document this in the bindings, saying that clock-names is
> option if there is only 1 clock (to be backward compatible with existing
optional
> Device Trees.
>
> Is this correct?
Exactly!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RE: [PATCH v2 04/15] dt-bindings: rcar-gen3-phy-usb2: Document use of usb_x1
Hi Geert, Thank you for the quick reply. On Mon, May 13, 2019, Geert Uytterhoeven wrote: > > [phy-rcar-gen3-usb2.c] > > usb_x1_clk = devm_clk_get(dev, "usb_x1"); > > if (!IS_ERR(usb_x1_clk))) > > if (clk_get_rate(usb_x1_clk)) > > if (!IS_ERR(usb_x1_clk) && clk_get_rate(usb_x1_clk)) :) > > Is this correct? > > Exactly! Thank you! #I'm trying to avoid a v4 Chris
[BUG REPORT] usb: dwc3: "failed to enable ep0out" when enabling mass storage mode
Hi Felipe, I'm picking up a bug my coworker Rob touched on in this thread: https://marc.info/?l=linux-usb&m=155349928622570&w=2 We occasionally see the following dmesg when enabling mass storage mode: dwc3 dwc3.1.auto: failed to enable ep0out To reproduce after a clean boot: Enable mass storage mode Disable mass storage mode Enable mass storage mode I don't need to plug any devices, just switch modes. The error does not happen every boot. If I don't get the error on that second enable, then as far as I can tell I won't get the error at all during that boot. I've attached the trace and regdump. When capturing these I was running a 4.9.115 kernel and using the g_mass_storage driver for simplicity. Here is the shell session: root@gnarbox-2:~# echo device > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role && modprobe g_mass_storage file=/dev/nvme0n1p7 iSerialNumber=90405 [ 118.627628] Mass Storage Function, version: 2009/09/11 [ 118.633426] LUN: removable file: (no medium) [ 118.638283] LUN: file: /dev/nvme0n1p7 [ 118.642397] Number of LUNs=1 [ 118.646080] g_mass_storage gadget: Mass Storage Gadget, version: 2009/09/11 [ 118.653902] g_mass_storage gadget: g_mass_storage ready root@gnarbox-2:~# modprobe -r g_mass_storage && echo host > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role root@gnarbox-2:~# echo device > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role && modprobe g_mass_storage file=/dev/nvme0n1p7 iSerialNumber=90405 [ 123.416789] Mass Storage Function, version: 2009/09/11 [ 123.422546] LUN: removable file: (no medium) [ 123.427386] LUN: file: /dev/nvme0n1p7 [ 123.431531] Number of LUNs=1 [ 123.435278] g_mass_storage gadget: Mass Storage Gadget, version: 2009/09/11 [ 123.443168] g_mass_storage gadget: g_mass_storage ready [ 123.451998] dwc3 dwc3.1.auto: failed to enable ep0out root@gnarbox-2:~# I still get the error when using the gadget configfs instead of g_mass_storage. I still get the error on kernel 5.0.5. Do you have any idea what's happening here? What steps can I take to help debug this? Thanks, Evan dwc3-trace-20190513.txz Description: application/xz
Lack of length checking in USB configuration may allow buffer overflow
Hey All,
I was seeing a linux VM crash due to malformed USB configuration
payloads being malformed. I'm testing this patch now which should
provide better security checking (but this is my first patch so be
kind if I have things wrong.)
R
>From d7b0dd52f3b3b38126504b17d2d9c9ceaa572edf Mon Sep 17 00:00:00 2001
From: Rick Mark
Date: Mon, 13 May 2019 19:06:46 -0700
Subject: [PATCH] Security checks in USB configurations
---
drivers/usb/core/config.c | 67 +++
1 file changed, 67 insertions(+)
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 7b5cb28ff..8cb9a136e 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -33,6 +33,13 @@ static int find_next_descriptor(unsigned char
*buffer, int size,
/* Find the next descriptor of type dt1 or dt2 */
while (size > 0) {
+ if (size < sizeof(struct usb_descriptor_header)) {
+ printk( KERN_ERR "usb config: find_next_descriptor "
+ "with size %d not sizeof("
+ "struct usb_descriptor_header)", size );
+ break;
+ }
+
h = (struct usb_descriptor_header *) buffer;
if (h->bDescriptorType == dt1 || h->bDescriptorType == dt2)
break;
@@ -58,6 +65,13 @@ static void
usb_parse_ssp_isoc_endpoint_companion(struct device *ddev,
* The SuperSpeedPlus Isoc endpoint companion descriptor immediately
* follows the SuperSpeed Endpoint Companion descriptor
*/
+ if (size < sizeof(struct usb_ssp_isoc_ep_comp_descriptor)) {
+dev_warn(ddev, "Invalid size %d for SuperSpeedPlus isoc
endpoint companion"
+ "for config %d interface %d altsetting %d ep %d.\n",
+ size, cfgno, inum, asnum, ep->desc.bEndpointAddress);
+return;
+ }
+
desc = (struct usb_ssp_isoc_ep_comp_descriptor *) buffer;
if (desc->bDescriptorType != USB_DT_SSP_ISOC_ENDPOINT_COMP ||
size < USB_DT_SSP_ISOC_EP_COMP_SIZE) {
@@ -76,6 +90,14 @@ static void usb_parse_ss_endpoint_companion(struct
device *ddev, int cfgno,
struct usb_ss_ep_comp_descriptor *desc;
int max_tx;
+ if (size < sizeof(struct usb_ss_ep_comp_descriptor)) {
+dev_warn(ddev, "Invalid size %d of SuperSpeed endpoint"
+ " companion for config %d "
+ " interface %d altsetting %d: "
+ "using minimum values\n",
+ size, cfgno, inum, asnum);
+return;
+ }
/* The SuperSpeed endpoint companion descriptor is supposed to
* be the first thing immediately following the endpoint descriptor.
*/
@@ -103,6 +125,16 @@ static void
usb_parse_ss_endpoint_companion(struct device *ddev, int cfgno,
ep->desc.wMaxPacketSize;
return;
}
+
+ if ((size - desc->bLength) < 0 ||
+ size < USB_DT_SS_EP_COMP_SIZE) {
+dev_warn(ddev, "Control endpoint with bMaxBurst = %d in "
+ "config %d interface %d altsetting %d ep %d: "
+ "has invalid bLength %d vs size %d\n", desc->bMaxBurst,
+ cfgno, inum, asnum, ep->desc.bEndpointAddress,
desc->bLength, size);
+return;
+ }
+
buffer += desc->bLength;
size -= desc->bLength;
memcpy(&ep->ss_ep_comp, desc, USB_DT_SS_EP_COMP_SIZE);
@@ -214,7 +246,24 @@ static int usb_parse_endpoint(struct device
*ddev, int cfgno, int inum,
unsigned int maxp;
const unsigned short *maxpacket_maxes;
+ if (size < sizeof(struct usb_endpoint_descriptor)) {
+dev_warn(ddev, "config %d interface %d altsetting %d has an "
+ "size %d smaller then endpoint descriptor, skipping\n",
+ cfgno, inum, asnum, size);
+
+return -EINVAL;
+ }
+
d = (struct usb_endpoint_descriptor *) buffer;
+
+ if ((size - d->bLength) < 0) {
+dev_warn(ddev, "config %d interface %d altsetting %d has an "
+ "invalid endpoint descriptor of length %d, skipping\n",
+ cfgno, inum, asnum, d->bLength);
+
+return -EINVAL;
+ }
+
buffer += d->bLength;
size -= d->bLength;
@@ -446,7 +495,18 @@ static int usb_parse_interface(struct device
*ddev, int cfgno,
int len, retval;
int num_ep, num_ep_orig;
+ if (size < sizeof(struct usb_interface_descriptor)) {
+dev_err(ddev, "config %d interface %d has an "
+ "invalid endpoint descriptor of length %d, skipping\n",
+ cfgno, inum, size);
+}
d = (struct usb_interface_descriptor *) buffer;
+
+ if ((size - d->bLength) < 0) {
+dev_err(ddev, "config %d interface %d has an "
+ "invalid endpoint descriptor of length %d, skipping\n",
+ cfgno, inum, d->bLength);
+ }
buffer += d->bLength;
size -= d->bLength;
@@ -514,6 +574,13 @@ static int usb_parse_interface(struct device
*ddev, int cfgno,
/* Parse all the endpoint descriptors */
n = 0;
while (size > 0) {
+ if (size < sizeof(struct usb_descriptor_header)) {
+dev_err(ddev, "config %d i
[PATCH] USB: serial: pl2303: add Allied Telesis VT-Kit3
This is adds the vendor and device id for the AT-VT-Kit3 which is a
pl2303-based device.
Signed-off-by: Chris Packham
---
drivers/usb/serial/pl2303.c | 1 +
drivers/usb/serial/pl2303.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 55122ac84518..d7abde14b3cf 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -106,6 +106,7 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(SANWA_VENDOR_ID, SANWA_PRODUCT_ID) },
{ USB_DEVICE(ADLINK_VENDOR_ID, ADLINK_ND6530_PRODUCT_ID) },
{ USB_DEVICE(SMART_VENDOR_ID, SMART_PRODUCT_ID) },
+ { USB_DEVICE(AT_VENDOR_ID, AT_VTKIT3_PRODUCT_ID) },
{ } /* Terminating entry */
};
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 559941ca884d..b0175f17d1a2 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -155,3 +155,6 @@
#define SMART_VENDOR_ID0x0b8c
#define SMART_PRODUCT_ID 0x2303
+/* Allied Telesis VT-Kit3 */
+#define AT_VENDOR_ID 0x0caa
+#define AT_VTKIT3_PRODUCT_ID 0x3001
--
2.21.0
