lkml.org 
[lkml]   [2006]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: ucb1x00 audio & zaurus touchscreen
On Thu, Mar 23, 2006 at 02:40:48PM +0000, David Vrabel wrote:
> @@ -58,9 +65,9 @@
> spin_lock_irqsave(&ucb->io_lock, flags);
> ucb->io_dir |= out;
> ucb->io_dir &= ~in;
> + spin_unlock_irqrestore(&ucb->io_lock, flags);
>
> ucb1x00_reg_write(ucb, UCB_IO_DIR, ucb->io_dir);
> - spin_unlock_irqrestore(&ucb->io_lock, flags);

Racy.

> @@ -86,9 +93,9 @@
> spin_lock_irqsave(&ucb->io_lock, flags);
> ucb->io_out |= set;
> ucb->io_out &= ~clear;
> + spin_unlock_irqrestore(&ucb->io_lock, flags);
>
> ucb1x00_reg_write(ucb, UCB_IO_DATA, ucb->io_out);
> - spin_unlock_irqrestore(&ucb->io_lock, flags);

Racy.

> @@ -301,22 +305,23 @@
> */
> void ucb1x00_disable_irq(struct ucb1x00 *ucb, unsigned int idx, int edges)
> {
> - unsigned long flags;
> -
> if (idx < 16) {
> - spin_lock_irqsave(&ucb->lock, flags);
> -
> - ucb1x00_enable(ucb);
> - if (edges & UCB_RISING) {
> + down(&ucb->lock);
> + /* This can't be right. Can it? */
> + if (edges & UCB_RISING)
> + ucb->irq_ris_enbl |= 1 << idx;
> + if (edges & UCB_FALLING)
> + ucb->irq_fal_enbl |= 1 << idx;
> + if (edges & UCB_RISING)
> ucb->irq_ris_enbl &= ~(1 << idx);
> - ucb1x00_reg_write(ucb, UCB_IE_RIS, ucb->irq_ris_enbl);
> - }
> - if (edges & UCB_FALLING) {
> + if (edges & UCB_FALLING)
> ucb->irq_fal_enbl &= ~(1 << idx);
> - ucb1x00_reg_write(ucb, UCB_IE_FAL, ucb->irq_fal_enbl);
> - }
> + up(&ucb->lock);
> +
> + ucb1x00_enable(ucb);
> + ucb1x00_reg_write(ucb, UCB_IE_RIS, ucb->irq_ris_enbl);
> + ucb1x00_reg_write(ucb, UCB_IE_FAL, ucb->irq_fal_enbl);

Racy.

I'm very much of the opinion that while UCB1400 may appear to be vaguely
register compatible with the UCB1200 and UCB1300 devices, it has
sufficiently different requirements which make any attempt at merging
drivers for both devices either racy or hard to read with additional
unnecessary complexity for the 1200 and 1300 devices.

Hence why I've been very reluctant to consider putting the UCB1400
stuff in - because it changes the existing UCB1x00 code in ways I just
don't like.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-03-23 16:47    [W:0.050 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site