RE: [RFC PATCH balbi-usb] usb: renesas_usbhs: usbhs_rcar3_notifier() can be static

2017-12-13 Thread Felipe Balbi

Hi,

Yoshihiro Shimoda  writes:
> Hi,
>
>> From: kbuild test robot, Sent: Thursday, December 14, 2017 2:21 AM
>> 
>> Fixes: 3a7cce26122e ("usb: renesas_usbhs: add extcon notifier to set mode 
>> for non-otg channel")
>> Signed-off-by: Fengguang Wu 
>
> Thank you for the patch!
>
> Acked-by: Yoshihiro Shimoda 

amended to patch which introduced the problem, thanks

-- 
balbi


signature.asc
Description: PGP signature


[PATCH] usb: core: add support for USB_REQ_SET_ISOCH_DELAY

2017-12-13 Thread Felipe Balbi
USB SS and SSP hubs provide wHubDelay values on their hub descriptor
which we should inform the USB Device about.

The USB Specification 3.0 explains, on section 9.4.11, how to
calculate the value and how to issue the request. Note that a
USB_REQ_SET_ISOCH_DELAY is valid on all device states (Default,
Address, Configured), we just *chose* to issue it from Address state
right after successfully fetching the USB Device Descriptor.

Signed-off-by: Felipe Balbi 
---

This has been sitting in Intel's test farm for a while now. Me and
Mathias have also booted machines ourselves and couldn't notice any
problems.

It may be a good idea to leave this in next for as long as possible to
make sure it gets as much testing as we can.

 drivers/usb/core/hub.c | 30 ++
 drivers/usb/core/message.c | 24 
 drivers/usb/core/usb.h |  1 +
 include/linux/usb.h|  6 ++
 4 files changed, 61 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cf7bbcb9a63c..ff0e614e06ff 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -38,6 +38,9 @@
 #define USB_VENDOR_GENESYS_LOGIC   0x05e3
 #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND   0x01
 
+#define USB_TP_TRANSMISSION_DELAY  40  /* ns */
+#define USB_TP_TRANSMISSION_DELAY_MAX  65535   /* ns */
+
 /* Protect struct usb_device->state and ->children members
  * Note: Both are also protected by ->dev.sem, except that ->state can
  * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
@@ -1352,6 +1355,20 @@ static int hub_configure(struct usb_hub *hub,
goto fail;
}
 
+   /*
+* Accumulate wHubDelay + 40ns for every hub in the tree of devices.
+* The resulting value will be used for SetIsochDelay() request.
+*/
+   if (hub_is_superspeed(hdev) || hub_is_superspeedplus(hdev)) {
+   u32 delay = __le16_to_cpu(hub->descriptor->u.ss.wHubDelay);
+
+   if (hdev->parent)
+   delay += hdev->parent->hub_delay;
+
+   delay += USB_TP_TRANSMISSION_DELAY;
+   hdev->hub_delay = min_t(u32, delay, 
USB_TP_TRANSMISSION_DELAY_MAX);
+   }
+
maxchild = hub->descriptor->bNbrPorts;
dev_info(hub_dev, "%d port%s detected\n", maxchild,
(maxchild == 1) ? "" : "s");
@@ -4599,7 +4616,20 @@ hub_port_init(struct usb_hub *hub, struct usb_device 
*udev, int port1,
if (retval >= 0)
retval = -EMSGSIZE;
} else {
+   u32 delay;
+
retval = 0;
+
+   delay = udev->parent->hub_delay;
+   udev->hub_delay = min_t(u32, delay,
+   USB_TP_TRANSMISSION_DELAY_MAX);
+   retval = usb_set_isoch_delay(udev);
+   if (retval) {
+   dev_dbg(>dev,
+   "Failed set isoch delay, error %d\n",
+   retval);
+   retval = 0;
+   }
break;
}
}
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 77001bcfc504..af0d708829dd 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -915,6 +915,30 @@ int usb_get_device_descriptor(struct usb_device *dev, 
unsigned int size)
return ret;
 }
 
+/*
+ * usb_set_isoch_delay - informs the device of the packet transmit delay
+ * @dev: the device whose delay is to be informed
+ * Context: !in_interrupt()
+ *
+ * Since this is an optional request, we don't bother if it fails.
+ */
+int usb_set_isoch_delay(struct usb_device *dev)
+{
+   /* skip hub devices */
+   if (dev->descriptor.bDeviceClass == USB_CLASS_HUB)
+   return 0;
+
+   /* skip non-SS/non-SSP devices */
+   if (dev->speed < USB_SPEED_SUPER)
+   return 0;
+
+   return usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+   USB_REQ_SET_ISOCH_DELAY,
+   USB_DIR_OUT | USB_TYPE_STANDARD | USB_RECIP_DEVICE,
+   cpu_to_le16(dev->hub_delay), 0, NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+}
+
 /**
  * usb_get_status - issues a GET_STATUS call
  * @dev: the device whose status is being checked
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 2bee08d084ae..149cc7480971 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -40,6 +40,7 @@ extern int usb_remove_device(struct usb_device *udev);
 
 extern int usb_get_device_descriptor(struct usb_device *dev,
unsigned int size);
+extern int usb_set_isoch_delay(struct usb_device *dev);
 extern int usb_get_bos_descriptor(struct usb_device *dev);
 extern 

Re: [BUG] usb/io_edgeport: a possible sleep-in-atomic bug in edge_bulk_in_callback

2017-12-13 Thread Jia-Ju Bai

Okay, I had submitted a patch yesterday. You can have a look :)

Thanks,
Jia-Ju Bai

On 2017/12/13 19:38, Johan Hovold wrote:

[ +CC: linux-usb]

On Wed, Dec 13, 2017 at 06:22:26PM +0800, Jia-Ju Bai wrote:

According to drivers/usb/serial/io_edgeport.c, the driver may sleep
under a spinlock.
The function call path is:
edge_bulk_in_callback (acquire the spinlock)
process_rcvd_data
  process_rcvd_status
change_port_settings
  send_iosp_ext_cmd
write_cmd_usb
  usb_kill_urb --> may sleep

I do not find a good way to fix it, so I only report.
This possible bug is found by my static analysis tool (DSAC) and checked
by my code review.

Good catch!

Fortunately, the fix here is just to remove that usb_kill_urb() from the
error path in write_cmd_usb() after usb_submit_urb() fails.

Care to submit a patch for that?

Thanks,
Johan


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC PATCH balbi-usb] usb: renesas_usbhs: usbhs_rcar3_notifier() can be static

2017-12-13 Thread Yoshihiro Shimoda
Hi,

> From: kbuild test robot, Sent: Thursday, December 14, 2017 2:21 AM
> 
> Fixes: 3a7cce26122e ("usb: renesas_usbhs: add extcon notifier to set mode for 
> non-otg channel")
> Signed-off-by: Fengguang Wu 

Thank you for the patch!

Acked-by: Yoshihiro Shimoda 

Best regards,
Yoshihiro Shimoda

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Dual-role behavior with USB-C?

2017-12-13 Thread Peter Chen
On Wed, Dec 13, 2017 at 02:37:14PM +0200, Heikki Krogerus wrote:
> Hi guys,
> 
> > > I think the USB-OTG PHY is part of i.MX6UL SoC?
> > 
> > Yes, but it is just USB PHY, not PD PHY. You need external pure CC-logic
> > chip (like PTN5150), or Type-C PD chip (like PTN5110) to support CC
> > or PD event.
> 
> Oh cool, PTP5150 seems to function autonomously if needed! So it would
> have worked even without driver support initially.
> 

Yes, if you configure PTN5150 as DRP, the ID pin for SoC will change
accordingly.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Dual-role behavior with USB-C?

2017-12-13 Thread Takashi Matsuzawa
Hello.
Now I am waiting for my USB-C breakout board to just re-using it for connecting 
to conventional micr-A/B receptacle.

BTW, Looking into schematic, another board HiKey960 with USB-C receptacle seems 
to have USB-C controller in its circuit.

>OK, from those schematics we can clearly see that the CC lines are
>really connected to the USB_OTG_ID signal. I guess the idea has been
>to attempt to get the USB_OTG_ID signal pulled down to ground when the
>partner is UFP (device) and has the CC line connected to the ground
>with the resistor Rd. That should put the board into host mode, but
>I guess it's not working.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] phy: rockchip-typec: Try to turn the PHY on several times

2017-12-13 Thread Doug Anderson
Hi,

On Wed, Dec 13, 2017 at 4:41 AM, Enric Balletbo Serra
 wrote:
> Hi Doug,
>
> 2017-12-11 22:45 GMT+01:00 Douglas Anderson :
>> Bind / unbind stress testing of the USB controller on rk3399 found
>> that we'd often end up with lots of failures that looked like this:
>>
>>   phy phy-ff80.phy.9: phy poweron failed --> -110
>>   dwc3 fe90.dwc3: failed to initialize core
>>   dwc3: probe of fe90.dwc3 failed with error -110
>>
>> Those errors were sometimes seen at bootup too, in which case USB
>> peripherals wouldn't work until unplugged and re-plugged in.
>>
>> I spent some time trying to figure out why the PHY was failing to
>> power on but I wasn't able to.  Possibly this has to do with the fact
>> that the PHY docs say that the USB controller "needs to be held in
>> reset to hold pipe power state in P2 before initializing the Type C
>> PHY" but that doesn't appear to be easy to do with the dwc3 driver
>> today.  Messing around with the ordering of the reset vs. the PHY
>> initialization in the dwc3 driver didn't seem to fix things.
>>
>> I did, however, find that if I simply retry the power on it seems to
>> have a good chance of working.  So let's add some retries.  I ran a
>> pretty tight bind/unbind loop overnight.  When I did so, I found that
>> I need to retry between 1% and 2% of the time.  Overnight I found only
>> a small handful of times where I needed 2 retries.  I never found a
>> case where I needed 3 retries.
>>
>> I'm completely aware of the fact that this is quite an ugly hack and I
>> wish I didn't have to resort to it, but I have no other real idea how
>> to make this hardware reliable.  If Rockchip in the future can come up
>> with a solution we can always revert this hack.  Until then, let's at
>> least have something that works.
>>
>> This patch is tested atop Enric's latest dwc3 patch series ending at:
>>   https://patchwork.kernel.org/patch/10095527/
>> ...but it could be applied independently of that series without any
>> bad effects.
>>
>> For some more details on this bug, you can refer to:
>>   https://bugs.chromium.org/p/chromium/issues/detail?id=783464
>>
>> Signed-off-by: Douglas Anderson 
>> ---
>>
>>  drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c 
>> b/drivers/phy/rockchip/phy-rockchip-typec.c
>> index ee85fa0ca4b0..5c2157156ce1 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
>> @@ -349,6 +349,8 @@
>>  #define MODE_DFP_USB   BIT(1)
>>  #define MODE_DFP_DPBIT(2)
>>
>> +#define POWER_ON_TRIES 5
>> +
>
> I did the test of increase the number of tries to 100 because
> unfortunately, even with this patch applied, I can see the problem on
> my kevin with current mainline.
>
> [  244.309094] rockchip-typec-phy ff80.phy: Turn on failed after 100 loops
>
> That's an extra debug print ^
>
> [  244.317019] phy phy-ff80.phy.8: phy poweron failed --> -110
> [  244.323824] dwc3 fe90.dwc3: failed to initialize core
> [  244.330057] dwc3: probe of fe90.dwc3 failed with error -110
>
> So I'm wondering if there is something else that I need to apply to
> really fix this as you didn't reproduce the issue doing lots of tests
> and I can reproduce the issue very easily.

Ah!  I added that message to the top of my upstream series and,
indeed, I sometimes see the PHY fail to turn on.  Doh.

OK, so here's what I've done:

* The place where I ran the overnight loops was actually the Chrome OS
4.4 kernel.  In that kernel I had a message very similar to yours and
I didn't hit it.  I just re-ran this for 20 minutes now and I can
re-confirm.  In the Chrome OS kernel I never see it needing more than
a 1 (or 2) loops and it doesn't ever get into the "totally failed"
case.

* Previously I ran ~10 minutes with the upstream kernel, but at the
time I didn't have your printout.  After 10 minutes I checked my logs
and I definitely saw the "Needed 1 loops to turn on", so I knew my
patch was doing something useful.  It didn't occur to me to re-confirm
that I didn't get the "totally failed" upstream, though now that I say
it out loud it's stupid that I didn't think to do this.

* Previously when playing with patches on the upstream kernel I saw
lots of problems powering on the PHY and I thought my patch was
helping, but that was all very non-scientific.


So to say it shortly:

* For me, my patch makes things a slightly better even on the upstream
kernel (I do sometimes see the "turned on after 1 tries")

* I can confirm that my patch doesn't fix everything upstream, so
there's something different about the Chrome OS tree still.

---

I also picked all the local patches from the Chrome OS kernel to the
PHY driver and now my PHY driver in the upstream and downstream trees

Re: [PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091

2017-12-13 Thread David Miller
From: ssjoh...@mac.com
Date: Mon, 11 Dec 2017 21:51:14 +0100

> From: Sebastian Sjoholm 
> 
> Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
> The USB id is added to qmi_wwan.c to allow QMI communication 
> with the EM7565.
> 
> Signed-off-by: Sebastian Sjoholm 
> Acked-by: Bjørn Mork 

Applied and queued up for -stable.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usbip: fix usbip bind writing random string after command in match_busid

2017-12-13 Thread Shuah Khan
On 12/13/2017 04:07 AM, Juan Zea wrote:
> usbip bind writes commands followed by random string when writing to
> match_busid attribute in sysfs, caused by using full variable size
> instead of string length.
> 
> Signed-off-by: Juan Zea 
> ---
>  tools/usb/usbip/src/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/usb/usbip/src/utils.c b/tools/usb/usbip/src/utils.c
> index 2b3d6d2..ea1a1af 100644
> --- a/tools/usb/usbip/src/utils.c
> +++ b/tools/usb/usbip/src/utils.c
> @@ -42,7 +42,7 @@ int modify_match_busid(char *busid, int add)
> snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
>  
> rc = write_sysfs_attribute(match_busid_attr_path, command,
> -  sizeof(command));
> +  strlen(command));
> if (rc < 0) {
> dbg("failed to write match_busid: %s", strerror(errno));
> return -1;
> 

Why not use the return value from snprintf() for length, instead of calling
strlen(command)?

thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[balbi-usb:testing/next 25/25] drivers/usb/renesas_usbhs/rcar3.c:115:5: sparse: symbol 'usbhs_rcar3_notifier' was not declared. Should it be static?

2017-12-13 Thread kbuild test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
testing/next
head:   3a7cce26122e3925993ee2ac0b45ac6b9bf6f65f
commit: 3a7cce26122e3925993ee2ac0b45ac6b9bf6f65f [25/25] usb: renesas_usbhs: 
add extcon notifier to set mode for non-otg channel
reproduce:
# apt-get install sparse
git checkout 3a7cce26122e3925993ee2ac0b45ac6b9bf6f65f
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)


Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH balbi-usb] usb: renesas_usbhs: usbhs_rcar3_notifier() can be static

2017-12-13 Thread kbuild test robot

Fixes: 3a7cce26122e ("usb: renesas_usbhs: add extcon notifier to set mode for 
non-otg channel")
Signed-off-by: Fengguang Wu 
---
 rcar3.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/rcar3.c 
b/drivers/usb/renesas_usbhs/rcar3.c
index d657309..d0ea4ff 100644
--- a/drivers/usb/renesas_usbhs/rcar3.c
+++ b/drivers/usb/renesas_usbhs/rcar3.c
@@ -112,8 +112,8 @@ static int usbhs_rcar3_get_id(struct platform_device *pdev)
return USBHS_GADGET;
 }
 
-int usbhs_rcar3_notifier(struct notifier_block *nb, unsigned long event,
-void *data)
+static int usbhs_rcar3_notifier(struct notifier_block *nb, unsigned long event,
+   void *data)
 {
struct usbhs_priv *priv = container_of(nb, struct usbhs_priv, nb);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Mikhail Zaytsev
On Wed, 13 Dec 2017 15:39:21 +0100 Oliver Neukum  wrote:

> But:
> 
> device->maxbaudrate = 38400
> 
> is better than
> 
> device->maxbaudrate = MAX_BAUD
> 
> You see the point?

Yes, I see. This is better,  because it's more important to know =, 
but not =.

thank you Oliver.

---
Mikhail
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: core: prevent malicious bNumInterfaces overflow

2017-12-13 Thread Alan Stern
On Wed, 13 Dec 2017, Greg KH wrote:

> > > --- usb-4.x.orig/drivers/usb/core/config.c
> > > +++ usb-4.x/drivers/usb/core/config.c
> > > @@ -555,6 +555,9 @@ static int usb_parse_configuration(struc
> > >   unsigned iad_num = 0;
> > >  
> > >   memcpy(>desc, buffer, USB_DT_CONFIG_SIZE);
> > > + nintf = nintf_orig = config->desc.bNumInterfaces;
> > > + config->desc.bNumInterfaces = 0;// Adjusted later
> > > +
> > 
> > The comment format?
> 
> Is fine, I've given up that fight :)

In fact, Linus posted an email sometime in the last few weeks, in which 
he said that he had changed his mind about // comments.  He called it 
the one thing that C++ got right!

Also, checkpatch didn't complain.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Oliver Neukum
Am Mittwoch, den 13.12.2017, 15:30 +0300 schrieb Mikhail Zaytsev:
> On Wed, 13 Dec 2017 12:40:48 +0100 Oliver Neukum  wrote:
> 
> > 
> > They give you nothing. If you are looking at a vendor ID nothing but the
> > bare number makes sense. You are just making peoples' life harder when
> > they have to look up that definition. A symbolic name is fine if it gives
> > meaning. Even if the information you give is that the value is magic
> > and therefore not understood. But a vendor ID is an arbitrary yet
> > meaningful number. There is no point in hiding it.
> 
> Thanks. I hear you, Oliver. What about:
> 
> - serstruct.baud_base = 460800;
> 
> Is it a magic number? I think yes.
> 

Hi,

yes sure. That is a candidate for a symbolic name. Though if you use
it once, I see no benefit, but it does not hurt either. The member
is named and that is the important thing.

A line like

if (rate > 38400) return -EINVAL;

is not so good

if (rate > MAX_BAUD) return -EINVAL;

better

But:

device->maxbaudrate = 38400

is better than

device->maxbaudrate = MAX_BAUD

You see the point?

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.

2017-12-13 Thread Mikhail Zaytsev
The patch removes unused TIOCSSERIAL ioctl case and adds the default block 
to the switch.

Signed-off-by: Mikhail Zaytsev 
---
 drivers/usb/serial/ark3116.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..2e957c76f 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -418,10 +418,8 @@ static int ark3116_ioctl(struct tty_struct *tty,
return -EFAULT;
 
return 0;
-   case TIOCSSERIAL:
-   if (copy_from_user(, user_arg, sizeof(serstruct)))
-   return -EFAULT;
-   return 0;
+   default:
+   break;
}
 
return -ENOIOCTLCMD;
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] USB: serial: ark3116.c: ioctl changes

2017-12-13 Thread Mikhail Zaytsev
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
moves to the get_serial_info() function. Some magic numbers moves to
 #define directives.  

Mikhail Zaytsev (2):
  USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
  USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.

 drivers/usb/serial/ark3116.c | 41 +++--
 1 file changed, 23 insertions(+), 18 deletions(-)

-- 
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.

2017-12-13 Thread Mikhail Zaytsev
The patch moves TIOCGSERIAL ioctl case to get_serial_info function.

Signed-off-by: Mikhail Zaytsev 
---
 drivers/usb/serial/ark3116.c | 35 +--
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 2e957c76f..2ce8fe10e 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,6 +36,7 @@
 #define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
 #define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
 #define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
 
 /* usb timeout of 1 second */
 #define ARK_TIMEOUT 1000
@@ -397,27 +398,33 @@ static int ark3116_open(struct tty_struct *tty, struct 
usb_serial_port *port)
return result;
 }
 
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+   unsigned long arg)
+{
+   struct serial_struct ser;
+
+   memset(, 0, sizeof(ser));
+
+   ser.type = PORT_16654;
+   ser.line = port->minor;
+   ser.port = port->port_number;
+   ser.custom_divisor = 0;
+   ser.baud_base = ARK3116_BAUDRATE;
+
+   if (copy_to_user((void __user *)arg, , sizeof(ser)))
+   return -EFAULT;
+
+   return 0;
+}
+
 static int ark3116_ioctl(struct tty_struct *tty,
 unsigned int cmd, unsigned long arg)
 {
struct usb_serial_port *port = tty->driver_data;
-   struct serial_struct serstruct;
-   void __user *user_arg = (void __user *)arg;
 
switch (cmd) {
case TIOCGSERIAL:
-   /* XXX: Some of these values are probably wrong. */
-   memset(, 0, sizeof(serstruct));
-   serstruct.type = PORT_16654;
-   serstruct.line = port->minor;
-   serstruct.port = port->port_number;
-   serstruct.custom_divisor = 0;
-   serstruct.baud_base = 460800;
-
-   if (copy_to_user(user_arg, , sizeof(serstruct)))
-   return -EFAULT;
-
-   return 0;
+   return ark3116_get_serial_info(port, arg);
default:
break;
}
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] phy: rockchip-typec: Try to turn the PHY on several times

2017-12-13 Thread Enric Balletbo Serra
Hi Doug,

2017-12-11 22:45 GMT+01:00 Douglas Anderson :
> Bind / unbind stress testing of the USB controller on rk3399 found
> that we'd often end up with lots of failures that looked like this:
>
>   phy phy-ff80.phy.9: phy poweron failed --> -110
>   dwc3 fe90.dwc3: failed to initialize core
>   dwc3: probe of fe90.dwc3 failed with error -110
>
> Those errors were sometimes seen at bootup too, in which case USB
> peripherals wouldn't work until unplugged and re-plugged in.
>
> I spent some time trying to figure out why the PHY was failing to
> power on but I wasn't able to.  Possibly this has to do with the fact
> that the PHY docs say that the USB controller "needs to be held in
> reset to hold pipe power state in P2 before initializing the Type C
> PHY" but that doesn't appear to be easy to do with the dwc3 driver
> today.  Messing around with the ordering of the reset vs. the PHY
> initialization in the dwc3 driver didn't seem to fix things.
>
> I did, however, find that if I simply retry the power on it seems to
> have a good chance of working.  So let's add some retries.  I ran a
> pretty tight bind/unbind loop overnight.  When I did so, I found that
> I need to retry between 1% and 2% of the time.  Overnight I found only
> a small handful of times where I needed 2 retries.  I never found a
> case where I needed 3 retries.
>
> I'm completely aware of the fact that this is quite an ugly hack and I
> wish I didn't have to resort to it, but I have no other real idea how
> to make this hardware reliable.  If Rockchip in the future can come up
> with a solution we can always revert this hack.  Until then, let's at
> least have something that works.
>
> This patch is tested atop Enric's latest dwc3 patch series ending at:
>   https://patchwork.kernel.org/patch/10095527/
> ...but it could be applied independently of that series without any
> bad effects.
>
> For some more details on this bug, you can refer to:
>   https://bugs.chromium.org/p/chromium/issues/detail?id=783464
>
> Signed-off-by: Douglas Anderson 
> ---
>
>  drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c 
> b/drivers/phy/rockchip/phy-rockchip-typec.c
> index ee85fa0ca4b0..5c2157156ce1 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -349,6 +349,8 @@
>  #define MODE_DFP_USB   BIT(1)
>  #define MODE_DFP_DPBIT(2)
>
> +#define POWER_ON_TRIES 5
> +

I did the test of increase the number of tries to 100 because
unfortunately, even with this patch applied, I can see the problem on
my kevin with current mainline.

[  244.309094] rockchip-typec-phy ff80.phy: Turn on failed after 100 loops

That's an extra debug print ^

[  244.317019] phy phy-ff80.phy.8: phy poweron failed --> -110
[  244.323824] dwc3 fe90.dwc3: failed to initialize core
[  244.330057] dwc3: probe of fe90.dwc3 failed with error -110

So I'm wondering if there is something else that I need to apply to
really fix this as you didn't reproduce the issue doing lots of tests
and I can reproduce the issue very easily.

>  struct usb3phy_reg {
> u32 offset;
> u32 enable_bit;
> @@ -818,9 +820,8 @@ static int tcphy_get_mode(struct rockchip_typec_phy 
> *tcphy)
> return mode;
>  }
>
> -static int rockchip_usb3_phy_power_on(struct phy *phy)
> +static int _rockchip_usb3_phy_power_on(struct rockchip_typec_phy *tcphy)
>  {
> -   struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> struct rockchip_usb3phy_port_cfg *cfg = >port_cfgs;
> const struct usb3phy_reg *reg = >pipe_status;
> int timeout, new_mode, ret = 0;
> @@ -867,6 +868,25 @@ static int rockchip_usb3_phy_power_on(struct phy *phy)
> return ret;
>  }
>
> +static int rockchip_usb3_phy_power_on(struct phy *phy)
> +{
> +   struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> +   int ret;
> +   int tries;
> +
> +   for (tries = 0; tries < POWER_ON_TRIES; tries++) {
> +   ret = _rockchip_usb3_phy_power_on(tcphy);
> +   if (!ret)
> +   break;
> +   }
> +
> +   if (tries && !ret)
> +   dev_info(tcphy->dev, "Needed %d loops to turn on\n", tries);
> +

It's curious that in my case I never see this message, or it works or
it fails after 100 retries. I'll do more longer tests and continue
investigating a little bit.

Regards,
 Enric
> +   return ret;
> +}
> +
> +
>  static int rockchip_usb3_phy_power_off(struct phy *phy)
>  {
> struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> --
> 2.15.1.424.g9478a66081-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at 

Re: Dual-role behavior with USB-C?

2017-12-13 Thread Heikki Krogerus
Hi guys,

On Wed, Dec 13, 2017 at 10:43:50AM +0800, Peter Chen wrote:
> On Wed, Dec 13, 2017 at 01:17:23AM +, Takashi Matsuzawa wrote:
> > Hello.
> > 
> > >If you have a Type-C connector on your board, then you also should
> > >have a USB Type-C PHY that takes care of CC logic. The host-to-device
> > >relationship is determined using the Configuration Channel (CC) that
> > >goes through the USB Type-C cable. Note that CC is not the same as ID!
> > 
> > I am playing with i.MX6 pico board, which has USB-OTG receptacle with USB-C 
> > physical shape. 
> > http://download.wandboard.org/hobbitboard-imx6ul/documentation/pico-imx6ul-emmc-hobbit-reva1-hardware-manual-20160328.pdf

OK, from those schematics we can clearly see that the CC lines are
really connected to the USB_OTG_ID signal. I guess the idea has been
to attempt to get the USB_OTG_ID signal pulled down to ground when the
partner is UFP (device) and has the CC line connected to the ground
with the resistor Rd. That should put the board into host mode, but
I guess it's not working.

With Type-C to Type-C connections, in case the partner is DFP (host)
or DRP (dual-role), that configuration will just confuse the partner.
The partner will now see that the CC is pulled high and probable think
that your board is also DFP (host), i.e. your board and the partner
connected to it, both think they need to be in device mode.

I don't think there are actually any guarantees that connecting the CC
lines to the ID input on the board will work in any scenario. There
really should be a CC-logic chip if dual-role support was needed. For
device mode only, simply connecting the CC lines to the ground from
the connector would have been enough.

> > I think the USB-OTG PHY is part of i.MX6UL SoC?
> 
> Yes, but it is just USB PHY, not PD PHY. You need external pure CC-logic
> chip (like PTN5150), or Type-C PD chip (like PTN5110) to support CC
> or PD event.

Oh cool, PTP5150 seems to function autonomously if needed! So it would
have worked even without driver support initially.


Cheers,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] usb/io_edgeport: Fix a possible sleep-in-atomic bug in edge_bulk_in_callback

2017-12-13 Thread Jia-Ju Bai
According to drivers/usb/serial/io_edgeport.c, the driver may sleep 
under a spinlock.
The function call path is:
edge_bulk_in_callback (acquire the spinlock)
   process_rcvd_data
 process_rcvd_status
   change_port_settings
 send_iosp_ext_cmd
   write_cmd_usb
 usb_kill_urb --> may sleep

To fix it, usb_kill_urb() is removed from the error path after usb_submit_urb() 
fails.

This possible bug is found by my static analysis tool (DSAC) and checked by my 
code review.

Signed-off-by: Jia-Ju Bai 
---
 drivers/usb/serial/io_edgeport.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index 219265c..17283f4 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -2282,7 +2282,6 @@ static int write_cmd_usb(struct edgeport_port *edge_port,
/* something went wrong */
dev_err(dev, "%s - usb_submit_urb(write command) failed, status 
= %d\n",
__func__, status);
-   usb_kill_urb(urb);
usb_free_urb(urb);
atomic_dec();
return status;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Mikhail Zaytsev
On Wed, 13 Dec 2017 12:40:48 +0100 Oliver Neukum  wrote:

> They give you nothing. If you are looking at a vendor ID nothing but the
> bare number makes sense. You are just making peoples' life harder when
> they have to look up that definition. A symbolic name is fine if it gives
> meaning. Even if the information you give is that the value is magic
> and therefore not understood. But a vendor ID is an arbitrary yet
> meaningful number. There is no point in hiding it.

Thanks. I hear you, Oliver. What about:

-   serstruct.baud_base = 460800;

Is it a magic number? I think yes.

-- 
Mikhail
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] Staging: rtl8723bs: Replace true with x and false with !x

2017-12-13 Thread Greg KH
On Wed, Dec 13, 2017 at 05:25:42PM +0530, Shreeya Patel wrote:
> Replace true and false keywords with "x" and "!x"
> respectively to follow the kernel coding style.
> 
> Signed-off-by: Shreeya Patel 
> ---
>  drivers/staging/rtl8723bs/hal/sdio_ops.c | 30 ++
>  1 file changed, 14 insertions(+), 16 deletions(-)

