Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[One Thousand Gnomes]

On Sun, 14 Aug 2016 17:34:18 +0800
Tom Yan  wrote:

> Since when it is expected that SATA disks will always be probed before
> USB disks? We can't guarantee that even if we make sure all ata
> drivers are loaded before usb-storage/uas. That's why we need
> consistent namings (e.g. /dev/disk/by-id/*).

It hasn't. It isn't even true on older kernels and can depend on all
sorts of things. I have boxes that will put the USB first, and a 10
second google search says I'm not alone

http://serverfault.com/questions/322946/ensure-usb-disk-is-never-sda-even-when-booting-from-it

So your "regression" goes back to at least 2011.

Alan
--
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: Add Fintek F81532/534 driver

[One Thousand Gnomes]

O
> +static int f81534_set_normal_register(struct usb_device *dev, u16 reg, u8 
> data)
> +{
> + size_t count = F81534_USB_MAX_RETRY;
> + int status;
> + u8 *tmp;
> +
> + tmp = kmalloc(sizeof(u8), GFP_KERNEL);
> + if (!tmp)
> + return -ENOMEM;

You end up doing huge numbers of tiny allocation and frees in some of the
code paths. I think it would be better to allocate them at a higher level
as they are not that cheap on CPU time.

> +static int f81534_read_data(struct usb_serial *usbserial, u32 address,
> + size_t size, unsigned char *buf)
> +{

Is a particularly good example - you do 4 mallocs plus two per byte of
data.

Alan
--
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 v1 1/1] usb: gadget: NCM: NULL pointer dereference in eth_start_xmit

[One Thousand Gnomes]

On Mon, 25 Apr 2016 16:25:03 +0100
 wrote:

> From: Jim Baxter 
> 
> "Unable to handle kernel NULL pointer dereference at virtual address
> 0078" is reported with "PC is at eth_start_xmit+0x19c/0x378 [u_ether]"
> 
> The failure scenario is seen when closing the NCM connection while the
> TCP/IPv6 stack is still trying to transmit over NCM.
> 
> Defensive code is missing from commit
> 6d3865f9d41f15ddbcecaa6722871fc0db21d7ab
> "usb: gadget: NCM: Add transmit multi-frame."

This looks inadequate. Surely you also need to hold dev->lock ?

Also it'll also crash at the no zlp test with the same problem.

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

Re: [PATCH v2 07/14] USB: ch341: add support for parity, frame length, stop bits

[One Thousand Gnomes]

On Sat,  2 Apr 2016 19:07:16 +0200
Grigori Goronzy  wrote:

> With the new reinitialization method, configuring parity, different
> frame lengths and different stop bit settings work as expected on
> both CH340G and CH341A.  This has been extensively tested with a
> logic analyzer.
> 
> Tested-by: Ryan Barber 
> Signed-off-by: Grigori Goronzy 
> ---
>  drivers/usb/serial/ch341.c | 36 ++--
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index c001773..4d66f0f 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -346,6 +346,7 @@ static void ch341_set_termios(struct tty_struct *tty,
>   unsigned baud_rate;
>   unsigned long flags;
>   unsigned char ctrl;
> + unsigned cflag = tty->termios.c_cflag;
>   int r;
>  
>   /* redundant changes may cause the chip to lose bytes */
> @@ -356,7 +357,35 @@ static void ch341_set_termios(struct tty_struct *tty,
>  
>   priv->baud_rate = baud_rate;
>  
> - ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
> + ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX;
> +
> + switch (cflag & CSIZE) {
> + case CS5:
> + ctrl |= CH341_LCR_CS5;
> + break;
> + case CS6:
> + ctrl |= CH341_LCR_CS6;
> + break;
> + case CS7:
> + ctrl |= CH341_LCR_CS7;
> + break;
> + case CS8:
> + default:
> + ctrl |= CH341_LCR_CS8;

In the default case tty-.termios.c_cflag should also be updated to show
CS8

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

Re: [PATCH v2 01/14] USB: ch341: improve documentation

[One Thousand Gnomes]

On Sat,  2 Apr 2016 19:07:10 +0200
Grigori Goronzy  wrote:

> Signed-off-by: Grigori Goronzy 
> ---
>  drivers/usb/serial/ch341.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index c73808f..43e4594 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -3,7 +3,8 @@
>   * Copyright 2007, Werner Cornelius 
>   * Copyright 2009, Boris Hajduk 
>   *
> - * ch341.c implements a serial port driver for the Winchiphead CH341.
> + * ch341.c implements a serial port driver for the Winchiphead CH341,
> + * the second-worst USB serial chip in the world.

Tempting as it may be it probably doesn't belong in the kernel.

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

Re: [PATCH v5] USB: serial: add Moxa UPORT 11x0 driver

[One Thousand Gnomes]

>   https://lkml.kernel.org/r/201102171210.03474.fr...@tennebo.com
> 
> However, the TIOCSRS485-ioctl might not even be the right interface for
> changing transceiver modes, and instead a sysfs-based
> usb-serial-specific interface could be more appropriate for example.

sysfs works out very badly if modes can be configured routinely at
runtime. In particular the sysfs interface is completely disconnected
from the vhangup() and security model of the tty interface.

If its the kind if thing just administratively configured at boot then
it's a great interface, just be careful about who has access to it.


Otherwise the traditional interface for that level of configuration has
probably been termiox, which we do have hooks for but don't really make
much use of.

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

Re: [PATCH v2 3/4] usb: serial: implement function for F81232

[One Thousand Gnomes]

 + if (cflag  PARENB) {
 + if (cflag  PARODD)
 + new_lcr |= UART_LCR_PARITY; /* odd */
 + else
 + new_lcr |= SERIAL_EVEN_PARITY; /* even */
 + }

If you don't support mark/space also clear CMSPAR in the passed termios

 - if (old_termios)
 - tty_termios_copy_hw(tty-termios, old_termios);

Also when you set the baud rate compute the resulting actual baud rate
you generated and set it with

/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);

so that the application gets told the baud rate it actually got if it
isn't close to the one they requested.

Alan
--
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: Perform verification for FTDI FT232R devices

[One Thousand Gnomes]

   drivers/usb/serial/ftdi_sio.c | 111 
  +-
   drivers/usb/serial/ftdi_sio.h |  41 
   2 files changed, 151 insertions(+), 1 deletion(-)
 
 Funny patch, you should have saved it for April 1, otherwise people
 might have actually taken this seriously :)
 
 Patches as performance art, now I've seen everything...

Chuckle. Sillyness aside a pure detection version of that patch might be
useful so it can warn users Running Windows may damage your adapter 8)

Is the 0x, 0x0401 they end up with consistent - can we add that to the
default table so end users can at least make use of devices that have
been attacked by malware ?

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

Re: [PATCH v2] usb:serial:pl2303: add GPIOs interface on PL2303

[One Thousand Gnomes]

On Wed, 23 Jul 2014 09:21:29 -0700
Greg KH gre...@linuxfoundation.org wrote:

 On Wed, Jul 23, 2014 at 05:03:14PM +0100, One Thousand Gnomes wrote:
   + if (gpiochip_remove(spriv-gpio-gpio_chip))
   + dev_err(serial-interface-dev, unable to remove 
   gpio_chip?\n);
   + kfree(spriv-gpio);
   + }
   +#endif
 kfree(spriv);
  
  Only other question I have - if I have multiple PL2303HX adapters how
  will I work out which GPIO lines belong to which /dev/ttyUSB* interface ?
 
 sysfs _should_ show you this, as it should point to the parent device,
 which will be associated with the ttyUSB interface.  Well, both the tty
 device and the gpio device will have the same parent, is that good
 enough to determine this, or should the gpio device have the tty device
 as its parent?

Good point - that's probably sufficient as is. The GPIO and tty lines are
sometimes used together and sometimes not.

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

Re: [PATCH v2] usb:serial:pl2303: add GPIOs interface on PL2303

[One Thousand Gnomes]

 --- a/drivers/usb/serial/pl2303.c
 +++ b/drivers/usb/serial/pl2303.c
 @@ -28,6 +28,9 @@
  #include linux/usb.h
  #include linux/usb/serial.h
  #include asm/unaligned.h
 +#ifdef CONFIG_USB_SERIAL_PL2303_GPIO
 +#include linux/gpio.h
 +#endif
  #include pl2303.h

Just include the file anyway it does no harm

  
  
 @@ -143,9 +146,27 @@ struct pl2303_type_data {
   unsigned long quirks;
  };
  
 +#ifdef CONFIG_USB_SERIAL_PL2303_GPIO
 +struct pl2303_gpio {
 + /*
 +  * 0..3: unknown (zero)
 +  * 4: gp0 output enable (1: gp0 pin is output, 0: gp0 pin is input)
 +  * 5: gp1 output enable (1: gp1 pin is output, 0: gp1 pin is input)
 +  * 6: gp0 pin value
 +  * 7: gp1 pin value
 +  */
 + u8 index;
 + struct usb_serial *serial;
 + struct gpio_chip gpio_chip;
 +};
 +#endif

Declaring the struct anyway does no harm


   struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
  
 +#ifdef CONFIG_USB_SERIAL_PL2303_GPIO
 + if (spriv  spriv-gpio) {

Can spriv ever be NULL - how would that occur?


 + if (gpiochip_remove(spriv-gpio-gpio_chip))
 + dev_err(serial-interface-dev, unable to remove 
 gpio_chip?\n);
 + kfree(spriv-gpio);
 + }
 +#endif
   kfree(spriv);

Only other question I have - if I have multiple PL2303HX adapters how
will I work out which GPIO lines belong to which /dev/ttyUSB* interface ?

Do we need a way to actually ask the serial port for its GPIO range ?
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] hso: fix deadlock when receiving bursts of data

[One Thousand Gnomes]

On Thu, 10 Jul 2014 14:37:37 +
David Laight david.lai...@aculab.com wrote:

 From: Olivier Sobrie
 ...
  The function put_rxbuf_data() is called from the urb completion handler.
  It puts the data of the urb transfer in the tty buffer with
  tty_insert_flip_string_flags() and schedules a work queue in order to
  push the data to the ldisc.
  Problem is that we are in a urb completion handler so we can't wait
  until there is room in the tty buffer.

The tty provides the input queue, if the queue is full then just chuck
the data in the bitbucket.  hso is trying to be far too clever.

If hso is fast enough that the buffering isn't sufficient on the tty side
then we need to fix the tty buffer size.

Arguably what we need are tty fastpaths for non N_TTY but thats rather
more work. There's no fundamental reason that hso can't throw the buffer
at buffer straight at the PPP ldisc and straight into the network stack -
just that 

1. The tty mid layer glue is standing in the way
2. The change of line discipline code has to lock against it

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

Re: [PATCH 2/2] hso: fix deadlock when receiving bursts of data

[One Thousand Gnomes]

 You really want to apply flow control back over the 'serial' link.
 That may just cause data discards earlier on the local system.
 But it is possible that not resubmitting the receive urb will cause the
 modem to flow control back to the sender.
 In which case there is some chance that data won't be lost.

If you are doing PPP and you can't keep up the sooner you chuck data the
better. Flow control actually works against performance and good network
behaviour. It's counter intuitive but TCP/IP works best if any
performance bottlenecks are immediately visible and not covered over.

Alan
--
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: ftdi_sio: add GPIO support

[One Thousand Gnomes]

  #include linux/kernel.h
  #include linux/errno.h
 +#ifdef CONFIG_GPIOLIB
 +#include linux/gpio.h
 +#endif


Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.

You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers


#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline  int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
#endif


that keeps the code itself clean and easy to read and also ensures all
the types are checked everywhere we want.

Functionality wise nothing stands out as a problem. I would expect -EBUSY
not -ENXIO if the port was being used for GPIO so could not be used for
tty.

I would favour the new config option.

Alan
--
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: pci-quirks: do not access OHCI_FMINTERVAL register on ULI hw

[One Thousand Gnomes]

 I don't know how linux usb subsystem should behave against such
 half-existing hardware. Perhaps hanging is not the best idea...
 but maybe it should be fixed elsewhere, e.g. by masking non-wired
 devices in platform PCI setup. Perhaps controlled by some device-tree
 key.

Does it have a unique svid/sdid set for the platform - if so you could
just blacklist that combination of vid/did/svid/sdid.

Alan
--
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 1/2] n_tty: fix dropped output characters

[One Thousand Gnomes]

On Fri, 11 Apr 2014 11:41:24 +0200
Johan Hovold jhov...@gmail.com wrote:

 Fix characters being dropped by n_tty_write() due to a failure to
 check the return value of tty_put_char() in do_output_char().
 
 Characters are currently being dropped by write if a tty driver claims
 to have write room available, but still fails to buffer any data

Your driver is buggy. If you advertise a buffer you must honour it and
neither shrink nor revoke it. 

For an URB based device you almost certainly want internal buffering so
you can do proper packetisation. USB serial these days gets it right - see
drivers/usb/serial/generic.c for a fairly simple kfifo based approach.
Whether applying it to cdc_acm would make sense I don't know but it looks
like it might be simpler over-all than the current arrangement.

Alan
--
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 1/2] n_tty: fix dropped output characters

[One Thousand Gnomes]

On Mon, 14 Apr 2014 15:05:49 +0200
Oliver Neukum oneu...@suse.de wrote:

 On Mon, 2014-04-14 at 13:53 +0100, One Thousand Gnomes wrote:
  On Fri, 11 Apr 2014 11:41:24 +0200
  Johan Hovold jhov...@gmail.com wrote:
  
   Fix characters being dropped by n_tty_write() due to a failure to
   check the return value of tty_put_char() in do_output_char().
   
   Characters are currently being dropped by write if a tty driver claims
   to have write room available, but still fails to buffer any data
  
  Your driver is buggy. If you advertise a buffer you must honour it and
  neither shrink nor revoke it.
 
 Is that a general rule or does it apply only till the next write?

Its a general rule. If you've advertised 5 bytes then you can only
advertise 0 after 5 bytes have been sent your way (or a hangup etc
occurred).

Alan
--
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] cdc-acm: some enhancement on acm delayed write

[One Thousand Gnomes]

 (2) acm tty port ASYNCB_INITIALIZED flag will be cleared when
 close. If acm resume callback run after ASYNCB_INITIALIZED flag
 cleared, there will have no chance for delayed write to start.
 That lead to acm_wb.use can't be cleared. If user space open
 acm tty again and try to setd, tty will be blocked in
 tty_wait_until_sent for ever.

If there is data pending when the close occurs the close path should
block until either the close timeout occurs or the buffer is written.
This sounds more like the implementation of the ACM chars_in_buffer
method is wrong and not counting the deferred bytes as unsent ?

Alan
--
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: SPDX-License-Identifier

[One Thousand Gnomes]

On Mon, 24 Feb 2014 06:26:52 -0800
Greg Kroah-Hartman gre...@linuxfoundation.org wrote:

 On Mon, Feb 24, 2014 at 03:03:25PM +0100, Michal Simek wrote:
  
  BTW: Isn't this a good topic for kernel-summit? :-)
 
 No, lawyers don't go to the summit, developers do.

More of a topic for the LF. Particularly as any attempt to touch license
statements in existing drivers would end up needing the corporate lawyer
of every rights holder on the planet for the file in question to be
consulted, which is not I suspect going to happen!

Alan
--
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