[PATCH 07/23] staging: xgifb: avoid direct references xgi_video_info

Dan Carpenter dan.carpenter at oracle.com
Sun Oct 9 20:42:50 UTC 2011


On Sun, Oct 09, 2011 at 11:22:07PM +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Sun, 9 Oct 2011, Dan Carpenter wrote:
> >On Sun, Oct 09, 2011 at 07:42:51PM +0300, Aaro Koskinen wrote:
> >>-	switch (xgi_video_info.disp_state & DISPTYPE_DISP2) {
> >>+	switch (xgifb_info->disp_state & DISPTYPE_DISP2) {
> >
> >This isn't related to this patch, but this data type is weird.
> >
> >It would be probably be more natural to break up ->disp_state into
> >two variables where one stored the display type and one stored the if
> >the displays were mirrored or single.  Can you connect a CRT and an
> >LCD at the same time?
> 
> Based on a quick look, the driver does not even use the single/mirror
> info for anything...
> 
> It should be possible to connect CRT+LCD at the same time.
> 

So then how does this switch statement work if they're both set?  It
reverts to the default case.  That's the oddness of using a bitfield
in a switch statement.

But again, this patch is fine as is, I was just wondering about that
unrelated thing.

regards,
dan carpenter




More information about the devel mailing list