Messages in this thread | | | Date | Sat, 23 Sep 2006 02:50:02 -0400 | From | "Mike Frysinger" <> | Subject | Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18 |
| |
On 9/22/06, Arnd Bergmann <arnd@arndb.de> wrote: > Use pr_debug here instead of making up your own macros
done
> static inline void
done
> It would be nice if you could use a generic way to pass this partition data > to the kernel from the mtd medium, instead of hardcoding it here.
i often wish for such things myself :)
unfortunately, the boot loader we utilize (u-boot) isnt exactly friendly to the idea of managing flash partitions like redboot, and what we have here is the current standard method for defining flash partitions with mtd
> Use C99 initializers here, like
done
> There is not much point in trying to use the same numbers as an existing > architecture if that means that you have to leave holes like setup(). > I don't know if you still have the choice of completely changing the > syscall numbers, but it would make it nicer in the future.
we do, fortunately, have this luxury ... so we can look forward to a nice cleaning of our syscall interface
> What's th point of these files, can't you just remove them all?
punted
> > +#define BUG() do { \ > > + dump_stack(); \ > > + printk("\nkernel BUG at %s:%d!\n", __FILE__, __LINE__); \ > > + panic("BUG!"); \ > > +} while (0) > > This is probably better done as an external function:
*shrug* i just did it the same way everyone else does ... i'm a sheep like that
> > diff -urN linux-2.6.18/include/asm-blackfin/mach-bf533/anomaly.h > linux-2.6.18.patch1/include/asm-blackfin/mach-bf533/anomaly.h > > --- linux-2.6.18/include/asm-blackfin/mach-bf533/anomaly.h 1970-01-01 > 08:00:00.000000000 +0800 > > +++ linux-2.6.18.patch1/include/asm-blackfin/mach-bf533/anomaly.h 2006-09-21 > 09:29:49.000000000 +0800 > > @@ -0,0 +1,172 @@ > > +/* > > + * File: include/asm-blackfin/mach-bf533/anomaly.h > > You seem to have lots of these machine specfic header files in include/asm. > Please move them to the respective machine implementation directory > if they are only used from there
these are arch specific, not machine (aka board)
this is what the blackfin family looks like (first entry is the architecture): - BF535 - BF533 / BF532 / BF531 - BF537 / BF536 / BF534 - BF561
which is why you find the anomaly definitions in the architecture specific header dir
> > +/* Clock and System Control (0xFFC0 0400-0xFFC0 07FF) */ > > +#define bfin_read_PLL_CTL() bfin_read16(PLL_CTL) > > +#define bfin_write_PLL_CTL(val) bfin_write16(PLL_CTL,val) > > +#define bfin_read_PLL_STAT() bfin_read16(PLL_STAT) > (and 700 more of these) > > What's the point, are you getting paid by lines of code? Just use > the registers directly!
in our last submission we were doing exactly that ... and we were told to switch to a function style method of reading/writing memory mapped registers
> One more thing about the headers: please add a Kbuild file in there so > 'make headers_install' works
will do
thanks for reading through that monster :) -mike - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |