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

[Wolfgang Zarre <info@essax.com>]

Hello guys,

> Hi Henrik,
>
> On 01/10/2012 12:41 PM, Henrik Maier wrote:
>> Hello guys,
>>
>> see comments below.
>>
>> On 9/01/2012 10:47 PM, Wolfgang Zarre wrote:
>>> 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
>>
>> The can4linux port I made was done for a specific project and a specific
>> hardware, the SBS PC7, and only ever tested on that hardware. The i82527
>> code was already in the can4linux 3.x archive but not maintained for a
>> while and has become non-functional over time.
>>
>> Initially we used the 2.4 Linux kernel and the can4linux version 3.1 as
>> basis for the re-integration of the i82527. I called our modified
>> version  "3.1i" (i for Intel). This is from the changelog:
>>
>> 3.1i - Integration of the i82527 code into can4linux version 3.1,
>>        major refactoring and clean-up of the i82527 code, fixed
>>        interrupt lock-up issue w/ sending while receiving in heavy load,
>>        fixed issue w/ stopping and re-starting the bus, added
>>        new acceptance mask function to bring code in line with SJA1000 code
>>        base, new I/O model to support the SJA1000 CANin/CANout macros,
>>        support added to the makefile for generic i82527 cards and
>>        SBS PC7compact DINrail mounted industry PC
>>        by Henrik Maier of FOCUS Software Engineering Pty Ltd
>> <www.focus-sw.com>
>>
>> As time passed the requirement for kernel 2.6 support emerged and we did
>> the same based on can4linux 3.3.2 and called it "3.3.2i" which later was
>> merged into the official can4linux archive and became 3.3.3.
>>
>> The "repeated TX IRQ" issue came up only with kernel 2.6 and was not
>> required in the 2.4 version. The way I fixed it may not have been the
>> best, but it solved the immediate need of finishing a project and
>> getting the driver stable. I am not a kernel driver expert.
>
> OK, then most likely it's not a hardware issue.
>
>> The PC7 hardware is now obsolete anyway. From my point we can do a
>> "clean" start and remove the old baggage.
>>
>> The PC7 CAN interface is ISA based and I don't have a hardware any more
>> to test.
>>
>> Some background info and our "i" versions are published here:
>>
>> http://www.focus-sw.com/can/can4linux.html
>>
>> I hope that helps a bit. It has been a few years since I worked with
>> can4linux but it is great to see that the community is further enhancing
>> CAN support for Linux.
>
> Yes, of course. Thanks for digging that out. As you suggest, we will
> remove that code and wait what happens.
>

Ok, I'll prepare this patch separately as said.

> Wolfgang.
>

Wolfgang