Re: [PATCH -v2 1/3] PCI: introduce PCIe Device Serial Number Capabilitysupport
From: Don Dutile
Date: Thu Jul 11 2013 - 18:18:25 EST
On 07/11/2013 04:09 PM, Bjorn Helgaas wrote:
On Thu, Jul 11, 2013 at 3:51 AM, Don Dutile<ddutile@xxxxxxxxxx> wrote:
On 07/11/2013 05:43 AM, Yijing Wang wrote:
Introduce PCIe Ext Capability Device Serial Number support,
so we can use the unique device serial number to identify
the physical device. During system suspend, if the PCIe
device was removed and inserted a new same device, after
system resume there is no good way to identify it, maybe
Device Serial Number is a good choice if device support.
Signed-off-by: Yijing Wang<wangyijing@xxxxxxxxxx>
---
drivers/pci/pci.c | 27 +++++++++++++++++++++++++++
drivers/pci/probe.c | 2 ++
include/linux/pci.h | 3 +++
3 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e37fea6..2e855b5 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2048,6 +2048,33 @@ void pci_free_cap_save_buffers(struct pci_dev *dev)
}
/**
+ * pci_device_serial_number - get device serial number
+ * @dev: the PCI device
+ *
+ * return the device serial number if device support,
+ * otherwise return 0.
+ */
+u64 pci_device_serial_number(struct pci_dev *dev)
See comment below:
void pci_device_serial_number(struct pci_dev *dev)
+{
+ int pos;
+ u64 sn;
+ u32 lo, hi;
+
+ if (!pci_is_pcie(dev))
+ return 0;
+
See comment below:
if (!pci_is_pcie(dev)) {
dev->sn = 0;
return;
}
+ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN);
+ if (!pos)
+ return 0;
+
+ pci_read_config_dword(dev, pos + 4,&lo);
+ pci_read_config_dword(dev, pos + 8,&hi);
+ sn = ((u64)hi<< 32) | lo;
See comment below:
dev->sn = ((u64)hi<< 32) | lo;
return;
+ return sn;
+}
+EXPORT_SYMBOL(pci_device_serial_number);
+
+/**
* pci_configure_ari - enable or disable ARI forwarding
* @dev: the PCI device
*
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 46ada5c..c4c1a2b 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1322,6 +1322,8 @@ static void pci_init_capabilities(struct pci_dev
*dev)
/* Power Management */
pci_pm_init(dev);
+ dev->sn = pci_device_serial_number(dev);
+
Finally, 'the comment below':
I know you were following Bjorn's suggestion, which I thought
was an improvement, but why not do above assignment in
pci_device_serial_number() ?
See above....
pci_device_serial_number() would then have the side-effect of saving
the result somewhere, and callers would have to know where to look.
Ah, like so many other features of a PCI device? -- what are all those
flags/status bits in pdev for ??? ;-)
Personally, I think it's simpler to return the serial number directly
and avoid the side-effect, but maybe this is just bike-shedding.
What struck me about rtning a value is in pci_init_capabilities(),
it was the only function with a rtn value that had to be put into a pci_dev
struct element,
while all the others stored their related capabilities in the pdev, or other struct
related to it.
So, maybe there should be a "pci_device_serial_number_init()" function
which does the storage in the pdev struct, and another (wrapper,
dare I shoot myself, inline) that is "pci_device_serial_number()" that returns
the value of the pdev's sn element, if another module or other core component
want to get it. .... and do that in a future patch... :-/
As long as we are bike-shedding, I'd just drop "sn" and do:
return ((u64)hi<< 32) | lo;
And of course, you need spaces before "&hi" and "&lo" in the
pci_read_config_dword() calls.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/