[PATCH RESEND] usb: gadget: gadgetfs: unregister gadget only if it got successfully registered
Gadgetfs driver called usb_gadget_unregister_driver unconditionally, even
if it didn't register it earlier due to other failures. This patch fixes
this.
Reported-by: Vegard Nossum
Signed-off-by: Marek Szyprowski
Tested-by: Vegard Nossum
---
drivers/usb/gadget/legacy/inode.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index 7a62a2f7bc18..cde6a2133c90 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -130,7 +130,8 @@ struct dev_data {
setup_can_stall : 1,
setup_out_ready : 1,
setup_out_error : 1,
- setup_abort : 1;
+ setup_abort : 1,
+ gadget_registered : 1;
unsignedsetup_wLength;
/* the rest is basically write-once */
@@ -1179,7 +1180,8 @@ dev_release (struct inode *inode, struct file *fd)
/* closing ep0 === shutdown all */
- usb_gadget_unregister_driver (&gadgetfs_driver);
+ if (dev->gadget_registered)
+ usb_gadget_unregister_driver (&gadgetfs_driver);
/* at this point "good" hardware has disconnected the
* device from USB; the host won't see it any more.
@@ -1825,6 +1827,7 @@ dev_config (struct file *fd, const char __user *buf,
size_t len, loff_t *ptr)
* kick in after the ep0 descriptor is closed.
*/
value = len;
+ dev->gadget_registered = true;
}
return value;
--
1.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 RESEND] usb: gadget: provide interface for legacy gadgets to get UDC name
Hi,
Marek Szyprowski writes:
> Since commit 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb: gadget:
> udc-core: independent registration of gadgets and gadget drivers") gadget
> drivers can not assume that UDC drivers are already available on their
> initialization. This broke the HACK, which was used in gadgetfs driver,
> to get UDC controller name. This patch removes this hack and replaces it
> by additional function in the UDC core (which is usefully only for legacy
> drivers, please don't use it in the new code).
>
> Reported-by: Vegard Nossum
> Signed-off-by: Marek Szyprowski
> Tested-by: Vegard Nossum
> ---
> changelog:
> v2:
> - properly report udc gagdet driver name instead of udc device name
>
> v1: https://lkml.org/lkml/2016/2/8/176
> - initial version
> ---
> drivers/usb/gadget/legacy/inode.c | 29 ++---
> drivers/usb/gadget/udc/udc-core.c | 33 +
> include/linux/usb/gadget.h| 1 +
> 3 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/inode.c
> b/drivers/usb/gadget/legacy/inode.c
> index 7e179f81d05c..7a62a2f7bc18 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -227,7 +227,7 @@ static void put_ep (struct ep_data *data)
> * implicitly, starting with the driver name and then endpoint names.
> */
>
> -static const char *CHIP;
> +static char CHIP[32];
>
> /*--*/
>
> @@ -1697,28 +1697,6 @@ static struct usb_gadget_driver gadgetfs_driver = {
> };
>
> /*--*/
> -
> -static void gadgetfs_nop(struct usb_gadget *arg) { }
> -
> -static int gadgetfs_probe(struct usb_gadget *gadget,
> - struct usb_gadget_driver *driver)
> -{
> - CHIP = gadget->name;
> - return -EISNAM;
> -}
> -
> -static struct usb_gadget_driver probe_driver = {
> - .max_speed = USB_SPEED_HIGH,
> - .bind = gadgetfs_probe,
> - .unbind = gadgetfs_nop,
> - .setup = (void *)gadgetfs_nop,
> - .disconnect = gadgetfs_nop,
> - .driver = {
> - .name = "nop",
> - },
> -};
> -
> -
> /* DEVICE INITIALIZATION
> *
> * fd = open ("/dev/gadget/$CHIP", O_RDWR)
> @@ -1968,10 +1946,7 @@ gadgetfs_fill_super (struct super_block *sb, void
> *opts, int silent)
> if (the_device)
> return -ESRCH;
>
> - /* fake probe to determine $CHIP */
> - CHIP = NULL;
> - usb_gadget_probe_driver(&probe_driver);
> - if (!CHIP)
> + if (usb_get_gadget_udc_name(CHIP, sizeof(CHIP)) != 0)
> return -ENODEV;
>
> /* superblock */
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index fd73a3ea07c2..4bde2e110e44 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -442,6 +442,39 @@ err1:
> EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
>
> /**
> + * usb_get_gadget_udc_name - get the name of the first UDC controller
> + * @dst_name
> + * @len
gotta describe these and add a blank line after @len too.
This also seems a little too big for -rc.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH 2/5] usb: musb: core: added helper functions for parsing DT
Hi, Petr Kulhavy writes: > On 17.02.2016 15:58, Felipe Balbi wrote: >> Hi, >> >> Petr Kulhavy writes: >>> This adds two functions to get DT properties "mentor,power" and "dr_mode": >>> musb_get_power() and musb_mode musb_get_mode() >>> >>> Signed-off-by: Petr Kulhavy >> seems like I don't have patch 1/5. After fixing Sergei's comments, >> please resend with his Acked-by already in place. >> >> thanks > Hi Felipe, > > I will do as soon as the patch 1/5 gets approved. > It seem to be a bit stuck at the moment as Rob Herring from the DT wants > the "mentor,power" > to be represented as a regulator, whereas Sergei and me want to stick to > the existing "mentor,power" integer property. > > As soon as this get clarified I will do the final updates and send the > patch again. > Maybe this is something you can help to clarify? I don't think that makes sense as a regulator. It's just a number which gets passed to USB Core as is. However, it seems like everything in kernel right now is passing it as 500. So why don't you deprecate that property, hardcode it to 500 and avoid the problem altogether ? The likelihood of new MUSB designs is rather low, anyway. -- balbi signature.asc Description: PGP signature
[PATCH] usb: storage: remove the US_DEBUG macro
Get rid of the US_DEBUG macro and use instead empty inline function definitions
when CONFIG_USB_STORAGE_DEBUG is not defined
Signed-off-by: Victor Dodon
---
drivers/usb/storage/debug.h | 18 +++---
drivers/usb/storage/ene_ub6250.c | 1 -
drivers/usb/storage/freecom.c| 12
drivers/usb/storage/transport.c | 2 --
drivers/usb/storage/usb.c| 2 +-
5 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/storage/debug.h b/drivers/usb/storage/debug.h
index 6b365ce..50070f0 100644
--- a/drivers/usb/storage/debug.h
+++ b/drivers/usb/storage/debug.h
@@ -52,9 +52,22 @@ void usb_stor_show_sense(const struct us_data *us, unsigned
char key,
unsigned char asc, unsigned char ascq);
__printf(2, 3) void usb_stor_dbg(const struct us_data *us,
const char *fmt, ...);
-
-#define US_DEBUG(x)x
#else
+static inline void _usb_stor_show_command(const struct us_data *us,
+ struct scsi_cmnd *srb)
+{
+}
+#define usb_stor_show_command(us, srb) \
+ do { if (0) _usb_stor_show_command(us, srb); } while (0)
+
+static inline void _usb_stor_show_sense(const struct us_data *us,
+ unsigned char key, unsigned char asc,
+ unsigned char ascq)
+{
+}
+#define usb_stor_show_sense(us, key, asc, ascq)\
+ do { if (0) _usb_stor_show_sense(us, key, asc, ascq); } while 0
+
__printf(2, 3)
static inline void _usb_stor_dbg(const struct us_data *us,
const char *fmt, ...)
@@ -62,7 +75,6 @@ static inline void _usb_stor_dbg(const struct us_data *us,
}
#define usb_stor_dbg(us, fmt, ...) \
do { if (0) _usb_stor_dbg(us, fmt, ##__VA_ARGS__); } while (0)
-#define US_DEBUG(x)
#endif
#endif
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index f3cf4ce..73cfd84 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -2296,7 +2296,6 @@ static int ene_transport(struct scsi_cmnd *srb, struct
us_data *us)
int result = 0;
struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);
- /*US_DEBUG(usb_stor_show_command(us, srb)); */
scsi_set_resid(srb, 0);
if (unlikely(!(info->SD_Status.Ready || info->MS_Status.Ready))) {
result = ene_init(us);
diff --git a/drivers/usb/storage/freecom.c b/drivers/usb/storage/freecom.c
index 3f2b089..9c37175 100644
--- a/drivers/usb/storage/freecom.c
+++ b/drivers/usb/storage/freecom.c
@@ -44,6 +44,10 @@ MODULE_LICENSE("GPL");
#ifdef CONFIG_USB_STORAGE_DEBUG
static void pdump(struct us_data *us, void *ibuffer, int length);
+#else
+static inline void pdump(struct us_data *us, void *ibuffer, int length)
+{
+}
#endif
/* Bits of HD_STATUS */
@@ -245,7 +249,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct
us_data *us)
memcpy (fcb->Atapi, srb->cmnd, 12);
memset (fcb->Filler, 0, sizeof (fcb->Filler));
- US_DEBUG(pdump(us, srb->cmnd, 12));
+ pdump(us, srb->cmnd, 12);
/* Send it out. */
result = usb_stor_bulk_transfer_buf (us, opipe, fcb,
@@ -267,7 +271,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct
us_data *us)
if (result != USB_STOR_XFER_GOOD)
return USB_STOR_TRANSPORT_ERROR;
- US_DEBUG(pdump(us, (void *)fst, partial));
+ pdump(us, (void *)fst, partial);
/* The firmware will time-out commands after 20 seconds. Some commands
* can legitimately take longer than this, so we use a different
@@ -308,7 +312,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct
us_data *us)
if (result != USB_STOR_XFER_GOOD)
return USB_STOR_TRANSPORT_ERROR;
- US_DEBUG(pdump(us, (void *)fst, partial));
+ pdump(us, (void *)fst, partial);
}
if (partial != 4)
@@ -365,7 +369,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct
us_data *us)
usb_stor_dbg(us, "Waiting for status\n");
result = usb_stor_bulk_transfer_buf (us, ipipe, fst,
FCM_PACKET_LENGTH, &partial);
- US_DEBUG(pdump(us, (void *)fst, partial));
+ pdump(us, (void *)fst, partial);
if (partial != 4 || result > USB_STOR_XFER_SHORT)
return USB_STOR_TRANSPORT_ERROR;
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 5e67f63..2f883a8 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -784,9 +784,7 @@ Retry_Sense:
usb_stor_dbg(us, "-- code: 0x%x, key: 0x%x, ASC: 0x%x, ASCQ:
0x%x\n",
sshdr.response_code, sshdr.sens
Re: [PATCH 1/3] usb: USB Type-C Connector Class
Hi Oliver, On Wed, Feb 17, 2016 at 03:07:27PM +0100, Oliver Neukum wrote: > On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote: > > 1. connected - Connection status of the connector > > 2. alternate_mode - The current Alternate Mode > > 3. alternate_modes - Lists all Alternate Modes the connector supports > > 4. partner_alt_modes - Lists partner's Alternate Modes when connected > > Now that I think about it, there's a gap. > Which SVIDs do we expose if we are UFP (slave)? In the alternate_modes listing the connectors alt modes, we can not have modes that the hardware can not support of course, and it is the responsibility of the drivers registering the type-c ports with this clss to make sure they are not part of the list. In partner alternate modes, we will list all alternate modes the partner supports, even the ones our connector doesn't. The modes that can actually be selected have to be supported by both the connector and the partner, and this is where I'm putting the ball on the userspace at the moment. I'm not offering a list of "possible_alternate_modes" where I list the combination, but instead expect the userspace to be figure out that on it's own. Do you think we should add "possible_alternate_modes" file? P.S. That reminds me, here's my current draft for the Documentation/ABI/. Could you take a look? Thanks, -- heikki What: /sys/class/type-c/ Date: February 2016 Contact:Heikki Krogerus Description: USB Type-C Connectors can be used for other purposes than just connecting USB peripherals to USB host. Devices with USB Type-C connector can be of one of the following types: 1. USB - In normal USB use 2. Accessory Mode - Audio or Debug. Special purpose modes for the connector which are defined in the USB Type-C specification. When an Accessory is connected to the connector, it can not be used for anything besides what the Accessory Mode defines, including USB. 3. Alternate Mode - Modes where the connector can be used also for other purposes then just connecting USB peripherals to USB hosts, and which are not defined in USB Type-C Specification. The Alternate Modes depend on USB Power Delivery support as they are controlled with protocol defined in USB Power Delivery specification. There is also only one type of USB Type-C Connector, so the connector will be the same for both host and peripheral. If two systems that are both dual-role capable (can act as both USB Host and USB Device) are connected together, the roles are selected randomly. This class gives the userspace the control over the USB role and the Alternate Mode with USB Type-C connectors. What: /sys/class/type-c/usbcN/ Date: February 2016 Contact:Heikki Krogerus Description: A /sys/class/type-C/usbcN directory is created for each USB Type-C connector in the system. The N is the index of the connector. What: /sys/class/type-c/usbcN/connected Date: February 2016 Contact:Heikki Krogerus Description: Connection status of the USB Type-C connector usbcN. "yes" when connected, otherwise "no". What: /sys/class/type-c/usbcN/current_alternate_mode Date: February 2016 Contact:Heikki Krogerus Description: When connected to an Alternate Mode, displays the current Mode ID and the Standard of Vendor ID (SVID) of the Alternate Mode in form: , This file is writable and can be used to enter other modes both the connector usbcN and the partner support. The modes are Alternate Mode specific, and before they are to be changed with this file, the exact details of the modes under the given SVID should be known by the user. What: /sys/class/type-c/usbcN/current_data_role Date: February 2016 Contact:Heikki Krogerus Description: The current USB data role, host or device. This file can be used by the userspace to execute data role swap by writing the requested role to it. When the connector is not connected, the file is used to store the preselected role which the system will attempt to use next
[PATCH v2] usb: usbtmc: Fix disconnect/poll interaction
When the device is disconnected poll waiters were not being woken.
Changes for v2:
- add commit summary
- add Fixes and Reported-by tags
Fixes: eb6b92ecc0f9 ("Add support for receiving USBTMC USB488 SRQ notifications
via poll/select")
Reported-by: Oliver Neukum
Signed-off-by: Dave Penkler
---
drivers/usb/class/usbtmc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 419c72e..917a55c 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1525,13 +1525,14 @@ static void usbtmc_disconnect(struct usb_interface
*intf)
dev_dbg(&intf->dev, "usbtmc_disconnect called\n");
data = usb_get_intfdata(intf);
- usbtmc_free_int(data);
usb_deregister_dev(intf, &usbtmc_class);
sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
mutex_lock(&data->io_mutex);
data->zombie = 1;
+ wake_up_all(&data->waitq);
mutex_unlock(&data->io_mutex);
+ usbtmc_free_int(data);
kref_put(&data->kref, usbtmc_delete);
}
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 17, 2016 at 04:28:16PM +0200, Heikki Krogerus wrote: > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: > > > > Hi, > > > > > IIRC mode and role negotiation goes via CC pins using the power delivery > > protocol. If I misunderstand anything, let me know. > > The data role swap with USB Type-C connectors is in no way tied to USB > Power Delivery. The USB Type-C spec defines that when USB PD is > available, DR_Swap USB PD function is used to swap the role, otherwise > emulated disconnect will do the trick. > I am interested in how you design role swap on the fly without USB PD. Do you follow the spec like USB OTG 3.0 RSP (Role Swap Protocol) or just echo the /sys to give up current role, and swap to another? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: USB Type-C Connector Class
On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote: Hi, > The modes that can actually be selected have to be supported by both > the connector and the partner, and this is where I'm putting the ball > on the userspace at the moment. I'm not offering a list of > "possible_alternate_modes" where I list the combination, but instead > expect the userspace to be figure out that on it's own. > > Do you think we should add "possible_alternate_modes" file? No, what do we answer to the DFP if we recieve "Discover SVIDs"? I don't think that we always should answer with all we physically can. If, for example, the hardware could do Thunderbolt, but the OS is not prepared to handle it, we shouldn't offer it. So this is a policy decision to be made in user space. Hence we need an API to tell it to the kernel. > P.S. That reminds me, here's my current draft for the > Documentation/ABI/. Could you take a look? OK Here are my comments: What: /sys/class/type-c/usbcN/connected Connection status of the USB Type-C connector usbcN. "yes" when connected, otherwise "no". Unnecessarily wordy. 0 and 1 would do What: /sys/class/type-c/usbcN/current_data_role Again, 0 and 1 would do What: /sys/class/type-c/usbcN/partner_alternate_modes You should say in which number base the values are given. What: /sys/class/type-c/usbcN/partner_type That could be combined with "connected" What: /sys/class/type-c/usbcN/supported_data_roles A connector can be both. How is that expressed? What: /sys/class/type-c/usbcN/supported_power_roles Again, what if it can do both? -- 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 0/3] usb: USB Type-C Class and driver for UCSI
On Wed, Feb 17, 2016 at 07:53:47PM +0100, Oliver Neukum wrote: > On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote: > > Hi, > > > > The OS, or more precisely the user space, needs to be able to control > > a few things regarding USB Type-C ports. The first thing that must be > > allowed to be controlled is the data role. USB Type-C ports will > > select the data role randomly with DRP ports. When USB PD is > > supported, also independent (from data role) power role swapping can > > be supported together with Alternate Mode control. > > What about S4? We need to restore the alternate mode upon resume, > if we are the DFP and as that might involve storage devices it > needs to be done in kernel space. That I'm expecting to be the responsibility of the drivers registering the ports at them moment, because I'm afraid of all the possible different kind of platform specific oddities that need to be considered. But I guess all we would need to do in the class is store the roles and mode during suspend, and restore them during resuming with dr_swap, pr_swap and set_alt_mode hooks if they changed. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 10, 2016 at 12:30:42PM +0200, Heikki Krogerus wrote: > On Tue, Feb 09, 2016 at 10:21:55AM -0800, Greg KH wrote: > > On Tue, Feb 09, 2016 at 07:01:22PM +0200, Heikki Krogerus wrote: > > > USB Type-C Connector System Software Interface (UCSI) is a > > > specification that defines registers and data structures > > > used to interface with the USB Type-C connectors on a system. > > > > > > The specification is public and available at: > > > http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html > > > > > > > What does this driver / code actually do? Why is it needed? What > > interface to the rest of the kernel / userspace does it provide? > > I will fix this describe these things in the commit message. I'll just > but some UCSI background in case somebody is interested. So UCSI is in > practice a standard for USB Type-C controllers.. > Does this UCSI spec has some similar things with USB Type-C Port Controller Interface Spec at usb.org? If not, how to co-work together in future? Peter > UCSI is the control interface for USB Type-C connectors (regardless > was USB PD supported or not) in MS Windows, so most likely all new HW > platforms designed to work also with Windows that are equipped with > USB Type-C will have UCSI device for controlling the USB Type-C ports. > In some cases the hardware for Type-C will be just a PHY like fusb30x > on these platforms (it's cheaper then USB PD or complete USB Type-C > controller), but in those cases the PHY is probable attached to an EC > or is completely controlled by system FW like BIOS together with any > USB PD communication in cases where USB PD is supported, and is in any > case not visible to the OS. Instead UCSI device is exposed to the OS > to give it means to apply its policies to the USB Type-C port. > > > Why would we care about this? > > I'll try to explain why it's important to export the control of USB > Type-C ports to the user space in my answer to your comments to the > first patch of this series, the one introducing the class. > > But surely everybody agrees that decision about the policies regarding > USB Type-C ports, like which data role to use, do we charge or are we > letting the other end charge, etc., belongs to the user? If you plug > your phone to your desktop, I would imagine that you want to see the > phone primarily as the USB device and the desktop as host, and to > achieve the device role, you don't want to be forced to unplug/replug > your phone from the desktop until you achieve device role, right? > > > You need to describe this a lot better than you did... > > Sure thing. I'm sorry about the poor description. I send these out too > hastily. > > > Thanks, > > -- > heikki > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: USB Type-C Connector Class
On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote: Hi, > P.S. That reminds me, here's my current draft for the > Documentation/ABI/. Could you take a look? And I am afraid, that I have a few remarks not bound to a specific entry. We have port directories for port power switching. How is the connector directory linked to them? Likewise, if we have USB PD, we have to know how that is linked to the connector directory. In addition, writes to those files have results. We need the error codes to be described. Furthermore, do these files support poll? And lastly we can get "Attention" as a message connected with a connector in an alternate mode. How does user space learn about that? I am sorry to be this obnoxious, but this is an API which will be with us for a long time, so we better get it right. HTH Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Thu, 2016-02-18 at 17:29 +0800, Peter Chen wrote: > Does this UCSI spec has some similar things with USB Type-C Port > Controller Interface Spec at usb.org? If not, how to co-work > together in future? USB Type-C Port Controller Interface Spec: What can a type C connector do? UCSI spec: How can the OS use ACPI to tell a type C connector what to do. Obviously UCSI depends on the type C spec, but they cover different things. HTH Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Thu, 2016-02-18 at 09:08 +0200, Felipe Balbi wrote: > Oliver Neukum writes: > >> Oliver Neukum writes: Hi, > > What exactly are you sure about about? > > heh, missed a NOT there :-) I am still confused :-) Do you think a sysfs interface is good, bad or good but insufficient? > >> that, eventually, we will need an actual stack exposed for the CC pin. > > > > The raw CC pin? What about the timing requirement? > > well, not the _raw_ CC pin, but a situation where the microcontroller > handling CC pin is "dumb", in the sense that it provides an interface > for us to request/start arbitrary transactions. That will, in turn, > shift the actual bits on the CC pin. Or something along these lines. Well, how else would we send vendor specific messages? > An example would be the alternate mode thing. CC microcontroller doesn't > have to know what are the available alternate modes, but it needs to be > able to tell processor what request is coming from the other end. Indeed. > I guess what I'm trying to say is that CC microcontroller might not be > the one controlling the multiplexer which switches USB pins to another > function. IOW: > > Change to alternate mode X message > CC microcontroller interrupts CPU > read status to get X >change multiplexer > Yes. But it seems to me that in this case we need a kernel driver without an API to user space. There are necessarily internal users of the PD controller. There are also external users. So the CC pins should be seen as a bus, which in essence they are (it reminds me of ancient ethernet actually), and if you really want full user space access, you'd need the quivalent of an sg driver. The issue is orthogonal to the question how we support UCSI, except that UCSI is a user of the CC pins and PD and frankly I don't see the firmware and a driver access this sanely simultaneously. Therefore I'd prefer we make an API here which does not depend on UCSI, but can, if necessary, work on top of a driver doing full hardware access. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
Hi, Oliver Neukum writes: >> > What exactly are you sure about about? >> >> heh, missed a NOT there :-) > > I am still confused :-) > Do you think a sysfs interface is good, bad or good > but insufficient? I'm not sure it's the best interface. My fear is that as new requirements/features come along, the amount of files will continue to grow. >> I guess what I'm trying to say is that CC microcontroller might not be >> the one controlling the multiplexer which switches USB pins to another >> function. IOW: >> >> Change to alternate mode X message >> CC microcontroller interrupts CPU >> read status to get X >>change multiplexer >> > > Yes. But it seems to me that in this case we need a kernel driver > without an API to user space. There are necessarily internal users that assumes kernel always knows all possible alternate modes. What do we about bogus requests (request alternate mode X when X is not supported) ? > of the PD controller. There are also external users. So the CC pins > should be seen as a bus, which in essence they are (it reminds me > of ancient ethernet actually), and if you really want full user > space access, you'd need the quivalent of an sg driver. > > The issue is orthogonal to the question how we support UCSI, except > that UCSI is a user of the CC pins and PD and frankly I don't see the > firmware and a driver access this sanely simultaneously. Therefore > I'd prefer we make an API here which does not depend on UCSI, but can, > if necessary, work on top of a driver doing full hardware access. fair enough. -- balbi signature.asc Description: PGP signature
[PATCH v3] usb: gadget: provide interface for legacy gadgets to get UDC name
Since commit 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb: gadget:
udc-core: independent registration of gadgets and gadget drivers") gadget
drivers can not assume that UDC drivers are already available on their
initialization. This broke the HACK, which was used in gadgetfs driver,
to get UDC controller name. This patch removes this hack and replaces it
by additional function in the UDC core (which is usefully only for legacy
drivers, please don't use it in the new code).
Reported-by: Vegard Nossum
Signed-off-by: Marek Szyprowski
Tested-by: Vegard Nossum
---
changelog:
v3:
- simplified code a bit
v2: https://lkml.org/lkml/2016/2/8/234
- properly report udc gagdet driver name instead of udc device name
v1: https://lkml.org/lkml/2016/2/8/176
- initial version
---
drivers/usb/gadget/legacy/inode.c | 28 +++-
drivers/usb/gadget/udc/udc-core.c | 30 ++
include/linux/usb/gadget.h| 1 +
3 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index 87fb0fd..5cdaf01 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -1699,28 +1699,6 @@ static struct usb_gadget_driver gadgetfs_driver = {
};
/*--*/
-
-static void gadgetfs_nop(struct usb_gadget *arg) { }
-
-static int gadgetfs_probe(struct usb_gadget *gadget,
- struct usb_gadget_driver *driver)
-{
- CHIP = gadget->name;
- return -EISNAM;
-}
-
-static struct usb_gadget_driver probe_driver = {
- .max_speed = USB_SPEED_HIGH,
- .bind = gadgetfs_probe,
- .unbind = gadgetfs_nop,
- .setup = (void *)gadgetfs_nop,
- .disconnect = gadgetfs_nop,
- .driver = {
- .name = "nop",
- },
-};
-
-
/* DEVICE INITIALIZATION
*
* fd = open ("/dev/gadget/$CHIP", O_RDWR)
@@ -1971,9 +1949,7 @@ gadgetfs_fill_super (struct super_block *sb, void *opts,
int silent)
if (the_device)
return -ESRCH;
- /* fake probe to determine $CHIP */
- CHIP = NULL;
- usb_gadget_probe_driver(&probe_driver);
+ CHIP = usb_get_gadget_udc_name();
if (!CHIP)
return -ENODEV;
@@ -2034,6 +2010,8 @@ gadgetfs_kill_sb (struct super_block *sb)
put_dev (the_device);
the_device = NULL;
}
+ kfree(CHIP);
+ CHIP = NULL;
}
/*--*/
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index fd73a3e..fd30242 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -442,6 +442,36 @@ err1:
EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
/**
+ * usb_get_gadget_udc_name - get the name of the first UDC controller
+ * This functions returns the name of the first UDC controller in the system.
+ * Please note that this interface is usefull only for legacy drivers which
+ * assume that there is only one UDC controller in the system and they need to
+ * get its name before initialization. There is no guarantee that the UDC
+ * of the returned name will be still available, when gadget driver registers
+ * itself.
+ *
+ * Returns pointer to string with UDC controller name on success, NULL
+ * otherwise. Caller should kfree() returned string.
+ */
+char *usb_get_gadget_udc_name(void)
+{
+ struct usb_udc *udc;
+ char *name = NULL;
+
+ /* For now we take the first available UDC */
+ mutex_lock(&udc_lock);
+ list_for_each_entry(udc, &udc_list, list) {
+ if (!udc->driver) {
+ name = kstrdup(udc->gadget->name, GFP_KERNEL);
+ break;
+ }
+ }
+ mutex_unlock(&udc_lock);
+ return name;
+}
+EXPORT_SYMBOL_GPL(usb_get_gadget_udc_name);
+
+/**
* usb_add_gadget_udc - adds a new gadget to the udc class driver list
* @parent: the parent device to this udc. Usually the controller
* driver's device.
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..547d86d 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1126,6 +1126,7 @@ extern int usb_add_gadget_udc_release(struct device
*parent,
struct usb_gadget *gadget, void (*release)(struct device *dev));
extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget
*gadget);
extern void usb_del_gadget_udc(struct usb_gadget *gadget);
+extern char *usb_get_gadget_udc_name(void);
/*-*/
--
1.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus wrote: > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: >> >> Hi, >> >> Heikki Krogerus writes: >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote: >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote: >> >> > Hi, >> >> > >> >> > Oliver Neukum writes: >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote: >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote: >> >> >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that >> >> > >> > is to be supported, it needs to be defined now. >> >> > >> >> >> > >> When you say API, do you mean the API the class provides to the >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this case? >> >> > > >> >> > > The API to user space. That is the point. We cannot break user space. >> >> > > Once this sysfs API is upstream we are stuck with it. >> >> > >> >> > yeah, in fact I have been wondering if sysfs is the best interface to >> >> >> >> That is the discussion we must have. >> >> >> >> > userspace. I talked with Heikki a few days back about this; I was >> >> > wondering if something like what the NFC folks did with netlink would be >> >> > better here. >> >> >> >> I doubt that, because the main user is likely to be udev scripts. >> >> They can easily deal with sysfs attributes. >> > >> > IMHO for high level interface like this, sysfs is ideal because of the >> > simple fact that you only need a shell to access the files. netlink >> > would make us depend on custom software, no? >> > >> > I'm not against using netlink, but what would be the benefit from it >> > in this case? >> >> With HW we see nowadays, CC stack is hidden on some microcontroller, but >> is it too far-fetched to consider a system where this is not the case ? > > There already are several USB PD stacks out there, like also Greg > pointed out. > >> Specially when we consider things like power delivery which, I know, you >> wanted to keep it out of this interface, however we would have two >> 'stacks' competing for access to the same pins, right ? > > No. This class would be the top layer for the coming stack, where ever > it ends up coming. The class is only the interface to the user space > and nothing else. > > By saying we need to keep USB Type-C separate from USB PD I meant that > the userspace access can not be mixed somewhere in layers of the USB > PD/CC stack like it has been in the USB PD stacks I've seen so far. > They assume that we always use the software USB PD stack with USB > Type-C, which as we can see is not true when the stack is implemented > in EC or firmware or some complex USB PD controller or what ever. > However, the operations the userspace needs to do are exactly the same > in both cases. > > - data role swapping > - power role swapping (depends on USB PD) > - Alternate Modes (depends on USB PD) > > And we really should not forget that we actually also have USB Type-C > PHYs that can't do any USB PD communication over the CC pin, so USB PD > is simply not always going to be available. But the data role swapping > and also accessories are still available with them, as the do not need > USB PD. > > This was the whole point with the class. It allows the different ways > of dealing with Type-C ports to be exposed to userspace in the same > way. > >> IIRC mode and role negotiation goes via CC pins using the power delivery >> protocol. If I misunderstand anything, let me know. > > The data role swap with USB Type-C connectors is in no way tied to USB > Power Delivery. The USB Type-C spec defines that when USB PD is Its not data role swap i guess its dual role, A Data role swap is tied with USB PD, > available, DR_Swap USB PD function is used to swap the role, otherwise > emulated disconnect will do the trick. I doubt a USB host with no device capability implement DRP ?? Also emulated trick(??) is not spec requirement rt ? > > Data role swapping is a must thing to have with USB Type-C connectors I guess you are referring to Dual role (DRP) and not data role (DRD). > because of the fact that the role is selected randomly. Regardless was > USB PD supported or not. > > > Thanks, > > -- > heikki > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Thu, 2016-02-18 at 12:30 +0200, Felipe Balbi wrote: > Hi, > > Oliver Neukum writes: > >> > What exactly are you sure about about? > >> > >> heh, missed a NOT there :-) > > > > I am still confused :-) > > Do you think a sysfs interface is good, bad or good > > but insufficient? > > I'm not sure it's the best interface. My fear is that as new > requirements/features come along, the amount of files will continue to > grow. That will happen. The alternative, however is a "typectool" or "usbpdtool" which would need to be updated for new features. > >> I guess what I'm trying to say is that CC microcontroller might not be > >> the one controlling the multiplexer which switches USB pins to another > >> function. IOW: > >> > >> Change to alternate mode X message > >> CC microcontroller interrupts CPU > >> read status to get X > >>change multiplexer > >> > > > > Yes. But it seems to me that in this case we need a kernel driver > > without an API to user space. There are necessarily internal users > > that assumes kernel always knows all possible alternate modes. What do > we about bogus requests (request alternate mode X when X is not > supported) ? Do as the spec says: NACK it. The questions which modes we offer, if we are a slave, still remains. And I think the API is deficient in that regard. But again that question is orthogonal of both issue, handling of bogus requests and how the CC pins are exported. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
Hi Peter, On Thu, Feb 18, 2016 at 05:07:04PM +0800, Peter Chen wrote: > On Wed, Feb 17, 2016 at 04:28:16PM +0200, Heikki Krogerus wrote: > > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: > > > IIRC mode and role negotiation goes via CC pins using the power delivery > > > protocol. If I misunderstand anything, let me know. > > > > The data role swap with USB Type-C connectors is in no way tied to USB > > Power Delivery. The USB Type-C spec defines that when USB PD is > > available, DR_Swap USB PD function is used to swap the role, otherwise > > emulated disconnect will do the trick. > > I am interested in how you design role swap on the fly without USB PD. > Do you follow the spec like USB OTG 3.0 RSP (Role Swap Protocol) or > just echo the /sys to give up current role, and swap to another? No OTG with USB Type-C. You echo the wanted role to the /sys/class/type-C/usbcN/data_role. This operation from userspace is the same regardless was USB PD supported or not. The actual operations needed for the role swap are of course platform specific, and the responsibility of the drivers that register the ports with type-c class. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Thu, Feb 18, 2016 at 04:07:54PM +0530, Rajaram R wrote: > On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus > wrote: > > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: > >> > >> Hi, > >> > >> Heikki Krogerus writes: > >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote: > >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote: > >> >> > Hi, > >> >> > > >> >> > Oliver Neukum writes: > >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote: > >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote: > >> >> > >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that > >> >> > >> > is to be supported, it needs to be defined now. > >> >> > >> > >> >> > >> When you say API, do you mean the API the class provides to the > >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this case? > >> >> > > > >> >> > > The API to user space. That is the point. We cannot break user > >> >> > > space. > >> >> > > Once this sysfs API is upstream we are stuck with it. > >> >> > > >> >> > yeah, in fact I have been wondering if sysfs is the best interface to > >> >> > >> >> That is the discussion we must have. > >> >> > >> >> > userspace. I talked with Heikki a few days back about this; I was > >> >> > wondering if something like what the NFC folks did with netlink would > >> >> > be > >> >> > better here. > >> >> > >> >> I doubt that, because the main user is likely to be udev scripts. > >> >> They can easily deal with sysfs attributes. > >> > > >> > IMHO for high level interface like this, sysfs is ideal because of the > >> > simple fact that you only need a shell to access the files. netlink > >> > would make us depend on custom software, no? > >> > > >> > I'm not against using netlink, but what would be the benefit from it > >> > in this case? > >> > >> With HW we see nowadays, CC stack is hidden on some microcontroller, but > >> is it too far-fetched to consider a system where this is not the case ? > > > > There already are several USB PD stacks out there, like also Greg > > pointed out. > > > >> Specially when we consider things like power delivery which, I know, you > >> wanted to keep it out of this interface, however we would have two > >> 'stacks' competing for access to the same pins, right ? > > > > No. This class would be the top layer for the coming stack, where ever > > it ends up coming. The class is only the interface to the user space > > and nothing else. > > > > By saying we need to keep USB Type-C separate from USB PD I meant that > > the userspace access can not be mixed somewhere in layers of the USB > > PD/CC stack like it has been in the USB PD stacks I've seen so far. > > They assume that we always use the software USB PD stack with USB > > Type-C, which as we can see is not true when the stack is implemented > > in EC or firmware or some complex USB PD controller or what ever. > > However, the operations the userspace needs to do are exactly the same > > in both cases. > > > > - data role swapping > > - power role swapping (depends on USB PD) > > - Alternate Modes (depends on USB PD) > > > > And we really should not forget that we actually also have USB Type-C > > PHYs that can't do any USB PD communication over the CC pin, so USB PD > > is simply not always going to be available. But the data role swapping > > and also accessories are still available with them, as the do not need > > USB PD. > > > > This was the whole point with the class. It allows the different ways > > of dealing with Type-C ports to be exposed to userspace in the same > > way. > > > >> IIRC mode and role negotiation goes via CC pins using the power delivery > >> protocol. If I misunderstand anything, let me know. > > > > The data role swap with USB Type-C connectors is in no way tied to USB > > Power Delivery. The USB Type-C spec defines that when USB PD is > > Its not data role swap i guess its dual role, A Data role swap is tied > with USB PD, > > > available, DR_Swap USB PD function is used to swap the role, otherwise > > emulated disconnect will do the trick. > > I doubt a USB host with no device capability implement DRP ?? Also > emulated trick(??) is not spec requirement rt ? > > > > > Data role swapping is a must thing to have with USB Type-C connectors > > I guess you are referring to Dual role (DRP) and not data role (DRD). There is no term "DRD" in USB Type-C spec. A quote from Type-C spec ch. 2.3.3: "Two methods are defined to allow a USB Type-C DRP to functionally swap data roles, one managed using USB PD DR_Swap and the other emulating a disconnect/reconnect sequence (see Figure 4-16)" Thanks, -- heikki -- 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 0/3] usb: USB Type-C Class and driver for UCSI
Hi Rajaram, On Thu, Feb 18, 2016 at 01:04:48AM +0530, Rajaram R wrote: > On Tue, Feb 9, 2016 at 10:31 PM, Heikki Krogerus > wrote: > > Hi, > > > > The OS, or more precisely the user space, needs to be able to control > > a few things regarding USB Type-C ports. The first thing that must be > > allowed to be controlled is the data role. USB Type-C ports will > > select the data role randomly with DRP ports. When USB PD is > > supported, also independent (from data role) power role swapping can > > be supported together with Alternate Mode control. > > > > I'm proposing with this set a Class for the Type-C connectors that > > gives the user space control over those things on top of getting basic > > details about the USB Type-C connectors and also partners. The details > > include the capabilities of the port, the supported data and power > > roles, supported accessories (audio and debug), supported Alternate > > Modes, USB PD support and of course the type of the partner (USB, Alt > > Mode, Accessory or Charger), and more or less the same details about > > the partner. > > > > I'm not considering cables with this Class, and I have deliberately > > Since we have capability details of ports in user space, I believe > cable capability is also necessary for policy decision(power, alt > mode). Is that something we are cautiously leaving out ? pls explain Adding the cable control to this interface will make it more complex from users perspective. However, nothing forces the user to control also the cable. I already decided to add vconn_swap support, so I'll try to add cable alt mode control and capabilities as well. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
On Thu, Feb 18, 2016 at 4:17 PM, Heikki Krogerus wrote: > On Thu, Feb 18, 2016 at 04:07:54PM +0530, Rajaram R wrote: >> On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus >> wrote: >> > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote: >> >> >> >> Hi, >> >> >> >> Heikki Krogerus writes: >> >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote: >> >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote: >> >> >> > Hi, >> >> >> > >> >> >> > Oliver Neukum writes: >> >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote: >> >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote: >> >> >> >> >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that >> >> >> > >> > is to be supported, it needs to be defined now. >> >> >> > >> >> >> >> > >> When you say API, do you mean the API the class provides to the >> >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this >> >> >> > >> case? >> >> >> > > >> >> >> > > The API to user space. That is the point. We cannot break user >> >> >> > > space. >> >> >> > > Once this sysfs API is upstream we are stuck with it. >> >> >> > >> >> >> > yeah, in fact I have been wondering if sysfs is the best interface to >> >> >> >> >> >> That is the discussion we must have. >> >> >> >> >> >> > userspace. I talked with Heikki a few days back about this; I was >> >> >> > wondering if something like what the NFC folks did with netlink >> >> >> > would be >> >> >> > better here. >> >> >> >> >> >> I doubt that, because the main user is likely to be udev scripts. >> >> >> They can easily deal with sysfs attributes. >> >> > >> >> > IMHO for high level interface like this, sysfs is ideal because of the >> >> > simple fact that you only need a shell to access the files. netlink >> >> > would make us depend on custom software, no? >> >> > >> >> > I'm not against using netlink, but what would be the benefit from it >> >> > in this case? >> >> >> >> With HW we see nowadays, CC stack is hidden on some microcontroller, but >> >> is it too far-fetched to consider a system where this is not the case ? >> > >> > There already are several USB PD stacks out there, like also Greg >> > pointed out. >> > >> >> Specially when we consider things like power delivery which, I know, you >> >> wanted to keep it out of this interface, however we would have two >> >> 'stacks' competing for access to the same pins, right ? >> > >> > No. This class would be the top layer for the coming stack, where ever >> > it ends up coming. The class is only the interface to the user space >> > and nothing else. >> > >> > By saying we need to keep USB Type-C separate from USB PD I meant that >> > the userspace access can not be mixed somewhere in layers of the USB >> > PD/CC stack like it has been in the USB PD stacks I've seen so far. >> > They assume that we always use the software USB PD stack with USB >> > Type-C, which as we can see is not true when the stack is implemented >> > in EC or firmware or some complex USB PD controller or what ever. >> > However, the operations the userspace needs to do are exactly the same >> > in both cases. >> > >> > - data role swapping >> > - power role swapping (depends on USB PD) >> > - Alternate Modes (depends on USB PD) >> > >> > And we really should not forget that we actually also have USB Type-C >> > PHYs that can't do any USB PD communication over the CC pin, so USB PD >> > is simply not always going to be available. But the data role swapping >> > and also accessories are still available with them, as the do not need >> > USB PD. >> > >> > This was the whole point with the class. It allows the different ways >> > of dealing with Type-C ports to be exposed to userspace in the same >> > way. >> > >> >> IIRC mode and role negotiation goes via CC pins using the power delivery >> >> protocol. If I misunderstand anything, let me know. >> > >> > The data role swap with USB Type-C connectors is in no way tied to USB >> > Power Delivery. The USB Type-C spec defines that when USB PD is >> >> Its not data role swap i guess its dual role, A Data role swap is tied >> with USB PD, >> >> > available, DR_Swap USB PD function is used to swap the role, otherwise >> > emulated disconnect will do the trick. >> >> I doubt a USB host with no device capability implement DRP ?? Also >> emulated trick(??) is not spec requirement rt ? >> >> > >> > Data role swapping is a must thing to have with USB Type-C connectors >> >> I guess you are referring to Dual role (DRP) and not data role (DRD). > > There is no term "DRD" in USB Type-C spec. A quote from Type-C spec Yes, not in Spec 1.1 but a new term to differentiate data and power roles . All I wanted to bring in some difference between data role swap and DRP > ch. 2.3.3: > > "Two methods are defined to allow a USB Type-C DRP to functionally > swap data roles, one managed using USB PD DR_Swap and the other > emulating a disconnect/reconnect sequence (see Figure 4-16
Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI
On Thu, 2016-02-18 at 13:05 +0200, Heikki Krogerus wrote: > > Since we have capability details of ports in user space, I believe > > cable capability is also necessary for policy decision(power, alt > > mode). Is that something we are cautiously leaving out ? pls explain > > Adding the cable control to this interface will make it more complex > from users perspective. However, nothing forces the user to control > also the cable. But we would like to indicate to the user that we cannot run an alternate mode because the cable is incapable as opposed to the device. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data
On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote: > > Hi, > > Heikki Krogerus writes: > > This should allow the core driver to drop handling of > > platform data and expect the platform specific details to > > always come from properties. > > > > Signed-off-by: Heikki Krogerus > > Cc: Huang Rui > > CC: John Youn > > doesn't compile dude: > > make -k -j16 -- drivers/usb/dwc3/ > CHK include/config/kernel.release > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > CHK include/generated/timeconst.h > CHK include/generated/bounds.h > CHK include/generated/asm-offsets.h > CALLscripts/checksyscalls.sh > CC [M] drivers/usb/dwc3/dwc3-pci.o > CC [M] drivers/usb/dwc3/dwc3-keystone.o > CC [M] drivers/usb/dwc3/dwc3-of-simple.o > drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’: > drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of function > ‘platform_device_add_properties’ [-Werror=implicit-function-declaration] > return platform_device_add_properties(dwc3, &pset); Which kernel are you using? platform_device_add_properties() is available from v4.5-rc1 onwards. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data
Heikki Krogerus writes: > On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote: >> >> Hi, >> >> Heikki Krogerus writes: >> > This should allow the core driver to drop handling of >> > platform data and expect the platform specific details to >> > always come from properties. >> > >> > Signed-off-by: Heikki Krogerus >> > Cc: Huang Rui >> > CC: John Youn >> >> doesn't compile dude: >> >> make -k -j16 -- drivers/usb/dwc3/ >> CHK include/config/kernel.release >> CHK include/generated/uapi/linux/version.h >> CHK include/generated/utsrelease.h >> CHK include/generated/timeconst.h >> CHK include/generated/bounds.h >> CHK include/generated/asm-offsets.h >> CALLscripts/checksyscalls.sh >> CC [M] drivers/usb/dwc3/dwc3-pci.o >> CC [M] drivers/usb/dwc3/dwc3-keystone.o >> CC [M] drivers/usb/dwc3/dwc3-of-simple.o >> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’: >> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of function >> ‘platform_device_add_properties’ [-Werror=implicit-function-declaration] >> return platform_device_add_properties(dwc3, &pset); > > Which kernel are you using? platform_device_add_properties() is > available from v4.5-rc1 onwards. v4.5-rc4, could it be you missed a header or the header misses a stub ? -- balbi signature.asc Description: PGP signature
Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data
On Thu, Feb 18, 2016 at 01:27:28PM +0200, Felipe Balbi wrote: > Heikki Krogerus writes: > > > On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote: > >> > >> Hi, > >> > >> Heikki Krogerus writes: > >> > This should allow the core driver to drop handling of > >> > platform data and expect the platform specific details to > >> > always come from properties. > >> > > >> > Signed-off-by: Heikki Krogerus > >> > Cc: Huang Rui > >> > CC: John Youn > >> > >> doesn't compile dude: > >> > >> make -k -j16 -- drivers/usb/dwc3/ > >> CHK include/config/kernel.release > >> CHK include/generated/uapi/linux/version.h > >> CHK include/generated/utsrelease.h > >> CHK include/generated/timeconst.h > >> CHK include/generated/bounds.h > >> CHK include/generated/asm-offsets.h > >> CALLscripts/checksyscalls.sh > >> CC [M] drivers/usb/dwc3/dwc3-pci.o > >> CC [M] drivers/usb/dwc3/dwc3-keystone.o > >> CC [M] drivers/usb/dwc3/dwc3-of-simple.o > >> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’: > >> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of function > >> ‘platform_device_add_properties’ [-Werror=implicit-function-declaration] > >> return platform_device_add_properties(dwc3, &pset); > > > > Which kernel are you using? platform_device_add_properties() is > > available from v4.5-rc1 onwards. > > v4.5-rc4, could it be you missed a header or the header misses a stub ? Both the platform bus and property.c are always build in? Adding Mika and Andy. Guys, do you have ideas? Thanks, -- heikki -- 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 v7 04/10] usb: dbc: add debug buffer
On 26.01.2016 14:58, Lu Baolu wrote:
"printk" is not suitable for dbc debugging especially when console
is in usage. This patch adds a debug buffer in dbc driver and puts
the debug messages in this local buffer. The debug buffer could be
dumped whenever the console is not in use. This part of code will
not be visible unless DBC_DEBUG is defined.
Signed-off-by: Lu Baolu
---
drivers/usb/early/xhci-dbc.c | 62 ++--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index 41ce116..6855048 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -32,8 +32,64 @@ static struct xdbc_state xdbc_stat;
static struct xdbc_state *xdbcp = &xdbc_stat;
#ifdef DBC_DEBUG
-/* place holder */
-#definexdbc_trace printk
+#defineXDBC_DEBUG_BUF_SIZE (PAGE_SIZE * 32)
Does it really need to be this huge? minimum 4096 * 32 ~ 128k
The kernel ring buffer is about the same size (16k - 256k)
+#defineMSG_MAX_LINE128
with 128 characters per line this would fit ~1000 lines
+static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
+static void xdbc_trace(const char *fmt, ...)
+{
+ int i, size;
+ va_list args;
+ static int pos;
+ char temp_buf[MSG_MAX_LINE];
+
+ if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+ return;
+
+ memset(temp_buf, 0, MSG_MAX_LINE);
+ va_start(args, fmt);
+ vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
+ va_end(args);
+
+ i = 0;
+ size = strlen(temp_buf);
+ while (i < size) {
+ xdbc_debug_buf[pos] = temp_buf[i];
+ pos++;
+ i++;
+
+ if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+ break;
+ }
how about something like:
size = min(XDBC_DEBUG_BUF_SIZE - pos, size)
memcpy(xdbc_debug_buf + pos, temp_buf, size)
pos += size;
(might need some "-1" and off by one checking..)
+}
+
+static void xdbc_dump_debug_buffer(void)
+{
+ int index = 0;
+ int count = 0;
+ char dump_buf[MSG_MAX_LINE];
+
+ xdbc_trace("The end of DbC trace buffer\n");
+ pr_notice("DBC debug buffer:\n");
+ memset(dump_buf, 0, MSG_MAX_LINE);
+
+ while (index < XDBC_DEBUG_BUF_SIZE) {
+ if (!xdbc_debug_buf[index])
+ break;
+
+ if (xdbc_debug_buf[index] == '\n' ||
+ count >= MSG_MAX_LINE - 1) {
+ pr_notice("DBC: @%08x %s\n", index, dump_buf);
Is showing the he index (position in debug buffer) useful here?
+ memset(dump_buf, 0, MSG_MAX_LINE);
+ count = 0;
+ } else {
+ dump_buf[count] = xdbc_debug_buf[index];
+ count++;
+ }
+
+ index++;
+ }
So we have one huge buffer that xdbc keeps on filling as the initialization
progresses.
It is never emptied, or overwritten (circular).
When dumped it always dumps the whole thing, copying one character
at a time.
As this is only used for debugging during xdbc development/debugging, and never
enabled
even if xdbc early printk is used, I don't think optimization really matters.
Perhaps take a look if we really need PAGE_SIZE * 32 bytes, is xdbc driver even
nearly
writing that much debug data.
-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: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data
On Thu, Feb 18, 2016 at 01:31:33PM +0200, Heikki Krogerus wrote: > On Thu, Feb 18, 2016 at 01:27:28PM +0200, Felipe Balbi wrote: > > Heikki Krogerus writes: > > > > > On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote: > > >> > > >> Hi, > > >> > > >> Heikki Krogerus writes: > > >> > This should allow the core driver to drop handling of > > >> > platform data and expect the platform specific details to > > >> > always come from properties. > > >> > > > >> > Signed-off-by: Heikki Krogerus > > >> > Cc: Huang Rui > > >> > CC: John Youn > > >> > > >> doesn't compile dude: > > >> > > >> make -k -j16 -- drivers/usb/dwc3/ > > >> CHK include/config/kernel.release > > >> CHK include/generated/uapi/linux/version.h > > >> CHK include/generated/utsrelease.h > > >> CHK include/generated/timeconst.h > > >> CHK include/generated/bounds.h > > >> CHK include/generated/asm-offsets.h > > >> CALLscripts/checksyscalls.sh > > >> CC [M] drivers/usb/dwc3/dwc3-pci.o > > >> CC [M] drivers/usb/dwc3/dwc3-keystone.o > > >> CC [M] drivers/usb/dwc3/dwc3-of-simple.o > > >> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’: > > >> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of > > >> function ‘platform_device_add_properties’ > > >> [-Werror=implicit-function-declaration] > > >> return platform_device_add_properties(dwc3, &pset); > > > > > > Which kernel are you using? platform_device_add_properties() is > > > available from v4.5-rc1 onwards. > > > > v4.5-rc4, could it be you missed a header or the header misses a stub ? > > Both the platform bus and property.c are always build in? > > Adding Mika and Andy. Guys, do you have ideas? I would like to reproduce this. Felipe, can you send me your .config? -- 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 v2] USB: option: add support for SIM7100E
On Sun, Jan 31, 2016 at 09:13:19PM +0100, Johan Hovold wrote: > On Fri, Jan 29, 2016 at 12:07:30AM +0300, Andrey Skvortsov wrote: > > $ lsusb: > > Bus 001 Device 101: ID 1e0e:9001 Qualcomm / Option > > > > $ usb-devices: > > T: Bus=01 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=101 Spd=480 MxCh= 0 > > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 2 > > P: Vendor=1e0e ProdID=9001 Rev= 2.32 > > S: Manufacturer=SimTech, Incorporated > > S: Product=SimTech, Incorporated > > S: SerialNumber=0123456789ABCDEF > > C:* #Ifs= 7 Cfg#= 1 Atr=80 MxPwr=500mA > > I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > > I:* If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > > I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > > I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > > I:* If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > > I:* If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > > I:* If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none) > > > > The last interface (6) is used for Android Composite ADB interface. > > > > Serial port layout: > > 0: QCDM/DIAG > > 1: NMEA > > 2: AT > > 3: AT/PPP > > 4: audio > > > > Signed-off-by: Andrey Skvortsov > > --- > > v2: respin v1 against usb-linus branch of > > git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git > > move SIM7100E product id to ALINK entries > > move SIM7100E usb_device_id initialization to ALINK initializations > > Thanks for respinning. I'll queue this one up for 4.5-rc. Now applied, thanks. Johan -- 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: cp210x: add IDs for GE B650V3 and B850V3 boards
On Mon, Feb 01, 2016 at 02:57:25PM -0500, Akshay Bhat wrote: > From: Ken Lin > > Add USB ID for cp2104/5 devices on GE B650v3 and B850v3 boards. > > Signed-off-by: Ken Lin > Signed-off-by: Akshay Bhat Now applied, thanks. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data
Hi, Felipe Balbi writes: > Mika Westerberg writes: >> On Thu, Feb 18, 2016 at 01:31:33PM +0200, Heikki Krogerus wrote: >>> On Thu, Feb 18, 2016 at 01:27:28PM +0200, Felipe Balbi wrote: >>> > Heikki Krogerus writes: >>> > >>> > > On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote: >>> > >> >>> > >> Hi, >>> > >> >>> > >> Heikki Krogerus writes: >>> > >> > This should allow the core driver to drop handling of >>> > >> > platform data and expect the platform specific details to >>> > >> > always come from properties. >>> > >> > >>> > >> > Signed-off-by: Heikki Krogerus >>> > >> > Cc: Huang Rui >>> > >> > CC: John Youn >>> > >> >>> > >> doesn't compile dude: >>> > >> >>> > >> make -k -j16 -- drivers/usb/dwc3/ >>> > >> CHK include/config/kernel.release >>> > >> CHK include/generated/uapi/linux/version.h >>> > >> CHK include/generated/utsrelease.h >>> > >> CHK include/generated/timeconst.h >>> > >> CHK include/generated/bounds.h >>> > >> CHK include/generated/asm-offsets.h >>> > >> CALLscripts/checksyscalls.sh >>> > >> CC [M] drivers/usb/dwc3/dwc3-pci.o >>> > >> CC [M] drivers/usb/dwc3/dwc3-keystone.o >>> > >> CC [M] drivers/usb/dwc3/dwc3-of-simple.o >>> > >> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’: >>> > >> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of >>> > >> function ‘platform_device_add_properties’ >>> > >> [-Werror=implicit-function-declaration] >>> > >> return platform_device_add_properties(dwc3, &pset); >>> > > >>> > > Which kernel are you using? platform_device_add_properties() is >>> > > available from v4.5-rc1 onwards. >>> > >>> > v4.5-rc4, could it be you missed a header or the header misses a stub ? >>> >>> Both the platform bus and property.c are always build in? >>> >>> Adding Mika and Andy. Guys, do you have ideas? >> >> I would like to reproduce this. Felipe, can you send me your .config? > > It could have been the one attached. But I don't know for sure anymore no, it wasn't this one. It's building now. Oh well... -- balbi signature.asc Description: PGP signature
Re: Spansion's CMSIS-DAP + COM Port
[ +CC: Oliver ] On Tue, Feb 09, 2016 at 09:27:39PM +0530, Thorsten Wilmer wrote: > Hi, > > I have a Starter Kit SK-FM4-176L-S6SE2CC > modprobe usbserial vendor=0x1a6a product=0x2000 > > makes the USB Port appear nicely (works with gtkterm) and at the same > time one can use the CMIS-DAP adapter with openocd. > > As suggested by the driver I would like to recommend the > vendor/product IDs to be added. This looks like a cdc-acm device to me. Any reason why you can't use that driver? If the driver is available, you should get a /dev/ttyACMn device node. If not, could you post the logs from when plugging the device in? > Bus 001 Device 002: ID 1a6a:2000 Spansion Inc. > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 1.01 > bDeviceClass0 (Defined at Interface level) > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize064 > idVendor 0x1a6a Spansion Inc. > idProduct 0x2000 > bcdDevice1.60 > iManufacturer 1 Spansion > iProduct2 Spansion CMSIS-DAP + COM Port > iSerial 0 > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 93 > bNumInterfaces 3 > bConfigurationValue 1 > iConfiguration 0 > bmAttributes 0x00 > (Missing must-be-set bit!) > (Bus Powered) > MaxPower 62mA > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber0 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass 3 Human Interface Device > bInterfaceSubClass 0 No Subclass > bInterfaceProtocol 0 None > iInterface 4 Spansion CMSIS-DAP > HID Device Descriptor: > bLength 9 > bDescriptorType33 > bcdHID 1.11 > bCountryCode0 Not supported > bNumDescriptors 1 > bDescriptorType34 Report > wDescriptorLength 29 > Report Descriptors: >** UNAVAILABLE ** > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x81 EP 1 IN > bmAttributes3 > Transfer TypeInterrupt > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 1 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x02 EP 2 OUT > bmAttributes3 > Transfer TypeInterrupt > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 1 > Interface Association: > bLength 8 > bDescriptorType11 > bFirstInterface 1 > bInterfaceCount 2 > bFunctionClass 2 Communications > bFunctionSubClass 2 Abstract (modem) > bFunctionProtocol 1 AT-commands (v.25ter) > iFunction 5 Spansion USB Serial Port > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber1 > bAlternateSetting 0 > bNumEndpoints 1 > bInterfaceClass 2 Communications > bInterfaceSubClass 2 Abstract (modem) > bInterfaceProtocol 1 AT-commands (v.25ter) > iInterface 0 > CDC Union: > bMasterInterface0 > bSlaveInterface 1 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x83 EP 3 IN > bmAttributes3 > Transfer TypeInterrupt > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 255 > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber2 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass10 CDC Data > bInterfaceSubClass 0 Unused > bInterfaceProtocol 0 > iInterface 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x04 EP 4 OUT > bmAttributes2 > Transfer TypeBulk > Synch Type None >
Re: [PATCH] USB: option: add "4G LTE usb-modem U901"
On Fri, Feb 12, 2016 at 04:40:00PM +0100, Bjørn Mork wrote: > Thomas reports: > > T: Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#= 4 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=05c6 ProdID=6001 Rev=00.00 > S: Manufacturer=USB Modem > S: Product=USB Modem > S: SerialNumber=1234567890ABCDEF > C: #Ifs= 5 Cfg#= 1 Atr=e0 MxPwr=500mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > I: If#= 4 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage > > Cc: > Reported-by: Thomas Schäfer > Signed-off-by: Bjørn Mork Now applied. Thanks, Johan -- 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: TODO List For Linux USB
Hi, On Wed, Feb 17, 2016 at 4:21 PM, Andiry Xu wrote: > On Tue, Feb 16, 2016 at 10:43 PM, Anil Nair wrote: >> Hi, >> >> On Wed, Feb 17, 2016 at 12:00 PM, Greg KH wrote: >>> On Wed, Feb 17, 2016 at 11:47:56AM +0530, Anil Nair wrote: Hi All, I would like to contribute to Linux USB kernel but i am confused on where to start, >>> >>> If you don't know where to start, or what to do, why do you want to >>> contribute? >> >> Because I have been on mailing list for quite a long time, I wanted to >> contribute something back to the community. >> If possible can you guys guide me regarding where to find the TODO list, so that I can start on with small patches. >>> >>> Run it, stress it, find problems, and start solving them. Read the >>> mailing list traffic and notice what others have problems with and work >>> on solving them. >> >> Most issues are beyond my scope it requires more experience at >> hardware level. Anyways i will observe and learn. >> > > That's why starting with drivers may not be a good idea because it > almost always requires some knowledge of hardware. > > Have you read the USB spec? Do you know the USB driver stack? If not, > there is little chance you can do real contributions. > > Try something hardware-unrelated, like file system or mm... > Okay will try it. > Thanks, > Andiry > >>> >>> Sorry, there is no TODO list. >> >> Ohh okay. >> >>> >>> best of luck, >>> >> >> Thanks. >> >>> greg k-h >> >> >> >> -- >> -- >> Regards, >> Anil Nair >> -- >> 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 -- -- Regards, Anil Nair -- 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 1/3] usb: USB Type-C Connector Class
On Thu, Feb 18, 2016 at 10:21:48AM +0100, Oliver Neukum wrote: > On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote: > > Hi, > > > The modes that can actually be selected have to be supported by both > > the connector and the partner, and this is where I'm putting the ball > > on the userspace at the moment. I'm not offering a list of > > "possible_alternate_modes" where I list the combination, but instead > > expect the userspace to be figure out that on it's own. > > > > Do you think we should add "possible_alternate_modes" file? > > No, what do we answer to the DFP if we recieve "Discover SVIDs"? > I don't think that we always should answer with all we physically > can. If, for example, the hardware could do Thunderbolt, but the OS > is not prepared to handle it, we shouldn't offer it. So this is > a policy decision to be made in user space. Hence we need > an API to tell it to the kernel. OK. Makes sense. > > P.S. That reminds me, here's my current draft for the > > Documentation/ABI/. Could you take a look? > > OK > > Here are my comments: > > What: /sys/class/type-c/usbcN/connected > > Connection status of the USB Type-C connector usbcN. "yes" when > connected, otherwise "no". > > Unnecessarily wordy. 0 and 1 would do That works for me. > What: /sys/class/type-c/usbcN/current_data_role > > Again, 0 and 1 would do I disagree with this one. What would 0 mean and what would 1? It would require us to make an agreement about the "index" of the role, which creates a small risk of somebody getting it wrong, but for what purpose? Why couldn't it be human readable "host" or "device" so there is never no confusion about it. > What: /sys/class/type-c/usbcN/partner_alternate_modes > > You should say in which number base the values are given. > > What: /sys/class/type-c/usbcN/partner_type > > That could be combined with "connected" Hmm, so in practice getting rid of "connected" completely.. I guess it's OK. > What: /sys/class/type-c/usbcN/supported_data_roles > > A connector can be both. How is that expressed? "host, device". > What: /sys/class/type-c/usbcN/supported_power_roles > > Again, what if it can do both? "source, sink". So these last two are now listing the values that can be entered to the current_data_role and current_power_role. Thanks, -- heikki -- 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 v7 05/10] usb: dbc: add bulk out and bulk in interfaces
On 26.01.2016 14:58, Lu Baolu wrote:
This patch adds interfaces for bulk out and bulk in ops. These
interfaces could be used to implement early printk bootconsole
or hook to various system debuggers.
Signed-off-by: Lu Baolu
---
drivers/usb/early/xhci-dbc.c | 373 +++
include/linux/usb/xhci-dbc.h | 30
2 files changed, 403 insertions(+)
...
+
+/*
+ * Check and dispatch events in event ring. It also checks status
+ * of hardware. This function will be called from multiple threads.
+ * An atomic lock is applied to protect the access of event ring.
+ */
+static int xdbc_check_event(void)
+{
+ /* event ring is under checking by other thread? */
+ if (!test_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags) &&
+ !test_and_set_bit(XDBC_ATOMIC_EVENT,
+ &xdbcp->atomic_flags))
+ return 0;
homemade trylock, can't the real ones be used?
+
+ xdbc_handle_events();
+
+ test_and_clear_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags);
+
+ return 0;
+}
+
+#defineBULK_IN_COMPLETED(p)((xdbcp->in_pending == (p)) && \
+xdbcp->in_complete)
+#defineBULK_OUT_COMPLETED(p) ((xdbcp->out_pending == (p)) && \
+xdbcp->out_complete)
+
...
+}
+
+int xdbc_bulk_read(void *data, int size, int loops)
+{
+ int ret;
+
+ do {
+ if (!test_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags) &&
+ !test_and_set_bit(XDBC_ATOMIC_BULKIN,
+ &xdbcp->atomic_flags))
+ break;
+ } while (1);
homemeade spin_lock, can't the real one be used?
If the xdbc_bulk_write() can be accessed from interrupt context (handler, soft,
timer) it
may deadlock
+
+ ret = xdbc_bulk_transfer(data, size, loops, true);
+
+ test_and_clear_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags);
+
+ return ret;
+}
+
+int xdbc_bulk_write(const char *bytes, int size)
+{
+ int ret;
+
+ do {
+ if (!test_bit(XDBC_ATOMIC_BULKOUT, &xdbcp->atomic_flags) &&
+ !test_and_set_bit(XDBC_ATOMIC_BULKOUT,
+ &xdbcp->atomic_flags))
+ break;
+ } while (1);
Another homemeade spin_lock, can't the real one be used?
same issue here, deadlock if accessible from interrupt context.
Would it make sense to have only one spinlock, and start one separate thread for
reading the event ring. The thread would, lock, handle pending events, unlock,
then call shedule, in a loop. ehci early debug code has some variant of this.
So the lock would be taken while events are being handled.
The same lock would be used for bulk_read and bulk_write. Yes this would
prevent read and
write at the same time, and the read and writes need to be modified to not
block until
the reansfer is finished, just to write the TRBs on the ring, update ring
pointers,
and ring the doorbell.
Or is all this impossibe due to the earlyness of the code?
-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 1/3] usb: USB Type-C Connector Class
Hi, On Thu, Feb 18, 2016 at 10:35:41AM +0100, Oliver Neukum wrote: > On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote: > > Hi, > > > P.S. That reminds me, here's my current draft for the > > Documentation/ABI/. Could you take a look? > > And I am afraid, that I have a few remarks not bound > to a specific entry. > > We have port directories for port power switching. How is > the connector directory linked to them? I'm sorry, I don't think I understand this point. > Likewise, if we have USB PD, we have to know how that > is linked to the connector directory. So you mean when we have USB PD PHY or controller, right? That will be the parent of the connector device if we have one on the platform. I think I'm misunderstanding this point as well.. > In addition, writes to those files have results. We need > the error codes to be described. Yes, I need to document those. > Furthermore, do these files support poll? > > And lastly we can get "Attention" as a message connected > with a connector in an alternate mode. How does user space > learn about that? The class should notify the userspace with uevent on connection/disconnection regardless what is being connected, or what mode the connector enters initially. So do you want to see that explained in the ABI document? The uevent does not contain any details, but I thought that it's OK to expect the userspace to read that separately. If this is not OK, let's add a new uevent variable that specifies what was just connected. I hope I did not misunderstand also this one. > I am sorry to be this obnoxious, but this is an API which > will be with us for a long time, so we better get it right. I would not say you are being obnoxious. If you are, feel free :). Your input is most welcome. Thanks a lot for that. And I agree, we need to make this solid from the beginning. Thanks, -- heikki -- 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 1/3] usb: USB Type-C Connector Class
On Thu, 2016-02-18 at 15:25 +0200, Heikki Krogerus wrote: Hi, > > We have port directories for port power switching. How is > > the connector directory linked to them? > > I'm sorry, I don't think I understand this point. Like this: oneukum@linux-dtbq:/sys/bus/usb/devices/3-0:1.0> ls -l total 0 -rw-r--r-- 1 root root 4096 Feb 18 14:34 authorized -r--r--r-- 1 root root 4096 Feb 18 14:34 bAlternateSetting -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceClass -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceNumber -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceProtocol -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceSubClass -r--r--r-- 1 root root 4096 Feb 18 14:34 bNumEndpoints lrwxrwxrwx 1 root root0 Feb 17 15:59 driver -> ../../../../../bus/usb/drivers/hub drwxr-xr-x 3 root root0 Feb 18 09:35 ep_81 -r--r--r-- 1 root root 4096 Feb 18 14:34 modalias drwxr-xr-x 2 root root0 Feb 18 09:35 power lrwxrwxrwx 1 root root0 Feb 17 15:59 subsystem -> ../../../../../bus/usb -r--r--r-- 1 root root 4096 Feb 18 14:34 supports_autosuspend -rw-r--r-- 1 root root 4096 Feb 18 14:34 uevent drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port1 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port10 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port11 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port12 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port13 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port14 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port15 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port2 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port3 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port4 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port5 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port6 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port7 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port8 drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port9 usb*-port* They correspond to the connectors a system has. It seems to me that we want a link connecting them if the correspondance is known. > > Likewise, if we have USB PD, we have to know how that > > is linked to the connector directory. > > So you mean when we have USB PD PHY or controller, right? That > will be the parent of the connector device if we have one on the > platform. So the parentage is different on whether a PD controller is present? That needs to be documented. And so we cannot deal with separate modules for a PD driver? [..] > > Furthermore, do these files support poll? At least the current role and mode can change, so in principle poll() makes sense. > > And lastly we can get "Attention" as a message connected > > with a connector in an alternate mode. How does user space > > learn about that? > > The class should notify the userspace with uevent on > connection/disconnection regardless what is being connected, or what > mode the connector enters initially. Yes, but "Attention" in the sense of 6.4.4.3.6 of the PD spec. Does this need to be handled in the kernel? Do we generate a uevent for that? > So do you want to see that explained in the ABI document? No. Regards Oliver -- 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: Spansion's CMSIS-DAP + COM Port
On Thu, 2016-02-18 at 13:05 +0100, Johan Hovold wrote: > > I have a Starter Kit SK-FM4-176L-S6SE2CC > > modprobe usbserial vendor=0x1a6a product=0x2000 > > > > makes the USB Port appear nicely (works with gtkterm) and at the > same > > time one can use the CMIS-DAP adapter with openocd. > > > > As suggested by the driver I would like to recommend the > > vendor/product IDs to be added. > > This looks like a cdc-acm device to me. Any reason why you can't use > that driver? If the driver is available, you should get a /dev/ttyACMn > device node. If not, could you post the logs from when plugging the > device in? It definitely looks like that. What kind of device is it? Regards Oliver -- 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 1/1] USB: core: let USB device know device node
On Tue, Feb 16, 2016 at 04:42:52PM +0800, Peter Chen wrote: > From: Peter Chen > > Although most of USB devices are hot-plug's, there are still some devices > are hard wired on the board, eg, for HSIC and SSIC interface USB devices. > If these kinds of USB devices are multiple functions, and they can supply > other interfaces like i2c, gpios for other devices, we may need to > describe these at device tree. > > In this commit, it uses "reg" in dts as physical port number to match > the phyiscal port number decided by USB core, if they are the same, > then the device node is for the device we are creating for USB core. > > Signed-off-by: Peter Chen > --- > Changes for v5: > - Refine the code how to get the device node at usb_alloc_dev according > to Alan's comment. > - Point out "usbVID,PID" is the recommented compatible, and others > compatibles at binding doc could also be used. > > Changes for v4: > - The range of "reg" should be 1-31, changing device node address > style as in lower case hexadecimal with leading zeroes suppressed > [binding doc, usb-device.txt] > - Improve the example at binding doc, it describes node from the top > (the controller) > - Delete the struct of_node * within struct usb_device > - Using usb_hcd_find_raw_port_number to get raw port number under root > hub port > > Changes for v3: > - typo: s/descirbe/describe/ > > Changes for v2: > - Fix build error reported by kbuild robot, lack of "static" for > inline usb_of_get_child_node > - Fix typo, "devcie_node" -> "device_node" > - Add kernel-doc for of_node at struct usb_device > > Changes from RFC: > - Fix the error address for binding doc, and add compatible for binding doc > - Change get child node API from "usb_of_find_node" to > "usb_of_get_child_node" > - Delete unecessary header files > - One typo > > .../devicetree/bindings/usb/usb-device.txt | 26 Looks good to me. Thanks for driving this to completion. Acked-by: Rob Herring > drivers/usb/core/Makefile | 2 +- > drivers/usb/core/of.c | 47 > ++ > drivers/usb/core/usb.c | 10 + > include/linux/usb/of.h | 7 > 5 files changed, 91 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/usb/usb-device.txt > create mode 100644 drivers/usb/core/of.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: NEC uPD720200 xHCI Controller dies when Runtime PM enabled
On 16.02.2016 23:58, main.ha...@googlemail.com wrote: On 2016-02-08 15:31, Mathias Nyman wrote: Hi On 06.02.2016 19:08, Mike Murdoch wrote: Bug ID: 111251 Hello, I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on kernel 4.4.1-gentoo. 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04) (prog-if 30 [XHCI]) Subsystem: Lenovo uPD720200 USB 3.0 Host Controller When runtime power control for this controller is disabled (/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller works fine and reaches over 120MB/s transfer rates. When runtime power control for this controller is enabled (/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects can be observed: - Transfer rates are much lower at around 30MB/s - During transfers, the controller dies after a couple of seconds: xhci_hcd :0e:00.0: xHCI host not responding to stop endpoint command. xhci_hcd :0e:00.0: Assuming host is dying, halting host. xhci_hcd :0e:00.0: Host not halted after 16000 microseconds. xhci_hcd :0e:00.0: Non-responsive xHCI host is not halting. xhci_hcd :0e:00.0: Completing active URBs anyway. xhci_hcd :0e:00.0: HC died; cleaning up sd 9:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK sd 9:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 19 a9 00 00 00 f0 00 blk_update_request: I/O error, dev sdc, sector 1681664 xhci_hcd :0e:00.0: Stopped the command ring failed, maybe the host is dead xhci_hcd :0e:00.0: Host not halted after 16000 microseconds. xhci_hcd :0e:00.0: Abort command ring failed xhci_hcd :0e:00.0: HC died; cleaning up At this point, a reboot is required to reactivate the controller, unloading and reloading the xhci_* modules does not work. With 120MB/s I assume it was a USB3 device. Was there any USB 2 device connected as well? Does this occur with only a USB2 device connected to xhci? xhci handles suspend/resume a bit differently for USB2 and USB3 roothubs. Does this happen on older kernels as well? 4.3 or 4.2 based? For more xhci debugging, do: echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control and check dmesg for more xhci info. If reloading the module did not help it is more likely that the controller is in some unexpected state. If however, it would instead be just bad timeout timer handling we could just return immediately in the timeout handler, and check if the usb device(s) continue to work normally. This could be done by editing drivers/usb/hosts/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -831,6 +831,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) struct xhci_virt_ep *ep; int ret, i, j; unsigned long flags; + return; -Mathias Hello Mat, thanks for your response. I have experimented with your suggestions. As for your questions: No, there was only one USB3 stick connected to the host controller during the tests. USB2 devices work fine too. Yes, I encountered this problem on a 4.1 series kernel aswell as the 4.4 series. I have enabled the debug controls and attached the results to this mail, along with some commentary. I am hoping this works in the mailing list. I've also tried your suggested modification, and it does seem to work! With it, the controller does not die, but it still sacrifices a lot of speed (as I had mentioned in the first mail of this thread) I hope this is helpful! Thanks, it is helpful Looks like when the USB3 device is inserted it is first detected as a USB2 device, then immediately afterwars as a USB3 device, the usb2 device stops responding so 5 seconds later we timeout, and kill everything. selected parts of the log: inserting usb3 storage device 20:03:33 xhci_hcd :0e:00.0: xhci_resume: starting port polling. 20:03:33 xhci_hcd :0e:00.0: Port Status Change Event for port 3 20:03:33 xhci_hcd :0e:00.0: get port status, actual port 0 status = 0x202e1 /* PORT 0 20:03:33 xhci_hcd :0e:00.0: get port status, actual port 1 status = 0x2a0 /* PORT 1 20:03:33 usb 1-1: new high-speed USB device number 2 using xhci_hcd 20:03:33 xhci_hcd :0e:00.0: Slot ID 1 Input Context: /* Found a HS device 20:03:33 xhci_hcd :0e:00.0: IN Endpoint 00 Context (ep_index 00): 20:03:33 xhci_hcd :0e:00.0: @8805fc8a5048 (virt) @a048 (dma) 0xfffdf001 - deq 20:03:33 xhci_hcd :0e:00.0: Successful setup context command * now we have a device at SLOT 1 with control endpoint 0 buffer at address 0xfffdf000 20:03:33 xhci_hcd :0e:00.0: Slot ID 2 Input Context: 20:03:33 xhci_hcd :0e:00.0: IN Endpoint 00 Context (ep_index 00): 20:03:33 xhci_hcd :0e:00.0: @8800b68d7048 (virt) @2048 (dma) 0xfffe1001 - deq * now we have another device at SLOT 2 with control endpoint buffer at 0xfffe1000 20:03:33 usb 2-1: new SuperSpeed USB device number 3 using xhci_hcd /* found SS device 20:03:33 usb 2-1: N
Re: [PATCH 1/3] usb: USB Type-C Connector Class
On Thu, Feb 18, 2016 at 02:44:25PM +0100, Oliver Neukum wrote: > On Thu, 2016-02-18 at 15:25 +0200, Heikki Krogerus wrote: > > Hi, > > > > > We have port directories for port power switching. How is > > > the connector directory linked to them? > > > > I'm sorry, I don't think I understand this point. > > Like this: > > oneukum@linux-dtbq:/sys/bus/usb/devices/3-0:1.0> ls -l > total 0 > -rw-r--r-- 1 root root 4096 Feb 18 14:34 authorized > -r--r--r-- 1 root root 4096 Feb 18 14:34 bAlternateSetting > -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceClass > -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceNumber > -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceProtocol > -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceSubClass > -r--r--r-- 1 root root 4096 Feb 18 14:34 bNumEndpoints > lrwxrwxrwx 1 root root0 Feb 17 15:59 driver > -> ../../../../../bus/usb/drivers/hub > drwxr-xr-x 3 root root0 Feb 18 09:35 ep_81 > -r--r--r-- 1 root root 4096 Feb 18 14:34 modalias > drwxr-xr-x 2 root root0 Feb 18 09:35 power > lrwxrwxrwx 1 root root0 Feb 17 15:59 subsystem > -> ../../../../../bus/usb > -r--r--r-- 1 root root 4096 Feb 18 14:34 supports_autosuspend > -rw-r--r-- 1 root root 4096 Feb 18 14:34 uevent > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port1 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port10 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port11 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port12 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port13 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port14 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port15 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port2 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port3 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port4 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port5 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port6 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port7 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port8 > drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port9 > > usb*-port* > > They correspond to the connectors a system has. > It seems to me that we want a link connecting them > if the correspondance is known. Ah, got it. In case of ACPI enumerated UCSI, we will have the actual ACPI device object for the port as a child device object. So when we attach the port ACPI companion to the connector device we create in the class, it will link us directly to the correct usb*-port*. I have not done it so far because the same port ACPI device object will also be bound to the usb peripheral once it gets enumerated, and I was worried if that would cause a problem. But after talking to guys that know more about ACPI then I do, I'm sure that is not going to be a problem. With ACPI, the binding should happen the same way even without UCSI. What ever device driver registers the connector device should have the port ACPI device object as it's child. So I'm thinking about doing this in typec_register_port() and not in UCSI driver. Thanks, -- heikki -- 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/3] usb: fix endpoint descriptor buffer size decrement
An endpoint descriptor may be followed by both SuperSpeed and
SuperSpeedPlus Isoc endpoint companion descriptors.
These descriptors are all stored one after another in a buffer.
The new SuperSpeedPlus Isoc endpoint companion parsing incorrectly
decreased the the remaining buffer size before comparing the size with the
expected length of the descriptor.
This lead to possible failure in reading the SuperSpeed endpoint companion
descriptor of the last endpoint, displaying an message like:
"No SuperSpeed endpoint companion for config 1 interface 0 altsetting 0
ep 129: using minimum values"
Fix it by decreasing the size after comparing it.
Signed-off-by: Mathias Nyman
---
drivers/usb/core/config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 5eb1a87..47fb58c 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -76,7 +76,6 @@ static void usb_parse_ss_endpoint_companion(struct device
*ddev, int cfgno,
*/
desc = (struct usb_ss_ep_comp_descriptor *) buffer;
buffer += desc->bLength;
- size -= desc->bLength;
if (desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP ||
size < USB_DT_SS_EP_COMP_SIZE) {
@@ -101,6 +100,7 @@ static void usb_parse_ss_endpoint_companion(struct device
*ddev, int cfgno,
return;
}
+ size -= desc->bLength;
memcpy(&ep->ss_ep_comp, desc, USB_DT_SS_EP_COMP_SIZE);
/* Check the various values */
--
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 2/3] usb: host: xhci-plat: add R-Car Gen2 and Gen3 fallback compatibility strings
From: Simon Horman
Add fallback compatibility strings for R-Car Gen2 and Gen3.
This is in keeping with the fallback scheme being adopted wherever
appropriate for drivers for Renesas SoCs.
Signed-off-by: Simon Horman
Acked-by: Geert Uytterhoeven
Acked-by: Gregory CLEMENT
Signed-off-by: Mathias Nyman
---
Documentation/devicetree/bindings/usb/usb-xhci.txt | 21 +
drivers/usb/host/xhci-plat.c | 5 +
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 0825732..6a17aa8 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -1,10 +1,23 @@
USB xHCI controllers
Required properties:
- - compatible: should be one of "generic-xhci",
-"marvell,armada-375-xhci", "marvell,armada-380-xhci",
-"renesas,xhci-r8a7790", "renesas,xhci-r8a7791", "renesas,xhci-r8a7793",
-"renesas,xhci-r8a7795" (deprecated: "xhci-platform").
+ - compatible: should be one or more of
+
+- "generic-xhci" for generic XHCI device
+- "marvell,armada-375-xhci" for Armada 375 SoCs
+- "marvell,armada-380-xhci" for Armada 38x SoCs
+- "renesas,xhci-r8a7790" for r8a7790 SoC
+- "renesas,xhci-r8a7791" for r8a7791 SoC
+- "renesas,xhci-r8a7793" for r8a7793 SoC
+- "renesas,xhci-r8a7795" for r8a7795 SoC
+- "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 compatible device
+- "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
+- "xhci-platform" (deprecated)
+
+When compatible with the generic version, nodes must list the
+SoC-specific version corresponding to the platform first
+followed by the generic version.
+
- reg: should contain address and length of the standard XHCI
register set for the device.
- interrupts: one XHCI interrupt should be described here.
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d39d6bf..3aede6e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -110,6 +110,11 @@ static const struct of_device_id usb_xhci_of_match[] = {
.compatible = "renesas,xhci-r8a7795",
.data = &xhci_plat_renesas_rcar_gen3,
}, {
+ .compatible = "renesas,rcar-gen2-xhci",
+ .data = &xhci_plat_renesas_rcar_gen2,
+ }, {
+ .compatible = "renesas,rcar-gen3-xhci",
+ .data = &xhci_plat_renesas_rcar_gen3,
},
};
MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
--
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/3] usb and xhci fixes for usb-next 4.6
Hi Greg A few more patches for 4.6. At least the first one would be important to get with the rest of the USB 3.1 SSP Isoc patches to 4.6 Without it the SSP Isoc endpoint companion descriptor parsing may mess up the normal SS endpoint companion parsing. Julia Lawall (1): usb: host: xhci-plat: fix of_table.cocci warnings Mathias Nyman (1): usb: fix endpoint descriptor buffer size decrement Simon Horman (1): usb: host: xhci-plat: add R-Car Gen2 and Gen3 fallback compatibility strings Documentation/devicetree/bindings/usb/usb-xhci.txt | 21 + drivers/usb/core/config.c | 2 +- drivers/usb/host/xhci-plat.c | 6 ++ 3 files changed, 24 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
[PATCH 3/3] usb: host: xhci-plat: fix of_table.cocci warnings
From: Julia Lawall
Make sure (of/i2c/platform)_device_id tables are NULL terminated
Generated by: scripts/coccinelle/misc/of_table.cocci
Signed-off-by: Fengguang Wu
Signed-off-by: Julia Lawall
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-plat.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3aede6e..5c15e9b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -116,6 +116,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
.compatible = "renesas,rcar-gen3-xhci",
.data = &xhci_plat_renesas_rcar_gen3,
},
+ {},
};
MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
#endif
--
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: [Bug 112171] UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:873:47
On Tue, 9 Feb 2016, Yaroslav Molochko wrote: > After upgrade to 4.5.0-rc3 I've got following stack trace, never seen > that before: > > Feb 08 22:58:56 x kernel: UBSAN: Undefined behaviour in > drivers/usb/host/ehci-hub.c:873:47 > Feb 08 22:58:56 x kernel: index -1 is out of range for type 'u32 [1]' > Feb 08 22:58:56 x kernel: CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 4.5.0-rc3 #98 > Feb 08 22:58:56 x kernel: Hardware name: LENOVO 20BSCTO1WW/20BSCTO1WW, > BIOS N14ET32W (1.10 ) 08/13/2015 > Feb 08 22:58:56 x kernel: 84b750fa > 880224deb358 81832ca4 > Feb 08 22:58:56 x kernel: 84b750fa > 880224deb380 > Feb 08 22:58:56 x kernel: 880224deb370 818a6360 > 833ea920 880224deb3c8 > Feb 08 22:58:56 x kernel: Call Trace: > Feb 08 22:58:56 x kernel: [] dump_stack+0xaf/0x10c > Feb 08 22:58:56 x kernel: [] ubsan_epilogue+0x14/0x56 > Feb 08 22:58:56 x kernel: [] > __ubsan_handle_out_of_bounds+0x86/0xb3 > Feb 08 22:58:56 x kernel: [] ehci_hub_control+0xcf/0x141e I have no idea why this didn't show up earlier. In any case it doesn't matter; the -1 index is part of an address calculation for a pointer that doesn't get used in this pathway. This seems to be a common problem with UBSAN. It spots invalid or questionable calculations in situations where they make no difference because the results don't get used. 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: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter
On Tue, 9 Feb 2016, Antony Pavlov wrote:
> This patch retrieves and configures the vbus control gpio via
> the device tree.
>
> This patch is based on a ehci-s5p.c commit fd81d59c90d38661
> ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
>
> Signed-off-by: Antony Pavlov
> Cc: Alan Stern
> Cc: Greg Kroah-Hartman
> Cc: linux-usb@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> drivers/usb/host/ehci-platform.c | 22 ++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/usb/host/ehci-platform.c
> b/drivers/usb/host/ehci-platform.c
> index bd7082f2..0d95ced 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
> .power_off =ehci_platform_power_off,
> };
>
> +static void setup_vbus_gpio(struct device *dev)
> +{
> + int err;
> + int gpio;
> +
> + if (!dev->of_node)
> + return;
> +
> + gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
> + if (!gpio_is_valid(gpio))
> + return;
> +
> + err = devm_gpio_request_one(dev, gpio,
> + GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
> + "ehci_vbus_gpio");
> + if (err)
> + dev_err(dev, "can't request ehci vbus gpio %d", gpio);
I don't understand this. If you get an error here, what's the point of
allowing the probe to continue? Shouldn't you return an error code so
the probe will fail?
Alan Stern
> +}
> +
> static int ehci_platform_probe(struct platform_device *dev)
> {
> struct usb_hcd *hcd;
> @@ -174,6 +194,8 @@ static int ehci_platform_probe(struct platform_device
> *dev)
> return irq;
> }
>
> + setup_vbus_gpio(&dev->dev);
> +
> hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
>dev_name(&dev->dev));
> if (!hcd)
>
--
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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Tue, 9 Feb 2016, Dâniel Fraga wrote: > I'm using Linux 4.4.1 kernel on a Dell Inspiron 15R 5537 and > everytime I wake up from S3 sleep, the syslog is full of the following > message: > > Feb 9 17:19:05 tux kernel: [19196.253206] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:05 tux kernel: [19196.406206] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:05 tux kernel: [19196.559239] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:06 tux kernel: [19197.738267] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:06 tux kernel: [19197.891218] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:07 tux kernel: [19198.044243] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:07 tux kernel: [19198.198231] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:08 tux kernel: [19199.388295] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:08 tux kernel: [19199.541307] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:08 tux kernel: [19199.694319] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:08 tux kernel: [19199.847338] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > Feb 9 17:19:09 tux kernel: [19201.026362] usb 3-1.6: reset full-speed USB > device number 6 using ehci-pci > > And it doesn't stop. The only way to stop it is to reload the ehci-pci > kernel module: > > sudo modprobe -r ehci-pci; sudo modprobe ehci-pci > > lsusb returns: > > Bus 003 Device 008: ID 064e:812e Suyin Corp. > Bus 003 Device 007: ID 0bda:0129 Realtek Semiconductor Corp. > Bus 003 Device 006: ID 04f3:0034 Elan Microelectronics Corp. > Bus 003 Device 005: ID 0cf3:0036 Atheros Communications, Inc. > Bus 003 Device 004: ID 1532:0016 Razer USA, Ltd > Bus 003 Device 003: ID 046d:c316 Logitech, Inc. HID-Compliant Keyboard > Bus 003 Device 002: ID 8087:8000 > Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub > Bus 001 Device 006: ID 046d:0990 Logitech, Inc. QuickCam Pro 9000 > Bus 001 Device 005: ID 0424:2228 Standard Microsystems Corp. 9-in-2 Card > Reader > Bus 001 Device 004: ID 0424:2602 Standard Microsystems Corp. > Bus 001 Device 003: ID 0424:2512 Standard Microsystems Corp. > Bus 001 Device 007: ID 051d:0002 American Power Conversion Uninterruptible > Power Supply > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > > So, any hints on what's going on? No idea. Can you collect a usbmon trace for bus 3, starting before the suspend and showing the problems? 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: NEC uPD720200 xHCI Controller dies when Runtime PM enabled
On 2016-02-18 16:12, Mathias Nyman wrote: > On 16.02.2016 23:58, main.ha...@googlemail.com wrote: >> >> >> On 2016-02-08 15:31, Mathias Nyman wrote: >>> Hi >>> >>> On 06.02.2016 19:08, Mike Murdoch wrote: Bug ID: 111251 Hello, I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on kernel 4.4.1-gentoo. 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04) (prog-if 30 [XHCI]) Subsystem: Lenovo uPD720200 USB 3.0 Host Controller When runtime power control for this controller is disabled (/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller works fine and reaches over 120MB/s transfer rates. When runtime power control for this controller is enabled (/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects can be observed: - Transfer rates are much lower at around 30MB/s - During transfers, the controller dies after a couple of seconds: xhci_hcd :0e:00.0: xHCI host not responding to stop endpoint command. xhci_hcd :0e:00.0: Assuming host is dying, halting host. xhci_hcd :0e:00.0: Host not halted after 16000 microseconds. xhci_hcd :0e:00.0: Non-responsive xHCI host is not halting. xhci_hcd :0e:00.0: Completing active URBs anyway. xhci_hcd :0e:00.0: HC died; cleaning up sd 9:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK sd 9:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 19 a9 00 00 00 f0 00 blk_update_request: I/O error, dev sdc, sector 1681664 xhci_hcd :0e:00.0: Stopped the command ring failed, maybe the host is dead xhci_hcd :0e:00.0: Host not halted after 16000 microseconds. xhci_hcd :0e:00.0: Abort command ring failed xhci_hcd :0e:00.0: HC died; cleaning up At this point, a reboot is required to reactivate the controller, unloading and reloading the xhci_* modules does not work. >>> >>> With 120MB/s I assume it was a USB3 device. >>> Was there any USB 2 device connected as well? >>> Does this occur with only a USB2 device connected to xhci? >>> >>> xhci handles suspend/resume a bit differently for USB2 and USB3 >>> roothubs. >>> >>> Does this happen on older kernels as well? 4.3 or 4.2 based? >>> >>> For more xhci debugging, do: >>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control >>> and check dmesg for more xhci info. >>> >>> If reloading the module did not help it is more likely that the >>> controller is in some >>> unexpected state. >>> If however, it would instead be just bad timeout timer handling we >>> could just return immediately >>> in the timeout handler, and check if the usb device(s) continue to >>> work normally. >>> >>> This could be done by editing drivers/usb/hosts/xhci-ring.c >>> >>> +++ b/drivers/usb/host/xhci-ring.c >>> @@ -831,6 +831,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned >>> long arg) >>> struct xhci_virt_ep *ep; >>> int ret, i, j; >>> unsigned long flags; >>> + return; >>> >>> -Mathias >>> >>> >> Hello Mat, >> >> thanks for your response. I have experimented with your suggestions. >> >> As for your questions: No, there was only one USB3 stick connected to >> the host controller during the tests. USB2 devices work fine too. >> >> Yes, I encountered this problem on a 4.1 series kernel aswell as the 4.4 >> series. >> >> I have enabled the debug controls and attached the results to this mail, >> along with some commentary. I am hoping this works in the mailing list. >> >> I've also tried your suggested modification, and it does seem to work! >> With it, the controller does not die, but it still sacrifices a lot of >> speed (as I had mentioned in the first mail of this thread) >> >> >> I hope this is helpful! >> > > Thanks, it is helpful > > Looks like when the USB3 device is inserted it is first detected as a > USB2 device, > then immediately afterwars as a USB3 device, the usb2 device stops > responding so 5 > seconds later we timeout, and kill everything. > > selected parts of the log: > > inserting usb3 storage device > 20:03:33 xhci_hcd :0e:00.0: xhci_resume: starting port polling. > 20:03:33 xhci_hcd :0e:00.0: Port Status Change Event for port 3 > 20:03:33 xhci_hcd :0e:00.0: get port status, actual port 0 status > = 0x202e1 /* PORT 0 > 20:03:33 xhci_hcd :0e:00.0: get port status, actual port 1 status > = 0x2a0 /* PORT 1 > 20:03:33 usb 1-1: new high-speed USB device number 2 using xhci_hcd > 20:03:33 xhci_hcd :0e:00.0: Slot ID 1 Input Context:/* > Found a HS device > 20:03:33 xhci_hcd :0e:00.0: IN Endpoint 00 Context (ep_index 00): > 20:03:33 xhci_hcd :0e:00.0: @8805fc8a5048 (virt) @a048 > (dma) 0xfffdf001 - deq > 20:03:33 xhci_hcd :0e:00.0: Successful setup context command > * now we have a device at SLOT
Re: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter
On 02/18/2016 05:12 PM, Alan Stern wrote:
> On Tue, 9 Feb 2016, Antony Pavlov wrote:
>
>> This patch retrieves and configures the vbus control gpio via
>> the device tree.
>>
>> This patch is based on a ehci-s5p.c commit fd81d59c90d38661
>> ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
>>
>> Signed-off-by: Antony Pavlov
>> Cc: Alan Stern
>> Cc: Greg Kroah-Hartman
>> Cc: linux-usb@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>> drivers/usb/host/ehci-platform.c | 22 ++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/usb/host/ehci-platform.c
>> b/drivers/usb/host/ehci-platform.c
>> index bd7082f2..0d95ced 100644
>> --- a/drivers/usb/host/ehci-platform.c
>> +++ b/drivers/usb/host/ehci-platform.c
>> @@ -28,6 +28,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
>> .power_off =ehci_platform_power_off,
>> };
>>
>> +static void setup_vbus_gpio(struct device *dev)
>> +{
>> +int err;
>> +int gpio;
>> +
>> +if (!dev->of_node)
>> +return;
>> +
>> +gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
>> +if (!gpio_is_valid(gpio))
>> +return;
>> +
>> +err = devm_gpio_request_one(dev, gpio,
>> +GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
>> +"ehci_vbus_gpio");
>> +if (err)
>> +dev_err(dev, "can't request ehci vbus gpio %d", gpio);
>
> I don't understand this. If you get an error here, what's the point of
> allowing the probe to continue? Shouldn't you return an error code so
> the probe will fail?
The idea is I believe that if there is no vbus gpio specified, the port
might just not have vbus control, so the probe can continue. But this
patch is irrelevant anyway, since Alexey will switch to CI HDRC driver
and use standard regulator, as it should be done.
--
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 v5 07/15] usb: ehci: add vbus-gpio parameter
On Thu, 18 Feb 2016 11:12:43 -0500 (EST)
Alan Stern wrote:
> On Tue, 9 Feb 2016, Antony Pavlov wrote:
>
> > This patch retrieves and configures the vbus control gpio via
> > the device tree.
> >
> > This patch is based on a ehci-s5p.c commit fd81d59c90d38661
> > ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
> >
> > Signed-off-by: Antony Pavlov
> > Cc: Alan Stern
> > Cc: Greg Kroah-Hartman
> > Cc: linux-usb@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org
> > ---
> > drivers/usb/host/ehci-platform.c | 22 ++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/usb/host/ehci-platform.c
> > b/drivers/usb/host/ehci-platform.c
> > index bd7082f2..0d95ced 100644
> > --- a/drivers/usb/host/ehci-platform.c
> > +++ b/drivers/usb/host/ehci-platform.c
> > @@ -28,6 +28,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
> > .power_off =ehci_platform_power_off,
> > };
> >
> > +static void setup_vbus_gpio(struct device *dev)
> > +{
> > + int err;
> > + int gpio;
> > +
> > + if (!dev->of_node)
> > + return;
> > +
> > + gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
> > + if (!gpio_is_valid(gpio))
> > + return;
> > +
> > + err = devm_gpio_request_one(dev, gpio,
> > + GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
> > + "ehci_vbus_gpio");
> > + if (err)
> > + dev_err(dev, "can't request ehci vbus gpio %d", gpio);
>
>
> I don't understand this. If you get an error here, what's the point of
> allowing the probe to continue? Shouldn't you return an error code so
> the probe will fail?
Please ignore the 'usb: ehci: add vbus-gpio parameter' patch!
In the new AR9331 patchseries I use chipidea USB driver (thanks to Marek for
the suggestion)
in the AR9331 dtsi-file:
usb: usb@1b000100 {
compatible = "chipidea,usb2";
reg = <0x1b00 0x200>;
interrupt-parent = <&cpuintc>;
interrupts = <3>;
resets = <&rst 5>;
phy-names = "usb-phy";
phys = <&usb_phy>;
status = "disabled";
};
so I use regulator in the TL-MR3020 board dts file:
reg_usb_vbus: reg_usb_vbus {
compatible = "regulator-fixed";
regulator-name = "usb_vbus";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
gpio = <&gpio 8 GPIO_ACTIVE_HIGH>;
enable-active-high;
};
&usb {
dr_mode = "host";
vbus-supply = <®_usb_vbus>;
status = "okay";
};
As a result there is no need in adding vbus-gpio parameter to ehci anymore!
> > +}
> > +
> > static int ehci_platform_probe(struct platform_device *dev)
> > {
> > struct usb_hcd *hcd;
> > @@ -174,6 +194,8 @@ static int ehci_platform_probe(struct platform_device
> > *dev)
> > return irq;
> > }
> >
> > + setup_vbus_gpio(&dev->dev);
> > +
> > hcd = usb_create_hcd(&ehci_platform_hc_driver, &dev->dev,
> > dev_name(&dev->dev));
> > if (!hcd)
> >
>
--
--
Best regards,
Antony Pavlov
--
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: OHCI-PCI: frame counter not updating, module disabled
On Thu, 18 Feb 2016 m...@superbash.de wrote: > This happens also with older kernel versions (3.x, etc.): > > A small LCD display (ID 1908:0102 GEMBIRD) connected to USB is used for > some years now. Computer (and display) runs always (24 hours/day). > 'lcd4linux' is used to update the displays contents. > > This works fine - but it fails after 1-2 days. > > The log shows: > > ohci-pci :00:12.0: frame counter not updating; disabled > ohci-pci :00:12.0: HC died; cleaning up > > The ohci-pci module will be disabled then, the display gets lost. That error message indicates a problem in the OHCI hardware. I don't what causes the problem or how to prevent it. > A workaround for this is to reload this module every 24 hours > (automated by a systemd service). So far this helps: I didn't have this > error anymore. This workaround is tested for a few months now and it > runs fine - but it's 'only' a workaround. ;) That's okay. Reinitializing the controller and the driver is a perfectly reasonable workaround for a hardware problem. 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 1/3] usb: fix endpoint descriptor buffer size decrement
On Thu, 18 Feb 2016, Mathias Nyman wrote:
> An endpoint descriptor may be followed by both SuperSpeed and
> SuperSpeedPlus Isoc endpoint companion descriptors.
> These descriptors are all stored one after another in a buffer.
>
> The new SuperSpeedPlus Isoc endpoint companion parsing incorrectly
> decreased the the remaining buffer size before comparing the size with the
> expected length of the descriptor.
>
> This lead to possible failure in reading the SuperSpeed endpoint companion
> descriptor of the last endpoint, displaying an message like:
>
> "No SuperSpeed endpoint companion for config 1 interface 0 altsetting 0
> ep 129: using minimum values"
>
> Fix it by decreasing the size after comparing it.
>
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/core/config.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 5eb1a87..47fb58c 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -76,7 +76,6 @@ static void usb_parse_ss_endpoint_companion(struct device
> *ddev, int cfgno,
>*/
> desc = (struct usb_ss_ep_comp_descriptor *) buffer;
> buffer += desc->bLength;
> - size -= desc->bLength;
>
> if (desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP ||
> size < USB_DT_SS_EP_COMP_SIZE) {
> @@ -101,6 +100,7 @@ static void usb_parse_ss_endpoint_companion(struct device
> *ddev, int cfgno,
> return;
> }
>
> + size -= desc->bLength;
> memcpy(&ep->ss_ep_comp, desc, USB_DT_SS_EP_COMP_SIZE);
>
> /* Check the various values */
It's generally better to change the two variables (buffer and size) at
the same time. At least, that's what people expect to see. How about
moving both statements down?
Also, wouldn't it be better in the original patch to call
usb_parse_ssp_isoc_endpoint_companion() after all the SS endpoint
companion stuff is finished, instead of in the middle?
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: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter
On 02/18/2016 09:06 PM, Antony Pavlov wrote:
This patch retrieves and configures the vbus control gpio via
the device tree.
This patch is based on a ehci-s5p.c commit fd81d59c90d38661
("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
Signed-off-by: Antony Pavlov
Cc: Alan Stern
Cc: Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/usb/host/ehci-platform.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index bd7082f2..0d95ced 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
.power_off =ehci_platform_power_off,
};
+static void setup_vbus_gpio(struct device *dev)
+{
+ int err;
+ int gpio;
+
+ if (!dev->of_node)
+ return;
+
+ gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
+ if (!gpio_is_valid(gpio))
+ return;
+
+ err = devm_gpio_request_one(dev, gpio,
+ GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
+ "ehci_vbus_gpio");
+ if (err)
+ dev_err(dev, "can't request ehci vbus gpio %d", gpio);
I don't understand this. If you get an error here, what's the point of
allowing the probe to continue? Shouldn't you return an error code so
the probe will fail?
Please ignore the 'usb: ehci: add vbus-gpio parameter' patch!
In the new AR9331 patchseries I use chipidea USB driver (thanks to Marek for
the suggestion)
in the AR9331 dtsi-file:
usb: usb@1b000100 {
compatible = "chipidea,usb2";
reg = <0x1b00 0x200>;
interrupt-parent = <&cpuintc>;
interrupts = <3>;
resets = <&rst 5>;
phy-names = "usb-phy";
phys = <&usb_phy>;
status = "disabled";
};
so I use regulator in the TL-MR3020 board dts file:
reg_usb_vbus: reg_usb_vbus {
compatible = "regulator-fixed";
regulator-name = "usb_vbus";
regulator-min-microvolt = <500>;
Not 0?
regulator-max-microvolt = <500>;
gpio = <&gpio 8 GPIO_ACTIVE_HIGH>;
Where's the switch if both voltages are equal?
enable-active-high;
};
&usb {
dr_mode = "host";
vbus-supply = <®_usb_vbus>;
status = "okay";
};
As a result there is no need in adding vbus-gpio parameter to ehci anymore!
[...]
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] usb: host: ohci-pxa27x: propagate the irq error code
On Sat, 13 Feb 2016, Robert Jarzmik wrote:
> In several drivers in the pxa architecture, it was found that the
> platform_get_irq() was not propagated. This breaks the the device-tree
> probe deferral path, if -EPROBE_DEFER is returned. Unfortunately, the
> error return in this case is transformed into -ENXIO, breaking the
> deferral mechanism.
>
> Even if in this specific case the driver was not broken, because the
> interrupt controller is always probed before drivers, propagate the
> proper return code.
>
> Signed-off-by: Robert Jarzmik
Acked-by: Alan Stern
> ---
> drivers/usb/host/ohci-pxa27x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index e8c006e7a960..a667cf2d5788 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -435,7 +435,7 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver,
> struct platform_device
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> pr_err("no resource of IORESOURCE_IRQ");
> - return -ENXIO;
> + return irq;
> }
>
> usb_clk = devm_clk_get(&pdev->dev, NULL);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: devio: Add ioctl to disallow detaching kernel USB drivers.
On Sun, 14 Feb 2016, Emilio López wrote: > From: Reilly Grant > > The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily > relinquish the ability to issue other ioctls that may interfere with > other processes and drivers that have claimed an interface on the > device. > > This commit also includes a simple utility to be able to test the > ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c > > Example (with qemu-kvm's input device): > > $ lsusb > ... > Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd > > $ usb-devices > ... > C: #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA > I: If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=00 Prot=02 Driver=usbhid > > $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002 > OK: privileges dropped! > Available options: > [0] Exit now > [1] Reset device. Should fail if device is in use > [2] Claim 4 interfaces. Should succeed where not in use > [3] Narrow interface permission mask > Which option shall I run?: 1 > ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted) > Which test shall I run next?: 2 > ERROR claiming if 0 (1 - Operation not permitted) > ERROR claiming if 1 (1 - Operation not permitted) > ERROR claiming if 2 (1 - Operation not permitted) > ERROR claiming if 3 (1 - Operation not permitted) > Which test shall I run next?: 0 > > After unbinding usbhid: > > $ usb-devices > ... > I: If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=00 Prot=02 Driver=(none) > > $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002 > ... > Which option shall I run?: 2 > OK: claimed if 0 > ERROR claiming if 1 (1 - Operation not permitted) > ERROR claiming if 2 (1 - Operation not permitted) > ERROR claiming if 3 (1 - Operation not permitted) > Which test shall I run next?: 1 > OK: USBDEVFS_RESET succeeded > Which test shall I run next?: 0 > > After unbinding usbhid and restricting the mask: > > $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002 > ... > Which option shall I run?: 3 > Insert new mask: 0 > OK: privileges dropped! > Which test shall I run next?: 2 > ERROR claiming if 0 (1 - Operation not permitted) > ERROR claiming if 1 (1 - Operation not permitted) > ERROR claiming if 2 (1 - Operation not permitted) > ERROR claiming if 3 (1 - Operation not permitted) > > Signed-off-by: Reilly Grant > Signed-off-by: Emilio López Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/1] USB: core: let USB device know device node
On Tue, 16 Feb 2016, Peter Chen wrote: > From: Peter Chen > > Although most of USB devices are hot-plug's, there are still some devices > are hard wired on the board, eg, for HSIC and SSIC interface USB devices. > If these kinds of USB devices are multiple functions, and they can supply > other interfaces like i2c, gpios for other devices, we may need to > describe these at device tree. > > In this commit, it uses "reg" in dts as physical port number to match > the phyiscal port number decided by USB core, if they are the same, > then the device node is for the device we are creating for USB core. > > Signed-off-by: Peter Chen Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Thu, 18 Feb 2016, Daniel Fraga wrote: > On Thu, 18 Feb 2016 11:18:37 -0500 (EST) > Alan Stern wrote: > > > No idea. Can you collect a usbmon trace for bus 3, starting before the > > suspend and showing the problems? > > Ok Alan, I attached the requested usbmon output for bus 3 (before and > after S3). It looks like there's some problem in the usbhid driver. Apparently hid_start_in() gets some sort of error when it submits the input URB, because that URB doesn't show up in the usbmon output. Can you add a debugging line near the end of hid_post_reset() in drivers/hid/usbhid/hid-core.c to find out what the return value from hid_start_in() is? 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 2/5] usb: musb: core: added helper functions for parsing DT
On 02/18/2016 11:18 AM, Felipe Balbi wrote: This adds two functions to get DT properties "mentor,power" and "dr_mode": musb_get_power() and musb_mode musb_get_mode() Signed-off-by: Petr Kulhavy seems like I don't have patch 1/5. After fixing Sergei's comments, please resend with his Acked-by already in place. thanks Hi Felipe, I will do as soon as the patch 1/5 gets approved. It seem to be a bit stuck at the moment as Rob Herring from the DT wants the "mentor,power" to be represented as a regulator, whereas Sergei and me want to stick to the existing "mentor,power" integer property. As soon as this get clarified I will do the final updates and send the patch again. Maybe this is something you can help to clarify? I don't think that makes sense as a regulator. It's just a number which gets passed to USB Core as is. Well, in case of DaVinci's it's an external GPIO controlled regulator indeed. However, it seems like everything in kernel right now is passing it as 500. So why don't you deprecate that property, hardcode it to 500 and avoid the problem altogether ? OMAP boards can only supply 100 mA, AFAIK. Isn't it too early for the deprecation? :-) 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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Thu, 18 Feb 2016 14:30:15 -0500 (EST) Alan Stern wrote: > It looks like there's some problem in the usbhid driver. Apparently > hid_start_in() gets some sort of error when it submits the input URB, > because that URB doesn't show up in the usbmon output. > > Can you add a debugging line near the end of hid_post_reset() in > drivers/hid/usbhid/hid-core.c to find out what the return value from > hid_start_in() is? Yes, can you provide the patch (or the specific line you need) so I can recompile it? Thanks. -- Linux 4.4.1: Blurry Fish Butt http://www.youtube.com/DanielFragaBR http://exchangewar.info Bitcoin: 12H6661yoLDUZaYPdah6urZS5WiXwTAUgL -- 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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Thu, 18 Feb 2016, Daniel Fraga wrote: > On Thu, 18 Feb 2016 14:30:15 -0500 (EST) > Alan Stern wrote: > > > It looks like there's some problem in the usbhid driver. Apparently > > hid_start_in() gets some sort of error when it submits the input URB, > > because that URB doesn't show up in the usbmon output. > > > > Can you add a debugging line near the end of hid_post_reset() in > > drivers/hid/usbhid/hid-core.c to find out what the return value from > > hid_start_in() is? > > Yes, can you provide the patch (or the specific line you need) > so I can recompile it? Something like the patch below (untested). Alan Stern Index: usb-4.4/drivers/hid/usbhid/hid-core.c === --- usb-4.4.orig/drivers/hid/usbhid/hid-core.c +++ usb-4.4/drivers/hid/usbhid/hid-core.c @@ -1458,6 +1458,7 @@ static int hid_post_reset(struct usb_int spin_unlock_irq(&usbhid->lock); hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0); status = hid_start_in(hid); + dev_info(&intf->dev, "post reset hid_start_in -> %d\n", status); if (status < 0) hid_io_error(hid); usbhid_restart_queues(usbhid); -- 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: cdc_subset: only build when one driver is enabled
From: Arnd Bergmann Date: Wed, 17 Feb 2016 23:25:11 +0100 > This avoids a harmless randconfig warning I get when USB_NET_CDC_SUBSET > is enabled, but all of the more specific drivers are not: > > drivers/net/usb/cdc_subset.c:241:2: #warning You need to configure some > hardware for this driver > > The current behavior is clearly intentional, giving a warning when > a user picks a configuration that won't do anything good. The only > reason for even addressing this is that I'm getting close to > eliminating all 'randconfig' warnings on ARM, and this came up > a couple of times. > > My workaround is to not even build the module when none of the > configurations are enable. > > Alternatively we could simply remove the #warning (nothing wrong > for compile-testing), turn it into a runtime warning, or > change the Kconfig options into a menu to hide CONFIG_USB_NET_CDC_SUBSET. > > Signed-off-by: Arnd Bergmann Applied, thanks Arnd. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 00/11] Fixes and improvements to f_fs and f_midi
Hi, Michal Nazarewicz writes: > Resending my previous two sets for f_fs and f_midi. This time rebased > on top of Felipe’s next branch. > > Dan Carpenter (1): > usb: gadget: f_midi: missing unlock on error path > > Du, Changbin (1): > usb: f_fs: avoid race condition with ffs_epfile_io_complete > > Felipe F. Tonello (1): > usb: gadget: f_midi: remove useless midi reference from port struct > > Michal Nazarewicz (8): > usb: f_fs: fix memory leak when ep changes during transfer > usb: f_fs: fix ffs_epfile_io returning success on req alloc failure > usb: f_fs: replace unnecessary goto with a return > usb: f_fs: refactor ffs_epfile_io > usb: gadget: f_midi: move some of f_midi_transmit to separate func > usb: gadget: f_midi: fix in_last_port looping logic > usb: gadget: f_midi: use flexible array member for gmidi_in_port > elements > usb: gadget: f_midi: stash substream in gmidi_in_port structure looks like I don't have these patches in my inbox, care to resend ? thanks -- balbi signature.asc Description: PGP signature
Re: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Thu, 18 Feb 2016 15:23:00 -0500 (EST) Alan Stern wrote: > Something like the patch below (untested). > + dev_info(&intf->dev, "post reset hid_start_in -> %d\n", status); Ok, so I got the following: Feb 18 19:22:26 tux kernel: [ 258.693120] usb 3-1.6: reset full-speed USB device number 6 using ehci-pci Feb 18 19:22:26 tux kernel: [ 258.783654] usbhid 3-1.6:1.0: post reset hid_start_in -> -22 Feb 18 19:22:26 tux kernel: [ 259.883071] usb 3-1.6: reset full-speed USB device number 6 using ehci-pci Feb 18 19:22:27 tux kernel: [ 259.973529] usbhid 3-1.6:1.0: post reset hid_start_in -> -22 Feb 18 19:22:27 tux kernel: [ 260.036047] usb 3-1.6: reset full-speed USB device number 6 using ehci-pci Feb 18 19:22:27 tux kernel: [ 260.126642] usbhid 3-1.6:1.0: post reset hid_start_in -> -22 Feb 18 19:22:28 tux kernel: [ 260.188998] usb 3-1.6: reset full-speed USB device number 6 using ehci-pci Feb 18 19:22:28 tux kernel: [ 260.279629] usbhid 3-1.6:1.0: post reset hid_start_in -> -22 Feb 18 19:22:28 tux kernel: [ 260.342019] usb 3-1.6: reset full-speed USB device number 6 using ehci-pci Feb 18 19:22:28 tux kernel: [ 260.432742] usbhid 3-1.6:1.0: post reset hid_start_in -> -22 (...) And usbmon shows the following: 88024568da80 1732564945 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568da80 1732565020 C Ci:3:002:0 0 4 = 1101 88024568da80 1732575950 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568da80 1732576021 C Ci:3:002:0 0 4 = 03011000 88024568da80 1732576035 S Co:3:002:0 s 23 01 0014 0006 0 88024568da80 1732576154 C Co:3:002:0 0 0 88024568d540 1732626948 S Co:3:000:0 s 00 05 0006 0 88024568d540 1732627022 C Co:3:000:0 0 0 88024568d540 1732638972 S Ci:3:006:0 s 80 06 0100 0012 18 < 88024568d540 1732639272 C Ci:3:006:0 0 18 = 12010002 0008 f3043400 1200040e 0001 88024568d540 1732639292 S Ci:3:006:0 s 80 06 0200 0029 41 < 88024568d540 1732639519 C Ci:3:006:0 0 41 = 09022900 010100e0 32090400 00020300 0921 10010001 22b70307 05810340 88024568d540 1732639533 S Co:3:006:0 s 00 09 0001 0 88024568d540 1732639644 C Co:3:006:0 0 0 88024568d540 1732639671 S Ci:3:006:0 s 81 06 2200 03b7 951 < 88024568d540 1732644165 C Ci:3:006:0 0 951 = 050d0904 a1018501 0922a102 09421500 25017501 95018102 75018103 75060951 88024568d540 1732644193 S Co:3:006:0 s 21 0a 0 88024568d540 1732644269 C Co:3:006:0 0 0 88024568da80 1732644989 S Co:3:002:0 s 23 03 0004 0006 0 88024568da80 1732645146 C Co:3:002:0 0 0 88024568d480 1732655954 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568d480 1732656023 C Ci:3:002:0 0 4 = 1101 88024568da80 1732666962 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568da80 1732667024 C Ci:3:002:0 0 4 = 03011000 88024568da80 1732667029 S Co:3:002:0 s 23 01 0014 0006 0 88024568da80 1732667144 C Co:3:002:0 0 0 88024568d780 1732717984 S Ci:3:000:0 s 80 06 0100 0040 64 < 88024568d780 1732718148 C Ci:3:000:0 0 8 = 12010002 0008 88024568d780 1732718168 S Co:3:002:0 s 23 03 0004 0006 0 88024568d780 1732718271 C Co:3:002:0 0 0 88024568d780 1732728982 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568d780 1732729059 C Ci:3:002:0 0 4 = 1101 88024568d780 1732739957 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568d780 1732740022 C Ci:3:002:0 0 4 = 03011000 88024568d780 1732740030 S Co:3:002:0 s 23 01 0014 0006 0 88024568d780 1732740146 C Co:3:002:0 0 0 88024568d780 1732790980 S Co:3:000:0 s 00 05 0006 0 88024568d780 1732791149 C Co:3:000:0 0 0 88024568d9c0 1732802963 S Ci:3:006:0 s 80 06 0100 0012 18 < 88024568d9c0 1732803282 C Ci:3:006:0 0 18 = 12010002 0008 f3043400 1200040e 0001 88024568d9c0 1732803305 S Ci:3:006:0 s 80 06 0200 0029 41 < 88024568d9c0 1732803667 C Ci:3:006:0 0 41 = 09022900 010100e0 32090400 00020300 0921 10010001 22b70307 05810340 88024568d9c0 1732803675 S Co:3:006:0 s 00 09 0001 0 88024568d9c0 1732803770 C Co:3:006:0 0 0 88024568d9c0 1732803797 S Ci:3:006:0 s 81 06 2200 03b7 951 < 88024568d9c0 1732808292 C Ci:3:006:0 0 951 = 050d0904 a1018501 0922a102 09421500 25017501 95018102 75018103 75060951 88024568d9c0 1732808300 S Co:3:006:0 s 21 0a 0 88024568d9c0 1732808398 C Co:3:006:0 0 0 88024568d780 1732809002 S Co:3:002:0 s 23 03 0004 0006 0 88024568d780 1732809146 C Co:3:002:0 0 0 88024568d780 1732819951 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568d780 1732820023 C Ci:3:002:0 0 4 = 1101 88024568d9c0 1732830954 S Ci:3:002:0 s a3 00 0006 0004 4 < 88024568d9c0 1732831033 C Ci:3:002:0 0 4 = 03011000 88024568d9c0 1732831053 S Co:3:002:0 s 23 01 0014 0006 0 88024568d9c0 1732831147 C Co:3:002:0 0 0 88024568d0c0 1732881971 S Ci:3:000:0 s 80 06 0100 0040 64 <
[ANNOUNCE] patch queue empty
Hi folks, it looks like I've finally caught up with my patch queue. Please go through my testing/fixes and testing/next and if you don't find your patches in either branch, resend them as I, likely, don't have them anymore. cheers -- balbi signature.asc Description: PGP signature
Re: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter
On Thu, 18 Feb 2016 21:31:20 +0300
Sergei Shtylyov wrote:
> On 02/18/2016 09:06 PM, Antony Pavlov wrote:
[...]
> > so I use regulator in the TL-MR3020 board dts file:
> >
> > reg_usb_vbus: reg_usb_vbus {
> > compatible = "regulator-fixed";
> > regulator-name = "usb_vbus";
> > regulator-min-microvolt = <500>;
>
> Not 0?
>
> > regulator-max-microvolt = <500>;
> > gpio = <&gpio 8 GPIO_ACTIVE_HIGH>;
>
> Where's the switch if both voltages are equal?
Here is a quote from
linux/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
Any property defined as part of the core regulator
binding, defined in regulator.txt, can also be used.
However a fixed voltage regulator is expected to have the
regulator-min-microvolt and regulator-max-microvolt
to be the same.
Moreover please see this of_get_fixed_voltage_config() code fragment
(please see linux/drivers/regulator/fixed.c for details):
if (init_data->constraints.min_uV == init_data->constraints.max_uV) {
config->microvolts = init_data->constraints.min_uV;
} else {
dev_err(dev,
"Fixed regulator specified with variable voltages\n");
return ERR_PTR(-EINVAL);
}
--
Best regards,
Antony Pavlov
--
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: [PATCHv3 00/11] Fixes and improvements to f_fs and f_midi
> Michal Nazarewicz writes: >> Resending my previous two sets for f_fs and f_midi. This time rebased >> on top of Felipe’s next branch. >> >> Dan Carpenter (1): >> usb: gadget: f_midi: missing unlock on error path >> >> Du, Changbin (1): >> usb: f_fs: avoid race condition with ffs_epfile_io_complete >> >> Felipe F. Tonello (1): >> usb: gadget: f_midi: remove useless midi reference from port struct >> >> Michal Nazarewicz (8): >> usb: f_fs: fix memory leak when ep changes during transfer >> usb: f_fs: fix ffs_epfile_io returning success on req alloc failure >> usb: f_fs: replace unnecessary goto with a return >> usb: f_fs: refactor ffs_epfile_io >> usb: gadget: f_midi: move some of f_midi_transmit to separate func >> usb: gadget: f_midi: fix in_last_port looping logic >> usb: gadget: f_midi: use flexible array member for gmidi_in_port >> elements >> usb: gadget: f_midi: stash substream in gmidi_in_port structure On Thu, Feb 18 2016, Felipe Balbi wrote: > looks like I don't have these patches in my inbox, care to resend ? Patches freshly rebased on your next branch coming right up. Also available at: git fetch https://github.com/mina86/linux.git for-felipe -- Best regards Liege of Serenely Enlightened Majesty of Computer Science, ミハウ “mina86” ナザレヴイツ -- 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 19/19] usb: dwc3: Enable SuperSpeedPlus
On 2/17/2016 10:39 PM, Felipe Balbi wrote: > John Youn writes: >> On 2/17/2016 12:39 AM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> John Youn writes: Enable SuperSpeedPlus by programming the DCFG.speed and after enumerating, set gadget->speed appropriately. Signed-off-by: John Youn --- drivers/usb/dwc3/gadget.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 73db723..5bbdf5d 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1629,6 +1629,9 @@ static int dwc3_gadget_start(struct usb_gadget *g, case USB_SPEED_HIGH: reg |= DWC3_DSTS_HIGHSPEED; break; + case USB_SPEED_SUPER_PLUS: + reg |= DWC3_DSTS_SUPERSPEED_PLUS; + break; case USB_SPEED_SUPER: /* FALLTHROUGH */ case USB_SPEED_UNKNOWN: /* FALTHROUGH */ default: >>> >>> I'm thinking about amending this change to this patch: >>> >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index 5bbdf5d9c35e..d8566ad60d9b 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -1633,9 +1633,14 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>> reg |= DWC3_DSTS_SUPERSPEED_PLUS; >>> break; >>> case USB_SPEED_SUPER: /* FALLTHROUGH */ >>> + reg |= DWC3_DSTS_SUPERSPEED; >>> + break; >>> case USB_SPEED_UNKNOWN: /* FALTHROUGH */ >>> default: >>> - reg |= DWC3_DSTS_SUPERSPEED; >>> + if (dwc_is_usb31(dwc)) >>> + reg |= DWC3_DSTS_SUPERSPEED_PLUS; >>> + else >>> + reg |= DWC3_DSTS_SUPERSPEED; >>> } >>> } >>> dwc3_writel(dwc->regs, DWC3_DCFG, reg); >>> >>> the reason being that default speed should always be the fastest speed >>> supported by current core and, after this patchset we start support >>> superspeed plus. >>> >>> What do you think ? >>> >> >> Hi Felipe, >> >> That check is already done on the dwc->maximum_speed parameter in the >> probe when set to UNKNOWN. Though that check also involves checking >> the GHWPARAMS3.SSPHY_IF. Which, I think you reported before that some >> systems may not set correctly. I was thinking those systems can just >> set maximum_speed if needed. > > In that case, the default case statement here is unnecessary. > The maximum_speed is not fully validated. It is only adjusted on USB_SPEED_UNKNOWN. So if a user passes in some completely invalid value then and we could end up in the default condition here. I'll fix it so that can't occur. But even then, I think it is reasonable to handle the default condition in the switch, if only to guard against programmer error in the future. Maybe print an error and have some safe fall-back. Are you ok with that? Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PATCH v6 00/10] add HNP polling support for usb otg fsm
HNP polling is a mechanism which allows the OTG device currently acting as host to determine when the other attached OTG device wishes to take the host role. When an OTG host, which supports HNP, is connected to an OTG peripheral which also supports HNP it shall poll the peripheral regularly to determine whether it requires a role-swap and grant this at the earliest opportunity. This patchset adds OTG HNP polling support, and enable for chipidea usb otg fsm driver, more patches for pass OTG certification will come later. changes for v6: - Remove patch of disable irq while stop host role. - Split B_AIDL_BDIS timer patch to be 2 patches(9/10 and 10/10). - Add Peter's ack for patch 3, 7, 8/10. changes for v5: - Instead of use stack memory, use kmalloc to allocate memory for host request flag one byte buffer(DMA read), and add host_req_flag pointer in struct otg_fsm for reference to that flag buffer. - Add one patch to disable irq while stop host role(7/10). - Add one patch to fix B_AIDL_BDIS timing issue(10/10). changes for v4: - Add OTG HNP capable check for connected device before sending HNP polling in patch 3/8. - Add comment to explain HNP test update in chipidea.txt in patch 8/8. - Fix some typo. - Add Peter's Ack in patch 1,2,4,5,6,7/8 of the series. Li Jun (10): usb: gadget: add hnp_polling_support and host_request_flag in usb_gadget usb: add OTG status selector definition for HNP polling usb: common: otg-fsm: add HNP polling support usb: chipidea: udc: bypass otg status selector handling to gadget driver usb: gadget: composite: handle otg status selector request from OTG host usb: chipidea: otg: set host_request_flag for gadget usb: chipidea: otg: enable HNP polling support for gadget and host Documentation: usb: chipidea: Update test procedure for HNP polling usb: otg-fsm: add B_AIDL_BDIS timer usb: chipidea: otg: add A idle to B disconnect timer Documentation/usb/chipidea.txt | 9 +++-- drivers/usb/chipidea/otg_fsm.c | 29 -- drivers/usb/chipidea/otg_fsm.h | 2 + drivers/usb/chipidea/udc.c | 3 +- drivers/usb/common/usb-otg-fsm.c | 87 drivers/usb/gadget/composite.c | 25 include/linux/usb/gadget.h | 6 +++ include/linux/usb/otg-fsm.h | 15 +++ include/uapi/linux/usb/ch9.h | 1 + 9 files changed, 161 insertions(+), 16 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
[RESEND PATCH v6 01/10] usb: gadget: add hnp_polling_support and host_request_flag in usb_gadget
Add 2 flags for USB OTG HNP polling, hnp_polling_support is to indicate
if the gadget can support HNP polling, host_request_flag is used for
gadget to store host request information from application, which can be
used to respond to HNP polling from host.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
include/linux/usb/gadget.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..737d823 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -595,6 +595,10 @@ struct usb_gadget_ops {
* only supports HNP on a different root port.
* @b_hnp_enable: OTG device feature flag, indicating that the A-Host
* enabled HNP support.
+ * @hnp_polling_support: OTG device feature flag, indicating if the OTG device
+ * in peripheral mode can support HNP polling.
+ * @host_request_flag: OTG device feature flag, indicating if A-Peripheral
+ * or B-Peripheral wants to take host role.
* @quirk_ep_out_aligned_size: epout requires buffer size to be aligned to
* MaxPacketSize.
* @is_selfpowered: if the gadget is self-powered.
@@ -642,6 +646,8 @@ struct usb_gadget {
unsignedb_hnp_enable:1;
unsigneda_hnp_support:1;
unsigneda_alt_hnp_support:1;
+ unsignedhnp_polling_support:1;
+ unsignedhost_request_flag:1;
unsignedquirk_ep_out_aligned_size:1;
unsignedquirk_altset_not_supp:1;
unsignedquirk_stall_not_supp:1;
--
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
[RESEND PATCH v6 02/10] usb: add OTG status selector definition for HNP polling
A host is required to use the GetStatus command, with wIndex set to the
OTG status selector(F000H) to request the Host request flag from the
peripheral.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
include/uapi/linux/usb/ch9.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
index 4338eb7..a4deb1d 100644
--- a/include/uapi/linux/usb/ch9.h
+++ b/include/uapi/linux/usb/ch9.h
@@ -690,6 +690,7 @@ struct usb_otg20_descriptor {
#define USB_OTG_HNP(1 << 1)/* swap host/device roles */
#define USB_OTG_ADP(1 << 2)/* support ADP */
+#define OTG_STS_SELECTOR 0xF000 /* OTG status selector */
/*-*/
/* USB_DT_DEBUG: for special highspeed devices, replacing serial console */
--
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
[RESEND PATCH v6 03/10] usb: common: otg-fsm: add HNP polling support
Adds HNP polling timer when transits to host state, the OTG status
request will be sent to peripheral after timeout, if host request flag
is set, it will switch to peripheral state, otherwise it will repeat HNP
polling every 1.5s and maintain the current session.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
drivers/usb/common/usb-otg-fsm.c | 87
include/linux/usb/otg-fsm.h | 14 +++
2 files changed, 101 insertions(+)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 61d538a..504708f 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -78,6 +78,8 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
usb_otg_state old_state)
fsm->b_srp_done = 0;
break;
case OTG_STATE_B_PERIPHERAL:
+ if (fsm->otg->gadget)
+ fsm->otg->gadget->host_request_flag = 0;
break;
case OTG_STATE_B_WAIT_ACON:
otg_del_timer(fsm, B_ASE0_BRST);
@@ -107,6 +109,8 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
usb_otg_state old_state)
case OTG_STATE_A_PERIPHERAL:
otg_del_timer(fsm, A_BIDL_ADIS);
fsm->a_bidl_adis_tmout = 0;
+ if (fsm->otg->gadget)
+ fsm->otg->gadget->host_request_flag = 0;
break;
case OTG_STATE_A_WAIT_VFALL:
otg_del_timer(fsm, A_WAIT_VFALL);
@@ -120,6 +124,87 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
usb_otg_state old_state)
}
}
+static void otg_hnp_polling_work(struct work_struct *work)
+{
+ struct otg_fsm *fsm = container_of(to_delayed_work(work),
+ struct otg_fsm, hnp_polling_work);
+ struct usb_device *udev;
+ enum usb_otg_state state = fsm->otg->state;
+ u8 flag;
+ int retval;
+
+ if (state != OTG_STATE_A_HOST && state != OTG_STATE_B_HOST)
+ return;
+
+ udev = usb_hub_find_child(fsm->otg->host->root_hub, 1);
+ if (!udev) {
+ dev_err(fsm->otg->host->controller,
+ "no usb dev connected, can't start HNP polling\n");
+ return;
+ }
+
+ *fsm->host_req_flag = 0;
+ /* Get host request flag from connected USB device */
+ retval = usb_control_msg(udev,
+ usb_rcvctrlpipe(udev, 0),
+ USB_REQ_GET_STATUS,
+ USB_DIR_IN | USB_RECIP_DEVICE,
+ 0,
+ OTG_STS_SELECTOR,
+ fsm->host_req_flag,
+ 1,
+ USB_CTRL_GET_TIMEOUT);
+ if (retval != 1) {
+ dev_err(&udev->dev, "Get one byte OTG status failed\n");
+ return;
+ }
+
+ flag = *fsm->host_req_flag;
+ if (flag == 0) {
+ /* Continue HNP polling */
+ schedule_delayed_work(&fsm->hnp_polling_work,
+ msecs_to_jiffies(T_HOST_REQ_POLL));
+ return;
+ } else if (flag != HOST_REQUEST_FLAG) {
+ dev_err(&udev->dev, "host request flag %d is invalid\n", flag);
+ return;
+ }
+
+ /* Host request flag is set */
+ if (state == OTG_STATE_A_HOST) {
+ /* Set b_hnp_enable */
+ if (!fsm->otg->host->b_hnp_enable) {
+ retval = usb_control_msg(udev,
+ usb_sndctrlpipe(udev, 0),
+ USB_REQ_SET_FEATURE, 0,
+ USB_DEVICE_B_HNP_ENABLE,
+ 0, NULL, 0,
+ USB_CTRL_SET_TIMEOUT);
+ if (retval >= 0)
+ fsm->otg->host->b_hnp_enable = 1;
+ }
+ fsm->a_bus_req = 0;
+ } else if (state == OTG_STATE_B_HOST) {
+ fsm->b_bus_req = 0;
+ }
+
+ otg_statemachine(fsm);
+}
+
+static void otg_start_hnp_polling(struct otg_fsm *fsm)
+{
+ /*
+* The memory of host_req_flag should be allocated by
+* controller driver, otherwise, hnp polling is not started.
+*/
+ if (!fsm->host_req_flag)
+ return;
+
+ INIT_DELAYED_WORK(&fsm->hnp_polling_work, otg_hnp_polling_work);
+ schedule_delayed_work(&fsm->hnp_polling_work,
+ msecs_to_jiffies(T_HOST_REQ_POLL));
+}
+
/* Called when entering a state */
static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
{
@@ -169,6 +254,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
otg_set_protocol(fsm, PROTO_HOST);
usb_bus_start_
[RESEND PATCH v6 04/10] usb: chipidea: udc: bypass otg status selector handling to gadget driver
Since gadget driver will handle this request, so controller driver bypass it. Acked-by: Peter Chen Signed-off-by: Li Jun --- drivers/usb/chipidea/udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 00250ab..065f5d9 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1067,7 +1067,8 @@ __acquires(ci->lock) } break; case USB_REQ_GET_STATUS: - if (type != (USB_DIR_IN|USB_RECIP_DEVICE) && + if ((type != (USB_DIR_IN|USB_RECIP_DEVICE) || + le16_to_cpu(req.wIndex) == OTG_STS_SELECTOR) && type != (USB_DIR_IN|USB_RECIP_ENDPOINT) && type != (USB_DIR_IN|USB_RECIP_INTERFACE)) goto delegate; -- 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
[RESEND PATCH v6 07/10] usb: chipidea: otg: enable HNP polling support for gadget and host
Enable HNP polling support for chipidea gadget and allocate memory for host request flag when otg fsm init. Acked-by: Peter Chen Signed-off-by: Li Jun --- drivers/usb/chipidea/otg_fsm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c index cb28e76..9a963a7 100644 --- a/drivers/usb/chipidea/otg_fsm.c +++ b/drivers/usb/chipidea/otg_fsm.c @@ -797,6 +797,10 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci) ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0; ci->fsm.otg->state = OTG_STATE_UNDEFINED; ci->fsm.ops = &ci_otg_ops; + ci->gadget.hnp_polling_support = 1; + ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL); + if (!ci->fsm.host_req_flag) + return -ENOMEM; mutex_init(&ci->fsm.lock); -- 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
[RESEND PATCH v6 10/10] usb: chipidea: otg: add A idle to B disconnect timer
B-device detects that bus is idle for more than TB_AIDL_BDIS min and
begins HNP by turning off pullup on DP, this allows the bus to discharge
to the SE0 state. This timer was missed and failed with PET test:
6.8.5 B-UUT HNP of USB OTG and EH automated compliance plan v1.2,
this patch is to fix this timing issue.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
drivers/usb/chipidea/otg_fsm.c | 12 ++--
drivers/usb/chipidea/otg_fsm.h | 2 ++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 9a963a7..de8e22e 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -209,6 +209,7 @@ static unsigned otg_timer_ms[] = {
TA_AIDL_BDIS,
TB_ASE0_BRST,
TA_BIDL_ADIS,
+ TB_AIDL_BDIS,
TB_SE0_SRP,
TB_SRP_FAIL,
0,
@@ -320,6 +321,12 @@ static int a_bidl_adis_tmout(struct ci_hdrc *ci)
return 0;
}
+static int b_aidl_bdis_tmout(struct ci_hdrc *ci)
+{
+ ci->fsm.a_bus_suspend = 1;
+ return 0;
+}
+
static int b_se0_srp_tmout(struct ci_hdrc *ci)
{
ci->fsm.b_se0_srp = 1;
@@ -364,6 +371,7 @@ static int (*otg_timer_handlers[])(struct ci_hdrc *) = {
a_aidl_bdis_tmout, /* A_AIDL_BDIS */
b_ase0_brst_tmout, /* B_ASE0_BRST */
a_bidl_adis_tmout, /* A_BIDL_ADIS */
+ b_aidl_bdis_tmout, /* B_AIDL_BDIS */
b_se0_srp_tmout,/* B_SE0_SRP */
b_srp_fail_tmout, /* B_SRP_FAIL */
NULL, /* A_WAIT_ENUM */
@@ -655,9 +663,9 @@ static void ci_otg_fsm_event(struct ci_hdrc *ci)
break;
case OTG_STATE_B_PERIPHERAL:
if ((intr_sts & USBi_SLI) && port_conn && otg_bsess_vld) {
- fsm->a_bus_suspend = 1;
- ci_otg_queue_work(ci);
+ ci_otg_add_timer(ci, B_AIDL_BDIS);
} else if (intr_sts & USBi_PCI) {
+ ci_otg_del_timer(ci, B_AIDL_BDIS);
if (fsm->a_bus_suspend == 1)
fsm->a_bus_suspend = 0;
}
diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
index 262d6ef..6366fe3 100644
--- a/drivers/usb/chipidea/otg_fsm.h
+++ b/drivers/usb/chipidea/otg_fsm.h
@@ -62,6 +62,8 @@
/* SSEND time before SRP */
#define TB_SSEND_SRP (1500)/* minimum 1.5 sec, section:5.1.2 */
+#define TB_AIDL_BDIS (20) /* 4ms ~ 150ms, section 5.2.1 */
+
#if IS_ENABLED(CONFIG_USB_OTG_FSM)
int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
--
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
[RESEND PATCH v6 09/10] usb: otg-fsm: add B_AIDL_BDIS timer
Add A-idle to B-disconnect timer, B-device detects that bus is idle
for more than TB_AIDL_BDIS min and begins HNP by turning off pullup
on D+. This allows the bus to discharge to the SE0 state.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
include/linux/usb/otg-fsm.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 3059a95..24198e1 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -60,6 +60,7 @@ enum otg_fsm_timer {
A_AIDL_BDIS,
B_ASE0_BRST,
A_BIDL_ADIS,
+ B_AIDL_BDIS,
/* Auxiliary timers */
B_SE0_SRP,
--
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
[RESEND PATCH v6 05/10] usb: gadget: composite: handle otg status selector request from OTG host
If gadget with HNP polling support receives GetStatus request of otg
status selector, it feedback to host with host request flag to indicate
if it wants to take host role.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
drivers/usb/gadget/composite.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 8b14c2a..709a4df 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1634,15 +1634,24 @@ composite_setup(struct usb_gadget *gadget, const struct
usb_ctrlrequest *ctrl)
*((u8 *)req->buf) = value;
value = min(w_length, (u16) 1);
break;
-
- /*
-* USB 3.0 additions:
-* Function driver should handle get_status request. If such cb
-* wasn't supplied we respond with default value = 0
-* Note: function driver should supply such cb only for the first
-* interface of the function
-*/
case USB_REQ_GET_STATUS:
+ if (gadget_is_otg(gadget) && gadget->hnp_polling_support &&
+ (w_index == OTG_STS_SELECTOR)) {
+ if (ctrl->bRequestType != (USB_DIR_IN |
+ USB_RECIP_DEVICE))
+ goto unknown;
+ *((u8 *)req->buf) = gadget->host_request_flag;
+ value = 1;
+ break;
+ }
+
+ /*
+* USB 3.0 additions:
+* Function driver should handle get_status request. If such cb
+* wasn't supplied we respond with default value = 0
+* Note: function driver should supply such cb only for the
+* first interface of the function
+*/
if (!gadget_is_superspeed(gadget))
goto unknown;
if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
--
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
[RESEND PATCH v6 08/10] Documentation: usb: chipidea: Update test procedure for HNP polling
Update HNP test procedure as HNP polling is supported. Acked-by: Peter Chen Signed-off-by: Li Jun --- Documentation/usb/chipidea.txt | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt index 05f735a..678741b 100644 --- a/Documentation/usb/chipidea.txt +++ b/Documentation/usb/chipidea.txt @@ -26,16 +26,17 @@ cat /sys/kernel/debug/ci_hdrc.0/registers On B-device: echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req - if HNP polling is not supported, also need: - On A-device: - echo 0 > /sys/bus/platform/devices/ci_hdrc.0/inputs/a_bus_req - B-device should take host role and enumrate A-device. 4) A-device switch back to host. On B-device: echo 0 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req + or, by introducing HNP polling, B-Host can know when A-peripheral wish + to be host role, so this role switch also can be trigged in A-peripheral + side by answering the polling from B-Host, this can be done on A-device: + echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/a_bus_req + A-device should switch back to host and enumrate B-device. 5) Remove B-device(unplug micro B plug) and insert again in 10 seconds, -- 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
[RESEND PATCH v6 06/10] usb: chipidea: otg: set host_request_flag for gadget
Set host_request_flag if the current peripheral wants to take host role
via changing a_bus_req or b_bus_req by user application.
Acked-by: Peter Chen
Signed-off-by: Li Jun
---
drivers/usb/chipidea/otg_fsm.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index ba90dc6..cb28e76 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -66,6 +66,11 @@ set_a_bus_req(struct device *dev, struct device_attribute
*attr,
return count;
}
ci->fsm.a_bus_req = 1;
+ if (ci->fsm.otg->state == OTG_STATE_A_PERIPHERAL) {
+ ci->gadget.host_request_flag = 1;
+ mutex_unlock(&ci->fsm.lock);
+ return count;
+ }
}
ci_otg_queue_work(ci);
@@ -144,8 +149,14 @@ set_b_bus_req(struct device *dev, struct device_attribute
*attr,
mutex_lock(&ci->fsm.lock);
if (buf[0] == '0')
ci->fsm.b_bus_req = 0;
- else if (buf[0] == '1')
+ else if (buf[0] == '1') {
ci->fsm.b_bus_req = 1;
+ if (ci->fsm.otg->state == OTG_STATE_B_PERIPHERAL) {
+ ci->gadget.host_request_flag = 1;
+ mutex_unlock(&ci->fsm.lock);
+ return count;
+ }
+ }
ci_otg_queue_work(ci);
mutex_unlock(&ci->fsm.lock);
--
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 v6 00/10] add HNP polling support for usb otg fsm
Hi Felipe I didn't get any response for this request, if you need a resend of the whole patchset, please let me know. Li Jun > -Original Message- > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > ow...@vger.kernel.org] On Behalf Of Jun Li > Sent: Monday, February 15, 2016 10:20 AM > To: Felipe Balbi > Cc: linux-usb@vger.kernel.org; Peter Chen ; > hzpeterc...@gmail.com; gre...@linuxfoundation.org > Subject: RE: [PATCH v6 00/10] add HNP polling support for usb otg fsm > > Resend with Felipe's new email address > > > -Original Message- > > From: Jun Li > > Sent: Monday, February 15, 2016 10:16 AM > > To: ba...@ti.com > > Cc: linux-usb@vger.kernel.org; Peter Chen ; > > hzpeterc...@gmail.com; gre...@linuxfoundation.org; Jun Li > > > > Subject: RE: [PATCH v6 00/10] add HNP polling support for usb otg fsm > > > > Ping... > > > > > -Original Message- > > > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- > > > ow...@vger.kernel.org] On Behalf Of Jun Li > > > Sent: Thursday, January 28, 2016 2:50 PM > > > To: ba...@ti.com > > > Cc: linux-usb@vger.kernel.org; Peter Chen ; > > > hzpeterc...@gmail.com; gre...@linuxfoundation.org > > > Subject: RE: [PATCH v6 00/10] add HNP polling support for usb otg > > > fsm > > > > > > Hi Felipe, > > > > > > > -Original Message- > > > > From: Jun Li > > > > Sent: Tuesday, January 26, 2016 3:19 PM > > > > To: Peter Chen ; hzpeterc...@gmail.com; > > > > ba...@ti.com; gre...@linuxfoundation.org > > > > Cc: linux-usb@vger.kernel.org; Jun Li > > > > Subject: [PATCH v6 00/10] add HNP polling support for usb otg fsm > > > > > > > > HNP polling is a mechanism which allows the OTG device currently > > > > acting as host to determine when the other attached OTG device > > > > wishes to take the host role. When an OTG host, which supports > > > > HNP, is connected to an OTG peripheral which also supports HNP it > > > > shall poll the peripheral regularly to determine whether it > > > > requires a role-swap and grant this at the earliest opportunity. > > > > This patchset adds OTG HNP polling support, and enable for > > > > chipidea usb otg fsm driver, more patches for pass OTG > > > > certification will come > > > later. > > > > > > > > changes for v6: > > > > - Remove patch of disable irq while stop host role. > > > > - Split B_AIDL_BDIS timer patch to be 2 patches(9/10 and 10/10). > > > > - Add Peter's ack for patch 3, 7, 8/10. > > > > > > > > changes for v5: > > > > - Instead of use stack memory, use kmalloc to allocate memory for > host > > > > request flag one byte buffer(DMA read), and add host_req_flag > > pointer > > > > in struct otg_fsm for reference to that flag buffer. > > > > - Add one patch to disable irq while stop host role(7/10). > > > > - Add one patch to fix B_AIDL_BDIS timing issue(10/10). > > > > > > > > changes for v4: > > > > - Add OTG HNP capable check for connected device before sending > > > > HNP polling > > > > in patch 3/8. > > > > - Add comment to explain HNP test update in chipidea.txt in patch > 8/8. > > > > - Fix some typo. > > > > - Add Peter's Ack in patch 1,2,4,5,6,7/8 of the series. > > > > > > > > Li Jun (10): > > > > usb: gadget: add hnp_polling_support and host_request_flag in > > > > usb_gadget > > > > usb: add OTG status selector definition for HNP polling > > > > usb: common: otg-fsm: add HNP polling support > > > > usb: chipidea: udc: bypass otg status selector handling to gadget > > > > driver > > > > usb: gadget: composite: handle otg status selector request from > OTG > > > > host > > > > usb: chipidea: otg: set host_request_flag for gadget > > > > usb: chipidea: otg: enable HNP polling support for gadget and host > > > > Documentation: usb: chipidea: Update test procedure for HNP > polling > > > > usb: otg-fsm: add B_AIDL_BDIS timer > > > > usb: chipidea: otg: add A idle to B disconnect timer > > > > > > > > Documentation/usb/chipidea.txt | 9 +++-- > > > > drivers/usb/chipidea/otg_fsm.c | 29 -- > > > > drivers/usb/chipidea/otg_fsm.h | 2 + > > > > drivers/usb/chipidea/udc.c | 3 +- > > > > drivers/usb/common/usb-otg-fsm.c | 87 > > > > > > > > drivers/usb/gadget/composite.c | 25 > > > > include/linux/usb/gadget.h | 6 +++ > > > > include/linux/usb/otg-fsm.h | 15 +++ > > > > include/uapi/linux/usb/ch9.h | 1 + > > > > 9 files changed, 161 insertions(+), 16 deletions(-) > > > > > > > > -- > > > > 1.9.1 > > > > > > Could you please take look this series again? Basically this is a > > > updated version based on v4 you reviewed, for common and gadget > > > part, the only major change is use dedicated buffer through kmalloc > > > instead of use a stack variable for host request flag(usb control > > > transfer, DMA > > read). > > > Other changes are for chipidea driver. > > > The common HNP polling implementation is generic so does not depend > > > on
[PATCH 0/2] usb: Use ARCH_RENESAS
Hi, this short series makes use of of ARCH_RENESAS in place of ARCH_SHMOBILE. This is part of an ongoing process to migrate from ARCH_SHMOBILE to ARCH_RENESAS the motivation for which being that RENESAS seems to be a more appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs. Simon Horman (2): usb: renesas_usbhs: Use ARCH_RENESAS usb: host: xhci-rcar: Use ARCH_RENESAS drivers/usb/host/Kconfig | 2 +- drivers/usb/renesas_usbhs/Kconfig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.1.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 1/2] usb: renesas_usbhs: Use ARCH_RENESAS
Make use of ARCH_RENESAS in place of ARCH_SHMOBILE. This is part of an ongoing process to migrate from ARCH_SHMOBILE to ARCH_RENESAS the motivation for which being that RENESAS seems to be a more appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs. Signed-off-by: Simon Horman --- drivers/usb/renesas_usbhs/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/renesas_usbhs/Kconfig b/drivers/usb/renesas_usbhs/Kconfig index ebc99ee076ce..b26d7c339c05 100644 --- a/drivers/usb/renesas_usbhs/Kconfig +++ b/drivers/usb/renesas_usbhs/Kconfig @@ -5,7 +5,7 @@ config USB_RENESAS_USBHS tristate 'Renesas USBHS controller' depends on USB_GADGET - depends on ARCH_SHMOBILE || SUPERH || COMPILE_TEST + depends on ARCH_RENESAS || SUPERH || COMPILE_TEST depends on EXTCON || !EXTCON # if EXTCON=m, USBHS cannot be built-in default n help -- 2.1.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 2/2] usb: host: xhci-rcar: Use ARCH_RENESAS
Make use of ARCH_RENESAS in place of ARCH_SHMOBILE. This is part of an ongoing process to migrate from ARCH_SHMOBILE to ARCH_RENESAS the motivation for which being that RENESAS seems to be a more appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs. Signed-off-by: Simon Horman --- drivers/usb/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 438dcf6289b0..ed9a90f601e8 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -64,7 +64,7 @@ config USB_XHCI_MVEBU config USB_XHCI_RCAR tristate "xHCI support for Renesas R-Car SoCs" select USB_XHCI_PLATFORM - depends on ARCH_SHMOBILE || COMPILE_TEST + depends on ARCH_RENESAS || COMPILE_TEST ---help--- Say 'Y' to enable the support for the xHCI host controller found in Renesas R-Car ARM SoCs. -- 2.1.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
Corsair HIDs not properly initializing
see ccMSC's CKB git project for more details (troubleshooting section) Many of the new corsair HID devices (mice and keyboards) fail to initialize correctly on kernel bootup. This is the workaround used to make them work. (Some device ID's, like for the K60 are missing) The code I use in the kernel commandline to bypass the issue on my K95-RGB looks like so: usbhid.quirks=0x1B1C:0x1B11:0x408 The issue has been confirmed on recent (2016) Arch and Fedora kernels for K95-RGB and on Ubuntu's El Capitan with K60. Please add support for these devices, and look into adding support for devices not listed in the above page (K60, K60-RGB, K50, K40, K30, K65(Non-RGB), K90, and the KATAR mouse; there may be some devices I left out but this should be the brunt of their current device names) -- 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: Corsair HIDs not properly initializing
On Fri, Feb 19, 2016 at 03:44:29AM +, Cestarian Inhabitant wrote: > see ccMSC's CKB git project for more details (troubleshooting section) What is that? > Many of the new corsair HID devices (mice and keyboards) fail to > initialize correctly on kernel bootup. This is the workaround used to > make them work. > > (Some device ID's, like for the K60 are missing) Missing where? > The code I use in the kernel commandline to bypass the issue on my > K95-RGB looks like so: usbhid.quirks=0x1B1C:0x1B11:0x408 > > The issue has been confirmed on recent (2016) Arch and Fedora kernels > for K95-RGB and on Ubuntu's El Capitan with K60. > > Please add support for these devices, and look into adding support for > devices not listed in the above page (K60, K60-RGB, K50, K40, K30, > K65(Non-RGB), K90, and the KATAR mouse; there may be some devices I > left out but this should be the brunt of their current device names) What page are you referring to here? thanks, 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] usb: chipidea: fix return value check in ci_hdrc_pci_probe()
On Tue, Feb 16, 2016 at 05:12:51PM +0800, Peter Chen wrote: > On Sat, Feb 06, 2016 at 10:57:06PM +0800, weiyj...@163.com wrote: > > From: Wei Yongjun > > > > In case of error, the function usb_phy_generic_register() > > returns ERR_PTR() and never returns NULL. The NULL test in > > the return value check should be replaced with IS_ERR(). > > > > Signed-off-by: Wei Yongjun > > --- > > drivers/usb/chipidea/ci_hdrc_pci.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c > > b/drivers/usb/chipidea/ci_hdrc_pci.c > > index b59195e..b635ab6 100644 > > --- a/drivers/usb/chipidea/ci_hdrc_pci.c > > +++ b/drivers/usb/chipidea/ci_hdrc_pci.c > > @@ -85,8 +85,8 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev, > > > > /* register a nop PHY */ > > ci->phy = usb_phy_generic_register(); > > - if (!ci->phy) > > - return -ENOMEM; > > + if (IS_ERR(ci->phy)) > > + return PTR_ERR(ci->phy); > > > > memset(res, 0, sizeof(res)); > > res[0].start= pci_resource_start(pdev, 0); > > > > The original code has several issues: > First, usb_phy_generic_register returns pointers of platform_device > Second, the generic USB PHY driver is related to struct usb_phy, but > not struct phy. > So, in this code, it needs to use ci->usb_phy to get the pointer > of struct usb_phy used at generic USB PHY driver. > > Would you mind sending the patches to fix them? > oops, The ci in this patch is struct ci_hdrc_pci *, but not struct ci_hdrc *. Your patch is ok, I will queue it, thanks. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] USB Chipidea fixes for v4.5-rc5
The following changes since commit 18558cae0272f8fd9647e69d3fec1565a7949865: Linux 4.5-rc4 (2016-02-14 13:05:20 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ tags/usb-ci-v4.5-rc5 for you to fetch changes up to 8c0614ca312c847ca5409ea11c39434dec69631d: usb: chipidea: fix return value check in ci_hdrc_pci_probe() (2016-02-19 14:13:44 +0800) Some tiny bug fixes for chipidea driver Alan (1): usb: chipidea: error on overflow for port_test_write Wei Yongjun (1): usb: chipidea: fix return value check in ci_hdrc_pci_probe() drivers/usb/chipidea/ci_hdrc_pci.c | 4 ++-- drivers/usb/chipidea/debug.c | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 04/10] usb: dbc: add debug buffer
On 02/18/2016 07:43 PM, Mathias Nyman wrote:
> On 26.01.2016 14:58, Lu Baolu wrote:
>> "printk" is not suitable for dbc debugging especially when console
>> is in usage. This patch adds a debug buffer in dbc driver and puts
>> the debug messages in this local buffer. The debug buffer could be
>> dumped whenever the console is not in use. This part of code will
>> not be visible unless DBC_DEBUG is defined.
>>
>> Signed-off-by: Lu Baolu
>> ---
>> drivers/usb/early/xhci-dbc.c | 62
>> ++--
>> 1 file changed, 60 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
>> index 41ce116..6855048 100644
>> --- a/drivers/usb/early/xhci-dbc.c
>> +++ b/drivers/usb/early/xhci-dbc.c
>> @@ -32,8 +32,64 @@ static struct xdbc_state xdbc_stat;
>> static struct xdbc_state *xdbcp = &xdbc_stat;
>>
>> #ifdef DBC_DEBUG
>> -/* place holder */
>> -#definexdbc_traceprintk
>> +#defineXDBC_DEBUG_BUF_SIZE(PAGE_SIZE * 32)
>
> Does it really need to be this huge? minimum 4096 * 32 ~ 128k
> The kernel ring buffer is about the same size (16k - 256k)
This debug buffer is only used when DBC_DEBUG is defined.
128k is a little large. I will change it to 16k.
>
>> +#defineMSG_MAX_LINE128
>
> with 128 characters per line this would fit ~1000 lines
>
>> +static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
>> +static void xdbc_trace(const char *fmt, ...)
>> +{
>> +int i, size;
>> +va_list args;
>> +static int pos;
>> +char temp_buf[MSG_MAX_LINE];
>> +
>> +if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
>> +return;
>> +
>> +memset(temp_buf, 0, MSG_MAX_LINE);
>> +va_start(args, fmt);
>> +vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
>> +va_end(args);
>> +
>> +i = 0;
>> +size = strlen(temp_buf);
>> +while (i < size) {
>> +xdbc_debug_buf[pos] = temp_buf[i];
>> +pos++;
>> +i++;
>> +
>> +if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
>> +break;
>> +}
>
> how about something like:
>
> size = min(XDBC_DEBUG_BUF_SIZE - pos, size)
> memcpy(xdbc_debug_buf + pos, temp_buf, size)
> pos += size;
>
> (might need some "-1" and off by one checking..)
Yes. This looks better.
>
>> +}
>> +
>> +static void xdbc_dump_debug_buffer(void)
>> +{
>> +int index = 0;
>> +int count = 0;
>> +char dump_buf[MSG_MAX_LINE];
>> +
>> +xdbc_trace("The end of DbC trace buffer\n");
>> +pr_notice("DBC debug buffer:\n");
>> +memset(dump_buf, 0, MSG_MAX_LINE);
>> +
>> +while (index < XDBC_DEBUG_BUF_SIZE) {
>> +if (!xdbc_debug_buf[index])
>> +break;
>> +
>> +if (xdbc_debug_buf[index] == '\n' ||
>> +count >= MSG_MAX_LINE - 1) {
>> +pr_notice("DBC: @%08x %s\n", index, dump_buf);
>
> Is showing the he index (position in debug buffer) useful here?
It helps to check the debug log especially when it shows the hardware
registers or memory block.
>
>
>> +memset(dump_buf, 0, MSG_MAX_LINE);
>> +count = 0;
>> +} else {
>> +dump_buf[count] = xdbc_debug_buf[index];
>> +count++;
>> +}
>> +
>> +index++;
>> +}
>
> So we have one huge buffer that xdbc keeps on filling as the initialization
> progresses.
> It is never emptied, or overwritten (circular).
> When dumped it always dumps the whole thing, copying one character
> at a time.
>
> As this is only used for debugging during xdbc development/debugging, and
> never enabled
> even if xdbc early printk is used, I don't think optimization really matters.
Yes, it's only a helper for the persons who develop and debug this driver.
>
> Perhaps take a look if we really need PAGE_SIZE * 32 bytes, is xdbc driver
> even nearly
> writing that much debug data.
I will reduce the debug buffer size.
>
> -Mathias
>
>
Thanks for your time.
Regards,
-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: driver migration
Hello I configured and setup a more recent kernel: V4.5.0-rc4 The driver compiles and inserts. When I plugin the USB-Dongle, the machine freezes (not sure whether it freezes during the firmware download or after it once it has reenumerated). I could observe a kernel oops message starting with BUG. Probably a null pointer dereference... Cheers Tilman -- 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 19/19] usb: dwc3: Enable SuperSpeedPlus
Hi, John Youn writes: diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 5bbdf5d9c35e..d8566ad60d9b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1633,9 +1633,14 @@ static int dwc3_gadget_start(struct usb_gadget *g, reg |= DWC3_DSTS_SUPERSPEED_PLUS; break; case USB_SPEED_SUPER: /* FALLTHROUGH */ + reg |= DWC3_DSTS_SUPERSPEED; + break; case USB_SPEED_UNKNOWN: /* FALTHROUGH */ default: - reg |= DWC3_DSTS_SUPERSPEED; + if (dwc_is_usb31(dwc)) + reg |= DWC3_DSTS_SUPERSPEED_PLUS; + else + reg |= DWC3_DSTS_SUPERSPEED; } } dwc3_writel(dwc->regs, DWC3_DCFG, reg); the reason being that default speed should always be the fastest speed supported by current core and, after this patchset we start support superspeed plus. What do you think ? >>> >>> Hi Felipe, >>> >>> That check is already done on the dwc->maximum_speed parameter in the >>> probe when set to UNKNOWN. Though that check also involves checking >>> the GHWPARAMS3.SSPHY_IF. Which, I think you reported before that some >>> systems may not set correctly. I was thinking those systems can just >>> set maximum_speed if needed. >> >> In that case, the default case statement here is unnecessary. >> > > The maximum_speed is not fully validated. It is only adjusted on > USB_SPEED_UNKNOWN. So if a user passes in some completely invalid > value then and we could end up in the default condition here. > > I'll fix it so that can't occur. > > But even then, I think it is reasonable to handle the default > condition in the switch, if only to guard against programmer error in > the future. Maybe print an error and have some safe fall-back. Are you > ok with that? sounds ok to me. :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH v7 05/10] usb: dbc: add bulk out and bulk in interfaces
On 02/18/2016 09:32 PM, Mathias Nyman wrote:
> On 26.01.2016 14:58, Lu Baolu wrote:
>> This patch adds interfaces for bulk out and bulk in ops. These
>> interfaces could be used to implement early printk bootconsole
>> or hook to various system debuggers.
>>
>> Signed-off-by: Lu Baolu
>> ---
>> drivers/usb/early/xhci-dbc.c | 373
>> +++
>> include/linux/usb/xhci-dbc.h | 30
>> 2 files changed, 403 insertions(+)
>>
>
> ...
>
>> +
>> +/*
>> + * Check and dispatch events in event ring. It also checks status
>> + * of hardware. This function will be called from multiple threads.
>> + * An atomic lock is applied to protect the access of event ring.
>> + */
>> +static int xdbc_check_event(void)
>> +{
>> +/* event ring is under checking by other thread? */
>> +if (!test_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags) &&
>> +!test_and_set_bit(XDBC_ATOMIC_EVENT,
>> +&xdbcp->atomic_flags))
>> +return 0;
>
> homemade trylock, can't the real ones be used?
>
>> +
>> +xdbc_handle_events();
>> +
>> +test_and_clear_bit(XDBC_ATOMIC_EVENT, &xdbcp->atomic_flags);
>> +
>> +return 0;
>> +}
>> +
>> +#defineBULK_IN_COMPLETED(p)((xdbcp->in_pending == (p)) && \
>> + xdbcp->in_complete)
>> +#defineBULK_OUT_COMPLETED(p)((xdbcp->out_pending == (p)) && \
>> + xdbcp->out_complete)
>> +
>
> ...
>
>> +}
>> +
>> +int xdbc_bulk_read(void *data, int size, int loops)
>> +{
>> +int ret;
>> +
>> +do {
>> +if (!test_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags) &&
>> +!test_and_set_bit(XDBC_ATOMIC_BULKIN,
>> +&xdbcp->atomic_flags))
>> +break;
>> +} while (1);
>
> homemeade spin_lock, can't the real one be used?
>
> If the xdbc_bulk_write() can be accessed from interrupt context (handler,
> soft, timer) it
> may deadlock
>
>> +
>> +ret = xdbc_bulk_transfer(data, size, loops, true);
>> +
>> +test_and_clear_bit(XDBC_ATOMIC_BULKIN, &xdbcp->atomic_flags);
>> +
>> +return ret;
>> +}
>> +
>> +int xdbc_bulk_write(const char *bytes, int size)
>> +{
>> +int ret;
>> +
>> +do {
>> +if (!test_bit(XDBC_ATOMIC_BULKOUT, &xdbcp->atomic_flags) &&
>> +!test_and_set_bit(XDBC_ATOMIC_BULKOUT,
>> +&xdbcp->atomic_flags))
>> +break;
>> +} while (1);
>
> Another homemeade spin_lock, can't the real one be used?
>
> same issue here, deadlock if accessible from interrupt context.
I will try to rework this spin_lock with the real one and keep avoiding
deadlock in mind.
>
>
> Would it make sense to have only one spinlock, and start one separate thread
> for
> reading the event ring. The thread would, lock, handle pending events,
> unlock,
> then call shedule, in a loop. ehci early debug code has some variant of this.
Let me try to find this part of code.
>
> So the lock would be taken while events are being handled.
>
> The same lock would be used for bulk_read and bulk_write. Yes this would
> prevent read and
> write at the same time, and the read and writes need to be modified to not
> block until
> the reansfer is finished, just to write the TRBs on the ring, update ring
> pointers,
> and ring the doorbell.
>
> Or is all this impossibe due to the earlyness of the code?
It's not only due to earlyness of the code. But also, these read/write ops were
designed to
be used by a debugger (for example kgdb) as well. Using the kernel provided
interface
might make things simple, but what should happen when the debugger is used
to debug the kernel subsystem itself?
So, it seems that I should implement read/write ops depends on the use case.
For this
time being, let's focus on the boot console case.
Some transfers take place when the thread/lock subsystem is not initialized yet.
But after thread/lock subsystem is able to be used, we are able to use the real
one.
Let me wrapper them in functions. For the transfers taken place before the
subsystem
initialization (that's single thread context, no worry about deadlock), it will
use the
current methods (it might be possible to drop lock due the single thread
context),
and after the subsystem being initialized, it will use those provided by the
kernel.
>
> -Mathias
>
Very appreciated for your time.
Regards,
-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: [PATCHv3 00/11] Fixes and improvements to f_fs and f_midi
Hi, Michal Nazarewicz writes: >> Michal Nazarewicz writes: >>> Resending my previous two sets for f_fs and f_midi. This time rebased >>> on top of Felipe’s next branch. >>> >>> Dan Carpenter (1): >>> usb: gadget: f_midi: missing unlock on error path >>> >>> Du, Changbin (1): >>> usb: f_fs: avoid race condition with ffs_epfile_io_complete >>> >>> Felipe F. Tonello (1): >>> usb: gadget: f_midi: remove useless midi reference from port struct >>> >>> Michal Nazarewicz (8): >>> usb: f_fs: fix memory leak when ep changes during transfer >>> usb: f_fs: fix ffs_epfile_io returning success on req alloc failure >>> usb: f_fs: replace unnecessary goto with a return >>> usb: f_fs: refactor ffs_epfile_io >>> usb: gadget: f_midi: move some of f_midi_transmit to separate func >>> usb: gadget: f_midi: fix in_last_port looping logic >>> usb: gadget: f_midi: use flexible array member for gmidi_in_port >>> elements >>> usb: gadget: f_midi: stash substream in gmidi_in_port structure > > On Thu, Feb 18 2016, Felipe Balbi wrote: >> looks like I don't have these patches in my inbox, care to resend ? > > Patches freshly rebased on your next branch coming right up. Also > available at: > > git fetch https://github.com/mina86/linux.git for-felipe thanks, I'll fetch your branch and apply the patches individually :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH 2/5] usb: musb: core: added helper functions for parsing DT
Hi, Sergei Shtylyov writes: > On 02/18/2016 11:18 AM, Felipe Balbi wrote: > > This adds two functions to get DT properties "mentor,power" and "dr_mode": > musb_get_power() and musb_mode musb_get_mode() > > Signed-off-by: Petr Kulhavy seems like I don't have patch 1/5. After fixing Sergei's comments, please resend with his Acked-by already in place. thanks >>> Hi Felipe, >>> >>> I will do as soon as the patch 1/5 gets approved. >>> It seem to be a bit stuck at the moment as Rob Herring from the DT wants >>> the "mentor,power" >>> to be represented as a regulator, whereas Sergei and me want to stick to >>> the existing "mentor,power" integer property. >>> >>> As soon as this get clarified I will do the final updates and send the >>> patch again. >>> Maybe this is something you can help to clarify? >> >> I don't think that makes sense as a regulator. It's just a number which >> gets passed to USB Core as is. > > Well, in case of DaVinci's it's an external GPIO controlled > regulator indeed. oh, I see. Not controller by SetPortPower. That's a shame. >> However, it seems like everything in kernel right now is passing it as >> 500. So why don't you deprecate that property, hardcode it to 500 and >> avoid the problem altogether ? > > OMAP boards can only supply 100 mA, AFAIK. Isn't it too early for the > deprecation? :-) $ git --no-pager grep -e mentor,power Documentation/devicetree/bindings/usb/am33xx-usb.txt:44:- mentor,power: Should be "500". This signifies the controller can supply up to Documentation/devicetree/bindings/usb/am33xx-usb.txt:112: mentor,power = <500>; Documentation/devicetree/bindings/usb/am33xx-usb.txt:157: mentor,power = <500>; arch/arm/boot/dts/am33xx.dtsi:580: mentor,power = <500>; arch/arm/boot/dts/am33xx.dtsi:627: mentor,power = <500>; arch/arm/boot/dts/dm814x.dtsi:91: mentor,power = <500>; arch/arm/boot/dts/dm814x.dtsi:129: mentor,power = <500>; arch/arm/boot/dts/dm816x.dtsi:414: mentor,power = <500>; arch/arm/boot/dts/dm816x.dtsi:454: mentor,power = <500>; drivers/usb/musb/musb_dsps.c:744: pdata.power = get_int_prop(dn, "mentor,power") / 2; Even documentation says it _must_ be 500. -- balbi signature.asc Description: PGP signature
