[PATCH] synclink_gt add gpio feature

[PATCH] synclink_gt add gpio feature

[Paul Fulghum]

Add driver support for general purpose I/O feature
of the Synclink GT adapters.

Signed-off-by: Paul Fulghum <paulkf@micrgate.com>

--- linux-2.6.16/drivers/char/synclink_gt.c	2006-03-19 23:53:29.000000000 -0600
+++ b/drivers/char/synclink_gt.c	2006-03-24 09:43:42.000000000 -0600
@@ -1,5 +1,5 @@
 /*
- * $Id: synclink_gt.c,v 4.22 2006/01/09 20:16:06 paulkf Exp $
+ * $Id: synclink_gt.c,v 4.25 2006/02/06 21:20:33 paulkf Exp $
  *
  * Device driver for Microgate SyncLink GT serial adapters.
  *
@@ -92,7 +92,7 @@
  * module identification
  */
 static char *driver_name     = "SyncLink GT";
-static char *driver_version  = "$Revision: 4.22 $";
+static char *driver_version  = "$Revision: 4.25 $";
 static char *tty_driver_name = "synclink_gt";
 static char *tty_dev_prefix  = "ttySLG";
 MODULE_LICENSE("GPL");
@@ -188,6 +188,20 @@ static void hdlcdev_exit(struct slgt_inf
 #define SLGT_REG_SIZE  256
 
 /*
+ * conditional wait facility
+ */
+struct cond_wait {
+	struct cond_wait *next;
+	wait_queue_head_t q;
+	wait_queue_t wait;
+	unsigned int data;
+};
+static void init_cond_wait(struct cond_wait *w, unsigned int data);
+static void add_cond_wait(struct cond_wait **head, struct cond_wait *w);
+static void remove_cond_wait(struct cond_wait **head, struct cond_wait *w);
+static void flush_cond_wait(struct cond_wait **head);
+
+/*
  * DMA buffer descriptor and access macros
  */
 struct slgt_desc
@@ -269,6 +283,9 @@ struct slgt_info {
 	struct timer_list	tx_timer;
 	struct timer_list	rx_timer;
 
+	unsigned int            gpio_present;
+	struct cond_wait        *gpio_wait_q;
+
 	spinlock_t lock;	/* spinlock for synchronizing with ISR */
 
 	struct work_struct task;
@@ -379,6 +396,11 @@ static MGSL_PARAMS default_params = {
 #define MASK_OVERRUN BIT4
 
 #define GSR   0x00 /* global status */
+#define JCR   0x04 /* JTAG control */
+#define IODR  0x08 /* GPIO direction */
+#define IOER  0x0c /* GPIO interrupt enable */
+#define IOVR  0x10 /* GPIO value */
+#define IOSR  0x14 /* GPIO interrupt status */
 #define TDR   0x80 /* tx data */
 #define RDR   0x80 /* rx data */
 #define TCR   0x82 /* tx control */
@@ -503,6 +525,9 @@ static int  tiocmset(struct tty_struct *
 static void set_break(struct tty_struct *tty, int break_state);
 static int  get_interface(struct slgt_info *info, int __user *if_mode);
 static int  set_interface(struct slgt_info *info, int if_mode);
+static int  set_gpio(struct slgt_info *info, struct gpio_desc __user *gpio);
+static int  get_gpio(struct slgt_info *info, struct gpio_desc __user *gpio);
+static int  wait_gpio(struct slgt_info *info, struct gpio_desc __user *gpio);
 
 /*
  * driver functions
@@ -1112,6 +1137,12 @@ static int ioctl(struct tty_struct *tty,
 		return get_interface(info, argp);
 	case MGSL_IOCSIF:
 		return set_interface(info,(int)arg);
+	case MGSL_IOCSGPIO:
+		return set_gpio(info, argp);
+	case MGSL_IOCGGPIO:
+		return get_gpio(info, argp);
+	case MGSL_IOCWAITGPIO:
+		return wait_gpio(info, argp);
 	case TIOCGICOUNT:
 		spin_lock_irqsave(&info->lock,flags);
 		cnow = info->icount;
@@ -2158,6 +2189,24 @@ static void isr_txeom(struct slgt_info *
 	}
 }
 
+static void isr_gpio(struct slgt_info *info, unsigned int changed, unsigned int state)
+{
+	struct cond_wait *w, *prev;
+
+	/* wake processes waiting for specific transitions */
+	for (w = info->gpio_wait_q, prev = NULL ; w != NULL ; w = w->next) {
+		if (w->data & changed) {
+			w->data = state;
+			wake_up_interruptible(&w->q);
+			if (prev != NULL)
+				prev->next = w->next;
+			else
+				info->gpio_wait_q = w->next;
+		} else
+			prev = w;
+	}
+}
+
 /* interrupt service routine
  *
  * 	irq	interrupt number
@@ -2193,6 +2242,22 @@ static irqreturn_t slgt_interrupt(int ir
 		}
 	}
 
+	if (info->gpio_present) {
+		unsigned int state;
+		unsigned int changed;
+		while ((changed = rd_reg32(info, IOSR)) != 0) {
+			DBGISR(("%s iosr=%08x\n", info->device_name, changed));
+			/* read latched state of GPIO signals */
+			state = rd_reg32(info, IOVR);
+			/* clear pending GPIO interrupt bits */
+			wr_reg32(info, IOSR, changed);
+			for (i=0 ; i < info->port_count ; i++) {
+				if (info->port_array[i] != NULL)
+					isr_gpio(info->port_array[i], changed, state);
+			}
+		}
+	}
+
 	for(i=0; i < info->port_count ; i++) {
 		struct slgt_info *port = info->port_array[i];
 
@@ -2276,6 +2341,8 @@ static void shutdown(struct slgt_info *i
 		set_signals(info);
 	}
 
+	flush_cond_wait(&info->gpio_wait_q);
+
 	spin_unlock_irqrestore(&info->lock,flags);
 
 	if (info->tty)
@@ -2650,6 +2717,175 @@ static int set_interface(struct slgt_inf
 	return 0;
 }
 
+/*
+ * set general purpose IO pin state and direction
+ *
+ * user_gpio fields:
+ * state   each bit indicates a pin state
+ * smask   set bit indicates pin state to set
+ * dir     each bit indicates a pin direction (0=input, 1=output)
+ * dmask   set bit indicates pin direction to set
+ */
+static int set_gpio(struct slgt_info *info, struct gpio_desc __user *user_gpio)
+{
+ 	unsigned long flags;
+	struct gpio_desc gpio;
+	__u32 data;
+
+	if (!info->gpio_present)
+		return -EINVAL;
+	if (copy_from_user(&gpio, user_gpio, sizeof(gpio)))
+		return -EFAULT;
+	DBGINFO(("%s set_gpio state=%08x smask=%08x dir=%08x dmask=%08x\n",
+		 info->device_name, gpio.state, gpio.smask,
+		 gpio.dir, gpio.dmask));
+
+	spin_lock_irqsave(&info->lock,flags);
+	if (gpio.dmask) {
+		data = rd_reg32(info, IODR);
+		data |= gpio.dmask & gpio.dir;
+		data &= ~(gpio.dmask & ~gpio.dir);
+		wr_reg32(info, IODR, data);
+	}
+	if (gpio.smask) {
+		data = rd_reg32(info, IOVR);
+		data |= gpio.smask & gpio.state;
+		data &= ~(gpio.smask & ~gpio.state);
+		wr_reg32(info, IOVR, data);
+	}
+	spin_unlock_irqrestore(&info->lock,flags);
+
+	return 0;
+}
+
+/*
+ * get general purpose IO pin state and direction
+ */
+static int get_gpio(struct slgt_info *info, struct gpio_desc __user *user_gpio)
+{
+	struct gpio_desc gpio;
+	if (!info->gpio_present)
+		return -EINVAL;
+	gpio.state = rd_reg32(info, IOVR);
+	gpio.smask = 0xffffffff;
+	gpio.dir   = rd_reg32(info, IODR);
+	gpio.dmask = 0xffffffff;
+	if (copy_to_user(user_gpio, &gpio, sizeof(gpio)))
+		return -EFAULT;
+	DBGINFO(("%s get_gpio state=%08x dir=%08x\n",
+		 info->device_name, gpio.state, gpio.dir));
+	return 0;
+}
+
+/*
+ * conditional wait facility
+ */
+static void init_cond_wait(struct cond_wait *w, unsigned int data)
+{
+	init_waitqueue_head(&w->q);
+	init_waitqueue_entry(&w->wait, current);
+	w->data = data;
+}
+
+static void add_cond_wait(struct cond_wait **head, struct cond_wait *w)
+{
+	set_current_state(TASK_INTERRUPTIBLE);
+	add_wait_queue(&w->q, &w->wait);
+	w->next = *head;
+	*head = w;
+}
+
+static void remove_cond_wait(struct cond_wait **head, struct cond_wait *cw)
+{
+	struct cond_wait *w, *prev;
+	remove_wait_queue(&cw->q, &cw->wait);
+	set_current_state(TASK_RUNNING);
+	for (w = *head, prev = NULL ; w != NULL ; prev = w, w = w->next) {
+		if (w == cw) {
+			if (prev != NULL)
+				prev->next = w->next;
+			else
+				*head = w->next;
+			break;
+		}
+	}
+}
+
+static void flush_cond_wait(struct cond_wait **head)
+{
+	while (*head != NULL) {
+		wake_up_interruptible(&(*head)->q);
+		*head = (*head)->next;
+	}
+}
+
+/*
+ * wait for general purpose I/O pin(s) to enter specified state
+ *
+ * user_gpio fields:
+ * state - bit indicates target pin state
+ * smask - set bit indicates watched pin
+ *
+ * The wait ends when at least one watched pin enters the specified
+ * state. When 0 (no error) is returned, user_gpio->state is set to the
+ * state of all GPIO pins when the wait ends.
+ *
+ * Note: Each pin may be a dedicated input, dedicated output, or
+ * configurable input/output. The number and configuration of pins
+ * varies with the specific adapter model. Only input pins (dedicated
+ * or configured) can be monitored with this function.
+ */
+static int wait_gpio(struct slgt_info *info, struct gpio_desc __user *user_gpio)
+{
+ 	unsigned long flags;
+	int rc = 0;
+	struct gpio_desc gpio;
+	struct cond_wait wait;
+	u32 state;
+
+	if (!info->gpio_present)
+		return -EINVAL;
+	if (copy_from_user(&gpio, user_gpio, sizeof(gpio)))
+		return -EFAULT;
+	DBGINFO(("%s wait_gpio() state=%08x smask=%08x\n",
+		 info->device_name, gpio.state, gpio.smask));
+	/* ignore output pins identified by set IODR bit */
+	if ((gpio.smask &= ~rd_reg32(info, IODR)) == 0)
+		return -EINVAL;
+	init_cond_wait(&wait, gpio.smask);
+
+	spin_lock_irqsave(&info->lock, flags);
+	/* enable interrupts for watched pins */
+	wr_reg32(info, IOER, rd_reg32(info, IOER) | gpio.smask);
+	/* get current pin states */
+	state = rd_reg32(info, IOVR);
+
+	if (gpio.smask & ~(state ^ gpio.state)) {
+		/* already in target state */
+		gpio.state = state;
+	} else {
+		/* wait for target state */
+		add_cond_wait(&info->gpio_wait_q, &wait);
+		spin_unlock_irqrestore(&info->lock, flags);
+		schedule();
+		if (signal_pending(current))
+			rc = -ERESTARTSYS;
+		else
+			gpio.state = wait.data;
+		spin_lock_irqsave(&info->lock, flags);
+		remove_cond_wait(&info->gpio_wait_q, &wait);
+	}
+
+	/* disable all GPIO interrupts if no waiting processes */
+	if (info->gpio_wait_q == NULL)
+		wr_reg32(info, IOER, 0);
+	spin_unlock_irqrestore(&info->lock,flags);
+
+	if ((rc == 0) && copy_to_user(user_gpio, &gpio, sizeof(gpio)))
+		rc = -EFAULT;
+	return rc;
+}
+
 static int modem_input_wait(struct slgt_info *info,int arg)
 {
  	unsigned long flags;
@@ -3166,8 +3402,10 @@ static void device_init(int adapter_num,
 		} else {
 			port_array[0]->irq_requested = 1;
 			adapter_test(port_array[0]);
-			for (i=1 ; i < port_count ; i++)
+			for (i=1 ; i < port_count ; i++) {
 				port_array[i]->init_error = port_array[0]->init_error;
+				port_array[i]->gpio_present = port_array[0]->gpio_present;
+			}
 		}
 	}
 }
@@ -4301,7 +4539,7 @@ static int register_test(struct slgt_inf
 			break;
 		}
 	}
-
+	info->gpio_present = (rd_reg32(info, JCR) & BIT5) ? 1 : 0;
 	info->init_error = rc ? 0 : DiagStatus_AddressFailure;
 	return rc;
 }
--- linux-2.6.16/include/linux/synclink.h	2006-03-19 23:53:29.000000000 -0600
+++ b/include/linux/synclink.h	2006-03-24 09:52:55.000000000 -0600
@@ -1,7 +1,7 @@
 /*
  * SyncLink Multiprotocol Serial Adapter Driver
  *
- * $Id: synclink.h,v 3.10 2005/11/08 19:50:54 paulkf Exp $
+ * $Id: synclink.h,v 3.11 2006/02/06 21:20:29 paulkf Exp $
  *
  * Copyright (C) 1998-2000 by Microgate Corporation
  *
@@ -221,6 +221,12 @@ struct mgsl_icount {
 	__u32	rxidle;
 };
 
+struct gpio_desc {
+	__u32 state;
+	__u32 smask;
+	__u32 dir;
+	__u32 dmask;
+};
 
 #define DEBUG_LEVEL_DATA	1
 #define DEBUG_LEVEL_ERROR 	2
@@ -276,5 +282,8 @@ struct mgsl_icount {
 #define MGSL_IOCLOOPTXDONE	_IO(MGSL_MAGIC_IOC,9)
 #define MGSL_IOCSIF		_IO(MGSL_MAGIC_IOC,10)
 #define MGSL_IOCGIF		_IO(MGSL_MAGIC_IOC,11)
+#define MGSL_IOCSGPIO		_IOW(MGSL_MAGIC_IOC,16,struct gpio_desc)
+#define MGSL_IOCGGPIO		_IOR(MGSL_MAGIC_IOC,17,struct gpio_desc)
+#define MGSL_IOCWAITGPIO	_IOWR(MGSL_MAGIC_IOC,18,struct gpio_desc)
 
 #endif /* _SYNCLINK_H_ */

