Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ARM: OMAP: Cleanup OMAP FB SDRAM reservation

The logic in this file is rather convoluted, but essentially:

1. region type 0 is SDRAM
2. referring to the code fragment
if (set_fbmem_region_type(&rg, OMAPFB_MEMTYPE_SDRAM,
sdram_start, sdram_size) < 0 ||
(rg.type != OMAPFB_MEMTYPE_SDRAM))
continue;
- if rg.type is not OMAPFB_MEMTYPE_SDRAM, set_fbmem_region_type()
returns zero immediately (since rg.type is non-zero), and so we
'continue'.
- if rg.type is OMAPFB_MEMTYPE_SDRAM, and rg.paddr is zero,
we fall through.
- if rg.type is OMAPFB_MEMTYPE_SDRAM, and the region lies within
SDRAM, we fall through.
- if rg.type is OMAPFB_MEMTYPE_SDRAM, and the region is not within
SDRAM, we 'continue'.
3. check_fbmem_region seems unnecessary.
- we know rg.type is OMAPFB_MEMTYPE_SDRAM
- we can check rg.size independently
- bootmem_reserve() can check for overlapping reservations itself
- we've already validated that the requested region lies within SDRAM.
4. avoid BUG()ing if the region entry is already set; print an error,
and mark the configuration invalid - at least we'll continue booting
so the error message has a chance of being logged/visible via serial
console.

With these changes in place, it makes the code much easier to understand
and hence easier to convert to LMB.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

+44 -17
+44 -17
arch/arm/plat-omap/fb.c
··· 171 171 return 0; 172 172 } 173 173 174 + static int valid_sdram(unsigned long addr, unsigned long size) 175 + { 176 + struct bootmem_data *bdata = NODE_DATA(0)->bdata; 177 + unsigned long sdram_start, sdram_end; 178 + 179 + sdram_start = bdata->node_min_pfn << PAGE_SHIFT; 180 + sdram_end = bdata->node_low_pfn << PAGE_SHIFT; 181 + 182 + return addr >= sdram_start && sdram_end - addr >= size; 183 + } 184 + 185 + static int reserve_sdram(unsigned long addr, unsigned long size) 186 + { 187 + return reserve_bootmem(addr, size, BOOTMEM_EXCLUSIVE); 188 + } 189 + 174 190 /* 175 191 * Called from map_io. We need to call to this early enough so that we 176 192 * can reserve the fixed SDRAM regions before VM could get hold of them. 177 193 */ 178 194 void __init omapfb_reserve_sdram(void) 179 195 { 180 - struct bootmem_data *bdata; 181 - unsigned long sdram_start, sdram_size; 182 - unsigned long reserved; 183 - int i; 196 + unsigned long reserved = 0; 197 + int i; 184 198 185 199 if (config_invalid) 186 200 return; 187 201 188 - bdata = NODE_DATA(0)->bdata; 189 - sdram_start = bdata->node_min_pfn << PAGE_SHIFT; 190 - sdram_size = (bdata->node_low_pfn << PAGE_SHIFT) - sdram_start; 191 - reserved = 0; 192 202 for (i = 0; ; i++) { 193 - struct omapfb_mem_region rg; 203 + struct omapfb_mem_region rg; 194 204 195 205 if (get_fbmem_region(i, &rg) < 0) 196 206 break; 207 + 197 208 if (i == OMAPFB_PLANE_NUM) { 198 - printk(KERN_ERR 199 - "Extraneous FB mem configuration entries\n"); 209 + pr_err("Extraneous FB mem configuration entries\n"); 200 210 config_invalid = 1; 201 211 return; 202 212 } 213 + 203 214 /* Check if it's our memory type. */ 204 - if (set_fbmem_region_type(&rg, OMAPFB_MEMTYPE_SDRAM, 205 - sdram_start, sdram_size) < 0 || 206 - (rg.type != OMAPFB_MEMTYPE_SDRAM)) 215 + if (rg.type != OMAPFB_MEMTYPE_SDRAM) 207 216 continue; 208 - BUG_ON(omapfb_config.mem_desc.region[i].size); 209 - if (check_fbmem_region(i, &rg, sdram_start, sdram_size) < 0) { 217 + 218 + /* Check if the region falls within SDRAM */ 219 + if (rg.paddr && !valid_sdram(rg.paddr, rg.size)) 220 + continue; 221 + 222 + if (rg.size == 0) { 223 + pr_err("Zero size for FB region %d\n", i); 210 224 config_invalid = 1; 211 225 return; 212 226 } 227 + 213 228 if (rg.paddr) { 214 - reserve_bootmem(rg.paddr, rg.size, BOOTMEM_DEFAULT); 229 + if (reserve_sdram(rg.paddr, rg.size)) { 230 + pr_err("Trying to use reserved memory for FB region %d\n", 231 + i); 232 + config_invalid = 1; 233 + return; 234 + } 215 235 reserved += rg.size; 216 236 } 237 + 238 + if (omapfb_config.mem_desc.region[i].size) { 239 + pr_err("FB region %d already set\n", i); 240 + config_invalid = 1; 241 + return; 242 + } 243 + 217 244 omapfb_config.mem_desc.region[i] = rg; 218 245 configured_regions++; 219 246 }