Hello guys,
quoted
quoted
quoted
quoted
- outb(reg, base);
- outb(val, base + 1);
+ outw( reg + ( val<< 8), base);
That modification does fix your problem, right? The others above
don't
quoted
quoted
quoted
help nor harm but we don't know if it's really realted to the same
problem. I wll dig a bit deeper.
Exactly. The others above I removed because facing the opposite,
even
quoted
quoted
missing interrupts but then just to avoid other possible side
effects
quoted
quoted
and then assuming that they might be related.
OK. My concern: Can we be sure that 16bit accesses are always
supported
quoted
by the hardware? Does a spinlock_irqsave/spinlock_irqrestore around
the
quoted
8bit accesses already help?
Hmmm... are there any register reads that need the
same 'double cycle' sequence ??
If so you need to stop reads being interleaved (with
themselves and writes) so requesting a 16bit access
doesn't help.
Which means you need a spinlock...
David
@David: Thank You very much for that hint. You are right and to
implement correct we need a spinlock.
@Wolfgang: I was thinking about Your question regarding 8/16 bit and
in fact it wouldn't work at all on a clean 8 bit cards.
Further it wouldn't work on 16 bit cards where the MSB is not equal
to base port +1 and anyway, it's depending always on how the chip is
interfaced to the ISA bus and in which mode the chip is configured.
And therefore I was giving David's hint a try in using a spinlock in
function cc770_isa_port_write_reg_indirect() and patched as follows:
---------------------------------------------------------------------
diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index 2d12f89..dad6707 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -460,15 +460,6 @@ static netdev_tx_t cc770_start_xmit(struct sk_buff *skb, struct net_device *dev)
stats->tx_bytes += dlc;
-
- /*
- * HM: We had some cases of repeated IRQs so make sure the
- * INT is acknowledged I know it's already further up, but
- * doing again fixed the issue
- */
- cc770_write_reg(priv, msgobj[mo].ctrl0,
- MSGVAL_UNC | TXIE_UNC | RXIE_UNC | INTPND_RES);
-
return NETDEV_TX_OK;
}
@@ -689,12 +680,6 @@ static void cc770_tx_interrupt(struct net_device *dev, unsigned int o)
/* Nothing more to send, switch off interrupts */
cc770_write_reg(priv, msgobj[mo].ctrl0,
MSGVAL_RES | TXIE_RES | RXIE_RES | INTPND_RES);
- /*
- * We had some cases of repeated IRQ so make sure the
- * INT is acknowledged
- */
- cc770_write_reg(priv, msgobj[mo].ctrl0,
- MSGVAL_UNC | TXIE_UNC | RXIE_UNC | INTPND_RES);
stats->tx_packets++;
can_get_echo_skb(dev, 0);
diff --git a/drivers/net/can/cc770/cc770_isa.c b/drivers/net/can/cc770/cc770_isa.c
index 4be5fe2..fe39eed 100644
--- a/drivers/net/can/cc770/cc770_isa.c
+++ b/drivers/net/can/cc770/cc770_isa.c
@@ -110,6 +110,9 @@ MODULE_PARM_DESC(bcr, "Bus configuration register (default=0x40 [CBY])");
#define CC770_IOSIZE 0x20
#define CC770_IOSIZE_INDIRECT 0x02
+/* Spinlock for cc770_isa_port_write_reg_indirect */
+static DEFINE_SPINLOCK( outb_lock);
+
static struct platform_device *cc770_isa_devs[MAXDEV];
static u8 cc770_isa_mem_read_reg(const struct cc770_priv *priv, int reg)
@@ -147,9 +150,12 @@ 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( &outb_lock, flags);
outb(reg, base);
outb(val, base + 1);
+ spin_unlock_irqrestore( &outb_lock, flags);
}
static int __devinit cc770_isa_probe(struct platform_device *pdev)
----------------------------------------------------------------------
The result is absolutely perfect. I was sending 2,000,000 telegrams
without any problem like the foregoing patch but now 8 bit compatible.
Further I have also investigated a bit the issue regarding the "HM"
patches.
I checked out the can4linux and I could find this patch in file
can_i82527funcs.c as mentioned by Oliver.
@Oliver: BTW thanks for Your investigation.
After studying the Makefile and some tests I discovered that
this file is just used for the target SBS_PC7 but funny enough not
for GENERIC_I82527.
I was seeking the declaration of CANout used in can_i82527funcs.c
to see if it's based on the same code we had and as well as in Lincan
but unfortunately the target SBS_PC7 doesn't compile with kernel
2.6.39-4 and moreover I got the message:
...can4linux/trunk/can4linux/i82527funcs.c:72:
error: implicit declaration of function 'CANout'
And additional not knowing if the board of SBS_PC7 is ISA or PCI based
I tried to find out and found after some research the following
thread:
http://old.nabble.com/-PATCH-v2--cc770-isa%3A-legacy-driver-for-CC770-i82725-on-the-ISA-bus-td24216347.html
According to that I assumed it's ISA based and is working with cc770_isa.
But not finding the declaration of CANout used in can_i82527funcs.c I
assumed further that it was done without spinlock which might cause
the effect of repeated or even lost interrupts as well depending on
the hardware configuration.
If this is the case then the HM patches would be obsolete and maybe
someone owning a SBS PC7 can test without these patches.
@Henrik: I would like to ask You if You can confirm this when You are
back.
Thanks a lot.
Wolfgang