6 comments:
Patch Set #6, Line 107: struct fb_info;
Shouldn't you #include <framebuffer_info.h> rather than declare this in two places?
Patch Set #6, Line 36: static struct fb_info *list;
Why not use <list.h>?
Patch Set #6, Line 95: info->edid_fb.blue_mask_size = blue_mask_size;
nit: at this point, isn't it simpler to just let the caller fill out the structure directly and then call something like fb_info_validate() to check invariants? Functions with a dozen arguments that are all integers aren't great, it's very easy to mix stuff up and not notice the error.
Patch Set #6, Line 96: info->valid = true;
I'm a bit confused who needs this. Can we ever have a framebuffer in the list that's not valid?
Patch Set #6, Line 148: fb_fill_framebuffer_set_orientation
nit: "fill" makes me think this fills out the whole thing, not just sets a single value. Why not just fb_set_orientation()?
Patch Set #6, Line 172: for (struct fb_info *i = list; i != NULL; i = i->next) {
If this is the only reason for having a list, you don't need a list. You just need a single pointer that you keep updating whenever someone creates a new valid FB.
To view, visit change 39002. To unsubscribe, or for help writing mail filters, visit settings.