> -----Original Message----- > From: Tomi Valkeinen [mailto:tomi.valkeinen@xxxxxx] > Sent: Wednesday, July 30, 2014 22:24 PM > > +static struct fb_info *hvfb_info; > > Static variables like these are usually a no-no. This prevents you from > having multiple device instances. I agree. > > static uint screen_width = HVFB_WIDTH; > > static uint screen_height = HVFB_HEIGHT; > > static uint screen_depth; > > static uint screen_fb_size; > > > > +/* If true, the VSC notifies the VSP on every framebuffer change */ > > +static bool synchronous_fb; > > + > > Same comment here. > > However, if (and only if) the driver is already designed to work only > with single device instance, then this patch is probably ok. But even IMO the host should only provide at most 1 synthetic video device to a VM. :-) > then, I'd prefer this to be handled without static variables so that the > driver could eventually be changed to support multiple device instances. Hi Tomi, Maybe we can remove these static stuff: +static struct fb_info *hvfb_info; +static struct notifier_block hvfb_panic_nb = { + .notifier_call = hvfb_on_panic, +}; by kmalloc()-ing a new struct: struct hv_fb_panic_nb { struct fb_info *hvfb_info; struct notifier_block nb; } ? I think in hvfb_on_panic() we should be able to get the hvfb_info pointer by hvfb_info = container_of(nb, struct hv_fb_panic_nb, nb). If you like that or have a better idea, please let me know so I can make a new patch. Thanks, -- Dexuan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
