Re: [PATCH] usb: Kconfig: make USB_ULPI_BUS select USB_COMMON
On Tuesday, September 13, 2016 9:48:55 AM CEST Peter Chen wrote:
> On Mon, Sep 12, 2016 at 05:36:23PM +0200, Arnd Bergmann wrote:
> > Moving the CONFIG_USB_ULPI_BUS option to the top-level Kconfig file
> > means that we can enable it without any of the other USB support,
> > leading to a build error because Kbuild never enters the
> > drivers/usb/common/ directory without CONFIG_USB_COMMON:
> >
> > ERROR: "ulpi_unregister_driver" [drivers/phy/phy-tusb1210.ko] undefined!
> > ERROR: "__ulpi_register_driver" [drivers/phy/phy-tusb1210.ko] undefined!
> > ERROR: "ulpi_write" [drivers/phy/phy-tusb1210.ko] undefined!
> >
>
> Thanks, Arnd. I have submitted the fix.
>
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=6406c3d226374c28d452b11db3b5ac241ce26191
I see you used 'depends on USB_COMMON' though instead of 'select USB_COMMON'.
This seems inconsistent with how we handle the other places after the patch
I did earlier this year [see below].
It probably works fine for now, but having a mix of 'depends on' and 'select'
can be confusing for users as well as create dependency loops that break
the build.
Arnd
commit badf6d47f8a93098c6e05fdeb735b44b61877451
Author: Arnd Bergmann
Date: Wed Mar 23 17:45:08 2016 +0100
usb: common: rework CONFIG_USB_COMMON logic
The phy-am335x driver selects 'USB_COMMON', but all other drivers
use 'depends on' for that symbol, and it depends on USB || USB_GADGET
itself, which causes a Kconfig warning:
warning: (AM335X_PHY_USB) selects USB_COMMON which has unmet direct
dependencies (USB_SUPPORT && (USB || USB_GADGET))
As suggested by Felipe Balbi, this turns the logic around, and makes
'USB_COMMON' selected by everything else that needs it, so we can
remove the dependencies.
Fixes: 59f042f644c5 ("usb: phy: phy-am335x: bypass first VBUS sensing for
host-only mode")
Signed-off-by: Arnd Bergmann
Acked-by: Felipe Balbi
Reviewed-by: Peter Chen
Signed-off-by: Felipe Balbi
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 26566db09de0..e92b97cd6056 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -250,7 +250,8 @@ config PHY_SUN9I_USB
tristate "Allwinner sun9i SoC USB PHY driver"
depends on ARCH_SUNXI && HAS_IOMEM && OF
depends on RESET_CONTROLLER
- depends on USB_COMMON
+ depends on USB_SUPPORT
+ select USB_COMMON
select GENERIC_PHY
help
Enable this to support the transceiver that is part of Allwinner
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451dd651e..8689dcba5201 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -31,8 +31,6 @@ if USB_SUPPORT
config USB_COMMON
tristate
- default y
- depends on USB || USB_GADGET
config USB_ARCH_HAS_HCD
def_bool y
@@ -41,6 +39,7 @@ config USB_ARCH_HAS_HCD
config USB
tristate "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
+ select USB_COMMON
select NLS # for UTF-8 strings
---help---
Universal Serial Bus (USB) is a specification for a serial bus
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index af5d922a8f5d..2057add439f0 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -15,6 +15,7 @@
menuconfig USB_GADGET
tristate "USB Gadget Support"
+ select USB_COMMON
select NLS
help
USB is a master/slave protocol, organized with one master
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 22/22] phy: Add support for Qualcomm's USB HS phy
On Wed, Sep 07, 2016 at 02:35:19PM -0700, Stephen Boyd wrote:
> The high-speed phy on qcom SoCs is controlled via the ULPI
> viewport.
>
Hi Stephen, I am a little puzzled how this driver co-work with chipidea
driver. According to nxp IC guys, the ULPI PHY's clock needs to be enabled
before access portsc.pts (calling hw_phymode_configure), otherwise,
the system will hang. But I find you call hw_phymode_configure before
phy->power_on, doesn't your design have this requirement?
Besides, you read ulpi id before phy->power_on, how can read work before
phy power on?
Peter
> Cc: Kishon Vijay Abraham I
> Cc:
> Signed-off-by: Stephen Boyd
> ---
> .../devicetree/bindings/phy/qcom,usb-hs-phy.txt| 83 ++
> drivers/phy/Kconfig| 8 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-qcom-usb-hs.c | 289
> +
> 4 files changed, 381 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
> create mode 100644 drivers/phy/phy-qcom-usb-hs.c
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
> b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
> new file mode 100644
> index ..d7eacd63d06b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
> @@ -0,0 +1,83 @@
> +Qualcomm's USB HS PHY
> +
> +PROPERTIES
> +
> +- compatible:
> +Usage: required
> +Value type:
> +Definition: Should contain "qcom,usb-hs-phy" and more specifically one
> of the
> +following:
> +
> +"qcom,usb-hs-phy-apq8064"
> +"qcom,usb-hs-phy-msm8916"
> +"qcom,usb-hs-phy-msm8974"
> +
> +- #phy-cells:
> +Usage: required
> +Value type:
> +Definition: Should contain 0
> +
> +- clocks:
> +Usage: required
> +Value type:
> +Definition: Should contain clock specifier for the reference and sleep
> +clocks
> +
> +- clock-names:
> +Usage: required
> +Value type:
> +Definition: Should contain "ref" and "sleep" for the reference and sleep
> +clocks respectively
> +
> +- resets:
> +Usage: required
> +Value type:
> +Definition: Should contain the phy and POR resets
> +
> +- reset-names:
> +Usage: required
> +Value type:
> +Definition: Should contain "phy" and "por" for the phy and POR resets
> +respectively
> +
> +- v3p3-supply:
> +Usage: required
> +Value type:
> +Definition: Should contain a reference to the 3.3V supply
> +
> +- v1p8-supply:
> +Usage: required
> +Value type:
> +Definition: Should contain a reference to the 1.8V supply
> +
> +- extcon:
> +Usage: optional
> +Value type:
> +Definition: Should contain the vbus and ID extcons in the first and
> second
> +cells respectively
> +
> +- qcom,init-seq:
> +Usage: optional
> +Value type:
> +Definition: Should contain a sequence of ULPI register and address pairs
> to
> +program into the ULPI_EXT_VENDOR_SPECIFIC area. This is
> related
> +to Device Mode Eye Diagram test.
> +
> +EXAMPLE
> +
> +otg: usb-controller {
> + ulpi {
> + phy {
> + compatible = "qcom,usb-hs-phy-msm8974",
> "qcom,usb-hs-phy";
> + #phy-cells = <0>;
> + clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
> + clock-names = "ref", "sleep";
> + resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;
> + reset-names = "phy", "por";
> + v3p3-supply = <&pm8941_l24>;
> + v1p8-supply = <&pm8941_l6>;
> + extcon = <&smbb>, <&usb_id>;
> + qcom,init-seq = /bits/ 8 <0x81 0x63>;
> + };
> + };
> +};
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 830c443eeabf..ee0ec021a98c 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -417,6 +417,14 @@ config PHY_QCOM_UFS
> help
> Support for UFS PHY on QCOM chipsets.
>
> +config PHY_QCOM_USB_HS
> + tristate "Qualcomm USB HS PHY module"
> + depends on USB_ULPI_BUS
> + select GENERIC_PHY
> + help
> + Support for the USB high-speed ULPI compliant phy on Qualcomm
> + chipsets.
> +
> config PHY_QCOM_USB_HSIC
> tristate "Qualcomm USB HSIC ULPI PHY module"
> depends on USB_ULPI_BUS
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 5422f543d17d..31c84faa07fa 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_PHY_STIH41X_USB) +=
> phy-stih41x-usb.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-u
Re: [PATCH] usb: Kconfig: make USB_ULPI_BUS select USB_COMMON
On Tue, Sep 13, 2016 at 09:04:08AM +0200, Arnd Bergmann wrote:
> On Tuesday, September 13, 2016 9:48:55 AM CEST Peter Chen wrote:
> > On Mon, Sep 12, 2016 at 05:36:23PM +0200, Arnd Bergmann wrote:
> > > Moving the CONFIG_USB_ULPI_BUS option to the top-level Kconfig file
> > > means that we can enable it without any of the other USB support,
> > > leading to a build error because Kbuild never enters the
> > > drivers/usb/common/ directory without CONFIG_USB_COMMON:
> > >
> > > ERROR: "ulpi_unregister_driver" [drivers/phy/phy-tusb1210.ko] undefined!
> > > ERROR: "__ulpi_register_driver" [drivers/phy/phy-tusb1210.ko] undefined!
> > > ERROR: "ulpi_write" [drivers/phy/phy-tusb1210.ko] undefined!
> > >
> >
> > Thanks, Arnd. I have submitted the fix.
> >
> > https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=6406c3d226374c28d452b11db3b5ac241ce26191
>
> I see you used 'depends on USB_COMMON' though instead of 'select USB_COMMON'.
>
> This seems inconsistent with how we handle the other places after the patch
> I did earlier this year [see below].
>
> It probably works fine for now, but having a mix of 'depends on' and 'select'
> can be confusing for users as well as create dependency loops that break
> the build.
>
> Arnd
I just see below Kconfig entry at the same Kconfig
(drivers/usb/Kconfig), and forget your changes.
config USB_LED_TRIG
bool "USB LED Triggers"
depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
help
This option adds LED triggers for USB host and/or gadget activity.
Say Y here if you are working on a system with led-class supported
LEDs and you want to use them as activity indicators for USB host or
gadget.
Just grep it, some Kconfig still uses depends on for USB_COMMON, let me
change it together.
b29397@b29397-desktop:~/work/projects/usb$ find . -name Kconfig | xargs
grep -nr "USB_COMMON"
./drivers/phy/Kconfig:274: select USB_COMMON
./drivers/usb/phy/Kconfig:72: select USB_COMMON
./drivers/usb/Kconfig:32:config USB_COMMON
./drivers/usb/Kconfig:42: select USB_COMMON
./drivers/usb/Kconfig:155: depends on LEDS_CLASS && USB_COMMON &&
LEDS_TRIGGERS
./drivers/usb/gadget/Kconfig:18:select USB_COMMON
./drivers/usb/usbip/Kconfig:3: depends on USB_COMMON && NET
Peter
>
> commit badf6d47f8a93098c6e05fdeb735b44b61877451
> Author: Arnd Bergmann
> Date: Wed Mar 23 17:45:08 2016 +0100
>
> usb: common: rework CONFIG_USB_COMMON logic
>
> The phy-am335x driver selects 'USB_COMMON', but all other drivers
> use 'depends on' for that symbol, and it depends on USB || USB_GADGET
> itself, which causes a Kconfig warning:
>
> warning: (AM335X_PHY_USB) selects USB_COMMON which has unmet direct
> dependencies (USB_SUPPORT && (USB || USB_GADGET))
>
> As suggested by Felipe Balbi, this turns the logic around, and makes
> 'USB_COMMON' selected by everything else that needs it, so we can
> remove the dependencies.
>
> Fixes: 59f042f644c5 ("usb: phy: phy-am335x: bypass first VBUS sensing for
> host-only mode")
> Signed-off-by: Arnd Bergmann
> Acked-by: Felipe Balbi
> Reviewed-by: Peter Chen
> Signed-off-by: Felipe Balbi
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 26566db09de0..e92b97cd6056 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -250,7 +250,8 @@ config PHY_SUN9I_USB
> tristate "Allwinner sun9i SoC USB PHY driver"
> depends on ARCH_SUNXI && HAS_IOMEM && OF
> depends on RESET_CONTROLLER
> - depends on USB_COMMON
> + depends on USB_SUPPORT
> + select USB_COMMON
> select GENERIC_PHY
> help
> Enable this to support the transceiver that is part of Allwinner
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8ed451dd651e..8689dcba5201 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -31,8 +31,6 @@ if USB_SUPPORT
>
> config USB_COMMON
> tristate
> - default y
> - depends on USB || USB_GADGET
>
> config USB_ARCH_HAS_HCD
> def_bool y
> @@ -41,6 +39,7 @@ config USB_ARCH_HAS_HCD
> config USB
> tristate "Support for Host-side USB"
> depends on USB_ARCH_HAS_HCD
> + select USB_COMMON
> select NLS # for UTF-8 strings
> ---help---
> Universal Serial Bus (USB) is a specification for a serial bus
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index af5d922a8f5d..2057add439f0 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -15,6 +15,7 @@
>
> menuconfig USB_GADGET
> tristate "USB Gadget Support"
> + select USB_COMMON
> select NLS
> help
> USB is a master/slave protocol, organized with one master
>
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@
Re: [PATCH] usb: Kconfig: make USB_ULPI_BUS select USB_COMMON
On Tuesday, September 13, 2016 3:19:42 PM CEST Peter Chen wrote: > > I just see below Kconfig entry at the same Kconfig > (drivers/usb/Kconfig), and forget your changes. > > config USB_LED_TRIG > bool "USB LED Triggers" > depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS > help > This option adds LED triggers for USB host and/or gadget activity. > > Say Y here if you are working on a system with led-class supported > LEDs and you want to use them as activity indicators for USB host or > gadget. > > Just grep it, some Kconfig still uses depends on for USB_COMMON, let me > change it together. > I suspect this one above should instead depend on "USB_SUPPORT". Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v16 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On Mon, Sep 12 2016, Mark Brown wrote: > [ Unknown signature status ] > On Mon, Sep 12, 2016 at 03:27:18PM +0200, NeilBrown wrote: >> On Mon, Sep 12 2016, Mark Brown wrote: > >> > It's no worse than any other board file situation - if someone has that >> > problem they get to fix it. > >> My point is that the present design does not appear to scale beyond a >> single USB power supply (as if there were two, they would be named in >> discovery order, which is not reliably stable). > > For the practical purposes of people making systems (as opposed to > upstream where this is likely to get most use) it pretty much is. > Though quite how many systems have multiple chargers is itself also a > question. > >> Your point is, I think, that when someone actually cares about that lack >> of scaling, they can fix it. > > Yes. > >> I am perfectly happy with that approach. However if the code doesn't >> scale beyond one charger, it shouldn't pretend that it does. >> i.e. >> Don't have "usb_charger_find_by_name()", just a global "usb_charger" >> (or similar). >> The first charger to register gets to be the "usb_charger". The >> second one gets an error. >> I could be quite happy with that sort of interface. > > Well, a fairly standard way of extending would be to allow the explicit > assignment of names to chargers so this'd avoid such churn. Sure, that might work. I'm just against a design that obviously cannot work. > >> > The whole point from the point of view of the wm831x driver is that it >> > just wants something to tell it how much current it's allowed to draw, I >> > appreciate that doesn't change your analysis of the bit in the middle >> > but the consumer driver bit seems fine here. > >> Yes, the wm831x driver probably does the right thing. >> Other drivers might want to know not only the minimum they are allowed >> to draw, but also the maximum they should try even if they are carefully >> monitoring the voltage. >> So wm831x is doing the right thing with the wrong interface. Maybe you >> can describe that as "fine". > > That's not actually 100% clear to me - for what the wm831x is doing it > probably *does* want the higher limit. This is a system inflow limit > (as it should be for this), at least the charger will adapt to voltage > variations though other users in the system are much less likely to do > so. Interesting ... I hadn't considered that possibility. As long as the current remains below the maximum, the charger will reduce the voltage towards 2V as load increases. Somewhere before it gets there, the system will not be able to make use of the power as the voltage will be too low to be usable. So that will naturally limit the current being drawn. Not having very much electrical engineering background, I cannot say for sure what will happen, but it seems likely that once the voltage drops much below 4.75V, the charger won't be operating at peak efficiency, which would be a waste. I can easily imagine that the hardware would switch off at some voltage level, rather than just making do with what is there. So I'm skeptical of this approach, but I'm open to being corrected by someone more knowledgeable than I. Looking at it from a different perspective, according to the patch set, the limits that wm831x is able to impose are: + 0, + 2, + 100, + 500, + 900, + 1500, + 1800, + 550, These are, from the battery charger spec, minimums rather than maximums. e.g. a CDP provides at least 1500, and as much as 5000. So it seems that the wm831x was designed to be told the minimum guaranteed available. But that is circumstantial evidence and might be misleading. NeilBrown signature.asc Description: PGP signature
Re: [PATCH 1/3] usb: gadgetfs: introduce feature control mechanism
On 09/12/2016 04:11 PM, Alan Stern wrote:
> On Mon, 12 Sep 2016, Binyamin Sharet (bsharet) wrote:
>
>>> On 8 Sep 2016, at 23:24, Alan Stern wrote:
>>>
>>> On Thu, 8 Sep 2016, Binyamin Sharet (bsharet) wrote:
>>>
> On 8 Sep 2016, at 22:20, Alan Stern wrote:
>
> On Thu, 8 Sep 2016, Binyamin Sharet (bsharet) wrote:
>
>>> I was thinking more like:
>>>
>>> struct usb_gadgetfs_ioctl_arg {
>>> uint16_t length;
>>> uint8_t reserved[2];
>>>
>>> uint8_t data[0];
>>> }
>>>
>>> but the principle is pretty much the same.
>>>
>>> Alan Stern
>>>
>> Won’t the user lose the relevant information (e.g. feature
>> structure) by using this model?
> What feature structure? Aren't your feature lists just vectors of 64
> bits? They can be stored in the .data field above.
>
> Alan Stern
>
Not “just” - they are platform-dependant uint64_t. which means they
won’t look the same on systems with different endianness. If the
user is unaware of this, it can cause confusion w/r/t which bit is which.
We can use 8-bit vectors instead and skip the endianness issue,
but why define a generic usb_gadgetfs_ioctl_args structure instead
of “features struct” for feature-related ioctls and different structs for
other types of ioctl (if we’ll have such in the future)?
>>> Good point. Yes, you can have different interfaces for different
>>> ioctls.
>>>
>>> This whole question arose because I complained that the features API
>>> looked too extravagant, and Felipe said that he didn't want to run out
>>> of available fields in case any features in the future need them.
>>>
>>> But if you're worried about that, why limit yourself to 64 features and
>>> 24 bytes of extra data? It doesn't make sense to think that some fixed
>>> limit might end up being too small, but then restrict yourself to a
>>> different fixed limit.
>>>
>>> Alan Stern
>> What about changing the ioctl API for the “features” handling, and instead
>> of passing an entire bitmap from user to kernel and vice versa, the user
>> will only operate on a single feature (by feature id)?
>>
>> This way we’ll have a much simpler (and intuitive) API with the user -
>> is_feature_supported(u64), is_feature_enabled(u64), enable_feature(u64)
>> and disabled_feature(u64).
>>
>> So the internal representation of the supported/enabled features will not
>> matter much to the user.
> That's fine with me. But since you're passing feature ID numbers to
> the kernel, instead of feature bitmaps, the argument type should be
> plain int, not u64. Unless you think somebody will implement more than
> 2 billion features... :-)
>
> Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Felipe - is it OK with you?
--
Binyamin Sharet,
Cisco, STARE-C
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] usb: core: setup dma_pfn_offset for USB devices and, interfaces
If dma_pfn_offset is not inherited correctly from the host controller, it might result in sub-optimal configuration as bounce buffer limit might be set to less than optimal level. Consider the mass storage device case. USB storage driver creates a scsi host for the mass storage interface in drivers/usb/storage/usb.c The scsi host parent device is nothing but the the USB interface device. Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out and set the block layer bounce limit. scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the bounce_limit. host_dev is nothing but the device representing the mass storage interface. If that device doesn't have the right dma_pfn_offset, then dma_max_pfn() is messed up and the bounce buffer limit is wrong. e.g. On Keystone 2 systems, dma_max_pfn() is 0x87 and dma_mask_pfn is 0xF. Consider a mass storage use case: Without this patch, usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting in a dma_max_pfn() of 0xF within the scsi layer (scsi_calculate_bounce_limit()). This will result in bounce buffers being unnecessarily used. Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset Signed-off-by: Roger Quadros --- Changelog: v4: - added comment in the code as to why we need to set dma_mask and dma_pfn_offset for usb devices. v3: - removed comments from code as commit log is sufficient. v2: - added more information in commit log and code. drivers/usb/core/message.c | 5 + drivers/usb/core/usb.c | 11 +++ 2 files changed, 16 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0406a59..bb617df 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1862,7 +1862,12 @@ free_interfaces: intf->dev.bus = &usb_bus_type; intf->dev.type = &usb_if_device_type; intf->dev.groups = usb_interface_groups; + /* +* Please refer to usb_alloc_dev() to see why we set +* dma_mask and dma_pfn_offset. +*/ intf->dev.dma_mask = dev->dev.dma_mask; + intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset; INIT_WORK(&intf->reset_ws, __usb_queue_reset_device); intf->minor = -1; device_initialize(&intf->dev); diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 5e80697..5921514 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -440,7 +440,18 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, dev->dev.bus = &usb_bus_type; dev->dev.type = &usb_device_type; dev->dev.groups = usb_device_groups; + /* +* Fake a dma_mask/offset for the USB device: +* We cannot really use the dma-mapping API (dma_alloc_* and +* dma_map_*) for USB devices but instead need to use +* usb_alloc_coherent and pass data in 'urb's, but some subsystems +* manually look into the mask/offset pair to determine whether +* they need bounce buffers. +* Note: calling dma_set_mask() on a USB device would set the +* mask for the entire HCD, so don't do that. +*/ dev->dev.dma_mask = bus->controller->dma_mask; + dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset; set_dev_node(&dev->dev, dev_to_node(bus->controller)); dev->state = USB_STATE_ATTACHED; dev->lpm_disable_count = 1; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: Kconfig: make USB_ULPI_BUS select USB_COMMON
On Tue, Sep 13, 2016 at 09:36:39AM +0200, Arnd Bergmann wrote: > On Tuesday, September 13, 2016 3:19:42 PM CEST Peter Chen wrote: > > > > I just see below Kconfig entry at the same Kconfig > > (drivers/usb/Kconfig), and forget your changes. > > > > config USB_LED_TRIG > > bool "USB LED Triggers" > > depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS > > help > > This option adds LED triggers for USB host and/or gadget activity. > > > > Say Y here if you are working on a system with led-class supported > > LEDs and you want to use them as activity indicators for USB host > > or > > gadget. > > > > Just grep it, some Kconfig still uses depends on for USB_COMMON, let me > > change it together. > > > > I suspect this one above should instead depend on "USB_SUPPORT". > No, it is embraced by "if USB_SUPPORT", so we don't need to add it additionally. Even USB_SUPPORT is chosen, the USB_COMMON may still not be chosen, so we need let USB_LET_TRIG select USB_COMMON explicitly. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: Kconfig: make USB_ULPI_BUS select USB_COMMON
On Tuesday, September 13, 2016 4:50:05 PM CEST Peter Chen wrote: > On Tue, Sep 13, 2016 at 09:36:39AM +0200, Arnd Bergmann wrote: > > On Tuesday, September 13, 2016 3:19:42 PM CEST Peter Chen wrote: > > > > > > I just see below Kconfig entry at the same Kconfig > > > (drivers/usb/Kconfig), and forget your changes. > > > > > > config USB_LED_TRIG > > > bool "USB LED Triggers" > > > depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS > > > help > > > This option adds LED triggers for USB host and/or gadget > > > activity. > > > > > > Say Y here if you are working on a system with led-class > > > supported > > > LEDs and you want to use them as activity indicators for USB > > > host or > > > gadget. > > > > > > Just grep it, some Kconfig still uses depends on for USB_COMMON, let me > > > change it together. > > > > > > > I suspect this one above should instead depend on "USB_SUPPORT". > > > > No, it is embraced by "if USB_SUPPORT", so we don't need to add it > additionally. Right, makes sense. > Even USB_SUPPORT is chosen, the USB_COMMON may still > not be chosen, so we need let USB_LET_TRIG select USB_COMMON > explicitly. My thought was that all users that could possibly call the USB_LED_TRIG interfaces already rely on USB_COMMON to be present. There is probably no real downside in having the 'select USB_COMMON', but it shouldn't actually be needed. It would be somewhat unusual though to enable USB_LED_TRIG and not build the file because we don't enter the directory, so maybe it's better to have it after all. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: core: setup dma_pfn_offset for USB devices and, interfaces
On Tuesday, September 13, 2016 11:16:03 AM CEST Roger Quadros wrote: > If dma_pfn_offset is not inherited correctly from the host controller, > it might result in sub-optimal configuration as bounce > buffer limit might be set to less than optimal level. > > Consider the mass storage device case. > USB storage driver creates a scsi host for the mass storage interface in > drivers/usb/storage/usb.c > The scsi host parent device is nothing but the the USB interface device. > Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out > and set the block layer bounce limit. > scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the > bounce_limit. host_dev is nothing but the device representing the > mass storage interface. > If that device doesn't have the right dma_pfn_offset, then dma_max_pfn() > is messed up and the bounce buffer limit is wrong. > > e.g. On Keystone 2 systems, dma_max_pfn() is 0x87 and dma_mask_pfn > is 0xF. Consider a mass storage use case: Without this patch, > usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting > in a dma_max_pfn() of 0xF within the scsi layer > (scsi_calculate_bounce_limit()). > This will result in bounce buffers being unnecessarily used. > > Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset > > Signed-off-by: Roger Quadros Acked-by: Arnd Bergmann -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH v7 0/4] the fix for the USB HOST1 at rk3288 platform
Hi Randy, could you check if the other host-only dwc2 are also affected by this (rk3188, rk3036) please? Because they also seem to act up in some strange way sometimes. Thanks Heiko Am Samstag, 10. September 2016, 02:59:36 CEST schrieb Randy Li: > At this stage it is the only "full features" and well verified > USB EHCI controller in this platform. More review is always necessary. > > Changelog: > - v7: > adding a wrapper for the reset operation for phy > using that wrapper > - v6: > move pwms pinctrl to pwms node > fix the order of the dtb file in Makefile > - v5: >- correct the mail format > - v4: >- re-order some nodes in alphabetical order >- fix some minor bugs >- add a entry in vendor list > - v3: >- fixing the rtc clock, using clock source from PMIC >- enable the tmu >- enable the fimc for elite board >- suuport the audio codec at elite board >- fixing minor bugs in the last commit > - v2: >- removing rtc node > the clock source driver is not done yet. >- adding exynos-bus >- fixing the MFC > > Randy Li (4): > phy: Add reset callback > phy: rockchip-usb: use rockchip_usb_phy_reset to reset phy during > wakeup > usb: dwc2: assert phy reset when waking up in rk3288 platform > ARM: dts: rockchip: Point rk3288 dwc2 usb at the full PHY reset > > .../devicetree/bindings/phy/rockchip-usb-phy.txt | 3 +++ > arch/arm/boot/dts/rk3288.dtsi| 4 > drivers/phy/phy-core.c | 14 ++ > drivers/phy/phy-rockchip-usb.c | 20 > drivers/usb/dwc2/core_intr.c | > 11 +++ include/linux/phy/phy.h | 3 > +++ > 6 files changed, 55 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH v7 0/4] the fix for the USB HOST1 at rk3288 platform
On 09/13/2016 07:06 PM, Heiko Stuebner wrote: Hi Randy, could you check if the other host-only dwc2 are also affected by this (rk3188, rk3036) please? Because they also seem to act up in some strange way But I don't have those board currently. I would arrange them anyway. Btw, I would send a new version to correct the fault reported by the auto build machine. sometimes. Thanks Heiko Am Samstag, 10. September 2016, 02:59:36 CEST schrieb Randy Li: At this stage it is the only "full features" and well verified USB EHCI controller in this platform. More review is always necessary. Changelog: - v7: adding a wrapper for the reset operation for phy using that wrapper - v6: move pwms pinctrl to pwms node fix the order of the dtb file in Makefile - v5: - correct the mail format - v4: - re-order some nodes in alphabetical order - fix some minor bugs - add a entry in vendor list - v3: - fixing the rtc clock, using clock source from PMIC - enable the tmu - enable the fimc for elite board - suuport the audio codec at elite board - fixing minor bugs in the last commit - v2: - removing rtc node the clock source driver is not done yet. - adding exynos-bus - fixing the MFC Randy Li (4): phy: Add reset callback phy: rockchip-usb: use rockchip_usb_phy_reset to reset phy during wakeup usb: dwc2: assert phy reset when waking up in rk3288 platform ARM: dts: rockchip: Point rk3288 dwc2 usb at the full PHY reset .../devicetree/bindings/phy/rockchip-usb-phy.txt | 3 +++ arch/arm/boot/dts/rk3288.dtsi| 4 drivers/phy/phy-core.c | 14 ++ drivers/phy/phy-rockchip-usb.c | 20 drivers/usb/dwc2/core_intr.c | 11 +++ include/linux/phy/phy.h | 3 +++ 6 files changed, 55 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about suspend/resume clock handling in dwc3-of-simple.c
On 09/12/2016 10:35 PM, Felipe Balbi wrote: Hi, Guenter Roeck writes: Should it be clk_disable_unprepare(), or maybe something like the following if (!pm_runtime_status_suspended(dev)) clk_disable_unprepare(); else clk_unprepare(); I'm not sure how balanced those calls are, yeah. I don't have HW to test PM with. But note that as it is, there is no actual runtime PM support, so clk_disable_unprepare() will always be necessary. Perhaps we will find further issues when someone tries to use runtime PM with dwc3-of-simple. ;-) We are working on code derived from it, so unless I can convince the author that he can not just use clk_unprepare() I suspect we'll hit the problem. If so, I'll let you know. Are you sending that upstream? Depending on your requirements, it might be easier to patch dwc3-of-simple.c then adding yet another glue layer :-) Yes. It will be a glue layer. So far that looks like the cleanest solution. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: core: setup dma_pfn_offset for USB devices and, interfaces
On Tue, 13 Sep 2016, Arnd Bergmann wrote: > On Tuesday, September 13, 2016 11:16:03 AM CEST Roger Quadros wrote: > > If dma_pfn_offset is not inherited correctly from the host controller, > > it might result in sub-optimal configuration as bounce > > buffer limit might be set to less than optimal level. > > > > Consider the mass storage device case. > > USB storage driver creates a scsi host for the mass storage interface in > > drivers/usb/storage/usb.c > > The scsi host parent device is nothing but the the USB interface device. > > Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out > > and set the block layer bounce limit. > > scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the > > bounce_limit. host_dev is nothing but the device representing the > > mass storage interface. > > If that device doesn't have the right dma_pfn_offset, then dma_max_pfn() > > is messed up and the bounce buffer limit is wrong. > > > > e.g. On Keystone 2 systems, dma_max_pfn() is 0x87 and dma_mask_pfn > > is 0xF. Consider a mass storage use case: Without this patch, > > usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting > > in a dma_max_pfn() of 0xF within the scsi layer > > (scsi_calculate_bounce_limit()). > > This will result in bounce buffers being unnecessarily used. > > > > Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset > > > > Signed-off-by: Roger Quadros > > Acked-by: Arnd Bergmann Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mapping USB-ports "by slot" rather than by "usb attributes"
A. (sorry for my stupid realisation-originated-question), for
the same serial-cable types with identical "idVendor" and "idProduct",
are they guaranteed to have unique serial-numbers?
We have the Prolific-PL2303-Serial-To-USB-converters with ID 067b:2303.
Right now I have two of these pieces, and they have identical 067b:2303, but
udevadm info -a -n /dev/ttyUSB0 | grep '{serial}' | head -n1
gives different serial-numbers for these two pieces.
Can I generalize this to any piece of the same cable-type?
If yes, I should be done :)
Either ways, thanks a ton Greg and Felipe for your prompt responses !!!
Thanks and Regards,
Ajay
On Tue, Sep 13, 2016 at 11:30 AM, Felipe Balbi
wrote:
>
> Hi,
>
> Ajay Garg writes:
>> Hi All.
>>
>> Till now, we had a single usb-serial-cable, that we needed to
>> associate to a static tty-name.
>> http://hintshop.ludvig.co.nz/show/persistent-names-usb-serial-devices/
>> worked perfect for our purpose (till now).
>>
>>
>> However, we now require that multiple "identical" usb-serial-cables be
>> inserted in our device, and we want to associate static tty-names to
>> slots, something like ::
>>
>> slot-1 ==> always /dev/ttyUSB0
>> slot-2 ==> always /dev/ttyUSB1
>>
>> and so on ...
>>
>> Since all slots will contain "identical" usb-serial-cables, so the
>> solution at
>> http://hintshop.ludvig.co.nz/show/persistent-names-usb-serial-devices/
>> (if I am not wrong) breaks.
>
> They should still have separate Serial Numbers, right? Anyway, it's not
> hard to find a device without Serial number.
>
>> So, is there a solution to our requirement (probably via udev-rules)?
>> If yes, I will be grateful to receive any pointers.
>
> You can use the symlinks in /dev/serial/by-path/. See if that's enough
> for you.
>
> --
> balbi
--
Regards,
Ajay
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mapping USB-ports "by slot" rather than by "usb attributes"
On Tue, Sep 13, 2016 at 07:20:01PM +0530, Ajay Garg wrote:
> A. (sorry for my stupid realisation-originated-question), for
> the same serial-cable types with identical "idVendor" and "idProduct",
> are they guaranteed to have unique serial-numbers?
No, there is no such USB requirement. Odds are, if it is a cheap
device, the serial numbers will all be the same.
> We have the Prolific-PL2303-Serial-To-USB-converters with ID 067b:2303.
> Right now I have two of these pieces, and they have identical 067b:2303, but
>
> udevadm info -a -n /dev/ttyUSB0 | grep '{serial}' | head -n1
>
> gives different serial-numbers for these two pieces.
Oh nice, you have some "good" ones :)
> Can I generalize this to any piece of the same cable-type?
Nope :(
That's why you should use /dev/serial/ it should give you something you
can use, even if the serial numbers are identical.
good luck!
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about suspend/resume clock handling in dwc3-of-simple.c
Hi, Guenter Roeck writes: > On 09/12/2016 10:35 PM, Felipe Balbi wrote: >> >> Hi, >> >> Guenter Roeck writes: > Should it be clk_disable_unprepare(), or maybe something like the > following > > if (!pm_runtime_status_suspended(dev)) > clk_disable_unprepare(); > else > clk_unprepare(); I'm not sure how balanced those calls are, yeah. I don't have HW to test PM with. But note that as it is, there is no actual runtime PM support, so clk_disable_unprepare() will always be necessary. Perhaps we will find further issues when someone tries to use runtime PM with dwc3-of-simple. ;-) >>> >>> We are working on code derived from it, so unless I can convince the author >>> that he can not just use clk_unprepare() I suspect we'll hit the problem. >>> If so, I'll let you know. >> >> Are you sending that upstream? Depending on your requirements, it might >> be easier to patch dwc3-of-simple.c then adding yet another glue layer :-) >> > Yes. It will be a glue layer. So far that looks like the cleanest solution. fair enough, take your time ;-) -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
Hi, On Mon, Sep 12, 2016 at 08:18:05PM -0700, Tony Lindgren wrote: > * Bin Liu [160912 11:36]: > > On Mon, Sep 12, 2016 at 08:05:30PM +0200, Andreas Kemnade wrote: > > > Hmm, then the question is: Couldn't the X_musb_disable simply be called > > > from X_probe if needed to be an the safe side? > > > > In general, we try not to do so if all possible. We want to put common > > code in the core, not repleat them in glue layers. > > > > In this specific case, we cannot do it. For example in dsps glue, the > > musb reset is done in dsps_musb_init(), so no place in dsps_probe() to > > call dsps_musb_disable(). > > OK yeah if we need to do something, then disabling the irq in > musb_core.c until we're done should be enough. I don't think we can disable it in musb core directly. *_musb_disable() in glue layers disables the musb wrapper's irq, not the core's irq. But I guess we can let *_musb_init() directly calls *_musb_disable() in the glue drivers right after musb reset. This would have to touch almost all glue drivers, but trivial change. Thoughts? > > Anyways, I tested the $subject patch also with am35x code with the > following patch and no issues. So I've now tested with omap3, > omap4, am335x, and am35x. I don't expect your v2 breaks am35x either, as I believe its default of out-of-reset already disables the irq. I just don't like that SW relies on unguaranteed HW default state. > There are also am35x musb device tree patches from last year by > Rolf Peukert : > > https://patchwork.kernel.org/project/linux-omap/list/?submitter=144061 Good to know this work. > > And we now have new drivers/phy/phy-da8xx-usb.c that might be the > same phy on am35x except with register bits moved around. > > So maybe we'll have it working with device tree quite easily. > Meanwhile, we should probably apply the following patch so we > get things working again. Yup. Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
On Tue, Sep 13, 2016 at 05:32:23PM +0300, Laurent Pinchart wrote: > Hi Bin, > > On Tuesday 13 Sep 2016 09:14:48 Bin Liu wrote: > > On Mon, Sep 12, 2016 at 08:18:05PM -0700, Tony Lindgren wrote: > > > * Bin Liu [160912 11:36]: > > > > On Mon, Sep 12, 2016 at 08:05:30PM +0200, Andreas Kemnade wrote: > > > > > Hmm, then the question is: Couldn't the X_musb_disable simply be > > > > > called > > > > > from X_probe if needed to be an the safe side? > > > > > > > > In general, we try not to do so if all possible. We want to put common > > > > code in the core, not repleat them in glue layers. > > > > > > > > In this specific case, we cannot do it. For example in dsps glue, the > > > > musb reset is done in dsps_musb_init(), so no place in dsps_probe() to > > > > call dsps_musb_disable(). > > > > > > OK yeah if we need to do something, then disabling the irq in > > > musb_core.c until we're done should be enough. > > > > I don't think we can disable it in musb core directly. *_musb_disable() > > in glue layers disables the musb wrapper's irq, not the core's irq. > > > > But I guess we can let *_musb_init() directly calls *_musb_disable() in > > the glue drivers right after musb reset. This would have to touch almost > > all glue drivers, but trivial change. Thoughts? > > Is there any use case for starting a glue layer with interrupts enabled ? If I cannot think of any, at least in the current musb drivers. > not I agree that all glue layers should do so in *_musb_init(). Probably not > by calling *_musb_disable() directly though, as that would reintroduce the > unbalanced PHY power control problem. Not a problem, such change will be done in each glue driver, so we don't do it for omap2430 glue. Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
* Bin Liu [160913 07:40]: > On Tue, Sep 13, 2016 at 05:32:23PM +0300, Laurent Pinchart wrote: > > Hi Bin, > > > > On Tuesday 13 Sep 2016 09:14:48 Bin Liu wrote: > > > On Mon, Sep 12, 2016 at 08:18:05PM -0700, Tony Lindgren wrote: > > > > * Bin Liu [160912 11:36]: > > > > > On Mon, Sep 12, 2016 at 08:05:30PM +0200, Andreas Kemnade wrote: > > > > > > Hmm, then the question is: Couldn't the X_musb_disable simply be > > > > > > called > > > > > > from X_probe if needed to be an the safe side? > > > > > > > > > > In general, we try not to do so if all possible. We want to put common > > > > > code in the core, not repleat them in glue layers. > > > > > > > > > > In this specific case, we cannot do it. For example in dsps glue, the > > > > > musb reset is done in dsps_musb_init(), so no place in dsps_probe() to > > > > > call dsps_musb_disable(). > > > > > > > > OK yeah if we need to do something, then disabling the irq in > > > > musb_core.c until we're done should be enough. > > > > > > I don't think we can disable it in musb core directly. *_musb_disable() > > > in glue layers disables the musb wrapper's irq, not the core's irq. > > > > > > But I guess we can let *_musb_init() directly calls *_musb_disable() in > > > the glue drivers right after musb reset. This would have to touch almost > > > all glue drivers, but trivial change. Thoughts? > > > > Is there any use case for starting a glue layer with interrupts enabled ? > > If > > I cannot think of any, at least in the current musb drivers. No reason that I can think of :) > > not I agree that all glue layers should do so in *_musb_init(). Probably > > not > > by calling *_musb_disable() directly though, as that would reintroduce the > > unbalanced PHY power control problem. > > Not a problem, such change will be done in each glue driver, so we don't > do it for omap2430 glue. Makes sense. Then we can just move all of musb_generic_enable/disable code into pm_runtime_resume/suspend in musb-core.c and make sure they are paired. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] cdc-acm: cleaning up debug in data submission path
Further cleanup making the debug messages more precise, useful
and removing mere trace points.
Signed-off-by: Oliver Neukum
---
drivers/usb/class/cdc-acm.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index faab151..15ffe38 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -415,8 +415,9 @@ static void acm_read_bulk_callback(struct urb *urb)
unsigned long flags;
int status = urb->status;
- dev_vdbg(&acm->data->dev, "%s - urb %d, len %d\n", __func__,
- rb->index, urb->actual_length);
+ dev_vdbg(&acm->data->dev, "got urb %d, len %d, status %d\n",
+ rb->index, urb->actual_length,
+ status);
if (!acm->dev) {
set_bit(rb->index, &acm->read_urbs_free);
@@ -426,8 +427,6 @@ static void acm_read_bulk_callback(struct urb *urb)
if (status) {
set_bit(rb->index, &acm->read_urbs_free);
- dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n",
- __func__, status);
if ((status != -ENOENT) || (urb->actual_length == 0))
return;
}
@@ -462,8 +461,7 @@ static void acm_write_bulk(struct urb *urb)
int status = urb->status;
if (status || (urb->actual_length != urb->transfer_buffer_length))
- dev_vdbg(&acm->data->dev, "%s - len %d/%d, status %d\n",
- __func__,
+ dev_vdbg(&acm->data->dev, "wrote len %d/%d, status %d\n",
urb->actual_length,
urb->transfer_buffer_length,
status);
@@ -478,8 +476,6 @@ static void acm_softint(struct work_struct *work)
{
struct acm *acm = container_of(work, struct acm, work);
- dev_vdbg(&acm->data->dev, "%s\n", __func__);
-
tty_port_tty_wakeup(&acm->port);
}
@@ -674,7 +670,7 @@ static int acm_tty_write(struct tty_struct *tty,
if (!count)
return 0;
- dev_vdbg(&acm->data->dev, "%s - count %d\n", __func__, count);
+ dev_vdbg(&acm->data->dev, "%d bytes from tty layer\n", count);
spin_lock_irqsave(&acm->write_lock, flags);
wbn = acm_wb_alloc(acm);
@@ -691,7 +687,7 @@ static int acm_tty_write(struct tty_struct *tty,
}
count = (count > acm->writesize) ? acm->writesize : count;
- dev_vdbg(&acm->data->dev, "%s - write %d\n", __func__, count);
+ dev_vdbg(&acm->data->dev, "writing %d bytes\n", count);
memcpy(wb->buf, buf, count);
wb->len = count;
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] cdc-acm: cleanup debugging in submission path
Actually make it retutn useful information.
Signed-off-by: Oliver Neukum
---
drivers/usb/class/cdc-acm.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index fef0d8f..faab151 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -368,17 +368,17 @@ static int acm_submit_read_urb(struct acm *acm, int
index, gfp_t mem_flags)
if (!test_and_clear_bit(index, &acm->read_urbs_free))
return 0;
- dev_vdbg(&acm->data->dev, "%s - urb %d\n", __func__, index);
-
res = usb_submit_urb(acm->read_urbs[index], mem_flags);
if (res) {
if (res != -EPERM) {
dev_err(&acm->data->dev,
- "%s - usb_submit_urb failed: %d\n",
- __func__, res);
+ "urb %d failed submission with %d\n",
+ index, res);
}
set_bit(index, &acm->read_urbs_free);
return res;
+ } else {
+ dev_vdbg(&acm->data->dev, "submitted urb %d\n", index);
}
return 0;
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH v7 0/4] the fix for the USB HOST1 at rk3288 platform
Am Dienstag, 13. September 2016, 19:26:03 CEST schrieb ayaka: > On 09/13/2016 07:06 PM, Heiko Stuebner wrote: > > Hi Randy, > > > > could you check if the other host-only dwc2 are also affected by this > > (rk3188, rk3036) please? Because they also seem to act up in some strange > > way > But I don't have those board currently. I would arrange them anyway. > Btw, I would send a new version to correct the fault reported by the > auto build machine. What I meant was, if you could check with some hardware-people first, if the other host-only dwc2 are supposedly also affected. Right now I'm trying to determine if this effect is somehow related with the usb debounce issue I'm seeing with some recent dwc2 patches [0]. Heiko [0] http://lists.infradead.org/pipermail/linux-rockchip/2016-September/012141.html https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=2938fc63e0c26bf694436ac81bc776c8b7eced0c -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
Hi Bin, On Tuesday 13 Sep 2016 09:14:48 Bin Liu wrote: > On Mon, Sep 12, 2016 at 08:18:05PM -0700, Tony Lindgren wrote: > > * Bin Liu [160912 11:36]: > > > On Mon, Sep 12, 2016 at 08:05:30PM +0200, Andreas Kemnade wrote: > > > > Hmm, then the question is: Couldn't the X_musb_disable simply be > > > > called > > > > from X_probe if needed to be an the safe side? > > > > > > In general, we try not to do so if all possible. We want to put common > > > code in the core, not repleat them in glue layers. > > > > > > In this specific case, we cannot do it. For example in dsps glue, the > > > musb reset is done in dsps_musb_init(), so no place in dsps_probe() to > > > call dsps_musb_disable(). > > > > OK yeah if we need to do something, then disabling the irq in > > musb_core.c until we're done should be enough. > > I don't think we can disable it in musb core directly. *_musb_disable() > in glue layers disables the musb wrapper's irq, not the core's irq. > > But I guess we can let *_musb_init() directly calls *_musb_disable() in > the glue drivers right after musb reset. This would have to touch almost > all glue drivers, but trivial change. Thoughts? Is there any use case for starting a glue layer with interrupts enabled ? If not I agree that all glue layers should do so in *_musb_init(). Probably not by calling *_musb_disable() directly though, as that would reintroduce the unbalanced PHY power control problem. > > Anyways, I tested the $subject patch also with am35x code with the > > following patch and no issues. So I've now tested with omap3, > > omap4, am335x, and am35x. > > I don't expect your v2 breaks am35x either, as I believe its default of > out-of-reset already disables the irq. I just don't like that SW relies > on unguaranteed HW default state. > > > There are also am35x musb device tree patches from last year by > > Rolf Peukert : > > > > https://patchwork.kernel.org/project/linux-omap/list/?submitter=144061 > > Good to know this work. > > > And we now have new drivers/phy/phy-da8xx-usb.c that might be the > > same phy on am35x except with register bits moved around. > > > > So maybe we'll have it working with device tree quite easily. > > Meanwhile, we should probably apply the following patch so we > > get things working again. > > Yup. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mapping USB-ports "by slot" rather than by "usb attributes"
Thanks Greg for the reply.
Following are some outputs for a cable ::
ls -lrth /dev/serial/by-path/
total 0
lrwxrwxrwx 1 root root 13 Sep 13 14:43
platform-3f98.usb-usb-0:1.4:1.0-port0 -> ../../ttyUSB0
ls -lrth /dev/serial/by-id/
total 0
lrwxrwxrwx 1 root root 13 Sep 13 14:43
usb-Prolific_Technology_Inc._USB-Serial_Controller-if00-port0 ->
../../ttyUSB0
So, what should be a udev-rule for this cable, so that only and only
this piece of cable is mapped to /dev/ttyUSB0 on this system, ever, in
whatever slot this cable is inserted?
For brevity, right now the rule would be ::
SUBSYSTEM=="tty", ATTRS{idVendor}=="067b", ATTRS{idProduct}=="2303",
ATTRS{serial}=="3f98.usb", SYMLINK+="ttyUSB0"
but since serial-numbers are not guaranteed to be the differentiator,
so I will be highly thankful for letting me know the unique udev-rule.
Looking forward to a reply :)
Thanks and Regards,
Ajay
On Tue, Sep 13, 2016 at 7:34 PM, Greg KH wrote:
> On Tue, Sep 13, 2016 at 07:20:01PM +0530, Ajay Garg wrote:
>> A. (sorry for my stupid realisation-originated-question), for
>> the same serial-cable types with identical "idVendor" and "idProduct",
>> are they guaranteed to have unique serial-numbers?
>
> No, there is no such USB requirement. Odds are, if it is a cheap
> device, the serial numbers will all be the same.
>
>> We have the Prolific-PL2303-Serial-To-USB-converters with ID 067b:2303.
>> Right now I have two of these pieces, and they have identical 067b:2303, but
>>
>> udevadm info -a -n /dev/ttyUSB0 | grep '{serial}' | head -n1
>>
>> gives different serial-numbers for these two pieces.
>
> Oh nice, you have some "good" ones :)
>
>> Can I generalize this to any piece of the same cable-type?
>
> Nope :(
>
> That's why you should use /dev/serial/ it should give you something you
> can use, even if the serial numbers are identical.
>
> good luck!
>
> greg k-h
--
Regards,
Ajay
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Hi Martin,
Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote:
> > Martin Blumenstingl writes:
> >
> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks
> >> wrote:
> >>> On 08/09/16 21:42, Kevin Hilman wrote:
>
> Ben Dooks writes:
>
> > On 08/09/16 20:52, Martin Blumenstingl wrote:
> >>
> >> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman
> >> wrote:
>
> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
> + if (IS_ERR(phy)) {
> + dev_err(&pdev->dev, "failed to create PHY\n");
> + return PTR_ERR(phy);
> + }
> +
> + if (usb_reset_refcnt++ == 0) {
> + ret = device_reset(&pdev->dev);
> + if (ret) {
> + dev_err(&phy->dev, "Failed to reset USB
> PHY\n");
> + return ret;
> + }
> + }
> >>>
> >>>
> >>> The ref count + reset here looks like something that could/should be
> >>> handled in a runtime PM callback.
> >>
> >> Unfortunately that doesn't work (as Jerome found out) because both
> >> PHYs are sharing the same reset line.
> >> So if the second PHY would call device_reset then it would also reset
> >> the first PHY!
> >>
> >> There's a comment above the declaration of usb_reset_refcnt which
> >> tries to explain this:
> >> "The PHYs are sharing a common reset line -> we are only allowed to
> >> reset once for all PHYs."
> >> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
> >> {" line to make it easier to see?
> >>
> >
> > pm-runtime has refcounting in it. When one of the nodes turns on,
> > the pm-runtime will call your driver to say there is a user when
> > this first use turns up.
> >
> > If all the sub-phys turn off and drop their refcount then the driver
> > is called to say there are no more users and you can go to sleep.
>
>
> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>
> The reason is because there are physically two PHY devices[1]. Those 2
> devices will be treated independely by runtime PM, and have separate
> use-counting, which means doing what I proposed would cause a reset to
> happen when either device was probed.
>
> So, I think it's OK as it is.
> >>>
> >>>
> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
> >>> device and do it that way?
> >> could you please be more specific with that (do you mean pdev->dev.parent)?
> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
> >> would still define the runtime_resume in our driver.
> >
> > You'd also need to do get/put on the children, but yes, that's what Ben
> > is suggesting.
> >
> > However, the problem with all of the solutions proposed (runtime PM ones
> > included) is that we're forcing a board-specific design issue (2 devices
> > sharing a reset line) into a driver that should not have any
> > board-specific assumptions in it.
> >
> > For example, if this driver is used on another platform where different
> > PHYs have different reset lines, then one of them (the unlucky one who
> > is not probed first) will never get reset. So any form of per-device
> > ref-counting is not a portable solution.
> indeed, so in simple words we would need something like
> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
> remember internally if any action has already been executed: if not it
> does a _reset, _assert or _deassert and otherwise it does nothing.
>
> > I'm not sure yet how the reset framework is supposed to handle shared
> > reset lines, but that needs some investigation. I quick glance and it
> > seems that reset controllers can have shared lines, so that should be
> > investigated.
> I added Philipp and Hans to this thread - maybe they can comment on this.
> To sum it up, our problem is:
> - there are two separate USB PHYs on Meson GXBB
> - both are sharing the same reset line (provided by the reset-meson driver)
> - during initialization of the PHYs we must only call
> reset_control_reset(rstc) once (if we do it for the first *and* second
> PHY then the first PHY gets confused once the second PHY uses the
> reset because the first PHY's state is reset as well)
If you have an initially asserted reset line and you can enable the
first module by deasserting the reset via reset_control_deassert (and
reset_control_assert to signal when the module may be disabled again
after use), shared resets are for you.
If you need a reset pulse or have no direct control over the reset line,
(device_reset), the reset framework currently has no solution for this.
The ugly thing about reset_control_once would be that it can't re-reset
modules when
Re: Mapping USB-ports "by slot" rather than by "usb attributes"
On Tue, Sep 13, 2016 at 08:45:14PM +0530, Ajay Garg wrote: > Thanks Greg for the reply. > > > > Following are some outputs for a cable :: > > ls -lrth /dev/serial/by-path/ > total 0 > lrwxrwxrwx 1 root root 13 Sep 13 14:43 > platform-3f98.usb-usb-0:1.4:1.0-port0 -> ../../ttyUSB0 > > ls -lrth /dev/serial/by-id/ > total 0 > lrwxrwxrwx 1 root root 13 Sep 13 14:43 > usb-Prolific_Technology_Inc._USB-Serial_Controller-if00-port0 -> > ../../ttyUSB0 > > > So, what should be a udev-rule for this cable, so that only and only > this piece of cable is mapped to /dev/ttyUSB0 on this system, ever, in > whatever slot this cable is inserted? That's the opposite of what you should use, it doesn't matter what device is assigned ttyUSB0 (as really, it will be random), you should use the symlinks in /dev/serial/ instead to always address the "correct" device. So, if you know you want to talk to the device on the hub that is connected into the 4th port, then open /dev/serial/by-path/platform-3f98.usb-usb-0:1.4:1.0-port0 When you start connecting and disconnecting the devices, the ttyUSB number will start to change, but the path to the device should be pretty stable (note, it isn't always as USB busses can be enumerated in different ways on different boots, but usually it is always the same) The best is to use the serial number symlink, if you have unique ones, as it sounds like you do, as those will always point to the correct device. hope this helps, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dmaengine: cppi41: Ignore EINPROGRESS for PM runtime
We can occasionally get -EINPROGRESS for pm_runtime_get. In that case
we can just continue as we're queueing transfers anyways when
pm_runtime_active is not set.
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Signed-off-by: Tony Lindgren
---
drivers/dma/cppi41.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -462,7 +462,7 @@ static void cppi41_dma_issue_pending(struct dma_chan *chan)
/* PM runtime paired with dmaengine_desc_get_callback_invoke */
error = pm_runtime_get(cdd->ddev.dev);
- if (error < 0) {
+ if ((error != -EINPROGRESS) && error < 0) {
dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
error);
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ULPI phy issue with
Hi Peter,
First thank you for your help.
On 13/09/16 08:52, Peter Chen wrote:
On Mon, Sep 12, 2016 at 12:06:10PM +0200, Fabien Lahoudere wrote:
On 12/09/16 11:46, Peter Chen wrote:
On Mon, Sep 12, 2016 at 11:13:01AM +0200, Fabien Lahoudere wrote:
Hi,
Yes, please send the patch and tell me which dts and ULPI code you are
using. We need to understand why the system is hang when configure the
PHY mode at portsc for your case, afaik, the PHY mode needs to be
configured as ULPI mode before using ULPI view port to access ULPI PHY.
Patch have been sent "usb: imx53 - Allow to configure ULPI mode for usb host
2 and 3", in reply to this thread.
Besides, do you enable this ULPI PHY at u-boot?
Yes, I configure imx53 in linux kernel in order to use ULPI mode. I remind
you that the patch works fine with my device.
I know you are using it at kernel. I just want to know if you enable it
at bootloader, besides, I need to know more about your platform since
the system hang usually dues to without PHY clock, so would you send
platform stuffs like dts (which dts), etc?
No USB are not used in uboot.
This is our device tree imx-ppd.dts:
I may know the reason why you meet hang at current flow, you are using
generic phy driver, and the PHY clock is enabled at phy_init which is
called later than setting portsc.pts. The current flow to enable ULPI
phy is like below:
This explains why the patch works if USBPHY_INTERFACE_MODE_ULPI works as
USBPHY_INTERFACE_MODE_UTMI. because _ci_usb_phy_init(ci) initialise
clock and generator.
1. Enable ULPI and choose its clock select at usbmisc, which you have
already done.
2. Enable the input clock for ULPI
This is done by _ci_usb_phy_init(ci); and this function only do this so
I think we should have in ci_usb_phy_init(ci);:
case USBPHY_INTERFACE_MODE_ULPI:
ret = _ci_usb_phy_init(ci);
if (!ret)
hw_wait_phy_stable();
else
return ret;
3. set portsc.pts
hw_phymode_configure(ci);
ci_ulpi_phy_init(ci); // to init ULPI specific config once portsc.pts
is enabled
break;
You may need to have a ULPI PHY driver which do some power sequence
(clock, regulator, etc) before setting portsc.pts and visit ULPI
register.
This is already done by _ci_usb_phy_init(ci);
In conclusion, I think USBPHY_INTERFACE_MODE_ULPI should work as
USBPHY_INTERFACE_MODE_UTMI but with an extra function to visit ULPI
register (ci_ulpi_phy_init(ci);).
Am I wrong?
I am also a little confused how Stephen Boyd's ULPI driver for qualcomm
platform, I will cc on discussion.
As you ask me earlier maybe he init clock from bootloader.
BR,
Fabien
Peter
/*
* Copyright 2014 General Electric Company
*
* The code contained herein is licensed under the GNU General Public
* License. You may obtain a copy of the GNU General Public License
* Version 2 or later at the following locations:
*
* http://www.opensource.org/licenses/gpl-license.html
* http://www.gnu.org/copyleft/gpl.html
*/
/dts-v1/;
#include "imx53.dtsi"
/ {
model = "Freescale i.MX53 CPUV0 PPD rev6";
compatible = "fsl,imx53-cpuvo", "fsl,imx53";
aliases {
spi0 = &cspi;
spi1 = &ecspi1;
spi2 = &ecspi2;
};
chosen {
stdout-path = "&uart1:115200n8";
};
memory {
reg = <0x7000 0x2000>,
<0xb000 0x2000>;
};
cko2_11M: sgtl_clock_cko2 {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <11289600>;
};
sgtlsound: sound {
compatible = "fsl,imx53-cpuvo-sgtl5000",
"fsl,imx-audio-sgtl5000";
model = "imx53-cpuvo-sgtl5000";
ssi-controller = <&ssi2>;
audio-codec = <&sgtl5000>;
audio-routing =
"MIC_IN", "Mic Jack",
"Mic Jack", "Mic Bias",
"Headphone Jack", "HP_OUT";
mux-int-port = <2>;
mux-ext-port = <6>;
status = "okay";
};
reg_sgtl5k: regulator-sgtl5k {
compatible = "regulator-fixed";
regulator-name = "regulator-sgtl5k";
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
regulator-always-on;
};
reg_usb_otg_vbus: regulator-usb-otg-vbus {
compatible = "regulator-fixed";
regulator-name = "usbotg_vbus";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
pinctrl-0 = <&pinctrl_usb_otg_vbus>;
gpio = <&gpio4 15 GPIO_ACTIVE_HIGH>;
enable-active-high;
Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Hi Philipp,
On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel wrote:
> Hi Martin,
>
> Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote:
>> > Martin Blumenstingl writes:
>> >
>> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks
>> >> wrote:
>> >>> On 08/09/16 21:42, Kevin Hilman wrote:
>>
>> Ben Dooks writes:
>>
>> > On 08/09/16 20:52, Martin Blumenstingl wrote:
>> >>
>> >> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman
>> >> wrote:
>>
>> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
>> + if (IS_ERR(phy)) {
>> + dev_err(&pdev->dev, "failed to create PHY\n");
>> + return PTR_ERR(phy);
>> + }
>> +
>> + if (usb_reset_refcnt++ == 0) {
>> + ret = device_reset(&pdev->dev);
>> + if (ret) {
>> + dev_err(&phy->dev, "Failed to reset USB
>> PHY\n");
>> + return ret;
>> + }
>> + }
>> >>>
>> >>>
>> >>> The ref count + reset here looks like something that could/should be
>> >>> handled in a runtime PM callback.
>> >>
>> >> Unfortunately that doesn't work (as Jerome found out) because both
>> >> PHYs are sharing the same reset line.
>> >> So if the second PHY would call device_reset then it would also reset
>> >> the first PHY!
>> >>
>> >> There's a comment above the declaration of usb_reset_refcnt which
>> >> tries to explain this:
>> >> "The PHYs are sharing a common reset line -> we are only allowed to
>> >> reset once for all PHYs."
>> >> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>> >> {" line to make it easier to see?
>> >>
>> >
>> > pm-runtime has refcounting in it. When one of the nodes turns on,
>> > the pm-runtime will call your driver to say there is a user when
>> > this first use turns up.
>> >
>> > If all the sub-phys turn off and drop their refcount then the driver
>> > is called to say there are no more users and you can go to sleep.
>>
>>
>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>
>> The reason is because there are physically two PHY devices[1]. Those 2
>> devices will be treated independely by runtime PM, and have separate
>> use-counting, which means doing what I proposed would cause a reset to
>> happen when either device was probed.
>>
>> So, I think it's OK as it is.
>> >>>
>> >>>
>> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
>> >>> device and do it that way?
>> >> could you please be more specific with that (do you mean
>> >> pdev->dev.parent)?
>> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
>> >> would still define the runtime_resume in our driver.
>> >
>> > You'd also need to do get/put on the children, but yes, that's what Ben
>> > is suggesting.
>> >
>> > However, the problem with all of the solutions proposed (runtime PM ones
>> > included) is that we're forcing a board-specific design issue (2 devices
>> > sharing a reset line) into a driver that should not have any
>> > board-specific assumptions in it.
>> >
>> > For example, if this driver is used on another platform where different
>> > PHYs have different reset lines, then one of them (the unlucky one who
>> > is not probed first) will never get reset. So any form of per-device
>> > ref-counting is not a portable solution.
>> indeed, so in simple words we would need something like
>> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
>> remember internally if any action has already been executed: if not it
>> does a _reset, _assert or _deassert and otherwise it does nothing.
>>
>> > I'm not sure yet how the reset framework is supposed to handle shared
>> > reset lines, but that needs some investigation. I quick glance and it
>> > seems that reset controllers can have shared lines, so that should be
>> > investigated.
>> I added Philipp and Hans to this thread - maybe they can comment on this.
>> To sum it up, our problem is:
>> - there are two separate USB PHYs on Meson GXBB
>> - both are sharing the same reset line (provided by the reset-meson driver)
>> - during initialization of the PHYs we must only call
>> reset_control_reset(rstc) once (if we do it for the first *and* second
>> PHY then the first PHY gets confused once the second PHY uses the
>> reset because the first PHY's state is reset as well)
>
> If you have an initially asserted reset line and you can enable the
> first module by deasserting the reset via reset_control_deassert (and
> reset_control_assert to signal when the module may be disabled again
> after use), shared resets are for you.
>
> If you need a reset pulse or have no dir
Re: [PATCH 1/2] usb: dwc3: Add ref clock period setting
On 9/12/2016 7:09 AM, Rob Herring wrote: > On Thu, Sep 01, 2016 at 02:32:30PM -0700, John Youn wrote: >> From: Thinh Nguyen >> >> Added ref_clk_per for writing to GUCTL.RefClkPer which >> sets the period of ref_clk in nano second. >> >> Signed-off-by: Thinh Nguyen >> Signed-off-by: John Youn >> --- >> Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++ >> drivers/usb/dwc3/core.c| 21 + >> drivers/usb/dwc3/core.h| 5 + >> drivers/usb/dwc3/dwc3-pci.c| 1 + >> 4 files changed, 29 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt >> b/Documentation/devicetree/bindings/usb/dwc3.txt >> index e3e6983..aa54ba7 100644 >> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >> @@ -50,6 +50,8 @@ Optional properties: >> - snps,hird-threshold: HIRD threshold >> - snps,hsphy_interface: High-Speed PHY interface selection between "utmi" >> for >> UTMI+ and "ulpi" for ULPI when the DWC_USB3_HSPHY_INTERFACE has value 3. >> + - snps,ref_clk_per: value for GUTCL.RefClkPer field that sets the period of >> +ref_clk in nano seconds. > > Use '-' rather than '_' and add a unit suffix (property-units.txt). > Ok. We'll change it to: snps,ref-clk-period-ns John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: dwc3: Added a property to set GFLADJ register
On 9/12/2016 8:30 AM, Rob Herring wrote: > On Thu, Sep 01, 2016 at 02:32:33PM -0700, John Youn wrote: >> From: Thinh Nguyen >> >> Added gfladj variable to control the core behavior with respect to >> SOF, ITP, and frame timer functionality. >> >> Currently there is dwc->fladj that holds a single field in GFLADJ >> register (GFLADJ.GFLADJ_30MHZ). A new variable gfladj is added to >> dwc structure to allow setting of the entire GFLADJ register. If >> dwc->gfladj is set, then it has a higher priority than dwc->fladj >> when writing to the GFLADJ register. > > I'm not a fan of magic register values for DT properties. > Sure. Felipe gave the same feedback. We'll fix it. > How many fields in this register that you will ever need to touch? > >> Synopsys HW setup (HAPS DX and phy board) requires a preset to this >> register to improve interoperablitity. For example, the value for >> GFLADJ_REFCLK_LPM_SEL should be set to 0 with ref_clk period of 50. > > This sounds like it should be handled in the driver. Is it a simple, > constant correlation of ref_clk period to this value? I don't know. I'll look into it. Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: cleanup with list_first_entry_or_null()
On 9/12/2016 10:52 PM, Felipe Balbi wrote: > > Hi Masahiro, > > Masahiro Yamada writes: >> The combo of list_empty() check and return list_first_entry() >> can be replaced with list_first_entry_or_null(). >> >> Signed-off-by: Masahiro Yamada >> --- > > Care to split this into two patches (one for dwc2 and one for dwc3)? > > thanks > For the dwc2 portion, you can add my acked-by. Acked-by: John Youn John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] System reboots instead of shutting down if xhci is enabled in BIOS and USB hub is connected
Hi Mathias, That worked, although I had to specify the quirks via a kernel boot parameter. Thanks for your help! Hasan. On Mon, Sep 12, 2016 at 9:40 AM, Mathias Nyman wrote: > On 05.09.2016 19:46, Hasan Mahmood wrote: >> >> System reboots instead of shutting down if xhci is enabled in BIOS and >> USB hub is connected >> >> When I connect the usb hub built into my monitor >> (http://www.samsung.com/uk/support/model/LS27D85KTSN/XU), the system >> will reboot instead of shutting down when I do a shut down. If I >> disable XCHI in the BIOS, the problem does not happen. If I disconnect >> the hub and enable XHCI, the problem does not occur. >> >> Possibly related bugs: >> https://bugzilla.kernel.org/show_bug.cgi?id=76291 >> https://bugzilla.kernel.org/show_bug.cgi?id=66171 >> >> dmi.bios.date: 06/06/2016 >> dmi.bios.vendor: Intel Corporation >> dmi.bios.version: RYBDWi35.86A.0358.2016.0606.1423 >> dmi.board.name: NUC5i5RYB >> dmi.board.vendor: Intel Corporation >> dmi.board.version: H40999-503 >> dmi.chassis.type: 3 >> dmi.modalias: >> dmi:bvnIntelCorporation:bvrRYBDWi35.86A.0358.2016.0606.1423:bd06/06/2016:svn:pn:pvr:rvnIntelCorporation:rnNUC5i5RYB:rvrH40999-503:cvn:ct3:cvr: >> >> $ cat /proc/version >> Linux version 4.8.0-040800rc5-lowlatency (kernel@tangerine) (gcc >> version 6.2.0 20160830 (Ubuntu 6.2.0-2ubuntu11) ) #201609041832 SMP >> PREEMPT Sun Sep 4 22:38:41 UTC 2016 >> >> launchpad bug: >> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1606030 >> > > Launchpad bug shows you machine has a Wildcat Point-LP USB xHCI Controller > [8086:9cb1] > > Launcpad dmesg log also show xhci loads with quirks 0x9810 missing > XHCI_SPURIOUS_REBOOT and XHCI_SPURIOUS_WAKEUP quirks > > xHCI on Wildcat point-LP should be quite similar to Lynxpoint-LP xHCI, > but driver does not set the quirks as they depend on xHCI Lynxpoint PCI > device ID > > Can you try reloading xhci with those quirks (BIT 13 and BIT 18) > In your case the old + new quirks should be 0x0004b810 > > Something like this should do it: > > sudo modprobe -r xhci_pci > sudo modprobe xhci_hcd quirks=0x0004b810 > sudo modprobe xhci_pci > (see from dmesg xhci loaded properly and with the new quirks) > try to shutdown with hub connected > > -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ohci: Allow ohci on omap5 also
* Alan Stern [160910 19:27]: > On Fri, 9 Sep 2016, Alan Stern wrote: > > > On Fri, 9 Sep 2016, Tony Lindgren wrote: > > > > > * Alan Stern [160909 13:41]: > > > > On Fri, 9 Sep 2016, Tony Lindgren wrote: > > > > > > > > > * Alan Stern [160909 12:47]: > > > > > > You know, as far as I can tell the only thing that is OMAP-specific > > > > > > about the ohci-omap3 driver is the initialization of OHCI_CTRL_RWC. > > > > > > > > > > > > This could be added to the platform data or the DT bindings, after > > > > > > which this driver wouldn't be needed at all -- the ohci_platform > > > > > > driver > > > > > > would work. Does this seem reasonable? > > > > > > > > > > Sure makes sense to me. Do we have some standard binding for > > > > > OCHI_CTRL_RWC? > > > > > > > > No, one would have to be created. > > > > > > OK I can do a patch for that, what should be binding be? > > > > > > ohci-ctrl-rwc-something? > > > > OHCI_CTRL_RWC is just a single-bit flag; the RWC part stands for > > "Remote Wakeup Control". > > Sorry, careless mistake on my part. It actually stands for "Remote > Wakeup Connected"; meaning that the remote-wakeup output signal from > the controller is connected to something really can wake up the system. > This is exactly the sort of hardware implementation detail that the > firmware is supposed to know. OK thanks. I think it can be set up as a generic wakeirq. > > The flag indicates whether the controller > > hardware is able to wake up the system, and it is supposed to be set > > initially by the firmware during boot-up. Under DT, the driver will > > have to set it manually. > > > > So you could call it ohci-ctrl-rwc-on, or ohci-ctrl-rwc-enable, or > > anything similar. Use your own judgment. OK Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: chipidea: Properly mark little endian descriptors
The DMA descriptors are little endian, and we do a pretty good
job of handling them with the proper le32_to_cpu() markings, but
we don't actually mark them as __le32. This means checkers like
sparse can't easily find new bugs. Let's mark the members of
structures properly and fix the few places where we're missing
conversions.
Cc: Peter Chen
Cc: Greg Kroah-Hartman
Signed-off-by: Stephen Boyd
---
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/chipidea/udc.h | 12 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 6acf4dba395e..61a65d1d05f2 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -364,7 +364,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct
ci_hw_req *hwreq,
if (hwreq->req.length == 0
|| hwreq->req.length % hwep->ep.maxpacket)
mul++;
- node->ptr->token |= mul << __ffs(TD_MULTO);
+ node->ptr->token |= cpu_to_le32(mul << __ffs(TD_MULTO));
}
temp = (u32) (hwreq->req.dma + hwreq->req.actual);
@@ -503,7 +503,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct
ci_hw_req *hwreq)
if (hwreq->req.length == 0
|| hwreq->req.length % hwep->ep.maxpacket)
mul++;
- hwep->qh.ptr->cap |= mul << __ffs(QH_MULT);
+ hwep->qh.ptr->cap |= cpu_to_le32(mul << __ffs(QH_MULT));
}
wmb(); /* synchronize before ep prime */
@@ -530,7 +530,7 @@ static void free_pending_td(struct ci_hw_ep *hwep)
static int reprime_dtd(struct ci_hdrc *ci, struct ci_hw_ep *hwep,
struct td_node *node)
{
- hwep->qh.ptr->td.next = node->dma;
+ hwep->qh.ptr->td.next = cpu_to_le32(node->dma);
hwep->qh.ptr->td.token &=
cpu_to_le32(~(TD_STATUS_HALTED | TD_STATUS_ACTIVE));
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index e66df0020bd4..2ecd1174d66c 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -22,11 +22,11 @@
/* DMA layout of transfer descriptors */
struct ci_hw_td {
/* 0 */
- u32 next;
+ __le32 next;
#define TD_TERMINATE BIT(0)
#define TD_ADDR_MASK (0xFFEUL << 5)
/* 1 */
- u32 token;
+ __le32 token;
#define TD_STATUS (0x00FFUL << 0)
#define TD_STATUS_TR_ERR BIT(3)
#define TD_STATUS_DT_ERR BIT(5)
@@ -36,7 +36,7 @@ struct ci_hw_td {
#define TD_IOCBIT(15)
#define TD_TOTAL_BYTES(0x7FFFUL << 16)
/* 2 */
- u32 page[5];
+ __le32 page[5];
#define TD_CURR_OFFSET(0x0FFFUL << 0)
#define TD_FRAME_NUM (0x07FFUL << 0)
#define TD_RESERVED_MASK (0x0FFFUL << 0)
@@ -45,18 +45,18 @@ struct ci_hw_td {
/* DMA layout of queue heads */
struct ci_hw_qh {
/* 0 */
- u32 cap;
+ __le32 cap;
#define QH_IOSBIT(15)
#define QH_MAX_PKT(0x07FFUL << 16)
#define QH_ZLTBIT(29)
#define QH_MULT (0x0003UL << 30)
#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
/* 1 */
- u32 curr;
+ __le32 curr;
/* 2 - 8 */
struct ci_hw_td td;
/* 9 */
- u32 RESERVED;
+ __le32 RESERVED;
struct usb_ctrlrequest setup;
} __attribute__ ((packed, aligned(4)));
--
2.9.0.rc2.8.ga28705d
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] usb: chipidea: Emulate OTGSC interrupt enable path
In the case of an extcon-usb-gpio device being used with the
chipidea driver we'll sometimes miss the BSVIS event in the OTGSC
register. Consider the case where we don't have a cable attached
and the id pin is indicating "host" mode. When we plug in the usb
cable for "device" mode a gpio goes high and indicates that we
should do the role switch and that vbus is high. When we're in
"host" mode the OTGSC register doesn't have BSVIE set.
The following scenario can happen:
CPU0
ci_cable_notifier()
update id cable state
ci_irq()
if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) { // true
ci->id_event = true;
ci_otg_queue_work()
schedule()
// same task as before
ci_cable_notifier()
update vbus cable state
ci_irq()
if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) // false
return IRQ_NONE
ci_otg_work() // switch task to the workqueue now
if (ci->id_event)
ci_handle_id_switch()
ci_role_stop()
host_stop()
hw_wait_vbus_lower_bsv(ci); // this times out because vbus is already set
ci_role_start()
udc_id_switch_for_device()
hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE, OTGSC_BSVIS | OTGSC_BSVIE);
At this point, we don't replay the vbus connect event because the
vbus event has already happened. This causes things like gadget
instances to never see vbus appear, and thus the gadget is never
started. Furthermore, we see timeout messages like:
timeout waiting for 800 in OTGSC
Let's workaround this by skiping the wait for BSV when we're
using an extcon for the vbus notification and let's properly
emulate the BSVIS event that would happen when we enable the
vbus interrupt while enabling "device" mode.
Cc: Greg Kroah-Hartman
Signed-off-by: Stephen Boyd
---
This is on top of my patch series that modifies how we handle the extcon
events here. The extcon handling looks racy in this driver even after
this patch. I think we may need to add a spinlock around the cable
state changes and the otgsc register read/write functions. The driver
doesn't seem to expect that the extcon notifiers could run in parallel
with the OTG state machine, and emulating the interrupts is "weird"
in the sense that most of the irq handler in core.c assumes that there's
only one interrupt and so we couldn't possibly be in the irq handler
at the same time on different CPUs (which it can!).
Also, can we just remove the BSV waiting part? From what I can tell, that
is racy if someone can get the workqueue to be delayed significantly enough
to have vbus go high (again) during the role switch. I understand that we're
doing it to prevent a vbus event from happening when we switch to the device
role even though there isn't a cable attached. It just doesn't seem like it's
safe to assume a high-low-high transition won't happen and be reflected in
the status bits. It's really easy to trigger this with an extcon-usb-gpio device
like can be found on 96boards platforms like db410c.
drivers/usb/chipidea/ci.h | 2 ++
drivers/usb/chipidea/core.c | 23 +--
drivers/usb/chipidea/otg.c | 31 ---
3 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 59e22389c10b..e099b8bc79e2 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -437,6 +437,8 @@ static inline void ci_ulpi_exit(struct ci_hdrc *ci) { }
static inline int ci_ulpi_resume(struct ci_hdrc *ci) { return 0; }
#endif
+irqreturn_t __ci_irq(int irq, struct ci_hdrc *ci);
+
u32 hw_read_intr_enable(struct ci_hdrc *ci);
u32 hw_read_intr_status(struct ci_hdrc *ci);
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index ba3a43bbe0ea..fbef1c961572 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -528,9 +528,8 @@ int hw_device_reset(struct ci_hdrc *ci)
return 0;
}
-static irqreturn_t ci_irq(int irq, void *data)
+irqreturn_t __ci_irq(int irq, struct ci_hdrc *ci)
{
- struct ci_hdrc *ci = data;
irqreturn_t ret = IRQ_NONE;
u32 otgsc = 0;
@@ -574,9 +573,20 @@ static irqreturn_t ci_irq(int irq, void *data)
return IRQ_HANDLED;
}
- /* Handle device/host interrupt */
- if (ci->role != CI_ROLE_END)
- ret = ci_role(ci)->irq(ci);
+ return ret;
+}
+
+static irqreturn_t ci_irq(int irq, void *data)
+{
+ irqreturn_t ret;
+ struct ci_hdrc *ci = data;
+
+ ret = __ci_irq(irq, ci);
+ if (ret == IRQ_NONE) {
+ /* Handle device/host interrupt */
+ if (ci->role != CI_ROLE_END)
+ ret = ci_role(ci)->irq(ci);
+ }
return ret;
}
@@ -590,7 +600,8 @@ static int ci_cable_notifier(struct notifier_block *nb,
unsigned long event,
cbl->connected = event;
cbl->changed = true;
- ci_irq(ci->irq, ci);
+ __ci_irq(ci->irq, ci);
+
return
Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Martin Blumenstingl writes: > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel wrote: [...] >>> I added Philipp and Hans to this thread - maybe they can comment on this. >>> To sum it up, our problem is: >>> - there are two separate USB PHYs on Meson GXBB >>> - both are sharing the same reset line (provided by the reset-meson driver) >>> - during initialization of the PHYs we must only call >>> reset_control_reset(rstc) once (if we do it for the first *and* second >>> PHY then the first PHY gets confused once the second PHY uses the >>> reset because the first PHY's state is reset as well) >> >> If you have an initially asserted reset line and you can enable the >> first module by deasserting the reset via reset_control_deassert (and >> reset_control_assert to signal when the module may be disabled again >> after use), shared resets are for you. >> >> If you need a reset pulse or have no direct control over the reset line, >> (device_reset), the reset framework currently has no solution for this. >> The ugly thing about reset_control_once would be that it can't re-reset >> modules when unloading and reloading driver modules. > > The corresponding reset driver in question is reset-meson, which only > implements reset (assert/deassert are not implemented). However, I > don't know if this is due to hardware design. > I think the hardware implements the latter, but maybe Neil can give > more information here (I currently don't have access to my board so I > cannot test how the hardware actually behaves). It's implemented that way because the hardware only supports a reset pulse. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
On the db410c 96boards platform we have a TC7USB40MU[1] on the
board to mux the D+/D- lines from the SoC between a micro usb
"device" port and a USB hub for "host" roles. Upon a role switch,
we need to change this mux to forward the D+/D- lines to either
the port or the hub. Therefore, introduce a driver for this
device that intercepts extcon USB_HOST events and logically
asserts a gpio to mux the "host" D+/D- lines when a host cable is
attached. When the cable goes away, it will logically deassert
the gpio and mux the "device" lines.
[1]
https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
Cc: MyungJoo Ham
Cc: Chanwoo Choi
Cc:
Signed-off-by: Stephen Boyd
---
Should I make the extcon part optional? I could see a case where there are two
"OTG" ports connected to the mux (or two hubs), and for some reason the
software may want to mux between them at runtime. If we mandate an extcon,
that won't be possible to support. Perhaps it would be better to have
the node, but connect it to the usb controller with a phandle (maybe of_graph
endpoints would be useful too) so that when the controller wants to mux over
a port it can do so.
Muxing the ports this way based on ID cable is pretty much a software
design decision. We could mux the ports during the role switch, and the
role switch can be entirely userspace driven with the chipidea controller
that I'm using (see the role switching support in the "role" file for
debugfs support in that driver). So extcon cables don't come into the picture
in that scenario.
.../devicetree/bindings/usb/toshiba,tc7usb40mu.txt | 34 +++
drivers/usb/misc/Kconfig | 9 ++
drivers/usb/misc/Makefile | 1 +
drivers/usb/misc/tc7usb40mu.c | 107 +
4 files changed, 151 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
create mode 100644 drivers/usb/misc/tc7usb40mu.c
diff --git a/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
new file mode 100644
index ..18e6607408fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
@@ -0,0 +1,34 @@
+Toshiba TC7USB40MU
+
+This device muxes USB D+/D- lines between two outputs called 1D+/1D- and
2D+/2D-.
+When the switch pin is asserted, we mux out 2D+/2D-, and when it's deasserted
we
+select 1D+/1D-.
+
+This can be used to mux USB D+/D- lines between a USB hub and an OTG port.
+
+PROPERTIES
+
+- compatible:
+Usage: required
+Value type:
+Definition: Should contain "toshiba,tc7usb40mu"
+
+- switch-gpios:
+Usage: required
+Value type:
+Definition: Should contain the gpio used to toggle the switch. Logically
+asserting the gpio will cause the device to mux the "host"
+D+/D- lines instead of the "device" lines.
+
+- extcon:
+Usage: required
+Value type:
+Definition: Should contain the extcon device for USB_HOST cable events
+
+Example:
+
+ usb-switch {
+ compatible = "toshiba,tc7usb40mu";
+ switch-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
+ extcon = <&usb_id>;
+ };
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 47b357760afc..3da568c751d2 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -46,6 +46,15 @@ config USB_SEVSEG
To compile this driver as a module, choose M here: the
module will be called usbsevseg.
+config USB_TC7USB40MU
+ tristate "TC7USB40MU USB mux support"
+ depends on (GPIOLIB && EXTCON) || COMPILE_TEST
+ help
+ Say Y here if you have a TC7USB40MU by Toshiba. If a USB ID cable is
+ present, a gpio will be asserted to mux out "host" D+/D- lines and
when
+ the ID cable is removed, a gpio will be deasserted to mux out "device"
+ D+/D- lines.
+
config USB_RIO500
tristate "USB Diamond Rio500 support"
help
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 3d1992750da4..d8f9ad1dee13 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_USB_LEGOTOWER) += legousbtower.o
obj-$(CONFIG_USB_RIO500) += rio500.o
obj-$(CONFIG_USB_TEST) += usbtest.o
obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)+= ehset.o
+obj-$(CONFIG_USB_TC7USB40MU) += tc7usb40mu.o
obj-$(CONFIG_USB_TRANCEVIBRATOR) += trancevibrator.o
obj-$(CONFIG_USB_USS720) += uss720.o
obj-$(CONFIG_USB_SEVSEG) += usbsevseg.o
diff --git a/drivers/usb/misc/tc7usb40mu.c b/drivers/usb/misc/tc7usb40mu.c
new file mode 100644
index ..9edcfe577ae4
--- /dev/null
+++ b/drivers/usb/misc/tc7usb40mu.c
@@ -0,0 +1,107 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can
[PATCH 1/1] usb: Kconfig: using select for USB_COMMON dependency
According to (badf6d47f8a9 "usb: common: rework CONFIG_USB_COMMON logic") we should select USB_COMMON at Kconfig when usb common stuffs are needed, but some of Kconfig enties have not followed it, update them. Cc: Arnd Bergmann Cc: Felipe Balbi Cc: Heikki Krogerus Signed-off-by: Peter Chen --- drivers/usb/Kconfig | 5 +++-- drivers/usb/usbip/Kconfig | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 2b9159a..644e978 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -152,7 +152,8 @@ source "drivers/usb/gadget/Kconfig" config USB_LED_TRIG bool "USB LED Triggers" - depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS + depends on LEDS_CLASS && LEDS_TRIGGERS + select USB_COMMON help This option adds LED triggers for USB host and/or gadget activity. @@ -162,7 +163,7 @@ config USB_LED_TRIG config USB_ULPI_BUS tristate "USB ULPI PHY interface support" - depends on USB_COMMON + select USB_COMMON help UTMI+ Low Pin Interface (ULPI) is specification for a commonly used USB 2.0 PHY interface. The ULPI specification defines a standard set diff --git a/drivers/usb/usbip/Kconfig b/drivers/usb/usbip/Kconfig index 29492c7..eeefa29 100644 --- a/drivers/usb/usbip/Kconfig +++ b/drivers/usb/usbip/Kconfig @@ -1,6 +1,7 @@ config USBIP_CORE tristate "USB/IP support" - depends on USB_COMMON && NET + depends on NET + select USB_COMMON ---help--- This enables pushing USB packets over IP to allow remote machines direct access to USB devices. It provides the -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 22/22] phy: Add support for Qualcomm's USB HS phy
On Tue, Sep 13, 2016 at 01:41:44PM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-09-13 00:03:58) > > On Wed, Sep 07, 2016 at 02:35:19PM -0700, Stephen Boyd wrote: > > > The high-speed phy on qcom SoCs is controlled via the ULPI > > > viewport. > > > > > > > Hi Stephen, I am a little puzzled how this driver co-work with chipidea > > driver. According to nxp IC guys, the ULPI PHY's clock needs to be enabled > > before access portsc.pts (calling hw_phymode_configure), otherwise, > > the system will hang. But I find you call hw_phymode_configure before > > phy->power_on, doesn't your design have this requirement? > > Which clk needs to be enabled? The xcvr_clk? I believe that clk > corresponds to the "core" clk that we enable in the msm glue driver > layer. When that clk is enabled, the ULPI phy is able to respond to > register read/writes via the ULPI viewport. > The input clock for ULPI PHY, maybe it is ref_clk at this PHY driver, so in your platform, even PHY clock is gated, you can still access portsc.pts to configure PHY mode at controller register? > > > > Besides, you read ulpi id before phy->power_on, how can read work before > > phy power on? > > > > I've found that even having the link clk enabled before phy->power_on > doesn't mean it's possible to read the id registers though. That's > because there can be other power supplies, like regulators, which need > to be on for the phy to operate properly. > Then I am puzzled the current initialization for your case, in my mind, it should like below: qcom_usb_hs_phy_probe->qcom_usb_hs_phy_power_on->ci_ulpi_init Like other PHYs, it should get PHY first, then power on it, after that, you can access its register. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: Properly mark little endian descriptors
On Tue, Sep 13, 2016 at 04:06:31PM -0700, Stephen Boyd wrote:
> The DMA descriptors are little endian, and we do a pretty good
> job of handling them with the proper le32_to_cpu() markings, but
> we don't actually mark them as __le32. This means checkers like
> sparse can't easily find new bugs. Let's mark the members of
> structures properly and fix the few places where we're missing
> conversions.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/chipidea/udc.h | 12 ++--
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 6acf4dba395e..61a65d1d05f2 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -364,7 +364,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct
> ci_hw_req *hwreq,
> if (hwreq->req.length == 0
> || hwreq->req.length % hwep->ep.maxpacket)
> mul++;
> - node->ptr->token |= mul << __ffs(TD_MULTO);
> + node->ptr->token |= cpu_to_le32(mul << __ffs(TD_MULTO));
> }
>
> temp = (u32) (hwreq->req.dma + hwreq->req.actual);
> @@ -503,7 +503,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep,
> struct ci_hw_req *hwreq)
> if (hwreq->req.length == 0
> || hwreq->req.length % hwep->ep.maxpacket)
> mul++;
> - hwep->qh.ptr->cap |= mul << __ffs(QH_MULT);
> + hwep->qh.ptr->cap |= cpu_to_le32(mul << __ffs(QH_MULT));
> }
>
> wmb(); /* synchronize before ep prime */
> @@ -530,7 +530,7 @@ static void free_pending_td(struct ci_hw_ep *hwep)
> static int reprime_dtd(struct ci_hdrc *ci, struct ci_hw_ep *hwep,
> struct td_node *node)
> {
> - hwep->qh.ptr->td.next = node->dma;
> + hwep->qh.ptr->td.next = cpu_to_le32(node->dma);
> hwep->qh.ptr->td.token &=
> cpu_to_le32(~(TD_STATUS_HALTED | TD_STATUS_ACTIVE));
>
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index e66df0020bd4..2ecd1174d66c 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -22,11 +22,11 @@
> /* DMA layout of transfer descriptors */
> struct ci_hw_td {
> /* 0 */
> - u32 next;
> + __le32 next;
> #define TD_TERMINATE BIT(0)
> #define TD_ADDR_MASK (0xFFEUL << 5)
> /* 1 */
> - u32 token;
> + __le32 token;
> #define TD_STATUS (0x00FFUL << 0)
> #define TD_STATUS_TR_ERR BIT(3)
> #define TD_STATUS_DT_ERR BIT(5)
> @@ -36,7 +36,7 @@ struct ci_hw_td {
> #define TD_IOCBIT(15)
> #define TD_TOTAL_BYTES(0x7FFFUL << 16)
> /* 2 */
> - u32 page[5];
> + __le32 page[5];
> #define TD_CURR_OFFSET(0x0FFFUL << 0)
> #define TD_FRAME_NUM (0x07FFUL << 0)
> #define TD_RESERVED_MASK (0x0FFFUL << 0)
> @@ -45,18 +45,18 @@ struct ci_hw_td {
> /* DMA layout of queue heads */
> struct ci_hw_qh {
> /* 0 */
> - u32 cap;
> + __le32 cap;
> #define QH_IOSBIT(15)
> #define QH_MAX_PKT(0x07FFUL << 16)
> #define QH_ZLTBIT(29)
> #define QH_MULT (0x0003UL << 30)
> #define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> /* 1 */
> - u32 curr;
> + __le32 curr;
> /* 2 - 8 */
> struct ci_hw_td td;
> /* 9 */
> - u32 RESERVED;
> + __le32 RESERVED;
> struct usb_ctrlrequest setup;
> } __attribute__ ((packed, aligned(4)));
>
> --
Good catch, thanks.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: Properly mark little endian descriptors
On Wed, Sep 14, 2016 at 10:37:40AM +0800, Peter Chen wrote:
> On Tue, Sep 13, 2016 at 04:06:31PM -0700, Stephen Boyd wrote:
> > The DMA descriptors are little endian, and we do a pretty good
> > job of handling them with the proper le32_to_cpu() markings, but
> > we don't actually mark them as __le32. This means checkers like
> > sparse can't easily find new bugs. Let's mark the members of
> > structures properly and fix the few places where we're missing
> > conversions.
> >
> > Cc: Peter Chen
> > Cc: Greg Kroah-Hartman
> > Signed-off-by: Stephen Boyd
> > ---
> > drivers/usb/chipidea/udc.c | 6 +++---
> > drivers/usb/chipidea/udc.h | 12 ++--
> > 2 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 6acf4dba395e..61a65d1d05f2 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -364,7 +364,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct
> > ci_hw_req *hwreq,
> > if (hwreq->req.length == 0
> > || hwreq->req.length % hwep->ep.maxpacket)
> > mul++;
> > - node->ptr->token |= mul << __ffs(TD_MULTO);
> > + node->ptr->token |= cpu_to_le32(mul << __ffs(TD_MULTO));
> > }
> >
> > temp = (u32) (hwreq->req.dma + hwreq->req.actual);
> > @@ -503,7 +503,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep,
> > struct ci_hw_req *hwreq)
> > if (hwreq->req.length == 0
> > || hwreq->req.length % hwep->ep.maxpacket)
> > mul++;
> > - hwep->qh.ptr->cap |= mul << __ffs(QH_MULT);
> > + hwep->qh.ptr->cap |= cpu_to_le32(mul << __ffs(QH_MULT));
> > }
> >
> > wmb(); /* synchronize before ep prime */
> > @@ -530,7 +530,7 @@ static void free_pending_td(struct ci_hw_ep *hwep)
> > static int reprime_dtd(struct ci_hdrc *ci, struct ci_hw_ep *hwep,
> >struct td_node *node)
> > {
> > - hwep->qh.ptr->td.next = node->dma;
> > + hwep->qh.ptr->td.next = cpu_to_le32(node->dma);
> > hwep->qh.ptr->td.token &=
> > cpu_to_le32(~(TD_STATUS_HALTED | TD_STATUS_ACTIVE));
> >
> > diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> > index e66df0020bd4..2ecd1174d66c 100644
> > --- a/drivers/usb/chipidea/udc.h
> > +++ b/drivers/usb/chipidea/udc.h
> > @@ -22,11 +22,11 @@
> > /* DMA layout of transfer descriptors */
> > struct ci_hw_td {
> > /* 0 */
> > - u32 next;
> > + __le32 next;
> > #define TD_TERMINATE BIT(0)
> > #define TD_ADDR_MASK (0xFFEUL << 5)
> > /* 1 */
> > - u32 token;
> > + __le32 token;
> > #define TD_STATUS (0x00FFUL << 0)
> > #define TD_STATUS_TR_ERR BIT(3)
> > #define TD_STATUS_DT_ERR BIT(5)
> > @@ -36,7 +36,7 @@ struct ci_hw_td {
> > #define TD_IOCBIT(15)
> > #define TD_TOTAL_BYTES(0x7FFFUL << 16)
> > /* 2 */
> > - u32 page[5];
> > + __le32 page[5];
> > #define TD_CURR_OFFSET(0x0FFFUL << 0)
> > #define TD_FRAME_NUM (0x07FFUL << 0)
> > #define TD_RESERVED_MASK (0x0FFFUL << 0)
> > @@ -45,18 +45,18 @@ struct ci_hw_td {
> > /* DMA layout of queue heads */
> > struct ci_hw_qh {
> > /* 0 */
> > - u32 cap;
> > + __le32 cap;
> > #define QH_IOSBIT(15)
> > #define QH_MAX_PKT(0x07FFUL << 16)
> > #define QH_ZLTBIT(29)
> > #define QH_MULT (0x0003UL << 30)
> > #define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> > /* 1 */
> > - u32 curr;
> > + __le32 curr;
> > /* 2 - 8 */
> > struct ci_hw_td td;
> > /* 9 */
> > - u32 RESERVED;
> > + __le32 RESERVED;
> > struct usb_ctrlrequest setup;
> > } __attribute__ ((packed, aligned(4)));
> >
> > --
I am afraid I can't apply for testing
Applying: usb: chipidea: Properly mark little endian descriptors
fatal: sha1 information is lacking or useless
(drivers/usb/chipidea/udc.c).
error: could not build fake ancestor
Patch failed at 0001 usb: chipidea: Properly mark little endian
descriptors
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Would you please rebase my ci-for-usb-next branch?
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> On the db410c 96boards platform we have a TC7USB40MU[1] on the
> board to mux the D+/D- lines from the SoC between a micro usb
> "device" port and a USB hub for "host" roles. Upon a role switch,
> we need to change this mux to forward the D+/D- lines to either
> the port or the hub. Therefore, introduce a driver for this
> device that intercepts extcon USB_HOST events and logically
> asserts a gpio to mux the "host" D+/D- lines when a host cable is
> attached. When the cable goes away, it will logically deassert
> the gpio and mux the "device" lines.
Would you please draw the design? It can also help me review your
chipidea patch well.
1. How many ports on the board?
2. How the lines are connected on the board?
Peter
>
> [1]
> https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
>
> Cc: MyungJoo Ham
> Cc: Chanwoo Choi
> Cc:
> Signed-off-by: Stephen Boyd
> ---
>
> Should I make the extcon part optional? I could see a case where there are two
> "OTG" ports connected to the mux (or two hubs), and for some reason the
> software may want to mux between them at runtime. If we mandate an extcon,
> that won't be possible to support. Perhaps it would be better to have
> the node, but connect it to the usb controller with a phandle (maybe of_graph
> endpoints would be useful too) so that when the controller wants to mux over
> a port it can do so.
>
> Muxing the ports this way based on ID cable is pretty much a software
> design decision. We could mux the ports during the role switch, and the
> role switch can be entirely userspace driven with the chipidea controller
> that I'm using (see the role switching support in the "role" file for
> debugfs support in that driver). So extcon cables don't come into the picture
> in that scenario.
>
> .../devicetree/bindings/usb/toshiba,tc7usb40mu.txt | 34 +++
> drivers/usb/misc/Kconfig | 9 ++
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/tc7usb40mu.c | 107
> +
> 4 files changed, 151 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> create mode 100644 drivers/usb/misc/tc7usb40mu.c
>
> diff --git a/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> new file mode 100644
> index ..18e6607408fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/toshiba,tc7usb40mu.txt
> @@ -0,0 +1,34 @@
> +Toshiba TC7USB40MU
> +
> +This device muxes USB D+/D- lines between two outputs called 1D+/1D- and
> 2D+/2D-.
> +When the switch pin is asserted, we mux out 2D+/2D-, and when it's
> deasserted we
> +select 1D+/1D-.
> +
> +This can be used to mux USB D+/D- lines between a USB hub and an OTG port.
> +
> +PROPERTIES
> +
> +- compatible:
> +Usage: required
> +Value type:
> +Definition: Should contain "toshiba,tc7usb40mu"
> +
> +- switch-gpios:
> +Usage: required
> +Value type:
> +Definition: Should contain the gpio used to toggle the switch. Logically
> +asserting the gpio will cause the device to mux the "host"
> +D+/D- lines instead of the "device" lines.
> +
> +- extcon:
> +Usage: required
> +Value type:
> +Definition: Should contain the extcon device for USB_HOST cable events
> +
> +Example:
> +
> + usb-switch {
> + compatible = "toshiba,tc7usb40mu";
> + switch-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
> + extcon = <&usb_id>;
> + };
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 47b357760afc..3da568c751d2 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -46,6 +46,15 @@ config USB_SEVSEG
> To compile this driver as a module, choose M here: the
> module will be called usbsevseg.
>
> +config USB_TC7USB40MU
> + tristate "TC7USB40MU USB mux support"
> + depends on (GPIOLIB && EXTCON) || COMPILE_TEST
> + help
> + Say Y here if you have a TC7USB40MU by Toshiba. If a USB ID cable is
> + present, a gpio will be asserted to mux out "host" D+/D- lines and
> when
> + the ID cable is removed, a gpio will be deasserted to mux out "device"
> + D+/D- lines.
> +
> config USB_RIO500
> tristate "USB Diamond Rio500 support"
> help
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 3d1992750da4..d8f9ad1dee13 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_USB_LEGOTOWER) += legousbtower.o
> obj-$(CONFIG_USB_RIO500) += rio500.o
> obj-$(CONFIG_USB_TEST) += usbtest.o
> obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)+= ehset.o
> +obj-$(CONFIG_USB_TC7USB40MU) += tc7usb40mu.o
> obj-$(CO
Re: [PATCH v4 22/22] phy: Add support for Qualcomm's USB HS phy
On Saturday 10 September 2016 05:48 PM, Kishon Vijay Abraham I wrote:
>
> On Wed, Sep 07, 2016 at 02:35:19PM -0700, Stephen Boyd wrote:
>> The high-speed phy on qcom SoCs is controlled via the ULPI
>> viewport.
>>
>> Cc: Kishon Vijay Abraham I
>> Cc:
>> Signed-off-by: Stephen Boyd
>
> merged this and the previous patch to linux-phy.
since there are pending discussions, I'll drop this patch for now.
Thanks
Kishon
>
> Thanks
> Kishon
>
>> ---
>> .../devicetree/bindings/phy/qcom,usb-hs-phy.txt| 83 ++
>> drivers/phy/Kconfig| 8 +
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-qcom-usb-hs.c | 289
>> +
>> 4 files changed, 381 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
>> create mode 100644 drivers/phy/phy-qcom-usb-hs.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
>> b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
>> new file mode 100644
>> index ..d7eacd63d06b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
>> @@ -0,0 +1,83 @@
>> +Qualcomm's USB HS PHY
>> +
>> +PROPERTIES
>> +
>> +- compatible:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain "qcom,usb-hs-phy" and more specifically one
>> of the
>> +following:
>> +
>> +"qcom,usb-hs-phy-apq8064"
>> +"qcom,usb-hs-phy-msm8916"
>> +"qcom,usb-hs-phy-msm8974"
>> +
>> +- #phy-cells:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain 0
>> +
>> +- clocks:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain clock specifier for the reference and sleep
>> +clocks
>> +
>> +- clock-names:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain "ref" and "sleep" for the reference and sleep
>> +clocks respectively
>> +
>> +- resets:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain the phy and POR resets
>> +
>> +- reset-names:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain "phy" and "por" for the phy and POR resets
>> +respectively
>> +
>> +- v3p3-supply:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain a reference to the 3.3V supply
>> +
>> +- v1p8-supply:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain a reference to the 1.8V supply
>> +
>> +- extcon:
>> +Usage: optional
>> +Value type:
>> +Definition: Should contain the vbus and ID extcons in the first and
>> second
>> +cells respectively
>> +
>> +- qcom,init-seq:
>> +Usage: optional
>> +Value type:
>> +Definition: Should contain a sequence of ULPI register and address
>> pairs to
>> +program into the ULPI_EXT_VENDOR_SPECIFIC area. This is
>> related
>> +to Device Mode Eye Diagram test.
>> +
>> +EXAMPLE
>> +
>> +otg: usb-controller {
>> +ulpi {
>> +phy {
>> +compatible = "qcom,usb-hs-phy-msm8974",
>> "qcom,usb-hs-phy";
>> +#phy-cells = <0>;
>> +clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
>> +clock-names = "ref", "sleep";
>> +resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;
>> +reset-names = "phy", "por";
>> +v3p3-supply = <&pm8941_l24>;
>> +v1p8-supply = <&pm8941_l6>;
>> +extcon = <&smbb>, <&usb_id>;
>> +qcom,init-seq = /bits/ 8 <0x81 0x63>;
>> +};
>> +};
>> +};
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 830c443eeabf..ee0ec021a98c 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -417,6 +417,14 @@ config PHY_QCOM_UFS
>> help
>>Support for UFS PHY on QCOM chipsets.
>>
>> +config PHY_QCOM_USB_HS
>> +tristate "Qualcomm USB HS PHY module"
>> +depends on USB_ULPI_BUS
>> +select GENERIC_PHY
>> +help
>> + Support for the USB high-speed ULPI compliant phy on Qualcomm
>> + chipsets.
>> +
>> config PHY_QCOM_USB_HSIC
>> tristate "Qualcomm USB HSIC ULPI PHY module"
>> depends on USB_ULPI_BUS
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 5422f543d17d..31c84faa07fa 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -50,6 +50,7 @@ obj-$(CONFIG_PHY_STIH41X_USB) +=
>> phy-stih41x-usb.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
>> +obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o
>> obj-$(CONFIG_
[PATCH/REBASED] usb: chipidea: Properly mark little endian descriptors
The DMA descriptors are little endian, and we do a pretty good
job of handling them with the proper le32_to_cpu() markings, but
we don't actually mark them as __le32. This means checkers like
sparse can't easily find new bugs. Let's mark the members of
structures properly and fix the few places where we're missing
conversions.
Cc: Peter Chen
Cc: Greg Kroah-Hartman
Signed-off-by: Stephen Boyd
---
Peter Chen wrote:
>
> I am afraid I can't apply for testing
>
> Applying: usb: chipidea: Properly mark little endian descriptors
> fatal: sha1 information is lacking or useless
> (drivers/usb/chipidea/udc.c).
> error: could not build fake ancestor
> Patch failed at 0001 usb: chipidea: Properly mark little endian
> descriptors
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Would you please rebase my ci-for-usb-next branch?
Sure no problem. Maybe you shouldn't specify a 3 way merge so that
it attempts to apply without building a fake ancestor? Anyway, here
it is.
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/chipidea/udc.h | 12 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 661f43fe0f9e..a9b07befd398 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -365,7 +365,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct
ci_hw_req *hwreq,
if (hwreq->req.length == 0
|| hwreq->req.length % hwep->ep.maxpacket)
mul++;
- node->ptr->token |= mul << __ffs(TD_MULTO);
+ node->ptr->token |= cpu_to_le32(mul << __ffs(TD_MULTO));
}
temp = (u32) (hwreq->req.dma + hwreq->req.actual);
@@ -504,7 +504,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct
ci_hw_req *hwreq)
if (hwreq->req.length == 0
|| hwreq->req.length % hwep->ep.maxpacket)
mul++;
- hwep->qh.ptr->cap |= mul << __ffs(QH_MULT);
+ hwep->qh.ptr->cap |= cpu_to_le32(mul << __ffs(QH_MULT));
}
ret = hw_ep_prime(ci, hwep->num, hwep->dir,
@@ -529,7 +529,7 @@ static void free_pending_td(struct ci_hw_ep *hwep)
static int reprime_dtd(struct ci_hdrc *ci, struct ci_hw_ep *hwep,
struct td_node *node)
{
- hwep->qh.ptr->td.next = node->dma;
+ hwep->qh.ptr->td.next = cpu_to_le32(node->dma);
hwep->qh.ptr->td.token &=
cpu_to_le32(~(TD_STATUS_HALTED | TD_STATUS_ACTIVE));
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index e66df0020bd4..2ecd1174d66c 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -22,11 +22,11 @@
/* DMA layout of transfer descriptors */
struct ci_hw_td {
/* 0 */
- u32 next;
+ __le32 next;
#define TD_TERMINATE BIT(0)
#define TD_ADDR_MASK (0xFFEUL << 5)
/* 1 */
- u32 token;
+ __le32 token;
#define TD_STATUS (0x00FFUL << 0)
#define TD_STATUS_TR_ERR BIT(3)
#define TD_STATUS_DT_ERR BIT(5)
@@ -36,7 +36,7 @@ struct ci_hw_td {
#define TD_IOCBIT(15)
#define TD_TOTAL_BYTES(0x7FFFUL << 16)
/* 2 */
- u32 page[5];
+ __le32 page[5];
#define TD_CURR_OFFSET(0x0FFFUL << 0)
#define TD_FRAME_NUM (0x07FFUL << 0)
#define TD_RESERVED_MASK (0x0FFFUL << 0)
@@ -45,18 +45,18 @@ struct ci_hw_td {
/* DMA layout of queue heads */
struct ci_hw_qh {
/* 0 */
- u32 cap;
+ __le32 cap;
#define QH_IOSBIT(15)
#define QH_MAX_PKT(0x07FFUL << 16)
#define QH_ZLTBIT(29)
#define QH_MULT (0x0003UL << 30)
#define QH_ISO_MULT(x) ((x >> 11) & 0x03)
/* 1 */
- u32 curr;
+ __le32 curr;
/* 2 - 8 */
struct ci_hw_td td;
/* 9 */
- u32 RESERVED;
+ __le32 RESERVED;
struct usb_ctrlrequest setup;
} __attribute__ ((packed, aligned(4)));
--
2.9.0.rc2.8.ga28705d
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/REBASED] usb: chipidea: Properly mark little endian descriptors
On Tue, Sep 13, 2016 at 10:53:02PM -0700, Stephen Boyd wrote:
> The DMA descriptors are little endian, and we do a pretty good
> job of handling them with the proper le32_to_cpu() markings, but
> we don't actually mark them as __le32. This means checkers like
> sparse can't easily find new bugs. Let's mark the members of
> structures properly and fix the few places where we're missing
> conversions.
>
> Cc: Peter Chen
> Cc: Greg Kroah-Hartman
> Signed-off-by: Stephen Boyd
> ---
>
> Peter Chen wrote:
> >
> > I am afraid I can't apply for testing
> >
> > Applying: usb: chipidea: Properly mark little endian descriptors
> > fatal: sha1 information is lacking or useless
> > (drivers/usb/chipidea/udc.c).
> > error: could not build fake ancestor
> > Patch failed at 0001 usb: chipidea: Properly mark little endian
> > descriptors
> > The copy of the patch that failed is found in: .git/rebase-apply/patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> > Would you please rebase my ci-for-usb-next branch?
>
> Sure no problem. Maybe you shouldn't specify a 3 way merge so that
> it attempts to apply without building a fake ancestor? Anyway, here
> it is.
I tried w/o 3 ways merge, the new one can be applied.
Peter
>
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/chipidea/udc.h | 12 ++--
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 661f43fe0f9e..a9b07befd398 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -365,7 +365,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct
> ci_hw_req *hwreq,
> if (hwreq->req.length == 0
> || hwreq->req.length % hwep->ep.maxpacket)
> mul++;
> - node->ptr->token |= mul << __ffs(TD_MULTO);
> + node->ptr->token |= cpu_to_le32(mul << __ffs(TD_MULTO));
> }
>
> temp = (u32) (hwreq->req.dma + hwreq->req.actual);
> @@ -504,7 +504,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep,
> struct ci_hw_req *hwreq)
> if (hwreq->req.length == 0
> || hwreq->req.length % hwep->ep.maxpacket)
> mul++;
> - hwep->qh.ptr->cap |= mul << __ffs(QH_MULT);
> + hwep->qh.ptr->cap |= cpu_to_le32(mul << __ffs(QH_MULT));
> }
>
> ret = hw_ep_prime(ci, hwep->num, hwep->dir,
> @@ -529,7 +529,7 @@ static void free_pending_td(struct ci_hw_ep *hwep)
> static int reprime_dtd(struct ci_hdrc *ci, struct ci_hw_ep *hwep,
> struct td_node *node)
> {
> - hwep->qh.ptr->td.next = node->dma;
> + hwep->qh.ptr->td.next = cpu_to_le32(node->dma);
> hwep->qh.ptr->td.token &=
> cpu_to_le32(~(TD_STATUS_HALTED | TD_STATUS_ACTIVE));
>
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index e66df0020bd4..2ecd1174d66c 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -22,11 +22,11 @@
> /* DMA layout of transfer descriptors */
> struct ci_hw_td {
> /* 0 */
> - u32 next;
> + __le32 next;
> #define TD_TERMINATE BIT(0)
> #define TD_ADDR_MASK (0xFFEUL << 5)
> /* 1 */
> - u32 token;
> + __le32 token;
> #define TD_STATUS (0x00FFUL << 0)
> #define TD_STATUS_TR_ERR BIT(3)
> #define TD_STATUS_DT_ERR BIT(5)
> @@ -36,7 +36,7 @@ struct ci_hw_td {
> #define TD_IOCBIT(15)
> #define TD_TOTAL_BYTES(0x7FFFUL << 16)
> /* 2 */
> - u32 page[5];
> + __le32 page[5];
> #define TD_CURR_OFFSET(0x0FFFUL << 0)
> #define TD_FRAME_NUM (0x07FFUL << 0)
> #define TD_RESERVED_MASK (0x0FFFUL << 0)
> @@ -45,18 +45,18 @@ struct ci_hw_td {
> /* DMA layout of queue heads */
> struct ci_hw_qh {
> /* 0 */
> - u32 cap;
> + __le32 cap;
> #define QH_IOSBIT(15)
> #define QH_MAX_PKT(0x07FFUL << 16)
> #define QH_ZLTBIT(29)
> #define QH_MULT (0x0003UL << 30)
> #define QH_ISO_MULT(x) ((x >> 11) & 0x03)
> /* 1 */
> - u32 curr;
> + __le32 curr;
> /* 2 - 8 */
> struct ci_hw_td td;
> /* 9 */
> - u32 RESERVED;
> + __le32 RESERVED;
> struct usb_ctrlrequest setup;
> } __attribute__ ((packed, aligned(4)));
>
> --
> 2.9.0.rc2.8.ga28705d
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a m
