Re: [PATCH v9 00/14] USB OTG/dual-role framework
On Wed, Jun 8, 2016 at 5:03 PM, Roger Quadros wrote:
> Hi,
>
> This series centralizes OTG/Dual-role functionality in the kernel.
> As of now I've got Dual-role functionality working pretty reliably on
> dra7-evm and am437x-gp-evm.
>
> DWC3 controller and TI platform related patches will be sent separately.
>
> Series is based on v4.7-rc1.
>
Roger, I will wait your dwc3 and TI patch set since one framework needs
one user at least, thanks.
Peter
> Why?:
>
>
> Currently there is no central location where OTG/dual-role functionality is
> implemented in the Linux USB stack and every USB controller driver is
> doing their own thing for OTG/dual-role. We can benefit from code-reuse
> and simplicity by adding the OTG/dual-role core driver.
>
> Newer OTG cores support standard host interface (e.g. xHCI) so
> host and gadget functionality are no longer closely knit like older
> cores. There needs to be a way to co-ordinate the operation of the
> host and gadget controllers in dual-role mode. i.e. to stop and start them
> from a central location. This central location should be the
> USB OTG/dual-role core.
>
> Host and gadget controllers might be sharing resources and can't
> be always running. One has to be stopped for the other to run.
> This couldn't be done till now but can be done from the OTG core.
>
> What?:
> -
>
> The OTG/dual-role core consists of a set of APIs that allow
> registration of OTG controller device and OTG capable host and gadget
> controllers.
>
> - The OTG controller driver can provide the OTG capabilities and the
> Finite State Machine work function via 'struct usb_otg_config'
> at the time of registration i.e. usb_otg_register();
>
> struct usb_otg *usb_otg_register(struct device *dev,
> struct usb_otg_config *config);
> int usb_otg_unregister(struct device *dev);
> /**
> * struct usb_otg_config - otg controller configuration
> * @caps: otg capabilities of the controller
> * @ops: otg fsm operations
> * @otg_work: optional custom otg state machine work function
> */
> struct usb_otg_config {
> struct usb_otg_caps *otg_caps;
> struct otg_fsm_ops *fsm_ops;
> void (*otg_work)(struct work_struct *work);
> };
>
> The dual-role state machine is built-into the OTG core so nothing
> special needs to be provided if only dual-role functionality is desired.
> The low level OTG controller driver ops are povided via
> 'struct otg_fsm_ops *fsm_ops' in the 'struct usb_otg_config'.
>
> After registration, the OTG core waits for host, gadget controller
> and the gadget function driver to be registered. Once all resources are
> available it instantiates the Finite State Machine (FSM).
> The host/gadget controllers are started/stopped according to the FSM.
>
> - Host and gadget controllers that are a part of OTG/dual-role port must
> use the OTG core provided APIs to add/remove the host/gadget.
> i.e. hosts must use usb_otg_add_hcd() usb_otg_remove_hcd(),,
> gadgets must use usb_otg_add_gadget_udc() usb_del_gadget_udc().
> This ensures that the host and gadget controllers are not started till
> the state machine is ready and the right bus conditions are met.
> It also allows the host and gadget controllers to provide the OTG
> controller device to link them together. For Device tree boots
> the related OTG controller is automatically picked up via the
> 'otg-controller' property in the Host/Gadget controller nodes.
>
> int usb_otg_add_hcd(struct usb_hcd *hcd,
> unsigned int irqnum, unsigned long irqflags,
> struct device *otg_dev);
> void usb_otg_remove_hcd(struct usb_hcd *hcd);
>
> int usb_otg_add_gadget_udc(struct device *parent,
>struct usb_gadget *gadget,
>struct device *otg_dev);
> usb_del_gadget_udc() must be used for removal.
>
>
> - During the lifetime of the FSM, the OTG controller driver can provide
> inputs event changes using usb_otg_sync_inputs(). The OTG core will
> then schedule the FSM work function (or internal dual-role state machine)
> to update the FSM state. The FSM then calls the OTG controller
> operations (fsm_ops) as necessary.
> void usb_otg_sync_inputs(struct usb_otg *otg);
>
> - The following 2 functions are provided as helpers for use by the
> OTG controller driver to start/stop the host/gadget controllers.
> int usb_otg_start_host(struct usb_otg *otg, int on);
> int usb_otg_start_gadget(struct usb_otg *otg, int on);
>
> - The following function is provided for use by the USB host stack
> to sync OTG related events to the OTG state machine.
> e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
> int usb_otg_kick_fsm(struct device *otg_device);
>
> - The following function is provided for use by the U
Re: [PATCH v3 06/12] power: pwrseq: simple: Add support for regulator and generic property
On 06/09/2016 04:34 AM, Chen-Yu Tsai wrote:
> Hi
>
> On Thu, Jun 9, 2016 at 3:03 AM, Rob Herring wrote:
>> On Tue, Jun 07, 2016 at 11:29:02AM +0200, Krzysztof Kozlowski wrote:
>>> On 06/03/2016 04:02 AM, Rob Herring wrote:
> Optional properties:
> - reset-gpios : contains a list of GPIO specifiers. The reset GPIOs are
> asserted
> @@ -16,6 +22,7 @@ Optional properties:
>See ../clocks/clock-bindings.txt for details.
> - clock-names : Must include the following entry:
>"ext_clock" (External clock provided to the card).
> +- ext-supply : External regulator supply
What happens when there are 2 supplies?
I'd prefer the name not be genericish and use the real supply names.
Then the power seq code should just turn on all supplies it finds. If
the order or timing to turn on matters, then sorry, no generic sequence.
>>>
>>> I think the generic part for regulators might be a problem. Regulator
>>> API requires a name for the supply... it cannot get "something" or
>>> "everything".
>>
>> That's the downside of variable property names...
>>
>>> The driver could attach itself to any kind of node (where power-sequence
>>> property exists) so the supply name depends on the bindings of device
>>> (not bindings of power sequence driver).
>>>
>>> The power sequence driver could however iterate over child properties
>>> and get the names of all supplies. It is a little bit ugly...
>>
>> Yes. Like this, right?
>>
>> for_each_property_of_node(np, pp) {
>> if (!strstr(pp->name, "-supply"))
>> continue;
>> // found supply
>> }
>>
>> The uglyness can always be improved with a function to do this parsing.
>
> There's already a version of this in simplefb. Maybe it's time to move
> this to a common function?
Thanks, I'll make a generic one and let's see how Mark will respond to it.
Best regards,
Krzysztof
--
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 00/21] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC macros. The macros are not y2038 safe. There is no plan to transition them into being y2038 safe. ktime_get_* api's can be used in their place. And, these are y2038 safe. All filesystem timestamps use current_fs_time() for the right granularity as mentioned in the respective commit texts of patches. This series also serves as a preparatory series to transition vfs to 64 bit timestamps as outlined here: https://lkml.org/lkml/2016/2/12/104 . As per Linus's suggestion in https://lkml.org/lkml/2016/5/24/663 , all the inode timestamp changes have been squashed into a single patch. Also, current_fs_time() now is used as a single generic filesystem timestamp api. Posting all patches together in a bigger series so that the big picture is clear. As per the suggestion in https://lwn.net/Articles/672598/ , CURRENT_TIME macro bug fixes are being handled in a series separate from transitioning vfs to use 64 bit timestamps. Some reviewers have requested not to change line wrapping only for the longer function call names, so checkpatch warnings for such cases are ignored in the patch series. Deepa Dinamani (21): fs: Replace CURRENT_TIME_SEC with current_fs_time() fs: ext4: Use current_fs_time() for inode timestamps fs: ubifs: Use current_fs_time() for inode timestamps fs: Replace CURRENT_TIME with current_fs_time() for inode timestamps fs: jfs: Replace CURRENT_TIME_SEC by current_fs_time() fs: udf: Replace CURRENT_TIME with current_fs_time() fs: cifs: Replace CURRENT_TIME by current_fs_time() fs: cifs: Replace CURRENT_TIME with ktime_get_real_ts() fs: cifs: Replace CURRENT_TIME by get_seconds fs: f2fs: Use ktime_get_real_seconds for sit_info times drivers: staging: lustre: Replace CURRENT_TIME with current_fs_time() block: rbd: Replace non inode CURRENT_TIME with current_fs_time() fs: ocfs2: Use time64_t to represent orphan scan times fs: ocfs2: Replace CURRENT_TIME with ktime_get_real_seconds() time: Add time64_to_tm() fnic: Use time64_t to represent trace timestamps audit: Use timespec64 to represent audit timestamps fs: nfs: Make nfs boot time y2038 safe libceph: Remove CURRENT_TIME references libceph: Replace CURRENT_TIME with ktime_get_real_ts time: Delete CURRENT_TIME_SEC and CURRENT_TIME macro arch/powerpc/platforms/cell/spufs/inode.c | 2 +- arch/s390/hypfs/inode.c| 4 +-- drivers/block/rbd.c| 2 +- drivers/infiniband/hw/qib/qib_fs.c | 2 +- drivers/misc/ibmasm/ibmasmfs.c | 2 +- drivers/oprofile/oprofilefs.c | 2 +- drivers/scsi/fnic/fnic_trace.c | 4 +-- drivers/scsi/fnic/fnic_trace.h | 2 +- drivers/staging/lustre/lustre/llite/llite_lib.c| 17 ++- drivers/staging/lustre/lustre/llite/namei.c| 4 +-- drivers/staging/lustre/lustre/mdc/mdc_reint.c | 6 ++-- .../lustre/lustre/obdclass/linux/linux-obdo.c | 6 ++-- drivers/staging/lustre/lustre/obdclass/obdo.c | 6 ++-- drivers/staging/lustre/lustre/osc/osc_io.c | 2 +- drivers/usb/core/devio.c | 19 ++-- drivers/usb/gadget/function/f_fs.c | 2 +- drivers/usb/gadget/legacy/inode.c | 2 +- fs/9p/vfs_inode.c | 2 +- fs/adfs/inode.c| 2 +- fs/affs/amigaffs.c | 6 ++-- fs/affs/inode.c| 2 +- fs/afs/inode.c | 3 +- fs/autofs4/inode.c | 2 +- fs/autofs4/root.c | 19 +++- fs/bfs/dir.c | 18 ++- fs/btrfs/inode.c | 2 +- fs/cifs/cifsencrypt.c | 4 ++- fs/cifs/cifssmb.c | 10 +++ fs/cifs/inode.c| 15 +- fs/coda/dir.c | 2 +- fs/coda/file.c | 2 +- fs/coda/inode.c| 2 +- fs/devpts/inode.c | 6 ++-- fs/efivarfs/inode.c| 2 +- fs/exofs/dir.c | 9 +++--- fs/exofs/inode.c | 7 +++-- fs/exofs/namei.c | 6 ++-- fs/ext2/acl.c | 2 +- fs/ext2/dir.c | 6 ++-- fs/ext2/ialloc.c | 2 +- fs/ext2/inode.c| 4 +-- fs/ext2/ioctl.c| 5 ++-- fs/ext2/namei.c|
Re: [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
Hi Felipe and Heikki, On 06/08/2016 12:42 PM, Greg Kroah-Hartman wrote: > On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote: >> Add support to retrieve fixed voltage configure information through >> ACPI interface. This is needed for Intel Bay Trail devices, where a >> GPIO is used to control the USB vbus. >> >> Signed-off-by: Lu Baolu >> --- >> drivers/regulator/fixed.c | 46 >> ++ >> 1 file changed, 46 insertions(+) > Can't do anything with this until I get an ack from the "owners" of this > file. > > And what happened to the acks from other Intel developers for this whole > patch series, I don't see that here :( > You have reviewed the whole patches in this series. Are you willing to ack them? Best regards, Lu Baolu -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
On Thu, Jun 9, 2016 at 5:27 AM, Jaret Cantu wrote:
> On 03/23/2016 10:21 PM, Peter Chen wrote:
>>
>> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>>>
>>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
On 03/23/2016 12:36 AM, Peter Chen wrote:
>
> On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>>
>> The TX settings can be calibrated for particular hardware. The
>> phy is reset by Linux, so this cannot be handled by the bootloader.
>>
>> The TRM mentions that the maximum resistance should be used for the
>> DN/DP calibration in order to pass USB certification.
>>
>> The values for the TX registers are poorly described in the TRM.
>> The meanings of the register values were taken from another
>> Freescale-provided document:
>> https://community.freescale.com/message/566147#comment-566912
>>
>> Signed-off-by: Jaret Cantu
>> ---
>> v3. Added unit suffix (-ohms) to tx-cal-45-d*
>>
>> v2. Copying devicetree list
>> Removed prettifying extra whitespace
>> Removed unnecessary register rewrite on resume
>> Use min and max constants for clarity
>>
>> .../devicetree/bindings/phy/mxs-usb-phy.txt| 10
>> drivers/usb/phy/phy-mxs-usb.c | 58
>>
>> 2 files changed, 68 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> index 379b84a..1d25b04 100644
>> --- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> @@ -12,6 +12,16 @@ Required properties:
>> - interrupts: Should contain phy interrupt
>> - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>> series
>>
>> +
>> +if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
>> +val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>> +/* scale to 4-bit value */
>> +val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>> +/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>> +mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>> +mxs_phy->tx_reg_set |= GM_USBPHY_TX_D_CAL(val);
>> +}
>> +
>
>
> I have tested "tx-d-cal", but it seems incorrect according to the xls
> you
> have provided, would you please check it again or am I wrong?
Gah! You're right. Some of the D_CAL values need to be rounded up to
match the xls. And even then, the value for 86 still doesn't play nice.
I was really hoping to avoid using a table for these values.
The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
possible register values and so are unaffected by a similar issue. I
rechecked their numbers just to be sure.
>>>
>>>
>>> The solution looks to be to scale D_CAL starting from 80 instead of
>>> 79. If you look at the xls listing, the jump from 79 to 83 is the
>>> only time two adjacent register values result in a change greater
>>> than 2% or 3%.
>>>
>>> This will result in some code ugliness as the minimum allowed
>>> percentage (79), per the Freescale document, and the point at which
>>> we are scaling the percentage values to register values (80) are
>>> different.
>>>
>>> And, as mentioned before, the values will also have to be rounded up.
>>>
>>> This quick shell code confirms that these sorts of calculations
>>> match up with the values in the spreadsheet:
>>>
>>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>>> d=$((d+1)); done
>>>
>>>
>>> I can't find any formula which would hit all of those same
>>> percentages without rounding up.
>>>
>>
>> Then, we had to use table for it. Besides, IC team confirms the default
>> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
>> so I am wondering how we describe it at binding doc.
>>
>
> Which next gen SoC would this be? The MX7? The documentation for the USB
> PHY in that reference manual is even sparser than the one for the MX6 in
> regards to these register values.
>
Here, I mean i.mx8
> The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and
> HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no listing
> as to the location of these registers, let alone their defaults/step values.
>
> Do you know where we could get the default and step values for the TXCAL
> registers on the new SoC? This information had to come from a Freescale
> community thread for the MX6 since it wasn't defined clearly elsewhere.
In theory, these information should be listed at SoC reference manual.
BR,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@
Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
Hi Greg, On 06/08/2016 11:45 PM, Greg Kroah-Hartman wrote: > On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote: >> Hi Greg, >> >> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote: >>> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote: In some Intel platforms, a single usb port is shared between USB host and device controllers. The shared port is under control of a switch which is defined in the Intel vendor defined extended capability for xHCI. This patch adds the support to detect and create the platform device for the port mux switch. >>> Why do you need a platform device for this? You do nothing with this >>> device, why create it at all? >> In this patch series, I have a generic framework for port mux devices >> and two port mux drivers sitting on top the generic code. >> >> In this patch, I create a platform device for the real mux device in >> Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux >> into the generic framework and handle the power management >> things in driver's pm entries (otherwise, the system can't be waken >> up from system suspend).:) >> >>> And why is it a platform device, isn't is really a PCI device? Why >>> would you ever find a "platform" device below a PCI device? Don't abuse >>> platform devices for things that aren't. It makes me want to delete >>> that whole interface more and more... >> Port mux devices are physical devices in Intel Cherry Trail and Broxton >> SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI >> space. OS kernel can enumerate it by looking up the xhci extended >> capability list with a vendor specific capability ID. > A physical device that maps registers into PCI space seems like a PCI > device of some type to me :) > > Again, I hate platform devices for obvious reasons like this... > It's not PCI configure space, but xhci's io memory. XHCI spec reserves a range in its extended capability list for vendor specific things. Intel's platform leverages this for the port mux device register mapping. It looks odd though. :) Best regards, Lu Baolu -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/12] power: pwrseq: simple: Add support for regulator and generic property
Hi
On Thu, Jun 9, 2016 at 3:03 AM, Rob Herring wrote:
> On Tue, Jun 07, 2016 at 11:29:02AM +0200, Krzysztof Kozlowski wrote:
>> On 06/03/2016 04:02 AM, Rob Herring wrote:
>> >> Optional properties:
>> >> - reset-gpios : contains a list of GPIO specifiers. The reset GPIOs are
>> >> asserted
>> >> @@ -16,6 +22,7 @@ Optional properties:
>> >>See ../clocks/clock-bindings.txt for details.
>> >> - clock-names : Must include the following entry:
>> >>"ext_clock" (External clock provided to the card).
>> >> +- ext-supply : External regulator supply
>> >
>> > What happens when there are 2 supplies?
>> >
>> > I'd prefer the name not be genericish and use the real supply names.
>> > Then the power seq code should just turn on all supplies it finds. If
>> > the order or timing to turn on matters, then sorry, no generic sequence.
>>
>> I think the generic part for regulators might be a problem. Regulator
>> API requires a name for the supply... it cannot get "something" or
>> "everything".
>
> That's the downside of variable property names...
>
>> The driver could attach itself to any kind of node (where power-sequence
>> property exists) so the supply name depends on the bindings of device
>> (not bindings of power sequence driver).
>>
>> The power sequence driver could however iterate over child properties
>> and get the names of all supplies. It is a little bit ugly...
>
> Yes. Like this, right?
>
> for_each_property_of_node(np, pp) {
> if (!strstr(pp->name, "-supply"))
> continue;
> // found supply
> }
>
> The uglyness can always be improved with a function to do this parsing.
There's already a version of this in simplefb. Maybe it's time to move
this to a common function?
ChenYu
--
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
[RESEND PATCH V2 1/2] soc: brcmstb: Add Product ID and Family ID helper functions
Signed-off-by: Al Cooper
---
drivers/soc/brcmstb/common.c| 12
include/linux/soc/brcmstb/brcmstb.h | 10 ++
2 files changed, 22 insertions(+)
diff --git a/drivers/soc/brcmstb/common.c b/drivers/soc/brcmstb/common.c
index 94e7335..454f4c2 100644
--- a/drivers/soc/brcmstb/common.c
+++ b/drivers/soc/brcmstb/common.c
@@ -40,6 +40,18 @@ bool soc_is_brcmstb(void)
return of_match_node(brcmstb_machine_match, root) != NULL;
}
+u32 brcmstb_get_family_id(void)
+{
+ return family_id;
+}
+EXPORT_SYMBOL(brcmstb_get_family_id);
+
+u32 brcmstb_get_product_id(void)
+{
+ return product_id;
+}
+EXPORT_SYMBOL(brcmstb_get_product_id);
+
static const struct of_device_id sun_top_ctrl_match[] = {
{ .compatible = "brcm,brcmstb-sun-top-ctrl", },
{ }
diff --git a/include/linux/soc/brcmstb/brcmstb.h
b/include/linux/soc/brcmstb/brcmstb.h
index 337ce41..4bf5edd 100644
--- a/include/linux/soc/brcmstb/brcmstb.h
+++ b/include/linux/soc/brcmstb/brcmstb.h
@@ -1,10 +1,20 @@
#ifndef __BRCMSTB_SOC_H
#define __BRCMSTB_SOC_H
+#define BRCM_ID(reg) ((u32)reg >> 28 ? (u32)reg >> 16 : (u32)reg >> 8)
+#define BRCM_REV(reg) ((u32)reg & 0xff)
+
/*
* Bus Interface Unit control register setup, must happen early during boot,
* before SMP is brought up, called by machine entry point.
*/
void brcmstb_biuctrl_init(void);
+/*
+* Helper functions for getting family or product id from the
+* SoC driver.
+*/
+u32 brcmstb_get_family_id(void);
+u32 brcmstb_get_product_id(void);
+
#endif /* __BRCMSTB_SOC_H */
--
1.9.0.138.g2de3478
--
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
[RESEND PATCH V2 0/2] Add Broadcom USB PHY driver for Broadcom STB SoCs
Add Broadcom USB PHY driver for Broadcom STB SoCs. This driver in combination with the generic ohci, ehci and xhci platform drivers will enable USB1.1, USB2.0 and USB3.0 support. NOTE: An unrelated patch is in the pipline to move the file drivers/soc/brcmstb/common.c to drivers/soc/bcm/brcmstb/common.c V2 - Change compatible name from "brcm,usb-phy" to "brcm,brcmstb-usb-phy" Al Cooper (2): soc: brcmstb: Add Product ID and Family ID helper functions usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver .../bindings/phy/brcm,brcmstb-usb-phy.txt | 39 + MAINTAINERS| 7 + drivers/phy/Kconfig| 10 + drivers/phy/Makefile | 2 + drivers/phy/phy-brcm-usb-init.c| 792 + drivers/phy/phy-brcm-usb-init.h| 49 ++ drivers/phy/phy-brcm-usb.c | 206 ++ drivers/soc/brcmstb/common.c | 12 + include/linux/soc/brcmstb/brcmstb.h| 10 + 9 files changed, 1127 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt create mode 100644 drivers/phy/phy-brcm-usb-init.c create mode 100644 drivers/phy/phy-brcm-usb-init.h create mode 100644 drivers/phy/phy-brcm-usb.c -- 1.9.0.138.g2de3478 -- 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
[RESEND PATCH V2 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB phy driver
Add a new USB Phy driver for Broadcom STB SoCs. This driver
supports all Broadcom STB ARM SoCs. This driver in combination
with the generic ohci, ehci and xhci platform drivers will enable
USB1.1, USB2.0 and USB3.0 support. This Phy driver also supports
the Broadcom UDC gadget driver.
Signed-off-by: Al Cooper
---
.../bindings/phy/brcm,brcmstb-usb-phy.txt | 39 +
MAINTAINERS| 7 +
drivers/phy/Kconfig| 10 +
drivers/phy/Makefile | 2 +
drivers/phy/phy-brcm-usb-init.c| 792 +
drivers/phy/phy-brcm-usb-init.h| 49 ++
drivers/phy/phy-brcm-usb.c | 206 ++
7 files changed, 1105 insertions(+)
create mode 100644
Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
create mode 100644 drivers/phy/phy-brcm-usb-init.c
create mode 100644 drivers/phy/phy-brcm-usb-init.h
create mode 100644 drivers/phy/phy-brcm-usb.c
diff --git a/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
new file mode 100644
index 000..34fa9dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
@@ -0,0 +1,39 @@
+Broadcom STB USB PHY
+
+Required properties:
+ - compatible: brcm,brcmstb-usb-phy
+ - reg: two offset and length pairs. The second pair specifies the
+USB 3.0 related registers and is only required for PHYs
+that support USB 3.0
+ - #phy-cells: Shall be 1 as it expects one argument for setting
+ the type of the PHY. Possible values are 0 (1.1 and 2.0),
+ 1 (3.0)
+
+
+Optional Properties:
+- clocks : phandle + clock specifier for the phy clocks
+- clock-names: string, clock name
+- ipp: Invert Port Power
+- ioc: Invert Over Current detection
+- has_xhci: Contains an optional 3.0 PHY
+- device: PHY Device mode. Possible values are: 0 (Host), 1 (Device)
+ or 2 (DRD)
+
+
+
+Example:
+
+usbphy_0: usb-phy@f0470200 {
+ reg = <0xf0470200 0xb8>,
+ <0xf0471940 0x6c0>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "brcm,brcmstb-usb-phy";
+ ioc = <1>;
+ ipp = <1>;
+ #phy-cells = <1>;
+ ranges;
+ has_xhci;
+ clocks = <&sw_usb20>;
+ clock-names = "sw_usb";
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index ed1229e..4d529e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2631,6 +2631,13 @@ S: Maintained
F: drivers/bcma/
F: include/linux/bcma/
+BROADCOM STB USB PHY DRIVER
+M: Al Cooper
+L: linux-usb@vger.kernel.org
+L: bcm-kernel-feedback-l...@broadcom.com
+S: Supported
+F: drivers/phy/phy-brcm-usb*
+
BROADCOM SYSTEMPORT ETHERNET DRIVER
M: Florian Fainelli
L: net...@vger.kernel.org
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index b869b98..61a0244 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -432,6 +432,16 @@ config PHY_CYGNUS_PCIE
Enable this to support the Broadcom Cygnus PCIe PHY.
If unsure, say N.
+config BRCM_USB_PHY
+ tristate "Broadcom USB PHY driver"
+ depends on OF && USB && ARCH_BRCMSTB
+ select GENERIC_PHY
+ default y
+ help
+ Enable this to support the Broadcom USB PHY on
+ Broadcom STB SoCs.
+ If unsure, say Y.
+
source "drivers/phy/tegra/Kconfig"
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9c3e73c..babb0c7 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -9,6 +9,8 @@ obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o
obj-$(CONFIG_PHY_DM816X_USB) += phy-dm816x-usb.o
obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
+obj-$(CONFIG_BRCM_USB_PHY) += phy-brcm-usb.o
+obj-$(CONFIG_BRCM_USB_PHY) += phy-brcm-usb-init.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o
diff --git a/drivers/phy/phy-brcm-usb-init.c b/drivers/phy/phy-brcm-usb-init.c
new file mode 100644
index 000..f5d8c32
--- /dev/null
+++ b/drivers/phy/phy-brcm-usb-init.c
@@ -0,0 +1,792 @@
+/*
+ * Copyright (C) 2014-2016 Broadcom
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
uac2: diagnosing uac2 audio gadget problems
Hello, I am attempting to use the USB audo gadget uac2 so that my i.MX6 based Wandboard can be a USB audio device. I am using a stock 4.5 linux with no modifications, direct from git and compiled locally: uname -a shows: Linux wandboard 4.5.0 #4 SMP Tue Jun 7 10:09:16 PDT 2016 armv7l armv7l armv7l GNU/Linux I have brought up and configfs setup, and have successfully gotten aplay and arecord to show the devices like this: aplay -L shows: ... hw:CARD=UAC2Gadget,DEV=0 UAC2_Gadget, UAC2 PCM Direct hardware device without any conversions ... and arecord -L shows: ... hw:CARD=UAC2Gadget,DEV=0 UAC2_Gadget, UAC2 PCM ... I have tried both 48000 and 64000 sample rates (via s_srate and c_srate), but I can't either play or record audio, and nothing goes over to the host. On the host side, I can play and record -- i.e. time progresses without error in audacity when I hit the record button (which does a full-duplex play/record). On the client side I get this, regardless of what's happening on the host side: ... root@wandboard:/home/caleb/gadget# aplay -D hw:CARD=UAC2Gadget,DEV=0 sine64k.wav Playing WAVE 'sine64k.wav' : Signed 16 bit Little Endian, Rate 64000 Hz, Stereo aplay: pcm_write:1939: write error: Input/output error ... Here is the script I use for starting the gadget interface: cd /sys/kernel/config/usb_gadget mkdir g1 cd g1 echo "0x1d6b" > idVendor echo "0x0104" > idProduct mkdir strings/0x409 echo "0123456789" > strings/0x409/serialnumber echo "Foo Inc." > strings/0x409/manufacturer echo "Bar Gadget" > strings/0x409/product mkdir functions/uac2.aud0 mkdir functions/ecm.usb0 #echo 64000 > functions/uac2.aud0/c_srate #echo 64000 > functions/uac2.aud0/p_srate mkdir configs/c.1 mkdir configs/c.1/strings/0x409 echo "CDC ECM + audio" > configs/c.1/strings/0x409/configuration ln -s functions/uac2.aud0 configs/c.1 ln -s functions/ecm.usb0 configs/c.1 echo `ls /sys/class/udc/` > /sys/kernel/config/usb_gadget/g1/UDC When I run the script above I get the following dumped into /var/log/syslog: Jun 8 23:08:14 wandboard kernel: [ 139.994081] using random self ethernet address Jun 8 23:08:14 wandboard kernel: [ 139.994107] using random host ethernet address Jun 8 23:08:14 wandboard NetworkManager[359]: nm_device_get_device_type: assertion 'NM_IS_DEVICE (self)' failed Jun 8 23:08:14 wandboard NetworkManager[359]: (usb0): new Generic device (carrier: OFF, driver: 'g_ether', ifindex: 4) Jun 8 23:08:14 wandboard kernel: [ 140.125255] usb0: HOST MAC 3e:52:2d:0d:98:fe Jun 8 23:08:14 wandboard kernel: [ 140.153803] usb0: MAC 0e:cf:aa:06:aa:8a Jun 8 23:08:14 wandboard systemd-udevd[773]: Failed to apply ACL on /dev/snd/controlC3: Operation not supported Jun 8 23:08:14 wandboard systemd-udevd[776]: Failed to apply ACL on /dev/snd/pcmC3D0c: Operation not supported Jun 8 23:08:14 wandboard systemd-udevd[775]: Failed to apply ACL on /dev/snd/pcmC3D0p: Operation not supported Jun 8 23:08:14 wandboard systemd-udevd[773]: Process '/usr/sbin/alsactl -E HOME=/var/run/alsa restore 3' failed with exit code 99. Jun 8 23:08:14 wandboard NetworkManager[359]: devices added (path: /sys/devices/soc0/soc/210.aips-bus/2184000.usb/ci_hdrc.0/gadget/net/usb0, iface: usb0) Jun 8 23:08:14 wandboard NetworkManager[359]: device added (path: /sys/devices/soc0/soc/210.aips-bus/2184000.usb/ci_hdrc.0/gadget/net/usb0, iface: usb0): no ifupdown configuration found. Any idea what could be wrong?Is the 'Failed to apply ACL' relavent? Thank you! Sincerely, -Caleb -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
On 03/23/2016 10:21 PM, Peter Chen wrote:
On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
On 03/23/2016 01:37 PM, Jaret Cantu wrote:
On 03/23/2016 12:36 AM, Peter Chen wrote:
On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
The TX settings can be calibrated for particular hardware. The
phy is reset by Linux, so this cannot be handled by the bootloader.
The TRM mentions that the maximum resistance should be used for the
DN/DP calibration in order to pass USB certification.
The values for the TX registers are poorly described in the TRM.
The meanings of the register values were taken from another
Freescale-provided document:
https://community.freescale.com/message/566147#comment-566912
Signed-off-by: Jaret Cantu
---
v3. Added unit suffix (-ohms) to tx-cal-45-d*
v2. Copying devicetree list
Removed prettifying extra whitespace
Removed unnecessary register rewrite on resume
Use min and max constants for clarity
.../devicetree/bindings/phy/mxs-usb-phy.txt| 10
drivers/usb/phy/phy-mxs-usb.c | 58
2 files changed, 68 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
index 379b84a..1d25b04 100644
--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
@@ -12,6 +12,16 @@ Required properties:
- interrupts: Should contain phy interrupt
- fsl,anatop: phandle for anatop register, it is only for imx6 SoC
series
+
+if (!of_property_read_u32(np, "fsl,tx-d-cal", &val) &&
+val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
+/* scale to 4-bit value */
+val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
+/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
+mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
+mxs_phy->tx_reg_set |= GM_USBPHY_TX_D_CAL(val);
+}
+
I have tested "tx-d-cal", but it seems incorrect according to the xls you
have provided, would you please check it again or am I wrong?
Gah! You're right. Some of the D_CAL values need to be rounded up to
match the xls. And even then, the value for 86 still doesn't play nice.
I was really hoping to avoid using a table for these values.
The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
possible register values and so are unaffected by a similar issue. I
rechecked their numbers just to be sure.
The solution looks to be to scale D_CAL starting from 80 instead of
79. If you look at the xls listing, the jump from 79 to 83 is the
only time two adjacent register values result in a change greater
than 2% or 3%.
This will result in some code ugliness as the minimum allowed
percentage (79), per the Freescale document, and the point at which
we are scaling the percentage values to register values (80) are
different.
And, as mentioned before, the values will also have to be rounded up.
This quick shell code confirms that these sorts of calculations
match up with the values in the spreadsheet:
for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
d=$((d+1)); done
I can't find any formula which would hit all of those same
percentages without rounding up.
Then, we had to use table for it. Besides, IC team confirms the default
value and the step for TXCAL45DP/DN are changed at next generation SoCs,
so I am wondering how we describe it at binding doc.
Which next gen SoC would this be? The MX7? The documentation for the
USB PHY in that reference manual is even sparser than the one for the
MX6 in regards to these register values.
The MX7 manual does still mention that HW_USBPHY_TX_TXCAL45DP and
HW_USBPHY_TX_TXCAL45DN should both be set to zero, but there is no
listing as to the location of these registers, let alone their
defaults/step values.
Do you know where we could get the default and step values for the TXCAL
registers on the new SoC? This information had to come from a Freescale
community thread for the MX6 since it wasn't defined clearly elsewhere.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: ohci-at91: Forcibly suspend ports while USB suspend
On 08/06/2016 at 15:26:51 -0500, Rob Herring wrote : > On Wed, Jun 08, 2016 at 12:15:10PM +0800, Wenyou Yang wrote: > > In order to the save power consumption, as a workaround, suspend > > forcibly the USB PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI > > Interrupt Configuration Register in the SFRs while OHCI USB suspend. > > > > This suspend operation must be done before the USB clock is disabled, > > resume after the USB clock is enabled. > > > > Signed-off-by: Wenyou Yang > > --- > > > > Changes in v3: > > - Change the compatible description for more precise. > > > > Changes in v2: > > - Add compatible to support forcibly suspend the ports. > > - Add soc/at91/at91_sfr.h to accommodate the defines. > > - Add error checking for .sfr_regmap. > > - Remove unnecessary regmap_read() statement. > > > > .../devicetree/bindings/usb/atmel-usb.txt | 6 +- > > drivers/usb/host/ohci-at91.c | 80 > > +- > > include/soc/at91/at91_sfr.h| 29 > > 3 files changed, 112 insertions(+), 3 deletions(-) > > create mode 100644 include/soc/at91/at91_sfr.h > > > > diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt > > b/Documentation/devicetree/bindings/usb/atmel-usb.txt > > index 5883b73..888deaa 100644 > > --- a/Documentation/devicetree/bindings/usb/atmel-usb.txt > > +++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt > > @@ -3,8 +3,10 @@ Atmel SOC USB controllers > > OHCI > > > > Required properties: > > - - compatible: Should be "atmel,at91rm9200-ohci" for USB controllers > > - used in host mode. > > + - compatible: Should be one of the following > > + "atmel,at91rm9200-ohci" for USB controllers used in host mode. > > + "atmel,sama5d2-ohci" for USB controllers used in host mode > > + on SAMA5D2 which can force to suspend. > > Guess I wasn't clear enough before. Drop "which can force to suspend". > Well, my point is that we don't need a new compatible anyway. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: ohci-at91: Forcibly suspend ports while USB suspend
On Wed, Jun 08, 2016 at 12:15:10PM +0800, Wenyou Yang wrote:
> In order to the save power consumption, as a workaround, suspend
> forcibly the USB PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI
> Interrupt Configuration Register in the SFRs while OHCI USB suspend.
>
> This suspend operation must be done before the USB clock is disabled,
> resume after the USB clock is enabled.
>
> Signed-off-by: Wenyou Yang
> ---
>
> Changes in v3:
> - Change the compatible description for more precise.
>
> Changes in v2:
> - Add compatible to support forcibly suspend the ports.
> - Add soc/at91/at91_sfr.h to accommodate the defines.
> - Add error checking for .sfr_regmap.
> - Remove unnecessary regmap_read() statement.
>
> .../devicetree/bindings/usb/atmel-usb.txt | 6 +-
> drivers/usb/host/ohci-at91.c | 80
> +-
> include/soc/at91/at91_sfr.h| 29
> 3 files changed, 112 insertions(+), 3 deletions(-)
> create mode 100644 include/soc/at91/at91_sfr.h
>
> diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt
> b/Documentation/devicetree/bindings/usb/atmel-usb.txt
> index 5883b73..888deaa 100644
> --- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
> @@ -3,8 +3,10 @@ Atmel SOC USB controllers
> OHCI
>
> Required properties:
> - - compatible: Should be "atmel,at91rm9200-ohci" for USB controllers
> - used in host mode.
> + - compatible: Should be one of the following
> +"atmel,at91rm9200-ohci" for USB controllers used in host mode.
> +"atmel,sama5d2-ohci" for USB controllers used in host mode
> +on SAMA5D2 which can force to suspend.
Guess I wasn't clear enough before. Drop "which can force to suspend".
> - reg: Address and length of the register set for the device
> - interrupts: Should contain ehci interrupt
> - clocks: Should reference the peripheral, host and system clocks
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index d177372..54e8feb 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -21,8 +21,11 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> +#include
>
> #include "ohci.h"
>
> @@ -45,12 +48,18 @@ struct at91_usbh_data {
> u8 overcurrent_changed[AT91_MAX_USBH_PORTS];
> };
>
> +struct ohci_at91_caps {
> + bool suspend_ctrl;
> +};
> +
> struct ohci_at91_priv {
> struct clk *iclk;
> struct clk *fclk;
> struct clk *hclk;
> bool clocked;
> bool wakeup;/* Saved wake-up state for resume */
> + const struct ohci_at91_caps *caps;
> + struct regmap *sfr_regmap;
> };
> /* interface and function clocks; sometimes also an AHB clock */
>
> @@ -132,6 +141,17 @@ static void at91_stop_hc(struct platform_device *pdev)
>
> /*-*/
>
> +struct regmap *at91_dt_syscon_sfr(void)
> +{
> + struct regmap *regmap;
> +
> + regmap = syscon_regmap_lookup_by_compatible("atmel,sama5d2-sfr");
> + if (IS_ERR(regmap))
> + regmap = NULL;
> +
> + return regmap;
> +}
> +
> static void usb_hcd_at91_remove (struct usb_hcd *, struct platform_device *);
>
> /* configure so an HC device and id are always provided */
> @@ -197,6 +217,17 @@ static int usb_hcd_at91_probe(const struct hc_driver
> *driver,
> goto err;
> }
>
> + ohci_at91->caps = (const struct ohci_at91_caps *)
> + of_device_get_match_data(&pdev->dev);
> + if (!ohci_at91->caps)
> + return -ENODEV;
> +
> + if (ohci_at91->caps->suspend_ctrl) {
> + ohci_at91->sfr_regmap = at91_dt_syscon_sfr();
> + if (!ohci_at91->sfr_regmap)
> + dev_warn(dev, "failed to find sfr node\n");
> + }
> +
> board = hcd->self.controller->platform_data;
> ohci = hcd_to_ohci(hcd);
> ohci->num_ports = board->ports;
> @@ -440,8 +471,17 @@ static irqreturn_t ohci_hcd_at91_overcurrent_irq(int
> irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static const struct ohci_at91_caps at91rm9200_caps = {
> + .suspend_ctrl = false,
> +};
> +
> +static const struct ohci_at91_caps sama5d2_caps = {
> + .suspend_ctrl = true,
> +};
> +
> static const struct of_device_id at91_ohci_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-ohci" },
> + { .compatible = "atmel,at91rm9200-ohci", .data = &at91rm9200_caps },
> + { .compatible = "atmel,sama5d2-ohci", .data = &sama5d2_caps },
> { /* sentinel */ }
> };
>
> @@ -581,6 +621,38 @@ static int ohci_hcd_at91_drv_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +static int ohci_at91_port_ctrl(struct regmap *regmap, bool enable)
> +{
> + u32 regval;
> + int ret;
> +
> + if (!regmap)
> +
Re: [PATCH] ehci-platform: Add support for shared reset controllers
Hi,
On 08-06-16 19:47, Alan Stern wrote:
On Wed, 8 Jun 2016, Hans de Goede wrote:
Add support for shared platform controllers by using
devm_reset_control_get_shared_by_index instead of
of_reset_control_get_by_index.
Note we use the devm function because there is no
of_reset_control_get_shared_by_index, this also leads
to a nice cleanup of the cleanup code.
This brings the ehci-platform reset handling code inline
with ohci-platform.
Signed-off-by: Hans de Goede
---
This is just the difference between what Greg has merged and what you
wanted to add, right? If so, then
Right.
Regards,
Hans
Acked-by: Alan Stern
Alan Stern
drivers/usb/host/ehci-platform.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index bc33f45..6816b8c 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -236,8 +236,8 @@ static int ehci_platform_probe(struct platform_device *dev)
}
for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
- priv->rsts[rst] = of_reset_control_get_by_index(
- dev->dev.of_node, rst);
+ priv->rsts[rst] = devm_reset_control_get_shared_by_index(
+ &dev->dev, rst);
if (IS_ERR(priv->rsts[rst])) {
err = PTR_ERR(priv->rsts[rst]);
if (err == -EPROBE_DEFER)
@@ -247,10 +247,8 @@ static int ehci_platform_probe(struct platform_device *dev)
}
err = reset_control_deassert(priv->rsts[rst]);
- if (err) {
- reset_control_put(priv->rsts[rst]);
+ if (err)
goto err_reset;
- }
}
if (pdata->big_endian_desc)
@@ -307,10 +305,8 @@ err_power:
if (pdata->power_off)
pdata->power_off(dev);
err_reset:
- while (--rst >= 0) {
+ while (--rst >= 0)
reset_control_assert(priv->rsts[rst]);
- reset_control_put(priv->rsts[rst]);
- }
err_put_clks:
while (--clk >= 0)
clk_put(priv->clks[clk]);
@@ -335,10 +331,8 @@ static int ehci_platform_remove(struct platform_device
*dev)
if (pdata->power_off)
pdata->power_off(dev);
- for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++) {
+ for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++)
reset_control_assert(priv->rsts[rst]);
- reset_control_put(priv->rsts[rst]);
- }
for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
clk_put(priv->clks[clk]);
--
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 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
On Tue, Jun 07, 2016 at 05:15:53PM +0800, Frank Wang wrote: > Signed-off-by: Frank Wang > --- > > Changes in v4: > - Used 'phy-supply' instead of 'vbus_*-supply'. > > Changes in v3: > - Added 'clocks' and 'clock-names' optional properties. > - Specified 'otg-port' and 'host-port' as the sub-node name. > > Changes in v2: > - Changed vbus_host optional property from gpio to regulator. > - Specified vbus_otg-supply optional property. > - Specified otg_id and otg_bvalid property. > > .../bindings/phy/phy-rockchip-inno-usb2.txt| 62 > > 1 file changed, 62 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt Acked-by: Rob Herring -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: ohci-at91: Forcibly suspend ports while USB suspend
On Wed, 8 Jun 2016, Wenyou Yang wrote: > In order to the save power consumption, as a workaround, suspend > forcibly the USB PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI > Interrupt Configuration Register in the SFRs while OHCI USB suspend. > > This suspend operation must be done before the USB clock is disabled, > resume after the USB clock is enabled. > > Signed-off-by: Wenyou Yang > --- You never answered the questions I posted for the first version of this patch: What does this mean? What does suspending a port do? Is it the same as a normal USB port suspend? If it is the same, why doesn't the USB_PORT_FEAT_SUSPEND subcase of the SetPortFeature case in ohci_hub_control() already take care of this? 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: [PATCH v3 06/12] power: pwrseq: simple: Add support for regulator and generic property
On Tue, Jun 07, 2016 at 11:29:02AM +0200, Krzysztof Kozlowski wrote:
> On 06/03/2016 04:02 AM, Rob Herring wrote:
> >> Optional properties:
> >> - reset-gpios : contains a list of GPIO specifiers. The reset GPIOs are
> >> asserted
> >> @@ -16,6 +22,7 @@ Optional properties:
> >>See ../clocks/clock-bindings.txt for details.
> >> - clock-names : Must include the following entry:
> >>"ext_clock" (External clock provided to the card).
> >> +- ext-supply : External regulator supply
> >
> > What happens when there are 2 supplies?
> >
> > I'd prefer the name not be genericish and use the real supply names.
> > Then the power seq code should just turn on all supplies it finds. If
> > the order or timing to turn on matters, then sorry, no generic sequence.
>
> I think the generic part for regulators might be a problem. Regulator
> API requires a name for the supply... it cannot get "something" or
> "everything".
That's the downside of variable property names...
> The driver could attach itself to any kind of node (where power-sequence
> property exists) so the supply name depends on the bindings of device
> (not bindings of power sequence driver).
>
> The power sequence driver could however iterate over child properties
> and get the names of all supplies. It is a little bit ugly...
Yes. Like this, right?
for_each_property_of_node(np, pp) {
if (!strstr(pp->name, "-supply"))
continue;
// found supply
}
The uglyness can always be improved with a function to do this parsing.
Rob
--
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] ehci-platform: Add support for shared reset controllers
On Wed, 8 Jun 2016, Hans de Goede wrote:
> Add support for shared platform controllers by using
> devm_reset_control_get_shared_by_index instead of
> of_reset_control_get_by_index.
>
> Note we use the devm function because there is no
> of_reset_control_get_shared_by_index, this also leads
> to a nice cleanup of the cleanup code.
>
> This brings the ehci-platform reset handling code inline
> with ohci-platform.
>
> Signed-off-by: Hans de Goede
> ---
This is just the difference between what Greg has merged and what you
wanted to add, right? If so, then
Acked-by: Alan Stern
Alan Stern
> drivers/usb/host/ehci-platform.c | 16 +---
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-platform.c
> b/drivers/usb/host/ehci-platform.c
> index bc33f45..6816b8c 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -236,8 +236,8 @@ static int ehci_platform_probe(struct platform_device
> *dev)
> }
>
> for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
> - priv->rsts[rst] = of_reset_control_get_by_index(
> - dev->dev.of_node, rst);
> + priv->rsts[rst] = devm_reset_control_get_shared_by_index(
> + &dev->dev, rst);
> if (IS_ERR(priv->rsts[rst])) {
> err = PTR_ERR(priv->rsts[rst]);
> if (err == -EPROBE_DEFER)
> @@ -247,10 +247,8 @@ static int ehci_platform_probe(struct platform_device
> *dev)
> }
>
> err = reset_control_deassert(priv->rsts[rst]);
> - if (err) {
> - reset_control_put(priv->rsts[rst]);
> + if (err)
> goto err_reset;
> - }
> }
>
> if (pdata->big_endian_desc)
> @@ -307,10 +305,8 @@ err_power:
> if (pdata->power_off)
> pdata->power_off(dev);
> err_reset:
> - while (--rst >= 0) {
> + while (--rst >= 0)
> reset_control_assert(priv->rsts[rst]);
> - reset_control_put(priv->rsts[rst]);
> - }
> err_put_clks:
> while (--clk >= 0)
> clk_put(priv->clks[clk]);
> @@ -335,10 +331,8 @@ static int ehci_platform_remove(struct platform_device
> *dev)
> if (pdata->power_off)
> pdata->power_off(dev);
>
> - for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++) {
> + for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++)
> reset_control_assert(priv->rsts[rst]);
> - reset_control_put(priv->rsts[rst]);
> - }
>
> for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
> clk_put(priv->clks[clk]);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 resend 1/2] ehci-platform: Add support for controllers with multiple reset lines
On Wed, Jun 08, 2016 at 06:52:30PM +0200, Hans de Goede wrote: > Hi, > > On 08-06-16 07:22, Greg Kroah-Hartman wrote: > > On Thu, Jun 02, 2016 at 05:14:05PM +0200, Hans de Goede wrote: > > > From: Reinder de Haan > > > > > > At least the EHCI/OHCI found on the Allwinnner H3 SoC needs multiple > > > reset lines, the controller will not initialize while the reset for > > > its companion is still asserted, which means we need to de-assert > > > 2 resets for the controller to work. > > > > > > Signed-off-by: Reinder de Haan > > > Signed-off-by: Hans de Goede > > > --- > > > Changes in v2: > > > -Use the new reset_control_[de]assert_shared reset-controller functions > > > --- > > > Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 +- > > > drivers/usb/host/ehci-platform.c | 41 > > > -- > > > 2 files changed, 24 insertions(+), 19 deletions(-) > > > > This patch doesn't apply to my usb-testing branch at all, what went > > wrong? > > You merged this patch: > > https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?id=d0e08b0077f49e209bc90305ddf1ca434ac6cc0e > > Which does almost the same, except that it only supports dedicated reset > controllers and not shared reset controllers. > > I'll send a new patch which changes things to add support for shared > reset controllers. > > This will also bring the ehci-platform reset handling code inline > with ohci-platform again. Thanks, I've merged your new patch now. 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] ehci-platform: Add support for shared reset controllers
Add support for shared platform controllers by using
devm_reset_control_get_shared_by_index instead of
of_reset_control_get_by_index.
Note we use the devm function because there is no
of_reset_control_get_shared_by_index, this also leads
to a nice cleanup of the cleanup code.
This brings the ehci-platform reset handling code inline
with ohci-platform.
Signed-off-by: Hans de Goede
---
drivers/usb/host/ehci-platform.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index bc33f45..6816b8c 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -236,8 +236,8 @@ static int ehci_platform_probe(struct platform_device *dev)
}
for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
- priv->rsts[rst] = of_reset_control_get_by_index(
- dev->dev.of_node, rst);
+ priv->rsts[rst] = devm_reset_control_get_shared_by_index(
+ &dev->dev, rst);
if (IS_ERR(priv->rsts[rst])) {
err = PTR_ERR(priv->rsts[rst]);
if (err == -EPROBE_DEFER)
@@ -247,10 +247,8 @@ static int ehci_platform_probe(struct platform_device *dev)
}
err = reset_control_deassert(priv->rsts[rst]);
- if (err) {
- reset_control_put(priv->rsts[rst]);
+ if (err)
goto err_reset;
- }
}
if (pdata->big_endian_desc)
@@ -307,10 +305,8 @@ err_power:
if (pdata->power_off)
pdata->power_off(dev);
err_reset:
- while (--rst >= 0) {
+ while (--rst >= 0)
reset_control_assert(priv->rsts[rst]);
- reset_control_put(priv->rsts[rst]);
- }
err_put_clks:
while (--clk >= 0)
clk_put(priv->clks[clk]);
@@ -335,10 +331,8 @@ static int ehci_platform_remove(struct platform_device
*dev)
if (pdata->power_off)
pdata->power_off(dev);
- for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++) {
+ for (rst = 0; rst < EHCI_MAX_RSTS && priv->rsts[rst]; rst++)
reset_control_assert(priv->rsts[rst]);
- reset_control_put(priv->rsts[rst]);
- }
for (clk = 0; clk < EHCI_MAX_CLKS && priv->clks[clk]; clk++)
clk_put(priv->clks[clk]);
--
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 v5 resend 1/2] ehci-platform: Add support for controllers with multiple reset lines
Hi, On 08-06-16 07:22, Greg Kroah-Hartman wrote: On Thu, Jun 02, 2016 at 05:14:05PM +0200, Hans de Goede wrote: From: Reinder de Haan At least the EHCI/OHCI found on the Allwinnner H3 SoC needs multiple reset lines, the controller will not initialize while the reset for its companion is still asserted, which means we need to de-assert 2 resets for the controller to work. Signed-off-by: Reinder de Haan Signed-off-by: Hans de Goede --- Changes in v2: -Use the new reset_control_[de]assert_shared reset-controller functions --- Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 +- drivers/usb/host/ehci-platform.c | 41 -- 2 files changed, 24 insertions(+), 19 deletions(-) This patch doesn't apply to my usb-testing branch at all, what went wrong? You merged this patch: https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?id=d0e08b0077f49e209bc90305ddf1ca434ac6cc0e Which does almost the same, except that it only supports dedicated reset controllers and not shared reset controllers. I'll send a new patch which changes things to add support for shared reset controllers. This will also bring the ehci-platform reset handling code inline with ohci-platform again. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 1/7] regulator: fixed: add support for ACPI interface
On Wed, Jun 08, 2016 at 02:43:27PM +0100, Mark Brown wrote: > On Tue, Jun 07, 2016 at 09:42:48PM -0700, Greg Kroah-Hartman wrote: > > On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote: > > > > Add support to retrieve fixed voltage configure information through > > > ACPI interface. This is needed for Intel Bay Trail devices, where a > > > GPIO is used to control the USB vbus. > > > Can't do anything with this until I get an ack from the "owners" of this > > file. > > Please allow a reasonable time for review, it's been under a week since > this was posted. I would *not* expect this to be applied to another > tree, please don't do that. I wouldn't consider it, my response was to the developer of the patch, sorry for the confusion, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote: > Hi Greg, > > On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote: > > On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote: > >> In some Intel platforms, a single usb port is shared between USB host > >> and device controllers. The shared port is under control of a switch > >> which is defined in the Intel vendor defined extended capability for > >> xHCI. > >> > >> This patch adds the support to detect and create the platform device > >> for the port mux switch. > > Why do you need a platform device for this? You do nothing with this > > device, why create it at all? > > In this patch series, I have a generic framework for port mux devices > and two port mux drivers sitting on top the generic code. > > In this patch, I create a platform device for the real mux device in > Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux > into the generic framework and handle the power management > things in driver's pm entries (otherwise, the system can't be waken > up from system suspend). > > > And why is it a platform device, isn't is really a PCI device? Why > > would you ever find a "platform" device below a PCI device? Don't abuse > > platform devices for things that aren't. It makes me want to delete > > that whole interface more and more... > > Port mux devices are physical devices in Intel Cherry Trail and Broxton > SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI > space. OS kernel can enumerate it by looking up the xhci extended > capability list with a vendor specific capability ID. A physical device that maps registers into PCI space seems like a PCI device of some type to me :) Again, I hate platform devices for obvious reasons like this... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/14] usb: otg: use dev_dbg() instead of VDBG()
On Wed, 2016-06-08 at 12:03 +0300, Roger Quadros wrote:
> Now that we have a device reference in struct usb_otg
> let's use dev_dbg() for debug messages.
dev_vdbg vs dev_dbg
The patch subject and commit message don't match the code changes.
> diff --git a/drivers/usb/common/usb-otg-fsm.c
> b/drivers/usb/common/usb-otg-fsm.c
[]
> @@ -44,8 +37,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int
> protocol)
> int ret = 0;
>
> if (fsm->protocol != protocol) {
> - VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
> - fsm->protocol, protocol);
> + dev_vdbg(otg->dev,
> + "Changing role fsm->protocol= %d; new protocol= %d\n",
> + fsm->protocol, protocol);
etc...
--
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 0/2] Add a new Rockchip usb2 phy driver
Hi Frank, Am Dienstag, 7. Juni 2016, 17:15:52 schrieb Frank Wang: > The newer SoCs (rk3366, rk3399) of Rock-chip take a different usb-phy > IP block than rk3288 and before, and most of phy-related registers are > also different from the past, so a new phy driver is required necessarily. > > These series patches add phy-rockchip-inno-usb2.c and the corresponding > documentation. please see the replies on the v3 thread (regarding handling of multiple phy blocks) . Thanks Heiko > > Changes in v4: > - Used 'phy-supply' instead of 'vbus_*-supply'. > > Changes in v3: > - Supplemented some hardware-description into the devicetree bindings. > - Resolved the mapping defect between fixed value in driver and the > property in devicetree. > - Code cleanup. > > Changes in v2: > - Specified more hardware-description into the devicetree bindings. > - Optimized some process of driver. > > Frank Wang (2): > Documentation: bindings: add DT documentation for Rockchip USB2PHY > phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy > > .../bindings/phy/phy-rockchip-inno-usb2.txt| 62 ++ > drivers/phy/Kconfig|7 + > drivers/phy/Makefile |1 + > drivers/phy/phy-rockchip-inno-usb2.c | 614 > 4 files changed, 684 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt create > mode 100644 drivers/phy/phy-rockchip-inno-usb2.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
Re: Unable to safely detach external HDD on USB 3.0
On 08.06.2016 16:47, Mathias Nyman wrote: On 06.06.2016 21:31, Alan Stern wrote: On Sat, 4 Jun 2016, Marco Chiappero wrote: On 31/05/2016 16:34, Alan Stern wrote: On Tue, 31 May 2016, Mathias Nyman wrote: xhci specs say that port will move from Disconnected (PLS = RX_DETECT) to Polling if "SuperSpeed far-end receiver terminations are detected" From power-off state (PP=0) the connect status changes are only reported if OCA is set so it seems that is not an option. I'm looking at the capture log and if I understand it correcty, in frame 49 SET FEATURE, PLS = ss.disabled (xhci will write PORT_PED bit) 59 SET FEATURE, PLS = RxDetect (xhci will set pls=RxDetect) .. get_port_status, nothing set, no changes reported (several of these) 73 SET FEATURE -> Port remote wake mask .. get_port status reports Connected, enabled and port status change event Could it be that setting the port remote wake mask somehow triggers the connect status change event? It's the first time we write to PORTSC for this port after setting link state to rx.detect It's possible, but I doubt it. More likely it just takes a few hundred ms for the RxDetect link training to realize there is a connection. Marco, you can check whether this is true by doing (as root): echo on >/sys/bus/usb/devices/usb4/power/control before starting your test. Alan Stern Hi, Sorry for my late reply but I've been very busy this week. You can find the new captures in the same directory. Or just use the following direct links: https://drive.google.com/open?id=0B2tgr9hETOtgTEU1Qmc2eWNIYlE https://drive.google.com/open?id=0B2tgr9hETOtgZUV2TkZRRGJjMWs I could not notice any difference though. Anyway I'm happy to test patches if you want to try changes. The new capture proves my earlier point. It shows the same sequence of link-state changes as before (DISABLED -> RX_DETECT -> CONNECTED), but the port's remote wakeup mask was not set. As far as I can tell, USB-3 does not have any link state that will avoid negotiating a connection with a plugged-in cable but will signal the host when the cable is unplugged. Perhaps the best we can do is put the link into U3 with remote wakeup disabled on the device. Mathias, what do you think? I found a device that I can reproduce this with, I also tried on windows and the analyzer shows "safely remove hardware" puts it into U3 So U3 makes sense, but hub_usb3_port_disable() is used in several places in usb core, and I'm not sure if U3 fits all of them. Right now I'm even suspecting that the drives that auto-remount are the ones that are working better according to the specs. Tried a quick hack that sets link state in hub_usb3_port_disable() to U3 and return, instead of SS_DISABLED -> RX_DETECT. Seems to work, the mass storage no longer re-mounts after "udisks --detach" Analyzer also shows that after unplugging the cable while device is in U3 the device (upstream port) goes SS.Disabled, and downstream port (root hub) goes to RX_DETECT, and new is detected again if cable is conneted back in. -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 v10 1/7] regulator: fixed: add support for ACPI interface
On Tue, Jun 07, 2016 at 09:42:48PM -0700, Greg Kroah-Hartman wrote: > On Thu, Jun 02, 2016 at 09:37:23AM +0800, Lu Baolu wrote: > > Add support to retrieve fixed voltage configure information through > > ACPI interface. This is needed for Intel Bay Trail devices, where a > > GPIO is used to control the USB vbus. > Can't do anything with this until I get an ack from the "owners" of this > file. Please allow a reasonable time for review, it's been under a week since this was posted. I would *not* expect this to be applied to another tree, please don't do that. signature.asc Description: PGP signature
Re: Unable to safely detach external HDD on USB 3.0
On 06.06.2016 21:31, Alan Stern wrote: On Sat, 4 Jun 2016, Marco Chiappero wrote: On 31/05/2016 16:34, Alan Stern wrote: On Tue, 31 May 2016, Mathias Nyman wrote: xhci specs say that port will move from Disconnected (PLS = RX_DETECT) to Polling if "SuperSpeed far-end receiver terminations are detected" From power-off state (PP=0) the connect status changes are only reported if OCA is set so it seems that is not an option. I'm looking at the capture log and if I understand it correcty, in frame 49 SET FEATURE, PLS = ss.disabled (xhci will write PORT_PED bit) 59 SET FEATURE, PLS = RxDetect (xhci will set pls=RxDetect) .. get_port_status, nothing set, no changes reported (several of these) 73 SET FEATURE -> Port remote wake mask .. get_port status reports Connected, enabled and port status change event Could it be that setting the port remote wake mask somehow triggers the connect status change event? It's the first time we write to PORTSC for this port after setting link state to rx.detect It's possible, but I doubt it. More likely it just takes a few hundred ms for the RxDetect link training to realize there is a connection. Marco, you can check whether this is true by doing (as root): echo on >/sys/bus/usb/devices/usb4/power/control before starting your test. Alan Stern Hi, Sorry for my late reply but I've been very busy this week. You can find the new captures in the same directory. Or just use the following direct links: https://drive.google.com/open?id=0B2tgr9hETOtgTEU1Qmc2eWNIYlE https://drive.google.com/open?id=0B2tgr9hETOtgZUV2TkZRRGJjMWs I could not notice any difference though. Anyway I'm happy to test patches if you want to try changes. The new capture proves my earlier point. It shows the same sequence of link-state changes as before (DISABLED -> RX_DETECT -> CONNECTED), but the port's remote wakeup mask was not set. As far as I can tell, USB-3 does not have any link state that will avoid negotiating a connection with a plugged-in cable but will signal the host when the cable is unplugged. Perhaps the best we can do is put the link into U3 with remote wakeup disabled on the device. Mathias, what do you think? I found a device that I can reproduce this with, I also tried on windows and the analyzer shows "safely remove hardware" puts it into U3 So U3 makes sense, but hub_usb3_port_disable() is used in several places in usb core, and I'm not sure if U3 fits all of them. Right now I'm even suspecting that the drives that auto-remount are the ones that are working better according to the specs. -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: Race condition in usbfs / libusb when using reap-after-disconnect
Hi, On 07-06-16 17:18, Alan Stern wrote: On Tue, 7 Jun 2016, Hans de Goede wrote: Hi, On 06-06-16 19:03, Alan Stern wrote: On Mon, 6 Jun 2016, Hans de Goede wrote: Hi, On 06-06-16 16:48, Greg Kroah-Hartman wrote: On Mon, Jun 06, 2016 at 01:44:23PM +0200, Hans de Goede wrote: Hi All, While looking at libusb today I ended up looking at the reap-after-disconnect code. What stands out is that libusb expects to be able to reap all outstanding urbs on a device on receiving a POLL_ERR status from poll (on supported kernels). But the usbfs poll implementation will return POLL_ERR as soon as ps->dev->state == USB_STATE_NOTATTACHED, and the kernel usb code sets state = USB_STATE_NOTATTACHED some time before usbdev_remove() gets called and moves all pending urbs to the async_completed list. This means that URBs can't be reaped immediately. But it will be possible to reap them shortly after libusb gets the POLLERR status, once the khubd work routine has processed the disconnection. Ack. So if libusb ends up calling poll between the setting state = USB_STATE_NOTATTACHED and usbdev_remove() being called then it ends up not reaping all urbs since it will stop monitoring the device after the first POLL_ERR. I'll write a libusb fix for this, but I'm wondering if this is something which we also want to fix on the kernel side? We could. Would it be worthwhile to do so? That is exactly the question this mail thread is intended to answer :) Would fixing this in the kernel mean that libusb fixes are not needed? If so, I would recommend doing this. A kernel side fix is non trivial, whereas the libusb fix is trivial since we also need to deal with kernels which do not support reaping urbs after disconnect at all. Basically the question is, do we consider not being able to reap all urbs after the first POLL_ERR a kernel bug, or should userspace keep track of how many urbs it has outstanding and call the reap ioctl (with some msleep after failure) until it has reaped all urbs ? We already have POLLOUT to indicate when a completed URB is available for reaping. Can libusb use that? libusb already use that normally, but on a POLLERR it will remove the fd from the pollfds since POLLERR will stay set once set, so leaving it in pollfds means we will just set there busy-waiting. Strictly speaking, POLLERR just indicates an error condition, of any kind. Ack, but the current semantics for how this works with usbfs are somewhat sub-optimal from a libusb pov. The fix for this is easy since libusb already has code in place to deal with older kernels which do not support REAP after disconnect at all and I plan to use that (rather then wait if we hit the race), but it does make the whole REAP after disconnect function a whole lot less useful. You also will have to handle kernels like the current one, that do support reap after disconnect but return POLLERR before the URBs are cancelled. Given that, how useful would changing the kernel be? That is a good question, right now not so useful, but given how simple your proposed fix is, I think it is worthwhile to fix this, so that in the future we can maybe simplify libusb a bit ? And it might also be useful for a libusb replacement which is something which some people have been discussing (not me, and I do not remember the details). We could alter usbdev_poll so that it returns POLLERR only after usbdev_remove has been called (say, only when ps->list is empty, ignoring ps->dev->state). That would be easy to do, like in the patch below. What do you think? The proposed fix looks good to me and is surprisingly simple :) Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/62] usb: dwc3: core: simplify suspend/resume operations
Felipe,
On 30/05/16 14:34, Felipe Balbi wrote:
> now that we have re-factored dwc3_core_init() and
> dwc3_core_exit() we can use them for suspend/resume
> operations.
>
> This will help us avoid some common mistakes when
> patching code when we have duplicated pieces of code
> doing the same thing.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/dwc3/core.c | 59
> +
> drivers/usb/dwc3/core.h | 3 ---
> 2 files changed, 5 insertions(+), 57 deletions(-)
This is the bad patch that breaks dwc3 on system suspend/resume.
Please see my comments below.
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index cbdefbb3d302..80e9affd3d77 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1113,33 +1113,19 @@ static int dwc3_remove(struct platform_device *pdev)
> static int dwc3_suspend(struct device *dev)
> {
> struct dwc3 *dwc = dev_get_drvdata(dev);
> - unsigned long flags;
> -
> - spin_lock_irqsave(&dwc->lock, flags);
Is it safe to call dwc3_gadget_suspend() or dwc3_core_exit without holding
dwc->lock?
>
> switch (dwc->dr_mode) {
> case USB_DR_MODE_PERIPHERAL:
> case USB_DR_MODE_OTG:
> dwc3_gadget_suspend(dwc);
> - /* FALLTHROUGH */
> + break;
> case USB_DR_MODE_HOST:
> default:
> - dwc3_event_buffers_cleanup(dwc);
> + /* do nothing */
> break;
> }
>
> - dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL);
> - spin_unlock_irqrestore(&dwc->lock, flags);
> -
> - usb_phy_shutdown(dwc->usb3_phy);
> - usb_phy_shutdown(dwc->usb2_phy);
> - phy_exit(dwc->usb2_generic_phy);
> - phy_exit(dwc->usb3_generic_phy);
> -
> - usb_phy_set_suspend(dwc->usb2_phy, 1);
> - usb_phy_set_suspend(dwc->usb3_phy, 1);
> - WARN_ON(phy_power_off(dwc->usb2_generic_phy) < 0);
> - WARN_ON(phy_power_off(dwc->usb3_generic_phy) < 0);
> + dwc3_core_exit(dwc);
dwc3_core_exit() does a phy_power_off() as well which
changes the behaviour.
How can we power off the PHY if we want remote wakeup or
connect/disconnect to be detected? Also, this would break the USB link
to a suspended device.
>
> pinctrl_pm_select_sleep_state(dev);
>
> @@ -1149,36 +1135,14 @@ static int dwc3_suspend(struct device *dev)
> static int dwc3_resume(struct device *dev)
> {
> struct dwc3 *dwc = dev_get_drvdata(dev);
> - unsigned long flags;
> int ret;
>
> pinctrl_pm_select_default_state(dev);
>
> - usb_phy_set_suspend(dwc->usb2_phy, 0);
> - usb_phy_set_suspend(dwc->usb3_phy, 0);
> - ret = phy_power_on(dwc->usb2_generic_phy);
> - if (ret < 0)
> + ret = dwc3_core_init(dwc);
> + if (ret)
> return ret;
You've replaced just restoring gctl with a full core_init().
We will loose XHCI host controller state and the devices connected to it
will have to be re-enumerated.
So this cant be done IMO.
Why not just restore GCTL instead of doing a full core re-init?
>
> - ret = phy_power_on(dwc->usb3_generic_phy);
> - if (ret < 0)
> - goto err_usb2phy_power;
> -
> - usb_phy_init(dwc->usb3_phy);
> - usb_phy_init(dwc->usb2_phy);
> - ret = phy_init(dwc->usb2_generic_phy);
> - if (ret < 0)
> - goto err_usb3phy_power;
> -
> - ret = phy_init(dwc->usb3_generic_phy);
> - if (ret < 0)
> - goto err_usb2phy_init;
> -
> - spin_lock_irqsave(&dwc->lock, flags);
> -
> - dwc3_event_buffers_setup(dwc);
> - dwc3_writel(dwc->regs, DWC3_GCTL, dwc->gctl);
> -
> switch (dwc->dr_mode) {
> case USB_DR_MODE_PERIPHERAL:
> case USB_DR_MODE_OTG:
> @@ -1190,24 +1154,11 @@ static int dwc3_resume(struct device *dev)
> break;
> }
>
> - spin_unlock_irqrestore(&dwc->lock, flags);
> -
> pm_runtime_disable(dev);
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
>
> return 0;
> -
> -err_usb2phy_init:
> - phy_exit(dwc->usb2_generic_phy);
> -
> -err_usb3phy_power:
> - phy_power_off(dwc->usb3_generic_phy);
> -
> -err_usb2phy_power:
> - phy_power_off(dwc->usb2_generic_phy);
> -
> - return ret;
> }
> #endif /* CONFIG_PM_SLEEP */
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index ce9596b1b1f4..58509cedb038 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -835,9 +835,6 @@ struct dwc3 {
>
> enum usb_dr_modedr_mode;
>
> - /* used for suspend/resume */
> - u32 gctl;
> -
> u32 fladj;
> u32 nr_scratch;
> u32 u1u2;
>
--
cheers,
-roger
--
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 v9 08/14] usb: otg: add OTG/dual-role core
On Wed, Jun 08, 2016 at 01:12:10PM +0300, Roger Quadros wrote:
> Hi,
>
> On 08/06/16 12:53, Peter Chen wrote:
> > On Wed, Jun 08, 2016 at 12:03:40PM +0300, Roger Quadros wrote:
> >> +int usb_otg_unregister(struct device *dev)
> >> +{
> >> + struct usb_otg *otg;
> >> +
> >> + mutex_lock(&otg_list_mutex);
> >> + otg = usb_otg_get_data(dev);
> >> + if (!otg) {
> >> + dev_err(dev, "otg: %s: device not in otg list\n",
> >> + __func__);
> >> + mutex_unlock(&otg_list_mutex);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + /* prevent unregister till both host & gadget have unregistered */
> >> + if (otg->host || otg->gadget) {
> >> + dev_err(dev, "otg: %s: host/gadget still registered\n",
> >> + __func__);
> >
> > You need to call mutex_unlock here
>
> Indeed. good catch.
> >
> >> +
> >> +int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
> >> +{
> >
> > What this API is for? Why need it in this version?
>
> we moved gadget to otg registration from udc_bind_to_driver() to
> usb_add_gadget_udc_release().
> This means there is a window when gadget function driver (e.g. g_zero) is not
> loaded.
> We don't want to start the gadget controller in that window.
>
> usb_otg_gadget_ready() is used by gadget core to notify the otg core when the
> function driver
> is ready or not-ready.
>
Why you need to move this from gadget's probe to udc's probe? Currently,
the sequence of gadget and udc's probe is random, but udc_bind_to_driver
is called when udc is ready.
--
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: Nokia N900: musb is in wrong state after boot
On 6/8/2016 3:18 PM, joerg Reisenweber wrote: Tony, what do you think about that patch? Tony, PING Yeah I don't know, AFAIK we don't have a generic way to force MUSB to change mode without ID pin. If you have figured something generic for that which does not actually tinker with the PHY registers directly, that should be the generic musb_set_mode() that we've been wondering about for years. #define MUSB_TEST_FORCE_HOST0x80 Can someone confirm on MUSB's docs (and actual running system) that this does what's supposed to do? The MUSB programmer's guide says the CID (sic) input is ignored when the Force_Host bit is set. The host mode is entered when the Session bit is set. But I don't have a MUSB hardware readily available to confirm. MBR, Sergei also says: While the FORCE_HOST bit remains set, the core will enter Host mode when the Session bit is set and **remain in Host mode until the Session bit is cleared even if a connected device is disconnected during the session.** The operating speed while in this mode **is determined for the setting of the FORCE_HS and FORCE_FS bits of the Testmode register** in Section 23.1.4.11. Yeah, the MUSB PG says the same. [...] /j MBR, Sergei -- 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: [balbi-usb:testing/next 64/67] phy-generic.c:undefined reference to `usb_gadget_vbus_connect'
On Wed, Jun 08, 2016 at 01:32:29PM +0300, Felipe Balbi wrote: > > Hi, > > Peter Chen writes: > >> > Would you consider only keeps vbus and pull dp API at gadget.h, and move > >> > out others gadget APIs which is dedicated for gadget function? > >> > >> no, sorry :-) All these drivers depend on the gadget API to work. For > >> host-only builds, they'll still work because of the stub implemented in > >> gadget.h. > > > > Do you want these drivers to add dependency on GADGET at Kconfig > > or not? If you want, we don't need stub at gadget.h since the gadget > > just look at what the patch does: > > if USB_GADGET=m, then NOP must be m too. if USB_GADGET=y, then NOP can > be y or m. > > > will be built if these driver is built. I assume you don't want, then > > that's ok, we don't need to change anything on Kconfig to fix build > > issue. > > There was, anyway, a bug in my original patch in that it didn't provide > stubs for cases were USB_GADGET=n. That bug is now fixed, I'll still > wait a couple days for any 0-day build problems. > > >> Also, did you test the version I've apended to previous email? I noticed > >> that had one extra mistake which I correct in the version below: > >> > > > > Sorry, I did not try since I still have some questions for solution:) > > heh. You're not even willing to try a patch, rather than complaining > about pointless stuff all day. Thanks > The reason why I did not try your patch is: I don't think the PHY needs to depend on GADGET since your 1st patch like below: + depends on USB_GADGET=y || USB_GADGET=NOP_USB_XCEIV + depends on NOP_USB_XCEIV But look at your the newest patch, you have fixed it like below: + depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, NOP can't be built-in I agree with that since PHY can not depend on GADGET if it does not use gadget APIs. After trying your patch, you only fixed NOP PHY build issue (chipidea build issue is fixed too), but without for other code. You may need to fix all like my last grep: b29397@shlinux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn "usb_gadget_vbus_connect" ./drivers/phy/phy-twl4030-usb.c:559: * REVISIT usb_gadget_vbus_connect(...) as needed, ditto ./drivers/usb/phy/phy-gpio-vbus-usb.c:126: usb_gadget_vbus_connect(gpio_vbus->phy.otg->gadget); ./drivers/usb/phy/phy-tahvo.c:89: usb_gadget_vbus_connect(tu->phy.otg->gadget); ./drivers/usb/phy/phy-isp1301-omap.c:929: usb_gadget_vbus_connect(isp->phy.otg->gadget); ./drivers/usb/phy/phy-isp1301-omap.c:1466: usb_gadget_vbus_connect(otg->gadget); ./drivers/usb/phy/phy-generic.c:122: usb_gadget_vbus_connect(otg->gadget); ./drivers/usb/phy/phy-generic.c:190:usb_gadget_vbus_connect(gadget); ./drivers/usb/phy/phy-msm-usb.c:783: usb_gadget_vbus_connect(phy->otg->gadget); ./drivers/usb/phy/phy-mv-usb.c:234: usb_gadget_vbus_connect(otg->gadget); ./drivers/usb/chipidea/otg.c:133: usb_gadget_vbus_connect(&ci->gadget); ./drivers/usb/chipidea/otg.c:138: usb_gadget_vbus_connect(&ci->gadget); ./drivers/usb/chipidea/udc.c:1964: usb_gadget_vbus_connect(&ci->gadget); ./drivers/usb/chipidea/otg_fsm.c:567: usb_gadget_vbus_connect(&ci->gadget); b29397@shlinux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn "usb_gadget_vbus_disconnect" ./drivers/usb/phy/phy-gpio-vbus-usb.c:145: usb_gadget_vbus_disconnect(gpio_vbus->phy.otg->gadget); ./drivers/usb/phy/phy-gpio-vbus-usb.c:200: usb_gadget_vbus_disconnect(otg->gadget); ./drivers/usb/phy/phy-tahvo.c:108: usb_gadget_vbus_disconnect(tu->phy.otg->gadget); ./drivers/usb/phy/phy-tahvo.c:165: usb_gadget_vbus_disconnect(tu->phy.otg->gadget); ./drivers/usb/phy/phy-tahvo.c:175: usb_gadget_vbus_disconnect(tu->phy.otg->gadget); ./drivers/usb/phy/phy-isp1301-omap.c:312: return usb_gadget_vbus_disconnect(isp->phy.otg->gadget); ./drivers/usb/phy/phy-isp1301-omap.c:989: usb_gadget_vbus_disconnect(otg->gadget); ./drivers/usb/phy/phy-isp1301-omap.c:1342: usb_gadget_vbus_disconnect(otg->gadget); ./drivers/usb/phy/phy-generic.c:133: usb_gadget_vbus_disconnect(otg->gadget); ./drivers/usb/phy/phy-fsl-usb.c:622: usb_gadget_vbus_disconnect(otg->gadget); ./drivers/usb/phy/phy-msm-usb.c:786: usb_gadget_vbus_disconnect(phy->otg->gadget); ./drivers/usb/phy/phy-mv-usb.c:236: usb_gadget_vbus_disconnect(otg->gadget); ./drivers/usb/chipidea/otg.c:140: usb_gadget_vbus_disconnect(&ci->gadget); ./drivers/usb/chipidea/otg_fsm.c:569: usb_gadget_vbus_disconnect(&ci->gadget); b29397@shlinux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn "usb_gadget_connect" ./drivers/power/isp1704_charger.c:272:
Re: Nokia N900: musb is in wrong state after boot
On Wed 08 June 2016 15:04:02 Sergei Shtylyov wrote: > On 6/8/2016 1:02 PM, Felipe Balbi wrote: > >> * Pali Rohár [160607 05:53]: > Tony, what do you think about that patch? > >>> > >>> Tony, PING > >> > >> Yeah I don't know, AFAIK we don't have a generic way to > >> force MUSB to change mode without ID pin. If you have figured > >> something generic for that which does not actually tinker with > >> the PHY registers directly, that should be the generic > >> musb_set_mode() that we've been wondering about for years. > > > > #define MUSB_TEST_FORCE_HOST0x80 > > > > Can someone confirm on MUSB's docs (and actual running system) that this > > does what's supposed to do? > > The MUSB programmer's guide says the CID (sic) input is ignored when the > Force_Host bit is set. The host mode is entered when the Session bit is > set. But I don't have a MUSB hardware readily available to confirm. > > MBR, Sergei also says: While the FORCE_HOST bit remains set, the core will enter Host mode when the Session bit is set and **remain in Host mode until the Session bit is cleared even if a connected device is disconnected during the session.** The operating speed while in this mode **is determined for the setting of the FORCE_HS and FORCE_FS bits of the Testmode register** in Section 23.1.4.11. see http://talk.maemo.org/showthread.php?p=685367 It's not any fully operational hostmode. /j -- () ascii ribbon campaign /\ against html e-mail - against proprietary attachments http://www.georgedillon.com/web/html_email_is_evil.shtml http://www.nonhtmlmail.org/campaign.html http://www.georgedillon.com/web/html_email_is_evil_still.shtml http://www.gerstbach.at/2004/ascii/ (German) -- 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 v9 12/14] usb: hcd: Adapt to OTG core
On 6/8/2016 3:06 PM, Roger Quadros wrote:
Introduce usb_otg_add/remove_hcd() for use by host
controllers that are part of OTG/dual-role port.
Non Device tree platforms can use the otg_dev argument
to specify the OTG controller device. If otg_dev is NULL
then the device tree node's otg-controller property is used to
get the otg_dev device.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/core/hcd.c | 55 +
include/linux/usb/hcd.h | 4
2 files changed, 59 insertions(+)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index ae6c76d..c6f4155 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
[...]
@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_remove_hcd);
+static struct otg_hcd_ops otg_hcd_intf = {
+.add = usb_add_hcd,
+.remove = usb_remove_hcd,
+.usb_bus_start_enum = usb_bus_start_enum,
+.usb_control_msg = usb_control_msg,
+.usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD
Device managing a driver? That's interesting. :-)
Well it is the OTG controller instance really.
How else do you want me to write it?
Just HC again? Or maybe HCI (HC interface)?
cheers,
-roger
MBR, Sergei
--
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 v9 03/14] usb: hcd.h: Add OTG to HCD interface
On 6/8/2016 3:04 PM, Roger Quadros wrote: The OTG core will use struct otg_hcd_ops to interface with the HCD controller. Host controller driver (HCD) controller? Maybe just HC? :-) OK. OTOH, my googling has shown that HCD may stand for both HC driver and HC device... The host controller device sounds a bit tautological however... [...] -- cheers, -roger MBR, Sergei -- 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 v9 12/14] usb: hcd: Adapt to OTG core
On 08/06/16 14:42, Sergei Shtylyov wrote:
> On 6/8/2016 12:03 PM, Roger Quadros wrote:
>
>> Introduce usb_otg_add/remove_hcd() for use by host
>> controllers that are part of OTG/dual-role port.
>>
>> Non Device tree platforms can use the otg_dev argument
>> to specify the OTG controller device. If otg_dev is NULL
>> then the device tree node's otg-controller property is used to
>> get the otg_dev device.
>>
>> Signed-off-by: Roger Quadros
>> Acked-by: Peter Chen
>> ---
>> drivers/usb/core/hcd.c | 55
>> +
>> include/linux/usb/hcd.h | 4
>> 2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index ae6c76d..c6f4155 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
> [...]
>> @@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
>> }
>> EXPORT_SYMBOL_GPL(usb_remove_hcd);
>>
>> +static struct otg_hcd_ops otg_hcd_intf = {
>> +.add = usb_add_hcd,
>> +.remove = usb_remove_hcd,
>> +.usb_bus_start_enum = usb_bus_start_enum,
>> +.usb_control_msg = usb_control_msg,
>> +.usb_hub_find_child = usb_hub_find_child,
>> +};
>> +
>> +/**
>> + * usb_otg_add_hcd - Register the HCD with OTG core.
>> + * @hcd: the usb_hcd structure to initialize
>> + * @irqnum: Interrupt line to allocate
>> + * @irqflags: Interrupt type flags
>> + * @otg_dev: OTG controller device managing this HCD
>
>Device managing a driver? That's interesting. :-)
Well it is the OTG controller instance really.
How else do you want me to write it?
cheers,
-roger
--
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 v9 03/14] usb: hcd.h: Add OTG to HCD interface
Hi,
On 08/06/16 14:39, Sergei Shtylyov wrote:
> Hello.
>
> On 6/8/2016 12:03 PM, Roger Quadros wrote:
>
>> The OTG core will use struct otg_hcd_ops to interface
>> with the HCD controller.
>
>Host controller driver (HCD) controller? Maybe just HC? :-)
OK.
>
>> The main purpose of this interface is to avoid directly
>> calling HCD APIs from the OTG core as they
>> wouldn't be defined in the built-in symbol table if
>> CONFIG_USB is m.
>>
>> Signed-off-by: Roger Quadros
>> Acked-by: Peter Chen
>> ---
>> include/linux/usb/hcd.h | 24
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
>> index 66fc137..7914bed 100644
>> --- a/include/linux/usb/hcd.h
>> +++ b/include/linux/usb/hcd.h
>> @@ -400,6 +400,30 @@ struct hc_driver {
>>
>> };
>>
>> +/**
>> + * struct otg_hcd_ops - Interface between OTG core and HCD
>> + *
>> + * Provided by the HCD core to allow the OTG core to interface with the HCD
>> + *
>> + * @add: function to add the HCD
>> + * @remove: function to remove the HCD
>> + * @usb_bus_start_enum: function to immediately start bus enumeration
>> + * @usb_control_msg: function to build and send of a control urb
>
>That "of" is not needed. And it's URB.
Right.
>
> [...]
>
--
cheers,
-roger
--
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: Nokia N900: musb is in wrong state after boot
On 6/8/2016 1:02 PM, Felipe Balbi wrote: * Pali Rohár [160607 05:53]: Tony, what do you think about that patch? Tony, PING Yeah I don't know, AFAIK we don't have a generic way to force MUSB to change mode without ID pin. If you have figured something generic for that which does not actually tinker with the PHY registers directly, that should be the generic musb_set_mode() that we've been wondering about for years. #define MUSB_TEST_FORCE_HOST0x80 Can someone confirm on MUSB's docs (and actual running system) that this does what's supposed to do? The MUSB programmer's guide says the CID (sic) input is ignored when the Force_Host bit is set. The host mode is entered when the Session bit is set. But I don't have a MUSB hardware readily available to confirm. MBR, Sergei -- 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 v9 12/14] usb: hcd: Adapt to OTG core
On 6/8/2016 12:03 PM, Roger Quadros wrote:
Introduce usb_otg_add/remove_hcd() for use by host
controllers that are part of OTG/dual-role port.
Non Device tree platforms can use the otg_dev argument
to specify the OTG controller device. If otg_dev is NULL
then the device tree node's otg-controller property is used to
get the otg_dev device.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/core/hcd.c | 55 +
include/linux/usb/hcd.h | 4
2 files changed, 59 insertions(+)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index ae6c76d..c6f4155 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
[...]
@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_remove_hcd);
+static struct otg_hcd_ops otg_hcd_intf = {
+ .add = usb_add_hcd,
+ .remove = usb_remove_hcd,
+ .usb_bus_start_enum = usb_bus_start_enum,
+ .usb_control_msg = usb_control_msg,
+ .usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD
Device managing a driver? That's interesting. :-)
[...]
MBR, Sergei
--
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 v9 03/14] usb: hcd.h: Add OTG to HCD interface
Hello.
On 6/8/2016 12:03 PM, Roger Quadros wrote:
The OTG core will use struct otg_hcd_ops to interface
with the HCD controller.
Host controller driver (HCD) controller? Maybe just HC? :-)
The main purpose of this interface is to avoid directly
calling HCD APIs from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB is m.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
include/linux/usb/hcd.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 66fc137..7914bed 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -400,6 +400,30 @@ struct hc_driver {
};
+/**
+ * struct otg_hcd_ops - Interface between OTG core and HCD
+ *
+ * Provided by the HCD core to allow the OTG core to interface with the HCD
+ *
+ * @add: function to add the HCD
+ * @remove: function to remove the HCD
+ * @usb_bus_start_enum: function to immediately start bus enumeration
+ * @usb_control_msg: function to build and send of a control urb
That "of" is not needed. And it's URB.
[...]
MBR, Sergei
--
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: Nokia N900: musb is in wrong state after boot
On Wed 08 June 2016 13:02:00 Felipe Balbi wrote: > Hi, > > Tony Lindgren writes: > > Yeah I don't know, AFAIK we don't have a generic way to > > force MUSB to change mode without ID pin. If you have figured > #define MUSB_TEST_FORCE_HOST 0x80 > > Can someone confirm on MUSB's docs (and actual running system) that this > does what's supposed to do? (disclaimer: all IIRC) Alas it doesn't, at least for the MentorGraphics MUSB in OMAP3. We use MUSB_TEST_FORCE_HOST in the H-E-N N900 hostmode hack (http://talk.maemo.org/showthread.php?p=696115) and it's really just a test thing that doesn't support full hostmode as we know it. There is definitely no other way to force the state machine in MUSB core into hostmode, than the "ID pin grounded" message from PHY via ULPI bus up to the MUSB, whether it's real or *sw-triggered by a command to PHY*. Shame on the MUSB core design for this. /jOERG signature.asc Description: This is a digitally signed message part.
Re: [PATCH v3 1/2] usb: ohci-at91: Forcibly suspend ports while USB suspend
Le 08/06/2016 12:04, Nicolas Ferre a écrit : > Le 08/06/2016 06:15, Wenyou Yang a écrit : >> In order to the save power consumption, as a workaround, suspend >> forcibly the USB PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI >> Interrupt Configuration Register in the SFRs while OHCI USB suspend. >> >> This suspend operation must be done before the USB clock is disabled, >> resume after the USB clock is enabled. >> >> Signed-off-by: Wenyou Yang > > Little nitpicking below... > >> --- >> >> Changes in v3: >> - Change the compatible description for more precise. >> >> Changes in v2: >> - Add compatible to support forcibly suspend the ports. >> - Add soc/at91/at91_sfr.h to accommodate the defines. >> - Add error checking for .sfr_regmap. >> - Remove unnecessary regmap_read() statement. >> >> .../devicetree/bindings/usb/atmel-usb.txt | 6 +- >> drivers/usb/host/ohci-at91.c | 80 >> +- >> include/soc/at91/at91_sfr.h| 29 Oops sorry, additional comment which is not nitpicking, this one: We already have SFR header file in this patch: Author: Cyrille Pitchen Date: Thu Mar 17 17:04:00 2016 +0100 ARM: dts: at91: sama5d2: add SFR node This SFR node is looked up by the I2S controller driver to tune the SFR_I2SCLKSEL register. Signed-off-by: Cyrille Pitchen Signed-off-by: Ludovic Desroches Acked-by: Alexandre Belloni Acked-by: Rob Herring Signed-off-by: Nicolas Ferre Which is already accepted by arm-soc guys for 4.7... So my ack transforms into a nack, sorry... We will have to coordinate the effort and maybe take the whole series with us. But for sure, you'll have to use the existing include/soc/at91/atmel-sfr.h file and build on top of it... Bye, >> 3 files changed, 112 insertions(+), 3 deletions(-) >> create mode 100644 include/soc/at91/at91_sfr.h [..] > > But you can take my: > > Acked-by: Nicolas Ferre > > with the little corrections listed. > > Alan, We plan to take the second patch of this series with AT91 git tree > through arm-soc. Do you agree to take this one through yours? Alan, forget this request, we'll have to coordinate differently. Sorry for the noise. Bye, -- Nicolas Ferre -- 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: [balbi-usb:testing/next 64/67] phy-generic.c:undefined reference to `usb_gadget_vbus_connect'
Hi, Peter Chen writes: >> > Would you consider only keeps vbus and pull dp API at gadget.h, and move >> > out others gadget APIs which is dedicated for gadget function? >> >> no, sorry :-) All these drivers depend on the gadget API to work. For >> host-only builds, they'll still work because of the stub implemented in >> gadget.h. > > Do you want these drivers to add dependency on GADGET at Kconfig > or not? If you want, we don't need stub at gadget.h since the gadget just look at what the patch does: if USB_GADGET=m, then NOP must be m too. if USB_GADGET=y, then NOP can be y or m. > will be built if these driver is built. I assume you don't want, then > that's ok, we don't need to change anything on Kconfig to fix build > issue. There was, anyway, a bug in my original patch in that it didn't provide stubs for cases were USB_GADGET=n. That bug is now fixed, I'll still wait a couple days for any 0-day build problems. >> Also, did you test the version I've apended to previous email? I noticed >> that had one extra mistake which I correct in the version below: >> > > Sorry, I did not try since I still have some questions for solution:) heh. You're not even willing to try a patch, rather than complaining about pointless stuff all day. Thanks -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 2/2] ARM: at91/dt: sama5d2: Use new compatible for ohci node
On 08/06/2016 at 12:06:11 +0200, Nicolas Ferre wrote :
> Le 08/06/2016 06:15, Wenyou Yang a écrit :
> > Use compatible "atmel,sama5d2-ohci" to be capable of suspending
> > ports while sleep to save the power consumption.
> >
> > Signed-off-by: Wenyou Yang
> > ---
> >
> > Changes in v3: None
> > Changes in v2:
> > - Use the new compatible for ohci-node.
> >
> > arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> > index 78996bd..03d6724 100644
> > --- a/arch/arm/boot/dts/sama5d2.dtsi
> > +++ b/arch/arm/boot/dts/sama5d2.dtsi
> > @@ -232,7 +232,7 @@
> > };
> >
> > usb1: ohci@0040 {
> > - compatible = "atmel,at91rm9200-ohci", "usb-ohci";
> > + compatible = "atmel,sama5d2-ohci", "usb-ohci";
>
> We must change this to:
> + compatible = "atmel,sama5d2-ohci",
> "atmel,at91rm9200-ohci", "usb-ohci";
>
> To make it independent from the one that may reach Mainline through the USB
> git tree.
>
Well, like discussed, I'd say that a new compatible is not needed as we
already know on which chip we are running depending on the SFR that we
can lookup. My concern was only to avoid calling a function
unnecessarily on !sama5d2 platforms.
>
> > reg = <0x0040 0x10>;
> > interrupts = <41 IRQ_TYPE_LEVEL_HIGH 2>;
> > clocks = <&uhphs_clk>, <&uhphs_clk>, <&uhpck>;
> >
>
> Bye,
> --
> Nicolas Ferre
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
Hi, On 08-06-16 12:23, Maxime Ripard wrote: Hi, On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote: phy-sun4i-usb now has proper dr_mode handling, it always registers an extcon, and sends a notify with the mode (even when in peripheral- / host-only mode) at least once. So we can simply the sunxi musb glue by always registering its extcon notifier and relying on sunxi_musb_work() to enable vbus when in host-only mode. This also enables host- and peripheral-only mode with vbus monitoring. Signed-off-by: Hans de Goede It's been a bit painful to track all the patches needed so that it applies properly, but I've finally been able to test it on a Sinlinx SinA33 with peripheral-only mUSB, and it works like a charm. You can add my Tested-by. Great, thanks for testing. This is the board which has an otg connector with vbus not connected, right? Yet it does have a functional id-pin, right ? In that case you should be able to put it in dual-role mode (only specify the id-pin in the phy dts node, no vbus / vbus-monitoring) and then it _should_ work in host mode if you use a powered hub. I'm fine with putting in peripheral-only mode, but as said dual-role might work with a powered hub. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Nokia N900: musb is in wrong state after boot
On Wednesday 08 June 2016 12:02:00 Felipe Balbi wrote: > Hi, > > Tony Lindgren writes: > > * Pali Rohár [160607 05:53]: > >> > Tony, what do you think about that patch? > >> > >> Tony, PING > > > > Yeah I don't know, AFAIK we don't have a generic way to > > force MUSB to change mode without ID pin. If you have figured > > something generic for that which does not actually tinker with > > the PHY registers directly, that should be the generic > > musb_set_mode() that we've been wondering about for years. > > #define MUSB_TEST_FORCE_HOST 0x80 > > Can someone confirm on MUSB's docs (and actual running system) that > this does what's supposed to do? Maybe this can help you: https://garage.maemo.org/plugins/ggit/browse.php/?p=kernel-power;a=blob;f=kernel-power-2.6.28/debian/patches/usbhostmode.diff Patch for 2.6.28 omap1 kernel which adds usb host mode support for Nokia N900. Look for MUSB_TESTMODE, MUSB_TEST_FORCE_HOST is called together with MUSB_TEST_FORCE_FS or MUSB_TEST_FORCE_HS (line 355). -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling
Hi, On Sun, Jun 05, 2016 at 04:59:36PM +0200, Hans de Goede wrote: > phy-sun4i-usb now has proper dr_mode handling, it always registers an > extcon, and sends a notify with the mode (even when in peripheral- / > host-only mode) at least once. > > So we can simply the sunxi musb glue by always registering its extcon > notifier and relying on sunxi_musb_work() to enable vbus when in > host-only mode. > > This also enables host- and peripheral-only mode with vbus monitoring. > > Signed-off-by: Hans de Goede It's been a bit painful to track all the patches needed so that it applies properly, but I've finally been able to test it on a Sinlinx SinA33 with peripheral-only mUSB, and it works like a charm. You can add my Tested-by. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [balbi-usb:testing/next 64/67] phy-generic.c:undefined reference to `usb_gadget_vbus_connect'
On Wed, Jun 08, 2016 at 12:50:57PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Peter Chen writes:
> >> >> > Hi Felipe,
> >> >> >
> >> >> > It causes any drivers which use symbols from gadget.h needs to build
> >> >> > USB_GADGET even it only wants to be host function. Any significant
> >> >> > benefits after moving it to udc-core.c?
> >> >>
> >> >> why is a host-only function calling into the gadget API?
> >> >>
> >> >
> >> > Well, Just like this case, the chipidea driver can be configured host
> >> > mode only, but it still uses generic phy.
> >>
> >> then we should allow for PHYs to not depend on the Gadget API, right?
> >
> > But you know it is not easy. The difficulty for gadget to do that is
> > vbus and pull dp, these two may be used by other subsystems, eg, vbus
> > may be used at PHY driver, dual-role switch driver, etc. The pull dp
> > may be used at USB charger driver, the secondary charger detection
> > needs to pull up dp for some charger designs.
>
> that's fine. And if they really need that, they depend on the Gadget
> API. That _is_ something the gadget API defines afterall.
>
> > See current kernel doc, it also says that.
> >
> > /**
> > * usb_gadget_vbus_connect - Notify controller that VBUS is powered
> > * @gadget:The device which now has VBUS power.
> > * Context: can sleep
> > *
> > * This call is used by a driver for an external transceiver (or GPIO)
> > * that detects a VBUS power session starting. Common responses include
> > * resuming the controller, activating the D+ (or D-) pullup to let the
> > * host detect that a USB device is attached, and starting to draw power
> > * (8mA or possibly more, especially after SET_CONFIGURATION).
> > *
> > * Returns zero on success, else negative errno.
> > */
> > static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
> > {
> > if (!gadget->ops->vbus_session)
> > return -EOPNOTSUPP;
> > return gadget->ops->vbus_session(gadget, 1);
> > }
> >
> > See current situations:
> >
> > b29397@linux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn
> > "usb_gadget_vbus_connect"
> > ./drivers/phy/phy-twl4030-usb.c:559: * REVISIT
> > usb_gadget_vbus_connect(...) as needed, ditto
> > ./drivers/usb/phy/phy-gpio-vbus-usb.c:126:
> > usb_gadget_vbus_connect(gpio_vbus->phy.otg->gadget);
> > ./drivers/usb/phy/phy-tahvo.c:89:
> > usb_gadget_vbus_connect(tu->phy.otg->gadget);
> > ./drivers/usb/phy/phy-isp1301-omap.c:929:
> > usb_gadget_vbus_connect(isp->phy.otg->gadget);
> > ./drivers/usb/phy/phy-isp1301-omap.c:1466:
> > usb_gadget_vbus_connect(otg->gadget);
> > ./drivers/usb/phy/phy-generic.c:122:
> > usb_gadget_vbus_connect(otg->gadget);
> > ./drivers/usb/phy/phy-generic.c:190:
> > usb_gadget_vbus_connect(gadget);
> > ./drivers/usb/phy/phy-msm-usb.c:783:
> > usb_gadget_vbus_connect(phy->otg->gadget);
> > ./drivers/usb/phy/phy-mv-usb.c:234:
> > usb_gadget_vbus_connect(otg->gadget);
> > ./drivers/usb/chipidea/otg.c:133:
> > usb_gadget_vbus_connect(&ci->gadget);
> > ./drivers/usb/chipidea/otg.c:138:
> > usb_gadget_vbus_connect(&ci->gadget);
> > ./drivers/usb/chipidea/udc.c:1964:
> > usb_gadget_vbus_connect(&ci->gadget);
> > ./drivers/usb/chipidea/otg_fsm.c:567:
> > usb_gadget_vbus_connect(&ci->gadget);
> > b29397@linux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn
> > "usb_gadget_vbus_disconnect"
> > ./drivers/usb/phy/phy-gpio-vbus-usb.c:145:
> > usb_gadget_vbus_disconnect(gpio_vbus->phy.otg->gadget);
> > ./drivers/usb/phy/phy-gpio-vbus-usb.c:200:
> > usb_gadget_vbus_disconnect(otg->gadget);
> > ./drivers/usb/phy/phy-tahvo.c:108:
> > usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
> > ./drivers/usb/phy/phy-tahvo.c:165:
> > usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
> > ./drivers/usb/phy/phy-tahvo.c:175:
> > usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
> > ./drivers/usb/phy/phy-isp1301-omap.c:312: return
> > usb_gadget_vbus_disconnect(isp->phy.otg->gadget);
> > ./drivers/usb/phy/phy-isp1301-omap.c:989:
> > usb_gadget_vbus_disconnect(otg->gadget);
> > ./drivers/usb/phy/phy-isp1301-omap.c:1342:
> > usb_gadget_vbus_disconnect(otg->gadget);
> > ./drivers/usb/phy/phy-generic.c:133:
> > usb_gadget_vbus_disconnect(otg->gadget);
> > ./drivers/usb/phy/phy-fsl-usb.c:622:
> > usb_gadget_vbus_disconnect(otg->gadget);
> > ./drivers/usb/phy/phy-msm-usb.c:786:
> > usb_gadget_vbus_disconnect(phy->otg->gadget);
> > ./drivers/usb/phy/phy-mv-usb.c:236:
> > usb_gadget_vbus_disconnect(otg->gadget);
> > ./drivers/usb/chipidea/otg.c:140:
> > usb_gadget_vbus_disconnect(&ci-
Re: [PATCH v9 08/14] usb: otg: add OTG/dual-role core
Hi,
On 08/06/16 12:53, Peter Chen wrote:
> On Wed, Jun 08, 2016 at 12:03:40PM +0300, Roger Quadros wrote:
>> +int usb_otg_unregister(struct device *dev)
>> +{
>> +struct usb_otg *otg;
>> +
>> +mutex_lock(&otg_list_mutex);
>> +otg = usb_otg_get_data(dev);
>> +if (!otg) {
>> +dev_err(dev, "otg: %s: device not in otg list\n",
>> +__func__);
>> +mutex_unlock(&otg_list_mutex);
>> +return -EINVAL;
>> +}
>> +
>> +/* prevent unregister till both host & gadget have unregistered */
>> +if (otg->host || otg->gadget) {
>> +dev_err(dev, "otg: %s: host/gadget still registered\n",
>> +__func__);
>
> You need to call mutex_unlock here
Indeed. good catch.
>
>> +
>> +int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
>> +{
>
> What this API is for? Why need it in this version?
we moved gadget to otg registration from udc_bind_to_driver() to
usb_add_gadget_udc_release().
This means there is a window when gadget function driver (e.g. g_zero) is not
loaded.
We don't want to start the gadget controller in that window.
usb_otg_gadget_ready() is used by gadget core to notify the otg core when the
function driver
is ready or not-ready.
>
>> +struct usb_otg *otg;
>> +struct device *gadget_dev = &gadget->dev;
>> +struct device *otg_dev = gadget->otg_dev;
>> +
>> +if (!otg_dev)
>> +return -EINVAL;
>> +
>> +mutex_lock(&otg_list_mutex);
>> +otg = usb_otg_get_data(otg_dev);
>> +mutex_unlock(&otg_list_mutex);
>> +if (!otg) {
>> +dev_err(gadget_dev,
>> +"otg: gadget %s wasn't registered with otg\n",
>> +dev_name(&gadget->dev));
>> +return -EINVAL;
>> +}
>> +
>> +mutex_lock(&otg->fsm.lock);
>> +if (otg->gadget != gadget) {
>> +mutex_unlock(&otg->fsm.lock);
>> +dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n",
>> +dev_name(&gadget->dev));
>> +return -EINVAL;
>> +}
>> +
>> +/* Start/stop FSM & gadget */
>> +otg->gadget_ready = ready;
>> +if (ready)
>> +usb_otg_start_fsm(otg);
>> +else
>> +usb_otg_stop_fsm(otg);
>> +
>> +dev_dbg(otg_dev, "otg: gadget %s %sready\n", dev_name(&gadget->dev),
>> +ready ? "" : "not ");
>> +
>> +mutex_unlock(&otg->fsm.lock);
>> +
>> +return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_otg_gadget_ready);
>> +
>> --- a/include/linux/usb/otg.h
>> +++ b/include/linux/usb/otg.h
>> @@ -10,10 +10,70 @@
>> #define __LINUX_USB_OTG_H
>>
>> #include
>> -#include
>> -#include
>> +#include
>> +#include
>> +#include
>
> The above two timer header files are not needed.
You had pointed this out earlier, but I missed it. Sorry.
--
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] ARM: at91/dt: sama5d2: Use new compatible for ohci node
Le 08/06/2016 06:15, Wenyou Yang a écrit :
> Use compatible "atmel,sama5d2-ohci" to be capable of suspending
> ports while sleep to save the power consumption.
>
> Signed-off-by: Wenyou Yang
> ---
>
> Changes in v3: None
> Changes in v2:
> - Use the new compatible for ohci-node.
>
> arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index 78996bd..03d6724 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -232,7 +232,7 @@
> };
>
> usb1: ohci@0040 {
> - compatible = "atmel,at91rm9200-ohci", "usb-ohci";
> + compatible = "atmel,sama5d2-ohci", "usb-ohci";
We must change this to:
+ compatible = "atmel,sama5d2-ohci",
"atmel,at91rm9200-ohci", "usb-ohci";
To make it independent from the one that may reach Mainline through the USB git
tree.
> reg = <0x0040 0x10>;
> interrupts = <41 IRQ_TYPE_LEVEL_HIGH 2>;
> clocks = <&uhphs_clk>, <&uhphs_clk>, <&uhpck>;
>
Bye,
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: ohci-at91: Forcibly suspend ports while USB suspend
Le 08/06/2016 06:15, Wenyou Yang a écrit :
> In order to the save power consumption, as a workaround, suspend
> forcibly the USB PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI
> Interrupt Configuration Register in the SFRs while OHCI USB suspend.
>
> This suspend operation must be done before the USB clock is disabled,
> resume after the USB clock is enabled.
>
> Signed-off-by: Wenyou Yang
Little nitpicking below...
> ---
>
> Changes in v3:
> - Change the compatible description for more precise.
>
> Changes in v2:
> - Add compatible to support forcibly suspend the ports.
> - Add soc/at91/at91_sfr.h to accommodate the defines.
> - Add error checking for .sfr_regmap.
> - Remove unnecessary regmap_read() statement.
>
> .../devicetree/bindings/usb/atmel-usb.txt | 6 +-
> drivers/usb/host/ohci-at91.c | 80
> +-
> include/soc/at91/at91_sfr.h| 29
> 3 files changed, 112 insertions(+), 3 deletions(-)
> create mode 100644 include/soc/at91/at91_sfr.h
>
> diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt
> b/Documentation/devicetree/bindings/usb/atmel-usb.txt
> index 5883b73..888deaa 100644
> --- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
> @@ -3,8 +3,10 @@ Atmel SOC USB controllers
> OHCI
>
> Required properties:
> - - compatible: Should be "atmel,at91rm9200-ohci" for USB controllers
> - used in host mode.
> + - compatible: Should be one of the following
> +"atmel,at91rm9200-ohci" for USB controllers used in host mode.
> +"atmel,sama5d2-ohci" for USB controllers used in host mode
> +on SAMA5D2 which can force to suspend.
What about "...on SAMA5D2 and compatible SoCs that must explicitly force
suspend through the Special Function Register (SFR)."
> - reg: Address and length of the register set for the device
> - interrupts: Should contain ehci interrupt
> - clocks: Should reference the peripheral, host and system clocks
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index d177372..54e8feb 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -21,8 +21,11 @@
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> +#include
>
> #include "ohci.h"
>
> @@ -45,12 +48,18 @@ struct at91_usbh_data {
> u8 overcurrent_changed[AT91_MAX_USBH_PORTS];
> };
>
> +struct ohci_at91_caps {
> + bool suspend_ctrl;
> +};
> +
> struct ohci_at91_priv {
> struct clk *iclk;
> struct clk *fclk;
> struct clk *hclk;
> bool clocked;
> bool wakeup;/* Saved wake-up state for resume */
> + const struct ohci_at91_caps *caps;
> + struct regmap *sfr_regmap;
> };
> /* interface and function clocks; sometimes also an AHB clock */
>
> @@ -132,6 +141,17 @@ static void at91_stop_hc(struct platform_device *pdev)
>
> /*-*/
>
> +struct regmap *at91_dt_syscon_sfr(void)
> +{
> + struct regmap *regmap;
> +
> + regmap = syscon_regmap_lookup_by_compatible("atmel,sama5d2-sfr");
> + if (IS_ERR(regmap))
> + regmap = NULL;
> +
> + return regmap;
> +}
> +
> static void usb_hcd_at91_remove (struct usb_hcd *, struct platform_device *);
>
> /* configure so an HC device and id are always provided */
> @@ -197,6 +217,17 @@ static int usb_hcd_at91_probe(const struct hc_driver
> *driver,
> goto err;
> }
>
> + ohci_at91->caps = (const struct ohci_at91_caps *)
> + of_device_get_match_data(&pdev->dev);
> + if (!ohci_at91->caps)
> + return -ENODEV;
> +
> + if (ohci_at91->caps->suspend_ctrl) {
> + ohci_at91->sfr_regmap = at91_dt_syscon_sfr();
> + if (!ohci_at91->sfr_regmap)
> + dev_warn(dev, "failed to find sfr node\n");
> + }
> +
> board = hcd->self.controller->platform_data;
> ohci = hcd_to_ohci(hcd);
> ohci->num_ports = board->ports;
> @@ -440,8 +471,17 @@ static irqreturn_t ohci_hcd_at91_overcurrent_irq(int
> irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static const struct ohci_at91_caps at91rm9200_caps = {
> + .suspend_ctrl = false,
> +};
> +
> +static const struct ohci_at91_caps sama5d2_caps = {
> + .suspend_ctrl = true,
> +};
> +
> static const struct of_device_id at91_ohci_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-ohci" },
> + { .compatible = "atmel,at91rm9200-ohci", .data = &at91rm9200_caps },
> + { .compatible = "atmel,sama5d2-ohci", .data = &sama5d2_caps },
> { /* sentinel */ }
> };
>
> @@ -581,6 +621,38 @@ static int ohci_hcd_at91_drv_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +static int ohci_at91_port_ctrl(struct regmap *regmap, bool enable)
> +{
Re: Nokia N900: musb is in wrong state after boot
Hi, Tony Lindgren writes: > * Pali Rohár [160607 05:53]: >> > Tony, what do you think about that patch? >> > >> >> Tony, PING > > Yeah I don't know, AFAIK we don't have a generic way to > force MUSB to change mode without ID pin. If you have figured > something generic for that which does not actually tinker with > the PHY registers directly, that should be the generic > musb_set_mode() that we've been wondering about for years. #define MUSB_TEST_FORCE_HOST0x80 Can someone confirm on MUSB's docs (and actual running system) that this does what's supposed to do? -- balbi signature.asc Description: PGP signature
Re: [balbi-usb:testing/next 64/67] otg.c:undefined reference to `usb_gadget_vbus_disconnect'
Hi,
kbuild test robot writes:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
> testing/next
> head: fc4d1f3f522648d92d0c046beedbc1469979499a
> commit: 74e51eb7b5b4f7ab33c099c20def3dce0c699006 [64/67] usb: gadget: move
> gadget API functions to udc-core
> config: i386-randconfig-h1-06081244 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
> reproduce:
> git checkout 74e51eb7b5b4f7ab33c099c20def3dce0c699006
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>drivers/built-in.o: In function `nop_set_peripheral':
>phy-generic.c:(.text+0x59f7a6): undefined reference to
> `usb_gadget_vbus_connect'
>drivers/built-in.o: In function `nop_gpio_vbus_thread':
>phy-generic.c:(.text+0x59f8a1): undefined reference to
> `usb_gadget_vbus_connect'
>phy-generic.c:(.text+0x59f925): undefined reference to
> `usb_gadget_vbus_disconnect'
>drivers/built-in.o: In function `gpio_vbus_set_peripheral':
>phy-gpio-vbus-usb.c:(.text+0x59ff45): undefined reference to
> `usb_gadget_vbus_disconnect'
>drivers/built-in.o: In function `gpio_vbus_work':
>phy-gpio-vbus-usb.c:(.text+0x5a02dd): undefined reference to
> `usb_gadget_vbus_disconnect'
>phy-gpio-vbus-usb.c:(.text+0x5a0333): undefined reference to
> `usb_gadget_vbus_connect'
>drivers/built-in.o: In function `ci_handle_vbus_change.part.6':
>>> otg.c:(.text+0x605e3b): undefined reference to `usb_gadget_vbus_disconnect'
>>> otg.c:(.text+0x605e49): undefined reference to `usb_gadget_vbus_connect'
>drivers/built-in.o: In function `isp1704_charger_work':
>isp1704_charger.c:(.text+0x648335): undefined reference to
> `usb_gadget_disconnect'
>isp1704_charger.c:(.text+0x64839d): undefined reference to
> `usb_gadget_connect'
>drivers/built-in.o: In function `isp1704_charger_probe':
>isp1704_charger.c:(.text+0x648760): undefined reference to
> `usb_gadget_disconnect'
thanks, fixed:
From a22b539c7082b6a3046b3a9cebb356a047a2d81d Mon Sep 17 00:00:00 2001
From: Felipe Balbi
Date: Tue, 31 May 2016 13:07:47 +0300
Subject: [PATCH] usb: gadget: move gadget API functions to udc-core
instead of defining all functions as static inlines,
let's move them to udc-core and export them with
EXPORT_SYMBOL_GPL, that way we can make sure that
only GPL drivers will use them.
As a side effect, it'll be nicer to add tracepoints
to the gadget API.
While at that, also fix Kconfig dependencies to
avoid randconfig build failures.
Signed-off-by: Felipe Balbi
---
drivers/usb/gadget/udc/udc-core.c | 573 +
drivers/usb/host/Kconfig | 2 +-
drivers/usb/phy/Kconfig | 5 +-
include/linux/usb/gadget.h| 585 --
4 files changed, 637 insertions(+), 528 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 6e8300d6a737..d2f28bb6dbda 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -59,6 +59,579 @@ static int udc_bind_to_driver(struct usb_udc *udc,
/* - */
+/**
+ * usb_ep_set_maxpacket_limit - set maximum packet size limit for endpoint
+ * @ep:the endpoint being configured
+ * @maxpacket_limit:value of maximum packet size limit
+ *
+ * This function should be used only in UDC drivers to initialize endpoint
+ * (usually in probe function).
+ */
+void usb_ep_set_maxpacket_limit(struct usb_ep *ep,
+ unsigned maxpacket_limit)
+{
+ ep->maxpacket_limit = maxpacket_limit;
+ ep->maxpacket = maxpacket_limit;
+}
+EXPORT_SYMBOL_GPL(usb_ep_set_maxpacket_limit);
+
+/**
+ * usb_ep_enable - configure endpoint, making it usable
+ * @ep:the endpoint being configured. may not be the endpoint named "ep0".
+ * drivers discover endpoints through the ep_list of a usb_gadget.
+ *
+ * When configurations are set, or when interface settings change, the driver
+ * will enable or disable the relevant endpoints. while it is enabled, an
+ * endpoint may be used for i/o until the driver receives a disconnect() from
+ * the host or until the endpoint is disabled.
+ *
+ * the ep0 implementation (which calls this routine) must ensure that the
+ * hardware capabilities of each endpoint match the descriptor provided
+ * for it. for example, an endpoint named "ep2in-bulk" would be usable
+ * for interrupt transfers as well as bulk, but it likely couldn't be used
+ * for iso transfers or for endpoint 14. some endpoints are fully
+ * configurable, with more generic names like "ep-a". (remember that for
+ * USB, "in" means "towards the USB master".)
+ *
+ * returns zero, or a negative error code.
+ */
+int usb_ep_enable(struct usb_ep *ep)
+{
+ int ret;
+
+ if (ep->enabled)
+ return 0;
+
+ ret = e
Re: [PATCH v9 08/14] usb: otg: add OTG/dual-role core
On Wed, Jun 08, 2016 at 12:03:40PM +0300, Roger Quadros wrote:
> +int usb_otg_unregister(struct device *dev)
> +{
> + struct usb_otg *otg;
> +
> + mutex_lock(&otg_list_mutex);
> + otg = usb_otg_get_data(dev);
> + if (!otg) {
> + dev_err(dev, "otg: %s: device not in otg list\n",
> + __func__);
> + mutex_unlock(&otg_list_mutex);
> + return -EINVAL;
> + }
> +
> + /* prevent unregister till both host & gadget have unregistered */
> + if (otg->host || otg->gadget) {
> + dev_err(dev, "otg: %s: host/gadget still registered\n",
> + __func__);
You need to call mutex_unlock here
> +
> +int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
> +{
What this API is for? Why need it in this version?
> + struct usb_otg *otg;
> + struct device *gadget_dev = &gadget->dev;
> + struct device *otg_dev = gadget->otg_dev;
> +
> + if (!otg_dev)
> + return -EINVAL;
> +
> + mutex_lock(&otg_list_mutex);
> + otg = usb_otg_get_data(otg_dev);
> + mutex_unlock(&otg_list_mutex);
> + if (!otg) {
> + dev_err(gadget_dev,
> + "otg: gadget %s wasn't registered with otg\n",
> + dev_name(&gadget->dev));
> + return -EINVAL;
> + }
> +
> + mutex_lock(&otg->fsm.lock);
> + if (otg->gadget != gadget) {
> + mutex_unlock(&otg->fsm.lock);
> + dev_err(otg_dev, "otg: gadget %s wasn't registered with otg\n",
> + dev_name(&gadget->dev));
> + return -EINVAL;
> + }
> +
> + /* Start/stop FSM & gadget */
> + otg->gadget_ready = ready;
> + if (ready)
> + usb_otg_start_fsm(otg);
> + else
> + usb_otg_stop_fsm(otg);
> +
> + dev_dbg(otg_dev, "otg: gadget %s %sready\n", dev_name(&gadget->dev),
> + ready ? "" : "not ");
> +
> + mutex_unlock(&otg->fsm.lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_otg_gadget_ready);
> +
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -10,10 +10,70 @@
> #define __LINUX_USB_OTG_H
>
> #include
> -#include
> -#include
> +#include
> +#include
> +#include
The above two timer header files are not needed.
Peter
> +#include
> #include
> +#include
> +#include
> +#include
> +
> +/**
> + * struct otg_hcd - host controller state and interface
> + *
> + * @hcd: host controller
> + * @irqnum: irq number
> + * @irqflags: irq flags
> + * @ops: otg to host controller interface
> + * @ops: otg to host controller interface
> + * @otg_dev: otg controller device
> + */
> +struct otg_hcd {
> + struct usb_hcd *hcd;
> + unsigned int irqnum;
> + unsigned long irqflags;
> + struct otg_hcd_ops *ops;
> + struct device *otg_dev;
> +};
> +
> +/**
> + * struct usb_otg_caps - describes the otg capabilities of the device
> + * @otg_rev: The OTG revision number the device is compliant with, it's
> + * in binary-coded decimal (i.e. 2.0 is 0200H).
> + * @hnp_support: Indicates if the device supports HNP.
> + * @srp_support: Indicates if the device supports SRP.
> + * @adp_support: Indicates if the device supports ADP.
> + */
> +struct usb_otg_caps {
> + u16 otg_rev;
> + bool hnp_support;
> + bool srp_support;
> + bool adp_support;
> +};
>
> +/**
> + * struct usb_otg - usb otg controller state
> + *
> + * @default_a: Indicates we are an A device. i.e. Host.
> + * @phy: USB phy interface
> + * @usb_phy: old usb_phy interface
> + * @host: host controller bus
> + * @gadget: gadget device
> + * @state: current otg state
> + * @dev: otg controller device
> + * @caps: otg capabilities revision, hnp, srp, etc
> + * @fsm: otg finite state machine
> + * @hcd_ops: host controller interface
> + * --- internal use only ---
> + * @primary_hcd: primary host state and interface
> + * @shared_hcd: shared host state and interface
> + * @gadget_ops: gadget controller interface
> + * @list: list of otg controllers
> + * @work: otg state machine work
> + * @wq: otg state machine work queue
> + * @flags: to track if host/gadget is running
> + */
> struct usb_otg {
> u8 default_a;
>
> @@ -24,9 +84,25 @@ struct usb_otg {
> struct usb_gadget *gadget;
>
> enum usb_otg_state state;
> + struct device *dev;
> + struct usb_otg_caps caps;
> struct otg_fsm fsm;
> struct otg_hcd_ops *hcd_ops;
>
> + /* internal use only */
> + struct otg_hcd primary_hcd;
> + struct otg_hcd shared_hcd;
> + struct otg_gadget_ops *gadget_ops;
> + bool gadget_ready;
> + struct list_head list;
> + struct work_struct work;
> + struct workqueue_struct *wq;
> + u32 flags;
> +#define OTG_FLAG_GADGET_RUNNING (1 << 0)
> +#define OTG_FLAG_HOST_RUNNING (1 << 1)
> + /* use otg->fsm.lock for serializing access */
> +
> +/*- deprecated
Re: [PATCH 23/62] usb: dwc3: implement runtime PM
Hi, Roger Quadros writes: >> Roger Quadros writes: Roger Quadros writes: > On 30/05/16 14:35, Felipe Balbi wrote: >> this patch implements the most basic pm_runtime >> support for dwc3. Whenever USB cable is dettached, >> then we will allow core to runtime_suspend. >> >> Runtime suspending will involve completely tearing >> down event buffers and require a full soft-reset of >> the IP. >> >> Note that a further optimization could be >> implemented once we decide to support hibernation, >> which is to allow runtime_suspend with cable >> connected when bus is in U3. That's subject to a >> separate patch, however. >> >> Tested-by: Baolin Wang >> Signed-off-by: Felipe Balbi > > We've discussed this offline but for the record, this patch > breaks DWC3 on OMAP platforms. At least on dra7-evm I could see > both USB host and gadget breaking. > > I will try to implement the runtime resume hooks for dwc3-omap > and let you know if we can make it work. cool, thanks :-) >>> >>> On testing branch commit [1], dwc3 breaks after >>> system suspend/resume on dra7-evm. >>> >>> [1] - cd45299a0f3a41f25729a523aecc0f3e6ad14d43 >> >> got some logs there, somewhere? :-) >> > If I'm on commit 9c34239b09894c76fe2f71f1ec8c443a2ae8bf2a okay. So ENDTRANSFER command failed. Can you capture trace output? Also, is reverting that enough to keep things working on your side? -- balbi signature.asc Description: PGP signature
Re: Nokia N900: musb is in wrong state after boot
* Pali Rohár [160607 05:53]: > > Tony, what do you think about that patch? > > > > Tony, PING Yeah I don't know, AFAIK we don't have a generic way to force MUSB to change mode without ID pin. If you have figured something generic for that which does not actually tinker with the PHY registers directly, that should be the generic musb_set_mode() that we've been wondering about for years. Maybe split the patches to the mode change and then the sysfs entry fixes and repost to linux-usb and linux-omap and make sure Bin as the new musb maintainer is included :) The custom sysfs flags we should maintain support for naturally. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 30/62] usb: dwc3: gadget: loop while (timeout)
Paul Zimmerman writes:
> Felipe Balbi writes:
>
>> instead of having infinite loop and always checking
>> timeout value as a break condition, we can just
>> decrement timeout inside while's condition.
>>
>> Signed-off-by: Felipe Balbi
>> ---
>> drivers/usb/dwc3/gadget.c | 18 ++
>> 1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index efb758e5c6c7..8a16d8b2da8a 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -327,19 +327,13 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep,
>> unsigned cmd,
>>
>> break;
>> }
>> +} while (timeout--);
>
> When 'timeout' reaches 0, the post-decrement means the value will be -1 when
> the loop exits.
>
>>
>> -/*
>> - * We can't sleep here, because it is also called from
>> - * interrupt context.
>> - */
>> -timeout--;
>> -if (!timeout) {
>> -dwc3_trace(trace_dwc3_gadget,
>> -"Command Timed Out");
>> -ret = -ETIMEDOUT;
>> -break;
>> -}
>> -} while (1);
>> +if (timeout == 0) {
>
> But here you are testing against 0 instead of -1.
good point :-) Fixed:
8<--
From 98ee80f8821471a0dba38f2207fbb96c1b6c538a Mon Sep 17 00:00:00 2001
From: Felipe Balbi
Date: Mon, 23 May 2016 13:53:34 +0300
Subject: [PATCH] usb: dwc3: gadget: loop while (timeout)
instead of having infinite loop and always checking
timeout value as a break condition, we can just
decrement timeout inside while's condition.
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/gadget.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index efb758e5c6c7..ca8269646b72 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -327,19 +327,13 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned
cmd,
break;
}
+ } while (--timeout);
- /*
-* We can't sleep here, because it is also called from
-* interrupt context.
-*/
- timeout--;
- if (!timeout) {
- dwc3_trace(trace_dwc3_gadget,
- "Command Timed Out");
- ret = -ETIMEDOUT;
- break;
- }
- } while (1);
+ if (timeout == 0) {
+ dwc3_trace(trace_dwc3_gadget,
+ "Command Timed Out");
+ ret = -ETIMEDOUT;
+ }
if (unlikely(susphy)) {
reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
--
2.8.3
--
balbi
signature.asc
Description: PGP signature
Re: [balbi-usb:testing/next 64/67] phy-generic.c:undefined reference to `usb_gadget_vbus_connect'
Hi,
Peter Chen writes:
>> >> > Hi Felipe,
>> >> >
>> >> > It causes any drivers which use symbols from gadget.h needs to build
>> >> > USB_GADGET even it only wants to be host function. Any significant
>> >> > benefits after moving it to udc-core.c?
>> >>
>> >> why is a host-only function calling into the gadget API?
>> >>
>> >
>> > Well, Just like this case, the chipidea driver can be configured host
>> > mode only, but it still uses generic phy.
>>
>> then we should allow for PHYs to not depend on the Gadget API, right?
>
> But you know it is not easy. The difficulty for gadget to do that is
> vbus and pull dp, these two may be used by other subsystems, eg, vbus
> may be used at PHY driver, dual-role switch driver, etc. The pull dp
> may be used at USB charger driver, the secondary charger detection
> needs to pull up dp for some charger designs.
that's fine. And if they really need that, they depend on the Gadget
API. That _is_ something the gadget API defines afterall.
> See current kernel doc, it also says that.
>
> /**
>* usb_gadget_vbus_connect - Notify controller that VBUS is powered
>* @gadget:The device which now has VBUS power.
>* Context: can sleep
>*
>* This call is used by a driver for an external transceiver (or GPIO)
>* that detects a VBUS power session starting. Common responses include
>* resuming the controller, activating the D+ (or D-) pullup to let the
>* host detect that a USB device is attached, and starting to draw power
>* (8mA or possibly more, especially after SET_CONFIGURATION).
>*
>* Returns zero on success, else negative errno.
>*/
> static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
> {
> if (!gadget->ops->vbus_session)
> return -EOPNOTSUPP;
> return gadget->ops->vbus_session(gadget, 1);
> }
>
> See current situations:
>
> b29397@linux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn
> "usb_gadget_vbus_connect"
> ./drivers/phy/phy-twl4030-usb.c:559: * REVISIT
> usb_gadget_vbus_connect(...) as needed, ditto
> ./drivers/usb/phy/phy-gpio-vbus-usb.c:126:
> usb_gadget_vbus_connect(gpio_vbus->phy.otg->gadget);
> ./drivers/usb/phy/phy-tahvo.c:89:
> usb_gadget_vbus_connect(tu->phy.otg->gadget);
> ./drivers/usb/phy/phy-isp1301-omap.c:929:
> usb_gadget_vbus_connect(isp->phy.otg->gadget);
> ./drivers/usb/phy/phy-isp1301-omap.c:1466:
> usb_gadget_vbus_connect(otg->gadget);
> ./drivers/usb/phy/phy-generic.c:122:
> usb_gadget_vbus_connect(otg->gadget);
> ./drivers/usb/phy/phy-generic.c:190: usb_gadget_vbus_connect(gadget);
> ./drivers/usb/phy/phy-msm-usb.c:783:
> usb_gadget_vbus_connect(phy->otg->gadget);
> ./drivers/usb/phy/phy-mv-usb.c:234:
> usb_gadget_vbus_connect(otg->gadget);
> ./drivers/usb/chipidea/otg.c:133:
> usb_gadget_vbus_connect(&ci->gadget);
> ./drivers/usb/chipidea/otg.c:138:
> usb_gadget_vbus_connect(&ci->gadget);
> ./drivers/usb/chipidea/udc.c:1964:
> usb_gadget_vbus_connect(&ci->gadget);
> ./drivers/usb/chipidea/otg_fsm.c:567:
> usb_gadget_vbus_connect(&ci->gadget);
> b29397@linux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn
> "usb_gadget_vbus_disconnect"
> ./drivers/usb/phy/phy-gpio-vbus-usb.c:145:
> usb_gadget_vbus_disconnect(gpio_vbus->phy.otg->gadget);
> ./drivers/usb/phy/phy-gpio-vbus-usb.c:200:
> usb_gadget_vbus_disconnect(otg->gadget);
> ./drivers/usb/phy/phy-tahvo.c:108:
> usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
> ./drivers/usb/phy/phy-tahvo.c:165:
> usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
> ./drivers/usb/phy/phy-tahvo.c:175:
> usb_gadget_vbus_disconnect(tu->phy.otg->gadget);
> ./drivers/usb/phy/phy-isp1301-omap.c:312: return
> usb_gadget_vbus_disconnect(isp->phy.otg->gadget);
> ./drivers/usb/phy/phy-isp1301-omap.c:989:
> usb_gadget_vbus_disconnect(otg->gadget);
> ./drivers/usb/phy/phy-isp1301-omap.c:1342:
> usb_gadget_vbus_disconnect(otg->gadget);
> ./drivers/usb/phy/phy-generic.c:133:
> usb_gadget_vbus_disconnect(otg->gadget);
> ./drivers/usb/phy/phy-fsl-usb.c:622:
> usb_gadget_vbus_disconnect(otg->gadget);
> ./drivers/usb/phy/phy-msm-usb.c:786:
> usb_gadget_vbus_disconnect(phy->otg->gadget);
> ./drivers/usb/phy/phy-mv-usb.c:236:
> usb_gadget_vbus_disconnect(otg->gadget);
> ./drivers/usb/chipidea/otg.c:140:
> usb_gadget_vbus_disconnect(&ci->gadget);
> ./drivers/usb/chipidea/otg_fsm.c:569:
> usb_gadget_vbus_disconnect(&ci->gadget);
> b29397@linux2:~/work/projects/usb$ find . -name *.c | xargs grep -rn
> "usb_gadget_connect"
> ./drivers/power/isp1704_charger.c:272:
Re: [PATCH 23/62] usb: dwc3: implement runtime PM
On 07/06/16 12:42, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >>> Roger Quadros writes: On 30/05/16 14:35, Felipe Balbi wrote: > this patch implements the most basic pm_runtime > support for dwc3. Whenever USB cable is dettached, > then we will allow core to runtime_suspend. > > Runtime suspending will involve completely tearing > down event buffers and require a full soft-reset of > the IP. > > Note that a further optimization could be > implemented once we decide to support hibernation, > which is to allow runtime_suspend with cable > connected when bus is in U3. That's subject to a > separate patch, however. > > Tested-by: Baolin Wang > Signed-off-by: Felipe Balbi We've discussed this offline but for the record, this patch breaks DWC3 on OMAP platforms. At least on dra7-evm I could see both USB host and gadget breaking. I will try to implement the runtime resume hooks for dwc3-omap and let you know if we can make it work. >>> >>> cool, thanks :-) >>> >> >> On testing branch commit [1], dwc3 breaks after >> system suspend/resume on dra7-evm. >> >> [1] - cd45299a0f3a41f25729a523aecc0f3e6ad14d43 > > got some logs there, somewhere? :-) > If I'm on commit 9c34239b09894c76fe2f71f1ec8c443a2ae8bf2a I see root@rockdesk:~# modprobe g_zero [ 38.387921] zero gadget: Gadget Zero, version: Cinco de Mayo 2008 [ 38.394318] zero gadget: zero ready root@rockdesk:~# [ 41.931890] zero gadget: high-speed config #3: source/sink root@rockdesk:~# ./suspend.sh [ 44.782122] PM: Syncing filesystems ... done. [ 44.815867] Freezing user space processes ... (elapsed 0.003 seconds) done. [ 44.827134] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 44.871147] PM: suspend of devices complete after 29.653 msecs [ 44.882335] PM: late suspend of devices complete after 5.026 msecs [ 44.901421] omap_hwmod: gpio7: _wait_target_disable failed [ 44.914115] omap_hwmod: gpio6: _wait_target_disable failed [ 44.920610] PM: noirq suspend of devices complete after 31.754 msecs [ 44.927376] Disabling non-boot CPUs ... [ 44.937050] CPU1: shutdown [ 44.964442] Powerdomain (l4per_pwrdm) didn't enter target state 1 [ 44.964442] Powerdomain (l3init_pwrdm) didn't enter target state 1 [ 44.964442] Could not enter target state in pm_suspend [ 44.964442] A possible cause could be an old bootloader - try u-boot >= v2012.07 [ 44.964652] Enabling non-boot CPUs ... [ 45.020168] CPU1: smp_ops.cpu_die() returned, trying to resuscitate [ 45.020948] CPU1 is up [ 45.034255] PM: noirq resume of devices complete after 4.226 msecs [ 45.044929] PM: early resume of devices complete after 3.385 msecs [ 45.054156] net eth0: initializing cpsw version 1.15 (0) [ 45.140277] Generic PHY 48485000.mdio:02: attached PHY driver [Generic PHY] (mii_bus:phy_addr=48485000.mdio:02, irq=-1) [ 45.161258] usb usb1: root hub lost power or was reset [ 45.166649] usb usb2: root hub lost power or was reset [ 45.276755] PM: resume of devices complete after 225.305 msecs [ 45.287862] Restarting tasks ... done. root@rockdesk:~# root@rockdesk:~# [ 45.369525] ata1: SATA link down (SStatus 0 SControl 300) root@rockdesk:~# [ 48.143304] cpsw 48484000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx [ 49.277235] [ cut here ] [ 49.282101] WARNING: CPU: 1 PID: 2049 at drivers/usb/dwc3/gadget.c:2274 dwc3_stop_active_transfer.constprop.1+0x94/0xb0 [dwc3] [ 49.294054] Modules linked in: usb_f_ss_lb g_zero libcomposite xhci_plat_hcd xhci_hcd usbcore evdev dwc3 udc_core usb_common m25p80 spi_nor snd_soc_davinci_mcasp snd_soc_edma snd_soc_omap snd_soc_simple_carde [ 49.336699] CPU: 1 PID: 2049 Comm: irq/451-dwc3 Tainted: GW 4.7.0-rc1-00022-g9c34239 #858 [ 49.346553] Hardware name: Generic DRA74X (Flattened Device Tree) [ 49.352961] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 49.361093] [] (show_stack) from [] (dump_stack+0xac/0xe0) [ 49.368681] [] (dump_stack) from [] (__warn+0xd8/0x104) [ 49.375986] [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [ 49.383944] [] (warn_slowpath_null) from [] (dwc3_stop_active_transfer.constprop.1+0x94/0xb0 [dwc3]) [ 49.395412] [] (dwc3_stop_active_transfer.constprop.1 [dwc3]) from [] (dwc3_remove_requests+0x20/0xac [dwc3]) [ 49.407664] [] (dwc3_remove_requests [dwc3]) from [] (__dwc3_gadget_ep_disable+0x30/0x118 [dwc3]) [ 49.418820] [] (__dwc3_gadget_ep_disable [dwc3]) from [] (dwc3_gadget_ep_disable+0x3c/0xc8 [dwc3]) [ 49.430068] [] (dwc3_gadget_ep_disable [dwc3]) from [] (disable_ep+0x2c/0x68 [usb_f_ss_lb]) [ 49.440665] [] (disable_ep [usb_f_ss_lb]) from [] (disable_endpoints+0x18/0x50 [usb_f_ss_lb]) [ 49.451445] [] (disable_endpoints [usb_f_ss_lb]) from [] (disable_source_sink+0x2c/0x34 [usb_f_ss_lb]) [ 49.463044] [] (disable_source_sink [usb_f_ss_lb])
[PATCH v9 01/14] usb: hcd: Initialize hcd->flags to 0
When using the OTG/drd library we can call hcd_add/remove consecutively without calling usb_put_hcd/usb_create_hcd in between so hcd->flags can be stale. If the HC dies due to whatever reason then without this patch we get the below error on next hcd_add. [ 91.494257] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up [ 91.502068] hub 3-0:1.0: state 0 ports 1 chg evt [ 91.510240] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller [ 91.516940] xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 4 [ 91.529745] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 91.540637] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003 [ 91.757865] irq 254: nobody cared (try booting with the "irqpoll" option) [ 91.757880] CPU: 0 PID: 68 Comm: kworker/u2:2 Not tainted 4.1.4-00828-g1f0ed8c-dirty #44 [ 91.757885] Hardware name: Generic AM43 (Flattened Device Tree) [ 91.757914] Workqueue: usb_otg usb_otg_work [ 91.757921] Backtrace: [ 91.757954] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [ 91.757972] r6:c089d4a4 r5: r4: r3:ee44 [ 91.757991] [] (show_stack) from [] (dump_stack+0x84/0xd0) [ 91.758008] [] (dump_stack) from [] (__report_bad_irq+0x28/0xc8) [ 91.758024] r7: r6:00fe r5: r4:ee514c40 [ 91.758037] [] (__report_bad_irq) from [] (note_interrupt+0x24c/0x2ac) [ 91.758052] r6:00fe r5: r4:ee514c40 r3: [ 91.758065] [] (note_interrupt) from [] (handle_irq_event_percpu+0xb0/0x158) [ 91.758085] r10:ee514c40 r9:c08ce49a r8:00fe r7: r6: r5: [ 91.758094] r4: r3: [ 91.758105] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x44/0x64) [ 91.758126] r10:0001 r9:ee441ab0 r8:ee441bb8 r7:c0858b4c r6:ed174280 r5:ee514ca0 [ 91.758132] r4:ee514c40 [ 91.758144] [] (handle_irq_event) from [] (handle_fasteoi_irq+0x100/0x1bc) [ 91.758159] r6:c085dba0 r5:ee514ca0 r4:ee514c40 r3: [ 91.758171] [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x28/0x38) [ 91.758186] r7:c0853d40 r6:c0858b4c r5:00fe r4:00fe [ 91.758197] [] (generic_handle_irq) from [] (__handle_domain_irq+0x98/0x12c) [ 91.758207] r4:c0853d40 r3:0100 [ 91.758219] [] (__handle_domain_irq) from [] (gic_handle_irq+0x28/0x68) [ 91.758239] r10:0001 r9:ee441bb8 r8:fa240100 r7:c0858d70 r6:ee441ab0 r5:00b8 [ 91.758245] r4:fa24010c [ 91.758264] [] (gic_handle_irq) from [] (__irq_svc+0x40/0x74) [ 91.758271] Exception stack(0xee441ab0 to 0xee441af8) [ 91.758280] 1aa0: c08d2980 ee441ac0 [ 91.758292] 1ac0: 0008 0089 c0858b4c c0858080 ee441bb8 0001 ee441b3c [ 91.758301] 1ae0: 0101 ee441af8 c02fc418 c0046a1c 2113 [ 91.758321] r8: r7:ee441ae4 r6: r5:2113 r4:c0046a1c r3:c02fc418 [ 91.758347] [] (__do_softirq) from [] (irq_exit+0xb8/0x104) [ 91.758367] r10:0001 r9:ee441bb8 r8: r7:c0853d40 r6:c0858b4c r5:0089 [ 91.758373] r4: [ 91.758386] [] (irq_exit) from [] (__handle_domain_irq+0xa0/0x12c) [ 91.758395] r4: r3:0100 [ 91.758406] [] (__handle_domain_irq) from [] (gic_handle_irq+0x28/0x68) [ 91.758426] r10:c08e3510 r9:2013 r8:fa240100 r7:c0858d70 r6:ee441bb8 r5:0039 [ 91.758433] r4:fa24010c [ 91.758445] [] (gic_handle_irq) from [] (__irq_svc+0x40/0x74) [ 91.758450] Exception stack(0xee441bb8 to 0xee441c00) [ 91.758457] 1ba0: 0001 [ 91.758468] 1bc0: ee44 c08e2524 004d 0274 2013 [ 91.758479] 1be0: c08e3510 ee441c4c ee441b60 ee441c00 c03acfec c0080d4c 6013 [ 91.758499] r8: r7:ee441bec r6: r5:6013 r4:c0080d4c r3:c03acfec [ 91.758524] [] (console_unlock) from [] (vprintk_emit+0x20c/0x500) [ 91.758544] r10:ee441cc0 r9:c08d3550 r8:c08e3ea0 r7: r6:0001 r5:003d [ 91.758551] r4:c08d3550 [ 91.758573] [] (vprintk_emit) from [] (dev_vprintk_emit+0x104/0x1ac) [ 91.758593] r10:ee441d8c r9:000e r8:c07951e0 r7:0006 r6:ee441cc0 r5:000d [ 91.758599] r4:ee731068 [ 91.758612] [] (dev_vprintk_emit) from [] (dev_printk_emit+0x28/0x30) [ 91.758632] r10:0001 r9:ee5f8410 r8:ee731000 r7:ed429000 r6:0006 r5:ee441dc0 [ 91.758638] r4:ee731068 [ 91.758651] [] (dev_printk_emit) from [] (__dev_printk+0x50/0x70) [ 91.758660] r3:bf2268cc r2:c07951e0 [ 91.758673] [] (__dev_printk) from [] (_dev_info+0x3c/0x48) [ 91.758686] r6: r5:ee731068 r4:ee731000 [ 91.758790] [] (_dev_info) from [] (usb_new_device+0x11c/0x518 [usbcore]) [ 91.758804] r3:0003 r2:1d6b r1:bf225bc4 [ 91.758881] [] (usb_new_device [usbcore]) from [] (usb_otg_add_hcd+0x514/0x7f8 [usbcore]) [ 91.758903] r10:0001 r9
[PATCH v9 00/14] USB OTG/dual-role framework
Hi,
This series centralizes OTG/Dual-role functionality in the kernel.
As of now I've got Dual-role functionality working pretty reliably on
dra7-evm and am437x-gp-evm.
DWC3 controller and TI platform related patches will be sent separately.
Series is based on v4.7-rc1.
Why?:
Currently there is no central location where OTG/dual-role functionality is
implemented in the Linux USB stack and every USB controller driver is
doing their own thing for OTG/dual-role. We can benefit from code-reuse
and simplicity by adding the OTG/dual-role core driver.
Newer OTG cores support standard host interface (e.g. xHCI) so
host and gadget functionality are no longer closely knit like older
cores. There needs to be a way to co-ordinate the operation of the
host and gadget controllers in dual-role mode. i.e. to stop and start them
from a central location. This central location should be the
USB OTG/dual-role core.
Host and gadget controllers might be sharing resources and can't
be always running. One has to be stopped for the other to run.
This couldn't be done till now but can be done from the OTG core.
What?:
-
The OTG/dual-role core consists of a set of APIs that allow
registration of OTG controller device and OTG capable host and gadget
controllers.
- The OTG controller driver can provide the OTG capabilities and the
Finite State Machine work function via 'struct usb_otg_config'
at the time of registration i.e. usb_otg_register();
struct usb_otg *usb_otg_register(struct device *dev,
struct usb_otg_config *config);
int usb_otg_unregister(struct device *dev);
/**
* struct usb_otg_config - otg controller configuration
* @caps: otg capabilities of the controller
* @ops: otg fsm operations
* @otg_work: optional custom otg state machine work function
*/
struct usb_otg_config {
struct usb_otg_caps *otg_caps;
struct otg_fsm_ops *fsm_ops;
void (*otg_work)(struct work_struct *work);
};
The dual-role state machine is built-into the OTG core so nothing
special needs to be provided if only dual-role functionality is desired.
The low level OTG controller driver ops are povided via
'struct otg_fsm_ops *fsm_ops' in the 'struct usb_otg_config'.
After registration, the OTG core waits for host, gadget controller
and the gadget function driver to be registered. Once all resources are
available it instantiates the Finite State Machine (FSM).
The host/gadget controllers are started/stopped according to the FSM.
- Host and gadget controllers that are a part of OTG/dual-role port must
use the OTG core provided APIs to add/remove the host/gadget.
i.e. hosts must use usb_otg_add_hcd() usb_otg_remove_hcd(),,
gadgets must use usb_otg_add_gadget_udc() usb_del_gadget_udc().
This ensures that the host and gadget controllers are not started till
the state machine is ready and the right bus conditions are met.
It also allows the host and gadget controllers to provide the OTG
controller device to link them together. For Device tree boots
the related OTG controller is automatically picked up via the
'otg-controller' property in the Host/Gadget controller nodes.
int usb_otg_add_hcd(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags,
struct device *otg_dev);
void usb_otg_remove_hcd(struct usb_hcd *hcd);
int usb_otg_add_gadget_udc(struct device *parent,
struct usb_gadget *gadget,
struct device *otg_dev);
usb_del_gadget_udc() must be used for removal.
- During the lifetime of the FSM, the OTG controller driver can provide
inputs event changes using usb_otg_sync_inputs(). The OTG core will
then schedule the FSM work function (or internal dual-role state machine)
to update the FSM state. The FSM then calls the OTG controller
operations (fsm_ops) as necessary.
void usb_otg_sync_inputs(struct usb_otg *otg);
- The following 2 functions are provided as helpers for use by the
OTG controller driver to start/stop the host/gadget controllers.
int usb_otg_start_host(struct usb_otg *otg, int on);
int usb_otg_start_gadget(struct usb_otg *otg, int on);
- The following function is provided for use by the USB host stack
to sync OTG related events to the OTG state machine.
e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
int usb_otg_kick_fsm(struct device *otg_device);
- The following function is provided for use by the USB gadget stack
to notify OTG/DRD about gadget function driver being ready/not-ready
int usb_otg_gadget_ready(struct usb_gadget *gadget, bool ready)
Changelog:
-
v9:
- In the DT bindings, clearly indicate which properties are for OTG controller
and which ones are for host/device controllers
- Removed host/gadget wait list from otg
[PATCH v9 03/14] usb: hcd.h: Add OTG to HCD interface
The OTG core will use struct otg_hcd_ops to interface
with the HCD controller.
The main purpose of this interface is to avoid directly
calling HCD APIs from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB is m.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
include/linux/usb/hcd.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 66fc137..7914bed 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -400,6 +400,30 @@ struct hc_driver {
};
+/**
+ * struct otg_hcd_ops - Interface between OTG core and HCD
+ *
+ * Provided by the HCD core to allow the OTG core to interface with the HCD
+ *
+ * @add: function to add the HCD
+ * @remove: function to remove the HCD
+ * @usb_bus_start_enum: function to immediately start bus enumeration
+ * @usb_control_msg: function to build and send of a control urb
+ * @usb_hub_find_child: function to get pointer to the child device
+ */
+struct otg_hcd_ops {
+ int (*add)(struct usb_hcd *hcd,
+ unsigned int irqnum, unsigned long irqflags);
+ void (*remove)(struct usb_hcd *hcd);
+ int (*usb_bus_start_enum)(struct usb_bus *bus, unsigned int port_num);
+ int (*usb_control_msg)(struct usb_device *dev, unsigned int pipe,
+ __u8 request, __u8 requesttype, __u16 value,
+ __u16 index, void *data, __u16 size,
+ int timeout);
+ struct usb_device * (*usb_hub_find_child)(struct usb_device *hdev,
+ int port1);
+};
+
static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
{
return hcd->driver->flags & HCD_BH;
--
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
[PATCH v9 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops
This is to prevent missing symbol build error if OTG is
enabled (built-in) and HCD core (CONFIG_USB) is module.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/chipidea/otg_fsm.c | 7 +++
drivers/usb/common/usb-otg-fsm.c | 15 +++
drivers/usb/phy/phy-fsl-usb.c| 7 +++
include/linux/usb/otg.h | 2 ++
4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 1c0c750..2d8d659 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -582,6 +582,12 @@ static struct otg_fsm_ops ci_otg_ops = {
.start_gadget = ci_otg_start_gadget,
};
+static struct otg_hcd_ops ci_hcd_ops = {
+ .usb_bus_start_enum = usb_bus_start_enum,
+ .usb_control_msg = usb_control_msg,
+ .usb_hub_find_child = usb_hub_find_child,
+};
+
int ci_otg_fsm_work(struct ci_hdrc *ci)
{
/*
@@ -804,6 +810,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
ci->otg.fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
ci->otg.state = OTG_STATE_UNDEFINED;
ci->otg.fsm.ops = &ci_otg_ops;
+ ci->otg.hcd_ops = &ci_hcd_ops;
ci->gadget.hnp_polling_support = 1;
ci->otg.fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
if (!ci->otg.fsm.host_req_flag)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 610b5bd6..482d4c9 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -141,11 +141,16 @@ static void otg_hnp_polling_work(struct work_struct *work)
enum usb_otg_state state = otg->state;
u8 flag;
int retval;
+ struct otg_hcd_ops *hcd_ops = otg->hcd_ops;
if (state != OTG_STATE_A_HOST && state != OTG_STATE_B_HOST)
return;
- udev = usb_hub_find_child(otg->host->root_hub, 1);
+ if (!hcd_ops || !hcd_ops->usb_control_msg ||
+ !hcd_ops->usb_hub_find_child)
+ return;
+
+ udev = hcd_ops->usb_hub_find_child(otg->host->root_hub, 1);
if (!udev) {
dev_err(otg->host->controller,
"no usb dev connected, can't start HNP polling\n");
@@ -154,7 +159,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
*fsm->host_req_flag = 0;
/* Get host request flag from connected USB device */
- retval = usb_control_msg(udev,
+ retval = hcd_ops->usb_control_msg(udev,
usb_rcvctrlpipe(udev, 0),
USB_REQ_GET_STATUS,
USB_DIR_IN | USB_RECIP_DEVICE,
@@ -183,7 +188,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
if (state == OTG_STATE_A_HOST) {
/* Set b_hnp_enable */
if (!otg->host->b_hnp_enable) {
- retval = usb_control_msg(udev,
+ retval = hcd_ops->usb_control_msg(udev,
usb_sndctrlpipe(udev, 0),
USB_REQ_SET_FEATURE, 0,
USB_DEVICE_B_HNP_ENABLE,
@@ -262,7 +267,9 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
otg_loc_conn(otg, 0);
otg_loc_sof(otg, 1);
otg_set_protocol(fsm, PROTO_HOST);
- usb_bus_start_enum(otg->host, otg->host->otg_port);
+ if (otg->hcd_ops && otg->hcd_ops->usb_bus_start_enum)
+ otg->hcd_ops->usb_bus_start_enum(otg->host,
+otg->host->otg_port);
otg_start_hnp_polling(fsm);
break;
case OTG_STATE_A_IDLE:
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 3ffa4c6..011d36e 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -792,6 +792,12 @@ static struct otg_fsm_ops fsl_otg_ops = {
.start_gadget = fsl_otg_start_gadget,
};
+static struct otg_hcd_ops fsl_hcd_ops = {
+ .usb_bus_start_enum = usb_bus_start_enum,
+ .usb_control_msg = usb_control_msg,
+ .usb_hub_find_child = usb_hub_find_child,
+};
+
/* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
static int fsl_otg_conf(struct platform_device *pdev)
{
@@ -820,6 +826,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
/* Set OTG state machine operations */
fsl_otg_tc->otg.fsm.ops = &fsl_otg_ops;
+ fsl_otg_tc->otg.hcd_ops = &fsl_hcd_ops;
/* initialize the otg structure */
fsl_otg_tc->phy.label = DRIVER_DESC;
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index e8a14dc..85b8fb5 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
struct usb_otg {
u8
[PATCH v9 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG
Let's use CONFIG_USB_OTG as a single config option to enable
USB OTG and the OTG FSM. This makes things a lot less confusing.
Update all users of CONFIG_USB_OTG_FSM to CONFIG_USB_OTG.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
Documentation/usb/chipidea.txt | 2 +-
drivers/usb/chipidea/Makefile | 2 +-
drivers/usb/chipidea/ci.h | 2 +-
drivers/usb/chipidea/otg_fsm.h | 2 +-
drivers/usb/common/Makefile| 3 ++-
drivers/usb/core/Kconfig | 8
drivers/usb/phy/Kconfig| 2 +-
7 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index edf7cdf..580d157 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -5,7 +5,7 @@ with 2 Freescale i.MX6Q sabre SD boards.
1.1 How to enable OTG FSM
---
-1.1.1 Select CONFIG_USB_OTG_FSM in menuconfig, rebuild kernel
+1.1.1 Select CONFIG_USB_OTG in menuconfig, rebuild kernel
Image and modules. If you want to check some internal
variables for otg fsm, mount debugfs, there are 2 files
which can show otg fsm variables and some controller registers value:
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 518e445..45aa24d 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc.o
ci_hdrc-y := core.o otg.o debug.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)+= host.o
-ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o
+ci_hdrc-$(CONFIG_USB_OTG) += otg_fsm.o
# Glue/Bridge layers go here
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index c523975..1a32b8c 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -406,7 +406,7 @@ static inline u32 hw_test_and_write(struct ci_hdrc *ci,
enum ci_hw_regs reg,
*/
static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
{
-#ifdef CONFIG_USB_OTG_FSM
+#ifdef CONFIG_USB_OTG
struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
index 6366fe3..2d451bb 100644
--- a/drivers/usb/chipidea/otg_fsm.h
+++ b/drivers/usb/chipidea/otg_fsm.h
@@ -64,7 +64,7 @@
#define TB_AIDL_BDIS (20) /* 4ms ~ 150ms, section 5.2.1 */
-#if IS_ENABLED(CONFIG_USB_OTG_FSM)
+#if IS_ENABLED(CONFIG_USB_OTG)
int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
int ci_otg_fsm_work(struct ci_hdrc *ci);
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..f8f2c88 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
usb-common-$(CONFIG_USB_LED_TRIG) += led.o
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
+usbotg-y := usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG) += usbotg.o
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index dd28010..ae228d0 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -75,14 +75,6 @@ config USB_OTG_BLACKLIST_HUB
and software costs by not supporting external hubs. So
are "Embedded Hosts" that don't offer OTG support.
-config USB_OTG_FSM
- tristate "USB 2.0 OTG FSM implementation"
- depends on USB && USB_OTG
- select USB_PHY
- help
- Implements OTG Finite State Machine as specified in On-The-Go
- and Embedded Host Supplement to the USB Revision 2.0 Specification.
-
config USB_ULPI_BUS
tristate "USB ULPI PHY interface support"
depends on USB_SUPPORT
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index c690474..06794e2 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -20,7 +20,7 @@ config AB8500_USB
config FSL_USB2_OTG
bool "Freescale USB OTG Transceiver Driver"
- depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM
+ depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG && PM
select USB_PHY
help
Enable this to support Freescale USB OTG transceiver.
--
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
[PATCH v9 06/14] usb: gadget.h: Add OTG to gadget interface
The OTG core will use struct otg_gadget_ops to
start/stop the gadget controller.
The main purpose of this interface is to avoid directly
calling usb_gadget_start/stop() from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB_GADGET is m.
Signed-off-by: Roger Quadros
---
include/linux/usb/gadget.h | 16
1 file changed, 16 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 457651b..51e3bde 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1100,6 +1100,22 @@ struct usb_gadget_driver {
};
+/*-*/
+
+/**
+ * struct otg_gadget_ops - Interface between OTG core and gadget
+ *
+ * Provided by the gadget core to allow the OTG core to start/stop the gadget
+ *
+ * @start: function to start the gadget
+ * @stop: function to stop the gadget
+ * @connect_control: function to connect/disconnect from the bus
+ */
+struct otg_gadget_ops {
+ int (*start)(struct usb_gadget *gadget);
+ int (*stop)(struct usb_gadget *gadget);
+ int (*connect_control)(struct usb_gadget *gadget, bool connect);
+};
/*-*/
--
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
[PATCH v9 04/14] usb: otg-fsm: use usb_otg wherever possible
Move otg_fsm into usb_otg and use usb_otg wherever possible
in the usb_otg APIs.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/chipidea/ci.h| 1 -
drivers/usb/chipidea/core.c | 14 +--
drivers/usb/chipidea/debug.c | 2 +-
drivers/usb/chipidea/otg_fsm.c | 169 ++--
drivers/usb/chipidea/udc.c | 17 ++--
drivers/usb/common/usb-otg-fsm.c | 180 ---
drivers/usb/phy/phy-fsl-usb.c| 141 +++---
drivers/usb/phy/phy-fsl-usb.h| 3 +-
include/linux/usb/otg-fsm.h | 132 +++-
include/linux/usb/otg.h | 107 +++
10 files changed, 383 insertions(+), 383 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index cd41455..c523975 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -209,7 +209,6 @@ struct ci_hdrc {
enum ci_rolerole;
boolis_otg;
struct usb_otg otg;
- struct otg_fsm fsm;
struct hrtimer otg_fsm_hrtimer;
ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS];
unsignedenabled_otg_timer_bits;
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..56b6ac6 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1085,8 +1085,8 @@ static int ci_hdrc_remove(struct platform_device *pdev)
/* Prepare wakeup by SRP before suspend */
static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc *ci)
{
- if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
- !hw_read_otgsc(ci, OTGSC_ID)) {
+ if ((ci->otg.state == OTG_STATE_A_IDLE) &&
+ !hw_read_otgsc(ci, OTGSC_ID)) {
hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_PP,
PORTSC_PP);
hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_WKCN,
@@ -1097,13 +1097,13 @@ static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc
*ci)
/* Handle SRP when wakeup by data pulse */
static void ci_otg_fsm_wakeup_by_srp(struct ci_hdrc *ci)
{
- if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
- (ci->fsm.a_bus_drop == 1) && (ci->fsm.a_bus_req == 0)) {
+ if ((ci->otg.state == OTG_STATE_A_IDLE) &&
+ (ci->otg.fsm.a_bus_drop == 1) && (ci->otg.fsm.a_bus_req == 0)) {
if (!hw_read_otgsc(ci, OTGSC_ID)) {
- ci->fsm.a_srp_det = 1;
- ci->fsm.a_bus_drop = 0;
+ ci->otg.fsm.a_srp_det = 1;
+ ci->otg.fsm.a_bus_drop = 0;
} else {
- ci->fsm.id = 1;
+ ci->otg.fsm.id = 1;
}
ci_otg_queue_work(ci);
}
diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index 6d23eed..374cdaa 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -224,7 +224,7 @@ static int ci_otg_show(struct seq_file *s, void *unused)
if (!ci || !ci_otg_is_fsm_mode(ci))
return 0;
- fsm = &ci->fsm;
+ fsm = &ci->otg.fsm;
/* -- State - */
seq_printf(s, "OTG state: %s\n\n",
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index de8e22e..1c0c750 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -40,7 +40,7 @@ get_a_bus_req(struct device *dev, struct device_attribute
*attr, char *buf)
next = buf;
size = PAGE_SIZE;
- t = scnprintf(next, size, "%d\n", ci->fsm.a_bus_req);
+ t = scnprintf(next, size, "%d\n", ci->otg.fsm.a_bus_req);
size -= t;
next += t;
@@ -56,25 +56,25 @@ set_a_bus_req(struct device *dev, struct device_attribute
*attr,
if (count > 2)
return -1;
- mutex_lock(&ci->fsm.lock);
+ mutex_lock(&ci->otg.fsm.lock);
if (buf[0] == '0') {
- ci->fsm.a_bus_req = 0;
+ ci->otg.fsm.a_bus_req = 0;
} else if (buf[0] == '1') {
/* If a_bus_drop is TRUE, a_bus_req can't be set */
- if (ci->fsm.a_bus_drop) {
- mutex_unlock(&ci->fsm.lock);
+ if (ci->otg.fsm.a_bus_drop) {
+ mutex_unlock(&ci->otg.fsm.lock);
return count;
}
- ci->fsm.a_bus_req = 1;
- if (ci->fsm.otg->state == OTG_STATE_A_PERIPHERAL) {
+ ci->otg.fsm.a_bus_req = 1;
+ if (ci->otg.state == OTG_STATE_A_PERIPHERAL) {
ci->gadget.host_request_flag = 1;
- mutex_unlock(&ci->fsm.lock);
+ mutex_unlock(&ci->otg
[PATCH v9 10/14] usb: otg: add hcd companion support
From: Yoshihiro Shimoda
Since some host controller (e.g. EHCI) needs a companion host controller
(e.g. OHCI), this patch adds such a configuration to use it in the OTG
core.
Signed-off-by: Yoshihiro Shimoda
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
Acked-by: Rob Herring
---
Documentation/devicetree/bindings/usb/generic.txt | 3 +++
drivers/usb/common/usb-otg.c | 32 ---
include/linux/usb/otg.h | 7 -
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/generic.txt
b/Documentation/devicetree/bindings/usb/generic.txt
index f6866c1..1db1c33 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -27,6 +27,9 @@ Optional properties:
- otg-controller: phandle to otg controller. Host or gadget controllers can
contain this property to link it to a particular OTG
controller.
+ - hcd-needs-companion: must be present if otg controller is dealing with
+ EHCI host controller that needs a companion OHCI host
+ controller.
This is an attribute to a USB controller such as:
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
index b5bdf62..98fcfa2 100644
--- a/drivers/usb/common/usb-otg.c
+++ b/drivers/usb/common/usb-otg.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -348,6 +349,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
else
INIT_WORK(&otg->work, usb_drd_work);
+ if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
+ config->hcd_needs_companion)/* needs companion ? */
+ otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
+
otg->wq = create_freezable_workqueue("usb_otg");
if (!otg->wq) {
dev_err(dev, "otg: %s: can't create workqueue\n",
@@ -562,15 +567,18 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned
int irqnum,
/* HCD will be started by OTG fsm when needed */
mutex_lock(&otg->fsm.lock);
if (otg->primary_hcd.hcd) {
- /* probably a shared HCD ? */
- if (usb_otg_hcd_is_primary_hcd(hcd)) {
+ /* probably a shared HCD or a companion OHCI HCD ? */
+ if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+ usb_otg_hcd_is_primary_hcd(hcd)) {
dev_err(otg_dev, "otg: primary host already
registered\n");
goto err;
}
- if (hcd->shared_hcd == otg->primary_hcd.hcd) {
+ if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
+ (hcd->shared_hcd == otg->primary_hcd.hcd)) {
if (otg->shared_hcd.hcd) {
- dev_err(otg_dev, "otg: shared host already
registered\n");
+ dev_err(otg_dev,
+ "otg: shared/companion host already
registered\n");
goto err;
}
@@ -578,10 +586,12 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned
int irqnum,
otg->shared_hcd.irqnum = irqnum;
otg->shared_hcd.irqflags = irqflags;
otg->shared_hcd.ops = ops;
- dev_info(otg_dev, "otg: shared host %s registered\n",
+ dev_info(otg_dev,
+"otg: shared/companion host %s registered\n",
dev_name(hcd->self.controller));
} else {
- dev_err(otg_dev, "otg: invalid shared host %s\n",
+ dev_err(otg_dev,
+ "otg: invalid shared/companion host %s\n",
dev_name(hcd->self.controller));
goto err;
}
@@ -604,14 +614,17 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned
int irqnum,
* we're ready only if we have shared HCD
* or we don't need shared HCD.
*/
- if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
+ if (otg->shared_hcd.hcd ||
+ (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+!otg->primary_hcd.hcd->shared_hcd)) {
otg->host = hcd_to_bus(hcd);
/* FIXME: set bus->otg_port if this is true OTG port with HNP */
/* start FSM */
usb_otg_start_fsm(otg);
} else {
- dev_dbg(otg_dev, "otg: can't start till shared host
registers\n");
+ dev_dbg(otg_dev,
+ "otg: can't start till shared/companion host
registers\n");
}
mutex_unlock(&otg->fsm.lock);
@@ -659,7 +672,8 @@
[PATCH v9 09/14] usb: of: add an API to get OTG device from USB controller node
The OTG controller and the USB controller can be linked via the
'otg-controller' property in the USB controller's device node.
of_usb_get_otg() can be used to get the OTG controller device
from the USB controller's device node.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
Acked-by: Rob Herring
---
Documentation/devicetree/bindings/usb/generic.txt | 3 +++
drivers/usb/common/common.c | 27 +++
include/linux/usb/of.h| 9
3 files changed, 39 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/generic.txt
b/Documentation/devicetree/bindings/usb/generic.txt
index bba8257..f6866c1 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -24,6 +24,9 @@ Optional properties:
optional for OTG device.
- adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
optional for OTG device.
+ - otg-controller: phandle to otg controller. Host or gadget controllers can
+ contain this property to link it to a particular OTG
+ controller.
This is an attribute to a USB controller such as:
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index e3d0161..d7ec471 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -238,6 +238,33 @@ int of_usb_update_otg_caps(struct device_node *np,
}
EXPORT_SYMBOL_GPL(of_usb_update_otg_caps);
+#ifdef CONFIG_USB_OTG
+/**
+ * of_usb_get_otg - get the OTG controller linked to the USB controller
+ * @np: Pointer to the device_node of the USB controller
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * Returns the OTG controller device or NULL on error.
+ */
+struct device *of_usb_get_otg(struct device_node *np)
+{
+ struct device_node *otg_np;
+ struct platform_device *pdev;
+
+ otg_np = of_parse_phandle(np, "otg-controller", 0);
+ if (!otg_np)
+ return NULL;
+
+ pdev = of_find_device_by_node(otg_np);
+ of_node_put(otg_np);
+ if (!pdev)
+ return NULL;
+
+ return &pdev->dev;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_otg);
+#endif
+
#endif
MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index de3237f..499a4e8 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -40,6 +40,15 @@ static inline struct device_node *usb_of_get_child_node
}
#endif
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_OTG)
+struct device *of_usb_get_otg(struct device_node *np);
+#else
+static inline struct device *of_usb_get_otg(struct device_node *np)
+{
+ return NULL;
+}
+#endif
+
#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_SUPPORT)
enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
#else
--
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 v8 13/14] usb: gadget: udc: adapt to OTG core
On Wed, Jun 08, 2016 at 10:32:19AM +0300, Roger Quadros wrote:
> Hi,
>
> On 24/05/16 05:53, Peter Chen wrote:
> > On Mon, May 23, 2016 at 01:36:51PM +0300, Roger Quadros wrote:
> >> On 23/05/16 13:34, Jun Li wrote:
> >>> Hi
> >>>
> -Original Message-
> From: Roger Quadros [mailto:rog...@ti.com]
> Sent: Monday, May 23, 2016 6:12 PM
> To: Peter Chen
> Cc: Jun Li ; peter.c...@freescale.com; ba...@kernel.org;
> t...@atomide.com; gre...@linuxfoundation.org; dan.j.willi...@intel.com;
> mathias.ny...@linux.intel.com; joao.pi...@synopsys.com;
> sergei.shtyl...@cogentembedded.com; jun...@freescale.com;
> grygorii.stras...@ti.com; yoshihiro.shimoda...@renesas.com;
> r...@kernel.org; nsek...@ti.com; b-...@ti.com; linux-usb@vger.kernel.org;
> linux-o...@vger.kernel.org; linux-ker...@vger.kernel.org;
> devicet...@vger.kernel.org
> Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
>
> On 23/05/16 06:21, Peter Chen wrote:
> > On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
> >> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
> >>> On 18/05/16 17:46, Jun Li wrote:
>
>
> >>>
> >>> I didn't want to have complex Kconfig so decided to have otg as
> >>> built-in only.
> >>> What do you want me to change in existing code? and why?
> >>
> >> Remove those stuff which only for pass diff driver config Like
> >> every controller driver need a duplicated
> >>
> >> static struct otg_hcd_ops ci_hcd_ops = {
> >> ...
> >> }
> >
> > This is an exception only. Every controller driver doesn't need to
> > implement hcd_ops. It is implemented in the hcd core.
> >
> >>
> >> And here is another example, for gadget connect, otg driver can
> >> directly call to usb_udc_vbus_handler() in drd state machine, but
> >> you create another interface:
> >>
> >> .connect_control = usb_gadget_connect_control,
> >>
> >> If the symbol is defined in one driver which is 'm', another
> >> driver reference it should be 'm' as well, then there is no this
> >> kind of problem as my understanding.
> >
> > That is fine as long as all are 'm'. but how do you solve the case
> > when Gadget is built in and host is 'm'? OTG has to be built-in
> > and you will need an hcd to gadget interface.
>
> Hcd to gadget interface? Or you want to say otg to host interface?
> >>>
> >>> Sorry, I meant to say host to otg interface.
> >>>
>
> I think hcd and gadget are independent each other, now
>
> Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
> >>>
> >>> It is actually a circular dependency for both.
> >>> hcd <--> otg and gadget <--> otg
> >>>
> >>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for
> >>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg,
> >>> usb_hub_find_child
> >>>
> >>> gadget -> otg for usb_otg_register/unregister_gadget
> >>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
> >>>
> >>> Now consider what will happen if I get rid of the otg_hcd and
> otg_gadget interfaces.
> >>> 'y' means built-in, 'm' means module.
> >>>
> >>> 1) hcd 'y', gadget 'y'
> >>> otg has to be 'y' for proper build.
> >>>
> >>> 2) hcd 'm', gadget 'm'
> >>> otg has to be 'm' for proper build.
> >>>
> >>> 3) hcd 'y', gadget 'm'
> >>> Build will fail always.
> >>> If otg is 'y', otg build will fail due to dependency on gadget.
> >>> If otg is 'm', hcd build will fail due to dependency on otg.
> >>>
> >>> 4) hcd 'm', gadget 'y'
> >>> Build will fail always.
> >>> If otg is 'y', otg build will fail due to dependency on hcd.
> >>> If otg is 'm', gadget build will fails due to dependency on otg.
> >>>
> >>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
> >>> to remove otg->hcd and otg->gadget dependency.
> >>>
> >>> Now we can address 3) and 4) like so
> >>>
> >>> 3) hcd 'y', gadget 'm'
> >>> otg has to be 'y' for proper build.
> >>>
> >>> 4) hcd 'm', gadget 'y'
> >>> otg has to be 'y' for proper build.
> >>>
> >>
> >> How about this:
> >> Moving usb_otg_register/unregister_hcd to host driver to remove
> >> dependency hcd->otg. And moving usb_otg_get_data to common.c.
> >>
> >> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
> >> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
> >> When the otg driver is probed successfully, the host/device will be
> >> re-probed again, and usb_otg_register_hcd will be called again.
> >>
>
[PATCH v9 11/14] usb: otg: use dev_dbg() instead of VDBG()
Now that we have a device reference in struct usb_otg
let's use dev_dbg() for debug messages.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/common/usb-otg-fsm.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 482d4c9..e6e58c2 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -30,13 +30,6 @@
#include
#include
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s] " fmt, \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
/* Change USB protocol when there is a protocol change */
static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
{
@@ -44,8 +37,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
int ret = 0;
if (fsm->protocol != protocol) {
- VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
- fsm->protocol, protocol);
+ dev_vdbg(otg->dev,
+"Changing role fsm->protocol= %d; new protocol= %d\n",
+fsm->protocol, protocol);
/* stop old protocol */
if (fsm->protocol == PROTO_HOST)
ret = otg_start_host(otg, 0);
@@ -226,7 +220,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
if (otg->state == new_state)
return 0;
- VDBG("Set state: %s\n", usb_otg_state_string(new_state));
+ dev_vdbg(otg->dev, "Set state: %s\n", usb_otg_state_string(new_state));
otg_leave_state(fsm, otg->state);
switch (new_state) {
case OTG_STATE_B_IDLE:
@@ -358,7 +352,7 @@ int otg_statemachine(struct usb_otg *otg)
switch (state) {
case OTG_STATE_UNDEFINED:
- VDBG("fsm->id = %d\n", fsm->id);
+ dev_vdbg(otg->dev, "fsm->id = %d\n", fsm->id);
if (fsm->id)
otg_set_state(fsm, OTG_STATE_B_IDLE);
else
@@ -466,7 +460,8 @@ int otg_statemachine(struct usb_otg *otg)
}
mutex_unlock(&fsm->lock);
- VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+ dev_vdbg(otg->dev, "quit statemachine, changed = %d\n",
+fsm->state_changed);
return fsm->state_changed;
}
EXPORT_SYMBOL_GPL(otg_statemachine);
--
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
[PATCH v9 12/14] usb: hcd: Adapt to OTG core
Introduce usb_otg_add/remove_hcd() for use by host
controllers that are part of OTG/dual-role port.
Non Device tree platforms can use the otg_dev argument
to specify the OTG controller device. If otg_dev is NULL
then the device tree node's otg-controller property is used to
get the otg_dev device.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/core/hcd.c | 55 +
include/linux/usb/hcd.h | 4
2 files changed, 59 insertions(+)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index ae6c76d..c6f4155 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -46,6 +46,11 @@
#include
#include
#include
+#include
+#include
+
+#include
+#include
#include "usb.h"
@@ -3025,6 +3030,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
}
EXPORT_SYMBOL_GPL(usb_remove_hcd);
+static struct otg_hcd_ops otg_hcd_intf = {
+ .add = usb_add_hcd,
+ .remove = usb_remove_hcd,
+ .usb_bus_start_enum = usb_bus_start_enum,
+ .usb_control_msg = usb_control_msg,
+ .usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD
+ *
+ * Registers the HCD with OTG core. OTG core will call usb_add_hcd()
+ * or usb_remove_hcd() as necessary.
+ * If otg_dev is NULL then device tree node is checked for OTG
+ * controller device via the otg-controller property.
+ */
+int usb_otg_add_hcd(struct usb_hcd *hcd,
+ unsigned int irqnum, unsigned long irqflags,
+ struct device *otg_dev)
+{
+ struct device *dev = hcd->self.controller;
+
+ if (!otg_dev) {
+ hcd->otg_dev = of_usb_get_otg(dev->of_node);
+ if (!hcd->otg_dev)
+ return -ENODEV;
+ } else {
+ hcd->otg_dev = otg_dev;
+ }
+
+ return usb_otg_register_hcd(hcd, irqnum, irqflags, &otg_hcd_intf);
+}
+EXPORT_SYMBOL_GPL(usb_otg_add_hcd);
+
+/**
+ * usb_otg_remove_hcd - Unregister the HCD with OTG core.
+ * @hcd: the usb_hcd structure to remove
+ *
+ * Unregisters the HCD from the OTG core.
+ */
+void usb_otg_remove_hcd(struct usb_hcd *hcd)
+{
+ usb_otg_unregister_hcd(hcd);
+}
+EXPORT_SYMBOL_GPL(usb_otg_remove_hcd);
+
void
usb_hcd_platform_shutdown(struct platform_device *dev)
{
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 845c761..2331b48 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -473,6 +473,10 @@ extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd);
extern int usb_add_hcd(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags);
extern void usb_remove_hcd(struct usb_hcd *hcd);
+extern int usb_otg_add_hcd(struct usb_hcd *hcd,
+ unsigned int irqnum, unsigned long irqflags,
+ struct device *otg_dev);
+extern void usb_otg_remove_hcd(struct usb_hcd *hcd);
extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1);
struct platform_device;
--
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
[PATCH v9 08/14] usb: otg: add OTG/dual-role core
It provides APIs for the following tasks
- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine
Provide a dual-role device (DRD) state machine.
DRD mode is a reduced functionality OTG mode. In this mode
we don't support SRP, HNP and dynamic role-swap.
In DRD operation, the controller mode (Host or Peripheral)
is decided based on the ID pin status. Once a cable plug (Type-A
or Type-B) is attached the controller selects the state
and doesn't change till the cable in unplugged and a different
cable type is inserted.
As we don't need most of the complex OTG states and OTG timers
we implement a lean DRD state machine in usb-otg.c.
The DRD state machine is only interested in 2 hardware inputs
'id' and 'b_sess_vld'.
Signed-off-by: Roger Quadros
---
drivers/usb/Kconfig | 18 +
drivers/usb/Makefile | 1 +
drivers/usb/common/Makefile | 6 +-
drivers/usb/common/usb-otg.c | 832 +++
drivers/usb/core/Kconfig | 14 -
drivers/usb/gadget/Kconfig | 1 +
include/linux/usb/gadget.h | 2 +
include/linux/usb/hcd.h | 1 +
include/linux/usb/otg-fsm.h | 7 +
include/linux/usb/otg.h | 183 +-
10 files changed, 1035 insertions(+), 30 deletions(-)
create mode 100644 drivers/usb/common/usb-otg.c
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8689dcb..ed596ec 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -32,6 +32,23 @@ if USB_SUPPORT
config USB_COMMON
tristate
+config USB_OTG_CORE
+ tristate
+
+config USB_OTG
+ bool "OTG/Dual-role support"
+ depends on PM && USB && USB_GADGET
+ default n
+ ---help---
+ The most notable feature of USB OTG is support for a
+ "Dual-Role" device, which can act as either a device
+ or a host. The initial role is decided by the type of
+ plug inserted and can be changed later when two dual
+ role devices talk to each other.
+
+ Select this only if your board has Mini-AB/Micro-AB
+ connector.
+
config USB_ARCH_HAS_HCD
def_bool y
@@ -40,6 +57,7 @@ config USB
tristate "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
select USB_COMMON
+ select USB_OTG_CORE
select NLS # for UTF-8 strings
---help---
Universal Serial Bus (USB) is a specification for a serial bus
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index dca7856..03f7204 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/
obj-$(CONFIG_USB_GADGET) += gadget/
obj-$(CONFIG_USB_COMMON) += common/
+obj-$(CONFIG_USB_OTG_CORE) += common/
obj-$(CONFIG_USBIP_CORE) += usbip/
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index f8f2c88..5122b3f 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -7,5 +7,7 @@ usb-common-y += common.o
usb-common-$(CONFIG_USB_LED_TRIG) += led.o
obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
-usbotg-y := usb-otg-fsm.o
-obj-$(CONFIG_USB_OTG) += usbotg.o
+ifeq ($(CONFIG_USB_OTG),y)
+usbotg-y := usb-otg.o usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG_CORE) += usbotg.o
+endif
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
new file mode 100644
index 000..b5bdf62
--- /dev/null
+++ b/drivers/usb/common/usb-otg.c
@@ -0,0 +1,832 @@
+/**
+ * drivers/usb/common/usb-otg.c - USB OTG core
+ *
+ * Copyright (C) 2016 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Roger Quadros
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* OTG device list */
+LIST_HEAD(otg_list);
+static DEFINE_MUTEX(otg_list_mutex);
+
+static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
+{
+ if (!hcd->primary_hcd)
+ return 1;
+ return hcd == hcd->primary_hcd;
+}
+
+/**
+ * Check if the OTG device is in our OTG list and return
+ * usb_otg data, else NULL.
+ *
+ * otg_list_mutex must be held.
+ */
+static struct usb_otg *usb_otg_get_data(struct device *otg_dev)
+{
+ struct usb_otg *otg;
+
+ if (!otg_dev)
+ return NULL;
+
+ list_for_each_entry(otg, &otg_list, list) {
+ if (otg->dev == otg_dev)
+ return otg;
+ }
+
+ retu
[PATCH v9 13/14] usb: gadget: udc: adapt to OTG core
The OTG state machine needs a mechanism to start and
stop the gadget controller as well as connect/disconnect
from the bus. Add usb_gadget_start(), usb_gadget_stop()
and usb_gadget_connect_control().
Introduce usb_otg_add_gadget_udc() to allow controller drivers
to register a gadget controller that is part of an OTG instance.
Register with OTG core when UDC is added in usb_add_gadget_udc_release()
and unregister on usb_del_gadget_udc().
Notify the OTG core when gadget function driver is available on
udc_bind_to_driver() and when it is removed in usb_gadget_remove_driver().
We need to unlock the usb_lock mutex before calling
usb_otg_register_gadget() else it will cause a circular
locking dependency.
Ignore softconnect sysfs control when we're in OTG
mode as OTG FSM should care of gadget softconnect using
the b_bus_req mechanism.
Signed-off-by: Roger Quadros
---
drivers/usb/gadget/udc/udc-core.c | 202 --
include/linux/usb/gadget.h| 4 +
2 files changed, 196 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 6e8300d..a80a6c9 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,11 @@
#include
#include
#include
+#include
+#include
+
+#include
+#include
/**
* struct usb_udc - describes one usb device controller
@@ -337,6 +342,113 @@ static inline void usb_gadget_udc_stop(struct usb_udc
*udc)
}
/**
+ * usb_gadget_to_udc - get the UDC owning the gadget
+ *
+ * udc_lock must be held.
+ * Returs NULL if UDC is not found.
+ */
+static struct usb_udc *usb_gadget_to_udc(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc;
+
+ list_for_each_entry(udc, &udc_list, list)
+ if (udc->gadget == gadget)
+ return udc;
+
+ return NULL;
+}
+
+/**
+ * usb_gadget_start - start the usb gadget controller
+ * @gadget: the gadget device to start
+ *
+ * This is external API for use by OTG core.
+ *
+ * Start the usb device controller. Does not connect to the bus.
+ */
+static int usb_gadget_start(struct usb_gadget *gadget)
+{
+ int ret;
+ struct usb_udc *udc;
+
+ mutex_lock(&udc_lock);
+ udc = usb_gadget_to_udc(gadget);
+ if (!udc) {
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ ret = usb_gadget_udc_start(udc);
+ if (ret)
+ dev_err(&udc->dev, "USB Device Controller didn't start: %d\n",
+ ret);
+
+ mutex_unlock(&udc_lock);
+
+ return ret;
+}
+
+/**
+ * usb_gadget_stop - stop the usb gadget controller
+ * @gadget: the gadget device we want to stop
+ *
+ * This is external API for use by OTG core.
+ *
+ * Stop the gadget controller. Does not disconnect from the bus.
+ * Caller must ensure that gadget has disconnected from the bus
+ * before calling usb_gadget_stop().
+ */
+static int usb_gadget_stop(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc;
+
+ mutex_lock(&udc_lock);
+ udc = usb_gadget_to_udc(gadget);
+ if (!udc) {
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ if (gadget->connected)
+ dev_dbg(gadget->dev.parent,
+ "%s: called while still connected\n", __func__);
+
+ usb_gadget_udc_stop(udc);
+ mutex_unlock(&udc_lock);
+
+ return 0;
+}
+
+static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
+{
+ struct usb_udc *udc;
+
+ mutex_lock(&udc_lock);
+ udc = usb_gadget_to_udc(gadget);
+ if (!udc) {
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+ }
+
+ if (connect) {
+ usb_gadget_connect(udc->gadget);
+ } else {
+ usb_gadget_disconnect(udc->gadget);
+ udc->driver->disconnect(udc->gadget);
+ }
+
+ mutex_unlock(&udc_lock);
+
+ return 0;
+}
+
+/**
* usb_udc_release - release the usb_udc struct
* @dev: the dev member within usb_udc
*
@@ -359,6 +471,12 @@ static void usb_udc_nop_release(struct device *dev)
dev_vdbg(dev, "%s\n", __func__);
}
+struct otg_gadget_ops otg_gadget_intf = {
+ .start = usb_gadget_start,
+ .stop = usb_gadget_stop,
+ .connect_control = usb_gadget_connect_control,
+};
+
/**
* usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
* @parent: the parent device to this udc. Usually the controller driver's
@@ -414,6 +532,14 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_g
[PATCH v9 14/14] usb: host: xhci-plat: Add otg device to platform data
Host controllers that are part of an OTG/dual-role instance
need to somehow pass the OTG controller device information
to the HCD core.
We use platform data to pass the OTG controller device.
Signed-off-by: Roger Quadros
Reviewed-by: Peter Chen
---
drivers/usb/host/xhci-plat.c | 35 ---
include/linux/usb/xhci_pdriver.h | 3 +++
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 676ea45..24d030a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -239,11 +239,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto put_usb3_hcd;
}
- ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+ if (pdata && pdata->otg_dev)
+ ret = usb_otg_add_hcd(hcd, irq, IRQF_SHARED, pdata->otg_dev);
+ else
+ ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+
if (ret)
goto disable_usb_phy;
- ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+ if (pdata && pdata->otg_dev)
+ ret = usb_otg_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED,
+ pdata->otg_dev);
+ else
+ ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+
if (ret)
goto dealloc_usb2_hcd;
@@ -251,7 +260,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
dealloc_usb2_hcd:
- usb_remove_hcd(hcd);
+ if (pdata && pdata->otg_dev)
+ usb_otg_remove_hcd(hcd);
+ else
+ usb_remove_hcd(hcd);
disable_usb_phy:
usb_phy_shutdown(hcd->usb_phy);
@@ -269,16 +281,25 @@ put_hcd:
return ret;
}
-static int xhci_plat_remove(struct platform_device *dev)
+static int xhci_plat_remove(struct platform_device *pdev)
{
- struct usb_hcd *hcd = platform_get_drvdata(dev);
+ struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
+ struct usb_xhci_pdata *pdata = dev_get_platdata(&pdev->dev);
+
+ if (pdata && pdata->otg_dev)
+ usb_otg_remove_hcd(xhci->shared_hcd);
+ else
+ usb_remove_hcd(xhci->shared_hcd);
- usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
- usb_remove_hcd(hcd);
+ if (pdata && pdata->otg_dev)
+ usb_otg_remove_hcd(hcd);
+ else
+ usb_remove_hcd(hcd);
+
usb_put_hcd(xhci->shared_hcd);
if (!IS_ERR(clk))
diff --git a/include/linux/usb/xhci_pdriver.h b/include/linux/usb/xhci_pdriver.h
index 376654b..5c68b83 100644
--- a/include/linux/usb/xhci_pdriver.h
+++ b/include/linux/usb/xhci_pdriver.h
@@ -18,10 +18,13 @@
*
* @usb3_lpm_capable: determines if this xhci platform supports USB3
* LPM capability
+ * @otg_dev: OTG controller device. Only requied if part of
+ * OTG/dual-role.
*
*/
struct usb_xhci_pdata {
unsignedusb3_lpm_capable:1;
+ struct device *otg_dev;
};
#endif /* __USB_CORE_XHCI_PDRIVER_H */
--
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
[PATCH v9 02/14] usb: otg-fsm: Prevent build warning "VDBG" redefined
If usb/otg-fsm.h and usb/composite.h are included together
then it results in the build warning [1].
Prevent that by defining VDBG locally.
Also get rid of MPC_LOC which doesn't seem to be used
by anyone.
[1] - warning fixed by this patch:
In file included from drivers/usb/dwc3/core.h:33,
from drivers/usb/dwc3/ep0.c:33:
include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
In file included from drivers/usb/dwc3/ep0.c:31:
include/linux/usb/composite.h:615:1: warning: this is the location
of the previous definition
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/common/usb-otg-fsm.c | 7 +++
drivers/usb/phy/phy-fsl-usb.c| 7 +++
include/linux/usb/otg-fsm.h | 15 ---
3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 9059b7d..199dee0 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -30,6 +30,13 @@
#include
#include
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s] " fmt, \
+__func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
/* Change USB protocol when there is a protocol change */
static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
{
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..a8784ec 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -44,6 +44,13 @@
#include "phy-fsl-usb.h"
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s] " fmt, \
+__func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
#define DRIVER_VERSION "Rev. 1.55"
#define DRIVER_AUTHOR "Jerry Huang/Li Yang"
#define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 7a03505..a0a8f87 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -21,21 +21,6 @@
#include
#include
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s] " fmt , \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
#define PROTO_UNDEF(0)
#define PROTO_HOST (1)
#define PROTO_GADGET (2)
--
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: OHCI: Don't mark EDs as ED_OPER if scheduling fails
> If the list pointer contains LIST_POISON then it's already too late; > we've been accessing memory that was deallocated. Not really. I figured that LIST_POISON happens if the ED had been linked and unlinked normally before being submitted abnormally. And in fact, knowing that, I justs managed to reproduce this crash: # Start X with USB mouse, HID submits some URBs [ 22.860629] ohci-pci :00:12.0: link ed 8800df04c070 branch 0 [92us.], interval 8 [ 36.221548] fuse init (API version 7.23) # Stop X, HID pauses [ 48.435810] ohci-pci :00:12.0: unlink ed 8800df04c070 branch 0 [92us.], interval 8 # Connect the modem and steal the mouses's bandwidth [ 87.617143] ohci-pci :00:12.0: rhsc [ 87.622003] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00010101 CSC PPS CCS [ 87.827192] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 87.887247] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 87.947278] usb 1-1: new full-speed USB device number 4 using ohci-pci [ 87.977299] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 88.037325] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 88.175069] NET: Registered protocol family 8 [ 88.179452] NET: Registered protocol family 20 [ 88.196990] usb 1-1: [ueagle-atm] ADSL device founded vid (0X1110) pid (0X9032) Rev (0X2000): Eagle III [ 88.201451] ohci-pci :00:12.0: rhsc [ 88.227503] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 88.230733] ohci-pci :00:12.0: rhsc [ 88.287502] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 88.347514] usb 1-1: reset full-speed USB device number 4 using ohci-pci [ 88.377554] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 88.381932] ohci-pci :00:12.0: rhsc [ 88.437560] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 88.532329] usb 1-1: [ueagle-atm] pre-firmware device, uploading firmware [ 88.536583] usb 1-1: [ueagle-atm] loading firmware ueagle-atm/eagleIII.fw [ 88.540877] usbcore: registered new interface driver ueagle-atm [ 89.849038] usb 1-1: [ueagle-atm] firmware uploaded [ 89.874678] ohci-pci :00:12.0: rhsc [ 89.879085] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00030100 PESC CSC PPS [ 89.883498] usb 1-1: USB disconnect, device number 4 [ 92.019520] ohci-pci :00:12.0: rhsc [ 92.024128] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00010101 CSC PPS CCS [ 92.179643] ohci-pci :00:12.0: rhsc [ 92.209245] ohci-pci :00:12.0: rhsc [ 92.213738] ohci-pci :00:12.0: port[0] reset timeout, stat 0111 [ 92.269662] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 92.329671] usb 1-1: new full-speed USB device number 5 using ohci-pci [ 92.359715] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 92.419743] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 92.572667] usb 1-1: [ueagle-atm] ADSL device founded vid (0X1110) pid (0X9031) Rev (0X200B): Eagle III [ 92.599827] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 92.659867] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 92.719894] usb 1-1: reset full-speed USB device number 5 using ohci-pci [ 92.749921] ohci-pci :00:12.0: port[0] reset timeout, stat 0113 [ 92.809973] ohci-pci :00:12.0: GetStatus roothub.portstatus [0] = 0x00100103 PRSC PPS PES CCS [ 92.962755] usb 1-1: [ueagle-atm] using iso mode [ 92.967497] ohci-pci :00:12.0: link ed 8800df04c150 branch 0 [35us.], interval 1 [ 92.975753] ohci-pci :00:12.0: link iso ed 8800df04c1c0 branch 0 [793us.], interval 1 [ 92.980543] usb 1-1: [ueagle-atm] (re)booting started [ 94.682703] usb 1-1: [ueagle-atm] ATU-R firmware version : 44e2ea17 [ 94.696215] usb 1-1: [Ueagle-atm] firmware ueagle-atm/CMVep.bin.v2 is corrupted, try to get older cmvs [ 94.701136] usb 1-1: [Ueagle-atm] use deprecated cmvs version, please update your firmware [ 94.737733] usb 1-1: [ueagle-atm] modem started, waiting synchronization... # Start X, HID resumes [ 97.953708] ohci-pci :00:12.0: ERR -28, interval 8 msecs, load 92 # ENOSPC, mouse doesn't work this time, but ed->state became ED_OPER # Stop X, nothing happens # Start X, ed_schedule bypassed thanks to ED_OPER, mouse works # Stop X, we hit LIST_POISON on unlinking [ 143.457333] ohci-pci :00:12.0: unlink ed 8800df04c070 branch0 [92us.], interval 8 [ 143.457548] list_del: 8800df04c070 dead0100 # Start X, previous unlink() decreased ohci->load so ed_schedule passes [ 162.648710] ohci-pci :00:12.0: link ed 8800df04c070 branch 0 [92us.], interval 8 # Stop X [ 181.477893] ohci-pci :00:12.0: unlink ed
Re: [PATCH v2] usb: dwc3: host: Set the dma_ops for xhci device
Hi, On 6 June 2016 at 22:59, Felipe Balbi wrote: > > Hi, > > Baolin Wang writes: >> On ARM64 platform, it will set 'dummy_dma_ops' for device dma_ops if >> it did not call 'arch_setup_dma_ops' at device creation time, that will >> cause failure when setting the dma mask for device. >> >> Thus this patch set the xhci device dma_ops from the parent device if >> the xhci device dma_ops is 'dummy_dma_ops'. >> >> Changes since v1: >> - Add CONFIG_ARM64 macro. >> >> Signed-off-by: Baolin Wang >> --- >> drivers/usb/dwc3/host.c |5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >> index c679f63..edb666d 100644 >> --- a/drivers/usb/dwc3/host.c >> +++ b/drivers/usb/dwc3/host.c >> @@ -32,6 +32,11 @@ int dwc3_host_init(struct dwc3 *dwc) >> return -ENOMEM; >> } >> >> +#ifdef CONFIG_ARM64 >> + if (get_dma_ops(&xhci->dev) == get_dma_ops(NULL)) >> + xhci->dev.archdata.dma_ops = get_dma_ops(dwc->dev); >> +#endif > > I don't like the ifdef and also don't like that this is done in dwc3 > itself. Seems like we need something like this done from the > platform_bus core. I've sent one patch to fix this issue in platform_bus core, and Robin explained "DMA-capable devices are real hardware, therefore don't spring out of thin air without being described in DT or ACPI." But the problem is here, how can we handle this issue to make the host work well? What about below modification? Thanks. diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index c679f63..f7c58f9 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -17,6 +17,7 @@ #include #include +#include #include "core.h" @@ -37,6 +38,7 @@ int dwc3_host_init(struct dwc3 *dwc) xhci->dev.parent= dwc->dev; xhci->dev.dma_mask = dwc->dev->dma_mask; xhci->dev.dma_parms = dwc->dev->dma_parms; + of_dma_configure(&xhci->dev, dwc->dev->of_node); dwc->xhci = xhci; -- Baolin.wang Best Regards -- 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
[balbi-usb:testing/next 64/67] otg.c:undefined reference to `usb_gadget_vbus_disconnect'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next head: fc4d1f3f522648d92d0c046beedbc1469979499a commit: 74e51eb7b5b4f7ab33c099c20def3dce0c699006 [64/67] usb: gadget: move gadget API functions to udc-core config: i386-randconfig-h1-06081244 (attached as .config) compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430 reproduce: git checkout 74e51eb7b5b4f7ab33c099c20def3dce0c699006 # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/built-in.o: In function `nop_set_peripheral': phy-generic.c:(.text+0x59f7a6): undefined reference to `usb_gadget_vbus_connect' drivers/built-in.o: In function `nop_gpio_vbus_thread': phy-generic.c:(.text+0x59f8a1): undefined reference to `usb_gadget_vbus_connect' phy-generic.c:(.text+0x59f925): undefined reference to `usb_gadget_vbus_disconnect' drivers/built-in.o: In function `gpio_vbus_set_peripheral': phy-gpio-vbus-usb.c:(.text+0x59ff45): undefined reference to `usb_gadget_vbus_disconnect' drivers/built-in.o: In function `gpio_vbus_work': phy-gpio-vbus-usb.c:(.text+0x5a02dd): undefined reference to `usb_gadget_vbus_disconnect' phy-gpio-vbus-usb.c:(.text+0x5a0333): undefined reference to `usb_gadget_vbus_connect' drivers/built-in.o: In function `ci_handle_vbus_change.part.6': >> otg.c:(.text+0x605e3b): undefined reference to `usb_gadget_vbus_disconnect' >> otg.c:(.text+0x605e49): undefined reference to `usb_gadget_vbus_connect' drivers/built-in.o: In function `isp1704_charger_work': isp1704_charger.c:(.text+0x648335): undefined reference to `usb_gadget_disconnect' isp1704_charger.c:(.text+0x64839d): undefined reference to `usb_gadget_connect' drivers/built-in.o: In function `isp1704_charger_probe': isp1704_charger.c:(.text+0x648760): undefined reference to `usb_gadget_disconnect' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [RFC PATCH 0/5] USB Audio Gadget refactoring
Hi guys, Any feedback on this patch series? Has anybody had a chance to test it? Regards, Ruslan On Tue, May 24, 2016 at 2:50 AM, Ruslan Bilovol wrote: > I came to this patch series when wanted to do two things: > - use UAC1 as virtual ALSA sound card on gadget side, >just like UAC2 is used so it's possible to do rate >resampling > - have both playback/capture support in UAC1 > > Since I wanted to have same behavior for both UAC1/UAC2, > obviously I've got an utility part (u_audio.c) for > virtual ALSA sound card handling like we have > for ethernet(u_ether) or serial(u_serial) functions. > Function-specific parts (f_uac1/f_uac2) became almost > as storage for class-specfic USB descriptors, some > boilerplate for configfs, binding and few USB > config request handling. > > Major change to f_uac1 it that it can't do > direct play to existing ALSA sound card anymore, > representing audio on gadget side as virtual > ALSA sound card where audio streams are simply > sinked to and sourced from it, so it may break > current usecase for some people (and that's why > it's RFC). > > Luckily, it's possible to use existing user-space > applications for audio routing between Audio Gadget > and real sound card. I personally use alsaloop tool > from alsautils and have ability to create PCM > loopback between two different ALSA cards using > rate resampling, which is not possible with previous > "direct play to ALSA card" approach in f_uac1. > > While here, also dropped redundant platform > driver/device creation in f_uac2 driver as well as > "never implemented" volume/mute functionality in f_uac1 > that made this work even easier to do. > > This series is tested with both legacy g_audio.ko and > modern configfs approaches under Ubuntu 14.04 (UAC1 and > UAC2) and under Windows7 x64 (UAC1 only) having > perfect results in all cases. > > Some changes may have lack of good description that may > be obvious for me but not so clear for others, but I > hope to fix it in next versions. > > Comments, testing are welcome. > > Ruslan Bilovol (5): > usb: gadget: f_uac2: remove platform driver/device creation > usb: gadget: f_uac2: split out audio core > usb: gadget: f_uac1: drop volume/mute functionality > usb: gadget: f_uac1: switch to u_audio core utilities > usb: gadget: f_uac1: add capture support > > drivers/usb/gadget/Kconfig| 13 +- > drivers/usb/gadget/function/Makefile | 3 +- > drivers/usb/gadget/function/f_uac1.c | 842 > +- > drivers/usb/gadget/function/f_uac2.c | 778 --- > drivers/usb/gadget/function/u_audio.c | 632 + > drivers/usb/gadget/function/u_audio.h | 93 > drivers/usb/gadget/function/u_uac1.c | 314 - > drivers/usb/gadget/function/u_uac1.h | 71 +-- > drivers/usb/gadget/legacy/Kconfig | 1 + > drivers/usb/gadget/legacy/audio.c | 54 ++- > 10 files changed, 1208 insertions(+), 1593 deletions(-) > create mode 100644 drivers/usb/gadget/function/u_audio.c > create mode 100644 drivers/usb/gadget/function/u_audio.h > delete mode 100644 drivers/usb/gadget/function/u_uac1.c > > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux
On 07/06/16 18:05, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >>> I might be able to find some time to implement a proof of concept which >>> would allow your platforms to get dual-role with code we already have, >>> but I need DWC3's OTG support which, I'm assuming, you already have :-) >>> >>> If you wanna try something offline, just ping me ;-) I'll be happy to >>> help. >> >> What you are proposing is a dwc3 only solution. With the otg/dual-role >> series we are trying to be generic as much as possible. > > Well, if there is a need for that, sure. Take MUSB for instance. It > makes use of nothing of the sorts, because it doesn't have to. > >> Whether controller drivers want to use it or not is upto the driver >> maintainers but we should at least ensure that user space ABI if any, >> is consistent across different implementations. > > Role decisions should not be exposed to userspace unless as debug > feature (using e.g. DebugFS). That should be done either by the HW or > within the kernel. > > If we're discussing userspace ABI here, there's something very wrong > with OTG/DRD layer design. Not really. There can be a need for user space application to control the port role. Consider Apple carplay for instance or even full OTG support which has user selectable role. > How are you switching the port mux between host and peripheral? Only by sysfs or do you have a GPIO for ID pin as well? >>> >>> depends. Some SoCs have GPIO-controller muxes while some just have mux's >>> select signals (one for ID, one for VBUS) mapped on xHCI's address >>> space. >>> What happens to the gadget controller when the port is muxed to the host controller? Is it stopped or it continues to run? >>> >>> it continues running, but that's pretty irrelevant for Intel's dual-role >> >> Isn't that unnecessary waste of power? Or you have firmware assisted >> low power mode? > > that's an implementation detail which brings nothing to this discussion, > right? :-) > > We can, certainly, put the other side to D3. > >>> setup. We have an actual physical (inside the die, though) mux which >>> muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only >>> DWC3. >>> >> >> Probably irrelevant for Intel's dual-role but many platforms that >> share the port can't have device controller running when port is in >> host mode and vice versa. > > but that doesn't mean we need an entire new layer added to the kernel > ;-) > > DWC3 already gives us all the information necessary to make a decision > on which role we should assume. Just consider your options. Here's how > things would look like without any OTG/DRD layer: > > -> DWC3 OTG IRQ > -> readl(OSTS); > -> if (OSTS & BIT(4)) >-> dwc3_host_exit(); __dwc3_gadget_start(); > -> else >-> __dwc3_gadget_stop(); dwc3_host_init(); > > Can you draw something similar for your proposed OTG/DRD layer? What about B_IDLE state? We don't want either peripheral or host to run when no cable is inserted. Have you tested if it works? I'd be happy to test if you can prepare a patch to get dual-role working on dwc3 without the OTG/DRD layer :). > > I remember there were at least two schedule_work(). IIRC it looked > something like below: > > -> DWC3 OTG IRQ > -> readl(OSTS); > -> if (OSTS & BIT(4)) >-> otg_set_mode(PERIPHERAL); > -> schedule_work(); > -> otg_ops->stop_host(); > -> usb_del_hcd(); > -> otg_ops->start_peripheral(); > -> usb_gadget_add_udc(); > -> else >-> otg_set_mode(HOST); > -> schedule_work(); > -> otg_ops->stop_peripheral(); > -> usb_gadget_del_udc(); > -> otg_ops->start_host(); > -> usb_add_hcd(); > > I'm probably missing some steps there. As a user you just need to do this reg = read(OSTS); dwc->otg->fsm.id = !!(reg & STS_ID); dwc->otg->fsm.b_sess_vld = !!(reg &STS_BSESVLD); usb_otg_sync_inputs(dwc->otg); And the layer does the rest. But as I said earlier. I have absolutely no issues if dwc3 doesn't use that layer as long as dual-role works on our platforms. > >> So there has to be a central point of control where the respective >> controllers are started/stopped. > > some implementations might need this, yes. DWC3 and MUSB don't seem to > be this type of system. OK. > >> That is the other point we are trying to address with the common >> otg/dual-role code. >> >> Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need >> to stop the host controller for device mode, right? > > yes, see above. We already have that code. Just code is not enough. We need to know if it works :). > >> If so then who will deal with start/stop of the controllers then? > > dwc3 itself. > OK. cheers, -roger signature.asc Description: OpenPGP digital signature
Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device
Hi Greg, On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote: > On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote: >> In some Intel platforms, a single usb port is shared between USB host >> and device controllers. The shared port is under control of a switch >> which is defined in the Intel vendor defined extended capability for >> xHCI. >> >> This patch adds the support to detect and create the platform device >> for the port mux switch. > Why do you need a platform device for this? You do nothing with this > device, why create it at all? In this patch series, I have a generic framework for port mux devices and two port mux drivers sitting on top the generic code. In this patch, I create a platform device for the real mux device in Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux into the generic framework and handle the power management things in driver's pm entries (otherwise, the system can't be waken up from system suspend). > And why is it a platform device, isn't is really a PCI device? Why > would you ever find a "platform" device below a PCI device? Don't abuse > platform devices for things that aren't. It makes me want to delete > that whole interface more and more... Port mux devices are physical devices in Intel Cherry Trail and Broxton SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI space. OS kernel can enumerate it by looking up the xhci extended capability list with a vendor specific capability ID. Best regards, Lu Baolu -- 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: Re: Re: Re: Re: [PATCH] usb: core: fix a double free in the usb driver
>On Tue, 7 Jun 2016, Chung-Geol Kim wrote: > >> = >> At *remove USB(3.0) Storage >> sequence <1> --> <5> ((Problem Case)) >> = >> VOLD >> | >> (uevent) >> |_ >>|<1> | >>|dwc3_otg_sm_work | >>|usb_put_hcd | >>|shared_hcd(kref=2)| >>|__| >> |_ >>|<2> | >>|New USB BUS #2| >>| | >>|shared_hcd(kref=1)| >>| | >> --(Link)-bandXX_mutex| >> | |__| >> | >> ___ | >>|<3>| | >>|dwc3_otg_sm_work | | >>|usb_put_hcd| | >>|primary_hcd(kref=1)| | >>|___| | >> _|_ | >>|<4>| | >>|New USB BUS #1 | | >>|hcd_release| | >>|primary_hcd(kref=0)| | >>| | | >>|bandXX_mutex(free) |<- >>|___| >>(( VOLD )) >> __|___ >>|<5> | >>| SCSI| >>|usb_put_hcd | >>|shared_hcd(kref=0)| >>|*hcd_release | >>|bandXX_mutex(free*)|<- double free >>|__| >> >> = > >Okay, now I understand the problem you want to solve. What we need to >do is make sure the mutex is deallocated when the _last_ hcd is >released, which is not necessarily the same as when the _primary_ hcd >is released. > >Can you please test the patch below? > >By the way, a good change (if you want to do it) would be to rename the >"shared_hcd" field to "other_hcd" or "peer_hcd". This is because it >always points to the other hcd in the peer set: In the primary >structure it points to the secondary, and in the secondary structure it >points to the primary. > >Alan Stern > Thank you for clear understanding the problem that I faced. When I tested with your below patch, it also works well. The description has been modified as follows as you suggested. = At *remove USB(3.0) Storage sequence <1> --> <5> ((Problem Case)) = VOLD | (uevent) |_ |<1> | |dwc3_otg_sm_work | |usb_put_hcd | |peer_hcd(kref=2)| |__| |_ |<2> | |New USB BUS #2| | | |peer_hcd(kref=1)| | | --(Link)-bandXX_mutex| | |__| | ___ | |<3>| | |dwc3_otg_sm_work | | |usb_put_hcd| | |primary_hcd(kref=1)| | |___| | _|_ | |<4>| | |New USB BUS #1 | | |hcd_release| | |primary_hcd(kref=0)| | | | | |bandXX_mutex(free) |<- |___| (( VOLD )) __|___ |<5> | | SCSI| |usb_put_hcd | |peer_hcd(kref=0)| |*hcd_release | |bandXX_mutex(free*)|<- double free |__| = > > >Index: usb-4.x/drivers/usb/core/hcd.c >=== >--- usb-4.x.orig/drivers/usb/core/hcd.c >+++ usb-4.x/drivers/usb/core/hcd.c >@@ -2588,24 +2588,22 @@ EXPORT_SYMBOL_GPL(usb_create_hcd); > * Don't deallocate the bandwidth_mutex until the last
Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
Hi,
On 24/05/16 05:53, Peter Chen wrote:
> On Mon, May 23, 2016 at 01:36:51PM +0300, Roger Quadros wrote:
>> On 23/05/16 13:34, Jun Li wrote:
>>> Hi
>>>
-Original Message-
From: Roger Quadros [mailto:rog...@ti.com]
Sent: Monday, May 23, 2016 6:12 PM
To: Peter Chen
Cc: Jun Li ; peter.c...@freescale.com; ba...@kernel.org;
t...@atomide.com; gre...@linuxfoundation.org; dan.j.willi...@intel.com;
mathias.ny...@linux.intel.com; joao.pi...@synopsys.com;
sergei.shtyl...@cogentembedded.com; jun...@freescale.com;
grygorii.stras...@ti.com; yoshihiro.shimoda...@renesas.com;
r...@kernel.org; nsek...@ti.com; b-...@ti.com; linux-usb@vger.kernel.org;
linux-o...@vger.kernel.org; linux-ker...@vger.kernel.org;
devicet...@vger.kernel.org
Subject: Re: [PATCH v8 13/14] usb: gadget: udc: adapt to OTG core
On 23/05/16 06:21, Peter Chen wrote:
> On Sat, May 21, 2016 at 10:29:40AM +0800, Peter Chen wrote:
>> On Thu, May 19, 2016 at 10:32:44AM +0300, Roger Quadros wrote:
>>> On 18/05/16 17:46, Jun Li wrote:
>>>
>>> I didn't want to have complex Kconfig so decided to have otg as
>>> built-in only.
>>> What do you want me to change in existing code? and why?
>>
>> Remove those stuff which only for pass diff driver config Like
>> every controller driver need a duplicated
>>
>> static struct otg_hcd_ops ci_hcd_ops = {
>> ...
>> }
>
> This is an exception only. Every controller driver doesn't need to
> implement hcd_ops. It is implemented in the hcd core.
>
>>
>> And here is another example, for gadget connect, otg driver can
>> directly call to usb_udc_vbus_handler() in drd state machine, but
>> you create another interface:
>>
>> .connect_control = usb_gadget_connect_control,
>>
>> If the symbol is defined in one driver which is 'm', another
>> driver reference it should be 'm' as well, then there is no this
>> kind of problem as my understanding.
>
> That is fine as long as all are 'm'. but how do you solve the case
> when Gadget is built in and host is 'm'? OTG has to be built-in
> and you will need an hcd to gadget interface.
Hcd to gadget interface? Or you want to say otg to host interface?
>>>
>>> Sorry, I meant to say host to otg interface.
>>>
I think hcd and gadget are independent each other, now
Hcd --> otg; and gadget --> otg, (hcd and gadget use otg's symbol)
>>>
>>> It is actually a circular dependency for both.
>>> hcd <--> otg and gadget <--> otg
>>>
>>> hcd -> otg for usb_otg_register/unregister_hcd otg -> hcd for
>>> usb_add/remove_hcd, usb_bus_start_enum, usb_control_msg,
>>> usb_hub_find_child
>>>
>>> gadget -> otg for usb_otg_register/unregister_gadget
>>> otg -> gadget for usb_gadget_start/stop, usb_udc_vbus_handler
>>>
>>> Now consider what will happen if I get rid of the otg_hcd and
otg_gadget interfaces.
>>> 'y' means built-in, 'm' means module.
>>>
>>> 1) hcd 'y', gadget 'y'
>>> otg has to be 'y' for proper build.
>>>
>>> 2) hcd 'm', gadget 'm'
>>> otg has to be 'm' for proper build.
>>>
>>> 3) hcd 'y', gadget 'm'
>>> Build will fail always.
>>> If otg is 'y', otg build will fail due to dependency on gadget.
>>> If otg is 'm', hcd build will fail due to dependency on otg.
>>>
>>> 4) hcd 'm', gadget 'y'
>>> Build will fail always.
>>> If otg is 'y', otg build will fail due to dependency on hcd.
>>> If otg is 'm', gadget build will fails due to dependency on otg.
>>>
>>> So I solve this problem by adding the otg_hcd_ops and otg_gadget_ops
>>> to remove otg->hcd and otg->gadget dependency.
>>>
>>> Now we can address 3) and 4) like so
>>>
>>> 3) hcd 'y', gadget 'm'
>>> otg has to be 'y' for proper build.
>>>
>>> 4) hcd 'm', gadget 'y'
>>> otg has to be 'y' for proper build.
>>>
>>
>> How about this:
>> Moving usb_otg_register/unregister_hcd to host driver to remove
>> dependency hcd->otg. And moving usb_otg_get_data to common.c.
>>
>> Delete the wait queue at usb-otg.c, and if calling usb_otg_get_data
>> returns NULL, the host/device driver's probe return -EPROBE_DEFER.
>> When the otg driver is probed successfully, the host/device will be
>> re-probed again, and usb_otg_register_hcd will be called again.
>>
>> And let OTG depends on HCD && GADGET, and delete otg_hcd_ops and
>> otg_gadget_ops. Below build dependency issues can be fixed.
>> What do you think?
>>
>>> 1) hcd 'y', gadget 'y'
>>> otg has to be 'y' for proper build.
>>>
>>> 2) hcd 'm', gadget 'm'
>>> otg h
[PATCH 2/2] usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable()
This patch fixes an issue that the CFIFOSEL register value is possible
to be changed by usbhsg_ep_enable() wrongly. And then, a data transfer
using CFIFO may not work correctly.
For example:
# modprobe g_multi file=usb-storage.bin
# ifconfig usb0 192.168.1.1 up
(During the USB host is sending file to the mass storage)
# ifconfig usb0 down
In this case, since the u_ether.c may call usb_ep_enable() in
eth_stop(), if the renesas_usbhs driver is also using CFIFO for
mass storage, the mass storage may not work correctly.
So, this patch adds usbhs_lock() and usbhs_unlock() calling in
usbhsg_ep_enable() to protect CFIFOSEL register. This is because:
- CFIFOSEL.CURPIPE = 0 is also needed for the pipe configuration
- The CFIFOSEL (fifo->sel) is already protected by usbhs_lock()
Fixes: 97664a207bc2 ("usb: renesas_usbhs: shrink spin lock area")
Cc: # v3.1+
Signed-off-by: Yoshihiro Shimoda
---
drivers/usb/renesas_usbhs/mod_gadget.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c
index 30345c2..50f3363 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -585,6 +585,9 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
struct usbhs_pipe *pipe;
int ret = -EIO;
+ unsigned long flags;
+
+ usbhs_lock(priv, flags);
/*
* if it already have pipe,
@@ -593,7 +596,8 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
if (uep->pipe) {
usbhs_pipe_clear(uep->pipe);
usbhs_pipe_sequence_data0(uep->pipe);
- return 0;
+ ret = 0;
+ goto usbhsg_ep_enable_end;
}
pipe = usbhs_pipe_malloc(priv,
@@ -621,6 +625,9 @@ static int usbhsg_ep_enable(struct usb_ep *ep,
ret = 0;
}
+usbhsg_ep_enable_end:
+ usbhs_unlock(priv, flags);
+
return ret;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: renesas_usbhs: fix NULL pointer dereference in xfer_work()
This patch fixes an issue that the xfer_work() is possible to cause
NULL pointer dereference if the usb cable is disconnected while data
transfer is running.
In such case, a gadget driver may call usb_ep_disable()) before
xfer_work() is actually called. In this case, the usbhs_pkt_pop()
will call usbhsf_fifo_unselect(), and then usbhs_pipe_to_fifo()
in xfer_work() will return NULL.
Fixes: e73a989 ("usb: renesas_usbhs: add DMAEngine support")
Cc: # v3.1+
Signed-off-by: Yoshihiro Shimoda
---
drivers/usb/renesas_usbhs/fifo.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 7be4e7d..280ed5f 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -810,20 +810,27 @@ static void xfer_work(struct work_struct *work)
{
struct usbhs_pkt *pkt = container_of(work, struct usbhs_pkt, work);
struct usbhs_pipe *pipe = pkt->pipe;
- struct usbhs_fifo *fifo = usbhs_pipe_to_fifo(pipe);
+ struct usbhs_fifo *fifo;
struct usbhs_priv *priv = usbhs_pipe_to_priv(pipe);
struct dma_async_tx_descriptor *desc;
- struct dma_chan *chan = usbhsf_dma_chan_get(fifo, pkt);
+ struct dma_chan *chan;
struct device *dev = usbhs_priv_to_dev(priv);
enum dma_transfer_direction dir;
+ unsigned long flags;
+ usbhs_lock(priv, flags);
+ fifo = usbhs_pipe_to_fifo(pipe);
+ if (!fifo)
+ goto xfer_work_end;
+
+ chan = usbhsf_dma_chan_get(fifo, pkt);
dir = usbhs_pipe_is_dir_in(pipe) ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV;
desc = dmaengine_prep_slave_single(chan, pkt->dma + pkt->actual,
pkt->trans, dir,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!desc)
- return;
+ goto xfer_work_end;
desc->callback = usbhsf_dma_complete;
desc->callback_param= pipe;
@@ -831,7 +838,7 @@ static void xfer_work(struct work_struct *work)
pkt->cookie = dmaengine_submit(desc);
if (pkt->cookie < 0) {
dev_err(dev, "Failed to submit dma descriptor\n");
- return;
+ goto xfer_work_end;
}
dev_dbg(dev, " %s %d (%d/ %d)\n",
@@ -842,6 +849,9 @@ static void xfer_work(struct work_struct *work)
usbhs_pipe_set_trans_count_if_bulk(pipe, pkt->trans);
dma_async_issue_pending(chan);
usbhs_pipe_enable(pipe);
+
+xfer_work_end:
+ usbhs_unlock(priv, flags);
}
/*
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: renesas_usbhs: fix issues on specific situations
This patch set is based on the latest Felipe's usb.git / testing/fixes branch. (commit id = 50c763f8c1bac0dc00f7788a75f227276c0efd54) Yoshihiro Shimoda (2): usb: renesas_usbhs: fix NULL pointer dereference in xfer_work() usb: renesas_usbhs: protect the CFIFOSEL setting in usbhsg_ep_enable() drivers/usb/renesas_usbhs/fifo.c | 18 ++ drivers/usb/renesas_usbhs/mod_gadget.c | 9 - 2 files changed, 22 insertions(+), 5 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] usb: gadget: Allow to build both USB functions and legacy gadgets
On Tue, Jun 07, 2016 at 11:59:57AM +0200, Krzysztof Opasiak wrote: > Hi, > > On Tue, Jun 7, 2016 at 11:06 AM, Peter Chen wrote: > > > > > >>-Original Message- > >>From: linux-usb-ow...@vger.kernel.org > >>[mailto:linux-usb-ow...@vger.kernel.org] > >>On Behalf Of Krzysztof Opasiak > >>Sent: Tuesday, June 07, 2016 3:46 PM > >>To: Peter Chen > >>Cc: Krzysztof Opasiak ; ba...@kernel.org; Greg Kroah- > >>Hartman ; Andrzej Pietrasiewicz > >>; Marek Szyprowski ; > >>linux-usb@vger.kernel.org > >>Subject: Re: [RFC][PATCH] usb: gadget: Allow to build both USB functions and > >>legacy gadgets > >> > >>Hi, > >> > >>On Tue, Jun 7, 2016 at 3:27 AM, Peter Chen wrote: > >>> On Mon, Jun 06, 2016 at 09:40:33PM +0200, Krzysztof Opasiak wrote: > Currently it is possible to build in some subset of usb functions > *OR* some gadget module. This is limited only by Kconfig not any > functionality. > > This patch removes this limitation. With this patch it is possible to > set up all build combinations: > 1) Multiple gadgets build in > >>> > >>> If that, what the user will expect if choosing multiple gadgets? > >>> Eg, if he chooses g_ncm and g_mass_storage, will he expect his udc has > >>> both mass_storage and ncm function, but it is not the fact, only the > >>> first gadget function will work. > >>> > >> > >>Not exactly one. You may build in multiple modules and use those multiple > >>modules if > >>you have multiple udcs. > >> > > > > My concern is: with your patch, the user may get unexpected results if he > > builds in > > multiple gadgets for this single udc. At current code, the user can't do > > that, then he will > > not get unexpected results. How do you consider that? > > > > That's true but even without this patch user may build multiple > gadgets as a modules. > > Even more, after commit (855ed04 "usb: gadget: udc-core: independent > registration of > gadgets and gadget drivers") it is possible to load multiple gadget > modules even if we have one UDC. > > So why would you like to disallow building in multiple gadgets if user > can build them as a modules and simply load? For module, the user can control which one to load, he or she can choose which function he needs. But if they are build-in, the user may make mistake and choose multiple gadgets, and get the unexpected results. This is only I concern. I just do not see the benefits that allow building in multiple gadgets, If you have some use cases, then that's reason for doing that:) > > If you concern about order of probing and binding to udc, I think that > we can add udc module param to all legacy gadgets so user can specify > in kernel cmd line where each gadget should be bound. > Yes, currently, we can't do that even the gadget is module, but the change may not be small. Now, we have configfs, it is not so necessary to add this future. -- 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
