On Mon, 2015-06-22 at 12:01 +0200, Johan Hovold wrote:
> On Thu, Jun 18, 2015 at 06:43:36AM -0500, Peter E. Berger wrote:
> > From: "Peter E. Berger" <pberger@xxxxxxxxxxx>
> >
> > When using newer Edgeport devices such as the EP/416, idle ports are
> > automatically bounced (disconnected and then reconnected) approximately
> > every 60 seconds. This breaks programs (e.g: minicom) where idle periods
> > are common, normal and expected.
> >
> > I confirmed with the manufacturer (Digi International) that some Edgeports
> > now ship from the factory with firmware that expects periodic "heartbeat"
> > queries from the driver to keep idle ports alive. This patch implements
> > heartbeat support using the mechanism Digi suggested (periodically
> > requesting an I2C descriptor address) that appears effective on Edgeports
> > running the newer firmware (that require it) and benign on Edgeport
> > devices running older firmware. Since we know that Edgeport firmware
> > version 4.80 (the version distributed in /lib/firmware/down3.bin and used
> > for Edgeports that are either running still older versions or have no
> > onboard non-volatile firmware image) does not require heartbeat support,
> > this patch schedules heartbeats only on devices running firmware versions
> > newer than 4.80.
> >
> > Signed-off-by: Peter E. Berger <pberger@xxxxxxxxxxx>
> > ---
> > drivers/usb/serial/io_ti.c | 78 +++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 77 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
> > index c76820b..b7c1ea1 100644
> > --- a/drivers/usb/serial/io_ti.c
> > +++ b/drivers/usb/serial/io_ti.c
> > @@ -101,6 +101,7 @@ struct edgeport_serial {
> > struct mutex es_lock;
> > int num_ports_open;
> > struct usb_serial *serial;
> > + struct delayed_work heartbeat_work;
> > int fw_version;
> > };
> >
> > @@ -206,6 +207,18 @@ static void edge_send(struct usb_serial_port *port, struct tty_struct *tty);
> > static int edge_create_sysfs_attrs(struct usb_serial_port *port);
> > static int edge_remove_sysfs_attrs(struct usb_serial_port *port);
> >
> > +/*
> > + * Some release of Edgeport firmware "down3.bin" after version 4.80
> > + * introduced code to automatically disconnect idle devices after periods
> > + * of inactivity, typically ~60 seconds. This occurs without regard to
> > + * whether ports on the device are open or not. Digi International Tech
> > + * Support suggested adding driver "heartbeat" code to reset the firmware
> > + * timer by requesting a descriptor record every 15 seconds, which should be
> > + * effective with newer firmware versions that require it, and benign with
> > + * older versions that do not. In practice 40 seconds seems often enough.
> > + */
> > +#define FW_HEARTBEAT_VERSION_CUTOFF ((4 << 8) + 80)
> > +#define FW_HEARTBEAT_SECS 40
> >
> > /* Timeouts in msecs: firmware downloads take longer */
> > #define TI_VSEND_TIMEOUT_DEFAULT 1000
> > @@ -2354,6 +2367,36 @@ static void edge_break(struct tty_struct *tty, int break_state)
> > __func__, status);
> > }
> >
> > +static void edge_heartbeat_work(struct work_struct *work)
> > +{
> > + struct edgeport_serial *serial;
> > + struct ti_i2c_desc *rom_desc;
> > +
> > + serial = container_of(work, struct edgeport_serial,
> > + heartbeat_work.work);
> > +
> > + rom_desc = kmalloc(sizeof(*rom_desc), GFP_KERNEL);
> > +
> > + /* Descriptor address request is enough to reset the firmware timer */
> > + if (!rom_desc || !get_descriptor_addr(serial, I2C_DESC_TYPE_ION,
> > + rom_desc)) {
> > + dev_err(&serial->serial->interface->dev,
> > + "%s - Incomplete heartbeat\n", __func__);
> > + }
> > + kfree(rom_desc);
> > +
> > + schedule_delayed_work(&serial->heartbeat_work, FW_HEARTBEAT_SECS * HZ);
>
> Use the helper here as well.
Done.
>
> > +}
> > +
> > +static inline void edge_heartbeat_reschedule(struct edgeport_serial
> > + *edge_serial)
>
> Odd line break, rename edge_serial or break after void instead if needed.
Done.
>
> Perhaps just edge_heartbeat_schedule would have been a better name.
Done.
>
> > +{
> > + if (edge_serial->fw_version > FW_HEARTBEAT_VERSION_CUTOFF) {
>
> Return if <= cutoff instead.
Done.
I'm zeroing in on a v7 patchset, which I hope to have ready tomorrow or
so.
Thanks.
--Peter
>
> > + schedule_delayed_work(&edge_serial->heartbeat_work,
> > + FW_HEARTBEAT_SECS * HZ);
> > + }
> > +}
>
> Looks good otherwise.
>
> Thanks,
> Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
