Re: [PATCH net-next v2 2/4] can: cc770: add legacy ISA bus driver for the CC770 and AN82527

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hello Wolfgang(s),

On 05.01.2012 12:51, Wolfgang Grandegger wrote:

>>>> diff --git a/drivers/net/can/cc770/cc770_isa.c
>>>> b/drivers/net/can/cc770/cc770_isa.c
>>>> index 4be5fe2..48fc128 100644
>>>> --- a/drivers/net/can/cc770/cc770_isa.c
>>>> +++ b/drivers/net/can/cc770/cc770_isa.c
>>>> @@ -148,8 +148,7 @@ static void cc770_isa_port_write_reg_indirect(const
>>>> struct cc770_priv *priv,
>>>>   {
>>>>       unsigned long base = (unsigned long)priv->reg_base;
>>>>
>>>> -    outb(reg, base);
>>>> -    outb(val, base + 1);
>>>> +    outw( reg + ( val<<  8), base);
>>>

>>> That modification does fix your problem, right? The others above don't
>>> 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
>> missing interrupts but then just to avoid other possible side effects
>> and then assuming that they might be related.
> 
> OK. My concern: Can we be sure that 16bit accesses are always supported
> by the hardware? Does a spinlock_irqsave/spinlock_irqrestore around the
> 8bit accesses already help?
> 
> About the "HM:" fixes, I did not find any info in the svn log. Maybe
> Oliver knows why they have been added.



No, in fact i don't know 8-)

As you can see from the initial commit msg here:

http://svn.berlios.de/wsvn/socketcan/branches/ha/candrv/kernel/2.6/drivers/net/can/i82527/?rev=197&peg=197

"First implementation of i82527 socketcan driver based on parts from
socketcan sja1000 netdev / ocan hal / can4linux register access.
Know issues: Currently only receives SFF CAN frames due different
handling of SFF/EFF and RTR frames in opposite to the SJA1000 controller."

I took my sja1000 SocketCAN driver and tried to pick the register access
sniplets for the i82527 from (mostly) can4linux and kicked it until it
compiled ...

This register access part is from can4linux which has a long(!) development
history too, where a special message regarding the ISR can be found, see:

http://can4linux.svn.sourceforge.net/viewvc/can4linux/trunk/can4linux/i82527funcs.c?revision=36&view=markup

/*
 * can_i82527funcs.c - i82527 related code
 *
 * Original version Written by Arnaud Westenberg email:arnaud@wanadoo.nl
 * This software is released under the GPL-License.
 *
 * Modified and extended to support the esd elctronic system
 * design gmbh PC/104-CAN Card (www.esd-electronics.com)
 * by Jean-Jacques Tchouto (tchouto@fokus.fraunhofer.de), 2003
 *
 * Major Refactoring and Integration into can4linux version 3.1 by
 * Henrik W Maier of FOCUS Software Engineering Pty Ltd <www.focus-sw.com>
 *
 * Modified and extended to support the SBS PC7compact DINrail mounted
 * industry PC by FOCUS Software Engineering Pty Ltd <www.focus-sw.com>
 *
 * Updated for 2.6 kernel by Henrik W Maier of FOCUS Software
 * Engineering Pty Ltd <www.focus-sw.com>.
 * Bugfix in CAN_SendMessage, added wake_up_interruptible for irq_write_handler
 * Disabled redundant status change interrupt for RX/TX to decrease int
 * load on CPU.
 * Fixed issue of a second iteration in the ISR when sending because the
 * send interrupt didn't sometimes get acknowledged.
 *
 * $Id: i82527funcs.c,v 1.1 2006/04/21 16:26:51 oe Exp $
 *
 */

My original hardware was the Eurotech COM 1274 board for the US DOT VII OBU:
http://www.eurotech.com/en/download/com-1274

Obviously the ISR quirk code was added by Henrik W Maier from FOCUS Software:

http://www.focus-sw.com/can/can4linux.html

Maybe Henrik or Heinz-Jürgen remember why this ISR handling has been added
more than seven years ago ... i added both of them in CC.

I hope the historical src code research helped a bit.

Indeed i was not aware of that long i82527 history inside the can4linux
driver. If there's any interest of the register access authors, we should
add some credits into the cc770 driver.

Best regards,
Oliver