-/*
- * The encapsultaion is designed to overcome difficulties with some USB hardware.
+/*
+ * The encapsultaion is designed to overcome difficulties with some USB
+ * hardware.
*
* While the USB protocol has a CRC over the data while in transit, i.e. while
- * being carried over the bus, there is no end to end protection. If the hardware
- * has any problems getting the data into or out of the USB transmit and receive
- * FIFO's then data can be lost.
+ * being carried over the bus, there is no end to end protection. If the
+ * hardware has any problems getting the data into or out of the USB transmit
+ * and receive FIFO's then data can be lost.
*
- * This protocol adds a two byte trailer to each USB packet to specify the number
- * of bytes of valid data and a 10 bit CRC that will allow the receiver to verify
- * that the entire USB packet was received without error.
+ * This protocol adds a two byte trailer to each USB packet to specify the
+ * number of bytes of valid data and a 10 bit CRC that will allow the receiver
+ * to verify that the entire USB packet was received without error.
*
- * Because in this case the sender and receiver are the class and function drivers
- * there is now end to end protection.
+ * Because in this case the sender and receiver are the class and function
+ * drivers there is now end to end protection.
*
- * There is an additional option that can be used to force all transmitted packets
- * to be padded to the maximum packet size. This provides a work around for some
- * devices which have problems with small USB packets.
+ * There is an additional option that can be used to force all transmitted
+ * packets to be padded to the maximum packet size. This provides a work
+ * around for some devices which have problems with small USB packets.
*
* Assuming a packetsize of N:
*
@@ -44,11 +45,12 @@
* | Data Length | 10 bit CRC |
* + 7 . 6 . 5 . 4 . 3 . 2 . 1 . 0 | 7 . 6 . 5 . 4 . 3 . 2 . 1 . 0 +
*
- * The 10 bit CRC is computed across the sent data, followed by the trailer with
- * the length set and the CRC set to zero. The CRC is then OR'd into the trailer.
+ * The 10 bit CRC is computed across the sent data, followed by the trailer
+ * with the length set and the CRC set to zero. The CRC is then OR'd into
+ * the trailer.
*
- * When received a 10 bit CRC is computed over the entire frame including the trailer
- * and should be equal to zero.
+ * When received a 10 bit CRC is computed over the entire frame including
+ * the trailer and should be equal to zero.
*
* Two module parameters are used to control the encapsulation, if both are
* turned of the module works as a simple serial device with NO
@@ -69,7 +71,7 @@
#include
#include
#include
-#include
+#include
#include
#include

/************************************************** ***************************
* mos7840_bulk_out_data_callback
- * this is the callback function for when we have finished sending serial data
- * on the bulk out endpoint.
+ * this is the callback function for when we have finished sending
+ * serial data on the bulk out endpoint.
************************************************** ***************************/

- /* Initialize all port interrupt end point to port 0 int endpoint *
- * Our device has only one interrupt end point comman to all port */
+ /* Initialize all port interrupt end point to port 0 int
+ * endpoint. Our device has only one interrupt end point
+ * common to all port */

[PATCH 64/70] tty-usb-oti6858: Coding style

diff --git a/drivers/usb/serial/oti6858.c b/drivers/usb/serial/oti6858.c
index 069d276..81db571 100644
--- a/drivers/usb/serial/oti6858.c
+++ b/drivers/usb/serial/oti6858.c
@@ -25,7 +25,8 @@
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License.
*
- * See Documentation/usb/usb-serial.txt for more information on using this driver
+ * See Documentation/usb/usb-serial.txt for more information on using this
+ * driver
*
* TODO:
* - implement correct flushing for ioctls and oti6858_close()
@@ -49,7 +50,7 @@
#include
#include
#include
-#include
+#include
#include "oti6858.h"

Re: [PATCH 00/70] tty updates proposed for 2.6.27

On Fri, Jun 20, 2008 at 08:57:55PM +0100, Alan Cox wrote:
> This patch series introduces the idea of a tty_port - a common structure
> that eventually all ports will have. Some drivers are migrated to make partial
> use of the tty port commonality and helpers but only a little.
>
> On the USB side the USB tty API is changed to fix a whole pile of races where
> tty->port->tty cannot be assumed to be tty (because of hangup/reopen). The
> USB drivers all get a spring clean and the entire pile have been dragged into
> CodingStyle near compliance (some cases where the checkpatch whines that
> make no sense have been ignored).
>
> Greg: At the end of this series there shouldn't be any white space issues
> although some earlier patches introduce them and later ones take them away

Ah, thanks for resending these, as they are primarily USB changes, any
objection for me to take these through my USB tree?

> Ah, thanks for resending these, as they are primarily USB changes, any
> objection for me to take these through my USB tree?

None at all. I've attached 23/70 which got eaten originally by a size
limit.

Alan

usb_serial: API all change

From: Alan Cox

USB serial likes to use port->tty back pointers for the real work it does and
to do so without any actual locking. Unfortunately when you consider hangup
events, hangup/parallel reopen or even worse hangup followed by parallel close
events the tty->port and port->tty pointers are not guaranteed to be the same
as port->tty is the active tty while tty->port is the port the tty may or
may not still be attached to.

So rework the entire API to pass the tty struct. For console cases we need
to pass both for now. This shows up multiple drivers that immediately crash
with USB console some of which have been fixed in the process.

/* we perform a CYPRESS_GET_CONFIG so that the current settings are
* filled into the private structure this should confirm that all is
* working if it returns what we just set */
- cypress_serial_control(port, 0, 0, 0, 0, 0, 0, CYPRESS_GET_CONFIG);
+ cypress_serial_control(tty, port, 0, 0, 0, 0, 0, 0, CYPRESS_GET_CONFIG);

/* Termios defaults are set by usb_serial_init. We don't change
- port->tty->termios - this would loose speed settings, etc.
+ tty->termios - this would loose speed settings, etc.
This is same behaviour as serial.c/rs_open() - Kuba */

