Messages in this thread | | | Date | Fri, 11 Nov 2005 19:26:31 +0800 | From | Luke Yang <> | Subject | Re: ADI Blackfin patch for kernel 2.6.14 |
| |
> > We've taken arch patches in a single hit before. It's not such a bad thing. > > A basic requirement should be "it should all compile before and after the > patch". That's pretty hard to do in this case if it's split up. > > That being said, a 1.6MB patch is a bit hard to review, mainly because it > doesn't fit through the email server. > > From a quick look: > > > +static spinlock_t dma_page_lock = SPIN_LOCK_UNLOCKED; > > DEFINE_SPIN_LOCK() > > > +static unsigned int dma_initialized = 0; > > Remove the `= 0' > > +/* > + * Initial task structure. > + * > + * All other task structs will be allocated on slabs in fork.c > + */ > +__asm__(".align 4"); > +struct task_struct init_task = INIT_TASK(init_task); > > weird. That align will probably go into .text, rather than where you want > it. Use __attribute__((__aligned__(4))) or ____cacheline_aligned, or just > remove it - the compiler will align this guy on a 4-byte boundary anyway. > > > Does this architecture support SMP? I see it's BROKEN_ON_SMP, but there > seems to be some smp-style stuff in there.
It doesn't support SMP now.
> > > One concern when adding a new architecture is: will it be maintained > long-term? We don't want to merge an arch and then have it bitrot. Who is > behind this port, and how do we know that they'll still be around and doing > things in two years' time?
I don't clearly know the process of maintaining an arch in kernel. But I am sure we can follow the right process. My question is: How do they maintain the m68knommu arch? I think it need the uclinux patch to run on real platfrom. What is the process like?
> > > Can this arch use the generic IRQ handling code in kernel/irq/?
Yes.
> > > The idle routines don't appear to be up-to-date wrt post-2.6.14 changes. > Or if they are, they won't be after I merge Nick's stuff ;) > > > get_reg() is way too big to be inlined. > > Ditto put_reg(). > > > Can this arch use the generic lib/semaphore-sleepers.c? > > > +extern void icache_init(void); > > +extern void dcache_init(void); > > +extern int read_iloc(void); > > +extern unsigned long ipdt_table[]; > > +extern unsigned long dpdt_table[]; > > +extern unsigned long icplb_table[]; > > +extern unsigned long dcplb_table[]; > > +int DmaMemCpy(char *dest_addr, char *source_addr, unsigned short size); > > +int DmaMemCpy16(char *dest_addr, char *source_addr, int size); > > extern decls should always go in header files. If things like > icache_init() aren't in any headers, well mutter. It'd be nice to fix > that. Involves touching all architectures, yeah, not your job... > > > touch_l1_data() can have static scope. Please review all global symbols > for this. > > > Does a new arch need to support old_mmap()? > > > old_select()? > > > > +void time_sched_init(irqreturn_t(*timer_routine) > > + (int, void *, struct pt_regs *)); > > +unsigned long gettimeoffset(void); > > +extern unsigned long wall_jiffies; > > +extern int setup_irq(unsigned int, struct irqaction *); > > +inline static void do_leds(void); > > + > > +extern u_long get_cclk(void); > > More extern-decls-in-c-files > > > +asmlinkage int sys_bfin_spinlock(int *spinlock) > > Whoa, that's a syscall I never expected to see. What's it do? Shouldn't > it be using get_user() and put_user()? Or will this forever be a nommu > arch? > > > What _is_ a bluefin, anyway? > > > Are precompiled cross-complers/assemblers available anywhere?
Yes, please visit blackfin.uclinux.org to get our toolchain.
> > > bix:/home/akpm> grep volatile bfin_r2_4kernel-2.6.14.patch | wc -l > 2901 > > Cow. You know that volatile in-kernel is basically always wrong? > To the other detail questions/issues, I'll write to you soon. Thank you for your help.
Luke Yang - 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/
| |