Messages in this thread | | | Date | Fri, 14 Aug 2009 14:54:40 -0700 | From | Dmitry Torokhov <> | Subject | Re: [PATCH] Samsung backlight driver |
| |
Hi Greg,
On Fri, Aug 14, 2009 at 12:48:15PM -0700, Greg KH wrote: > + > +static int update_status(struct backlight_device *bd) > +{ > + if (!pci_device) > + return -ENODEV; > + if (!backlight_device)
How is this possible?
> + return -ENODEV; > + > + current_brightness = bd->props.brightness; > + set_brightness(); > + return 0; > +} > + > +static struct backlight_ops backlight_ops = { > + .get_brightness = get_brightness, > + .update_status = update_status, > +}; > + > +static int find_video_card(void) > +{ > + struct pci_dev *dev = NULL; > + > + while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) { > + /* Found one, so let's save it off */ > + if (!pci_device) > + pci_device = pci_dev_get(dev);
If you need to iterate all matching devices you need to save them all or release references to "extras" right here... Or, better yet, just break after you find the first one.
> + } > + > + if (!pci_device) > + return 0;
Why not -ENODEV? What is the point of having this driver loaded if the device is not available? Then you could drop all silly "if (!pci_device)" checks.
-- Dmitry
| |