On Wed, Mar 21, 2007 at 06:06:58PM +0800, Wu, Bryan wrote:
> +++ linux-2.6/arch/blackfin/Kconfig   2007-03-21 14:38:42.000000000 +0800
> @@ -21,6 +21,10 @@ config RWSEM_XCHGADD_ALGORITHM
>       bool
>       default n
>  
> +config BLACKFIN
> +     bool
> +     default y
> +
>  config BFIN
>       bool
>       default y

Again, there's no reason to have both of these. Pick one and stick with
it.

> diff -purN linux-2.6-orig/arch/blackfin/kernel/flat.c 
> linux-2.6/arch/blackfin/kernel/flat.c
> --- linux-2.6-orig/arch/blackfin/kernel/flat.c        1970-01-01 
> 08:00:00.000000000 +0800
> +++ linux-2.6/arch/blackfin/kernel/flat.c     2007-03-21 14:43:37.000000000 
> +0800
[snip]
> +     switch (type) {
> +             case FLAT_BFIN_RELOC_TYPE_16_BIT:
> +             case FLAT_BFIN_RELOC_TYPE_16H_BIT:
> +                     usptr = (unsigned short *)ptr;
> +#ifdef DEBUG_BFIN_RELOC
> +                     pr_info(" *usptr = %x", get_unaligned(usptr));
> +#endif
> +                     val = get_unaligned(usptr);

The debugging here looks broken, just use pr_debug() and DEBUG as normal,
then you can kill off this DEBUG_BFIN_RELOC thing.

> @@ -346,3 +351,46 @@ unsigned long get_wchan(struct task_stru
>       while (count++ < 16);
>       return 0;
>  }
> +
> +#if !defined(CONFIG_NO_ACCESS_CHECK)
> +int _access_ok(unsigned long addr, unsigned long size)

You may want to change this to CONFIG_ACCESS_CHECK instead, it's a bit
more intuitive than checking ! CONFIG_NO_ACCESS_CHECK.

> --- linux-2.6-orig/arch/blackfin/mach-bf533/boards/ezkit.c    2007-03-21 
> 17:32:30.000000000 +0800
> +++ linux-2.6/arch/blackfin/mach-bf533/boards/ezkit.c 2007-03-21 
> 14:42:17.000000000 +0800
> @@ -6,7 +6,7 @@
>   * Created:      2005
>   * Description:
>   *
> - * Rev:          $Id: ezkit.c,v 1.27 2006/11/24 10:23:54 aubrey Exp $
> + * Rev:          $Id: ezkit.c 2794 2007-03-05 05:27:47Z cooloney $
>   *
>   * Modified:     Robin Getz <[EMAIL PROTECTED]> - Named the boards
>   *               Copyright 2005 National ICT Australia (NICTA)

The RCS tags should be killed off as well, it's clear that they don't
really mean anthing, even in this case.

> diff -purN linux-2.6-orig/arch/blackfin/mach-bf533/boards/generic_board.c 
> linux-2.6/arch/blackfin/mach-bf533/boards/generic_board.c
> --- linux-2.6-orig/arch/blackfin/mach-bf533/boards/generic_board.c    
> 2007-03-21 17:32:30.000000000 +0800
> +++ linux-2.6/arch/blackfin/mach-bf533/boards/generic_board.c 2007-03-21 
> 14:42:17.000000000 +0800
> @@ -6,7 +6,7 @@
>   * Created:      2005
>   * Description:
>   *
> - * Rev:          $Id: generic_board.c,v 1.9 2006/09/23 06:35:21 vapier Exp $
> + * Rev:          $Id: generic_board.c 2684 2007-01-22 03:59:04Z vapier $
>   *
>   * Modified:
>   *               Copyright 2005 National ICT Australia (NICTA)

An then do-nothing patches like this can be left out..

> +#if defined(CONFIG_USB_ISP1760_HCD) || defined(CONFIG_USB_ISP1760_HCD_MODULE)
> +static struct resource bfin_isp1761_resources[] = {
> +     [0] = {
> +             .name   = "isp1761-regs",
> +             .start  = ISP1761_BASE + 0x00000000,
> +             .end    = ISP1761_BASE + 0x000fffff,
> +             .flags  = IORESOURCE_MEM,
> +     },
> +     [1] = {
> +             .start  = ISP1761_IRQ,
> +             .end    = ISP1761_IRQ,
> +             .flags  = IORESOURCE_IRQ | IORESOURCE_IRQ_LOWLEVEL,
> +     },

Here you have low level.

> +};
> +
> +static struct platform_device bfin_isp1761_device = {
> +     .name           = "isp1761",
> +     .id             = 0,
> +     .num_resources  = ARRAY_SIZE(bfin_isp1761_resources),
> +     .resource       = bfin_isp1761_resources,
> +};
> +
> +static struct platform_device *bfin_isp1761_devices[] = {
> +     &bfin_isp1761_device,
> +};
> +
> +int __init bfin_isp1761_init(void)
> +{
> +     unsigned int num_devices=ARRAY_SIZE(bfin_isp1761_devices);
> +
> +     printk(KERN_INFO "%s(): registering device resources\n", __FUNCTION__);
> +     set_irq_type(ISP1761_IRQ, IRQF_TRIGGER_FALLING);
> +
But here you set it to falling?

> diff -purN linux-2.6-orig/include/asm-blackfin/asm-offsets.h 
> linux-2.6/include/asm-blackfin/asm-offsets.h
> --- linux-2.6-orig/include/asm-blackfin/asm-offsets.h 1970-01-01 
> 08:00:00.000000000 +0800
> +++ linux-2.6/include/asm-blackfin/asm-offsets.h      2007-03-21 
> 15:21:10.000000000 +0800

This is generated by Kbuild, there's no point in including it in the
diff.

> -
> +/* CSYNC implementation for C file */
>  #if defined(ANOMALY_05000312) && defined(ANOMALY_05000244)

Is there some reason these aren't config options? Perhaps an errata
sub-menu might be more intuitive.

> diff -purN linux-2.6-orig/include/asm-blackfin/entry.h 
> linux-2.6/include/asm-blackfin/entry.h
> --- linux-2.6-orig/include/asm-blackfin/entry.h       2007-03-21 
> 17:32:31.000000000 +0800
> +++ linux-2.6/include/asm-blackfin/entry.h    2007-03-21 14:15:41.000000000 
> +0800
> @@ -52,10 +52,8 @@
>  #define RESTORE_ALL_SYS              restore_context_no_interrupts
>  #define RESTORE_CONTEXT              restore_context_with_interrupts
>  
> -#define STR(X) STR1(X)
> -#define STR1(X) #X
> -# define PT_OFF_ORIG_P0              208
> -# define PT_OFF_SR           8
> +#define PT_OFF_ORIG_P0               208
> +#define PT_OFF_SR            8
>  

Should these be in asm-offsets?

The rest of the patch looks good, this is certainly an improvement!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to