$ ./scripts/get_maintainer.pl --file drivers/staging/rtl8723bs/hal/sdio_ops.c
Greg Kroah-Hartman  (supporter:STAGING 
SUBSYSTEM,commit_signer:4/4=100%)
Larry Finger  (commit_signer:1/4=25%)
Aishwarya Pant  
(commit_signer:1/4=25%,authored:1/4=25%,removed_lines:4/10=40%)
Himanshu Jha  
(commit_signer:1/4=25%,authored:1/4=25%,removed_lines:1/10=10%)
Hans de Goede  
(commit_signer:1/4=25%,authored:1/4=25%,added_lines:1296/1304=99%)
Joe Perches  (authored:1/4=25%,removed_lines:5/10=50%)
de...@driverdev.osuosl.org (open list:STAGING SUBSYSTEM)
linux-ker...@vger.kernel.org (open list)

You got the wrong list, and wrong person, and didn't send all 4 patches :(

Please fix up and try again.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] Staging: rtl8723bs: Change names to conform to the kernel code

2017-12-13 Thread Shreeya Patel
Change names of some variables and functions to conform
to the kernel coding style.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/rtl8723bs/hal/sdio_ops.c | 186 +++
 1 file changed, 93 insertions(+), 93 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c 
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index aa52c31..e72f80f 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -29,34 +29,34 @@
 /*  */
 static void HalSdioGetCmdAddr8723BSdio(
struct adapter *padapter,
-   u8 DeviceID,
-   u32 Addr,
-   u32 *pCmdAddr
+   u8 device_id,
+   u32 addr,
+   u32 *pcmdaddr
 )
 {
-   switch (DeviceID) {
+   switch (device_id) {
case SDIO_LOCAL_DEVICE_ID:
-   *pCmdAddr = ((SDIO_LOCAL_DEVICE_ID << 13) | (Addr & 
SDIO_LOCAL_MSK));
+   *pcmdaddr = ((SDIO_LOCAL_DEVICE_ID << 13) | (addr & 
SDIO_LOCAL_MSK));
break;
 
case WLAN_IOREG_DEVICE_ID:
-   *pCmdAddr = ((WLAN_IOREG_DEVICE_ID << 13) | (Addr & 
WLAN_IOREG_MSK));
+   *pcmdaddr = ((WLAN_IOREG_DEVICE_ID << 13) | (addr & 
WLAN_IOREG_MSK));
break;
 
case WLAN_TX_HIQ_DEVICE_ID:
-   *pCmdAddr = ((WLAN_TX_HIQ_DEVICE_ID << 13) | (Addr & 
WLAN_FIFO_MSK));
+   *pcmdaddr = ((WLAN_TX_HIQ_DEVICE_ID << 13) | (addr & 
WLAN_FIFO_MSK));
break;
 
case WLAN_TX_MIQ_DEVICE_ID:
-   *pCmdAddr = ((WLAN_TX_MIQ_DEVICE_ID << 13) | (Addr & 
WLAN_FIFO_MSK));
+   *pcmdaddr = ((WLAN_TX_MIQ_DEVICE_ID << 13) | (addr & 
WLAN_FIFO_MSK));
break;
 
case WLAN_TX_LOQ_DEVICE_ID:
-   *pCmdAddr = ((WLAN_TX_LOQ_DEVICE_ID << 13) | (Addr & 
WLAN_FIFO_MSK));
+   *pcmdaddr = ((WLAN_TX_LOQ_DEVICE_ID << 13) | (addr & 
WLAN_FIFO_MSK));
break;
 
case WLAN_RX0FF_DEVICE_ID:
-   *pCmdAddr = ((WLAN_RX0FF_DEVICE_ID << 13) | (Addr & 
WLAN_RX0FF_MSK));
+   *pcmdaddr = ((WLAN_RX0FF_DEVICE_ID << 13) | (addr & 
WLAN_RX0FF_MSK));
break;
 
default:
@@ -66,64 +66,64 @@ static void HalSdioGetCmdAddr8723BSdio(
 
 static u8 get_deviceid(u32 addr)
 {
-   u8 devideId;
-   u16 pseudoId;
+   u8 devide_id;
+   u16 pseudo_id;
 
 
-   pseudoId = (u16)(addr >> 16);
-   switch (pseudoId) {
+   pseudo_id = (u16)(addr >> 16);
+   switch (pseudo_id) {
case 0x1025:
-   devideId = SDIO_LOCAL_DEVICE_ID;
+   devide_id = SDIO_LOCAL_DEVICE_ID;
break;
 
case 0x1026:
-   devideId = WLAN_IOREG_DEVICE_ID;
+   devide_id = WLAN_IOREG_DEVICE_ID;
break;
 
 /* case 0x1027: */
-/* devideId = SDIO_FIRMWARE_FIFO; */
+/* devide_id = SDIO_FIRMWARE_FIFO; */
 /* break; */
 
case 0x1031:
-   devideId = WLAN_TX_HIQ_DEVICE_ID;
+   devide_id = WLAN_TX_HIQ_DEVICE_ID;
break;
 
case 0x1032:
-   devideId = WLAN_TX_MIQ_DEVICE_ID;
+   devide_id = WLAN_TX_MIQ_DEVICE_ID;
break;
 
case 0x1033:
-   devideId = WLAN_TX_LOQ_DEVICE_ID;
+   devide_id = WLAN_TX_LOQ_DEVICE_ID;
break;
 
case 0x1034:
-   devideId = WLAN_RX0FF_DEVICE_ID;
+   devide_id = WLAN_RX0FF_DEVICE_ID;
break;
 
default:
-/* devideId = (u8)((addr >> 13) & 0xF); */
-   devideId = WLAN_IOREG_DEVICE_ID;
+/* devide_id = (u8)((addr >> 13) & 0xF); */
+   devide_id = WLAN_IOREG_DEVICE_ID;
break;
}
 
-   return devideId;
+   return devide_id;
 }
 
 /*
  * Ref:
  *HalSdioGetCmdAddr8723BSdio()
  */
-static u32 _cvrt2ftaddr(const u32 addr, u8 *pdeviceId, u16 *poffset)
+static u32 _cvrt2ftaddr(const u32 addr, u8 *pdevice_id, u16 *poffset)
 {
-   u8 deviceId;
+   u8 device_id;
u16 offset;
u32 ftaddr;
 
 
-   deviceId = get_deviceid(addr);
+   device_id = get_deviceid(addr);
offset = 0;
 
-   switch (deviceId) {
+   switch (device_id) {
case SDIO_LOCAL_DEVICE_ID:
offset = addr & SDIO_LOCAL_MSK;
break;
@@ -140,14 +140,14 @@ static u32 _cvrt2ftaddr(const u32 addr, u8 *pdeviceId, 
u16 *poffset)
 
case WLAN_IOREG_DEVICE_ID:
default:
-   deviceId = WLAN_IOREG_DEVICE_ID;
+   device_id = WLAN_IOREG_DEVICE_ID;
offset = addr & WLAN_IOREG_MSK;
break;
}
-   ftaddr = (deviceId << 13) | offset;
+   ftaddr = (device_id << 13) | offset;
 
-   if (pdeviceId)
-   *pdeviceId = deviceId;
+   if (pdevice_id)
+

[PATCH 4/4] Staging: rtl8723bs: Use !x instead of NULL comparison

2017-12-13 Thread Shreeya Patel
If "x" is compared to NULL, use "!x" instead of it, so as
to follow the kernel coding style.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/rtl8723bs/hal/sdio_ops.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c 
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index 67c4cca..d6a4572 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -215,7 +215,7 @@ static u32 sdio_read32(struct intf_hdl *pintfhdl, u32 addr)
u8 *ptmpbuf;
 
ptmpbuf = rtw_malloc(8);
-   if (NULL == ptmpbuf) {
+   if (!ptmpbuf) {
DBG_8192C(KERN_ERR "%s: Allocate memory FAIL!(size =8) 
addr = 0x%x\n", __func__, addr);
return SDIO_ERR_VAL32;
}
@@ -264,7 +264,7 @@ static s32 sdio_readN(struct intf_hdl *pintfhdl, u32 addr, 
u32 cnt, u8 *pbuf)
ftaddr &= ~(u16)0x3;
n = cnt + shift;
ptmpbuf = rtw_malloc(n);
-   if (NULL == ptmpbuf)
+   if (!ptmpbuf)
return -1;
 
err = sd_read(pintfhdl, ftaddr, n, ptmpbuf);
@@ -367,7 +367,7 @@ static s32 sdio_writeN(struct intf_hdl *pintfhdl, u32 addr, 
u32 cnt, u8 *pbuf)
ftaddr &= ~(u16)0x3;
n = cnt + shift;
ptmpbuf = rtw_malloc(n);
-   if (NULL == ptmpbuf)
+   if (!ptmpbuf)
return -1;
err = sd_read(pintfhdl, ftaddr, 4, ptmpbuf);
if (err) {
@@ -457,7 +457,7 @@ static u32 sdio_read_port(
 #ifdef SDIO_DYNAMIC_ALLOC_MEM
oldmem = mem;
mem = rtw_malloc(cnt);
-   if (mem == NULL) {
+   if (!mem) {
DBG_8192C(KERN_WARNING "%s: allocate memory %d bytes 
fail!\n", __func__, cnt);
mem = oldmem;
oldmem = NULL;
@@ -745,7 +745,7 @@ static s32 ReadInterrupt8723BSdio(struct adapter *padapter, 
u32 *phisr)
u8 val8, hisr_len;
 
 
-   if (phisr == NULL)
+   if (!phisr)
return false;
 
himr = GET_HAL_DATA(padapter)->sdio_himr;
@@ -969,13 +969,13 @@ static struct recv_buf *sd_recv_rxfifo(struct adapter 
*padapter, u32 size)
/* 3 1. alloc recvbuf */
precvpriv = >recvpriv;
precvbuf = rtw_dequeue_recvbuf(>free_recv_buf_queue);
-   if (precvbuf == NULL) {
+   if (!precvbuf) {
DBG_871X_LEVEL(_drv_err_, "%s: alloc recvbuf FAIL!\n", 
__func__);
return NULL;
}
 
/* 3 2. alloc skb */
-   if (precvbuf->pskb == NULL) {
+   if (!precvbuf->pskb) {
SIZE_PTR tmpaddr = 0;
SIZE_PTR alignment = 0;
 
@@ -989,7 +989,7 @@ static struct recv_buf *sd_recv_rxfifo(struct adapter 
*padapter, u32 size)
skb_reserve(precvbuf->pskb, (RECVBUFF_ALIGN_SZ - 
alignment));
}
 
-   if (precvbuf->pskb == NULL) {
+   if (!precvbuf->pskb) {
DBG_871X("%s: alloc_skb fail! read =%d\n", __func__, 
readsize);
return NULL;
}
@@ -1247,7 +1247,7 @@ u8 RecvOnePkt(struct adapter *padapter, u32 size)
 
DBG_871X("+%s: size: %d+\n", __func__, size);
 
-   if (padapter == NULL) {
+   if (!padapter) {
DBG_871X(KERN_ERR "%s: padapter is NULL!\n", __func__);
return false;
}
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] Staging: rtl8723bs: Change condition to assignment

2017-12-13 Thread Shreeya Patel
Change the conditional operator to assignment as it is
not a conditional statement.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/rtl8723bs/hal/sdio_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c 
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index e72f80f..67c4cca 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -460,7 +460,7 @@ static u32 sdio_read_port(
if (mem == NULL) {
DBG_8192C(KERN_WARNING "%s: allocate memory %d bytes 
fail!\n", __func__, cnt);
mem = oldmem;
-   oldmem == NULL;
+   oldmem = NULL;
}
 #else
/*  in this case, caller should gurante the buffer is big 
enough */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] Staging: rtl8723bs: Replace true with x and false with !x

2017-12-13 Thread Shreeya Patel
Replace true and false keywords with "x" and "!x"
respectively to follow the kernel coding style.

Signed-off-by: Shreeya Patel 
---
 drivers/staging/rtl8723bs/hal/sdio_ops.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c 
b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index 93ac083..aa52c31 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -191,8 +191,8 @@ static u32 sdio_read32(struct intf_hdl *pintfhdl, u32 addr)
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
if (
((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
-   (false == bMacPwrCtrlOn) ||
-   (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+   (!bMacPwrCtrlOn) ||
+   (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
) {
err = sd_cmd52_read(pintfhdl, ftaddr, 4, (u8 *)_tmp);
 #ifdef SDIO_DEBUG_IO
@@ -248,8 +248,8 @@ static s32 sdio_readN(struct intf_hdl *pintfhdl, u32 addr, 
u32 cnt, u8 *pbuf)
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
if (
((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
-   (false == bMacPwrCtrlOn) ||
-   (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+   (!bMacPwrCtrlOn) ||
+   (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_read(pintfhdl, ftaddr, cnt, pbuf);
 
@@ -352,8 +352,8 @@ static s32 sdio_writeN(struct intf_hdl *pintfhdl, u32 addr, 
u32 cnt, u8 *pbuf)
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
if (
((deviceId == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
-   (false == bMacPwrCtrlOn) ||
-   (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+   (!bMacPwrCtrlOn) ||
+   (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_write(pintfhdl, ftaddr, cnt, pbuf);
 
@@ -513,7 +513,7 @@ static u32 sdio_write_port(
padapter = pintfhdl->padapter;
psdio = _to_dvobj(padapter)->intf_data;
 
-   if (padapter->hw_init_completed == false) {
+   if (!padapter->hw_init_completed) {
DBG_871X("%s [addr = 0x%x cnt =%d] padapter->hw_init_completed 
== false\n", __func__, addr, cnt);
return _FAIL;
}
@@ -577,7 +577,7 @@ static s32 _sdio_local_read(
HalSdioGetCmdAddr8723BSdio(padapter, SDIO_LOCAL_DEVICE_ID, addr, );
 
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
-   if (false == bMacPwrCtrlOn)
+   if (!bMacPwrCtrlOn)
return _sd_cmd52_read(pintfhdl, addr, cnt, pbuf);
 
n = RND4(cnt);
@@ -616,8 +616,8 @@ s32 sdio_local_read(
 
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
if (
-   (false == bMacPwrCtrlOn) ||
-   (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+   (!bMacPwrCtrlOn) ||
+   (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_read(pintfhdl, addr, cnt, pbuf);
 
@@ -662,8 +662,8 @@ s32 sdio_local_write(
 
rtw_hal_get_hwreg(padapter, HW_VAR_APFM_ON_MAC, );
if (
-   (false == bMacPwrCtrlOn) ||
-   (true == adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
+   (!bMacPwrCtrlOn) ||
+   (adapter_to_pwrctl(padapter)->bFwCurrentInPSMode)
)
return sd_cmd52_write(pintfhdl, addr, cnt, pbuf);
 
@@ -843,8 +843,7 @@ void ClearInterrupt8723BSdio(struct adapter *padapter)
struct hal_com_data *pHalData;
u8 *clear;
 
-
-   if (true == padapter->bSurpriseRemoved)
+   if (padapter->bSurpriseRemoved)
return;
 
pHalData = GET_HAL_DATA(padapter);
@@ -1161,8 +1160,7 @@ void sd_int_hdl(struct adapter *padapter)
 
 
if (
-   (padapter->bDriverStopped == true) ||
-   (padapter->bSurpriseRemoved == true)
+   (padapter->bDriverStopped) || (padapter->bSurpriseRemoved)
)
return;
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] Remove checkpatch warnings

2017-12-13 Thread Shreeya Patel
This patchset removes some warnings generated by checkpatch
for cleanup of the rtl8723bs driver.
Also some additional cleanups are introduced in the
*[1/4] and *[3/4] patches to make the code according to the
kernel coding style.


Shreeya Patel (4):
  Staging: rtl8723bs: Replace true with x and false with !x
  Staging: rtl8723bs: Change names to conform to the kernel code
  Staging: rtl8723bs: Change condition to assignment
  Staging: rtl8723bs: Use !x instead of NULL comparison

 drivers/staging/rtl8723bs/hal/sdio_ops.c | 224 +++
 1 file changed, 111 insertions(+), 113 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback

2017-12-13 Thread Johan Hovold
On Wed, Dec 13, 2017 at 12:32:47PM +0100, Ladislav Michl wrote:
> On Wed, Dec 13, 2017 at 12:16:05PM +0100, Johan Hovold wrote:
> > On Mon, Dec 11, 2017 at 01:44:57PM +0100, Ladislav Michl wrote:
> > > On Mon, Dec 11, 2017 at 12:52:46PM +0100, Johan Hovold wrote:
> > > > On Mon, Dec 11, 2017 at 12:32:49PM +0100, Ladislav Michl wrote:
> > > > > On Mon, Dec 11, 2017 at 11:10:35AM +0100, Johan Hovold wrote:
> > > [snip]
> > > > > > I'm afraid I don't consider this an improvement. I prefer using 
> > > > > > gotos
> > > > > > for error paths, while keeping the success path out of the status
> > > > > > switch.
> > > > > > 
> > > > > > Furthermore, this isn't functionally equivalent as we'd not longer 
> > > > > > log
> > > > > > an error for -EPIPE.
> > > > > 
> > > > > Yes, you are right... Now, shouldn't we react somehow to stalled 
> > > > > endpoint?
> > > > > Tty side seems to be unaware of it.
> > > > 
> > > > Recovering from a stalled endpoint is a bit involved, so for now we
> > > > typically just log an error an bail out (forcing the user to reopen the
> > > > port). This seems to work well enough as this condition should be rare.
> > > 
> > > I just do not see this in code. I would expect pending tty I/O operation
> > > would fail once USB device errors out with -EPIPE, so tty side consumer 
> > > gets
> > > notified about error. Either it is not there or I did not look hard 
> > > enough :)
> > 
> > No, we do not provide any error notification besides logging an error
> > when an endpoint has been stalled (i.e. in case of a read, you would not
> > receive any more data before the halt condition has been cleared).
> 
> Hmm, could we make 1aba579f3cf5 a bit more generic then?

I'm not sure this is worth the complexity it adds, or that it really
solves anything as the fundamental problem remains; simply clearing the
Halt is not sufficient unless the underlying reason for the condition
triggering the stall has first been removed (and that is going to be
device specific).

> > Given that there's been no reports about this being an issue (for the
> > past ten years and that I can recall), this crude handling appears to
> > suffice.
> 
> Because embedded people tend to power cycle stuck device on timeout.
> That involves project specific hacks in userspace, which could be
> probably handled better.

I haven't seen any reports of this being an issue (and your stalled CDC
device was really due to a hardware/power issue IIRC) so again, I'm not
convinced this gives us anything.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Oliver Neukum
Am Mittwoch, den 13.12.2017, 14:31 +0300 schrieb Mikhail Zaytsev:
> On Wed, 13 Dec 2017 11:17:28 +0100 Oliver Neukum  wrote:
> 
> > 
> > Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> > > 
> > > +#define RS232_VENDOR 0x6547
> > > +#define RS232_PRODUCT 0x0232
> > > +#define IRDA_VENDOR 0x18ec
> > > +#define IRDA_PRODUCT 0x3118
> > >  
> > >  /* usb timeout of 1 second */
> > >  #define ARK_TIMEOUT 1000
> > >  
> > >  static const struct usb_device_id id_table[] = {
> > > -   { USB_DEVICE(0x6547, 0x0232) },
> > > -   { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> > > +   { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> > > +   { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) },  /* USB to IrDA 
> > > adapter */  
> > 
> > Hi,
> > 
> > what is the purpose of this change? It just makes it harder to grep.
> > The constants are arbitrary and they are clearly device IDs.
> 
> The constants are using in several places. 
> I think the names easier to read.

They give you nothing. If you are looking at a vendor ID nothing but the
bare number makes sense. You are just making peoples' life harder when
they have to look up that definition. A symbolic name is fine if it gives
meaning. Even if the information you give is that the value is magic
and therefore not understood. But a vendor ID is an arbitrary yet
meaningful number. There is no point in hiding it.

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] usb/io_edgeport: a possible sleep-in-atomic bug in edge_bulk_in_callback

2017-12-13 Thread Johan Hovold
[ +CC: linux-usb]

On Wed, Dec 13, 2017 at 06:22:26PM +0800, Jia-Ju Bai wrote:
> According to drivers/usb/serial/io_edgeport.c, the driver may sleep 
> under a spinlock.
> The function call path is:
> edge_bulk_in_callback (acquire the spinlock)
>process_rcvd_data
>  process_rcvd_status
>change_port_settings
>  send_iosp_ext_cmd
>write_cmd_usb
>  usb_kill_urb --> may sleep
> 
> I do not find a good way to fix it, so I only report.
> This possible bug is found by my static analysis tool (DSAC) and checked 
> by my code review.

Good catch!

Fortunately, the fix here is just to remove that usb_kill_urb() from the
error path in write_cmd_usb() after usb_submit_urb() fails.

Care to submit a patch for that?

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback

2017-12-13 Thread Ladislav Michl
On Wed, Dec 13, 2017 at 12:16:05PM +0100, Johan Hovold wrote:
> On Mon, Dec 11, 2017 at 01:44:57PM +0100, Ladislav Michl wrote:
> > On Mon, Dec 11, 2017 at 12:52:46PM +0100, Johan Hovold wrote:
> > > On Mon, Dec 11, 2017 at 12:32:49PM +0100, Ladislav Michl wrote:
> > > > On Mon, Dec 11, 2017 at 11:10:35AM +0100, Johan Hovold wrote:
> > [snip]
> > > > > I'm afraid I don't consider this an improvement. I prefer using gotos
> > > > > for error paths, while keeping the success path out of the status
> > > > > switch.
> > > > > 
> > > > > Furthermore, this isn't functionally equivalent as we'd not longer log
> > > > > an error for -EPIPE.
> > > > 
> > > > Yes, you are right... Now, shouldn't we react somehow to stalled 
> > > > endpoint?
> > > > Tty side seems to be unaware of it.
> > > 
> > > Recovering from a stalled endpoint is a bit involved, so for now we
> > > typically just log an error an bail out (forcing the user to reopen the
> > > port). This seems to work well enough as this condition should be rare.
> > 
> > I just do not see this in code. I would expect pending tty I/O operation
> > would fail once USB device errors out with -EPIPE, so tty side consumer gets
> > notified about error. Either it is not there or I did not look hard enough 
> > :)
> 
> No, we do not provide any error notification besides logging an error
> when an endpoint has been stalled (i.e. in case of a read, you would not
> receive any more data before the halt condition has been cleared).

Hmm, could we make 1aba579f3cf5 a bit more generic then?

> Given that there's been no reports about this being an issue (for the
> past ten years and that I can recall), this crude handling appears to
> suffice.

Because embedded people tend to power cycle stuck device on timeout.
That involves project specific hacks in userspace, which could be
probably handled better.

Best regards,
ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Mikhail Zaytsev
On Wed, 13 Dec 2017 11:17:28 +0100 Oliver Neukum  wrote:

> Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> > +#define RS232_VENDOR 0x6547
> > +#define RS232_PRODUCT 0x0232
> > +#define IRDA_VENDOR 0x18ec
> > +#define IRDA_PRODUCT 0x3118
> >  
> >  /* usb timeout of 1 second */
> >  #define ARK_TIMEOUT 1000
> >  
> >  static const struct usb_device_id id_table[] = {
> > -   { USB_DEVICE(0x6547, 0x0232) },
> > -   { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> > +   { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> > +   { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) },  /* USB to IrDA adapter 
> > */  
> 
> Hi,
> 
> what is the purpose of this change? It just makes it harder to grep.
> The constants are arbitrary and they are clearly device IDs.

The constants are using in several places. 
I think the names easier to read.

-- 
Mikhail
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: core: prevent malicious bNumInterfaces overflow

2017-12-13 Thread Greg KH
On Tue, Dec 12, 2017 at 02:25:13PM -0500, Alan Stern wrote:
> A malicious USB device with crafted descriptors can cause the kernel
> to access unallocated memory by setting the bNumInterfaces value too
> high in a configuration descriptor.  Although the value is adjusted
> during parsing, this adjustment is skipped in one of the error return
> paths.
> 
> This patch prevents the problem by setting bNumInterfaces to 0
> initially.  The existing code already sets it to the proper value
> after parsing is complete.
> 
> Signed-off-by: Alan Stern 
> Reported-by: Andrey Konovalov 
> CC: 
> 
> ---
> 
> 
> [as1855]
> 
> 
>  drivers/usb/core/config.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Index: usb-4.x/drivers/usb/core/config.c
> ===
> --- usb-4.x.orig/drivers/usb/core/config.c
> +++ usb-4.x/drivers/usb/core/config.c
> @@ -555,6 +555,9 @@ static int usb_parse_configuration(struc
>   unsigned iad_num = 0;
>  
>   memcpy(>desc, buffer, USB_DT_CONFIG_SIZE);
> + nintf = nintf_orig = config->desc.bNumInterfaces;
> + config->desc.bNumInterfaces = 0;// Adjusted later
> +
>   if (config->desc.bDescriptorType != USB_DT_CONFIG ||
>   config->desc.bLength < USB_DT_CONFIG_SIZE ||
>   config->desc.bLength > size) {
> @@ -568,7 +571,6 @@ static int usb_parse_configuration(struc
>   buffer += config->desc.bLength;
>   size -= config->desc.bLength;
>  
> - nintf = nintf_orig = config->desc.bNumInterfaces;

Ugh, I tried to find this place to do this, but couldn't.  Nice job,
I'll revert my patch and apply yours instead, thanks for this.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Johan Hovold
On Wed, Dec 13, 2017 at 12:30:04PM +0300, Mikhail Zaytsev wrote:
> The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
>  moves to the get_serial_info() function. Some magic numbers moves to
>  #define directives.

You need to split logical changes up in separate patches, and there's at
least three things being done here.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ti_usb_3410_5052: Avoid goto in bulk in callback

2017-12-13 Thread Johan Hovold
On Mon, Dec 11, 2017 at 01:44:57PM +0100, Ladislav Michl wrote:
> On Mon, Dec 11, 2017 at 12:52:46PM +0100, Johan Hovold wrote:
> > On Mon, Dec 11, 2017 at 12:32:49PM +0100, Ladislav Michl wrote:
> > > On Mon, Dec 11, 2017 at 11:10:35AM +0100, Johan Hovold wrote:
> [snip]
> > > > I'm afraid I don't consider this an improvement. I prefer using gotos
> > > > for error paths, while keeping the success path out of the status
> > > > switch.
> > > > 
> > > > Furthermore, this isn't functionally equivalent as we'd not longer log
> > > > an error for -EPIPE.
> > > 
> > > Yes, you are right... Now, shouldn't we react somehow to stalled endpoint?
> > > Tty side seems to be unaware of it.
> > 
> > Recovering from a stalled endpoint is a bit involved, so for now we
> > typically just log an error an bail out (forcing the user to reopen the
> > port). This seems to work well enough as this condition should be rare.
> 
> I just do not see this in code. I would expect pending tty I/O operation
> would fail once USB device errors out with -EPIPE, so tty side consumer gets
> notified about error. Either it is not there or I did not look hard enough :)

No, we do not provide any error notification besides logging an error
when an endpoint has been stalled (i.e. in case of a read, you would not
receive any more data before the halt condition has been cleared).

Given that there's been no reports about this being an issue (for the
past ten years and that I can recall), this crude handling appears to
suffice.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] usbip: fix usbip bind writing random string after command in match_busid

2017-12-13 Thread Juan Zea
usbip bind writes commands followed by random string when writing to
match_busid attribute in sysfs, caused by using full variable size
instead of string length.

Signed-off-by: Juan Zea 
---
 tools/usb/usbip/src/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/usb/usbip/src/utils.c b/tools/usb/usbip/src/utils.c
index 2b3d6d2..ea1a1af 100644
--- a/tools/usb/usbip/src/utils.c
+++ b/tools/usb/usbip/src/utils.c
@@ -42,7 +42,7 @@ int modify_match_busid(char *busid, int add)
snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
 
rc = write_sysfs_attribute(match_busid_attr_path, command,
-  sizeof(command));
+  strlen(command));
if (rc < 0) {
dbg("failed to write match_busid: %s", strerror(errno));
return -1;
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: xhci: Add XHCI_TRUST_TX_LENGTH for Renesas uPD720201

2017-12-13 Thread Mathias Nyman

On 12.12.2017 18:54, Ard Biesheuvel wrote:

On 12 December 2017 at 16:47, Daniel Thompson
 wrote:

When plugging in a USB webcam I see the following message:
xhci_hcd :04:00.0: WARN Successful completion on short TX: needs
XHCI_TRUST_TX_LENGTH quirk?
handle_tx_event: 913 callbacks suppressed

All is quiet again with this patch (and I've done a fair but of soak
testing with the camera since).

Cc: 
Signed-off-by: Daniel Thompson 


I have been setting this quirk manually for ages on my kernel command
line, for the same reason (MS HD Webcam), and with the same positive
result, so

Acked-by: Ard Biesheuvel 


Thanks, adding

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] xhci: Fix use-after-free in xhci debugfs

2017-12-13 Thread Mathias Nyman

On 11.12.2017 00:14, Alexander Kappner wrote:

Trying to read from debugfs after the system has resumed from
  hibernate causes a use-after-free and thus a protection fault.

Steps to reproduce:
Hibernate system, resume from hibernate, then run
$ cat /sys/kernel/debug/usb/xhci/*/command-ring/enqueue

dmesg below:

[ 3902.765086] general protection fault:  [#1] PREEMPT SMP
[ 3902.765095] Modules linked in: ipheth nvidia_modeset(PO) iwlmvm mac80211 
nvidia(PO) iwlwifi qmi_wwan cfg80211 thinkpad_acpi rfkill
[ 3902.765118] CPU: 4 PID: 3591 Comm: cat Tainted: P   O 
4.14.0.1-12769-g1deab8c #1
[ 3902.765121] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS N1EET62W (1.35 
) 11/10/2016
[ 3902.765125] task: 88100263d040 task.stack: c90006e5
[ 3902.765136] RIP: 0010:xhci_trb_virt_to_dma.part.50+0x5/0x30
[ 3902.765140] RSP: 0018:c90006e53da8 EFLAGS: 00010286
[ 3902.765144] RAX: 88100a57e680 RBX: 8810032f1900 RCX: 881013800900
[ 3902.765147] RDX:  RSI: 881000abf488 RDI: 0d0c0a0bb1637008
[ 3902.765151] RBP: c90006e53f00 R08:  R09: 
[ 3902.765154] R10: 7ffcd73165e0 R11: 88100263d040 R12: 880bd5afcc00
[ 3902.765157] R13: 0002 R14: 0001 R15: 8810032f1900
[ 3902.765161] FS:  7f6aa6cec700() GS:881053d0() 
knlGS:
[ 3902.765165] CS:  0010 DS:  ES:  CR0: 80050033
[ 3902.765168] CR2: 7f6aa6d08008 CR3: 0010033f9003 CR4: 003606e0
[ 3902.765172] DR0:  DR1:  DR2: 
[ 3902.765175] DR3:  DR6: fffe0ff0 DR7: 0400
[ 3902.765178] Call Trace:
[ 3902.765188]  xhci_ring_enqueue_show+0x1e/0x40
[ 3902.765197]  seq_read+0xdb/0x3a0
[ 3902.765204]  ? __handle_mm_fault+0x5fb/0x1210
[ 3902.765211]  full_proxy_read+0x4a/0x70
[ 3902.765219]  __vfs_read+0x23/0x120
[ 3902.765228]  vfs_read+0x8e/0x130
[ 3902.765235]  SyS_read+0x42/0x90
[ 3902.765242]  do_syscall_64+0x6b/0x290
[ 3902.765251]  entry_SYSCALL64_slow_path+0x25/0x25
[ 3902.765256] RIP: 0033:0x7f6aa683cba0
[ 3902.765260] RSP: 002b:7ffcd7316818 EFLAGS: 0246 ORIG_RAX: 

[ 3902.765264] RAX: ffda RBX: 0002 RCX: 7f6aa683cba0
[ 3902.765267] RDX: 0002 RSI: 7f6aa6d09000 RDI: 0003
[ 3902.765270] RBP: 0002 R08:  R09: 
[ 3902.765274] R10: 7ffcd73165e0 R11: 0246 R12: 7f6aa6d09000
[ 3902.765277] R13: 0003 R14:  R15: 0002
[ 3902.765282] Code: 0f 44 c0 49 8b 04 24 52 48 c7 c2 30 cd cb 81 48 8d b0 98 00 00 
00 31 c0 e8 39 e6 dc ff 58 e9 75 ff ff ff 0f 1f 00 0f 1f 44 00 00 <48> 8b 17 31 
c0 48 39 f2 77 13 48 29 d6 48 81 fe ff 0f 00 00 77
[ 3902.765371] RIP: xhci_trb_virt_to_dma.part.50+0x5/0x30 RSP: c90006e53da8
[ 3902.765376] ---[ end trace 9ee53de1dccf7d8e ]---

The issue is caused by the xhci ring structures being reallocated
when the system is resumed, but pointers to the old structures
being retained in the debugfs files "private" field:

(gdb) list *(xhci_ring_enqueue_show+0x1e)
0x8170403e is in xhci_ring_enqueue_show 
(drivers/usb/host/xhci-debugfs.c:168).
163 {
164 dma_addr_t  dma;
165 struct xhci_ring*ring = s->private;
166
167 dma = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
168 seq_printf(s, "%pad\n", );
169
170 return 0;
171 }
172

The proposed patch fixes this issue by storing a pointer to the xhci_ring
field in the xhci device structure in debugfs rather than directly
storing a pointer to the xhci_ring.

Signed-off-by: Alexander Kappner 
---


Thanks, that should work.
I'll try it out, and shorten the commit message a bit.
If everything works I'll apply it to my for-usb-linus branch

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usbip: fix usbip bind writing random string after command in match_busid

2017-12-13 Thread Greg KH
On Wed, Dec 13, 2017 at 11:16:03AM +0100, Juan Zea wrote:
> usbip bind writes commands followed by random string when writing to
> match_busid attribute in sysfs, caused by using full variable size
> instead of string length.
> 
> Signed-off-by: Juan Zea 
> ---
>  tools/usb/usbip/src/utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

You forgot to cc: the maintainers of this file:

$ ./scripts/get_maintainer.pl --file tools/usb/usbip/src/utils.c
Valentina Manea  (maintainer:USB OVER IP DRIVER)
Shuah Khan  (maintainer:USB OVER IP DRIVER)
linux-usb@vger.kernel.org (open list:USB OVER IP DRIVER)
linux-ker...@vger.kernel.org (open list)

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Oliver Neukum
Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> +#define RS232_VENDOR 0x6547
> +#define RS232_PRODUCT 0x0232
> +#define IRDA_VENDOR 0x18ec
> +#define IRDA_PRODUCT 0x3118
>  
>  /* usb timeout of 1 second */
>  #define ARK_TIMEOUT 1000
>  
>  static const struct usb_device_id id_table[] = {
> -   { USB_DEVICE(0x6547, 0x0232) },
> -   { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> +   { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> +   { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) },  /* USB to IrDA adapter */

Hi,

what is the purpose of this change? It just makes it harder to grep.
The constants are arbitrary and they are clearly device IDs.

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] usbip: fix usbip bind writing random string after command in match_busid

2017-12-13 Thread Juan Zea
usbip bind writes commands followed by random string when writing to
match_busid attribute in sysfs, caused by using full variable size
instead of string length.

Signed-off-by: Juan Zea 
---
 tools/usb/usbip/src/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/usb/usbip/src/utils.c b/tools/usb/usbip/src/utils.c
index 2b3d6d2..ea1a1af 100644
--- a/tools/usb/usbip/src/utils.c
+++ b/tools/usb/usbip/src/utils.c
@@ -42,7 +42,7 @@ int modify_match_busid(char *busid, int add)
snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
 
rc = write_sysfs_attribute(match_busid_attr_path, command,
-  sizeof(command));
+  strlen(command));
if (rc < 0) {
dbg("failed to write match_busid: %s", strerror(errno));
return -1;
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: gadget: configfs: Disallow empty function instance name

2017-12-13 Thread Krzysztof Opasiak



On 12/13/2017 10:29 AM, Felipe Balbi wrote:


Hi,

Alan Stern  writes:

Krzysztof Opasiak  writes:

On 12/12/2017 01:31 PM, Felipe Balbi wrote:


Hi,

Krzysztof Opasiak  writes:

Every function should have a type and instance name.
Unfortunately in most cases instance name was left unused and unchecked.
This may lead to situations like FunctionFS device name identified by ""
or some misleading debug messages from TCM like:

tcm: Activating

To avoid this let's add a check that instance name should have at least
one character.

Reported-by: Stefan Agner 
Signed-off-by: Krzysztof Opasiak 
---
   drivers/usb/gadget/configfs.c | 5 +
   1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index aeb9f3c40521..bdc9ec597d6a 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -548,6 +548,11 @@ static struct config_group *function_make(
*instance_name = '\0';
instance_name++;
   
+	if (*instance_name == '\0') {

+   pr_err("Instance name (after .) should not be empty\n");
+   return ERR_PTR(-EINVAL);
+   }


aand just like that you break potentially existing scripts :-)

We need to find a better way of enforcing a name which doesn't break
existing users.


I'm really open for suggestions how to enforce this without breaking
those scripts ;)

The origin of this commit is github issue for libusbgx[1].
So the problem is that library allows to create a function with empty
name (because I mistakenly assumed that kernel rejects this) but then it
is unable to reinitialize the ConfigFS state because there is a check
that disallows this. From my point of view I'd be happy to disallow
empty names because it causes some problems (f_fs) or weird debug
messages (f_tcm) so is generally inconvenient and seems to be
unintentional. But I would like to keep this consistent with kernel policy.


I think we need to first fix libusbgx to prevent empty names.

I don't want to be the one hearing from Linus that "we don't break
userspace". It's clear that empty names shouldn't be allowed, but they
_are_ allowed as of today, so how can we cause a regression all of a
sudden?

Alan, Greg, any suggestions?


You could do some silly name munging, like changing an empty name to
" " whenever you encounter it.  Or adding an '_' to the end of any name
that consists of nothing but '_' characters.


Hmm, that could be done. So everytime userspace gives us an empty name,
we would convert to '_'. That still doesn't solve the problems of
mounting functionfs, though. But I guess there's nothing that can be
done in that case.



How is it different from disallowing empty name?
It may also cause some "broken" scripts stop working.
Isn't it going to introduce some weird problems like:

mkdir g1/function/ffs._
mkdir g2/function/ffs.
-EBUSY

Best regards,
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: gadget: configfs: Disallow empty function instance name

2017-12-13 Thread Felipe Balbi

Hi,

Alan Stern  writes:
>> Krzysztof Opasiak  writes:
>> > On 12/12/2017 01:31 PM, Felipe Balbi wrote:
>> >> 
>> >> Hi,
>> >> 
>> >> Krzysztof Opasiak  writes:
>> >>> Every function should have a type and instance name.
>> >>> Unfortunately in most cases instance name was left unused and unchecked.
>> >>> This may lead to situations like FunctionFS device name identified by ""
>> >>> or some misleading debug messages from TCM like:
>> >>>
>> >>> tcm: Activating
>> >>>
>> >>> To avoid this let's add a check that instance name should have at least
>> >>> one character.
>> >>>
>> >>> Reported-by: Stefan Agner 
>> >>> Signed-off-by: Krzysztof Opasiak 
>> >>> ---
>> >>>   drivers/usb/gadget/configfs.c | 5 +
>> >>>   1 file changed, 5 insertions(+)
>> >>>
>> >>> diff --git a/drivers/usb/gadget/configfs.c 
>> >>> b/drivers/usb/gadget/configfs.c
>> >>> index aeb9f3c40521..bdc9ec597d6a 100644
>> >>> --- a/drivers/usb/gadget/configfs.c
>> >>> +++ b/drivers/usb/gadget/configfs.c
>> >>> @@ -548,6 +548,11 @@ static struct config_group *function_make(
>> >>>  *instance_name = '\0';
>> >>>  instance_name++;
>> >>>   
>> >>> +if (*instance_name == '\0') {
>> >>> +pr_err("Instance name (after .) should not be empty\n");
>> >>> +return ERR_PTR(-EINVAL);
>> >>> +}
>> >> 
>> >> aand just like that you break potentially existing scripts :-)
>> >> 
>> >> We need to find a better way of enforcing a name which doesn't break
>> >> existing users.
>> >
>> > I'm really open for suggestions how to enforce this without breaking 
>> > those scripts ;)
>> >
>> > The origin of this commit is github issue for libusbgx[1].
>> > So the problem is that library allows to create a function with empty 
>> > name (because I mistakenly assumed that kernel rejects this) but then it 
>> > is unable to reinitialize the ConfigFS state because there is a check 
>> > that disallows this. From my point of view I'd be happy to disallow 
>> > empty names because it causes some problems (f_fs) or weird debug 
>> > messages (f_tcm) so is generally inconvenient and seems to be 
>> > unintentional. But I would like to keep this consistent with kernel policy.
>> 
>> I think we need to first fix libusbgx to prevent empty names.
>> 
>> I don't want to be the one hearing from Linus that "we don't break
>> userspace". It's clear that empty names shouldn't be allowed, but they
>> _are_ allowed as of today, so how can we cause a regression all of a
>> sudden?
>> 
>> Alan, Greg, any suggestions?
>
> You could do some silly name munging, like changing an empty name to
> " " whenever you encounter it.  Or adding an '_' to the end of any name
> that consists of nothing but '_' characters.

Hmm, that could be done. So everytime userspace gives us an empty name,
we would convert to '_'. That still doesn't solve the problems of
mounting functionfs, though. But I guess there's nothing that can be
done in that case.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Mikhail Zaytsev
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
 moves to the get_serial_info() function. Some magic numbers moves to
 #define directives.

Signed-off-by: Mikhail Zaytsev 
---
 drivers/usb/serial/ark3116.c | 54 ++--
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..f45f69e18 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,13 +36,19 @@
 #define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
 #define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
 #define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
+
+#define RS232_VENDOR 0x6547
+#define RS232_PRODUCT 0x0232
+#define IRDA_VENDOR 0x18ec
+#define IRDA_PRODUCT 0x3118
 
 /* usb timeout of 1 second */
 #define ARK_TIMEOUT 1000
 
 static const struct usb_device_id id_table[] = {
-   { USB_DEVICE(0x6547, 0x0232) },
-   { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
+   { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
+   { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) },  /* USB to IrDA adapter */
{ },
 };
 MODULE_DEVICE_TABLE(usb, id_table);
@@ -50,8 +56,8 @@ MODULE_DEVICE_TABLE(usb, id_table);
 static int is_irda(struct usb_serial *serial)
 {
struct usb_device *dev = serial->dev;
-   if (le16_to_cpu(dev->descriptor.idVendor) == 0x18ec &&
-   le16_to_cpu(dev->descriptor.idProduct) == 0x3118)
+   if (le16_to_cpu(dev->descriptor.idVendor) == IRDA_VENDOR &&
+   le16_to_cpu(dev->descriptor.idProduct) == IRDA_PRODUCT)
return 1;
return 0;
 }
@@ -397,31 +403,35 @@ static int ark3116_open(struct tty_struct *tty, struct 
usb_serial_port *port)
return result;
 }
 
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+   unsigned long arg)
+{
+   struct serial_struct ser;
+
+   memset(, 0, sizeof(ser));
+
+   ser.type = PORT_16654;
+   ser.line = port->minor;
+   ser.port = port->port_number;
+   ser.custom_divisor = 0;
+   ser.baud_base = ARK3116_BAUDRATE;
+
+   if (copy_to_user((void __user *)arg, , sizeof(ser)))
+   return -EFAULT;
+
+   return 0;
+}
+
 static int ark3116_ioctl(struct tty_struct *tty,
 unsigned int cmd, unsigned long arg)
 {
struct usb_serial_port *port = tty->driver_data;
-   struct serial_struct serstruct;
-   void __user *user_arg = (void __user *)arg;
 
switch (cmd) {
case TIOCGSERIAL:
-   /* XXX: Some of these values are probably wrong. */
-   memset(, 0, sizeof(serstruct));
-   serstruct.type = PORT_16654;
-   serstruct.line = port->minor;
-   serstruct.port = port->port_number;
-   serstruct.custom_divisor = 0;
-   serstruct.baud_base = 460800;
-
-   if (copy_to_user(user_arg, , sizeof(serstruct)))
-   return -EFAULT;
-
-   return 0;
-   case TIOCSSERIAL:
-   if (copy_from_user(, user_arg, sizeof(serstruct)))
-   return -EFAULT;
-   return 0;
+   return ark3116_get_serial_info(port, arg);
+   default:
+   break;
}
 
return -ENOIOCTLCMD;
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] kaweth: a possible sleep-in-atomic bug in kaweth_start_xmit

2017-12-13 Thread Oliver Neukum
Am Mittwoch, den 13.12.2017, 16:57 +0800 schrieb Jia-Ju Bai:
> According to drivers/net/usb/kaweth.c, the driver may sleep under a 
> spinlock.
> The function call path is:
> kaweth_start_xmit (acquire the spinlock)
>kaweth_async_set_rx_mode
>  kaweth_control
>kaweth_internal_control_msg
>  usb_start_wait_urb
>wait_event_timeout --> may sleep
>usb_kill_urb --> may sleep
> 
> I do not find a good way to fix it, so I only report.
> This possible bug is found by my static analysis tool (DSAC) and checked 
> by my code review.
> 

Hi,

thanks for reporting. I need to get out my old test device.
It will take a few days. The obvious fix would be to set this
filter only on initialization. Unfortunately this needs to
be tested.

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] kaweth: a possible sleep-in-atomic bug in kaweth_start_xmit

2017-12-13 Thread Jia-Ju Bai
According to drivers/net/usb/kaweth.c, the driver may sleep under a 
spinlock.

The function call path is:
kaweth_start_xmit (acquire the spinlock)
  kaweth_async_set_rx_mode
kaweth_control
  kaweth_internal_control_msg
usb_start_wait_urb
  wait_event_timeout --> may sleep
  usb_kill_urb --> may sleep

I do not find a good way to fix it, so I only report.
This possible bug is found by my static analysis tool (DSAC) and checked 
by my code review.



Thanks,
Jia-Ju Bai
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl

2017-12-13 Thread Mikhail Zaytsev
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
 moves to the get_serial_info() function. Any magic numbers moves to
 #define directives.

Signed-off-by: Mikhail Zaytsev 
---
 drivers/usb/serial/ark3116.c | 54 ++--
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..f45f69e18 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,13 +36,19 @@
 #define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
 #define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
 #define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
+
+#define RS232_VENDOR 0x6547
+#define RS232_PRODUCT 0x0232
+#define IRDA_VENDOR 0x18ec
+#define IRDA_PRODUCT 0x3118
 
 /* usb timeout of 1 second */
 #define ARK_TIMEOUT 1000
 
 static const struct usb_device_id id_table[] = {
-   { USB_DEVICE(0x6547, 0x0232) },
-   { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
+   { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
+   { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) },  /* USB to IrDA adapter */
{ },
 };
 MODULE_DEVICE_TABLE(usb, id_table);
@@ -50,8 +56,8 @@ MODULE_DEVICE_TABLE(usb, id_table);
 static int is_irda(struct usb_serial *serial)
 {
struct usb_device *dev = serial->dev;
-   if (le16_to_cpu(dev->descriptor.idVendor) == 0x18ec &&
-   le16_to_cpu(dev->descriptor.idProduct) == 0x3118)
+   if (le16_to_cpu(dev->descriptor.idVendor) == IRDA_VENDOR &&
+   le16_to_cpu(dev->descriptor.idProduct) == IRDA_PRODUCT)
return 1;
return 0;
 }
@@ -397,31 +403,35 @@ static int ark3116_open(struct tty_struct *tty, struct 
usb_serial_port *port)
return result;
 }
 
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+   unsigned long arg)
+{
+   struct serial_struct ser;
+
+   memset(, 0, sizeof(ser));
+
+   ser.type = PORT_16654;
+   ser.line = port->minor;
+   ser.port = port->port_number;
+   ser.custom_divisor = 0;
+   ser.baud_base = ARK3116_BAUDRATE;
+
+   if (copy_to_user((void __user *)arg, , sizeof(ser)))
+   return -EFAULT;
+
+   return 0;
+}
+
 static int ark3116_ioctl(struct tty_struct *tty,
 unsigned int cmd, unsigned long arg)
 {
struct usb_serial_port *port = tty->driver_data;
-   struct serial_struct serstruct;
-   void __user *user_arg = (void __user *)arg;
 
switch (cmd) {
case TIOCGSERIAL:
-   /* XXX: Some of these values are probably wrong. */
-   memset(, 0, sizeof(serstruct));
-   serstruct.type = PORT_16654;
-   serstruct.line = port->minor;
-   serstruct.port = port->port_number;
-   serstruct.custom_divisor = 0;
-   serstruct.baud_base = 460800;
-
-   if (copy_to_user(user_arg, , sizeof(serstruct)))
-   return -EFAULT;
-
-   return 0;
-   case TIOCSSERIAL:
-   if (copy_from_user(, user_arg, sizeof(serstruct)))
-   return -EFAULT;
-   return 0;
+   return ark3116_get_serial_info(port, arg);
+   default:
+   break;
}
 
return -ENOIOCTLCMD;
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Add your device DWM-222 to a proper driver

2017-12-13 Thread Lars Melin

On 12/13/2017 14:18, martin.er...@centrum.cz wrote:

Dear linux development,

please to add device to a proper driver. I wrote this requirement according log 
list message.

My USB modem is D-link DWM-222

Vendor    3G Standard    Default     Id        Modem Id    User     Modem 
Storage    Diag    Protocol(s)

DWM-222 A1        LTE CAT4    2001:ab00    2001:7e35    2    1    5    0    serial 
& qmi



Dear linux user,

DWM-222 A1 with USB Id 2001:7e35 was added to the option driver 4-5 
month ago so it is either a matter of loading the driver or updating 
your kernel to a more recent version.



rgds
Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html