Re: [PATCH v6 3/3] USB: io_ti: Add heartbeat to keep idle Edgeport ports from disconnecting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> +}
> +
> +static inline void edge_heartbeat_reschedule(struct edgeport_serial
> +		*edge_serial)

Odd line break, rename edge_serial or break after void instead if needed.

Perhaps just edge_heartbeat_schedule would have been a better name.

> +{
> +	if (edge_serial->fw_version > FW_HEARTBEAT_VERSION_CUTOFF) {

Return if <= cutoff instead.

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux