On Fri, Dec 11, 2009 at 10:17 AM, Mike Frysinger <[email protected]> wrote: > On Thu, Dec 10, 2009 at 21:05, Barry Song wrote: >> 2009/12/11 David Miller <[email protected]>: >>> From: Mike Frysinger <[email protected]> >>>> On Thu, Dec 10, 2009 at 05:48, Wolfgang Grandegger wrote: >>>>> Wolfgang Grandegger wrote: >>>>>> Mike Frysinger wrote: >>>>>>> On Thu, Dec 10, 2009 at 04:11, Wolfgang Grandegger wrote: >>>>>>>> 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 [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
