Hi Joe, please reply to all of this email, because some developers need
to consider your review.
Hi Michael, how do you think of Joe's idea as below.
Thanks,
Best Regards,
- Bryan Wu
On Wed, 2007-07-25 at 09:46 -0700, Joe Perches wrote:
> On Wed, 2007-07-25 at 23:26 +0800, Bryan Wu wrote:
> > diff --git a/arch/blackfin/kernel/bfin_gpio.c
> > b/arch/blackfin/kernel/bfin_gpio.c
> > index bafcfa5..979cf79 100644
> > --- a/arch/blackfin/kernel/bfin_gpio.c
> > +++ b/arch/blackfin/kernel/bfin_gpio.c
> > @@ -143,22 +148,100 @@ inline int check_gpio(unsigned short gpio)
>
> []
>
> > +#ifdef BF537_FAMILY
> > +
> > +#define PMUX_LUT_RES 0
> > +#define PMUX_LUT_OFFSET 1
> > +#define PMUX_LUT_ENTRIES 41
> > +#define PMUX_LUT_SIZE 2
> > +
> > +static unsigned short port_mux_lut[PMUX_LUT_ENTRIES][PMUX_LUT_SIZE] = {
> > + {P_PPI0_D13, 11}, {P_PPI0_D14, 11}, {P_PPI0_D15, 11},
> > + {P_SPORT1_TFS, 11}, {P_SPORT1_TSCLK, 11}, {P_SPORT1_DTPRI, 11},
> > + {P_PPI0_D10, 10}, {P_PPI0_D11, 10}, {P_PPI0_D12, 10},
> > + {P_SPORT1_RSCLK, 10}, {P_SPORT1_RFS, 10}, {P_SPORT1_DRPRI, 10},
> > + {P_PPI0_D8, 9}, {P_PPI0_D9, 9}, {P_SPORT1_DRSEC, 9},
> > + {P_SPORT1_DTSEC, 9}, {P_TMR2, 8}, {P_PPI0_FS3, 8}, {P_TMR3, 7},
> > + {P_SPI0_SSEL4, 7}, {P_TMR4, 6}, {P_SPI0_SSEL5, 6}, {P_TMR5, 5},
> > + {P_SPI0_SSEL6, 5}, {P_UART1_RX, 4}, {P_UART1_TX, 4}, {P_TMR6, 4},
> > + {P_TMR7, 4}, {P_UART0_RX, 3}, {P_UART0_TX, 3}, {P_DMAR0, 3},
> > + {P_DMAR1, 3}, {P_SPORT0_DTSEC, 1}, {P_SPORT0_DRSEC, 1},
> > + {P_CAN0_RX, 1}, {P_CAN0_TX, 1}, {P_SPI0_SSEL7, 1},
> > + {P_SPORT0_TFS, 0}, {P_SPORT0_DTPRI, 0}, {P_SPI0_SSEL2, 0},
> > + {P_SPI0_SSEL3, 0}
> > +};
>
> Wouldn't this be better as a struct?
>
> struct PMUX_LUT {
> unsigned short res;
> unsigned short offset;
> }
>
> struct PMUX_LUT port_mux_lut = {
> {.res = P_PPI0_D13, .offset = 11},
> {.res = P_PPi0_D14, .offset = 11},
> etc?
> };
>
> > +
> > +static void portmux_setup(unsigned short per, unsigned short
> > function)
> > +{
> > + u16 y, muxreg, offset;
>
> Shouldn't y be int?
>
> > @@ -179,22 +262,15 @@ static void default_gpio(unsigned short gpio)
> >
> > static int __init bfin_gpio_init(void)
> > {
> > - int i;
> > -
> > - printk(KERN_INFO "Blackfin GPIO Controller\n");
> >
> > - for (i = 0; i < MAX_BLACKFIN_GPIOS; i += GPIO_BANKSIZE)
> > - reserved_map[gpio_bank(i)] = 0;
> > + str_ident = kzalloc(RESOURCE_LABEL_SIZE * 256, GFP_KERNEL);
>
> 256?
>
> perhaps better to use kcalloc(RESOURCE_LABEL_SIZE, 256, GFP_KERNEL)
> and reference str_ident as an array of char* instead of the
> multiplies in set_label, get_label and cmp_label?
>
> > + if (!str_ident)
> > + return -ENOMEM;
>
> cheers, Joe
-
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/
