* [PATCH 0/5] pci: implement function to read Device Serial Number @ 2020-02-27 22:36 Jacob Keller 2020-02-27 22:36 ` [PATCH] ice-shared: add macro specifying max NVM offset Jacob Keller ` (5 more replies) 0 siblings, 6 replies; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev; +Cc: Jacob Keller Several drivers read the Device Serial Number from the PCIe extended configuration space. Each of these drivers implements a similar approach to finding the position and then extracting the 8 bytes of data. Implement a new helper function, pci_get_dsn, which can be used to extract this data into an 8 byte array. Modify the bnxt_en, qedf, ice, and ixgbe driver to use this new function. I left the implementation in the netronome nfp driver alone because they appear to extract parts of the DSN into separate locations and the transformation was not as obvious. The intent for this is to reduce duplicate code across the various drivers, and make it easier to write future code that wants to read the DSN. In particular the ice driver will be using the DSN as its serial number when implementing the DEVLINK_CMD_INFO_GET. I'm not entirely sure what tree these patches should go through, since it includes a core PCI change, as well as changes for both networking drivers and a scsi driver. Jacob Keller (5): pci: introduce pci_get_dsn bnxt_en: use pci_get_dsn scsi: qedf: use pci_get_dsn ice: use pci_get_dsn ixgbe: use pci_get_dsn drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 +++----- drivers/net/ethernet/intel/ice/ice_main.c | 32 ++++++++---------- drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 16 ++++----- drivers/pci/pci.c | 33 +++++++++++++++++++ drivers/scsi/qedf/qedf_main.c | 16 ++++----- include/linux/pci.h | 5 +++ 6 files changed, 68 insertions(+), 48 deletions(-) -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] ice-shared: add macro specifying max NVM offset 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller @ 2020-02-27 22:36 ` Jacob Keller 2020-02-27 22:43 ` Jacob Keller 2020-02-27 22:36 ` [PATCH 1/5] pci: introduce pci_get_dsn Jacob Keller ` (4 subsequent siblings) 5 siblings, 1 reply; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev; +Cc: Jacob Keller, ND Linux CI Server, Allan, Bruce W The ice_aq_read_nvm function uses a somewhat weird construction for verifying that the incoming offset is valid. Replace this construction with a simple greater-than expression, and define the maximum value (24bits) in the ice_adminq_cmd.h By providing a macro, the check becomes more clear. Additionally the maximum offset can be used in other locations. Change-Id: I50993e53aca7c073d6906f48f8c9a6ecc05248d4 Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Title: ice-shared: add macro specifying max NVM offset Change-type: ImplementationChange Reviewed-on: https://git-amr-3.devtools.intel.com/gerrit/250507 Tested-by: ND Linux CI Server <nd.linux.ci.server@intel.com> Reviewed-by: Allan, Bruce W <bruce.w.allan@intel.com> --- ice_adminq_cmd.h | 1 + ice_nvm.c | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ice_adminq_cmd.h b/ice_adminq_cmd.h index 38134e9dab11..fbbe676d1f7d 100644 --- a/ice_adminq_cmd.h +++ b/ice_adminq_cmd.h @@ -2778,6 +2778,7 @@ ICE_CHECK_PARAM_LEN(ice_aqc_sff_eeprom); * NVM Shadow RAM Dump commands (direct 0x0707) */ struct ice_aqc_nvm { +#define ICE_AQC_NVM_MAX_OFFSET 0xFFFFFF __le16 offset_low; u8 offset_high; #ifdef PREBOOT_SUPPORT diff --git a/ice_nvm.c b/ice_nvm.c index b075a05eb38f..0e6d8390deb8 100644 --- a/ice_nvm.c +++ b/ice_nvm.c @@ -35,8 +35,7 @@ ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length, cmd = &desc.params.nvm; - /* In offset the highest byte must be zeroed. */ - if (offset & 0xFF000000) + if (offset > ICE_AQC_NVM_MAX_OFFSET) return ICE_ERR_PARAM; ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_nvm_read); -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] ice-shared: add macro specifying max NVM offset 2020-02-27 22:36 ` [PATCH] ice-shared: add macro specifying max NVM offset Jacob Keller @ 2020-02-27 22:43 ` Jacob Keller 0 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:43 UTC (permalink / raw) To: jacob.e.keller; +Cc: Allan, linux-pci, nd.linux.ci.server, netdev This patch was sent in error. Please ignore it, as it's not part of the series. Thanks, Jake ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/5] pci: introduce pci_get_dsn 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller 2020-02-27 22:36 ` [PATCH] ice-shared: add macro specifying max NVM offset Jacob Keller @ 2020-02-27 22:36 ` Jacob Keller 2020-03-01 5:27 ` David Miller 2020-03-02 22:25 ` Bjorn Helgaas 2020-02-27 22:36 ` [PATCH 2/5] bnxt_en: use pci_get_dsn Jacob Keller ` (3 subsequent siblings) 5 siblings, 2 replies; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev Cc: Jacob Keller, Bjorn Helgaas, Jeff Kirsher, QLogic-Storage-Upstream, Michael Chan Several device drivers read their Device Serial Number from the PCIe extended config space. Introduce a new helper function, pci_get_dsn, which will read the eight bytes of the DSN into the provided buffer. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Cc: QLogic-Storage-Upstream@cavium.com Cc: Michael Chan <michael.chan@broadcom.com> --- drivers/pci/pci.c | 33 +++++++++++++++++++++++++++++++++ include/linux/pci.h | 5 +++++ 2 files changed, 38 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d828ca835a98..12d8101724d7 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -33,6 +33,7 @@ #include <linux/pci-ats.h> #include <asm/setup.h> #include <asm/dma.h> +#include <asm/unaligned.h> #include <linux/aer.h> #include "pci.h" @@ -557,6 +558,38 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap) } EXPORT_SYMBOL_GPL(pci_find_ext_capability); +/** + * pci_get_dsn - Read the 8-byte Device Serial Number + * @dev: PCI device to query + * @dsn: storage for the DSN. Must be at least 8 bytes + * + * Looks up the PCI_EXT_CAP_ID_DSN and reads the 8 bytes into the dsn storage. + * Returns -EOPNOTSUPP if the device does not have the capability. + */ +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) +{ + u32 dword; + int pos; + + + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); + if (!pos) + return -EOPNOTSUPP; + + /* + * The Device Serial Number is two dwords offset 4 bytes from the + * capability position. + */ + pos += 4; + pci_read_config_dword(dev, pos, &dword); + put_unaligned_le32(dword, &dsn[0]); + pci_read_config_dword(dev, pos + 4, &dword); + put_unaligned_le32(dword, &dsn[4]); + + return 0; +} +EXPORT_SYMBOL_GPL(pci_get_dsn); + static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap) { int rc, ttl = PCI_FIND_CAP_TTL; diff --git a/include/linux/pci.h b/include/linux/pci.h index 3840a541a9de..883562323df3 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1045,6 +1045,8 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap); int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap); struct pci_bus *pci_find_next_bus(const struct pci_bus *from); +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]); + struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, struct pci_dev *from); struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, @@ -1699,6 +1701,9 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) { return 0; } +static inline int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) +{ return -EOPNOTSUPP; } + /* Power management related routines */ static inline int pci_save_state(struct pci_dev *dev) { return 0; } static inline void pci_restore_state(struct pci_dev *dev) { } -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-02-27 22:36 ` [PATCH 1/5] pci: introduce pci_get_dsn Jacob Keller @ 2020-03-01 5:27 ` David Miller 2020-03-02 19:58 ` Jacob Keller 2020-03-02 22:25 ` Bjorn Helgaas 1 sibling, 1 reply; 28+ messages in thread From: David Miller @ 2020-03-01 5:27 UTC (permalink / raw) To: jacob.e.keller Cc: linux-pci, netdev, bhelgaas, jeffrey.t.kirsher, QLogic-Storage-Upstream, michael.chan From: Jacob Keller <jacob.e.keller@intel.com> Date: Thu, 27 Feb 2020 14:36:31 -0800 > +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) > +{ > + u32 dword; > + int pos; > + > + Just one empty line after the local variable declarations please. Thank you. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-03-01 5:27 ` David Miller @ 2020-03-02 19:58 ` Jacob Keller 0 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-02 19:58 UTC (permalink / raw) To: David Miller Cc: linux-pci, netdev, bhelgaas, jeffrey.t.kirsher, QLogic-Storage-Upstream, michael.chan On 2/29/2020 9:27 PM, David Miller wrote: > From: Jacob Keller <jacob.e.keller@intel.com> > Date: Thu, 27 Feb 2020 14:36:31 -0800 > >> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) >> +{ >> + u32 dword; >> + int pos; >> + >> + > > Just one empty line after the local variable declarations please. > > Thank you. > I've fixed this locally, but am going to wait to see if there is any further feedback before sending a v2. Thanks, Jake ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-02-27 22:36 ` [PATCH 1/5] pci: introduce pci_get_dsn Jacob Keller 2020-03-01 5:27 ` David Miller @ 2020-03-02 22:25 ` Bjorn Helgaas 2020-03-02 22:33 ` Jacob Keller 1 sibling, 1 reply; 28+ messages in thread From: Bjorn Helgaas @ 2020-03-02 22:25 UTC (permalink / raw) To: Jacob Keller Cc: linux-pci, netdev, Jeff Kirsher, QLogic-Storage-Upstream, Michael Chan PCI: Introduce pci_get_dsn() I learned this from "git log --oneline drivers/pci/pci.c". It looks like the other patches could benefit from this as well. On Thu, Feb 27, 2020 at 02:36:31PM -0800, Jacob Keller wrote: > Several device drivers read their Device Serial Number from the PCIe > extended config space. > > Introduce a new helper function, pci_get_dsn, which will read the > eight bytes of the DSN into the provided buffer. "pci_get_dsn()" > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > Cc: QLogic-Storage-Upstream@cavium.com > Cc: Michael Chan <michael.chan@broadcom.com> > --- > drivers/pci/pci.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/pci.h | 5 +++++ > 2 files changed, 38 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index d828ca835a98..12d8101724d7 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -33,6 +33,7 @@ > #include <linux/pci-ats.h> > #include <asm/setup.h> > #include <asm/dma.h> > +#include <asm/unaligned.h> > #include <linux/aer.h> > #include "pci.h" > > @@ -557,6 +558,38 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap) > } > EXPORT_SYMBOL_GPL(pci_find_ext_capability); > > +/** > + * pci_get_dsn - Read the 8-byte Device Serial Number > + * @dev: PCI device to query > + * @dsn: storage for the DSN. Must be at least 8 bytes > + * > + * Looks up the PCI_EXT_CAP_ID_DSN and reads the 8 bytes into the dsn storage. > + * Returns -EOPNOTSUPP if the device does not have the capability. > + */ > +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) > +{ > + u32 dword; > + int pos; > + > + > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); > + if (!pos) > + return -EOPNOTSUPP; > + > + /* > + * The Device Serial Number is two dwords offset 4 bytes from the > + * capability position. > + */ > + pos += 4; > + pci_read_config_dword(dev, pos, &dword); > + put_unaligned_le32(dword, &dsn[0]); > + pci_read_config_dword(dev, pos + 4, &dword); > + put_unaligned_le32(dword, &dsn[4]); Since the serial number is a 64-bit value, can we just return a u64 and let the caller worry about any alignment and byte-order issues? This would be the only use of asm/unaligned.h in driver/pci, and I don't think DSN should be that special. I think it's OK if we return 0 if the device doesn't have a DSN capability. A DSN that actually contains a zero serial number would be dubious at best. > + return 0; > +} > +EXPORT_SYMBOL_GPL(pci_get_dsn); > + > static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap) > { > int rc, ttl = PCI_FIND_CAP_TTL; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 3840a541a9de..883562323df3 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1045,6 +1045,8 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap); > int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap); > struct pci_bus *pci_find_next_bus(const struct pci_bus *from); > > +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]); > + > struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, > struct pci_dev *from); > struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, > @@ -1699,6 +1701,9 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, > static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) > { return 0; } > > +static inline int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) > +{ return -EOPNOTSUPP; } > + > /* Power management related routines */ > static inline int pci_save_state(struct pci_dev *dev) { return 0; } > static inline void pci_restore_state(struct pci_dev *dev) { } > -- > 2.25.0.368.g28a2d05eebfb > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-03-02 22:25 ` Bjorn Helgaas @ 2020-03-02 22:33 ` Jacob Keller 2020-03-02 23:20 ` Bjorn Helgaas 0 siblings, 1 reply; 28+ messages in thread From: Jacob Keller @ 2020-03-02 22:33 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, netdev, Jeff Kirsher, QLogic-Storage-Upstream, Michael Chan On 3/2/2020 2:25 PM, Bjorn Helgaas wrote: > PCI: Introduce pci_get_dsn() > > I learned this from "git log --oneline drivers/pci/pci.c". It looks > like the other patches could benefit from this as well. > Sure, will follow that precedent. > On Thu, Feb 27, 2020 at 02:36:31PM -0800, Jacob Keller wrote: >> Several device drivers read their Device Serial Number from the PCIe >> extended config space. >> >> Introduce a new helper function, pci_get_dsn, which will read the >> eight bytes of the DSN into the provided buffer. > > "pci_get_dsn()" > >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> >> Cc: Bjorn Helgaas <bhelgaas@google.com> >> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> >> Cc: QLogic-Storage-Upstream@cavium.com >> Cc: Michael Chan <michael.chan@broadcom.com> >> --- >> drivers/pci/pci.c | 33 +++++++++++++++++++++++++++++++++ >> include/linux/pci.h | 5 +++++ >> 2 files changed, 38 insertions(+) >> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index d828ca835a98..12d8101724d7 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -33,6 +33,7 @@ >> #include <linux/pci-ats.h> >> #include <asm/setup.h> >> #include <asm/dma.h> >> +#include <asm/unaligned.h> >> #include <linux/aer.h> >> #include "pci.h" >> >> @@ -557,6 +558,38 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap) >> } >> EXPORT_SYMBOL_GPL(pci_find_ext_capability); >> >> +/** >> + * pci_get_dsn - Read the 8-byte Device Serial Number >> + * @dev: PCI device to query >> + * @dsn: storage for the DSN. Must be at least 8 bytes >> + * >> + * Looks up the PCI_EXT_CAP_ID_DSN and reads the 8 bytes into the dsn storage. >> + * Returns -EOPNOTSUPP if the device does not have the capability. >> + */ >> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) >> +{ >> + u32 dword; >> + int pos; >> + >> + >> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); >> + if (!pos) >> + return -EOPNOTSUPP; >> + >> + /* >> + * The Device Serial Number is two dwords offset 4 bytes from the >> + * capability position. >> + */ >> + pos += 4; >> + pci_read_config_dword(dev, pos, &dword); >> + put_unaligned_le32(dword, &dsn[0]); >> + pci_read_config_dword(dev, pos + 4, &dword); >> + put_unaligned_le32(dword, &dsn[4]); > > Since the serial number is a 64-bit value, can we just return a u64 > and let the caller worry about any alignment and byte-order issues? > > This would be the only use of asm/unaligned.h in driver/pci, and I > don't think DSN should be that special. I suppose that's fair, but it ends up leaving most callers having to fix this immediately after calling this function. > > I think it's OK if we return 0 if the device doesn't have a DSN > capability. A DSN that actually contains a zero serial number would > be dubious at best. Hmm. I was trying to match how pre-existing code behaved, based on the ice and bnxt drivers. By returning 0s, we'd have to then perform a memcmp or something to catch it. > >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(pci_get_dsn); >> + >> static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap) >> { >> int rc, ttl = PCI_FIND_CAP_TTL; >> diff --git a/include/linux/pci.h b/include/linux/pci.h >> index 3840a541a9de..883562323df3 100644 >> --- a/include/linux/pci.h >> +++ b/include/linux/pci.h >> @@ -1045,6 +1045,8 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap); >> int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap); >> struct pci_bus *pci_find_next_bus(const struct pci_bus *from); >> >> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]); >> + >> struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, >> struct pci_dev *from); >> struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, >> @@ -1699,6 +1701,9 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, >> static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) >> { return 0; } >> >> +static inline int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) >> +{ return -EOPNOTSUPP; } >> + >> /* Power management related routines */ >> static inline int pci_save_state(struct pci_dev *dev) { return 0; } >> static inline void pci_restore_state(struct pci_dev *dev) { } >> -- >> 2.25.0.368.g28a2d05eebfb >> ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-03-02 22:33 ` Jacob Keller @ 2020-03-02 23:20 ` Bjorn Helgaas 2020-03-02 23:24 ` Jacob Keller 0 siblings, 1 reply; 28+ messages in thread From: Bjorn Helgaas @ 2020-03-02 23:20 UTC (permalink / raw) To: Jacob Keller Cc: linux-pci, netdev, Jeff Kirsher, QLogic-Storage-Upstream, Michael Chan On Mon, Mar 02, 2020 at 02:33:12PM -0800, Jacob Keller wrote: > On 3/2/2020 2:25 PM, Bjorn Helgaas wrote: > >> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) > >> +{ > >> + u32 dword; > >> + int pos; > >> + > >> + > >> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); > >> + if (!pos) > >> + return -EOPNOTSUPP; > >> + > >> + /* > >> + * The Device Serial Number is two dwords offset 4 bytes from the > >> + * capability position. > >> + */ > >> + pos += 4; > >> + pci_read_config_dword(dev, pos, &dword); > >> + put_unaligned_le32(dword, &dsn[0]); > >> + pci_read_config_dword(dev, pos + 4, &dword); > >> + put_unaligned_le32(dword, &dsn[4]); > > > > Since the serial number is a 64-bit value, can we just return a u64 > > and let the caller worry about any alignment and byte-order issues? > > > > This would be the only use of asm/unaligned.h in driver/pci, and I > > don't think DSN should be that special. > > I suppose that's fair, but it ends up leaving most callers having to fix > this immediately after calling this function. PCIe doesn't impose any structure on the value; it just says the first dword is the lower DW and the second is the upper DW. As long as we put that together correctly into a u64, I think further interpretation is caller-specific. > > I think it's OK if we return 0 if the device doesn't have a DSN > > capability. A DSN that actually contains a zero serial number would > > be dubious at best. > > Hmm. I was trying to match how pre-existing code behaved, based on the > ice and bnxt drivers. > > By returning 0s, we'd have to then perform a memcmp or something to > catch it. Can you just do this: dsn = pci_get_dsn(pdev); if (!dsn) return NULL; snprintf(opt_fw_filename, ...); return opt_fw_filename; Or am I missing something? > >> + return 0; > >> +} > >> +EXPORT_SYMBOL_GPL(pci_get_dsn); > >> + > >> static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap) > >> { > >> int rc, ttl = PCI_FIND_CAP_TTL; > >> diff --git a/include/linux/pci.h b/include/linux/pci.h > >> index 3840a541a9de..883562323df3 100644 > >> --- a/include/linux/pci.h > >> +++ b/include/linux/pci.h > >> @@ -1045,6 +1045,8 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap); > >> int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap); > >> struct pci_bus *pci_find_next_bus(const struct pci_bus *from); > >> > >> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]); > >> + > >> struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, > >> struct pci_dev *from); > >> struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, > >> @@ -1699,6 +1701,9 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, > >> static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) > >> { return 0; } > >> > >> +static inline int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) > >> +{ return -EOPNOTSUPP; } > >> + > >> /* Power management related routines */ > >> static inline int pci_save_state(struct pci_dev *dev) { return 0; } > >> static inline void pci_restore_state(struct pci_dev *dev) { } > >> -- > >> 2.25.0.368.g28a2d05eebfb > >> ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-03-02 23:20 ` Bjorn Helgaas @ 2020-03-02 23:24 ` Jacob Keller 2020-03-02 23:39 ` Bjorn Helgaas 0 siblings, 1 reply; 28+ messages in thread From: Jacob Keller @ 2020-03-02 23:24 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, netdev, Jeff Kirsher, QLogic-Storage-Upstream, Michael Chan On 3/2/2020 3:20 PM, Bjorn Helgaas wrote: > On Mon, Mar 02, 2020 at 02:33:12PM -0800, Jacob Keller wrote: >> On 3/2/2020 2:25 PM, Bjorn Helgaas wrote: > >>>> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) >>>> +{ >>>> + u32 dword; >>>> + int pos; >>>> + >>>> + >>>> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); >>>> + if (!pos) >>>> + return -EOPNOTSUPP; >>>> + >>>> + /* >>>> + * The Device Serial Number is two dwords offset 4 bytes from the >>>> + * capability position. >>>> + */ >>>> + pos += 4; >>>> + pci_read_config_dword(dev, pos, &dword); >>>> + put_unaligned_le32(dword, &dsn[0]); >>>> + pci_read_config_dword(dev, pos + 4, &dword); >>>> + put_unaligned_le32(dword, &dsn[4]); >>> >>> Since the serial number is a 64-bit value, can we just return a u64 >>> and let the caller worry about any alignment and byte-order issues? >>> >>> This would be the only use of asm/unaligned.h in driver/pci, and I >>> don't think DSN should be that special. >> >> I suppose that's fair, but it ends up leaving most callers having to fix >> this immediately after calling this function. > > PCIe doesn't impose any structure on the value; it just says the first > dword is the lower DW and the second is the upper DW. As long as we > put that together correctly into a u64, I think further interpretation > is caller-specific. > Makes sense. So basically, convert pci_get_dsn to a simply return a u64 instead of copying to an array, and then make callers assume that a value of 0 is invalid? Thanks, Jake ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] pci: introduce pci_get_dsn 2020-03-02 23:24 ` Jacob Keller @ 2020-03-02 23:39 ` Bjorn Helgaas 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller 0 siblings, 1 reply; 28+ messages in thread From: Bjorn Helgaas @ 2020-03-02 23:39 UTC (permalink / raw) To: Jacob Keller Cc: Bjorn Helgaas, linux-pci, netdev, Jeff Kirsher, QLogic-Storage-Upstream, Michael Chan On Mon, Mar 2, 2020 at 5:24 PM Jacob Keller <jacob.e.keller@intel.com> wrote: > > On 3/2/2020 3:20 PM, Bjorn Helgaas wrote: > > On Mon, Mar 02, 2020 at 02:33:12PM -0800, Jacob Keller wrote: > >> On 3/2/2020 2:25 PM, Bjorn Helgaas wrote: > > > >>>> +int pci_get_dsn(struct pci_dev *dev, u8 dsn[]) > >>>> +{ > >>>> + u32 dword; > >>>> + int pos; > >>>> + > >>>> + > >>>> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); > >>>> + if (!pos) > >>>> + return -EOPNOTSUPP; > >>>> + > >>>> + /* > >>>> + * The Device Serial Number is two dwords offset 4 bytes from the > >>>> + * capability position. > >>>> + */ > >>>> + pos += 4; > >>>> + pci_read_config_dword(dev, pos, &dword); > >>>> + put_unaligned_le32(dword, &dsn[0]); > >>>> + pci_read_config_dword(dev, pos + 4, &dword); > >>>> + put_unaligned_le32(dword, &dsn[4]); > >>> > >>> Since the serial number is a 64-bit value, can we just return a u64 > >>> and let the caller worry about any alignment and byte-order issues? > >>> > >>> This would be the only use of asm/unaligned.h in driver/pci, and I > >>> don't think DSN should be that special. > >> > >> I suppose that's fair, but it ends up leaving most callers having to fix > >> this immediately after calling this function. > > > > PCIe doesn't impose any structure on the value; it just says the first > > dword is the lower DW and the second is the upper DW. As long as we > > put that together correctly into a u64, I think further interpretation > > is caller-specific. > > Makes sense. So basically, convert pci_get_dsn to a simply return a u64 > instead of copying to an array, and then make callers assume that a > value of 0 is invalid? Yep, that's what I would do. You might have to re-jigger the snprintfs so they still pull out the same bytes they did before. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 0/6] PCI: Implement function to read Device Serial Number 2020-03-02 23:39 ` Bjorn Helgaas @ 2020-03-03 2:24 ` Jacob Keller 2020-03-03 2:25 ` [PATCH v2 1/6] PCI: Introduce pci_get_dsn Jacob Keller ` (7 more replies) 0 siblings, 8 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:24 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller Several drivers read the Device Serial Number from the PCIe extended configuration space. Each of these drivers implements a similar approach to finding the position and then extracting the 8 bytes of data. Implement a new helper function, pci_get_dsn, which can be used to extract this data into an 8 byte array. Modify the bnxt_en, qedf, ice, ixgbe and nfp drivers to use this new function. The intent for this is to reduce duplicate code across the various drivers, and make it easier to write future code that wants to read the DSN. In particular the ice driver will be using the DSN as its serial number when implementing the DEVLINK_CMD_INFO_GET. The new implementation in v2 significantly simplifies some of the callers which just want to print the value out in MSB order. By returning things as a u64 in CPU Endian order, the "%016llX" printf format specifier can be used to correctly format the value. Per patch changes since v1 PCI: Introduce pci_get_dsn * Update commit message based on feedback from Bjorn Helgaas * Modify the function to return a u64 (zero on no capability) * This new implementation ensures that the first dword is the lower 32 bits and the second dword is the upper 32 bits. bnxt_en: Use pci_get_dsn() * Use the u64 return value from pci_get_dsn() * Copy it into the dsn[] array by using put_unaligned_le64 * Fix a pre-existing typo in the netdev_info error message scsi: qedf: Use pci_get_dsn() * Use the u64 return value from pci_get_dsn() * simplify the snprintf to use "%016llX" * remove the unused 'i' variable ice: Use pci_get_dsn() * Use the u64 return value from pci_get_dsn() * simplify the snprintf to use "%016llX" ixgbe: Use pci_get_dsn() * Use the u64 return value from pci_get_dsn() * simplify the snprintf to use "%016llX" nfp: Use pci_get_dsn() * Added in v2 Jacob Keller (6): PCI: Introduce pci_get_dsn bnxt_en: Use pci_get_dsn() scsi: qedf: Use pci_get_dsn() ice: Use pci_get_dsn() ixgbe: Use pci_get_dsn() nfp: Use pci_get_dsn() drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 +++----- drivers/net/ethernet/intel/ice/ice_main.c | 30 ++++++---------- drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 18 ++++------ .../netronome/nfp/nfpcore/nfp6000_pcie.c | 24 +++++-------- drivers/pci/pci.c | 34 +++++++++++++++++++ drivers/scsi/qedf/qedf_main.c | 19 ++++------- include/linux/pci.h | 5 +++ 7 files changed, 76 insertions(+), 68 deletions(-) -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/6] PCI: Introduce pci_get_dsn 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller @ 2020-03-03 2:25 ` Jacob Keller 2020-03-04 22:42 ` Bjorn Helgaas 2020-03-03 2:25 ` [PATCH v2 2/6] bnxt_en: Use pci_get_dsn() Jacob Keller ` (6 subsequent siblings) 7 siblings, 1 reply; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:25 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller, Jeff Kirsher, Michael Chan Several device drivers read their Device Serial Number from the PCIe extended config space. Introduce a new helper function, pci_get_dsn(). This function reads the eight bytes of the DSN and returns them as a u64. If the capability does not exist for the device, the function returns 0. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Cc: Michael Chan <michael.chan@broadcom.com> --- drivers/pci/pci.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/pci.h | 5 +++++ 2 files changed, 39 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d828ca835a98..03f50e706c0d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -557,6 +557,40 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap) } EXPORT_SYMBOL_GPL(pci_find_ext_capability); +/** + * pci_get_dsn - Read and return the 8-byte Device Serial Number + * @dev: PCI device to query + * + * Looks up the PCI_EXT_CAP_ID_DSN and reads the 8 bytes of the Device Serial + * Number. + * + * Returns the DSN, or zero if the capability does not exist. + */ +u64 pci_get_dsn(struct pci_dev *dev) +{ + u32 dword; + u64 dsn; + int pos; + + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); + if (!pos) + return 0; + + /* + * The Device Serial Number is two dwords offset 4 bytes from the + * capability position. The specification says that the first dword is + * the lower half, and the second dword is the upper half. + */ + pos += 4; + pci_read_config_dword(dev, pos, &dword); + dsn = (u64)dword; + pci_read_config_dword(dev, pos + 4, &dword); + dsn |= ((u64)dword) << 32; + + return dsn; +} +EXPORT_SYMBOL_GPL(pci_get_dsn); + static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap) { int rc, ttl = PCI_FIND_CAP_TTL; diff --git a/include/linux/pci.h b/include/linux/pci.h index 3840a541a9de..d92cf2da1ae1 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1045,6 +1045,8 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap); int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap); struct pci_bus *pci_find_next_bus(const struct pci_bus *from); +u64 pci_get_dsn(struct pci_dev *dev); + struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, struct pci_dev *from); struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, @@ -1699,6 +1701,9 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) { return 0; } +static inline u64 pci_get_dsn(struct pci_dev *dev) +{ return 0; } + /* Power management related routines */ static inline int pci_save_state(struct pci_dev *dev) { return 0; } static inline void pci_restore_state(struct pci_dev *dev) { } -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/6] PCI: Introduce pci_get_dsn 2020-03-03 2:25 ` [PATCH v2 1/6] PCI: Introduce pci_get_dsn Jacob Keller @ 2020-03-04 22:42 ` Bjorn Helgaas 0 siblings, 0 replies; 28+ messages in thread From: Bjorn Helgaas @ 2020-03-04 22:42 UTC (permalink / raw) To: Jacob Keller Cc: linux-pci, netdev, Jakub Kicinski, David Miller, Jeff Kirsher, Michael Chan On Mon, Mar 02, 2020 at 06:25:00PM -0800, Jacob Keller wrote: > Several device drivers read their Device Serial Number from the PCIe > extended config space. > > Introduce a new helper function, pci_get_dsn(). This function reads the > eight bytes of the DSN and returns them as a u64. If the capability does not > exist for the device, the function returns 0. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > Cc: Michael Chan <michael.chan@broadcom.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> Looks good, thanks! Feel free to merge this via the net tree. > --- > drivers/pci/pci.c | 34 ++++++++++++++++++++++++++++++++++ > include/linux/pci.h | 5 +++++ > 2 files changed, 39 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index d828ca835a98..03f50e706c0d 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -557,6 +557,40 @@ int pci_find_ext_capability(struct pci_dev *dev, int cap) > } > EXPORT_SYMBOL_GPL(pci_find_ext_capability); > > +/** > + * pci_get_dsn - Read and return the 8-byte Device Serial Number > + * @dev: PCI device to query > + * > + * Looks up the PCI_EXT_CAP_ID_DSN and reads the 8 bytes of the Device Serial > + * Number. > + * > + * Returns the DSN, or zero if the capability does not exist. > + */ > +u64 pci_get_dsn(struct pci_dev *dev) > +{ > + u32 dword; > + u64 dsn; > + int pos; > + > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DSN); > + if (!pos) > + return 0; > + > + /* > + * The Device Serial Number is two dwords offset 4 bytes from the > + * capability position. The specification says that the first dword is > + * the lower half, and the second dword is the upper half. > + */ > + pos += 4; > + pci_read_config_dword(dev, pos, &dword); > + dsn = (u64)dword; > + pci_read_config_dword(dev, pos + 4, &dword); > + dsn |= ((u64)dword) << 32; > + > + return dsn; > +} > +EXPORT_SYMBOL_GPL(pci_get_dsn); > + > static int __pci_find_next_ht_cap(struct pci_dev *dev, int pos, int ht_cap) > { > int rc, ttl = PCI_FIND_CAP_TTL; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 3840a541a9de..d92cf2da1ae1 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1045,6 +1045,8 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap); > int pci_find_next_ht_capability(struct pci_dev *dev, int pos, int ht_cap); > struct pci_bus *pci_find_next_bus(const struct pci_bus *from); > > +u64 pci_get_dsn(struct pci_dev *dev); > + > struct pci_dev *pci_get_device(unsigned int vendor, unsigned int device, > struct pci_dev *from); > struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device, > @@ -1699,6 +1701,9 @@ static inline int pci_find_next_capability(struct pci_dev *dev, u8 post, > static inline int pci_find_ext_capability(struct pci_dev *dev, int cap) > { return 0; } > > +static inline u64 pci_get_dsn(struct pci_dev *dev) > +{ return 0; } > + > /* Power management related routines */ > static inline int pci_save_state(struct pci_dev *dev) { return 0; } > static inline void pci_restore_state(struct pci_dev *dev) { } > -- > 2.25.0.368.g28a2d05eebfb > ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/6] bnxt_en: Use pci_get_dsn() 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller 2020-03-03 2:25 ` [PATCH v2 1/6] PCI: Introduce pci_get_dsn Jacob Keller @ 2020-03-03 2:25 ` Jacob Keller 2020-03-03 2:25 ` [PATCH v2 3/6] scsi: qedf: " Jacob Keller ` (5 subsequent siblings) 7 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:25 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller, Michael Chan Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn(). Use of put_unaligned_le64 should be correct. pci_get_dsn() will perform two pci_read_config_dword calls. The first dword will be placed in the first 32 bits of the u64, while the second dword will be placed in the upper 32 bits of the u64. On Little Endian systems, the least significant byte comes first, which will be the least significant byte of the first dword, followed by the least significant byte of the second dword. Since the _le32 variations do not perform byte swapping, we will correctly copy the dwords into the dsn[] array in the same order as before. On Big Endian systems, the most significant byte of the second dword will come first. put_unaligned_le64 will perform a CPU_TO_LE64, which will swap things correctly before copying. This should also end up with the correct bytes in the dsn[] array. While at it, fix a small typo in the netdev_info error message when the DSN cannot be read. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Michael Chan <michael.chan@broadcom.com> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 597e6fd5bfea..49874079084d 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -11755,20 +11755,16 @@ static int bnxt_init_mac_addr(struct bnxt *bp) static int bnxt_pcie_dsn_get(struct bnxt *bp, u8 dsn[]) { struct pci_dev *pdev = bp->pdev; - int pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); - u32 dw; + u64 qword; - if (!pos) { - netdev_info(bp->dev, "Unable do read adapter's DSN"); + qword = pci_get_dsn(pdev); + if (!qword) { + netdev_info(bp->dev, "Unable to read adapter's DSN"); return -EOPNOTSUPP; } - /* DSN (two dw) is at an offset of 4 from the cap pos */ - pos += 4; - pci_read_config_dword(pdev, pos, &dw); - put_unaligned_le32(dw, &dsn[0]); - pci_read_config_dword(pdev, pos + 4, &dw); - put_unaligned_le32(dw, &dsn[4]); + put_unaligned_le64(qword, dsn); + bp->flags |= BNXT_FLAG_DSN_VALID; return 0; } -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/6] scsi: qedf: Use pci_get_dsn() 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller 2020-03-03 2:25 ` [PATCH v2 1/6] PCI: Introduce pci_get_dsn Jacob Keller 2020-03-03 2:25 ` [PATCH v2 2/6] bnxt_en: Use pci_get_dsn() Jacob Keller @ 2020-03-03 2:25 ` Jacob Keller 2020-03-03 2:25 ` [PATCH v2 4/6] ice: " Jacob Keller ` (4 subsequent siblings) 7 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:25 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn(). The original code used a for-loop that looped over each of the 8 bytes and copied them into a temporary buffer. pci_get_dsn() uses two calls to pci_read_config_dword, and correctly bitwise ORs them into a u64. Thus, we can simplify the snprintf significantly using %016llX on a u64 value. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- The QLogic-Storage-Upstream@cavium.com appears to be a dud. I'm not sure who maintains this driver, and thus am not sure who to add to the Cc. drivers/scsi/qedf/qedf_main.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c index 604856e72cfb..5b19f5175c5c 100644 --- a/drivers/scsi/qedf/qedf_main.c +++ b/drivers/scsi/qedf/qedf_main.c @@ -1577,8 +1577,7 @@ static void qedf_setup_fdmi(struct qedf_ctx *qedf) { struct fc_lport *lport = qedf->lport; struct fc_host_attrs *fc_host = shost_to_fc_host(lport->host); - u8 buf[8]; - int i, pos; + u64 dsn; /* * fdmi_enabled needs to be set for libfc to execute FDMI registration. @@ -1591,18 +1590,11 @@ static void qedf_setup_fdmi(struct qedf_ctx *qedf) */ /* Get the PCI-e Device Serial Number Capability */ - pos = pci_find_ext_capability(qedf->pdev, PCI_EXT_CAP_ID_DSN); - if (pos) { - pos += 4; - for (i = 0; i < 8; i++) - pci_read_config_byte(qedf->pdev, pos + i, &buf[i]); - + dsn = pci_get_dsn(qedf->pdev); + if (dsn) snprintf(fc_host->serial_number, - sizeof(fc_host->serial_number), - "%02X%02X%02X%02X%02X%02X%02X%02X", - buf[7], buf[6], buf[5], buf[4], - buf[3], buf[2], buf[1], buf[0]); - } else + sizeof(fc_host->serial_number), "%016llX", dsn); + else snprintf(fc_host->serial_number, sizeof(fc_host->serial_number), "Unknown"); -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/6] ice: Use pci_get_dsn() 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller ` (2 preceding siblings ...) 2020-03-03 2:25 ` [PATCH v2 3/6] scsi: qedf: " Jacob Keller @ 2020-03-03 2:25 ` Jacob Keller 2020-03-03 2:25 ` [PATCH v2 5/6] ixgbe: " Jacob Keller ` (3 subsequent siblings) 7 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:25 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller, Jeff Kirsher Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn(). The pci_get_dsn() function will perform two pci_read_config_dword calls to read the lower and upper config dwords. It bitwise ORs them into a u64 value. Instead of using put_unaligned_le32 to convert the value to LE32 format, just use the %016llX printf specifier. This will print the u64 correct, putting the most significant byte of the value first. Since pci_get_dsn() correctly orders the two dwords into a u64, this should produce equivalent results in less code. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> --- drivers/net/ethernet/intel/ice/ice_main.c | 30 +++++++++-------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 5ae671609f98..c633519b1555 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -3099,30 +3099,22 @@ static char *ice_get_opt_fw_name(struct ice_pf *pf) * followed by a EUI-64 identifier (PCIe Device Serial Number) */ struct pci_dev *pdev = pf->pdev; - char *opt_fw_filename = NULL; - u32 dword; - u8 dsn[8]; - int pos; + char *opt_fw_filename; + u64 dsn; /* Determine the name of the optional file using the DSN (two * dwords following the start of the DSN Capability). */ - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); - if (pos) { - opt_fw_filename = kzalloc(NAME_MAX, GFP_KERNEL); - if (!opt_fw_filename) - return NULL; + dsn = pci_get_dsn(pdev); + if (!dsn) + return NULL; - pci_read_config_dword(pdev, pos + 4, &dword); - put_unaligned_le32(dword, &dsn[0]); - pci_read_config_dword(pdev, pos + 8, &dword); - put_unaligned_le32(dword, &dsn[4]); - snprintf(opt_fw_filename, NAME_MAX, - "%sice-%02x%02x%02x%02x%02x%02x%02x%02x.pkg", - ICE_DDP_PKG_PATH, - dsn[7], dsn[6], dsn[5], dsn[4], - dsn[3], dsn[2], dsn[1], dsn[0]); - } + opt_fw_filename = kzalloc(NAME_MAX, GFP_KERNEL); + if (!opt_fw_filename) + return NULL; + + snprintf(opt_fw_filename, NAME_MAX, "%sice-%016llX.pkg", + ICE_DDP_PKG_PATH, dsn); return opt_fw_filename; } -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 5/6] ixgbe: Use pci_get_dsn() 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller ` (3 preceding siblings ...) 2020-03-03 2:25 ` [PATCH v2 4/6] ice: " Jacob Keller @ 2020-03-03 2:25 ` Jacob Keller 2020-03-03 2:25 ` [PATCH v2 6/6] nfp: " Jacob Keller ` (2 subsequent siblings) 7 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:25 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller, Jeff Kirsher Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn(). The original code used a simple for-loop to read the bytes in order into a buffer one byte at a time. The pci_get_dsn() function returns the DSN as a u64, correctly ordering the upper and lower 32 bit dwords. Simplify the display code by using %016llX to display the u64 DSN. This should have equivalent behavior on both Little and Big Endian systems. The bus will have correctly ordered the dwords in the CPU endian format, while pci_get_dsn() will correctly order the lower and higher dwords into a u64. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c index ccd852ad62a4..d733165c4a2e 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c @@ -968,8 +968,8 @@ int ixgbe_fcoe_get_hbainfo(struct net_device *netdev, { struct ixgbe_adapter *adapter = netdev_priv(netdev); struct ixgbe_hw *hw = &adapter->hw; - int i, pos; - u8 buf[8]; + u64 dsn; + int i; if (!info) return -EINVAL; @@ -985,17 +985,11 @@ int ixgbe_fcoe_get_hbainfo(struct net_device *netdev, /* Serial Number */ /* Get the PCI-e Device Serial Number Capability */ - pos = pci_find_ext_capability(adapter->pdev, PCI_EXT_CAP_ID_DSN); - if (pos) { - pos += 4; - for (i = 0; i < 8; i++) - pci_read_config_byte(adapter->pdev, pos + i, &buf[i]); - + dsn = pci_get_dsn(adapter->pdev); + if (dsn) snprintf(info->serial_number, sizeof(info->serial_number), - "%02X%02X%02X%02X%02X%02X%02X%02X", - buf[7], buf[6], buf[5], buf[4], - buf[3], buf[2], buf[1], buf[0]); - } else + "%016llX", dsn); + else snprintf(info->serial_number, sizeof(info->serial_number), "Unknown"); -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 6/6] nfp: Use pci_get_dsn() 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller ` (4 preceding siblings ...) 2020-03-03 2:25 ` [PATCH v2 5/6] ixgbe: " Jacob Keller @ 2020-03-03 2:25 ` Jacob Keller 2020-03-03 3:40 ` Jakub Kicinski 2020-03-04 22:28 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number David Miller 2020-03-06 1:30 ` David Miller 7 siblings, 1 reply; 28+ messages in thread From: Jacob Keller @ 2020-03-03 2:25 UTC (permalink / raw) To: linux-pci, netdev, Bjorn Helgaas Cc: Jakub Kicinski, David Miller, Jacob Keller Use the newly added pci_get_dsn() function for obtaining the 64-bit Device Serial Number in the nfp6000_read_serial and nfp_6000_get_interface functions. pci_get_dsn() reports the Device Serial number as a u64 value created by combining two pci_read_config_dword functions. The lower 16 bits represent the device interface value, and the next 48 bits represent the serial value. Use put_unaligned_be32 and put_unaligned_be16 to convert the serial value portion into a Big Endian formatted serial u8 array. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Jakub Kicinski <kuba@kernel.org> --- .../netronome/nfp/nfpcore/nfp6000_pcie.c | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c index b454db283aef..8fde6c1f681b 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c @@ -1247,19 +1247,16 @@ static void nfp6000_free(struct nfp_cpp *cpp) static int nfp6000_read_serial(struct device *dev, u8 *serial) { struct pci_dev *pdev = to_pci_dev(dev); - int pos; - u32 reg; + u64 dsn; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); - if (!pos) { + dsn = pci_get_dsn(pdev); + if (!dsn) { dev_err(dev, "can't find PCIe Serial Number Capability\n"); return -EINVAL; } - pci_read_config_dword(pdev, pos + 4, ®); - put_unaligned_be16(reg >> 16, serial + 4); - pci_read_config_dword(pdev, pos + 8, ®); - put_unaligned_be32(reg, serial); + put_unaligned_be32((u32)(dsn >> 32), serial); + put_unaligned_be16((u16)(dsn >> 16), serial + 4); return 0; } @@ -1267,18 +1264,15 @@ static int nfp6000_read_serial(struct device *dev, u8 *serial) static int nfp6000_get_interface(struct device *dev) { struct pci_dev *pdev = to_pci_dev(dev); - int pos; - u32 reg; + u64 dsn; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); - if (!pos) { + dsn = pci_get_dsn(pdev); + if (!dsn) { dev_err(dev, "can't find PCIe Serial Number Capability\n"); return -EINVAL; } - pci_read_config_dword(pdev, pos + 4, ®); - - return reg & 0xffff; + return dsn & 0xffff; } static const struct nfp_cpp_operations nfp6000_pcie_ops = { -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] nfp: Use pci_get_dsn() 2020-03-03 2:25 ` [PATCH v2 6/6] nfp: " Jacob Keller @ 2020-03-03 3:40 ` Jakub Kicinski 2020-03-03 17:36 ` Jacob Keller 0 siblings, 1 reply; 28+ messages in thread From: Jakub Kicinski @ 2020-03-03 3:40 UTC (permalink / raw) To: Jacob Keller; +Cc: linux-pci, netdev, Bjorn Helgaas, David Miller On Mon, 2 Mar 2020 18:25:05 -0800 Jacob Keller wrote: > Use the newly added pci_get_dsn() function for obtaining the 64-bit > Device Serial Number in the nfp6000_read_serial and > nfp_6000_get_interface functions. > > pci_get_dsn() reports the Device Serial number as a u64 value created by > combining two pci_read_config_dword functions. The lower 16 bits > represent the device interface value, and the next 48 bits represent the > serial value. Use put_unaligned_be32 and put_unaligned_be16 to convert > the serial value portion into a Big Endian formatted serial u8 array. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Cc: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jakub Kicinski <kuba@kernel.org> Thanks! > - pci_read_config_dword(pdev, pos + 4, ®); > - put_unaligned_be16(reg >> 16, serial + 4); > - pci_read_config_dword(pdev, pos + 8, ®); > - put_unaligned_be32(reg, serial); > + put_unaligned_be32((u32)(dsn >> 32), serial); > + put_unaligned_be16((u16)(dsn >> 16), serial + 4); nit: the casts and extra brackets should be unnecessary, in case you're respinning.. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/6] nfp: Use pci_get_dsn() 2020-03-03 3:40 ` Jakub Kicinski @ 2020-03-03 17:36 ` Jacob Keller 0 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-03-03 17:36 UTC (permalink / raw) To: Jakub Kicinski; +Cc: linux-pci, netdev, Bjorn Helgaas, David Miller On 3/2/2020 7:40 PM, Jakub Kicinski wrote: > On Mon, 2 Mar 2020 18:25:05 -0800 Jacob Keller wrote: >> Use the newly added pci_get_dsn() function for obtaining the 64-bit >> Device Serial Number in the nfp6000_read_serial and >> nfp_6000_get_interface functions. >> >> pci_get_dsn() reports the Device Serial number as a u64 value created by >> combining two pci_read_config_dword functions. The lower 16 bits >> represent the device interface value, and the next 48 bits represent the >> serial value. Use put_unaligned_be32 and put_unaligned_be16 to convert >> the serial value portion into a Big Endian formatted serial u8 array. >> >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> >> Cc: Jakub Kicinski <kuba@kernel.org> > > Reviewed-by: Jakub Kicinski <kuba@kernel.org> > > Thanks! > >> - pci_read_config_dword(pdev, pos + 4, ®); >> - put_unaligned_be16(reg >> 16, serial + 4); >> - pci_read_config_dword(pdev, pos + 8, ®); >> - put_unaligned_be32(reg, serial); >> + put_unaligned_be32((u32)(dsn >> 32), serial); >> + put_unaligned_be16((u16)(dsn >> 16), serial + 4); > > nit: the casts and extra brackets should be unnecessary, in case > you're respinning.. > Ah, yea because the argument will get converted properly by the function. It's a bit of a habit since we use one of the -W warnings that warns about implicit conversions that use truncation. I can remove them if I need to respin. It looks like this got picked up by one of the kbuild bots but got applied without the main PCI patch that implemented pci_get_dsn. Thanks, Jake ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/6] PCI: Implement function to read Device Serial Number 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller ` (5 preceding siblings ...) 2020-03-03 2:25 ` [PATCH v2 6/6] nfp: " Jacob Keller @ 2020-03-04 22:28 ` David Miller 2020-03-06 1:30 ` David Miller 7 siblings, 0 replies; 28+ messages in thread From: David Miller @ 2020-03-04 22:28 UTC (permalink / raw) To: jacob.e.keller; +Cc: linux-pci, netdev, bhelgaas, kuba From: Jacob Keller <jacob.e.keller@intel.com> Date: Mon, 2 Mar 2020 18:24:59 -0800 > Several drivers read the Device Serial Number from the PCIe extended > configuration space. Each of these drivers implements a similar approach to > finding the position and then extracting the 8 bytes of data. > > Implement a new helper function, pci_get_dsn, which can be used to extract > this data into an 8 byte array. ... Bjorn, another series that needs your review and could go via my networking tree. Please take a look, thanks. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/6] PCI: Implement function to read Device Serial Number 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller ` (6 preceding siblings ...) 2020-03-04 22:28 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number David Miller @ 2020-03-06 1:30 ` David Miller 7 siblings, 0 replies; 28+ messages in thread From: David Miller @ 2020-03-06 1:30 UTC (permalink / raw) To: jacob.e.keller; +Cc: linux-pci, netdev, bhelgaas, kuba From: Jacob Keller <jacob.e.keller@intel.com> Date: Mon, 2 Mar 2020 18:24:59 -0800 > Several drivers read the Device Serial Number from the PCIe extended > configuration space. Each of these drivers implements a similar approach to > finding the position and then extracting the 8 bytes of data. > > Implement a new helper function, pci_get_dsn, which can be used to extract > this data into an 8 byte array. ... Series applied to net-next, thanks. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/5] bnxt_en: use pci_get_dsn 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller 2020-02-27 22:36 ` [PATCH] ice-shared: add macro specifying max NVM offset Jacob Keller 2020-02-27 22:36 ` [PATCH 1/5] pci: introduce pci_get_dsn Jacob Keller @ 2020-02-27 22:36 ` Jacob Keller 2020-03-02 22:25 ` Bjorn Helgaas 2020-02-27 22:36 ` [PATCH 3/5] scsi: qedf: " Jacob Keller ` (2 subsequent siblings) 5 siblings, 1 reply; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev; +Cc: Jacob Keller, Michael Chan Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Michael Chan <michael.chan@broadcom.com> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 597e6fd5bfea..55f078fc067e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -11755,20 +11755,14 @@ static int bnxt_init_mac_addr(struct bnxt *bp) static int bnxt_pcie_dsn_get(struct bnxt *bp, u8 dsn[]) { struct pci_dev *pdev = bp->pdev; - int pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); - u32 dw; + int err; - if (!pos) { + err = pci_get_dsn(pdev, dsn); + if (err) { netdev_info(bp->dev, "Unable do read adapter's DSN"); - return -EOPNOTSUPP; + return err; } - /* DSN (two dw) is at an offset of 4 from the cap pos */ - pos += 4; - pci_read_config_dword(pdev, pos, &dw); - put_unaligned_le32(dw, &dsn[0]); - pci_read_config_dword(pdev, pos + 4, &dw); - put_unaligned_le32(dw, &dsn[4]); bp->flags |= BNXT_FLAG_DSN_VALID; return 0; } -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] bnxt_en: use pci_get_dsn 2020-02-27 22:36 ` [PATCH 2/5] bnxt_en: use pci_get_dsn Jacob Keller @ 2020-03-02 22:25 ` Bjorn Helgaas 0 siblings, 0 replies; 28+ messages in thread From: Bjorn Helgaas @ 2020-03-02 22:25 UTC (permalink / raw) To: Jacob Keller; +Cc: linux-pci, netdev, Michael Chan On Thu, Feb 27, 2020 at 02:36:32PM -0800, Jacob Keller wrote: > Replace the open-coded implementation for reading the PCIe DSN with > pci_get_dsn. > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > Cc: Michael Chan <michael.chan@broadcom.com> > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > index 597e6fd5bfea..55f078fc067e 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > @@ -11755,20 +11755,14 @@ static int bnxt_init_mac_addr(struct bnxt *bp) > static int bnxt_pcie_dsn_get(struct bnxt *bp, u8 dsn[]) > { > struct pci_dev *pdev = bp->pdev; > - int pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); > - u32 dw; > + int err; > > - if (!pos) { > + err = pci_get_dsn(pdev, dsn); > + if (err) { > netdev_info(bp->dev, "Unable do read adapter's DSN"); "Unable *to* read..." Not your fault, I guess :) > - return -EOPNOTSUPP; > + return err; > } > > - /* DSN (two dw) is at an offset of 4 from the cap pos */ > - pos += 4; > - pci_read_config_dword(pdev, pos, &dw); > - put_unaligned_le32(dw, &dsn[0]); > - pci_read_config_dword(pdev, pos + 4, &dw); > - put_unaligned_le32(dw, &dsn[4]); > bp->flags |= BNXT_FLAG_DSN_VALID; > return 0; > } > -- > 2.25.0.368.g28a2d05eebfb > ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] scsi: qedf: use pci_get_dsn 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller ` (2 preceding siblings ...) 2020-02-27 22:36 ` [PATCH 2/5] bnxt_en: use pci_get_dsn Jacob Keller @ 2020-02-27 22:36 ` Jacob Keller 2020-02-27 22:36 ` [PATCH 4/5] ice: " Jacob Keller 2020-02-27 22:36 ` [PATCH 5/5] ixgbe: " Jacob Keller 5 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev; +Cc: Jacob Keller, QLogic-Storage-Upstream Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: QLogic-Storage-Upstream@cavium.com --- drivers/scsi/qedf/qedf_main.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c index 604856e72cfb..6ef688ef465c 100644 --- a/drivers/scsi/qedf/qedf_main.c +++ b/drivers/scsi/qedf/qedf_main.c @@ -1578,7 +1578,7 @@ static void qedf_setup_fdmi(struct qedf_ctx *qedf) struct fc_lport *lport = qedf->lport; struct fc_host_attrs *fc_host = shost_to_fc_host(lport->host); u8 buf[8]; - int i, pos; + int i, err; /* * fdmi_enabled needs to be set for libfc to execute FDMI registration. @@ -1591,20 +1591,16 @@ static void qedf_setup_fdmi(struct qedf_ctx *qedf) */ /* Get the PCI-e Device Serial Number Capability */ - pos = pci_find_ext_capability(qedf->pdev, PCI_EXT_CAP_ID_DSN); - if (pos) { - pos += 4; - for (i = 0; i < 8; i++) - pci_read_config_byte(qedf->pdev, pos + i, &buf[i]); - + err = pci_get_dsn(qedf->pdev, buf); + if (err) + snprintf(fc_host->serial_number, + sizeof(fc_host->serial_number), "Unknown"); + else snprintf(fc_host->serial_number, sizeof(fc_host->serial_number), "%02X%02X%02X%02X%02X%02X%02X%02X", buf[7], buf[6], buf[5], buf[4], buf[3], buf[2], buf[1], buf[0]); - } else - snprintf(fc_host->serial_number, - sizeof(fc_host->serial_number), "Unknown"); snprintf(fc_host->manufacturer, sizeof(fc_host->manufacturer), "%s", "Cavium Inc."); -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/5] ice: use pci_get_dsn 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller ` (3 preceding siblings ...) 2020-02-27 22:36 ` [PATCH 3/5] scsi: qedf: " Jacob Keller @ 2020-02-27 22:36 ` Jacob Keller 2020-02-27 22:36 ` [PATCH 5/5] ixgbe: " Jacob Keller 5 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev; +Cc: Jacob Keller, Jeff Kirsher Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> --- drivers/net/ethernet/intel/ice/ice_main.c | 32 ++++++++++------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 5ae671609f98..77b56dcae331 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -3099,30 +3099,26 @@ static char *ice_get_opt_fw_name(struct ice_pf *pf) * followed by a EUI-64 identifier (PCIe Device Serial Number) */ struct pci_dev *pdev = pf->pdev; - char *opt_fw_filename = NULL; - u32 dword; + char *opt_fw_filename; u8 dsn[8]; - int pos; + int err; /* Determine the name of the optional file using the DSN (two * dwords following the start of the DSN Capability). */ - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_DSN); - if (pos) { - opt_fw_filename = kzalloc(NAME_MAX, GFP_KERNEL); - if (!opt_fw_filename) - return NULL; + err = pci_get_dsn(pdev, dsn); + if (err) + return NULL; - pci_read_config_dword(pdev, pos + 4, &dword); - put_unaligned_le32(dword, &dsn[0]); - pci_read_config_dword(pdev, pos + 8, &dword); - put_unaligned_le32(dword, &dsn[4]); - snprintf(opt_fw_filename, NAME_MAX, - "%sice-%02x%02x%02x%02x%02x%02x%02x%02x.pkg", - ICE_DDP_PKG_PATH, - dsn[7], dsn[6], dsn[5], dsn[4], - dsn[3], dsn[2], dsn[1], dsn[0]); - } + opt_fw_filename = kzalloc(NAME_MAX, GFP_KERNEL); + if (!opt_fw_filename) + return NULL; + + snprintf(opt_fw_filename, NAME_MAX, + "%sice-%02x%02x%02x%02x%02x%02x%02x%02x.pkg", + ICE_DDP_PKG_PATH, + dsn[7], dsn[6], dsn[5], dsn[4], + dsn[3], dsn[2], dsn[1], dsn[0]); return opt_fw_filename; } -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/5] ixgbe: use pci_get_dsn 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller ` (4 preceding siblings ...) 2020-02-27 22:36 ` [PATCH 4/5] ice: " Jacob Keller @ 2020-02-27 22:36 ` Jacob Keller 5 siblings, 0 replies; 28+ messages in thread From: Jacob Keller @ 2020-02-27 22:36 UTC (permalink / raw) To: linux-pci, netdev; +Cc: Jacob Keller, Jeff Kirsher Replace the open-coded implementation for reading the PCIe DSN with pci_get_dsn. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c index ccd852ad62a4..74ee12b87fc3 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c @@ -968,7 +968,7 @@ int ixgbe_fcoe_get_hbainfo(struct net_device *netdev, { struct ixgbe_adapter *adapter = netdev_priv(netdev); struct ixgbe_hw *hw = &adapter->hw; - int i, pos; + int i, err; u8 buf[8]; if (!info) @@ -985,19 +985,15 @@ int ixgbe_fcoe_get_hbainfo(struct net_device *netdev, /* Serial Number */ /* Get the PCI-e Device Serial Number Capability */ - pos = pci_find_ext_capability(adapter->pdev, PCI_EXT_CAP_ID_DSN); - if (pos) { - pos += 4; - for (i = 0; i < 8; i++) - pci_read_config_byte(adapter->pdev, pos + i, &buf[i]); - + err = pci_get_dsn(adapter->pdev, buf); + if (err) + snprintf(info->serial_number, sizeof(info->serial_number), + "Unknown"); + else snprintf(info->serial_number, sizeof(info->serial_number), "%02X%02X%02X%02X%02X%02X%02X%02X", buf[7], buf[6], buf[5], buf[4], buf[3], buf[2], buf[1], buf[0]); - } else - snprintf(info->serial_number, sizeof(info->serial_number), - "Unknown"); /* Hardware Version */ snprintf(info->hardware_version, -- 2.25.0.368.g28a2d05eebfb ^ permalink raw reply related [flat|nested] 28+ messages in thread
end of thread, other threads:[~2020-03-06 1:30 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-27 22:36 [PATCH 0/5] pci: implement function to read Device Serial Number Jacob Keller 2020-02-27 22:36 ` [PATCH] ice-shared: add macro specifying max NVM offset Jacob Keller 2020-02-27 22:43 ` Jacob Keller 2020-02-27 22:36 ` [PATCH 1/5] pci: introduce pci_get_dsn Jacob Keller 2020-03-01 5:27 ` David Miller 2020-03-02 19:58 ` Jacob Keller 2020-03-02 22:25 ` Bjorn Helgaas 2020-03-02 22:33 ` Jacob Keller 2020-03-02 23:20 ` Bjorn Helgaas 2020-03-02 23:24 ` Jacob Keller 2020-03-02 23:39 ` Bjorn Helgaas 2020-03-03 2:24 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number Jacob Keller 2020-03-03 2:25 ` [PATCH v2 1/6] PCI: Introduce pci_get_dsn Jacob Keller 2020-03-04 22:42 ` Bjorn Helgaas 2020-03-03 2:25 ` [PATCH v2 2/6] bnxt_en: Use pci_get_dsn() Jacob Keller 2020-03-03 2:25 ` [PATCH v2 3/6] scsi: qedf: " Jacob Keller 2020-03-03 2:25 ` [PATCH v2 4/6] ice: " Jacob Keller 2020-03-03 2:25 ` [PATCH v2 5/6] ixgbe: " Jacob Keller 2020-03-03 2:25 ` [PATCH v2 6/6] nfp: " Jacob Keller 2020-03-03 3:40 ` Jakub Kicinski 2020-03-03 17:36 ` Jacob Keller 2020-03-04 22:28 ` [PATCH v2 0/6] PCI: Implement function to read Device Serial Number David Miller 2020-03-06 1:30 ` David Miller 2020-02-27 22:36 ` [PATCH 2/5] bnxt_en: use pci_get_dsn Jacob Keller 2020-03-02 22:25 ` Bjorn Helgaas 2020-02-27 22:36 ` [PATCH 3/5] scsi: qedf: " Jacob Keller 2020-02-27 22:36 ` [PATCH 4/5] ice: " Jacob Keller 2020-02-27 22:36 ` [PATCH 5/5] ixgbe: " Jacob Keller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.