On Fri, May 17, 2019 at 12:03:12PM +0100, Jeremy Sowden wrote:
> The call-backs used the same recipe to get the pcard from dev:
> 
>   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;
> 
> where to_pci_dev is a wrapper for container_of.
> 
> However, pci_set_drvdata is called before the sysfs files are created:
> 
>   int  kp2000_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   {
>     // ...
> 
>     pcard = kzalloc(sizeof(struct kp2000_device), GFP_KERNEL);
> 
>     // ...
> 
>     pcard->pdev = pdev;
>     pci_set_drvdata(pdev, pcard);
> 
>     // ...
> 
>     err = sysfs_create_files(&(pdev->dev.kobj), kp_attr_list);
> 
> Therefore, to_pci_dev and pci_get_drvdata cannot return NULL, and pcard
> can be initialized directly from dev:
> 
>   struct kp2000_device *pcard = dev_get_drvdata(dev);
> 
> Signed-off-by: Jeremy Sowden <jer...@azazel.net>
> ---
>  drivers/staging/kpc2000/kpc2000/core.c | 24 +++---------------------
>  1 file changed, 3 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000/core.c 
> b/drivers/staging/kpc2000/kpc2000/core.c
> index eb8bac62d33d..9425c4dbc2f2 100644
> --- a/drivers/staging/kpc2000/kpc2000/core.c
> +++ b/drivers/staging/kpc2000/kpc2000/core.c
> @@ -24,12 +24,7 @@
>    ******************************************************/
>  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 :(

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?

thanks,

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

Reply via email to