Re: [PATCH v3 3/6] staging: kpc2000: simplified kp2000_device retrieval in device attributes call-backs.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 21, 2019 at 02:23:13PM +0300, Dan Carpenter wrote:
> On Fri, May 17, 2019 at 01:54:51PM +0200, Greg KH wrote:
> > On Fri, May 17, 2019 at 12:03:12PM +0100, Jeremy Sowden wrote:
> > >  static ssize_t  show_attr(struct device *dev, struct device_attribute *attr, char *buf)
> > >  {
> > > -    struct pci_dev *pdev = to_pci_dev(dev);
> > > -    struct kp2000_device *pcard;
> > > -
> > > -    if (!pdev)  return -ENXIO;
> > > -    pcard = pci_get_drvdata(pdev);
> > > -    if (!pcard)  return -ENXIO;
> > > +    struct kp2000_device *pcard = dev_get_drvdata(dev);
> > 
> > Wait, dev_get_drvdata() is not returning you the same pointer that
> > pci_get_drvdata() does.  So I think this is now broken :(
> > 
> 
> It looks sort of weird but it's fine.
> 
> > What this should look like is this:
> > 	struct pci_dev *pdev = to_pci_dev(dev);
> > 	struct kp200_device *pcard = pci_get_drvdata(pdev);
> > 
> > 	if (!pcard)
> > 		return -ENODEV;
> > 
> > that is IF the driver really is setting the pci dev data to NULL when
> > the device is removed from the driver.  Is it?
> 
> Yes.  The pci_get_drvdata() is only set to NULL after we remove the
> sysfs files so pci_get_drvdata() always returns a valid pointer.

Ugh, I am wrong, it's not as if I didn't actually write the
dev_get_drvdata() and pci_get_drvdata() code 15+ years ago, you would
think I would have remembered something like this :(

Anyway, patches look good, sorry for the noise...

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux