6 comments:
Patch Set #6, Line 107: struct fb_info;
`framebuffer_info.h` is the public API and doesn't provide more information (doesn't share internals either).
Yeah, that's why I'm unclear why we wouldn't just #include it. It does provide the declaration of struct fb_info which is all that's needed here.
Patch Set #7, Line 34: edid_fb
nit: Would probably be a good occasion to rename this field since it really has nothing to do with EDID anymore. Just 'fb' would probably be fine?
Patch Set #7, Line 36: static struct fb_info list;
This should just be a raw struct list_node.
Patch Set #7, Line 56: struct fb_info *info,
Why does this work so differently from fb_set_framebuffer_info()? Why can't you call fb_new_framebuffer_info() internally from here too (so you don't have to export that function at all)? If you're just doing this to share code, you can have a static function that does this, but the exported wrappers should have a consistent interface.
Patch Set #7, Line 63: uint8_t blue_mask_size)
I still really don't like this super long function signature. How about passing in a struct lb_framebuffer pointer instead (which is memcpied to the new fb_info) so that the caller can pass the same values, but in a more structured (less error prone) manner?
'set' seems like the wrong verb here, especially in comparison to fb_set_orientation() (which changes an existing fb_info rather than create a new one). Maybe fb_add_framebuffer_info() would be better?
To view, visit change 39002. To unsubscribe, or for help writing mail filters, visit settings.