Re: [Uclinux-dist-devel] [PATCH v3] add the driver for Analog Devices Blackfin on-chip CAN controllers

On Fri, Dec 11, 2009 at 10:17 AM, Mike Frysinger [off-list ref] wrote:

On Thu, Dec 10, 2009 at 21:05, Barry Song wrote:

quoted

2009/12/11 David Miller [off-list ref]:

quoted

From: Mike Frysinger <redacted>

quoted

On Thu, Dec 10, 2009 at 05:48, Wolfgang Grandegger wrote:

quoted

Wolfgang Grandegger wrote:

quoted

Mike Frysinger wrote:

quoted

On Thu, Dec 10, 2009 at 04:11, Wolfgang Grandegger wrote:

quoted

Well, I'm still not a friend of the following inline functions,
especially the *one-liners* which are called just *once*. With the usage
of structs they seem even more useless.

seems like it would make more sense to not even use the read/write
functions either. �,A just declare the regs as volatile and assign/read
the struct directly.

Two times no. Don't use volatile and proper accessor functions. See:

http://lxr.linux.no/#linux+v2.6.32/Documentation/volatile-considered-harmful.txt

I was just wondering if bfin_read/write16 would not be the proper
accessor functions. readw/writew seems to be implemented differently:

http://lxr.linux.no/#linux+v2.6.32/arch/blackfin/include/asm/io.h#L44

Puh, they do an cli,nop,nop,sync..sti for the access. This also nicely
shows why accessor functions should be used to access device registers.

Well, just curious. I don't really know the blackfin arch.

the common I/O functions need to account for issues surrounding the
bus that has arbitrary devices memory mapped to it.  on-chip devices
(like what we're talking about here) do not have these issues and so
using the common functions is awful overhead.

Then create special accessors (perhaps with the same names as the
existing ones, but with "__" prepended) that lack all of the
interrupt disabling, syncs, etc.

Really it _is_ cleaner and makes your driver look a lot nicer.

I think Mike has said the functions are bfin_read/bfin_write in
blackfin arch since those CAN registers are located in memory mapped
area but not async memory and have less overhead than common io
functions? Is it acceptable to use those functions in this driver?

yes, bfin_{read,write} should be used

Wolfgang/David, are you ok with that too? If so, I will send a -v4
patch using bfin_read/write, with all fix according to your other
comments.

-mike

_______________________________________________
Socketcan-core mailing list
Socketcan-core@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/socketcan-core