Hi Krzysztof and Bjorn, Thanks for your comments and your time.Your questions are answered inline. I have checked and tested *pci_device_is_present* as pointed out by Bjorn. I think that API could satisfy my needs. So this patch request could be dropped. Sorry for the inconvenience. On Tue, May 25, 2021 at 9:20 PM Krzysztof Wilczyński <kw@xxxxxxxxx> wrote: > > Hi Lambert, > > Thank you for sending the patch over! > > To match the style of other patches you'd need to capitalise "PCI" in > the subject, see the following for some examples: > > $ git log --oneline drivers/pci/pci.c > > Also, it might be worth mentioning in the subject that this is a new API > that will be added. > I will be careful on the styles of patch title and description in my future patches. > > Device drivers use this API to proactively check if the device > > is alive or not. It is helpful for some PCI devices to detect > > surprise removal and do recovery when Hotplug function is disabled. > > > > Note: Device in power states larger than D0 is also treated not alive > > by this function. > [...] > > Question to you: do you have any particular users of this new API in > mind? Or is this solving some problem you've seen and/or reported via > the kernel Bugzilla? > The user is our new PCI driver under development for WWAN devices . Surprise removal could happen under multiple circumstances. e.g. Exception, Link Failure, etc. We wanted this API to detect surprise removal or check device recovery when AER and Hotplug are disabled. I thought the API could be commonly used for many similar devices. > > +/** > > + * pci_dev_is_alive - check the pci device is alive or not > > + * @pdev: the PCI device > > That would be "PCI" in the function brief above. Also, try to be > consistent and capitalise everything plus add missing periods, see the > following for an example on how to write kernel-doc: > > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation > > > + * Device drivers use this API to proactively check if the device > > + * is alive or not. It is helpful for some PCI devices to detect > > + * surprise removal and do recovery when Hotplug function is disabled. > > As per my question above - what users of this new API do you have in > mind? Are they any other patches pending adding users of this API? > > > + * Note: Device in power state larger than D0 is also treated not alive > > + * by this function. > > + * > > + * Returns true if the device is alive. > > + */ > > +bool pci_dev_is_alive(struct pci_dev *pdev) > > +{ > > + u16 vendor; > > + > > + pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor); > > + > > + return vendor == pdev->vendor; > > +} > > +EXPORT_SYMBOL(pci_dev_is_alive); > > Why not use the EXPORT_SYMBOL_GPL()? > > Krzysztof
