Hi Wolfgang,

On 01/10/2012 05:13 PM, Wolfgang Zarre wrote:

Hello Wolfgang,

quoted

On 01/10/2012 01:41 PM, Wolfgang Zarre wrote:

quoted

Hello David,

quoted

quoted

cc770_isa_port_write_reg_indirect(const struct cc770_priv *priv,
                            int reg, u8 val)
    {
        unsigned long base = (unsigned long)priv->reg_base;
+    unsigned long flags;

+    spin_lock_irqsave(&outb_lock, flags);
        outb(reg, base);
        outb(val, base + 1);
+    spin_unlock_irqrestore(&outb_lock, flags);

Is there a 'read_reg_indirect' function??

Yes, there is.

quoted

If so it also needs to use the same mutex.

Actually, I don't think that we have a problem with mutex
beside that it's using just one inb() statement but having
for sure with an interrupt between both outb() statements which
seems to be critical for the cc770.

But the indirect read function also sets the address register before
reading the data using inb(). This sequence should also not be
interrupted and therefore we need to synchronize. For the indirect
access of the SJA1000 we also need to add spinlocks. Wonder why nobody
complained so far.

So, if I understand correct that means that inb() can be interrupted
between
setting the address and reading. If this is the case then yes, we need
spinlock if this is not the case then IMHO we wouldn't need or am I wrong?

I think we speak about different things. inb() cannot be interrupted but
outb() followed by inb(). For indirect accesses we need something like:

/* Spinlock for cc770_isa_port_write_reg_indirect */
static DEFINE_SPINLOCK(cc770_isa_port_lock);

static u8 cc770_isa_port_read_reg_indirect(const struct cc770_priv *priv,
                                             int reg)
{
        unsigned long base = (unsigned long)priv->reg_base;
	unsigned long flags;
	u8 val;

	spin_lock_irqsave(&cc770_isa_port_lock, flags);
        outb(reg, base);
        val = inb(base + 1);
	spin_unlock_irqrestore(&cc770_isa_port_lock, flags);    

	return val;
}

static void cc770_isa_port_write_reg_indirect(const struct cc770_priv *priv,
                                                int reg, u8 val)
{
        unsigned long base = (unsigned long)priv->reg_base;
	unsigned long flags;

	spin_lock_irqsave(&cc770_isa_port_lock, flags);
        outb(reg, base);
        outb(val, base + 1);
	spin_unlock_irqrestore(&cc770_isa_port_lock, flags);    
}

Hope we are in synch now.

Wolfgang.