Re: [PATCH] synclink_gt add gpio feature

[Andrew Morton]

Paul Fulghum <paulkf@microgate.com> wrote:
>
> Add driver support for general purpose I/O feature
> of the Synclink GT adapters.
> 
>  
> ...
>
>  /*
> + * conditional wait facility
> + */
> +struct cond_wait {
> +	struct cond_wait *next;
> +	wait_queue_head_t q;
> +	wait_queue_t wait;
> +	unsigned int data;
> +};
> +static void init_cond_wait(struct cond_wait *w, unsigned int data);
> +static void add_cond_wait(struct cond_wait **head, struct cond_wait *w);
> +static void remove_cond_wait(struct cond_wait **head, struct cond_wait *w);
> +static void flush_cond_wait(struct cond_wait **head);

Adding new generic-looking infrastructure into a driver is a worry.  Either
we're missing some facility, or the driver is doing something unnecessary
or the driver's requirements are unique.

Tell use more about conditional waits?

> ...
> +static int wait_gpio(struct slgt_info *info, struct gpio_desc __user *user_gpio)
> +{
> + 	unsigned long flags;
> +	int rc = 0;
> +	struct gpio_desc gpio;
> +	struct cond_wait wait;
> +	u32 state;
> +
> +	if (!info->gpio_present)
> +		return -EINVAL;
> +	if (copy_from_user(&gpio, user_gpio, sizeof(gpio)))
> +		return -EFAULT;
> +	DBGINFO(("%s wait_gpio() state=%08x smask=%08x\n",
> +		 info->device_name, gpio.state, gpio.smask));
> +	/* ignore output pins identified by set IODR bit */
> +	if ((gpio.smask &= ~rd_reg32(info, IODR)) == 0)
> +		return -EINVAL;
> +	init_cond_wait(&wait, gpio.smask);
> +
> +	spin_lock_irqsave(&info->lock, flags);
> +	/* enable interrupts for watched pins */
> +	wr_reg32(info, IOER, rd_reg32(info, IOER) | gpio.smask);
> +	/* get current pin states */
> +	state = rd_reg32(info, IOVR);
> +
> +	if (gpio.smask & ~(state ^ gpio.state)) {
> +		/* already in target state */
> +		gpio.state = state;
> +	} else {
> +		/* wait for target state */
> +		add_cond_wait(&info->gpio_wait_q, &wait);
> +		spin_unlock_irqrestore(&info->lock, flags);
> +		schedule();
> +		if (signal_pending(current))
> +			rc = -ERESTARTSYS;
> +		else
> +			gpio.state = wait.data;
> +		spin_lock_irqsave(&info->lock, flags);
> +		remove_cond_wait(&info->gpio_wait_q, &wait);
> +	}
> +
> +	/* disable all GPIO interrupts if no waiting processes */
> +	if (info->gpio_wait_q == NULL)
> +		wr_reg32(info, IOER, 0);

Should we be dong that write if rc!=0?  I guess so..

> +	spin_unlock_irqrestore(&info->lock,flags);
> +
> +	if ((rc == 0) && copy_to_user(user_gpio, &gpio, sizeof(gpio)))
> +		rc = -EFAULT;
> +	return rc;
> +}