-
-
/*
* The timer is currently only used to send queued packets to
* the tty in cases where the protocol provides no own handshaking
@@ -1526,7 +1509,7 @@ static void timeout_handler(unsigned long data)

// Since we have more credit, check if more data can be sent
send_more_port_data(edge_serial, edge_port);
@@ -760,7 +761,7 @@ static void edge_bulk_out_data_callback (struct urb *urb)
__func__, status);
}

/* see if we've set up our endpoint info yet (can't set it up in edge_startup
as the structures were not set up at that time.) */
serial = port->serial;
edge_serial = usb_get_serial_data(serial);
- if (edge_serial == NULL) {
+ if (edge_serial == NULL)
return -ENODEV;
- }
if (edge_serial->interrupt_in_buffer == NULL) {
struct usb_serial_port *port0 = serial->port[0];

/* send the current line settings to the port so we are in sync with any further termios calls */
- if (edge_port->port->tty)
- change_port_settings (edge_port, edge_port->port->tty->termios);
+ /* FIXME: locking on tty */
+ if (edge_port->port->port.tty)
+ change_port_settings(edge_port->port->port.tty, edge_port, edge_port->port->port.tty->termios);

- /* We should be aligned now -- can write max page size bytes at a time */
+ /* We should be aligned now -- can write
+ max page size bytes at a time */
while (length) {
if (length > EPROM_PAGE_SIZE)
write_length = EPROM_PAGE_SIZE;
else
write_length = length;

-static int TIChooseConfiguration (struct usb_device *dev)
+static int choose_config(struct usb_device *dev)
{
- // There may be multiple configurations on this device, in which case
- // we would need to read and parse all of them to find out which one
- // we want. However, we just support one config at this point,
- // configuration # 1, which is Config Descriptor 0.
+ /*
+ * There may be multiple configurations on this device, in which case
+ * we would need to read and parse all of them to find out which one
+ * we want. However, we just support one config at this point,
+ * configuration # 1, which is Config Descriptor 0.
+ */

- // In order to update the I2C firmware we must change the type 2 record to type 0xF2.
- // This will force the UMP to come up in Boot Mode. Then while in boot mode, the driver
- // will download the latest firmware (padded to 15.5k) into the UMP ram.
- // And finally when the device comes back up in download mode the driver will cause
- // the new firmware to be copied from the UMP Ram to I2C and the firmware will update
- // the record type from 0xf2 to 0x02.
-
- // Allocate a 15.5k buffer + 2 bytes for version number (Firmware Record)
- buffer_size = (((1024 * 16) - 512 )+ sizeof(struct ti_i2c_firmware_rec));
-
- buffer = kmalloc (buffer_size, GFP_KERNEL);
+ /*
+ * In order to update the I2C firmware we must change the type 2
+ * record to type 0xF2. This will force the UMP to come up in Boot
+ * Mode. Then while in boot mode, the driver will download the latest
+ * firmware (padded to 15.5k) into the UMP ram. Finally when the device
+ * comes back up in download mode the driver will cause the new
+ * firmware to be copied from the UMP Ram to I2C and the firmware
+ * will update the record type from 0xf2 to 0x02.
+ */
+
+ /* Allocate a 15.5k buffer + 2 bytes for version number
+ (Firmware Record) */
+ buffer_size = (((1024 * 16) - 512) +
+ sizeof(struct ti_i2c_firmware_rec));
+
+ buffer = kmalloc(buffer_size, GFP_KERNEL);
if (!buffer) {
- dev_err (dev, "%s - out of memory\n", __func__);
+ dev_err(dev, "%s - out of memory\n", __func__);
return -ENOMEM;
}
-
- // Set entire image of 0xffs
- memset (buffer, 0xff, buffer_size);

- // Setup initial mode -- the default mode 0 is TI_MODE_CONFIGURING
- // if we have more than one endpoint we are definitely in download mode
+ /*
+ * Setup initial mode -- the default mode 0 is TI_MODE_CONFIGURING
+ * if we have more than one endpoint we are definitely in download
+ * mode
+ */
if (interface->bNumEndpoints > 1)
serial->product_info.TiMode = TI_MODE_DOWNLOAD;
else
- // Otherwise we will remain in configuring mode
+ /* Otherwise we will remain in configuring mode */
serial->product_info.TiMode = TI_MODE_CONFIGURING;

- // Check if we have an old version in the I2C and update if necessary
+ /* Check if we have an old version in the I2C and
+ update if necessary */
if (download_cur_ver != download_new_ver) {
- dbg ("%s - Update I2C Download from %d.%d to %d.%d",
+ dbg("%s - Update I2C dld from %d.%d to %d.%d",
__func__,
firmware_version->Ver_Major,
firmware_version->Ver_Minor,
- OperationalCodeImageVersion.MajorVersion,
- OperationalCodeImageVersion.MinorVersion);
-
- // In order to update the I2C firmware we must change the type 2 record to type 0xF2.
- // This will force the UMP to come up in Boot Mode. Then while in boot mode, the driver
- // will download the latest firmware (padded to 15.5k) into the UMP ram.
- // And finally when the device comes back up in download mode the driver will cause
- // the new firmware to be copied from the UMP Ram to I2C and the firmware will update
- // the record type from 0xf2 to 0x02.
+ op_fw_version.MajorVersion,
+ op_fw_version.MinorVersion);
+
+ /* In order to update the I2C firmware we must
+ * change the type 2 record to type 0xF2. This
+ * will force the UMP to come up in Boot Mode.
+ * Then while in boot mode, the driver will
+ * download the latest firmware (padded to
+ * 15.5k) into the UMP ram. Finally when the
+ * device comes back up in download mode the
+ * driver will cause the new firmware to be
+ * copied from the UMP Ram to I2C and the
+ * firmware will update the record type from
+ * 0xf2 to 0x02.
+ */

- // verify the write -- must do this in order for write to
- // complete before we do the hardware reset
- status = TIReadRom (serial,
+ /* verify the write -- must do this in order
+ * for write to complete before we do the
+ * hardware reset
+ */
+ status = read_rom(serial,
start_address,
sizeof(record),
&record);

- // verify the write -- must do this in order for write to
- // complete before we do the hardware reset
- status = TIReadRom (serial,
- start_address,
- HEADER_SIZE,
- vheader);
+ /* verify the write -- must do this in order for
+ write to complete before we do the hardware reset */
+ status = read_rom(serial, start_address,
+ HEADER_SIZE, vheader);

- // Configure the TI device so we can use the BULK pipes for download
- status = TIConfigureBootDevice (serial->serial->dev);
+ /* Configure the TI device so we can use the BULK pipes for download */
+ status = config_boot_dev(serial->serial->dev);
if (status)
return status;

- // In order to update the I2C firmware we must change the type 2 record to type 0xF2.
- // This will force the UMP to come up in Boot Mode. Then while in boot mode, the driver
- // will download the latest firmware (padded to 15.5k) into the UMP ram.
- // And finally when the device comes back up in download mode the driver will cause
- // the new firmware to be copied from the UMP Ram to I2C and the firmware will update
- // the record type from 0xf2 to 0x02.
-
/*
+ * In order to update the I2C firmware we must change the type
+ * 2 record to type 0xF2. This will force the UMP to come up
+ * in Boot Mode. Then while in boot mode, the driver will
+ * download the latest firmware (padded to 15.5k) into the
+ * UMP ram. Finally when the device comes back up in download
+ * mode the driver will cause the new firmware to be copied
+ * from the UMP Ram to I2C and the firmware will update the
+ * record type from 0xf2 to 0x02.
+ *
* Do we really have to copy the whole firmware image,
* or could we do this in place!
*/

/* wakeup any process waiting for writes to complete */
/* there is now more room in the buffer for new writes */
- if (tty) {
- /* let the tty driver wakeup if it has a special write_wakeup function */
+ if (tty)
tty_wakeup(tty);
- }
}

- /* if we are implementing XON/XOFF, set the start and stop character in the device */
- if (I_IXOFF(tty) || I_IXON(tty)) {
- config->cXon = START_CHAR(tty);
- config->cXoff = STOP_CHAR(tty);
+ /* if we are implementing XON/XOFF, set the start and stop
+ character in the device */
+ config->cXon = START_CHAR(tty);
+ config->cXoff = STOP_CHAR(tty);

- if (port->tty)
- port->tty->low_latency = 1;
+ if (tty)
+ tty->low_latency = 1;
/* Check to see if we've set up our endpoint info yet *
* (can't set it up in mos7840_startup as the structures *
* were not set up at that time.) */
@@ -1104,11 +1105,12 @@ static int mos7840_open(struct usb_serial_port *port, struct file *filp)
* been written, but hasn't made it out the port yet)
* If successful, we return the number of bytes left to be written in the
* system,
- * Otherwise we return a negative error number.
+ * Otherwise we return zero.
************************************************** ***************************/

- /* open_count is managed under the mutex lock for the tty so cannot
+ /* count is managed under the mutex lock for the tty so cannot
drop to zero until after the last close completes */
- WARN_ON(!port->open_count);
+ WARN_ON(!port->port.count);

Re: [PATCH 00/70] tty updates proposed for 2.6.27

Hi Greg,
> > This patch series introduces the idea of a tty_port - a common structure
> > that eventually all ports will have. Some drivers are migrated to make partial
> > use of the tty port commonality and helpers but only a little.
> >
> > On the USB side the USB tty API is changed to fix a whole pile of races where
> > tty->port->tty cannot be assumed to be tty (because of hangup/reopen). The
> > USB drivers all get a spring clean and the entire pile have been dragged into
> > CodingStyle near compliance (some cases where the checkpatch whines that
> > make no sense have been ignored).
> >
> > Greg: At the end of this series there shouldn't be any white space issues
> > although some earlier patches introduce them and later ones take them away
>
> Ah, thanks for resending these, as they are primarily USB changes, any
> objection for me to take these through my USB tree?

for everything that Alan touches in the Bluetooth drivers, you have an
ack from me. I am happy if this goes in quickly and I only have to
re-base my tree once.

On Sun, Jun 22, 2008 at 06:07:56PM +0100, Alan Cox wrote:
> > The USB_SERIAL_TI_FIRMWARE bits don't seem to belong here?
>
> They seem to have gotten merged together by accident - shouldn't do any
> harm and both want doing. I can take a look at splitting it if Greg
> wants ?

I'll look at them on Monday when I'm going through applying the series
and see if it really matters or not

Re: [PATCH 01/70] tty: Ldisc revamp

On Fri, Jun 20, 2008 at 08:58:05PM +0100, Alan Cox wrote:
> From: Alan Cox
>
> Move the line disciplines towards a conventional ->ops arrangement. For the
> moment the actual 'tty_ldisc' struct in the tty is kept as part of the tty
> struct but this can then be changed if it turns out that when it all settles
> down we want to refcount ldiscs separately to the tty.
>
> Pull the ldisc code out of /proc and put it with our ldisc code.
>

[...]
> @@ -1386,7 +1467,7 @@ static void tty_reset_termios(struct tty_struct *tty)
> * do_tty_hangup - actual handler for hangup events
> * @work: tty device
> *
> - * This can be called by the "eventd" kernel thread. That is process
> +k * This can be called by the "eventd" kernel thread. That is process
> * synchronous but doesn't hold any locks, so we need to make sure we
> * have the appropriate locks for what we're doing.
> *

Sorry for such a trivial comment, but the leading 'k' above looks like a typo.

Re: [PATCH 01/70] tty: Ldisc revamp

> > - * This can be called by the "eventd" kernel thread. That is process
> > +k * This can be called by the "eventd" kernel thread. That is process
> > * synchronous but doesn't hold any locks, so we need to make sure we
> > * have the appropriate locks for what we're doing.
> > *
>
> Sorry for such a trivial comment, but the leading 'k' above looks like a typo.