[PATCH] net: usb: Merge cpu_to_le32s + memcpy to put_unaligned_le32
Merge the combo uses of cpu_to_le32s and memcpy.
Use put_unaligned_le32 instead.
This simplifies the code.
Signed-off-by: Chuhong Yuan
---
drivers/net/usb/asix_common.c | 9 -
drivers/net/usb/ax88179_178a.c | 11 ---
drivers/net/usb/lan78xx.c | 11 ---
drivers/net/usb/smsc75xx.c | 11 ---
drivers/net/usb/sr9800.c | 9 -
5 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index b39ee714fb01..e39f41efda3e 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -221,6 +221,7 @@ struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct
sk_buff *skb,
int tailroom = skb_tailroom(skb);
u32 packet_len;
u32 padbytes = 0x;
+ void *ptr;
padlen = ((skb->len + 4) & (dev->maxpacket - 1)) ? 0 : 4;
@@ -256,13 +257,11 @@ struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct
sk_buff *skb,
}
packet_len = ((skb->len ^ 0x) << 16) + skb->len;
- skb_push(skb, 4);
- cpu_to_le32s(&packet_len);
- skb_copy_to_linear_data(skb, &packet_len, sizeof(packet_len));
+ ptr = skb_push(skb, 4);
+ put_unaligned_le32(packet_len, ptr);
if (padlen) {
- cpu_to_le32s(&padbytes);
- memcpy(skb_tail_pointer(skb), &padbytes, sizeof(padbytes));
+ put_unaligned_le32(padbytes, skb_tail_pointer(skb));
skb_put(skb, sizeof(padbytes));
}
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 72d165114b67..daa54486ab09 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1421,6 +1421,7 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
gfp_t flags)
int frame_size = dev->maxpacket;
int mss = skb_shinfo(skb)->gso_size;
int headroom;
+ void *ptr;
tx_hdr1 = skb->len;
tx_hdr2 = mss;
@@ -1435,13 +1436,9 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff
*skb, gfp_t flags)
return NULL;
}
- skb_push(skb, 4);
- cpu_to_le32s(&tx_hdr2);
- skb_copy_to_linear_data(skb, &tx_hdr2, 4);
-
- skb_push(skb, 4);
- cpu_to_le32s(&tx_hdr1);
- skb_copy_to_linear_data(skb, &tx_hdr1, 4);
+ ptr = skb_push(skb, 8);
+ put_unaligned_le32(tx_hdr1, ptr);
+ put_unaligned_le32(tx_hdr2, ptr + 4);
return skb;
}
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 9c33b35bd155..769bb262fbec 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2729,6 +2729,7 @@ static struct sk_buff *lan78xx_tx_prep(struct lan78xx_net
*dev,
struct sk_buff *skb, gfp_t flags)
{
u32 tx_cmd_a, tx_cmd_b;
+ void *ptr;
if (skb_cow_head(skb, TX_OVERHEAD)) {
dev_kfree_skb_any(skb);
@@ -2757,13 +2758,9 @@ static struct sk_buff *lan78xx_tx_prep(struct
lan78xx_net *dev,
tx_cmd_b |= skb_vlan_tag_get(skb) & TX_CMD_B_VTAG_MASK_;
}
- skb_push(skb, 4);
- cpu_to_le32s(&tx_cmd_b);
- memcpy(skb->data, &tx_cmd_b, 4);
-
- skb_push(skb, 4);
- cpu_to_le32s(&tx_cmd_a);
- memcpy(skb->data, &tx_cmd_a, 4);
+ ptr = skb_push(skb, 8);
+ put_unaligned_le32(tx_cmd_a, ptr);
+ put_unaligned_le32(tx_cmd_b, ptr + 4);
return skb;
}
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 7fac9db5380d..9556d431885f 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2255,6 +2255,7 @@ static struct sk_buff *smsc75xx_tx_fixup(struct usbnet
*dev,
struct sk_buff *skb, gfp_t flags)
{
u32 tx_cmd_a, tx_cmd_b;
+ void *ptr;
if (skb_cow_head(skb, SMSC75XX_TX_OVERHEAD)) {
dev_kfree_skb_any(skb);
@@ -2275,13 +2276,9 @@ static struct sk_buff *smsc75xx_tx_fixup(struct usbnet
*dev,
tx_cmd_b = 0;
}
- skb_push(skb, 4);
- cpu_to_le32s(&tx_cmd_b);
- memcpy(skb->data, &tx_cmd_b, 4);
-
- skb_push(skb, 4);
- cpu_to_le32s(&tx_cmd_a);
- memcpy(skb->data, &tx_cmd_a, 4);
+ ptr = skb_push(skb, 8);
+ put_unaligned_le32(tx_cmd_a, ptr);
+ put_unaligned_le32(tx_cmd_b, ptr + 4);
return skb;
}
diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index 35f39f23d881..c5d4a0060124 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -115,6 +115,7 @@ static struct sk_buff *sr_tx_fixup(struct usbnet *dev,
struct sk_buff *skb,
u32 padbytes = 0x;
u32 packet_len;
int padlen;
+ void *ptr;
padlen = ((skb->len + 4) % (dev->maxpacket - 1)) ? 0 : 4;
@@ -133,14 +134,12 @@ static struct sk_buff *sr_tx_fixup(struct usbnet *dev,
struct sk_buf
Re: [PATCH 1/3] Revert "usb: usb251xb: Add US lanes inversion dts-bindings"
Hi Serge,
On 19-07-19 13:13, Serge Semin wrote:
> Hello Lucas
>
> On Fri, Jul 19, 2019 at 10:44:05AM +0200, Lucas Stach wrote:
> > This reverts commit 3342ce35a1, as there is no need for this separate
> > property and it breaks compatibility with existing devicetree files
> > (arch/arm64/boot/dts/freescale/imx8mq.dtsi).
> >
>
> Hmm, didn't know there had been anything staged to merge and touching this
> property before submitting the update. We must have done it nearly at the same
> time, or your patch hasn't been merged at the time I prepared mine.
>
> Anyway why would you prefer to change the interface again instead of
> following the existing way? Firstly It is much easier to fix the dts-file
> than to revert the interface back and break dts-files of possible other users.
Since the dtbs are firmware thats not possible everytime. You can't even
say that nobody uses that binding because it's not grepable within the
kernel repo. Most vendors do not publish their dts files but use the
bindings and rely on stable bindings.
> Secondly the chip documentation doesn't have anything regarding port 0.
Thats not true. I've checked the usb2517 documentation and PRTSP register
description. Bit-0 points to the upstream port and the dt-binding
is such generic to cover that too. By swap-dx-lanes I mean swap d+/d-
lanes else it would be something like swap-downstream-lanes. Since the
upstream port have d+/d- lanes too it would be covered too.
> It states to swap the Ports from 1 to 4 (usb2514) corresponding to the bits
> 1 - 4 of the 'PORT SWAP' register, while bit 0 is connected with explicitly
> named Upstream Port (without any numbering). Thirdly having a separate
> property for US port makes the driver bindings interface a bit better
> readable/logical, since in current implementation there is no
> implicit/unspoken/hidden
> rule that port 0 corresponds to the Upstream Port, Port 0 just doesn't exists
> (following the chip datasheet text), and the other port-related properties are
> only applicable for downstream ports. So the driver code rejects them being
So the correct fix should be extending the documentation rather than
introducing a new binding?
> utilized for a port with 0 identifier. The only port-related setting being
> exposed by the interface is the swap-port-one and it has a separately bound
> property 'swap-us-lanes' for the Upstream port.
>
> As for me, all of this makes more sense than having an implicit Port 0 -
> Upstream
> port binding (as you suggest). Although the final decision of which solution
> is
> better is after the subsystem maintainer after all.
That's true but he had to cover the dt-backward compatibility.
Regards,
Marco
> Regards,
> -Sergey
>
> > CC: sta...@vger.kernel.org #5.2
> > Signed-off-by: Lucas Stach
> > ---
> > Documentation/devicetree/bindings/usb/usb251xb.txt | 6 ++
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt
> > b/Documentation/devicetree/bindings/usb/usb251xb.txt
> > index bc7945e9dbfe..17915f64b8ee 100644
> > --- a/Documentation/devicetree/bindings/usb/usb251xb.txt
> > +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
> > @@ -64,10 +64,8 @@ Optional properties :
> > - power-on-time-ms : Specifies the time it takes from the time the host
> > initiates the power-on sequence to a port until the port has adequate
> > power. The value is given in ms in a 0 - 510 range (default is 100ms).
> > - - swap-dx-lanes : Specifies the downstream ports which will swap the
> > - differential-pair (D+/D-), default is not-swapped.
> > - - swap-us-lanes : Selects the upstream port differential-pair (D+/D-)
> > - swapping (boolean, default is not-swapped)
> > + - swap-dx-lanes : Specifies the ports which will swap the
> > differential-pair
> > + (D+/D-), default is not-swapped.
> >
> > Examples:
> > usb2512b@2c {
> > --
> > 2.20.1
> >
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: Titan Ridge xHCI may stop to working after re-plugging the dock
Hi Felipe, at 18:51, Felipe Balbi wrote: Hi, Kai Heng Feng writes: Oliver Neukum writes: Am Dienstag, den 09.07.2019, 21:10 +0800 schrieb Kai-Heng Feng: Hi Mika and Mathias, I’ve filed a bug [1] which renders docking station unusable. I am not sure it's a bug in PCI, Thunderbolt or xHCI so raise the issue to you both. [1] https://bugzilla.kernel.org/show_bug.cgi?id=203885 Kai-Heng The issue starts before you unplug. In fact it starts before the dock is even detected the first time: [ 13.171167] rfkill: input handler disabled [ 19.781905] pcieport :00:1c.0: PME: Spurious native interrupt! [ 19.781909] pcieport :00:1c.0: PME: Spurious native interrupt! [ 20.109251] usb 4-1: new SuperSpeedPlus Gen 2 USB device number 2 using xhci_hcd [ 20.136000] usb 4-1: New USB device found, idVendor=0bda, idProduct=0487, bcdDevice= 1.47 [ 20.136004] usb 4-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [ 20.136007] usb 4-1: Product: Dell dock [ 20.136009] usb 4-1: Manufacturer: Dell Inc. [ 20.140607] hub 4-1:1.0: USB hub found [ 20.141004] hub 4-1:1.0: 4 ports detected [ 20.253025] usb 1-4: new high-speed USB device number 5 using xhci_hcd [ 20.403520] usb 1-4: New USB device found, idVendor=0bda, idProduct=5487, bcdDevice= 1.47 [ 20.403521] usb 1-4: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [ 20.403522] usb 1-4: Product: Dell dock [ 20.403522] usb 1-4: Manufacturer: Dell Inc. [ 20.404348] hub 1-4:1.0: USB hub found This looks like a PCI issue. In general, this kind of reporting sucks. We have to guess what you did at 19.781905 It might be nice to know which device is generating that and why it's not found. This may help: diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c index f38e6c19dd50..33285ef29362 100644 --- a/drivers/pci/pcie/pme.c +++ b/drivers/pci/pcie/pme.c @@ -203,7 +203,7 @@ static void pcie_pme_handle_request(struct pci_dev *port, u16 req_id) out: if (!found) - pci_info(port, "Spurious native interrupt!\n"); + pci_info(port, "Spurious native interrupt! (Bus# %d DevFn %d)\n", busnr, devfn); } /** Also, according to what Kai-Heng said, xHCI stops working even after repluggin the Dock. We could be dealing with two bugs here: 1. Spurious PME event being generated by an unexistent device 2. xHCI not handling hot-plug very well Kai-Heng, please run your tests again and make note of when you unplugged the dock and when you replugged it so we can correlate the time stampts with what you have done otherwise we will never be able to pin-point what's going on. I upgraded the system firmware, TBT firmware and docking station firmware to latest, and used latest mainline kernel. Now the issue can be reproduced at the very first time I plugged the docking station. Attach dmesg to BKO since there are lots of message after XHCI dyndbg is enabled. I saw that you annotated the plug, but not the unplug. Where does the unplug start? There are many places where it could be, but I need to be sure. Request log attached to Bugzilla. Also, wasn't it so that the problem is when you *replug* the dock? So can you better describe what you're doing? Are you booting with dock connected then disconnect and reconnect or are you booting without dock and it fails on first plug? The weird behavior I described in my previous replay is because the devices need to be fully power cycled after firmware upgrade. So it’s false alarm. The only issue now is the original bug. What are you consider a fail here? Can't you see the xhci bus? USB Devices don't show? What do you have on lsusb -t? The :39:00.0 xHCI stops working, so the USB ethernet (r8152) connects to it doesn’t work anymore. Normal case: /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 5000M |__ Port 3: Dev 3, If 0, Class=Hub, Driver=hub/4p, 5000M |__ Port 4: Dev 4, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/10p, 5000M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/16p, 480M |__ Port 4: Dev 2, If 0, Class=Hub, Driver=hub/5p, 480M |__ Port 5: Dev 6, If 0, Class=Human Interface Device, Driver=usbhid, 480M |__ Port 3: Dev 4, If 0, Class=Hub, Driver=hub/6p, 480M |__ Port 4: Dev 7, If 0, Class=Audio, Driver=snd-usb-audio, 480M |__ Port 4: Dev 7, If 3, Class=Audio, Driver=snd-usb-audio, 480M |__ Port 4: Dev 7, If 1, Class=Audio, Driver=snd-usb-audio, 480M |__ Port 4: Dev 7, If 2, Class=Audio, Driver=snd-usb-audio, 480M |__ Port 5: Dev 9, If 0, Class=Human Interface Device, Driver=usbhid, 480M |__ Port 9: Dev 3, If 0, Class=Human Interface Device, Driver=usbhid, 12M |__ Port 10: Dev 5, If 2, Class=Chip
RE: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
Hi, > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. >> >> The Cadence USBSS DRD Controller is a highly configurable IP Core which >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and >> Host Only (XHCI)configurations. > >I see you are using debugfs to select between DRD, peripheral-onlyh and XHCI... > >Is that good idea? Yes driver allows selecting dr_mode by debugfs. Controller also support such functionality so I don't understand why would it not be a good idea. I personally use this for testing but it can be used to limit controller functionality without recompiling kernel. >This is at least 3rd driver needing that capability, and debugfs does not >sound like a good match. > Pawell
RE: [PATCH v10 2/6] usb:common Separated decoding functions from dwc3 driver.
Hi,
>
>On Sun, 2019-07-21 at 19:32 +0100, Pawel Laszczak wrote:
>> Patch moves some decoding functions from driver/usb/dwc3/debug.h driver
>> to driver/usb/common/debug.c file. These moved functions include:
>[]
>> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
>[]
>> +static void usb_decode_set_clear_feature(__u8 bRequestType, __u8 bRequest,
>> + __u16 wValue, __u16 wIndex,
>> + char *str, size_t size)
>
>It's probably not necessary to use Hungarian
>when moving these functions into generic code.
In my opinion it's ok in this place. It's consistence with USB specification
ch9 and with
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/usb/ch9.h.
(look at usb_ctrlrequest).
>
>> +{
>> +switch (bRequestType & USB_RECIP_MASK) {
>> +case USB_RECIP_DEVICE:
>> +snprintf(str, size, "%s Device Feature(%s%s)",
>> + bRequest == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
>> + ({char *s;
>> +switch (wValue) {
>> +case USB_DEVICE_SELF_POWERED:
>> +s = "Self Powered";
>> +break;
>> +case USB_DEVICE_REMOTE_WAKEUP:
>> +s = "Remote Wakeup";
>> +break;
>> +case USB_DEVICE_TEST_MODE:
>> +s = "Test Mode";
>> +break;
>> +case USB_DEVICE_U1_ENABLE:
>> +s = "U1 Enable";
>> +break;
>> +case USB_DEVICE_U2_ENABLE:
>> +s = "U2 Enable";
>> +break;
>> +case USB_DEVICE_LTM_ENABLE:
>> +s = "LTM Enable";
>> +break;
>> +default:
>> +s = "UNKNOWN";
>> +} s; }),
>> + wValue == USB_DEVICE_TEST_MODE ?
>> + ({ char *s;
>> +switch (wIndex) {
>> +case TEST_J:
>> +s = ": TEST_J";
>> +break;
>> +case TEST_K:
>> +s = ": TEST_K";
>> +break;
>> +case TEST_SE0_NAK:
>> +s = ": TEST_SE0_NAK";
>> +break;
>> +case TEST_PACKET:
>> +s = ": TEST_PACKET";
>> +break;
>> +case TEST_FORCE_EN:
>> +s = ": TEST_FORCE_EN";
>> +break;
>> +default:
>> +s = ": UNKNOWN";
>> +} s; }) : "");
>
>I always find this sort of embedded switch/case char *
>statement expressions difficult to read and think it's
>better to use separate lookup functions.
>
It has been changed in next patch in the series.
>I would much prefer something like:
>
>static const char *usb_device_mode_desc(u16 mode)
>{
> switch (mode) {
> case USB_DEVICE_SELF_POWERED:
> return "Self Powered";
> case USB_DEVICE_REMOTE_WAKEUP:
> return "Remote Wakeup";
> case USB_DEVICE_TEST_MODE:
> return "Test Mode";
> case USB_DEVICE_U1_ENABLE:
> return "U1 Enable";
> case USB_DEVICE_U2_ENABLE:
> return "U2 Enable";
> case USB_DEVICE_LTM_ENABLE:
> return "LTM Enable";
> }
> return "UNKNOWN";
>}
>
> snprintf(str, size, "%s Device Feature(%s%s)",
>> bRequest == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
>usb_device_mode_desc(wValue),
>etc...);
>
>
>etc...
>
Cheers,
Pawell
RE: [PATCH v10 3/6] usb:common Patch simplify usb_decode_set_clear_feature function.
>
>
>On Sun, 2019-07-21 at 19:32 +0100, Pawel Laszczak wrote:
>> Patch adds usb_decode_test_mode and usb_decode_device_feature functions,
>> which allow to make more readable and simplify the
>> usb_decode_set_clear_feature function.
>
> I need to read entire patch series before
>commenting more I guess...
>
>> Signed-off-by: Pawel Laszczak
>> ---
>> drivers/usb/common/debug.c | 89 ++
>> 1 file changed, 43 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
>[]
>> +static const char *usb_decode_device_feature(u16 wValue)
>
>I believe this is still unnecessary hungarian.
it's common in usb subsystem if driver refers to descriptors fields.
>
>> +{
>> +switch (wValue) {
>> +case USB_DEVICE_SELF_POWERED:
>> +return "Self Powered";
>> +case USB_DEVICE_REMOTE_WAKEUP:
>> +return "Remote Wakeup";
>> +case USB_DEVICE_TEST_MODE:
>> +return "Test Mode";
>> +case USB_DEVICE_U1_ENABLE:
>> +return "U1 Enable";
>> +case USB_DEVICE_U2_ENABLE:
>> +return "U2 Enable";
>> +case USB_DEVICE_LTM_ENABLE:
>> +return "LTM Enable";
>> +default:
>> +return "UNKNOWN";
>> +}
>> +}
>
>
But yeah, exactly like this... ;)
>
Pawell
[PATCH v3] usb-storage: Add a limitation for blk_queue_max_hw_sectors()
This patch fixes an issue that the following error happens on
swiotlb environment:
xhci-hcd ee00.usb: swiotlb buffer is full (sz: 524288 bytes), total
32768 (slots), used 1338 (slots)
On the kernel v5.1, block settings of a usb-storage with SuperSpeed
were the following so that the block layer will allocate buffers
up to 64 KiB, and then the issue didn't happen.
max_segment_size = 65536
max_hw_sectors_kb = 1024
After the commit 09324d32d2a0 ("block: force an unlimited segment
size on queues with a virt boundary") is applied, the block settings
are the following. So, the block layer will allocate buffers up to
1024 KiB, and then the issue happens:
max_segment_size = 4294967295
max_hw_sectors_kb = 1024
To fix the issue, the usb-storage driver checks the maximum size of
a mapping for the device and then adjusts the max_hw_sectors_kb
if required. After this patch is applied, the block settings will
be the following, and then the issue doesn't happen.
max_segment_size = 4294967295
max_hw_sectors_kb = 256
Fixes: 09324d32d2a0 ("block: force an unlimited segment size on queues with a
virt boundary")
Signed-off-by: Yoshihiro Shimoda
Acked-by: Alan Stern
---
This patch can be applied on v5.3-rc1.
Changes from v2:
- Add Alan's Acked-by.
https://patchwork.kernel.org/patch/10992539/
Changes from v1:
- Call blk_queue_max_hw_sectors() for the maximum size of mapping
unconditionally to simplify the code by using read the value back
from the queue in the end.
- Add a comment on the code.
- On v1, I got Reviewed-by from Christoph. But, I changed the code a little,
I removed the Reviewed-by.
https://patchwork.kernel.org/patch/10992985/
drivers/usb/storage/scsiglue.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 3079024..05b8021 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -28,6 +28,8 @@
* status of a command.
*/
+#include
+#include
#include
#include
@@ -99,6 +101,7 @@ static int slave_alloc (struct scsi_device *sdev)
static int slave_configure(struct scsi_device *sdev)
{
struct us_data *us = host_to_us(sdev->host);
+ struct device *dev = us->pusb_dev->bus->sysdev;
/*
* Many devices have trouble transferring more than 32KB at a time,
@@ -129,6 +132,14 @@ static int slave_configure(struct scsi_device *sdev)
}
/*
+* The max_hw_sectors should be up to maximum size of a mapping for
+* the device. Otherwise, a DMA API might fail on swiotlb environment.
+*/
+ blk_queue_max_hw_sectors(sdev->request_queue,
+ min_t(size_t, queue_max_hw_sectors(sdev->request_queue),
+ dma_max_mapping_size(dev) >> SECTOR_SHIFT));
+
+ /*
* Some USB host controllers can't do DMA; they have to use PIO.
* They indicate this by setting their dma_mask to NULL. For
* such controllers we need to make sure the block layer sets
--
2.7.4
Re: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
On Mon, Jul 22, 2019 at 09:58:42AM +, Pawel Laszczak wrote: > Hi, > > > > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. > >> > >> The Cadence USBSS DRD Controller is a highly configurable IP Core which > >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and > >> Host Only (XHCI)configurations. > > > >I see you are using debugfs to select between DRD, peripheral-onlyh and > >XHCI... > > > >Is that good idea? > > Yes driver allows selecting dr_mode by debugfs. Controller also support such > functionality > so I don't understand why would it not be a good idea. > > I personally use this for testing but it can be used to limit controller > functionality without > recompiling kernel. debugfs is ONLY for debugging, never rely on it being enabled, or mounted, on a system in order to have any normal operation happen. So for testing, yes, this is fine. If this is going to be the normal api/interface for how to control this driver, no, that is not acceptable at all. thanks, greg k-h
Re: [PATCH v10 2/6] usb:common Separated decoding functions from dwc3 driver.
On Mon, Jul 22, 2019 at 10:06:38AM +, Pawel Laszczak wrote: > Hi, > > > > >On Sun, 2019-07-21 at 19:32 +0100, Pawel Laszczak wrote: > >> Patch moves some decoding functions from driver/usb/dwc3/debug.h driver > >> to driver/usb/common/debug.c file. These moved functions include: > >[] > >> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c > >[] > >> +static void usb_decode_set_clear_feature(__u8 bRequestType, __u8 bRequest, > >> + __u16 wValue, __u16 wIndex, > >> + char *str, size_t size) > > > >It's probably not necessary to use Hungarian > >when moving these functions into generic code. > > In my opinion it's ok in this place. It's consistence with USB specification > ch9 and with > https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/usb/ch9.h. > (look at usb_ctrlrequest). Yes, this is fine, the USB subsystem has this everywhere, and we all know exactly what it means when we see it. thanks, greg k-h
Re: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
Hi! > > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. > > >> > > >> The Cadence USBSS DRD Controller is a highly configurable IP Core which > > >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and > > >> Host Only (XHCI)configurations. > > > > > >I see you are using debugfs to select between DRD, peripheral-onlyh and > > >XHCI... > > > > > >Is that good idea? > > > > Yes driver allows selecting dr_mode by debugfs. Controller also support > > such functionality > > so I don't understand why would it not be a good idea. > > > > I personally use this for testing but it can be used to limit controller > > functionality without > > recompiling kernel. > > debugfs is ONLY for debugging, never rely on it being enabled, or > mounted, on a system in order to have any normal operation happen. > > So for testing, yes, this is fine. If this is going to be the normal > api/interface for how to control this driver, no, that is not acceptable > at all. It makes a lot of sense for end-user to toggle this... for example when he is lacking right cable for proper otg detection. As it is third driver offering this functionality, I believe we should stop treating it as debugging. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
On Mon, Jul 22, 2019 at 01:56:45PM +0200, Pavel Machek wrote: > Hi! > > > > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. > > > >> > > > >> The Cadence USBSS DRD Controller is a highly configurable IP Core which > > > >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and > > > >> Host Only (XHCI)configurations. > > > > > > > >I see you are using debugfs to select between DRD, peripheral-onlyh and > > > >XHCI... > > > > > > > >Is that good idea? > > > > > > Yes driver allows selecting dr_mode by debugfs. Controller also support > > > such functionality > > > so I don't understand why would it not be a good idea. > > > > > > I personally use this for testing but it can be used to limit controller > > > functionality without > > > recompiling kernel. > > > > debugfs is ONLY for debugging, never rely on it being enabled, or > > mounted, on a system in order to have any normal operation happen. > > > > So for testing, yes, this is fine. If this is going to be the normal > > api/interface for how to control this driver, no, that is not acceptable > > at all. > > It makes a lot of sense for end-user to toggle this... for example > when he is lacking right cable for proper otg detection. As it is > third driver offering this functionality, I believe we should stop > treating it as debugging. Then it needs to get out of debugfs, as again, that can not be used for any normal end-user operation. thanks, greg k-h
RE: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
> >Hi! > >> > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. >> > >> >> > >> The Cadence USBSS DRD Controller is a highly configurable IP Core which >> > >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and >> > >> Host Only (XHCI)configurations. >> > > >> > >I see you are using debugfs to select between DRD, peripheral-onlyh and >> > >XHCI... >> > > >> > >Is that good idea? >> > >> > Yes driver allows selecting dr_mode by debugfs. Controller also support >> > such functionality >> > so I don't understand why would it not be a good idea. >> > >> > I personally use this for testing but it can be used to limit controller >> > functionality without >> > recompiling kernel. >> >> debugfs is ONLY for debugging, never rely on it being enabled, or >> mounted, on a system in order to have any normal operation happen. >> >> So for testing, yes, this is fine. If this is going to be the normal >> api/interface for how to control this driver, no, that is not acceptable >> at all. > >It makes a lot of sense for end-user to toggle this... for example >when he is lacking right cable for proper otg detection. As it is >third driver offering this functionality, I believe we should stop >treating it as debugging. > Exactly I use this for this purpose. Depending on my testing platform I have the adapter with Typ-c plugs or normal Type A plugs. In the second case, by means of this property I can force Device or Host mode from driver without changing adapter. Bu as I wrote I use it only for debugging purpose. I believe that device on the market should not work in this way. Cheers, Pawel.
WARNING in shark_write_val/usb_submit_urb
Hello, syzbot found the following crash on: HEAD commit:6a3599ce usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=138da9d060 kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae dashboard link: https://syzkaller.appspot.com/bug?extid=1cb937c125adb93fad2d compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=159ab95860 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177bd6afa0 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+1cb937c125adb93fa...@syzkaller.appspotmail.com usb 1-1: string descriptor 0 read error: -22 usb 1-1: New USB device found, idVendor=077d, idProduct=627a, bcdDevice= 0.01 usb 1-1: New USB device strings: Mfr=1, Product=64, SerialNumber=255 [ cut here ] usb 1-1: BOGUS urb xfer, pipe 1 != type 3 WARNING: CPU: 1 PID: 21 at drivers/usb/core/urb.c:477 usb_submit_urb+0x1188/0x13b0 /drivers/usb/core/urb.c:477 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.2.0-rc6+ #15 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack /lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e /lib/dump_stack.c:113 panic+0x292/0x6c9 /kernel/panic.c:219 __warn.cold+0x20/0x4b /kernel/panic.c:576 report_bug+0x262/0x2a0 /lib/bug.c:186 fixup_bug /arch/x86/kernel/traps.c:179 [inline] fixup_bug /arch/x86/kernel/traps.c:174 [inline] do_error_trap+0x12b/0x1e0 /arch/x86/kernel/traps.c:272 do_invalid_op+0x32/0x40 /arch/x86/kernel/traps.c:291 invalid_op+0x14/0x20 /arch/x86/entry/entry_64.S:986 RIP: 0010:usb_submit_urb+0x1188/0x13b0 /drivers/usb/core/urb.c:477 Code: 4d 85 ed 74 2c e8 f8 d3 f4 fd 4c 89 f7 e8 a0 51 1c ff 41 89 d8 44 89 e1 4c 89 ea 48 89 c6 48 c7 c7 00 0e f7 85 e8 83 98 ca fd <0f> 0b e9 20 f4 ff ff e8 cc d3 f4 fd 4c 89 f2 48 b8 00 00 00 00 00 RSP: 0018:8881d9efefe0 EFLAGS: 00010286 RAX: RBX: 0003 RCX: RDX: RSI: 8127ef3d RDI: ed103b3dfdee RBP: 8881d41e5e60 R08: 8881d9e36000 R09: R10: R11: R12: 0001 R13: 8881cff4cbd0 R14: 8881d7b33c20 R15: 8881d41c9b00 usb_start_wait_urb+0x108/0x2b0 /drivers/usb/core/message.c:57 usb_bulk_msg+0x228/0x550 /drivers/usb/core/message.c:253 shark_write_val+0x20b/0x310 /drivers/media/radio/radio-shark.c:94 snd_tea575x_write+0x78/0x330 /drivers/media/radio/tea575x.c:88 snd_tea575x_hw_init+0x8d/0x170 /drivers/media/radio/tea575x.c:506 snd_tea575x_init+0x1f/0x6b8 /drivers/media/radio/tea575x.c:521 usb_shark_probe+0x5e1/0x770 /drivers/media/radio/radio-shark.c:353 usb_probe_interface+0x305/0x7a0 /drivers/usb/core/driver.c:361 really_probe+0x281/0x660 /drivers/base/dd.c:509 driver_probe_device+0x104/0x210 /drivers/base/dd.c:670 __device_attach_driver+0x1c2/0x220 /drivers/base/dd.c:777 bus_for_each_drv+0x15c/0x1e0 /drivers/base/bus.c:454 __device_attach+0x217/0x360 /drivers/base/dd.c:843 bus_probe_device+0x1e4/0x290 /drivers/base/bus.c:514 device_add+0xae6/0x16f0 /drivers/base/core.c:2111 usb_set_configuration+0xdf6/0x1670 /drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 /drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 /drivers/usb/core/driver.c:266 really_probe+0x281/0x660 /drivers/base/dd.c:509 driver_probe_device+0x104/0x210 /drivers/base/dd.c:670 __device_attach_driver+0x1c2/0x220 /drivers/base/dd.c:777 bus_for_each_drv+0x15c/0x1e0 /drivers/base/bus.c:454 __device_attach+0x217/0x360 /drivers/base/dd.c:843 bus_probe_device+0x1e4/0x290 /drivers/base/bus.c:514 device_add+0xae6/0x16f0 /drivers/base/core.c:2111 usb_new_device.cold+0x6a4/0xe61 /drivers/usb/core/hub.c:2536 hub_port_connect /drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change /drivers/usb/core/hub.c:5213 [inline] port_event /drivers/usb/core/hub.c:5359 [inline] hub_event+0x1abd/0x3550 /drivers/usb/core/hub.c:5441 process_one_work+0x905/0x1570 /kernel/workqueue.c:2269 worker_thread+0x96/0xe20 /kernel/workqueue.c:2415 kthread+0x30b/0x410 /kernel/kthread.c:255 ret_from_fork+0x24/0x30 /arch/x86/entry/entry_64.S:352 Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
WARNING in amradio_send_cmd/usb_submit_urb
Hello, syzbot found the following crash on: HEAD commit:6a3599ce usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=111a1ed060 kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae dashboard link: https://syzkaller.appspot.com/bug?extid=485b10e300244dc0046c compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=154d3d4860 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12ef163460 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+485b10e300244dc00...@syzkaller.appspotmail.com usb 1-1: string descriptor 0 read error: -22 usb 1-1: New USB device found, idVendor=07ca, idProduct=b800, bcdDevice=d3.2a usb 1-1: New USB device strings: Mfr=5, Product=3, SerialNumber=255 [ cut here ] usb 1-1: BOGUS urb xfer, pipe 1 != type 3 WARNING: CPU: 1 PID: 21 at drivers/usb/core/urb.c:477 usb_submit_urb+0x1188/0x13b0 /drivers/usb/core/urb.c:477 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.2.0-rc6+ #15 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack /lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e /lib/dump_stack.c:113 panic+0x292/0x6c9 /kernel/panic.c:219 __warn.cold+0x20/0x4b /kernel/panic.c:576 report_bug+0x262/0x2a0 /lib/bug.c:186 fixup_bug /arch/x86/kernel/traps.c:179 [inline] fixup_bug /arch/x86/kernel/traps.c:174 [inline] do_error_trap+0x12b/0x1e0 /arch/x86/kernel/traps.c:272 do_invalid_op+0x32/0x40 /arch/x86/kernel/traps.c:291 invalid_op+0x14/0x20 /arch/x86/entry/entry_64.S:986 RIP: 0010:usb_submit_urb+0x1188/0x13b0 /drivers/usb/core/urb.c:477 Code: 4d 85 ed 74 2c e8 f8 d3 f4 fd 4c 89 f7 e8 a0 51 1c ff 41 89 d8 44 89 e1 4c 89 ea 48 89 c6 48 c7 c7 00 0e f7 85 e8 83 98 ca fd <0f> 0b e9 20 f4 ff ff e8 cc d3 f4 fd 4c 89 f2 48 b8 00 00 00 00 00 RSP: 0018:8881d9eff090 EFLAGS: 00010282 RAX: RBX: 0003 RCX: RDX: RSI: 8127ef3d RDI: ed103b3dfe04 RBP: 8881cfd4a540 R08: 8881d9e36000 R09: R10: R11: R12: 0001 R13: 8881d3629f48 R14: 8881cfdc80a0 R15: 8881d41fa300 usb_start_wait_urb+0x108/0x2b0 /drivers/usb/core/message.c:57 usb_bulk_msg+0x228/0x550 /drivers/usb/core/message.c:253 amradio_send_cmd+0x2e4/0x800 /drivers/media/radio/radio-mr800.c:150 amradio_set_mute /drivers/media/radio/radio-mr800.c:182 [inline] usb_amradio_init /drivers/media/radio/radio-mr800.c:414 [inline] usb_amradio_probe+0x409/0x6b2 /drivers/media/radio/radio-mr800.c:555 usb_probe_interface+0x305/0x7a0 /drivers/usb/core/driver.c:361 really_probe+0x281/0x660 /drivers/base/dd.c:509 driver_probe_device+0x104/0x210 /drivers/base/dd.c:670 __device_attach_driver+0x1c2/0x220 /drivers/base/dd.c:777 bus_for_each_drv+0x15c/0x1e0 /drivers/base/bus.c:454 __device_attach+0x217/0x360 /drivers/base/dd.c:843 bus_probe_device+0x1e4/0x290 /drivers/base/bus.c:514 device_add+0xae6/0x16f0 /drivers/base/core.c:2111 usb_set_configuration+0xdf6/0x1670 /drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 /drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 /drivers/usb/core/driver.c:266 really_probe+0x281/0x660 /drivers/base/dd.c:509 driver_probe_device+0x104/0x210 /drivers/base/dd.c:670 __device_attach_driver+0x1c2/0x220 /drivers/base/dd.c:777 bus_for_each_drv+0x15c/0x1e0 /drivers/base/bus.c:454 __device_attach+0x217/0x360 /drivers/base/dd.c:843 bus_probe_device+0x1e4/0x290 /drivers/base/bus.c:514 device_add+0xae6/0x16f0 /drivers/base/core.c:2111 usb_new_device.cold+0x6a4/0xe61 /drivers/usb/core/hub.c:2536 hub_port_connect /drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change /drivers/usb/core/hub.c:5213 [inline] port_event /drivers/usb/core/hub.c:5359 [inline] hub_event+0x1abd/0x3550 /drivers/usb/core/hub.c:5441 process_one_work+0x905/0x1570 /kernel/workqueue.c:2269 worker_thread+0x96/0xe20 /kernel/workqueue.c:2415 kthread+0x30b/0x410 /kernel/kthread.c:255 ret_from_fork+0x24/0x30 /arch/x86/entry/entry_64.S:352 Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Regression xhci_hcd: WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state
Hi Mathias, around 1.5 weeks ago I've sent the dmesg log and xhci_hcd tracing file to you. Is there anything else that needs to be provided? How should we precede otherwise? The problem occurs since kernel version 4.20 and it looks like more and more people are affected by this, most of them blame their wifi driver for it. Maybe it would be the best to just revert the patch that is causing the problem? The regression is caused by the changes in process_bulk_intr_td(), it's part of this commit: commit f8f80be501aa2f10669585c3e328fad079d8cb3a Author: Mathias Nyman mailto:mathias.ny...@linux.intel.com>> Date: Thu Sep 20 19:13:37 2018 +0300 xhci: Use soft retry to recover faster from transaction errors In case you missed the mail with the log files, I've uploaded them on transfer.sh: https://transfer.sh/KDEeE/dmesg and https://transfer.sh/14Imam/trace - Bernhard
[PATCH v4 1/6] usb: gadget: u_serial: add missing port entry locking
gserial_alloc_line() misses locking (for a release barrier) while resetting port entry on TTY allocation failure. Fix this. Cc: sta...@vger.kernel.org Signed-off-by: Michał Mirosław Reviewed-by: Greg Kroah-Hartman Tested-by: Ladislav Michl --- v4: no changes v3: cc-stable v2: no changes --- drivers/usb/gadget/function/u_serial.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index 65f634ec7fc2..bb1e2e1d0076 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1239,8 +1239,10 @@ int gserial_alloc_line(unsigned char *line_num) __func__, port_num, PTR_ERR(tty_dev)); ret = PTR_ERR(tty_dev); + mutex_lock(&ports[port_num].lock); port = ports[port_num].port; ports[port_num].port = NULL; + mutex_unlock(&ports[port_num].lock); gserial_free_port(port); goto err; } -- 2.20.1
[PATCH v4 3/6] usb: gadget: u_serial: make OBEX port not a console
Prevent OBEX serial port from ever becoming a console. Console messages
will definitely break the protocol, and since you have to instantiate
the port making it explicitly for OBEX, there is no point in allowing
console to break it by mistake.
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
---
v4: no changes
v3: rename gserial_alloc_line_raw() -> gserial_alloc_line_no_console()
v2: change of API
+ commit message massage
---
drivers/usb/gadget/function/f_obex.c | 2 +-
drivers/usb/gadget/function/u_serial.c | 16
drivers/usb/gadget/function/u_serial.h | 1 +
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/function/f_obex.c
b/drivers/usb/gadget/function/f_obex.c
index 55b7f57d2dc7..ab26d84ed95e 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -432,7 +432,7 @@ static struct usb_function_instance *obex_alloc_inst(void)
return ERR_PTR(-ENOMEM);
opts->func_inst.free_func_inst = obex_free_inst;
- ret = gserial_alloc_line(&opts->port_num);
+ ret = gserial_alloc_line_no_console(&opts->port_num);
if (ret) {
kfree(opts);
return ERR_PTR(ret);
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 94f6999e8262..62280c23cde2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1180,7 +1180,7 @@ void gserial_free_line(unsigned char port_num)
}
EXPORT_SYMBOL_GPL(gserial_free_line);
-int gserial_alloc_line(unsigned char *line_num)
+int gserial_alloc_line_no_console(unsigned char *line_num)
{
struct usb_cdc_line_coding coding;
struct gs_port *port;
@@ -1221,12 +1221,20 @@ int gserial_alloc_line(unsigned char *line_num)
goto err;
}
*line_num = port_num;
-
- if (!port_num)
- gs_console_init(port);
err:
return ret;
}
+EXPORT_SYMBOL_GPL(gserial_alloc_line_no_console);
+
+int gserial_alloc_line(unsigned char *line_num)
+{
+ int ret = gserial_alloc_line_no_console(line_num);
+
+ if (!ret && !*line_num)
+ gs_console_init(ports[*line_num].port);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(gserial_alloc_line);
/**
diff --git a/drivers/usb/gadget/function/u_serial.h
b/drivers/usb/gadget/function/u_serial.h
index 9acaac1cbb75..8b472b0c8cb4 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -54,6 +54,7 @@ struct usb_request *gs_alloc_req(struct usb_ep *ep, unsigned
len, gfp_t flags);
void gs_free_req(struct usb_ep *, struct usb_request *req);
/* management of individual TTY ports */
+int gserial_alloc_line_no_console(unsigned char *port_line);
int gserial_alloc_line(unsigned char *port_line);
void gserial_free_line(unsigned char port_line);
--
2.20.1
[PATCH v4 0/6] usb: gadget: u_serial: console improvements
This series makes it possible to have more control over console using USB serial gadget ports. This can be useful when you need more than one USB console or are configuring multiple serial port function via configfs. The patches are against usb-next tree. You can also pull from: https://rere.qmqm.pl/git/linux usb-console Michał Mirosław (6): usb: gadget: u_serial: add missing port entry locking usb: gadget: u_serial: reimplement console support usb: gadget: u_serial: make OBEX port not a console usb: gadget: u_serial: allow more console gadget ports usb: gadget: u_serial: diagnose missed console messages USB: gadget: legacy/serial: allow dynamic removal drivers/usb/gadget/function/f_acm.c| 21 ++ drivers/usb/gadget/function/f_obex.c | 2 +- drivers/usb/gadget/function/f_serial.c | 21 ++ drivers/usb/gadget/function/u_serial.c | 420 ++--- drivers/usb/gadget/function/u_serial.h | 8 + drivers/usb/gadget/legacy/serial.c | 49 ++- 6 files changed, 333 insertions(+), 188 deletions(-) -- 2.20.1
[PATCH v4 6/6] USB: gadget: legacy/serial: allow dynamic removal
Legacy serial USB gadget is still useful as an early console,
before userspace is up. Later it could be replaced with proper
configfs-configured composite gadget - that use case is enabled
by this patch.
Signed-off-by: Michał Mirosław
---
v4: initial revision, new in the patchset
---
drivers/usb/gadget/legacy/serial.c | 49 +-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/legacy/serial.c
b/drivers/usb/gadget/legacy/serial.c
index de30d7628eef..da44f89f5e73 100644
--- a/drivers/usb/gadget/legacy/serial.c
+++ b/drivers/usb/gadget/legacy/serial.c
@@ -97,6 +97,36 @@ static unsigned n_ports = 1;
module_param(n_ports, uint, 0);
MODULE_PARM_DESC(n_ports, "number of ports to create, default=1");
+static bool enable = true;
+
+static int switch_gserial_enable(bool do_enable);
+
+static int enable_set(const char *s, const struct kernel_param *kp)
+{
+ bool do_enable;
+ int ret;
+
+ if (!s) /* called for no-arg enable == default */
+ return 0;
+
+ ret = strtobool(s, &do_enable);
+ if (ret || enable == do_enable)
+ return ret;
+
+ ret = switch_gserial_enable(do_enable);
+ if (!ret)
+ enable = do_enable;
+
+ return ret;
+}
+
+static const struct kernel_param_ops enable_ops = {
+ .set = enable_set,
+ .get = param_get_bool,
+};
+
+module_param_cb(enable, &enable_ops, &enable, 0644);
+
/*-*/
static struct usb_configuration serial_config_driver = {
@@ -240,6 +270,19 @@ static struct usb_composite_driver gserial_driver = {
.unbind = gs_unbind,
};
+static int switch_gserial_enable(bool do_enable)
+{
+ if (!serial_config_driver.label)
+ /* init() was not called, yet */
+ return 0;
+
+ if (do_enable)
+ return usb_composite_probe(&gserial_driver);
+
+ usb_composite_unregister(&gserial_driver);
+ return 0;
+}
+
static int __init init(void)
{
/* We *could* export two configs; that'd be much cleaner...
@@ -266,12 +309,16 @@ static int __init init(void)
}
strings_dev[STRING_DESCRIPTION_IDX].s = serial_config_driver.label;
+ if (!enable)
+ return 0;
+
return usb_composite_probe(&gserial_driver);
}
module_init(init);
static void __exit cleanup(void)
{
- usb_composite_unregister(&gserial_driver);
+ if (enable)
+ usb_composite_unregister(&gserial_driver);
}
module_exit(cleanup);
--
2.20.1
[PATCH v4 5/6] usb: gadget: u_serial: diagnose missed console messages
Insert markers in console stream marking places where data
is missing. This makes the hole in the data stand out clearly
instead of glueing together unrelated messages.
Example output as seen from USB host side:
[0.064078] pinctrl core: registered pin 16 (UART3_RTS_N PC0) on
7868.pinmux
[0.064130] pinctrl
[missed 114987 bytes]
[4.302299] udevd[134]: starting version 3.2.5
[4.306845] random: udevd: uninitialized urandom read (16 bytes read)
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
---
v4: no changes
v3: added example output
+ lowercase "missed"
v2: commit message massage
---
drivers/usb/gadget/function/u_serial.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index ccfa8c6a35ac..c88ad020f9e2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -88,6 +88,7 @@ struct gs_console {
spinlock_t lock;
struct usb_request *req;
struct kfifobuf;
+ size_t missed;
};
/*
@@ -931,6 +932,15 @@ static void __gs_console_push(struct gs_console *cons)
if (!size)
return;
+ if (cons->missed && ep->maxpacket >= 64) {
+ char buf[64];
+ size_t len;
+
+ len = sprintf(buf, "\n[missed %zu bytes]\n", cons->missed);
+ kfifo_in(&cons->buf, buf, len);
+ cons->missed = 0;
+ }
+
req->length = size;
if (usb_ep_queue(ep, req, GFP_ATOMIC))
req->length = 0;
@@ -952,10 +962,13 @@ static void gs_console_write(struct console *co,
{
struct gs_console *cons = container_of(co, struct gs_console, console);
unsigned long flags;
+ size_t n;
spin_lock_irqsave(&cons->lock, flags);
- kfifo_in(&cons->buf, buf, count);
+ n = kfifo_in(&cons->buf, buf, count);
+ if (n < count)
+ cons->missed += count - n;
if (cons->req && !cons->req->length)
schedule_work(&cons->work);
--
2.20.1
[PATCH v4 2/6] usb: gadget: u_serial: reimplement console support
Rewrite console support to fix a few shortcomings of the old code
preventing its use with multiple ports. This removes some duplicated
code and replaces a custom kthread with simpler workqueue item.
Only port ttyGS0 gets to be a console for now.
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
Tested-by: Ladislav Michl
---
v4: cosmetic change to __gs_console_push()
v3: no changes
v2: no changes
---
drivers/usb/gadget/function/u_serial.c | 351 -
1 file changed, 164 insertions(+), 187 deletions(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index bb1e2e1d0076..94f6999e8262 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -82,14 +82,12 @@
#define GS_CONSOLE_BUF_SIZE8192
/* console info */
-struct gscons_info {
- struct gs_port *port;
- struct task_struct *console_thread;
- struct kfifocon_buf;
- /* protect the buf and busy flag */
- spinlock_t con_lock;
- int req_busy;
- struct usb_request *console_req;
+struct gs_console {
+ struct console console;
+ struct work_struct work;
+ spinlock_t lock;
+ struct usb_request *req;
+ struct kfifobuf;
};
/*
@@ -101,6 +99,9 @@ struct gs_port {
spinlock_t port_lock; /* guard port_* access */
struct gserial *port_usb;
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ struct gs_console *console;
+#endif
boolopenclose; /* open/close in progress */
u8 port_num;
@@ -889,36 +890,9 @@ static struct tty_driver *gs_tty_driver;
#ifdef CONFIG_U_SERIAL_CONSOLE
-static struct gscons_info gscons_info;
-static struct console gserial_cons;
-
-static struct usb_request *gs_request_new(struct usb_ep *ep)
+static void gs_console_complete_out(struct usb_ep *ep, struct usb_request *req)
{
- struct usb_request *req = usb_ep_alloc_request(ep, GFP_ATOMIC);
- if (!req)
- return NULL;
-
- req->buf = kmalloc(ep->maxpacket, GFP_ATOMIC);
- if (!req->buf) {
- usb_ep_free_request(ep, req);
- return NULL;
- }
-
- return req;
-}
-
-static void gs_request_free(struct usb_request *req, struct usb_ep *ep)
-{
- if (!req)
- return;
-
- kfree(req->buf);
- usb_ep_free_request(ep, req);
-}
-
-static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
-{
- struct gscons_info *info = &gscons_info;
+ struct gs_console *cons = req->context;
switch (req->status) {
default:
@@ -927,12 +901,12 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
/* fall through */
case 0:
/* normal completion */
- spin_lock(&info->con_lock);
- info->req_busy = 0;
- spin_unlock(&info->con_lock);
-
- wake_up_process(info->console_thread);
+ spin_lock(&cons->lock);
+ req->length = 0;
+ schedule_work(&cons->work);
+ spin_unlock(&cons->lock);
break;
+ case -ECONNRESET:
case -ESHUTDOWN:
/* disconnect */
pr_vdebug("%s: %s shutdown\n", __func__, ep->name);
@@ -940,190 +914,190 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
}
}
-static int gs_console_connect(int port_num)
+static void __gs_console_push(struct gs_console *cons)
{
- struct gscons_info *info = &gscons_info;
- struct gs_port *port;
+ struct usb_request *req = cons->req;
struct usb_ep *ep;
+ size_t size;
- if (port_num != gserial_cons.index) {
- pr_err("%s: port num [%d] is not support console\n",
- __func__, port_num);
- return -ENXIO;
- }
+ if (!req)
+ return; /* disconnected */
- port = ports[port_num].port;
- ep = port->port_usb->in;
- if (!info->console_req) {
- info->console_req = gs_request_new(ep);
- if (!info->console_req)
- return -ENOMEM;
- info->console_req->complete = gs_complete_out;
- }
+ if (req->length)
+ return; /* busy */
- info->port = port;
- spin_lock(&info->con_lock);
- info->req_busy = 0;
- spin_unlock(&info->con_lock);
- pr_vdebug("port[%d] console connect!\n", port_num);
- return 0;
-}
-
-static void gs_console_disconnect(struct usb_ep *ep)
-{
- struct gscons_info *info = &gscons_info;
- struct usb_request *req = info->console_req;
-
- gs_request_free(req, ep);
- info->console_req = NULL;
-}
-
-static int
[PATCH v4 4/6] usb: gadget: u_serial: allow more console gadget ports
Allow configuring more than one console using USB serial or ACM gadget.
By default, only first (ttyGS0) is a console, but this may be changed
using function's new "console" attribute.
Signed-off-by: Michał Mirosław
---
v4: fixed locking in gserial_set_console()
v3: no changes
v2: no changes
---
drivers/usb/gadget/function/f_acm.c| 21 +++
drivers/usb/gadget/function/f_serial.c | 21 +++
drivers/usb/gadget/function/u_serial.c | 48 ++
drivers/usb/gadget/function/u_serial.h | 7
4 files changed, 97 insertions(+)
diff --git a/drivers/usb/gadget/function/f_acm.c
b/drivers/usb/gadget/function/f_acm.c
index 9fc98de83624..7c152c28b26c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -771,6 +771,24 @@ static struct configfs_item_operations acm_item_ops = {
.release= acm_attr_release,
};
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_acm_console_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ return gserial_set_console(to_f_serial_opts(item)->port_num,
+ page, count);
+}
+
+static ssize_t f_acm_console_show(struct config_item *item, char *page)
+{
+ return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_acm_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
static ssize_t f_acm_port_num_show(struct config_item *item, char *page)
{
return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -779,6 +797,9 @@ static ssize_t f_acm_port_num_show(struct config_item
*item, char *page)
CONFIGFS_ATTR_RO(f_acm_, port_num);
static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ &f_acm_attr_console,
+#endif
&f_acm_attr_port_num,
NULL,
};
diff --git a/drivers/usb/gadget/function/f_serial.c
b/drivers/usb/gadget/function/f_serial.c
index c860f30a0ea2..1406255d0865 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -266,6 +266,24 @@ static struct configfs_item_operations serial_item_ops = {
.release= serial_attr_release,
};
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_serial_console_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ return gserial_set_console(to_f_serial_opts(item)->port_num,
+ page, count);
+}
+
+static ssize_t f_serial_console_show(struct config_item *item, char *page)
+{
+ return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_serial_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
static ssize_t f_serial_port_num_show(struct config_item *item, char *page)
{
return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -274,6 +292,9 @@ static ssize_t f_serial_port_num_show(struct config_item
*item, char *page)
CONFIGFS_ATTR_RO(f_serial_, port_num);
static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ &f_serial_attr_console,
+#endif
&f_serial_attr_port_num,
NULL,
};
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 62280c23cde2..ccfa8c6a35ac 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1081,6 +1081,54 @@ static void gs_console_exit(struct gs_port *port)
port->console = NULL;
}
+ssize_t gserial_set_console(unsigned char port_num, const char *page, size_t
count)
+{
+ struct gs_port *port;
+ bool enable;
+ int ret;
+
+ ret = strtobool(page, &enable);
+ if (ret)
+ return ret;
+
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+
+ if (WARN_ON(port == NULL)) {
+ ret = -ENXIO;
+ goto out;
+ }
+
+ if (enable)
+ ret = gs_console_init(port);
+ else
+ gs_console_exit(port);
+out:
+ mutex_unlock(&ports[port_num].lock);
+
+ return ret < 0 ? ret : count;
+}
+EXPORT_SYMBOL_GPL(gserial_set_console);
+
+ssize_t gserial_get_console(unsigned char port_num, char *page)
+{
+ struct gs_port *port;
+ ssize_t ret = -ENXIO;
+
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+
+ if (WARN_ON(port == NULL))
+ goto out;
+
+ ret = sprintf(page, "%u\n", !!port->console);
+
+ mutex_unlock(&ports[port_num].lock);
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gserial_get_console);
+
#else
static int gs_console_connect(struct gs_port *port)
diff --git a/drivers/usb/gadget/function/u_serial.h
b/drivers/usb/gadget/function/u_serial.h
index 8b472b0c8cb4..e5b08ab8cf7a 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -58,6 +58,13 @@ int gserial_allo
[PATCH v5 0/6] usb: gadget: u_serial: console improvements
This series makes it possible to have more control over console using USB serial gadget ports. This can be useful when you need more than one USB console or are configuring multiple serial port function via configfs. The patches are against usb-next tree. You can also pull from: https://rere.qmqm.pl/git/linux usb-console Michał Mirosław (6): usb: gadget: u_serial: add missing port entry locking usb: gadget: u_serial: reimplement console support usb: gadget: u_serial: make OBEX port not a console usb: gadget: u_serial: allow more console gadget ports usb: gadget: u_serial: diagnose missed console messages usb: gadget: legacy/serial: allow dynamic removal drivers/usb/gadget/function/f_acm.c| 21 ++ drivers/usb/gadget/function/f_obex.c | 2 +- drivers/usb/gadget/function/f_serial.c | 21 ++ drivers/usb/gadget/function/u_serial.c | 420 ++--- drivers/usb/gadget/function/u_serial.h | 8 + drivers/usb/gadget/legacy/serial.c | 49 ++- 6 files changed, 333 insertions(+), 188 deletions(-) -- 2.20.1
[PATCH v5 1/6] usb: gadget: u_serial: add missing port entry locking
gserial_alloc_line() misses locking (for a release barrier) while resetting port entry on TTY allocation failure. Fix this. Cc: sta...@vger.kernel.org Signed-off-by: Michał Mirosław Reviewed-by: Greg Kroah-Hartman Tested-by: Ladislav Michl --- v5: no changes v4: no changes v3: cc-stable v2: no changes --- drivers/usb/gadget/function/u_serial.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index 65f634ec7fc2..bb1e2e1d0076 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1239,8 +1239,10 @@ int gserial_alloc_line(unsigned char *line_num) __func__, port_num, PTR_ERR(tty_dev)); ret = PTR_ERR(tty_dev); + mutex_lock(&ports[port_num].lock); port = ports[port_num].port; ports[port_num].port = NULL; + mutex_unlock(&ports[port_num].lock); gserial_free_port(port); goto err; } -- 2.20.1
[PATCH v5 5/6] usb: gadget: u_serial: diagnose missed console messages
Insert markers in console stream marking places where data
is missing. This makes the hole in the data stand out clearly
instead of glueing together unrelated messages.
Example output as seen from USB host side:
[0.064078] pinctrl core: registered pin 16 (UART3_RTS_N PC0) on
7868.pinmux
[0.064130] pinctrl
[missed 114987 bytes]
[4.302299] udevd[134]: starting version 3.2.5
[4.306845] random: udevd: uninitialized urandom read (16 bytes read)
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
---
v5: no changes
v4: no changes
v3: added example output
+ lowercase "missed"
v2: commit message massage
---
drivers/usb/gadget/function/u_serial.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 0da00546006f..a248ed0fd5d2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -88,6 +88,7 @@ struct gs_console {
spinlock_t lock;
struct usb_request *req;
struct kfifobuf;
+ size_t missed;
};
/*
@@ -931,6 +932,15 @@ static void __gs_console_push(struct gs_console *cons)
if (!size)
return;
+ if (cons->missed && ep->maxpacket >= 64) {
+ char buf[64];
+ size_t len;
+
+ len = sprintf(buf, "\n[missed %zu bytes]\n", cons->missed);
+ kfifo_in(&cons->buf, buf, len);
+ cons->missed = 0;
+ }
+
req->length = size;
if (usb_ep_queue(ep, req, GFP_ATOMIC))
req->length = 0;
@@ -952,10 +962,13 @@ static void gs_console_write(struct console *co,
{
struct gs_console *cons = container_of(co, struct gs_console, console);
unsigned long flags;
+ size_t n;
spin_lock_irqsave(&cons->lock, flags);
- kfifo_in(&cons->buf, buf, count);
+ n = kfifo_in(&cons->buf, buf, count);
+ if (n < count)
+ cons->missed += count - n;
if (cons->req && !cons->req->length)
schedule_work(&cons->work);
--
2.20.1
[PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
Rewrite console support to fix a few shortcomings of the old code
preventing its use with multiple ports. This removes some duplicated
code and replaces a custom kthread with simpler workqueue item.
Only port ttyGS0 gets to be a console for now.
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
Tested-by: Ladislav Michl
---
v5: no changes
v4: cosmetic change to __gs_console_push()
v3: no changes
v2: no changes
---
drivers/usb/gadget/function/u_serial.c | 351 -
1 file changed, 164 insertions(+), 187 deletions(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index bb1e2e1d0076..94f6999e8262 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -82,14 +82,12 @@
#define GS_CONSOLE_BUF_SIZE8192
/* console info */
-struct gscons_info {
- struct gs_port *port;
- struct task_struct *console_thread;
- struct kfifocon_buf;
- /* protect the buf and busy flag */
- spinlock_t con_lock;
- int req_busy;
- struct usb_request *console_req;
+struct gs_console {
+ struct console console;
+ struct work_struct work;
+ spinlock_t lock;
+ struct usb_request *req;
+ struct kfifobuf;
};
/*
@@ -101,6 +99,9 @@ struct gs_port {
spinlock_t port_lock; /* guard port_* access */
struct gserial *port_usb;
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ struct gs_console *console;
+#endif
boolopenclose; /* open/close in progress */
u8 port_num;
@@ -889,36 +890,9 @@ static struct tty_driver *gs_tty_driver;
#ifdef CONFIG_U_SERIAL_CONSOLE
-static struct gscons_info gscons_info;
-static struct console gserial_cons;
-
-static struct usb_request *gs_request_new(struct usb_ep *ep)
+static void gs_console_complete_out(struct usb_ep *ep, struct usb_request *req)
{
- struct usb_request *req = usb_ep_alloc_request(ep, GFP_ATOMIC);
- if (!req)
- return NULL;
-
- req->buf = kmalloc(ep->maxpacket, GFP_ATOMIC);
- if (!req->buf) {
- usb_ep_free_request(ep, req);
- return NULL;
- }
-
- return req;
-}
-
-static void gs_request_free(struct usb_request *req, struct usb_ep *ep)
-{
- if (!req)
- return;
-
- kfree(req->buf);
- usb_ep_free_request(ep, req);
-}
-
-static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
-{
- struct gscons_info *info = &gscons_info;
+ struct gs_console *cons = req->context;
switch (req->status) {
default:
@@ -927,12 +901,12 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
/* fall through */
case 0:
/* normal completion */
- spin_lock(&info->con_lock);
- info->req_busy = 0;
- spin_unlock(&info->con_lock);
-
- wake_up_process(info->console_thread);
+ spin_lock(&cons->lock);
+ req->length = 0;
+ schedule_work(&cons->work);
+ spin_unlock(&cons->lock);
break;
+ case -ECONNRESET:
case -ESHUTDOWN:
/* disconnect */
pr_vdebug("%s: %s shutdown\n", __func__, ep->name);
@@ -940,190 +914,190 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
}
}
-static int gs_console_connect(int port_num)
+static void __gs_console_push(struct gs_console *cons)
{
- struct gscons_info *info = &gscons_info;
- struct gs_port *port;
+ struct usb_request *req = cons->req;
struct usb_ep *ep;
+ size_t size;
- if (port_num != gserial_cons.index) {
- pr_err("%s: port num [%d] is not support console\n",
- __func__, port_num);
- return -ENXIO;
- }
+ if (!req)
+ return; /* disconnected */
- port = ports[port_num].port;
- ep = port->port_usb->in;
- if (!info->console_req) {
- info->console_req = gs_request_new(ep);
- if (!info->console_req)
- return -ENOMEM;
- info->console_req->complete = gs_complete_out;
- }
+ if (req->length)
+ return; /* busy */
- info->port = port;
- spin_lock(&info->con_lock);
- info->req_busy = 0;
- spin_unlock(&info->con_lock);
- pr_vdebug("port[%d] console connect!\n", port_num);
- return 0;
-}
-
-static void gs_console_disconnect(struct usb_ep *ep)
-{
- struct gscons_info *info = &gscons_info;
- struct usb_request *req = info->console_req;
-
- gs_request_free(req, ep);
- info->console_req = NULL;
[PATCH v5 4/6] usb: gadget: u_serial: allow more console gadget ports
Allow configuring more than one console using USB serial or ACM gadget.
By default, only first (ttyGS0) is a console, but this may be changed
using function's new "console" attribute.
Signed-off-by: Michał Mirosław
---
v5: fixed locking in gserial_get_console()
v4: fixed locking in gserial_set_console()
v3: no changes
v2: no changes
---
drivers/usb/gadget/function/f_acm.c| 21 +++
drivers/usb/gadget/function/f_serial.c | 21 +++
drivers/usb/gadget/function/u_serial.c | 48 ++
drivers/usb/gadget/function/u_serial.h | 7
4 files changed, 97 insertions(+)
diff --git a/drivers/usb/gadget/function/f_acm.c
b/drivers/usb/gadget/function/f_acm.c
index 9fc98de83624..7c152c28b26c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -771,6 +771,24 @@ static struct configfs_item_operations acm_item_ops = {
.release= acm_attr_release,
};
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_acm_console_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ return gserial_set_console(to_f_serial_opts(item)->port_num,
+ page, count);
+}
+
+static ssize_t f_acm_console_show(struct config_item *item, char *page)
+{
+ return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_acm_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
static ssize_t f_acm_port_num_show(struct config_item *item, char *page)
{
return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -779,6 +797,9 @@ static ssize_t f_acm_port_num_show(struct config_item
*item, char *page)
CONFIGFS_ATTR_RO(f_acm_, port_num);
static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ &f_acm_attr_console,
+#endif
&f_acm_attr_port_num,
NULL,
};
diff --git a/drivers/usb/gadget/function/f_serial.c
b/drivers/usb/gadget/function/f_serial.c
index c860f30a0ea2..1406255d0865 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -266,6 +266,24 @@ static struct configfs_item_operations serial_item_ops = {
.release= serial_attr_release,
};
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_serial_console_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ return gserial_set_console(to_f_serial_opts(item)->port_num,
+ page, count);
+}
+
+static ssize_t f_serial_console_show(struct config_item *item, char *page)
+{
+ return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_serial_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
static ssize_t f_serial_port_num_show(struct config_item *item, char *page)
{
return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -274,6 +292,9 @@ static ssize_t f_serial_port_num_show(struct config_item
*item, char *page)
CONFIGFS_ATTR_RO(f_serial_, port_num);
static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ &f_serial_attr_console,
+#endif
&f_serial_attr_port_num,
NULL,
};
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 62280c23cde2..0da00546006f 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1081,6 +1081,54 @@ static void gs_console_exit(struct gs_port *port)
port->console = NULL;
}
+ssize_t gserial_set_console(unsigned char port_num, const char *page, size_t
count)
+{
+ struct gs_port *port;
+ bool enable;
+ int ret;
+
+ ret = strtobool(page, &enable);
+ if (ret)
+ return ret;
+
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+
+ if (WARN_ON(port == NULL)) {
+ ret = -ENXIO;
+ goto out;
+ }
+
+ if (enable)
+ ret = gs_console_init(port);
+ else
+ gs_console_exit(port);
+out:
+ mutex_unlock(&ports[port_num].lock);
+
+ return ret < 0 ? ret : count;
+}
+EXPORT_SYMBOL_GPL(gserial_set_console);
+
+ssize_t gserial_get_console(unsigned char port_num, char *page)
+{
+ struct gs_port *port;
+ ssize_t ret;
+
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+
+ if (WARN_ON(port == NULL))
+ ret = -ENXIO;
+ else
+ ret = sprintf(page, "%u\n", !!port->console);
+
+ mutex_unlock(&ports[port_num].lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gserial_get_console);
+
#else
static int gs_console_connect(struct gs_port *port)
diff --git a/drivers/usb/gadget/function/u_serial.h
b/drivers/usb/gadget/function/u_serial.h
index 8b472b0c8cb4..e5b08ab8cf7a 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/
[PATCH v5 3/6] usb: gadget: u_serial: make OBEX port not a console
Prevent OBEX serial port from ever becoming a console. Console messages
will definitely break the protocol, and since you have to instantiate
the port making it explicitly for OBEX, there is no point in allowing
console to break it by mistake.
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
---
v5: no changes
v4: no changes
v3: rename gserial_alloc_line_raw() -> gserial_alloc_line_no_console()
v2: change of API
+ commit message massage
---
drivers/usb/gadget/function/f_obex.c | 2 +-
drivers/usb/gadget/function/u_serial.c | 16
drivers/usb/gadget/function/u_serial.h | 1 +
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/function/f_obex.c
b/drivers/usb/gadget/function/f_obex.c
index 55b7f57d2dc7..ab26d84ed95e 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -432,7 +432,7 @@ static struct usb_function_instance *obex_alloc_inst(void)
return ERR_PTR(-ENOMEM);
opts->func_inst.free_func_inst = obex_free_inst;
- ret = gserial_alloc_line(&opts->port_num);
+ ret = gserial_alloc_line_no_console(&opts->port_num);
if (ret) {
kfree(opts);
return ERR_PTR(ret);
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 94f6999e8262..62280c23cde2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1180,7 +1180,7 @@ void gserial_free_line(unsigned char port_num)
}
EXPORT_SYMBOL_GPL(gserial_free_line);
-int gserial_alloc_line(unsigned char *line_num)
+int gserial_alloc_line_no_console(unsigned char *line_num)
{
struct usb_cdc_line_coding coding;
struct gs_port *port;
@@ -1221,12 +1221,20 @@ int gserial_alloc_line(unsigned char *line_num)
goto err;
}
*line_num = port_num;
-
- if (!port_num)
- gs_console_init(port);
err:
return ret;
}
+EXPORT_SYMBOL_GPL(gserial_alloc_line_no_console);
+
+int gserial_alloc_line(unsigned char *line_num)
+{
+ int ret = gserial_alloc_line_no_console(line_num);
+
+ if (!ret && !*line_num)
+ gs_console_init(ports[*line_num].port);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(gserial_alloc_line);
/**
diff --git a/drivers/usb/gadget/function/u_serial.h
b/drivers/usb/gadget/function/u_serial.h
index 9acaac1cbb75..8b472b0c8cb4 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -54,6 +54,7 @@ struct usb_request *gs_alloc_req(struct usb_ep *ep, unsigned
len, gfp_t flags);
void gs_free_req(struct usb_ep *, struct usb_request *req);
/* management of individual TTY ports */
+int gserial_alloc_line_no_console(unsigned char *port_line);
int gserial_alloc_line(unsigned char *port_line);
void gserial_free_line(unsigned char port_line);
--
2.20.1
[PATCH v5 6/6] usb: gadget: legacy/serial: allow dynamic removal
Legacy serial USB gadget is still useful as an early console,
before userspace is up. Later it could be replaced with proper
configfs-configured composite gadget - that use case is enabled
by this patch.
Signed-off-by: Michał Mirosław
---
v5: no changes
v4: initial revision, new in the patchset
---
drivers/usb/gadget/legacy/serial.c | 49 +-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/legacy/serial.c
b/drivers/usb/gadget/legacy/serial.c
index de30d7628eef..da44f89f5e73 100644
--- a/drivers/usb/gadget/legacy/serial.c
+++ b/drivers/usb/gadget/legacy/serial.c
@@ -97,6 +97,36 @@ static unsigned n_ports = 1;
module_param(n_ports, uint, 0);
MODULE_PARM_DESC(n_ports, "number of ports to create, default=1");
+static bool enable = true;
+
+static int switch_gserial_enable(bool do_enable);
+
+static int enable_set(const char *s, const struct kernel_param *kp)
+{
+ bool do_enable;
+ int ret;
+
+ if (!s) /* called for no-arg enable == default */
+ return 0;
+
+ ret = strtobool(s, &do_enable);
+ if (ret || enable == do_enable)
+ return ret;
+
+ ret = switch_gserial_enable(do_enable);
+ if (!ret)
+ enable = do_enable;
+
+ return ret;
+}
+
+static const struct kernel_param_ops enable_ops = {
+ .set = enable_set,
+ .get = param_get_bool,
+};
+
+module_param_cb(enable, &enable_ops, &enable, 0644);
+
/*-*/
static struct usb_configuration serial_config_driver = {
@@ -240,6 +270,19 @@ static struct usb_composite_driver gserial_driver = {
.unbind = gs_unbind,
};
+static int switch_gserial_enable(bool do_enable)
+{
+ if (!serial_config_driver.label)
+ /* init() was not called, yet */
+ return 0;
+
+ if (do_enable)
+ return usb_composite_probe(&gserial_driver);
+
+ usb_composite_unregister(&gserial_driver);
+ return 0;
+}
+
static int __init init(void)
{
/* We *could* export two configs; that'd be much cleaner...
@@ -266,12 +309,16 @@ static int __init init(void)
}
strings_dev[STRING_DESCRIPTION_IDX].s = serial_config_driver.label;
+ if (!enable)
+ return 0;
+
return usb_composite_probe(&gserial_driver);
}
module_init(init);
static void __exit cleanup(void)
{
- usb_composite_unregister(&gserial_driver);
+ if (enable)
+ usb_composite_unregister(&gserial_driver);
}
module_exit(cleanup);
--
2.20.1
Re: [PATCH] phy: core: document calibrate() method
Hi Kishon,
On 2019-07-19 14:25, Kishon Vijay Abraham I wrote:
> On 19/07/19 5:51 PM, Marek Szyprowski wrote:
>> On 2019-07-19 12:04, Kishon Vijay Abraham I wrote:
>>> On 19/07/19 3:22 PM, Marek Szyprowski wrote:
Commit 36914111e682 ("drivers: phy: add calibrate method") added support
for generic phy_calibrate() method, but it didn't explain in detail when
such method is supposed to be called. Add some more documentation directly
to the phy.h to make it clean that it is intended to be called after every
host controller reset.
Signed-off-by: Marek Szyprowski
---
include/linux/phy/phy.h | 12
1 file changed, 12 insertions(+)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 15032f145063..46775e8b0ed9 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -101,6 +101,18 @@ struct phy_ops {
int (*validate)(struct phy *phy, enum phy_mode mode, int
submode,
union phy_configure_opts *opts);
int (*reset)(struct phy *phy);
+
+ /**
+ * @calibrate:
+ *
+ * Optional.
+ *
+ * Used to calibrate phy, typically by adjusting some parameters
+ * in runtime, which are otherwise lost after host controller
+ * reset and cannot be set in phy_init() and phy_power_on().
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
int (*calibrate)(struct phy *phy);
>>> This should be added in drivers/phy/phy-core.c before phy_calibrate()?
>> Okay. Do you want to keep it also in phy.h (like for validate and
>> configure)?
> No. Adding in phy.txt and before phy_calibrate() should suffice IMO.
OKay, I will move it to phy.c. Generic PHYs documentation is converted
to RST (Documentation/driver-api/phy/phy.rst), so it could simply
include those comments from .c file. This can be done in separate patch.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
[PATCH v2] phy: core: document phy_calibrate()
Commit 36914111e682 ("drivers: phy: add calibrate method") added support
for generic phy_calibrate() method, but it didn't explain in detail when
such method is supposed to be called. Add some more documentation directly
to the phy.h to make it clean that it is intended to be called after every
host controller reset.
Signed-off-by: Marek Szyprowski
---
drivers/phy/phy-core.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index e3880c4a15f2..b04f4fe85ac2 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -394,6 +394,16 @@ int phy_reset(struct phy *phy)
}
EXPORT_SYMBOL_GPL(phy_reset);
+/**
+ * phy_calibrate() - Tunes the phy hw parameters for current configuration
+ * @phy: the phy returned by phy_get()
+ *
+ * Used to calibrate phy hardware, typically by adjusting some parameters in
+ * runtime, which are otherwise lost after host controller reset and cannot
+ * be applied in phy_init() or phy_power_on().
+ *
+ * Returns: 0 if successful, an negative error code otherwise
+ */
int phy_calibrate(struct phy *phy)
{
int ret;
--
2.17.1
Re: [PATCH v2 1/2] usb: core: phy: add support for PHY calibration
Hi Marek,
On Fri, 19 Jul 2019 at 13:43, Marek Szyprowski wrote:
>
> Some PHYs (for example Exynos5 USB3.0 DRD PHY) require calibration to be
> done after every USB HCD reset. Generic PHY framework has been already
> extended with phy_calibrate() function in commit 36914111e682 ("drivers:
> phy: add calibrate method"). This patch adds support for it to generic
> PHY handling code in USB HCD core.
>
Tested on my XU3 / XU4 / HC1
Tested-by: Anand Moon
> Signed-off-by: Marek Szyprowski
> ---
> drivers/usb/core/hcd.c | 7 +++
> drivers/usb/core/phy.c | 21 +
> drivers/usb/core/phy.h | 1 +
> 3 files changed, 29 insertions(+)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 88533938ce19..b89936c1df23 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2291,6 +2291,9 @@ int hcd_bus_resume(struct usb_device *rhdev,
> pm_message_t msg)
> hcd->state = HC_STATE_RESUMING;
> status = hcd->driver->bus_resume(hcd);
> clear_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
> + if (status == 0)
> + status = usb_phy_roothub_calibrate(hcd->phy_roothub);
> +
> if (status == 0) {
> struct usb_device *udev;
> int port1;
> @@ -2864,6 +2867,10 @@ int usb_add_hcd(struct usb_hcd *hcd,
> }
> hcd->rh_pollable = 1;
>
> + retval = usb_phy_roothub_calibrate(hcd->phy_roothub);
> + if (retval)
> + goto err_hcd_driver_setup;
> +
> /* NOTE: root hub and controller capabilities may not be the same */
> if (device_can_wakeup(hcd->self.controller)
> && device_can_wakeup(&hcd->self.root_hub->dev))
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index 7580493b867a..fb1588e7c282 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -151,6 +151,27 @@ int usb_phy_roothub_set_mode(struct usb_phy_roothub
> *phy_roothub,
> }
> EXPORT_SYMBOL_GPL(usb_phy_roothub_set_mode);
>
> +int usb_phy_roothub_calibrate(struct usb_phy_roothub *phy_roothub)
> +{
> + struct usb_phy_roothub *roothub_entry;
> + struct list_head *head;
> + int err;
> +
> + if (!phy_roothub)
> + return 0;
> +
> + head = &phy_roothub->list;
> +
> + list_for_each_entry(roothub_entry, head, list) {
> + err = phy_calibrate(roothub_entry->phy);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_calibrate);
> +
> int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub)
> {
> struct usb_phy_roothub *roothub_entry;
> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
> index dad564e2d2d4..20a267cd986b 100644
> --- a/drivers/usb/core/phy.h
> +++ b/drivers/usb/core/phy.h
> @@ -18,6 +18,7 @@ int usb_phy_roothub_exit(struct usb_phy_roothub
> *phy_roothub);
>
> int usb_phy_roothub_set_mode(struct usb_phy_roothub *phy_roothub,
> enum phy_mode mode);
> +int usb_phy_roothub_calibrate(struct usb_phy_roothub *phy_roothub);
> int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
> void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
>
> --
> 2.17.1
>
Re: [PATCH v2 2/2] usb: dwc3: remove generic PHY calibrate() calls
Hi Marek, On Fri, 19 Jul 2019 at 13:43, Marek Szyprowski wrote: > > Calls to USB2 generic PHY calibrate() method has been moved to HCD core, > which now successfully handles generic PHYs and their calibration after > every HCD reset. This fixes all the timing issues related to PHY > calibration done directly from DWC3 driver: incorrect operation after > system suspend/resume or USB3.0 detection failure when XHCI-plat driver > compiled as separate module. > Tested on my XU3 / XU4 / HC1 Tested-by: Anand Moon > Signed-off-by: Marek Szyprowski > --- > drivers/usb/dwc3/core.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index c9bb93a2c81e..7dd6d419254d 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -168,7 +168,6 @@ static void __dwc3_set_mode(struct work_struct *work) > otg_set_vbus(dwc->usb2_phy->otg, true); > phy_set_mode(dwc->usb2_generic_phy, > PHY_MODE_USB_HOST); > phy_set_mode(dwc->usb3_generic_phy, > PHY_MODE_USB_HOST); > - phy_calibrate(dwc->usb2_generic_phy); > } > break; > case DWC3_GCTL_PRTCAP_DEVICE: > @@ -1166,7 +1165,6 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > dev_err(dev, "failed to initialize host\n"); > return ret; > } > - phy_calibrate(dwc->usb2_generic_phy); > break; > case USB_DR_MODE_OTG: > INIT_WORK(&dwc->drd_work, __dwc3_set_mode); > -- > 2.17.1 >
Re: [PATCH] net: lan78xx: Merge memcpy + lexx_to_cpus to get_unaligned_lexx
From: Chuhong Yuan Date: Fri, 19 Jul 2019 15:36:15 +0800 > Merge the combo use of memcpy and lexx_to_cpus. > Use get_unaligned_lexx instead. > This simplifies the code. > > Signed-off-by: Chuhong Yuan Applied.
Re: [PATCH] usbnet: smsc75xx: Merge memcpy + le32_to_cpus to get_unaligned_le32
From: Chuhong Yuan Date: Fri, 19 Jul 2019 16:27:31 +0800 > Merge the combo use of memcpy and le32_to_cpus. > Use get_unaligned_le32 instead. > This simplifies the code. > > Signed-off-by: Chuhong Yuan Applied.
Re: [PATCH] ax88179_178a: Merge memcpy + le32_to_cpus to get_unaligned_le32
From: Chuhong Yuan Date: Fri, 19 Jul 2019 17:07:15 +0800 > Merge the combo use of memcpy and le32_to_cpus. > Use get_unaligned_le32 instead. > This simplifies the code. > > Signed-off-by: Chuhong Yuan Applied.
Re: [PATCH v2 1/2] usbip: Skip DMA mapping and unmapping for urb at vhci
Hi Suwan,
On 7/5/19 10:43 AM, Suwan Kim wrote:
vhci doesn’t do DMA for remote device. Actually, the real DMA
operation is done by network card driver. vhci just passes virtual
address of the buffer to the network stack, so vhci doesn’t use and
need dma address of the buffer of the URB.
When it comes to supporting SG for vhci, it is useful to use native
SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
fnuction can adjust the number of SG list (urb->num_mapped_sgs).
But HCD provides DMA mapping and unmapping function by default.
Moreover, it causes unnecessary DMA mapping and unmapping which
will be done again at the NIC driver and it wastes CPU cycles.
So, implement map_urb_for_dma and unmap_urb_for_dma function for
vhci in order to skip the DMA mapping and unmapping procedure.
To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in
urb->transfer_flags if URB has SG list and this flag will tell the
stub driver to use SG list.
Signed-off-by: Suwan Kim
---
drivers/usb/usbip/vhci_hcd.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 000ab7225717..14fc6d9f4e6a 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct
usb_device *udev,
return 0;
}
+static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ dev_dbg(hcd->self.controller, "vhci does not map urb for dma\n");
+
+ if (urb->num_sgs)
+ urb->transfer_flags |= URB_DMA_MAP_SG;
+
Shouldn't this be part of patch 2. The debug message saying "no map"
and setting flag doesn't make sense.
+ return 0;
This should be a tab and no spaces btw. chekpatch isn't happy.
+}
+
+static void vhci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
+{
+ dev_dbg(hcd->self.controller, "vhci does not unmap urb for dma\n");
This should be a tab and no spaces btw. chekpatch isn't happy.
WARNING: please, no spaces at the start of a line
#144: FILE: drivers/usb/usbip/vhci_hcd.c:1299:
+ return 0;$
WARNING: please, no spaces at the start of a line
#149: FILE: drivers/usb/usbip/vhci_hcd.c:1304:
+ dev_dbg(hcd->self.controller, "vhci does not unmap urb for dma\n");$
total: 0 errors, 2 warnings, 31 lines checked
+}
+
static const struct hc_driver vhci_hc_driver = {
.description= driver_name,
.product_desc = driver_desc,
@@ -1304,6 +1320,9 @@ static const struct hc_driver vhci_hc_driver = {
.get_frame_number = vhci_get_frame_number,
+ .map_urb_for_dma = vhci_map_urb_for_dma,
+ .unmap_urb_for_dma = vhci_unmap_urb_for_dma,
+
.hub_status_data = vhci_hub_status,
.hub_control= vhci_hub_control,
.bus_suspend= vhci_bus_suspend,
thanks,
-- Shuah
Re: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
On Mon 2019-07-22 13:56:44, Pavel Machek wrote: > Hi! > > > > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. > > > >> > > > >> The Cadence USBSS DRD Controller is a highly configurable IP Core which > > > >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and > > > >> Host Only (XHCI)configurations. > > > > > > > >I see you are using debugfs to select between DRD, peripheral-onlyh and > > > >XHCI... > > > > > > > >Is that good idea? > > > > > > Yes driver allows selecting dr_mode by debugfs. Controller also support > > > such functionality > > > so I don't understand why would it not be a good idea. > > > > > > I personally use this for testing but it can be used to limit controller > > > functionality without > > > recompiling kernel. > > > > debugfs is ONLY for debugging, never rely on it being enabled, or > > mounted, on a system in order to have any normal operation happen. > > > > So for testing, yes, this is fine. If this is going to be the normal > > api/interface for how to control this driver, no, that is not acceptable > > at all. > > It makes a lot of sense for end-user to toggle this... for example > when he is lacking right cable for proper otg detection. As it is > third driver offering this functionality, I believe we should stop > treating it as debugging. At least renesas usb controller seems to have variables in sysfs: drivers/phy/renesas/phy-rcar-gen3-usb2.c : functions role_show and role_store. See also Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2 . I believe this driver should do same. Thanks and best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany signature.asc Description: Digital signature
Enumeration of USB keyboard connected to dock fails during boot
Hi, I have a USB keyboard that works in UEFI apps (and grub) but if plugged into a dock stops working when Linux boots. It works when hotplugged into the dock or plugged into the laptop directly before or after booting. A mouse plugged into the dock does not share this problem. (I have no other keyboard to test at hand.) Kernel version: 5.1.18-300.fc30.x86_64 XHCI USB controller: Advanced Micro Devices, Inc. [AMD] Raven USB 3.1 Hub: 17ef:a38f Lenovo USB2.0 Hub Keyboard: 04d9:0192 Holtek Semiconductor, Inc. USB-HID Keyboard Relevant kernel messages are attached. My ignorant initial thought is that the hub should be transparent to the keyboard, so that the fault is unlikely to lie with the keyboard. I have unsuccessfully tried the delayed initialisation quirk on the keyboard. I have asked the Fedora kernel developers about this problem first -- they told me to ask here. Thanks for your help. Jul 22 19:16:14 kernel: usb 2-1.3.3.3: new full-speed USB device number 9 using xhci_hcd Jul 22 19:16:14 kernel: usb 2-1.3.3.3: device descriptor read/64, error -32 Jul 22 19:16:14 kernel: usb 2-1.3.3.3: device descriptor read/64, error -32 Jul 22 19:16:15 kernel: usb 2-1.3.3.3: new full-speed USB device number 10 using xhci_hcd Jul 22 19:16:15 kernel: usb 2-1.3.3.3: device descriptor read/64, error -32 Jul 22 19:16:15 kernel: usb 2-1.3.3.3: device descriptor read/64, error -32 Jul 22 19:16:15 kernel: usb 2-1.3.3-port3: attempt power cycle Jul 22 19:16:15 kernel: usb 2-1.3.3.3: new full-speed USB device number 11 using xhci_hcd Jul 22 19:16:15 kernel: usb 2-1.3.3.3: Device not responding to setup address. Jul 22 19:16:16 kernel: usb 2-1.3.3.3: Device not responding to setup address. Jul 22 19:16:16 kernel: usb 2-1.3.3.3: device not accepting address 11, error -71 Jul 22 19:16:16 kernel: usb 2-1.3.3.3: new full-speed USB device number 12 using xhci_hcd Jul 22 19:16:16 kernel: usb 2-1.3.3.3: Device not responding to setup address. Jul 22 19:16:16 kernel: usb 2-1.3.3.3: Device not responding to setup address. Jul 22 19:16:16 kernel: usb 2-1.3.3.3: device not accepting address 12, error -71 Jul 22 19:16:16 kernel: usb 2-1.3.3-port3: unable to enumerate USB device
Re: [PATCH] net: usb: Merge cpu_to_le32s + memcpy to put_unaligned_le32
From: Chuhong Yuan Date: Mon, 22 Jul 2019 15:41:34 +0800 > Merge the combo uses of cpu_to_le32s and memcpy. > Use put_unaligned_le32 instead. > This simplifies the code. > > Signed-off-by: Chuhong Yuan Isn't the skb->data aligned to 4 bytes in these situations? If so, we should use the aligned variants. Thank you.
Re: [PATCH] net: usb: Merge cpu_to_le32s + memcpy to put_unaligned_le32
David Miller 于2019年7月23日周二 上午9:22写道： > > From: Chuhong Yuan > Date: Mon, 22 Jul 2019 15:41:34 +0800 > > > Merge the combo uses of cpu_to_le32s and memcpy. > > Use put_unaligned_le32 instead. > > This simplifies the code. > > > > Signed-off-by: Chuhong Yuan > > Isn't the skb->data aligned to 4 bytes in these situations? > > If so, we should use the aligned variants. > > Thank you. I have checked the five changed files. I find that they all have used get_unaligned_le32 for skb->data according to my previous applied patches and existing code. So I think the skb->data is unaligned in these situations. Usages of get_unaligned_le32: asix_common.c: line 104 and 133 ax88179_178a.c: https://lkml.org/lkml/2019/7/19/652 lan78xx.c: https://lkml.org/lkml/2019/7/19/573 smsc75xx.c: https://lkml.org/lkml/2019/7/19/617 sr9800.c: line 73
Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
Hi Michal,
On Mon, 22 Jul 2019 at 23:26, Michał Mirosław wrote:
>
> Rewrite console support to fix a few shortcomings of the old code
> preventing its use with multiple ports. This removes some duplicated
> code and replaces a custom kthread with simpler workqueue item.
Could you elaborate on why changing kthread to a workqueue? The
purpose of using kthread here is considering that the kthead has a
better scheduler response than pooled kworker.
>
> Only port ttyGS0 gets to be a console for now.
>
> Signed-off-by: Michał Mirosław
> Reviewed-by: Greg Kroah-Hartman
> Tested-by: Ladislav Michl
>
> ---
> v5: no changes
> v4: cosmetic change to __gs_console_push()
> v3: no changes
> v2: no changes
>
> ---
> drivers/usb/gadget/function/u_serial.c | 351 -
> 1 file changed, 164 insertions(+), 187 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_serial.c
> b/drivers/usb/gadget/function/u_serial.c
> index bb1e2e1d0076..94f6999e8262 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -82,14 +82,12 @@
> #define GS_CONSOLE_BUF_SIZE8192
>
> /* console info */
> -struct gscons_info {
> - struct gs_port *port;
> - struct task_struct *console_thread;
> - struct kfifocon_buf;
> - /* protect the buf and busy flag */
> - spinlock_t con_lock;
> - int req_busy;
> - struct usb_request *console_req;
> +struct gs_console {
> + struct console console;
> + struct work_struct work;
> + spinlock_t lock;
> + struct usb_request *req;
> + struct kfifobuf;
> };
>
> /*
> @@ -101,6 +99,9 @@ struct gs_port {
> spinlock_t port_lock; /* guard port_* access */
>
> struct gserial *port_usb;
> +#ifdef CONFIG_U_SERIAL_CONSOLE
> + struct gs_console *console;
> +#endif
>
> boolopenclose; /* open/close in progress */
> u8 port_num;
> @@ -889,36 +890,9 @@ static struct tty_driver *gs_tty_driver;
>
> #ifdef CONFIG_U_SERIAL_CONSOLE
>
> -static struct gscons_info gscons_info;
> -static struct console gserial_cons;
> -
> -static struct usb_request *gs_request_new(struct usb_ep *ep)
> +static void gs_console_complete_out(struct usb_ep *ep, struct usb_request
> *req)
> {
> - struct usb_request *req = usb_ep_alloc_request(ep, GFP_ATOMIC);
> - if (!req)
> - return NULL;
> -
> - req->buf = kmalloc(ep->maxpacket, GFP_ATOMIC);
> - if (!req->buf) {
> - usb_ep_free_request(ep, req);
> - return NULL;
> - }
> -
> - return req;
> -}
> -
> -static void gs_request_free(struct usb_request *req, struct usb_ep *ep)
> -{
> - if (!req)
> - return;
> -
> - kfree(req->buf);
> - usb_ep_free_request(ep, req);
> -}
> -
> -static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
> -{
> - struct gscons_info *info = &gscons_info;
> + struct gs_console *cons = req->context;
>
> switch (req->status) {
> default:
> @@ -927,12 +901,12 @@ static void gs_complete_out(struct usb_ep *ep, struct
> usb_request *req)
> /* fall through */
> case 0:
> /* normal completion */
> - spin_lock(&info->con_lock);
> - info->req_busy = 0;
> - spin_unlock(&info->con_lock);
> -
> - wake_up_process(info->console_thread);
> + spin_lock(&cons->lock);
> + req->length = 0;
> + schedule_work(&cons->work);
> + spin_unlock(&cons->lock);
> break;
> + case -ECONNRESET:
> case -ESHUTDOWN:
> /* disconnect */
> pr_vdebug("%s: %s shutdown\n", __func__, ep->name);
> @@ -940,190 +914,190 @@ static void gs_complete_out(struct usb_ep *ep, struct
> usb_request *req)
> }
> }
>
> -static int gs_console_connect(int port_num)
> +static void __gs_console_push(struct gs_console *cons)
> {
> - struct gscons_info *info = &gscons_info;
> - struct gs_port *port;
> + struct usb_request *req = cons->req;
> struct usb_ep *ep;
> + size_t size;
>
> - if (port_num != gserial_cons.index) {
> - pr_err("%s: port num [%d] is not support console\n",
> - __func__, port_num);
> - return -ENXIO;
> - }
> + if (!req)
> + return; /* disconnected */
>
> - port = ports[port_num].port;
> - ep = port->port_usb->in;
> - if (!info->console_req) {
> - info->console_req = gs_request_new(ep);
> - if (!info->console_req)
> - return -ENOMEM;
> - info->console_req->complete = gs_complete_out;
[PATCH] usb: chipidea: msm: Use device-managed registration API
Use devm_reset_controller_register to get rid of manual unregistration. Signed-off-by: Chuhong Yuan --- drivers/usb/chipidea/ci_hdrc_msm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index bb4645a8ca46..067542e84aea 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -216,7 +216,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) ci->rcdev.ops = &ci_hdrc_msm_reset_ops; ci->rcdev.of_node = pdev->dev.of_node; ci->rcdev.nr_resets = 2; - ret = reset_controller_register(&ci->rcdev); + ret = devm_reset_controller_register(&pdev->dev, &ci->rcdev); if (ret) return ret; @@ -272,7 +272,6 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) err_iface: clk_disable_unprepare(ci->core_clk); err_fs: - reset_controller_unregister(&ci->rcdev); return ret; } @@ -284,7 +283,6 @@ static int ci_hdrc_msm_remove(struct platform_device *pdev) ci_hdrc_remove_device(ci->ci); clk_disable_unprepare(ci->iface_clk); clk_disable_unprepare(ci->core_clk); - reset_controller_unregister(&ci->rcdev); return 0; } -- 2.20.1
Re: [PATCH v4 4/6] usb: gadget: u_serial: allow more console gadget ports (fwd)
Hello,
Should the out: label be moved up one line?
julia
-- Forwarded message --
Date: Tue, 23 Jul 2019 10:54:22 +0800
From: kbuild test robot
To: kbu...@01.org
Cc: Julia Lawall
Subject: Re: [PATCH v4 4/6] usb: gadget: u_serial: allow more console gadget
ports
CC: kbuild-...@01.org
In-Reply-To:
<0f82c726dcf2d4c7819a7ccfb7be250b031884f7.1563808218.git.mirq-li...@rere.qmqm.pl>
References:
<0f82c726dcf2d4c7819a7ccfb7be250b031884f7.1563808218.git.mirq-li...@rere.qmqm.pl>
TO: "Micha?? Miros??aw"
CC: linux-usb@vger.kernel.org
CC: Felipe Balbi , Greg Kroah-Hartman
, Ladislav Michl
Hi "Micha??,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc1 next-20190722]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Micha-Miros-aw/usb-gadget-u_serial-console-improvements/20190723-084448
:: branch date: 2 hours ago
:: commit date: 2 hours ago
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Julia Lawall
>> drivers/usb/gadget/function/u_serial.c:1128:1-7: preceding lock on line 1118
#
https://github.com/0day-ci/linux/commit/a4ada1ad2a00fcc75fb461bd7733cb02c74bdd39
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a4ada1ad2a00fcc75fb461bd7733cb02c74bdd39
vim +1128 drivers/usb/gadget/function/u_serial.c
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1112
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1113 ssize_t gserial_get_console(unsigned char port_num, char
*page)
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1114 {
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1115 struct gs_port *port;
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1116 ssize_t ret = -ENXIO;
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1117
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 @1118 mutex_lock(&ports[port_num].lock);
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1119 port = ports[port_num].port;
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1120
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1121 if (WARN_ON(port == NULL))
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1122 goto out;
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1123
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1124 ret = sprintf(page, "%u\n", !!port->console);
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1125
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1126 mutex_unlock(&ports[port_num].lock);
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1127 out:
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 @1128 return ret;
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1129 }
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1130 EXPORT_SYMBOL_GPL(gserial_get_console);
a4ada1ad2a00fcc drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1131
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin Wang
2015-11-21 1132 #else
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin Wang
2015-11-21 1133
e273a647995aac7 drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1134 static int gs_console_connect(struct gs_port *port)
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin Wang
2015-11-21 1135 {
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin Wang
2015-11-21 1136 return 0;
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin Wang
2015-11-21 1137 }
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin Wang
2015-11-21 1138
e273a647995aac7 drivers/usb/gadget/function/u_serial.c Micha?? Miros??aw
2019-07-22 1139 static void gs_console_disconnect(struct gs_port *port)
a5beaaf39455e43 drivers/usb/gadget/function/u_serial.c Baolin W
Re: [PATCH] net: usb: Merge cpu_to_le32s + memcpy to put_unaligned_le32
From: Chuhong Yuan Date: Tue, 23 Jul 2019 10:16:27 +0800 > David Miller 于2019年7月23日周二 上午9:22写道： >> >> From: Chuhong Yuan >> Date: Mon, 22 Jul 2019 15:41:34 +0800 >> >> > Merge the combo uses of cpu_to_le32s and memcpy. >> > Use put_unaligned_le32 instead. >> > This simplifies the code. >> > >> > Signed-off-by: Chuhong Yuan >> >> Isn't the skb->data aligned to 4 bytes in these situations? >> >> If so, we should use the aligned variants. >> >> Thank you. > > I have checked the five changed files. > I find that they all have used get_unaligned_le32 for skb->data > according to my previous applied patches and existing code. > So I think the skb->data is unaligned in these situations. Thank you for checking. Patch applied to net-next.
Re: [PATCH v2 2/2] usbip: Implement SG support to vhci
On 7/5/19 10:43 AM, Suwan Kim wrote:
There are bugs on vhci with usb 3.0 storage device. Originally, vhci
doesn't supported SG, so USB storage driver on vhci breaks SG list
grammar - Currently vhci doesn't support?
into multiple URBs and it causes error that a transfer got terminated
too early because the transfer length for one of the URBs was not
divisible by the maxpacket size.
In this patch, vhci basically support SG and it sends each SG list
What does basically support mean here? Do you mean basic support?
entry to the stub driver. Then, the stub driver sees the total length
of the buffer and allocates SG table and pages according to the total
buffer length calling sgl_alloc(). After the stub driver receives
completed URB, it again sends each SG list entry to vhci.
If HCD of the server doesn't support SG, the stub driver breaks a
single SG reqeust into several URBs and submit them to the server's
HCD. When all the split URBs are completed, the stub driver
reassembles the URBs into a single return command and sends it to
vhci.
Signed-off-by: Suwan Kim
---
drivers/usb/usbip/stub.h | 7 +-
drivers/usb/usbip/stub_main.c| 52 +---
drivers/usb/usbip/stub_rx.c | 207 ++-
drivers/usb/usbip/stub_tx.c | 108 +++-
drivers/usb/usbip/usbip_common.c | 60 +++--
drivers/usb/usbip/vhci_hcd.c | 10 +-
drivers/usb/usbip/vhci_tx.c | 49 ++--
7 files changed, 372 insertions(+), 121 deletions(-)
btw checkpatch isn't very happy with this patch. A few coding style
issues. Please run checkpatch before sending patches.
diff --git a/drivers/usb/usbip/stub.h b/drivers/usb/usbip/stub.h
index 35618ceb2791..d11270560c24 100644
--- a/drivers/usb/usbip/stub.h
+++ b/drivers/usb/usbip/stub.h
@@ -52,7 +52,11 @@ struct stub_priv {
unsigned long seqnum;
struct list_head list;
struct stub_device *sdev;
- struct urb *urb;
+ struct urb **urbs;
+ struct scatterlist *sgl;
+ int num_urbs;
+ int completed_urbs;
+ int urb_status;
int unlinking;
};
@@ -86,6 +90,7 @@ extern struct usb_device_driver stub_driver;
struct bus_id_priv *get_busid_priv(const char *busid);
void put_busid_priv(struct bus_id_priv *bid);
int del_match_busid(char *busid);
+void stub_free_priv_and_urb(struct stub_priv *priv);
void stub_device_cleanup_urbs(struct stub_device *sdev);
/* stub_rx.c */
diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
index 2e4bfccd4bfc..dec5af9f7179 100644
--- a/drivers/usb/usbip/stub_main.c
+++ b/drivers/usb/usbip/stub_main.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include "usbip_common.h"
#include "stub.h"
@@ -288,6 +289,39 @@ static struct stub_priv
*stub_priv_pop_from_listhead(struct list_head *listhead)
return NULL;
}
+void stub_free_priv_and_urb(struct stub_priv *priv)
+{
+ struct urb *urb;
+ int i;
+
+ for (i = 0; i < priv->num_urbs; i++) {
+ urb = priv->urbs[i];
+ if (urb->setup_packet) {
+ kfree(urb->setup_packet);
+ urb->setup_packet = NULL;
+ }
+
You don't need urb->setup_packet null check. kfree() is safe
to call with null pointer. btw checkpatch tells you this.
+ if (urb->transfer_buffer && !priv->sgl) {
Is this conditional necessary? Why are you checking priv->sgl?
Are there cases where you have memory leak? Is there a case
when urb->transfer_buffer valid when priv->sgl isn't null?
+ kfree(urb->transfer_buffer);
+ urb->transfer_buffer = NULL;
+ }
+
+ if (urb->num_sgs) {
+ sgl_free(urb->sg);
+ urb->sg = NULL;
+ urb->num_sgs = 0;
+ }
+
+ usb_free_urb(urb);
+ }
+
+ list_del(&priv->list);
+ if (priv->sgl)
+ sgl_free(priv->sgl);
+ kfree(priv->urbs);
+ kmem_cache_free(stub_priv_cache, priv);
+}
+
static struct stub_priv *stub_priv_pop(struct stub_device *sdev)
{
unsigned long flags;
@@ -314,25 +348,15 @@ static struct stub_priv *stub_priv_pop(struct stub_device
*sdev)
void stub_device_cleanup_urbs(struct stub_device *sdev)
{
struct stub_priv *priv;
- struct urb *urb;
+ int i;
dev_dbg(&sdev->udev->dev, "Stub device cleaning up urbs\n");
while ((priv = stub_priv_pop(sdev))) {
- urb = priv->urb;
- dev_dbg(&sdev->udev->dev, "free urb seqnum %lu\n",
- priv->seqnum);
- usb_kill_urb(urb);
-
- kmem_cache_free(stub_priv_cache, priv);
-
- kfree(urb->transfer_buffer);
- urb->transfer_buffer = NULL;
+ for (i = 0; i < priv->num_urbs; i++)
+ usb_kill_urb(priv->urbs[i])
RE: [PATCH v10 0/6] Introduced new Cadence USBSS DRD Driver.
Hi, >On Mon 2019-07-22 13:56:44, Pavel Machek wrote: >> Hi! >> >> > > >> This patch introduce new Cadence USBSS DRD driver to linux kernel. >> > > >> >> > > >> The Cadence USBSS DRD Controller is a highly configurable IP Core >> > > >> which >> > > >> can be instantiated as Dual-Role Device (DRD), Peripheral Only and >> > > >> Host Only (XHCI)configurations. >> > > > >> > > >I see you are using debugfs to select between DRD, peripheral-onlyh and >> > > >XHCI... >> > > > >> > > >Is that good idea? >> > > >> > > Yes driver allows selecting dr_mode by debugfs. Controller also support >> > > such functionality >> > > so I don't understand why would it not be a good idea. >> > > >> > > I personally use this for testing but it can be used to limit controller >> > > functionality without >> > > recompiling kernel. >> > >> > debugfs is ONLY for debugging, never rely on it being enabled, or >> > mounted, on a system in order to have any normal operation happen. >> > >> > So for testing, yes, this is fine. If this is going to be the normal >> > api/interface for how to control this driver, no, that is not acceptable >> > at all. >> >> It makes a lot of sense for end-user to toggle this... for example >> when he is lacking right cable for proper otg detection. As it is >> third driver offering this functionality, I believe we should stop >> treating it as debugging. > >At least renesas usb controller seems to have variables in sysfs: >drivers/phy/renesas/phy-rcar-gen3-usb2.c : functions role_show and >role_store. See also >Documentation/ABI/testing/sysfs-platform-phy-rcar-gen3-usb2 . > >I believe this driver should do same. > CDNS3 driver use the role framework and also has such variable defined in role switch framework. https://elixir.bootlin.com/linux/latest/source/drivers/usb/roles/class.c Regards, Pawel