Re: [PATCH] synclink_gt add gpio feature

[Paul Fulghum]

Andrew Morton wrote:
> Adding new generic-looking infrastructure into a driver is a worry.  Either
> we're missing some facility, or the driver is doing something unnecessary
> or the driver's requirements are unique.
> 
> Tell use more about conditional waits?

I needed a way to wake only processes waiting for specific
GPIO transitions out of 32 signals, with 2 possible transitions
per signal (up/down). I also need to return the state of all signals
to each waiter as exists at the time the specific transition occurs.
This has to be done in the ISR as that state is lost when
the interrupt is cleared. So I implemented a wrapper around
the existing wait code that allows waking only processes waiting
for specific transitions and passing the associated state back
to each woken process.

I could use the existing wait infrastructure and wake
all threads waiting on any GPIO transition. That could
cause a lot of unnecessary waking/sleeping. I would also still
need to implement some way to relay the associated state for
each individual transition to the correct waiter.

I could implement a separate normal wait queue for each
transition type (64 queues), but that seems excessive.

The wrapper seems to be the minimal and most efficient
way of implementing this. Maybe I missed some existing
infrastructure that implements the same features?

>>+	/* disable all GPIO interrupts if no waiting processes */
>>+	if (info->gpio_wait_q == NULL)
>>+		wr_reg32(info, IOER, 0);
> 
> 
> Should we be dong that write if rc!=0?  I guess so..

Yes, if for what ever reason (error or otherwise) there are no waiters
there is no point in leaving the GPIO interrupts enabled.
Interrupts will be reenabled as necessary on reentry.

--
Paul

Re: [PATCH] synclink_gt add gpio feature

[Andrew Morton]

Paul Fulghum <paulkf@microgate.com> wrote:
>
> Andrew Morton wrote:
> > Adding new generic-looking infrastructure into a driver is a worry.  Either
> > we're missing some facility, or the driver is doing something unnecessary
> > or the driver's requirements are unique.
> > 
> > Tell use more about conditional waits?
> 
> I needed a way to wake only processes waiting for specific
> GPIO transitions out of 32 signals, with 2 possible transitions
> per signal (up/down). I also need to return the state of all signals
> to each waiter as exists at the time the specific transition occurs.
> This has to be done in the ISR as that state is lost when
> the interrupt is cleared. So I implemented a wrapper around
> the existing wait code that allows waking only processes waiting
> for specific transitions and passing the associated state back
> to each woken process.
> 
> I could use the existing wait infrastructure and wake
> all threads waiting on any GPIO transition. That could
> cause a lot of unnecessary waking/sleeping. I would also still
> need to implement some way to relay the associated state for
> each individual transition to the correct waiter.
> 
> I could implement a separate normal wait queue for each
> transition type (64 queues), but that seems excessive.

OIC.

> The wrapper seems to be the minimal and most efficient
> way of implementing this. Maybe I missed some existing
> infrastructure that implements the same features?

wait_on_bit()/wake_up_bit() might be usable?

Re: [PATCH] synclink_gt add gpio feature

[Paul Fulghum]

On Fri, 2006-03-24 at 15:12 -0800, Andrew Morton wrote:
> Paul Fulghum <paulkf@microgate.com> wrote:
> > The wrapper seems to be the minimal and most efficient
> > way of implementing this. Maybe I missed some existing
> > infrastructure that implements the same features?
> 
> wait_on_bit()/wake_up_bit() might be usable?

Interesting, I had not noticed this previously.
There are similarities to what I am doing.

The difference is that a conditional wait allows a caller
to wait for one or more 'bits' (in my specific case one
or more signal transitions) with a single wait call.
The caller wakes when at least one of the conditions is met.

wake_up_bit does not directly allow communicating state back
to the waiter, because the only state the waiter is interested
in is defined in the wait_on_bit call.

A concise description of the conditional wait is that each
wait queue implements an event type where additional wake
discrimination is provided by an arbitrary input data field.
The same data field allows arbitrary per waiter state to be passed back
to the waiter. Interpretation of the input and output data
is specific to an event type and the associated waiter/waker code.

In my case I use this facility to allow a waiter to say:
wait for one or more specific GPIO transitions
and tell me the state of all GPIO when at least
one of the specified transitions occur.

--
Paul

Re: [PATCH] synclink_gt add gpio feature

[Andrew Morton]

Paul Fulghum <paulkf@microgate.com> wrote:
>
> On Fri, 2006-03-24 at 15:12 -0800, Andrew Morton wrote:
> > Paul Fulghum <paulkf@microgate.com> wrote:
> > > The wrapper seems to be the minimal and most efficient
> > > way of implementing this. Maybe I missed some existing
> > > infrastructure that implements the same features?
> > 
> > wait_on_bit()/wake_up_bit() might be usable?
> 
> Interesting, I had not noticed this previously.
> There are similarities to what I am doing.
> 
> The difference is that a conditional wait allows a caller
> to wait for one or more 'bits' (in my specific case one
> or more signal transitions) with a single wait call.
> The caller wakes when at least one of the conditions is met.
> 
> wake_up_bit does not directly allow communicating state back
> to the waiter, because the only state the waiter is interested
> in is defined in the wait_on_bit call.
> 
> A concise description of the conditional wait is that each
> wait queue implements an event type where additional wake
> discrimination is provided by an arbitrary input data field.
> The same data field allows arbitrary per waiter state to be passed back
> to the waiter. Interpretation of the input and output data
> is specific to an event type and the associated waiter/waker code.
> 
> In my case I use this facility to allow a waiter to say:
> wait for one or more specific GPIO transitions
> and tell me the state of all GPIO when at least
> one of the specified transitions occur.
> 

It should be possible to communicate between waker and waiter via
__wait_queue.private and __wait_queue.func.  Make ->private point at some
on-stack thing, let the waker read and write that.

That'd involve some rather low-level poking at waitqueues, but I don't
expect those facilities are going away.

Re: [PATCH] synclink_gt add gpio feature

[Paul Fulghum]

On Fri, 2006-03-24 at 16:13 -0800, Andrew Morton wrote:
> It should be possible to communicate between waker and waiter via
> __wait_queue.private and __wait_queue.func.  Make ->private point at some
> on-stack thing, let the waker read and write that.
> 
> That'd involve some rather low-level poking at waitqueues, but I don't
> expect those facilities are going away.

__wait_queue.private already holds a
pointer to the task structure of the waiting process

I might be able to implement what I need in a way
that more closely resembles how wait_on_bit extends
the standard wait queue. But the result is the same:
a new wrapper (new structure containing wait_queue_t
and access/init functions) built on top of the
existing wait queue.

I'll revisit this tomorrow to make sure I'm
thinking about this correctly.

--
Paul