On Sun, Jun 7, 2009 at 18:48, Robin Getz wrote: > On Sun 7 Jun 2009 18:21, Mike Frysinger pondered: >> On Sun, Jun 7, 2009 at 18:08, Robin Getz wrote: >> > On Sun 7 Jun 2009 17:21, [email protected] pondered: >> >> --- trunk/arch/blackfin/kernel/process.c 2009-06-07 19:11:50 UTC >> >> (rev 6602) >> >> +++ trunk/arch/blackfin/kernel/process.c 2009-06-07 21:21:31 UTC >> >> (rev 6603) >> >> +int bfin_mem_access_type(unsigned long addr, unsigned long size) >> >> +{ >> >> + /* Check that things do not wrap around */ >> >> + if (addr > ULONG_MAX - size) >> >> + return -EFAULT; >> >> + >> >> + if (addr >= FIXED_CODE_START && (addr + size) <= physical_mem_end) >> >> + return 0; >> >> + >> >> + if (IN_MEM(addr, size, get_l1_code_start(), L1_CODE_LENGTH)) >> >> + return 1; >> >> + >> >> + if (IN_MEM(addr, size, get_l1_scratch_start(), L1_SCRATCH_LENGTH)) >> >> + return 0; >> >> + if (IN_MEM(addr, size, get_l1_data_a_start(), L1_DATA_A_LENGTH)) >> >> + return 0; >> >> + if (IN_MEM(addr, size, get_l1_data_b_start(), L1_DATA_B_LENGTH)) >> >> + return 0; >> >> + if (IN_MEM(addr, size, L2_START, L2_LENGTH)) >> >> + return 0; >> >> + >> >> + if (addr >= SYSMMR_BASE) >> >> + return 0; >> >> + >> >> + /* We can't read EBIU banks that aren't enabled or we end up hanging >> >> + * on the access to the async space. >> >> + */ >> >> + if (IN_MEM(addr, size, ASYNC_BANK0_BASE, ASYNC_BANK0_SIZE)) >> >> + return IN_ASYNC(0, 0); >> >> + if (IN_MEM(addr, size, ASYNC_BANK1_BASE, ASYNC_BANK1_SIZE)) >> >> + return IN_ASYNC(1, 0); >> >> + if (IN_MEM(addr, size, ASYNC_BANK2_BASE, ASYNC_BANK2_SIZE)) >> >> + return IN_ASYNC(2, 1); >> >> + if (IN_MEM(addr, size, ASYNC_BANK3_BASE, ASYNC_BANK3_SIZE)) >> >> + return IN_ASYNC(3, 1); >> >> + >> >> + return -EFAULT; >> >> +} >> > >> > Rather than returning magic numbers (0, 1 or 2) - can we define some >> > constants in the .h file, and use those? >> >> the header file defines the return values in the kernel documentation, >> but using an enum or defines doesnt matter to me > > I just thought it would make the calling functions easier to > understand than 0/1/2 (could remove the comment)... > >> + switch (bfin_mem_access_type(addr, 2)) { >> + case 0: /* core access */ >> + *val = *address; >> + return true; >> + case 1: /* L1 inst (dma access) */ >> + isram_memcpy(val, address, 2); >> + return true; >> + default: /* invalid access */ >> +
it probably would, but i didnt want to go through pondering a good define naming scheme that wasnt ambiguous and didnt accidentally get leveraged elsewhere considering there are only two consumers of this function. -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
