linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] PCI, CPER: Trivial cleanups
@ 2019-03-25 18:14 helgaas
  2019-03-25 18:14 ` [PATCH 1/4] PCI: Cleanup register definition width and whitespace helgaas
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: helgaas @ 2019-03-25 18:14 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Clean up whitespace, fix some typos, add some spec references, and convert
unnecessary __u32 usage to u32.

Bjorn Helgaas (4):
  PCI: Cleanup register definition width and whitespace
  PCI: Fix comment typos
  CPER: Add UEFI spec references
  CPER: Remove unnecessary use of user-space types

 drivers/pci/controller/dwc/pci-keystone.c |   2 +-
 drivers/pci/controller/pci-host-generic.c |   2 +-
 drivers/pci/controller/pcie-iproc-msi.c   |   2 +-
 drivers/pci/pci.c                         | 328 +++++++++++----------
 include/linux/cper.h                      | 336 +++++++++++-----------
 include/uapi/linux/pci_regs.h             | 132 +++++----
 6 files changed, 405 insertions(+), 397 deletions(-)

-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] PCI: Cleanup register definition width and whitespace
  2019-03-25 18:14 [PATCH 0/4] PCI, CPER: Trivial cleanups helgaas
@ 2019-03-25 18:14 ` helgaas
  2019-03-25 18:14 ` [PATCH 2/4] PCI: Fix comment typos helgaas
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: helgaas @ 2019-03-25 18:14 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Follow the file conventions of:

  - register offsets not indented
  - fields within a register indented one space
  - field masks use same width as register
  - register field values indented an additional space

No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/uapi/linux/pci_regs.h | 132 +++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 67 deletions(-)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 5c98133f2c94..f7d3e7831fa8 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1,7 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
 /*
- *	pci_regs.h
- *
  *	PCI standard defines
  *	Copyright 1994, Drew Eckhardt
  *	Copyright 1997--1999 Martin Mares <mj@ucw.cz>
@@ -15,7 +13,7 @@
  *	PCI System Design Guide
  *
  *	For HyperTransport information, please consult the following manuals
- *	from http://www.hypertransport.org
+ *	from http://www.hypertransport.org :
  *
  *	The HyperTransport I/O Link Specification
  */
@@ -301,7 +299,7 @@
 #define  PCI_SID_ESR_FIC	0x20	/* First In Chassis Flag */
 #define PCI_SID_CHASSIS_NR	3	/* Chassis Number */
 
-/* Message Signalled Interrupts registers */
+/* Message Signalled Interrupt registers */
 
 #define PCI_MSI_FLAGS		2	/* Message Control */
 #define  PCI_MSI_FLAGS_ENABLE	0x0001	/* MSI feature enabled */
@@ -319,7 +317,7 @@
 #define PCI_MSI_MASK_64		16	/* Mask bits register for 64-bit devices */
 #define PCI_MSI_PENDING_64	20	/* Pending intrs for 64-bit devices */
 
-/* MSI-X registers */
+/* MSI-X registers (in MSI-X capability) */
 #define PCI_MSIX_FLAGS		2	/* Message Control */
 #define  PCI_MSIX_FLAGS_QSIZE	0x07FF	/* Table size */
 #define  PCI_MSIX_FLAGS_MASKALL	0x4000	/* Mask all vectors for this function */
@@ -333,13 +331,13 @@
 #define PCI_MSIX_FLAGS_BIRMASK	PCI_MSIX_PBA_BIR /* deprecated */
 #define PCI_CAP_MSIX_SIZEOF	12	/* size of MSIX registers */
 
-/* MSI-X Table entry format */
+/* MSI-X Table entry format (in memory mapped by a BAR) */
 #define PCI_MSIX_ENTRY_SIZE		16
-#define  PCI_MSIX_ENTRY_LOWER_ADDR	0
-#define  PCI_MSIX_ENTRY_UPPER_ADDR	4
-#define  PCI_MSIX_ENTRY_DATA		8
-#define  PCI_MSIX_ENTRY_VECTOR_CTRL	12
-#define   PCI_MSIX_ENTRY_CTRL_MASKBIT	1
+#define PCI_MSIX_ENTRY_LOWER_ADDR	0  /* Message Address */
+#define PCI_MSIX_ENTRY_UPPER_ADDR	4  /* Message Upper Address */
+#define PCI_MSIX_ENTRY_DATA		8  /* Message Data */
+#define PCI_MSIX_ENTRY_VECTOR_CTRL	12 /* Vector Control */
+#define  PCI_MSIX_ENTRY_CTRL_MASKBIT	0x00000001
 
 /* CompactPCI Hotswap Register */
 
@@ -465,19 +463,19 @@
 /* PCI Express capability registers */
 
 #define PCI_EXP_FLAGS		2	/* Capabilities register */
-#define PCI_EXP_FLAGS_VERS	0x000f	/* Capability version */
-#define PCI_EXP_FLAGS_TYPE	0x00f0	/* Device/Port type */
-#define  PCI_EXP_TYPE_ENDPOINT	0x0	/* Express Endpoint */
-#define  PCI_EXP_TYPE_LEG_END	0x1	/* Legacy Endpoint */
-#define  PCI_EXP_TYPE_ROOT_PORT 0x4	/* Root Port */
-#define  PCI_EXP_TYPE_UPSTREAM	0x5	/* Upstream Port */
-#define  PCI_EXP_TYPE_DOWNSTREAM 0x6	/* Downstream Port */
-#define  PCI_EXP_TYPE_PCI_BRIDGE 0x7	/* PCIe to PCI/PCI-X Bridge */
-#define  PCI_EXP_TYPE_PCIE_BRIDGE 0x8	/* PCI/PCI-X to PCIe Bridge */
-#define  PCI_EXP_TYPE_RC_END	0x9	/* Root Complex Integrated Endpoint */
-#define  PCI_EXP_TYPE_RC_EC	0xa	/* Root Complex Event Collector */
-#define PCI_EXP_FLAGS_SLOT	0x0100	/* Slot implemented */
-#define PCI_EXP_FLAGS_IRQ	0x3e00	/* Interrupt message number */
+#define  PCI_EXP_FLAGS_VERS	0x000f	/* Capability version */
+#define  PCI_EXP_FLAGS_TYPE	0x00f0	/* Device/Port type */
+#define   PCI_EXP_TYPE_ENDPOINT	   0x0	/* Express Endpoint */
+#define   PCI_EXP_TYPE_LEG_END	   0x1	/* Legacy Endpoint */
+#define   PCI_EXP_TYPE_ROOT_PORT   0x4	/* Root Port */
+#define   PCI_EXP_TYPE_UPSTREAM	   0x5	/* Upstream Port */
+#define   PCI_EXP_TYPE_DOWNSTREAM  0x6	/* Downstream Port */
+#define   PCI_EXP_TYPE_PCI_BRIDGE  0x7	/* PCIe to PCI/PCI-X Bridge */
+#define   PCI_EXP_TYPE_PCIE_BRIDGE 0x8	/* PCI/PCI-X to PCIe Bridge */
+#define   PCI_EXP_TYPE_RC_END	   0x9	/* Root Complex Integrated Endpoint */
+#define   PCI_EXP_TYPE_RC_EC	   0xa	/* Root Complex Event Collector */
+#define  PCI_EXP_FLAGS_SLOT	0x0100	/* Slot implemented */
+#define  PCI_EXP_FLAGS_IRQ	0x3e00	/* Interrupt message number */
 #define PCI_EXP_DEVCAP		4	/* Device capabilities */
 #define  PCI_EXP_DEVCAP_PAYLOAD	0x00000007 /* Max_Payload_Size */
 #define  PCI_EXP_DEVCAP_PHANTOM	0x00000018 /* Phantom functions */
@@ -616,8 +614,8 @@
 #define PCI_EXP_RTCAP		30	/* Root Capabilities */
 #define  PCI_EXP_RTCAP_CRSVIS	0x0001	/* CRS Software Visibility capability */
 #define PCI_EXP_RTSTA		32	/* Root Status */
-#define PCI_EXP_RTSTA_PME	0x00010000 /* PME status */
-#define PCI_EXP_RTSTA_PENDING	0x00020000 /* PME pending */
+#define  PCI_EXP_RTSTA_PME	0x00010000 /* PME status */
+#define  PCI_EXP_RTSTA_PENDING	0x00020000 /* PME pending */
 /*
  * The Device Capabilities 2, Device Status 2, Device Control 2,
  * Link Capabilities 2, Link Status 2, Link Control 2,
@@ -637,13 +635,13 @@
 #define  PCI_EXP_DEVCAP2_OBFF_MASK	0x000c0000 /* OBFF support mechanism */
 #define  PCI_EXP_DEVCAP2_OBFF_MSG	0x00040000 /* New message signaling */
 #define  PCI_EXP_DEVCAP2_OBFF_WAKE	0x00080000 /* Re-use WAKE# for OBFF */
-#define PCI_EXP_DEVCAP2_EE_PREFIX	0x00200000 /* End-End TLP Prefix */
+#define  PCI_EXP_DEVCAP2_EE_PREFIX	0x00200000 /* End-End TLP Prefix */
 #define PCI_EXP_DEVCTL2		40	/* Device Control 2 */
 #define  PCI_EXP_DEVCTL2_COMP_TIMEOUT	0x000f	/* Completion Timeout Value */
 #define  PCI_EXP_DEVCTL2_COMP_TMOUT_DIS	0x0010	/* Completion Timeout Disable */
 #define  PCI_EXP_DEVCTL2_ARI		0x0020	/* Alternative Routing-ID */
-#define PCI_EXP_DEVCTL2_ATOMIC_REQ	0x0040	/* Set Atomic requests */
-#define PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK 0x0080 /* Block atomic egress */
+#define  PCI_EXP_DEVCTL2_ATOMIC_REQ	0x0040	/* Set Atomic requests */
+#define  PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK 0x0080 /* Block atomic egress */
 #define  PCI_EXP_DEVCTL2_IDO_REQ_EN	0x0100	/* Allow IDO for requests */
 #define  PCI_EXP_DEVCTL2_IDO_CMP_EN	0x0200	/* Allow IDO for completions */
 #define  PCI_EXP_DEVCTL2_LTR_EN		0x0400	/* Enable LTR mechanism */
@@ -659,11 +657,11 @@
 #define  PCI_EXP_LNKCAP2_SLS_16_0GB	0x00000010 /* Supported Speed 16GT/s */
 #define  PCI_EXP_LNKCAP2_CROSSLINK	0x00000100 /* Crosslink supported */
 #define PCI_EXP_LNKCTL2		48	/* Link Control 2 */
-#define PCI_EXP_LNKCTL2_TLS		0x000f
-#define PCI_EXP_LNKCTL2_TLS_2_5GT	0x0001 /* Supported Speed 2.5GT/s */
-#define PCI_EXP_LNKCTL2_TLS_5_0GT	0x0002 /* Supported Speed 5GT/s */
-#define PCI_EXP_LNKCTL2_TLS_8_0GT	0x0003 /* Supported Speed 8GT/s */
-#define PCI_EXP_LNKCTL2_TLS_16_0GT	0x0004 /* Supported Speed 16GT/s */
+#define  PCI_EXP_LNKCTL2_TLS		0x000f
+#define  PCI_EXP_LNKCTL2_TLS_2_5GT	0x0001 /* Supported Speed 2.5GT/s */
+#define  PCI_EXP_LNKCTL2_TLS_5_0GT	0x0002 /* Supported Speed 5GT/s */
+#define  PCI_EXP_LNKCTL2_TLS_8_0GT	0x0003 /* Supported Speed 8GT/s */
+#define  PCI_EXP_LNKCTL2_TLS_16_0GT	0x0004 /* Supported Speed 16GT/s */
 #define PCI_EXP_LNKSTA2		50	/* Link Status 2 */
 #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	52	/* v2 endpoints with link end here */
 #define PCI_EXP_SLTCAP2		52	/* Slot Capabilities 2 */
@@ -752,18 +750,18 @@
 #define  PCI_ERR_CAP_ECRC_CHKE	0x00000100	/* ECRC Check Enable */
 #define PCI_ERR_HEADER_LOG	28	/* Header Log Register (16 bytes) */
 #define PCI_ERR_ROOT_COMMAND	44	/* Root Error Command */
-#define PCI_ERR_ROOT_CMD_COR_EN		0x00000001 /* Correctable Err Reporting Enable */
-#define PCI_ERR_ROOT_CMD_NONFATAL_EN	0x00000002 /* Non-Fatal Err Reporting Enable */
-#define PCI_ERR_ROOT_CMD_FATAL_EN	0x00000004 /* Fatal Err Reporting Enable */
+#define  PCI_ERR_ROOT_CMD_COR_EN	0x00000001 /* Correctable Err Reporting Enable */
+#define  PCI_ERR_ROOT_CMD_NONFATAL_EN	0x00000002 /* Non-Fatal Err Reporting Enable */
+#define  PCI_ERR_ROOT_CMD_FATAL_EN	0x00000004 /* Fatal Err Reporting Enable */
 #define PCI_ERR_ROOT_STATUS	48
-#define PCI_ERR_ROOT_COR_RCV		0x00000001 /* ERR_COR Received */
-#define PCI_ERR_ROOT_MULTI_COR_RCV	0x00000002 /* Multiple ERR_COR */
-#define PCI_ERR_ROOT_UNCOR_RCV		0x00000004 /* ERR_FATAL/NONFATAL */
-#define PCI_ERR_ROOT_MULTI_UNCOR_RCV	0x00000008 /* Multiple FATAL/NONFATAL */
-#define PCI_ERR_ROOT_FIRST_FATAL	0x00000010 /* First UNC is Fatal */
-#define PCI_ERR_ROOT_NONFATAL_RCV	0x00000020 /* Non-Fatal Received */
-#define PCI_ERR_ROOT_FATAL_RCV		0x00000040 /* Fatal Received */
-#define PCI_ERR_ROOT_AER_IRQ		0xf8000000 /* Advanced Error Interrupt Message Number */
+#define  PCI_ERR_ROOT_COR_RCV		0x00000001 /* ERR_COR Received */
+#define  PCI_ERR_ROOT_MULTI_COR_RCV	0x00000002 /* Multiple ERR_COR */
+#define  PCI_ERR_ROOT_UNCOR_RCV		0x00000004 /* ERR_FATAL/NONFATAL */
+#define  PCI_ERR_ROOT_MULTI_UNCOR_RCV	0x00000008 /* Multiple FATAL/NONFATAL */
+#define  PCI_ERR_ROOT_FIRST_FATAL	0x00000010 /* First UNC is Fatal */
+#define  PCI_ERR_ROOT_NONFATAL_RCV	0x00000020 /* Non-Fatal Received */
+#define  PCI_ERR_ROOT_FATAL_RCV		0x00000040 /* Fatal Received */
+#define  PCI_ERR_ROOT_AER_IRQ		0xf8000000 /* Advanced Error Interrupt Message Number */
 #define PCI_ERR_ROOT_ERR_SRC	52	/* Error Source Identification */
 
 /* Virtual Channel */
@@ -875,12 +873,12 @@
 
 /* Page Request Interface */
 #define PCI_PRI_CTRL		0x04	/* PRI control register */
-#define  PCI_PRI_CTRL_ENABLE	0x01	/* Enable */
-#define  PCI_PRI_CTRL_RESET	0x02	/* Reset */
+#define  PCI_PRI_CTRL_ENABLE	0x0001	/* Enable */
+#define  PCI_PRI_CTRL_RESET	0x0002	/* Reset */
 #define PCI_PRI_STATUS		0x06	/* PRI status register */
-#define  PCI_PRI_STATUS_RF	0x001	/* Response Failure */
-#define  PCI_PRI_STATUS_UPRGI	0x002	/* Unexpected PRG index */
-#define  PCI_PRI_STATUS_STOPPED	0x100	/* PRI Stopped */
+#define  PCI_PRI_STATUS_RF	0x0001	/* Response Failure */
+#define  PCI_PRI_STATUS_UPRGI	0x0002	/* Unexpected PRG index */
+#define  PCI_PRI_STATUS_STOPPED	0x0100	/* PRI Stopped */
 #define  PCI_PRI_STATUS_PASID	0x8000	/* PRG Response PASID Required */
 #define PCI_PRI_MAX_REQ		0x08	/* PRI max reqs supported */
 #define PCI_PRI_ALLOC_REQ	0x0c	/* PRI max reqs allowed */
@@ -898,16 +896,16 @@
 
 /* Single Root I/O Virtualization */
 #define PCI_SRIOV_CAP		0x04	/* SR-IOV Capabilities */
-#define  PCI_SRIOV_CAP_VFM	0x01	/* VF Migration Capable */
+#define  PCI_SRIOV_CAP_VFM	0x00000001  /* VF Migration Capable */
 #define  PCI_SRIOV_CAP_INTR(x)	((x) >> 21) /* Interrupt Message Number */
 #define PCI_SRIOV_CTRL		0x08	/* SR-IOV Control */
-#define  PCI_SRIOV_CTRL_VFE	0x01	/* VF Enable */
-#define  PCI_SRIOV_CTRL_VFM	0x02	/* VF Migration Enable */
-#define  PCI_SRIOV_CTRL_INTR	0x04	/* VF Migration Interrupt Enable */
-#define  PCI_SRIOV_CTRL_MSE	0x08	/* VF Memory Space Enable */
-#define  PCI_SRIOV_CTRL_ARI	0x10	/* ARI Capable Hierarchy */
+#define  PCI_SRIOV_CTRL_VFE	0x0001	/* VF Enable */
+#define  PCI_SRIOV_CTRL_VFM	0x0002	/* VF Migration Enable */
+#define  PCI_SRIOV_CTRL_INTR	0x0004	/* VF Migration Interrupt Enable */
+#define  PCI_SRIOV_CTRL_MSE	0x0008	/* VF Memory Space Enable */
+#define  PCI_SRIOV_CTRL_ARI	0x0010	/* ARI Capable Hierarchy */
 #define PCI_SRIOV_STATUS	0x0a	/* SR-IOV Status */
-#define  PCI_SRIOV_STATUS_VFM	0x01	/* VF Migration Status */
+#define  PCI_SRIOV_STATUS_VFM	0x0001	/* VF Migration Status */
 #define PCI_SRIOV_INITIAL_VF	0x0c	/* Initial VFs */
 #define PCI_SRIOV_TOTAL_VF	0x0e	/* Total VFs */
 #define PCI_SRIOV_NUM_VF	0x10	/* Number of VFs */
@@ -937,13 +935,13 @@
 
 /* Access Control Service */
 #define PCI_ACS_CAP		0x04	/* ACS Capability Register */
-#define  PCI_ACS_SV		0x01	/* Source Validation */
-#define  PCI_ACS_TB		0x02	/* Translation Blocking */
-#define  PCI_ACS_RR		0x04	/* P2P Request Redirect */
-#define  PCI_ACS_CR		0x08	/* P2P Completion Redirect */
-#define  PCI_ACS_UF		0x10	/* Upstream Forwarding */
-#define  PCI_ACS_EC		0x20	/* P2P Egress Control */
-#define  PCI_ACS_DT		0x40	/* Direct Translated P2P */
+#define  PCI_ACS_SV		0x0001	/* Source Validation */
+#define  PCI_ACS_TB		0x0002	/* Translation Blocking */
+#define  PCI_ACS_RR		0x0004	/* P2P Request Redirect */
+#define  PCI_ACS_CR		0x0008	/* P2P Completion Redirect */
+#define  PCI_ACS_UF		0x0010	/* Upstream Forwarding */
+#define  PCI_ACS_EC		0x0020	/* P2P Egress Control */
+#define  PCI_ACS_DT		0x0040	/* Direct Translated P2P */
 #define PCI_ACS_EGRESS_BITS	0x05	/* ACS Egress Control Vector Size */
 #define PCI_ACS_CTRL		0x06	/* ACS Control Register */
 #define PCI_ACS_EGRESS_CTL_V	0x08	/* ACS Egress Control Vector */
@@ -993,9 +991,9 @@
 #define  PCI_EXP_DPC_CAP_DL_ACTIVE	0x1000	/* ERR_COR signal on DL_Active supported */
 
 #define PCI_EXP_DPC_CTL			6	/* DPC control */
-#define  PCI_EXP_DPC_CTL_EN_FATAL 	0x0001	/* Enable trigger on ERR_FATAL message */
-#define  PCI_EXP_DPC_CTL_EN_NONFATAL 	0x0002	/* Enable trigger on ERR_NONFATAL message */
-#define  PCI_EXP_DPC_CTL_INT_EN 	0x0008	/* DPC Interrupt Enable */
+#define  PCI_EXP_DPC_CTL_EN_FATAL	0x0001	/* Enable trigger on ERR_FATAL message */
+#define  PCI_EXP_DPC_CTL_EN_NONFATAL	0x0002	/* Enable trigger on ERR_NONFATAL message */
+#define  PCI_EXP_DPC_CTL_INT_EN		0x0008	/* DPC Interrupt Enable */
 
 #define PCI_EXP_DPC_STATUS		8	/* DPC Status */
 #define  PCI_EXP_DPC_STATUS_TRIGGER	    0x0001 /* Trigger Status */
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] PCI: Fix comment typos
  2019-03-25 18:14 [PATCH 0/4] PCI, CPER: Trivial cleanups helgaas
  2019-03-25 18:14 ` [PATCH 1/4] PCI: Cleanup register definition width and whitespace helgaas
@ 2019-03-25 18:14 ` helgaas
  2019-03-30 17:56   ` Mukesh Ojha
  2019-03-25 18:14 ` [PATCH 3/4] CPER: Add UEFI spec references helgaas
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: helgaas @ 2019-03-25 18:14 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Fix spelling errors and format function comments consistently.  Changes
whitespace and comments only; no functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/controller/dwc/pci-keystone.c |   2 +-
 drivers/pci/controller/pci-host-generic.c |   2 +-
 drivers/pci/controller/pcie-iproc-msi.c   |   2 +-
 drivers/pci/pci.c                         | 328 +++++++++++-----------
 4 files changed, 173 insertions(+), 161 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 14f2b0b4ed5e..9b4112095658 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -661,7 +661,7 @@ static int ks_pcie_get_irq_controller_info(struct keystone_pcie *ks_pcie,
 			(legacy ? "legacy" : "MSI"), temp);
 
 	/*
-	 * support upto max_host_irqs. In dt from index 0 to 3 (legacy) or 0 to
+	 * support up to max_host_irqs. In DT from index 0 to 3 (legacy) or 0 to
 	 * 7 (MSI)
 	 */
 	for (temp = 0; temp < max_host_irqs; temp++) {
diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
index dea3ec7592a2..75a2fb930d4b 100644
--- a/drivers/pci/controller/pci-host-generic.c
+++ b/drivers/pci/controller/pci-host-generic.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Simple, generic PCI host controller driver targetting firmware-initialised
+ * Simple, generic PCI host controller driver targeting firmware-initialised
  * systems and virtual machines (e.g. the PCI emulation provided by kvmtool).
  *
  * Copyright (C) 2014 ARM Limited
diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
index cb3401a931f8..0a3f61be5625 100644
--- a/drivers/pci/controller/pcie-iproc-msi.c
+++ b/drivers/pci/controller/pcie-iproc-msi.c
@@ -367,7 +367,7 @@ static void iproc_msi_handler(struct irq_desc *desc)
 
 		/*
 		 * Now go read the tail pointer again to see if there are new
-		 * oustanding events that came in during the above window.
+		 * outstanding events that came in during the above window.
 		 */
 	} while (true);
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7c1b362f599a..530eec3191e7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -197,8 +197,8 @@ EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
 
 /**
  * pci_dev_str_match_path - test if a path string matches a device
- * @dev:    the PCI device to test
- * @path:   string to match the device against
+ * @dev: the PCI device to test
+ * @path: string to match the device against
  * @endptr: pointer to the string after the match
  *
  * Test if a string (typically from a kernel parameter) formatted as a
@@ -280,8 +280,8 @@ static int pci_dev_str_match_path(struct pci_dev *dev, const char *path,
 
 /**
  * pci_dev_str_match - test if a string matches a device
- * @dev:    the PCI device to test
- * @p:      string to match the device against
+ * @dev: the PCI device to test
+ * @p: string to match the device against
  * @endptr: pointer to the string after the match
  *
  * Test if a string (typically from a kernel parameter) matches a specified
@@ -341,7 +341,7 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 	} else {
 		/*
 		 * PCI Bus, Device, Function IDs are specified
-		 *  (optionally, may include a path of devfns following it)
+		 * (optionally, may include a path of devfns following it)
 		 */
 		ret = pci_dev_str_match_path(dev, p, &p);
 		if (ret < 0)
@@ -425,7 +425,7 @@ static int __pci_bus_find_cap_start(struct pci_bus *bus,
  * Tell if a device supports a given PCI capability.
  * Returns the address of the requested capability structure within the
  * device's PCI configuration space or 0 in case the device does not
- * support it.  Possible values for @cap:
+ * support it.  Possible values for @cap include:
  *
  *  %PCI_CAP_ID_PM           Power Management
  *  %PCI_CAP_ID_AGP          Accelerated Graphics Port
@@ -450,11 +450,11 @@ EXPORT_SYMBOL(pci_find_capability);
 
 /**
  * pci_bus_find_capability - query for devices' capabilities
- * @bus:   the PCI bus to query
+ * @bus: the PCI bus to query
  * @devfn: PCI device to query
- * @cap:   capability code
+ * @cap: capability code
  *
- * Like pci_find_capability() but works for pci devices that do not have a
+ * Like pci_find_capability() but works for PCI devices that do not have a
  * pci_dev structure set up yet.
  *
  * Returns the address of the requested capability structure within the
@@ -535,7 +535,7 @@ EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
  *
  * Returns the address of the requested extended capability structure
  * within the device's PCI configuration space or 0 if the device does
- * not support it.  Possible values for @cap:
+ * not support it.  Possible values for @cap include:
  *
  *  %PCI_EXT_CAP_ID_ERR		Advanced Error Reporting
  *  %PCI_EXT_CAP_ID_VC		Virtual Channel
@@ -618,12 +618,13 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
 EXPORT_SYMBOL_GPL(pci_find_ht_capability);
 
 /**
- * pci_find_parent_resource - return resource region of parent bus of given region
+ * pci_find_parent_resource - return resource region of parent bus of given
+ *			      region
  * @dev: PCI device structure contains resources to be searched
  * @res: child resource record for which parent is sought
  *
- *  For given resource region of given device, return the resource
- *  region of parent bus the given region is contained in.
+ * For given resource region of given device, return the resource region of
+ * parent bus the given region is contained in.
  */
 struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 					  struct resource *res)
@@ -800,7 +801,7 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
 
 /**
  * pci_raw_set_power_state - Use PCI PM registers to set the power state of
- *                           given PCI device
+ *			     given PCI device
  * @dev: PCI device to handle.
  * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
  *
@@ -826,7 +827,8 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
 	if (state < PCI_D0 || state > PCI_D3hot)
 		return -EINVAL;
 
-	/* Validate current state:
+	/*
+	 * Validate current state:
 	 * Can enter D0 from any state, but if we can only go deeper
 	 * to sleep if we're already in a low power state
 	 */
@@ -837,14 +839,15 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
 		return -EINVAL;
 	}
 
-	/* check if this device supports the desired state */
+	/* Check if this device supports the desired state */
 	if ((state == PCI_D1 && !dev->d1_support)
 	   || (state == PCI_D2 && !dev->d2_support))
 		return -EIO;
 
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 
-	/* If we're (effectively) in D3, force entire word to 0.
+	/*
+	 * If we're (effectively) in D3, force entire word to 0.
 	 * This doesn't affect PME_Status, disables PME_En, and
 	 * sets PowerState to 0.
 	 */
@@ -867,11 +870,13 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
 		break;
 	}
 
-	/* enter specified state */
+	/* Enter specified state */
 	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 
-	/* Mandatory power management transition delays */
-	/* see PCI PM 1.1 5.6.1 table 18 */
+	/*
+	 * Mandatory power management transition delays; see PCI PM 1.1
+	 * 5.6.1 table 18
+	 */
 	if (state == PCI_D3hot || dev->current_state == PCI_D3hot)
 		pci_dev_d3_sleep(dev);
 	else if (state == PCI_D2 || dev->current_state == PCI_D2)
@@ -1085,16 +1090,18 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
 	int error;
 
-	/* bound the state we're entering */
+	/* Bound the state we're entering */
 	if (state > PCI_D3cold)
 		state = PCI_D3cold;
 	else if (state < PCI_D0)
 		state = PCI_D0;
 	else if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
+
 		/*
-		 * If the device or the parent bridge do not support PCI PM,
-		 * ignore the request if we're doing anything other than putting
-		 * it into D0 (which would only happen on boot).
+		 * If the device or the parent bridge do not support PCI
+		 * PM, ignore the request if we're doing anything other
+		 * than putting it into D0 (which would only happen on
+		 * boot).
 		 */
 		return 0;
 
@@ -1104,8 +1111,10 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 
 	__pci_start_power_transition(dev, state);
 
-	/* This device is quirked not to be put into D3, so
-	   don't put it in D3 */
+	/*
+	 * This device is quirked not to be put into D3, so don't put it in
+	 * D3
+	 */
 	if (state >= PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3))
 		return 0;
 
@@ -1127,12 +1136,11 @@ EXPORT_SYMBOL(pci_set_power_state);
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
  * @state: target sleep state for the whole system. This is the value
- *	that is passed to suspend() function.
+ *	   that is passed to suspend() function.
  *
  * Returns PCI power state suitable for given device and given system
  * message.
  */
-
 pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state)
 {
 	pci_power_t ret;
@@ -1310,8 +1318,9 @@ static void pci_restore_ltr_state(struct pci_dev *dev)
 }
 
 /**
- * pci_save_state - save the PCI configuration space of a device before suspending
- * @dev: - PCI device that we're dealing with
+ * pci_save_state - save the PCI configuration space of a device before
+ *		    suspending
+ * @dev: PCI device that we're dealing with
  */
 int pci_save_state(struct pci_dev *dev)
 {
@@ -1422,7 +1431,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
 
 /**
  * pci_restore_state - Restore the saved state of a PCI device
- * @dev: - PCI device that we're dealing with
+ * @dev: PCI device that we're dealing with
  */
 void pci_restore_state(struct pci_dev *dev)
 {
@@ -1599,8 +1608,8 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
  * pci_reenable_device - Resume abandoned device
  * @dev: PCI device to be resumed
  *
- *  Note this function is a backend of pci_default_resume and is not supposed
- *  to be called by normal code, write proper resume handler and use it instead.
+ * NOTE: This function is a backend of pci_default_resume() and is not supposed
+ * to be called by normal code, write proper resume handler and use it instead.
  */
 int pci_reenable_device(struct pci_dev *dev)
 {
@@ -1675,9 +1684,9 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
  * pci_enable_device_io - Initialize a device for use with IO space
  * @dev: PCI device to be initialized
  *
- *  Initialize device before it's used by a driver. Ask low-level code
- *  to enable I/O resources. Wake up the device if it was suspended.
- *  Beware, this function can fail.
+ * Initialize device before it's used by a driver. Ask low-level code
+ * to enable I/O resources. Wake up the device if it was suspended.
+ * Beware, this function can fail.
  */
 int pci_enable_device_io(struct pci_dev *dev)
 {
@@ -1689,9 +1698,9 @@ EXPORT_SYMBOL(pci_enable_device_io);
  * pci_enable_device_mem - Initialize a device for use with Memory space
  * @dev: PCI device to be initialized
  *
- *  Initialize device before it's used by a driver. Ask low-level code
- *  to enable Memory resources. Wake up the device if it was suspended.
- *  Beware, this function can fail.
+ * Initialize device before it's used by a driver. Ask low-level code
+ * to enable Memory resources. Wake up the device if it was suspended.
+ * Beware, this function can fail.
  */
 int pci_enable_device_mem(struct pci_dev *dev)
 {
@@ -1703,12 +1712,12 @@ EXPORT_SYMBOL(pci_enable_device_mem);
  * pci_enable_device - Initialize device before it's used by a driver.
  * @dev: PCI device to be initialized
  *
- *  Initialize device before it's used by a driver. Ask low-level code
- *  to enable I/O and memory. Wake up the device if it was suspended.
- *  Beware, this function can fail.
+ * Initialize device before it's used by a driver. Ask low-level code
+ * to enable I/O and memory. Wake up the device if it was suspended.
+ * Beware, this function can fail.
  *
- *  Note we don't actually enable the device many times if we call
- *  this function repeatedly (we just increment the count).
+ * Note we don't actually enable the device many times if we call
+ * this function repeatedly (we just increment the count).
  */
 int pci_enable_device(struct pci_dev *dev)
 {
@@ -1717,8 +1726,8 @@ int pci_enable_device(struct pci_dev *dev)
 EXPORT_SYMBOL(pci_enable_device);
 
 /*
- * Managed PCI resources.  This manages device on/off, intx/msi/msix
- * on/off and BAR regions.  pci_dev itself records msi/msix status, so
+ * Managed PCI resources.  This manages device on/off, INTx/MSI/MSI-X
+ * on/off and BAR regions.  pci_dev itself records MSI/MSI-X status, so
  * there's no need to track it separately.  pci_devres is initialized
  * when a device is enabled using managed PCI device enable interface.
  */
@@ -1836,7 +1845,8 @@ int __weak pcibios_add_device(struct pci_dev *dev)
 }
 
 /**
- * pcibios_release_device - provide arch specific hooks when releasing device dev
+ * pcibios_release_device - provide arch specific hooks when releasing
+ *			    device dev
  * @dev: the PCI device being released
  *
  * Permits the platform to provide architecture specific functionality when
@@ -1927,8 +1937,7 @@ EXPORT_SYMBOL(pci_disable_device);
  * @dev: the PCIe device reset
  * @state: Reset state to enter into
  *
- *
- * Sets the PCIe reset state for the device. This is the default
+ * Set the PCIe reset state for the device. This is the default
  * implementation. Architecture implementations can override this.
  */
 int __weak pcibios_set_pcie_reset_state(struct pci_dev *dev,
@@ -1942,7 +1951,6 @@ int __weak pcibios_set_pcie_reset_state(struct pci_dev *dev,
  * @dev: the PCIe device reset
  * @state: Reset state to enter into
  *
- *
  * Sets the PCI reset state for the device.
  */
 int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
@@ -2339,7 +2347,8 @@ static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
 }
 
 /**
- * pci_prepare_to_sleep - prepare PCI device for system-wide transition into a sleep state
+ * pci_prepare_to_sleep - prepare PCI device for system-wide transition
+ *			  into a sleep state
  * @dev: Device to handle.
  *
  * Choose the power state appropriate for the device depending on whether
@@ -2367,7 +2376,8 @@ int pci_prepare_to_sleep(struct pci_dev *dev)
 EXPORT_SYMBOL(pci_prepare_to_sleep);
 
 /**
- * pci_back_from_sleep - turn PCI device on during system-wide transition into working state
+ * pci_back_from_sleep - turn PCI device on during system-wide transition
+ *			 into working state
  * @dev: Device to handle.
  *
  * Disable device's system wake-up capability and put it into D0.
@@ -3005,7 +3015,7 @@ static void pci_add_saved_cap(struct pci_dev *pci_dev,
 
 /**
  * _pci_add_cap_save_buffer - allocate buffer for saving given
- *                            capability registers
+ *			      capability registers
  * @dev: the PCI device
  * @cap: the capability to allocate the buffer for
  * @extended: Standard or Extended capability ID
@@ -3186,7 +3196,7 @@ static void pci_disable_acs_redir(struct pci_dev *dev)
 }
 
 /**
- * pci_std_enable_acs - enable ACS on devices using standard ACS capabilites
+ * pci_std_enable_acs - enable ACS on devices using standard ACS capabilities
  * @dev: the PCI device
  */
 static void pci_std_enable_acs(struct pci_dev *dev)
@@ -3609,13 +3619,14 @@ u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp)
 EXPORT_SYMBOL_GPL(pci_common_swizzle);
 
 /**
- *	pci_release_region - Release a PCI bar
- *	@pdev: PCI device whose resources were previously reserved by pci_request_region
- *	@bar: BAR to release
+ * pci_release_region - Release a PCI bar
+ * @pdev: PCI device whose resources were previously reserved by
+ *	  pci_request_region()
+ * @bar: BAR to release
  *
- *	Releases the PCI I/O and memory resources previously reserved by a
- *	successful call to pci_request_region.  Call this function only
- *	after all use of the PCI regions has ceased.
+ * Releases the PCI I/O and memory resources previously reserved by a
+ * successful call to pci_request_region().  Call this function only
+ * after all use of the PCI regions has ceased.
  */
 void pci_release_region(struct pci_dev *pdev, int bar)
 {
@@ -3637,23 +3648,23 @@ void pci_release_region(struct pci_dev *pdev, int bar)
 EXPORT_SYMBOL(pci_release_region);
 
 /**
- *	__pci_request_region - Reserved PCI I/O and memory resource
- *	@pdev: PCI device whose resources are to be reserved
- *	@bar: BAR to be reserved
- *	@res_name: Name to be associated with resource.
- *	@exclusive: whether the region access is exclusive or not
+ * __pci_request_region - Reserved PCI I/O and memory resource
+ * @pdev: PCI device whose resources are to be reserved
+ * @bar: BAR to be reserved
+ * @res_name: Name to be associated with resource.
+ * @exclusive: whether the region access is exclusive or not
  *
- *	Mark the PCI region associated with PCI device @pdev BR @bar as
- *	being reserved by owner @res_name.  Do not access any
- *	address inside the PCI regions unless this call returns
- *	successfully.
+ * Mark the PCI region associated with PCI device @pdev BAR @bar as
+ * being reserved by owner @res_name.  Do not access any
+ * address inside the PCI regions unless this call returns
+ * successfully.
  *
- *	If @exclusive is set, then the region is marked so that userspace
- *	is explicitly not allowed to map the resource via /dev/mem or
- *	sysfs MMIO access.
+ * If @exclusive is set, then the region is marked so that userspace
+ * is explicitly not allowed to map the resource via /dev/mem or
+ * sysfs MMIO access.
  *
- *	Returns 0 on success, or %EBUSY on error.  A warning
- *	message is also printed on failure.
+ * Returns 0 on success, or %EBUSY on error.  A warning
+ * message is also printed on failure.
  */
 static int __pci_request_region(struct pci_dev *pdev, int bar,
 				const char *res_name, int exclusive)
@@ -3687,18 +3698,18 @@ static int __pci_request_region(struct pci_dev *pdev, int bar,
 }
 
 /**
- *	pci_request_region - Reserve PCI I/O and memory resource
- *	@pdev: PCI device whose resources are to be reserved
- *	@bar: BAR to be reserved
- *	@res_name: Name to be associated with resource
+ * pci_request_region - Reserve PCI I/O and memory resource
+ * @pdev: PCI device whose resources are to be reserved
+ * @bar: BAR to be reserved
+ * @res_name: Name to be associated with resource
  *
- *	Mark the PCI region associated with PCI device @pdev BAR @bar as
- *	being reserved by owner @res_name.  Do not access any
- *	address inside the PCI regions unless this call returns
- *	successfully.
+ * Mark the PCI region associated with PCI device @pdev BAR @bar as
+ * being reserved by owner @res_name.  Do not access any
+ * address inside the PCI regions unless this call returns
+ * successfully.
  *
- *	Returns 0 on success, or %EBUSY on error.  A warning
- *	message is also printed on failure.
+ * Returns 0 on success, or %EBUSY on error.  A warning
+ * message is also printed on failure.
  */
 int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
 {
@@ -3707,22 +3718,22 @@ int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
 EXPORT_SYMBOL(pci_request_region);
 
 /**
- *	pci_request_region_exclusive - Reserved PCI I/O and memory resource
- *	@pdev: PCI device whose resources are to be reserved
- *	@bar: BAR to be reserved
- *	@res_name: Name to be associated with resource.
+ * pci_request_region_exclusive - Reserved PCI I/O and memory resource
+ * @pdev: PCI device whose resources are to be reserved
+ * @bar: BAR to be reserved
+ * @res_name: Name to be associated with resource.
  *
- *	Mark the PCI region associated with PCI device @pdev BR @bar as
- *	being reserved by owner @res_name.  Do not access any
- *	address inside the PCI regions unless this call returns
- *	successfully.
+ * Mark the PCI region associated with PCI device @pdev BAR @bar as
+ * being reserved by owner @res_name.  Do not access any
+ * address inside the PCI regions unless this call returns
+ * successfully.
  *
- *	Returns 0 on success, or %EBUSY on error.  A warning
- *	message is also printed on failure.
+ * Returns 0 on success, or %EBUSY on error.  A warning
+ * message is also printed on failure.
  *
- *	The key difference that _exclusive makes it that userspace is
- *	explicitly not allowed to map the resource via /dev/mem or
- *	sysfs.
+ * The key difference that _exclusive makes it that userspace is
+ * explicitly not allowed to map the resource via /dev/mem or
+ * sysfs.
  */
 int pci_request_region_exclusive(struct pci_dev *pdev, int bar,
 				 const char *res_name)
@@ -3791,12 +3802,13 @@ int pci_request_selected_regions_exclusive(struct pci_dev *pdev, int bars,
 EXPORT_SYMBOL(pci_request_selected_regions_exclusive);
 
 /**
- *	pci_release_regions - Release reserved PCI I/O and memory resources
- *	@pdev: PCI device whose resources were previously reserved by pci_request_regions
+ * pci_release_regions - Release reserved PCI I/O and memory resources
+ * @pdev: PCI device whose resources were previously reserved by
+ *	  pci_request_regions()
  *
- *	Releases all PCI I/O and memory resources previously reserved by a
- *	successful call to pci_request_regions.  Call this function only
- *	after all use of the PCI regions has ceased.
+ * Releases all PCI I/O and memory resources previously reserved by a
+ * successful call to pci_request_regions().  Call this function only
+ * after all use of the PCI regions has ceased.
  */
 
 void pci_release_regions(struct pci_dev *pdev)
@@ -3806,17 +3818,17 @@ void pci_release_regions(struct pci_dev *pdev)
 EXPORT_SYMBOL(pci_release_regions);
 
 /**
- *	pci_request_regions - Reserved PCI I/O and memory resources
- *	@pdev: PCI device whose resources are to be reserved
- *	@res_name: Name to be associated with resource.
+ * pci_request_regions - Reserve PCI I/O and memory resources
+ * @pdev: PCI device whose resources are to be reserved
+ * @res_name: Name to be associated with resource.
  *
- *	Mark all PCI regions associated with PCI device @pdev as
- *	being reserved by owner @res_name.  Do not access any
- *	address inside the PCI regions unless this call returns
- *	successfully.
+ * Mark all PCI regions associated with PCI device @pdev as
+ * being reserved by owner @res_name.  Do not access any
+ * address inside the PCI regions unless this call returns
+ * successfully.
  *
- *	Returns 0 on success, or %EBUSY on error.  A warning
- *	message is also printed on failure.
+ * Returns 0 on success, or %EBUSY on error.  A warning
+ * message is also printed on failure.
  */
 int pci_request_regions(struct pci_dev *pdev, const char *res_name)
 {
@@ -3825,20 +3837,19 @@ int pci_request_regions(struct pci_dev *pdev, const char *res_name)
 EXPORT_SYMBOL(pci_request_regions);
 
 /**
- *	pci_request_regions_exclusive - Reserved PCI I/O and memory resources
- *	@pdev: PCI device whose resources are to be reserved
- *	@res_name: Name to be associated with resource.
+ * pci_request_regions_exclusive - Reserve PCI I/O and memory resources
+ * @pdev: PCI device whose resources are to be reserved
+ * @res_name: Name to be associated with resource.
  *
- *	Mark all PCI regions associated with PCI device @pdev as
- *	being reserved by owner @res_name.  Do not access any
- *	address inside the PCI regions unless this call returns
- *	successfully.
+ * Mark all PCI regions associated with PCI device @pdev as being reserved
+ * by owner @res_name.  Do not access any address inside the PCI regions
+ * unless this call returns successfully.
  *
- *	pci_request_regions_exclusive() will mark the region so that
- *	/dev/mem and the sysfs MMIO access will not be allowed.
+ * pci_request_regions_exclusive() will mark the region so that /dev/mem
+ * and the sysfs MMIO access will not be allowed.
  *
- *	Returns 0 on success, or %EBUSY on error.  A warning
- *	message is also printed on failure.
+ * Returns 0 on success, or %EBUSY on error.  A warning message is also
+ * printed on failure.
  */
 int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
 {
@@ -3849,7 +3860,7 @@ EXPORT_SYMBOL(pci_request_regions_exclusive);
 
 /*
  * Record the PCI IO range (expressed as CPU physical address + size).
- * Return a negative value if an error has occured, zero otherwise
+ * Return a negative value if an error has occurred, zero otherwise
  */
 int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
 			resource_size_t	size)
@@ -3905,14 +3916,14 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
 }
 
 /**
- *	pci_remap_iospace - Remap the memory mapped I/O space
- *	@res: Resource describing the I/O space
- *	@phys_addr: physical address of range to be mapped
+ * pci_remap_iospace - Remap the memory mapped I/O space
+ * @res: Resource describing the I/O space
+ * @phys_addr: physical address of range to be mapped
  *
- *	Remap the memory mapped I/O space described by the @res
- *	and the CPU physical address @phys_addr into virtual address space.
- *	Only architectures that have memory mapped IO functions defined
- *	(and the PCI_IOBASE value defined) should call this function.
+ * Remap the memory mapped I/O space described by the @res and the CPU
+ * physical address @phys_addr into virtual address space.  Only
+ * architectures that have memory mapped IO functions defined (and the
+ * PCI_IOBASE value defined) should call this function.
  */
 int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
 {
@@ -3928,8 +3939,10 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
 	return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
 				  pgprot_device(PAGE_KERNEL));
 #else
-	/* this architecture does not have memory mapped I/O space,
-	   so this function should never be called */
+	/*
+	 * This architecture does not have memory mapped I/O space,
+	 * so this function should never be called
+	 */
 	WARN_ONCE(1, "This architecture does not support memory mapped I/O\n");
 	return -ENODEV;
 #endif
@@ -3937,12 +3950,12 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
 EXPORT_SYMBOL(pci_remap_iospace);
 
 /**
- *	pci_unmap_iospace - Unmap the memory mapped I/O space
- *	@res: resource to be unmapped
+ * pci_unmap_iospace - Unmap the memory mapped I/O space
+ * @res: resource to be unmapped
  *
- *	Unmap the CPU virtual address @res from virtual address space.
- *	Only architectures that have memory mapped IO functions defined
- *	(and the PCI_IOBASE value defined) should call this function.
+ * Unmap the CPU virtual address @res from virtual address space.  Only
+ * architectures that have memory mapped IO functions defined (and the
+ * PCI_IOBASE value defined) should call this function.
  */
 void pci_unmap_iospace(struct resource *res)
 {
@@ -4288,7 +4301,7 @@ EXPORT_SYMBOL(pci_clear_mwi);
  * @pdev: the PCI device to operate on
  * @enable: boolean: whether to enable or disable PCI INTx
  *
- * Enables/disables PCI INTx for device dev
+ * Enables/disables PCI INTx for device @pdev
  */
 void pci_intx(struct pci_dev *pdev, int enable)
 {
@@ -4364,9 +4377,8 @@ static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask)
  * pci_check_and_mask_intx - mask INTx on pending interrupt
  * @dev: the PCI device to operate on
  *
- * Check if the device dev has its INTx line asserted, mask it and
- * return true in that case. False is returned if no interrupt was
- * pending.
+ * Check if the device dev has its INTx line asserted, mask it and return
+ * true in that case. False is returned if no interrupt was pending.
  */
 bool pci_check_and_mask_intx(struct pci_dev *dev)
 {
@@ -4378,9 +4390,9 @@ EXPORT_SYMBOL_GPL(pci_check_and_mask_intx);
  * pci_check_and_unmask_intx - unmask INTx if no interrupt is pending
  * @dev: the PCI device to operate on
  *
- * Check if the device dev has its INTx line asserted, unmask it if not
- * and return true. False is returned and the mask remains active if
- * there was still an interrupt pending.
+ * Check if the device dev has its INTx line asserted, unmask it if not and
+ * return true. False is returned and the mask remains active if there was
+ * still an interrupt pending.
  */
 bool pci_check_and_unmask_intx(struct pci_dev *dev)
 {
@@ -4389,7 +4401,7 @@ bool pci_check_and_unmask_intx(struct pci_dev *dev)
 EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
 
 /**
- * pci_wait_for_pending_transaction - waits for pending transaction
+ * pci_wait_for_pending_transaction - wait for pending transaction
  * @dev: the PCI device to operate on
  *
  * Return 0 if transaction is pending 1 otherwise.
@@ -4447,7 +4459,7 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
 
 /**
  * pcie_has_flr - check if a device supports function level resets
- * @dev:	device to check
+ * @dev: device to check
  *
  * Returns true if the device advertises support for PCIe function level
  * resets.
@@ -4466,7 +4478,7 @@ EXPORT_SYMBOL_GPL(pcie_has_flr);
 
 /**
  * pcie_flr - initiate a PCIe function level reset
- * @dev:	device to reset
+ * @dev: device to reset
  *
  * Initiate a function level reset on @dev.  The caller should ensure the
  * device supports FLR before calling this function, e.g. by using the
@@ -4810,6 +4822,7 @@ static void pci_dev_restore(struct pci_dev *dev)
  *
  * The device function is presumed to be unused and the caller is holding
  * the device mutex lock when this function is called.
+ *
  * Resetting the device will make the contents of PCI configuration space
  * random, so any caller of this must be prepared to reinitialise the
  * device including MSI, bus mastering, BARs, decoding IO and memory spaces,
@@ -5373,8 +5386,8 @@ EXPORT_SYMBOL_GPL(pci_reset_bus);
  * pcix_get_max_mmrbc - get PCI-X maximum designed memory read byte count
  * @dev: PCI device to query
  *
- * Returns mmrbc: maximum designed memory read count in bytes
- *    or appropriate error value.
+ * Returns mmrbc: maximum designed memory read count in bytes or
+ * appropriate error value.
  */
 int pcix_get_max_mmrbc(struct pci_dev *dev)
 {
@@ -5396,8 +5409,8 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
  * pcix_get_mmrbc - get PCI-X maximum memory read byte count
  * @dev: PCI device to query
  *
- * Returns mmrbc: maximum memory read count in bytes
- *    or appropriate error value.
+ * Returns mmrbc: maximum memory read count in bytes or appropriate error
+ * value.
  */
 int pcix_get_mmrbc(struct pci_dev *dev)
 {
@@ -5421,7 +5434,7 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
  * @mmrbc: maximum memory read count in bytes
  *    valid values are 512, 1024, 2048, 4096
  *
- * If possible sets maximum memory read byte count, some bridges have erratas
+ * If possible sets maximum memory read byte count, some bridges have errata
  * that prevent this.
  */
 int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
@@ -5466,8 +5479,7 @@ EXPORT_SYMBOL(pcix_set_mmrbc);
  * pcie_get_readrq - get PCI Express read request size
  * @dev: PCI device to query
  *
- * Returns maximum memory read request in bytes
- *    or appropriate error value.
+ * Returns maximum memory read request in bytes or appropriate error value.
  */
 int pcie_get_readrq(struct pci_dev *dev)
 {
@@ -5495,10 +5507,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
 		return -EINVAL;
 
 	/*
-	 * If using the "performance" PCIe config, we clamp the
-	 * read rq size to the max packet size to prevent the
-	 * host bridge generating requests larger than we can
-	 * cope with
+	 * If using the "performance" PCIe config, we clamp the read rq
+	 * size to the max packet size to keep the host bridge from
+	 * generating requests larger than we can cope with.
 	 */
 	if (pcie_bus_config == PCIE_BUS_PERFORMANCE) {
 		int mps = pcie_get_mps(dev);
@@ -6144,6 +6155,7 @@ static int of_pci_bus_find_domain_nr(struct device *parent)
 
 	if (parent)
 		domain = of_get_pci_domain_nr(parent->of_node);
+
 	/*
 	 * Check DT domain and use_dt_domains values.
 	 *
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] CPER: Add UEFI spec references
  2019-03-25 18:14 [PATCH 0/4] PCI, CPER: Trivial cleanups helgaas
  2019-03-25 18:14 ` [PATCH 1/4] PCI: Cleanup register definition width and whitespace helgaas
  2019-03-25 18:14 ` [PATCH 2/4] PCI: Fix comment typos helgaas
@ 2019-03-25 18:14 ` helgaas
  2019-03-25 18:14 ` [PATCH 4/4] CPER: Remove unnecessary use of user-space types helgaas
  2019-03-27 13:56 ` [PATCH 0/4] PCI, CPER: Trivial cleanups Bjorn Helgaas
  4 siblings, 0 replies; 12+ messages in thread
From: helgaas @ 2019-03-25 18:14 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas@google.com>

Add UEFI spec references for CPER UUIDs and structures, fix a few typos,
and remove some useless comments.  No functional change intended.

Link: http://www.uefi.org/specifications
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/linux/cper.h | 54 +++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/include/linux/cper.h b/include/linux/cper.h
index 9c703a0abe6e..2f361a96dc76 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -44,7 +44,7 @@
  */
 #define CPER_REC_LEN					256
 /*
- * Severity difinition for error_severity in struct cper_record_header
+ * Severity definition for error_severity in struct cper_record_header
  * and section_severity in struct cper_section_descriptor
  */
 enum {
@@ -55,24 +55,21 @@ enum {
 };
 
 /*
- * Validation bits difinition for validation_bits in struct
+ * Validation bits definition for validation_bits in struct
  * cper_record_header. If set, corresponding fields in struct
  * cper_record_header contain valid information.
- *
- * corresponds platform_id
  */
 #define CPER_VALID_PLATFORM_ID			0x0001
-/* corresponds timestamp */
 #define CPER_VALID_TIMESTAMP			0x0002
-/* corresponds partition_id */
 #define CPER_VALID_PARTITION_ID			0x0004
 
 /*
  * Notification type used to generate error record, used in
- * notification_type in struct cper_record_header
- *
- * Corrected Machine Check
+ * notification_type in struct cper_record_header.  These UUIDs are defined
+ * in the UEFI spec v2.7, sec N.2.1.
  */
+
+/* Corrected Machine Check */
 #define CPER_NOTIFY_CMC							\
 	GUID_INIT(0x2DCE8BB1, 0xBDD7, 0x450e, 0xB9, 0xAD, 0x9C, 0xF4,	\
 		  0xEB, 0xD4, 0xF8, 0x90)
@@ -122,14 +119,11 @@ enum {
 #define CPER_SEC_REV				0x0100
 
 /*
- * Validation bits difinition for validation_bits in struct
+ * Validation bits definition for validation_bits in struct
  * cper_section_descriptor. If set, corresponding fields in struct
  * cper_section_descriptor contain valid information.
- *
- * corresponds fru_id
  */
 #define CPER_SEC_VALID_FRU_ID			0x1
-/* corresponds fru_text */
 #define CPER_SEC_VALID_FRU_TEXT			0x2
 
 /*
@@ -165,10 +159,11 @@ enum {
 
 /*
  * Section type definitions, used in section_type field in struct
- * cper_section_descriptor
- *
- * Processor Generic
+ * cper_section_descriptor.  These UUIDs are defined in the UEFI spec
+ * v2.7, sec N.2.2.
  */
+
+/* Processor Generic */
 #define CPER_SEC_PROC_GENERIC						\
 	GUID_INIT(0x9876CCAD, 0x47B4, 0x4bdb, 0xB6, 0x5E, 0x16, 0xF1,	\
 		  0x93, 0xC4, 0xF3, 0xDB)
@@ -325,6 +320,7 @@ enum {
  */
 #pragma pack(1)
 
+/* Record Header, UEFI v2.7 sec N.2.1 */
 struct cper_record_header {
 	char	signature[CPER_SIG_SIZE];	/* must be CPER_SIG_RECORD */
 	__u16	revision;			/* must be CPER_RECORD_REV */
@@ -344,6 +340,7 @@ struct cper_record_header {
 	__u8	reserved[12];			/* must be zero */
 };
 
+/* Section Descriptor, UEFI v2.7 sec N.2.2 */
 struct cper_section_descriptor {
 	__u32	section_offset;		/* Offset in bytes of the
 					 *  section body from the base
@@ -359,7 +356,7 @@ struct cper_section_descriptor {
 	__u8	fru_text[20];
 };
 
-/* Generic Processor Error Section */
+/* Generic Processor Error Section, UEFI v2.7 sec N.2.4.1 */
 struct cper_sec_proc_generic {
 	__u64	validation_bits;
 	__u8	proc_type;
@@ -378,14 +375,14 @@ struct cper_sec_proc_generic {
 	__u64	ip;
 };
 
-/* IA32/X64 Processor Error Section */
+/* IA32/X64 Processor Error Section, UEFI v2.7 sec N.2.4.2 */
 struct cper_sec_proc_ia {
 	__u64	validation_bits;
 	__u64	lapic_id;
 	__u8	cpuid[48];
 };
 
-/* IA32/X64 Processor Error Information Structure */
+/* IA32/X64 Processor Error Information Structure, UEFI v2.7 sec N.2.4.2.1 */
 struct cper_ia_err_info {
 	guid_t	err_type;
 	__u64	validation_bits;
@@ -396,7 +393,7 @@ struct cper_ia_err_info {
 	__u64	ip;
 };
 
-/* IA32/X64 Processor Context Information Structure */
+/* IA32/X64 Processor Context Information Structure, UEFI v2.7 sec N.2.4.2.2 */
 struct cper_ia_proc_ctx {
 	__u16	reg_ctx_type;
 	__u16	reg_arr_size;
@@ -404,7 +401,7 @@ struct cper_ia_proc_ctx {
 	__u64	mm_reg_addr;
 };
 
-/* ARM Processor Error Section */
+/* ARM Processor Error Section, UEFI v2.7 sec N.2.4.4 */
 struct cper_sec_proc_arm {
 	__u32	validation_bits;
 	__u16	err_info_num;		/* Number of Processor Error Info */
@@ -418,7 +415,7 @@ struct cper_sec_proc_arm {
 	__u32	psci_state;
 };
 
-/* ARM Processor Error Information Structure */
+/* ARM Processor Error Information Structure, UEFI v2.7 sec N.2.4.4.1 */
 struct cper_arm_err_info {
 	__u8	version;
 	__u8	length;
@@ -431,14 +428,14 @@ struct cper_arm_err_info {
 	__u64	physical_fault_addr;
 };
 
-/* ARM Processor Context Information Structure */
+/* ARM Processor Context Information Structure, UEFI v2.7 sec N.2.4.4.2 */
 struct cper_arm_ctx_info {
 	__u16	version;
 	__u16	type;
 	__u32	size;
 };
 
-/* Old Memory Error Section UEFI 2.1, 2.2 */
+/* Old Memory Error Section, UEFI v2.1, v2.2 */
 struct cper_sec_mem_err_old {
 	__u64	validation_bits;
 	__u64	error_status;
@@ -458,7 +455,7 @@ struct cper_sec_mem_err_old {
 	__u8	error_type;
 };
 
-/* Memory Error Section UEFI >= 2.3 */
+/* Memory Error Section (UEFI >= v2.3), UEFI v2.7 sec N.2.5 */
 struct cper_sec_mem_err {
 	__u64	validation_bits;
 	__u64	error_status;
@@ -478,8 +475,8 @@ struct cper_sec_mem_err {
 	__u8	error_type;
 	__u8	reserved;
 	__u16	rank;
-	__u16	mem_array_handle;	/* card handle in UEFI 2.4 */
-	__u16	mem_dev_handle;		/* module handle in UEFI 2.4 */
+	__u16	mem_array_handle;	/* "card handle" in UEFI 2.4 */
+	__u16	mem_dev_handle;		/* "module handle" in UEFI 2.4 */
 };
 
 struct cper_mem_err_compact {
@@ -500,6 +497,7 @@ struct cper_mem_err_compact {
 	__u16	mem_dev_handle;
 };
 
+/* PCI Express Error Section, UEFI v2.7 sec N.2.7 */
 struct cper_sec_pcie {
 	__u64		validation_bits;
 	__u32		port_type;
@@ -538,7 +536,7 @@ struct cper_sec_pcie {
 /* Reset to default packing */
 #pragma pack()
 
-extern const char * const cper_proc_error_type_strs[4];
+extern const char *const cper_proc_error_type_strs[4];
 
 u64 cper_next_record_id(void);
 const char *cper_severity_str(unsigned int);
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] CPER: Remove unnecessary use of user-space types
  2019-03-25 18:14 [PATCH 0/4] PCI, CPER: Trivial cleanups helgaas
                   ` (2 preceding siblings ...)
  2019-03-25 18:14 ` [PATCH 3/4] CPER: Add UEFI spec references helgaas
@ 2019-03-25 18:14 ` helgaas
  2019-03-25 18:26   ` Bjorn Helgaas
  2019-03-27 13:56 ` [PATCH 0/4] PCI, CPER: Trivial cleanups Bjorn Helgaas
  4 siblings, 1 reply; 12+ messages in thread
From: helgaas @ 2019-03-25 18:14 UTC (permalink / raw)
  To: linux-pci, linux-kernel
  Cc: Bjorn Helgaas, Masahiro Yamada, Greg Kroah-Hartman, Andrew Morton

From: Bjorn Helgaas <bhelgaas@google.com>

"__u32" and similar types are intended for things exported to user-space,
including structs used in ioctls; see include/uapi/asm-generic/int-l64.h.

They are not needed for the CPER struct definitions, which not exported to
user-space and not used in ioctls.  Replace them with the typical "u32" and
similar types.  No functional change intended.

The reason for changing this is to remove the question of "why do we use
__u32 here instead of u32?"  We should use __u32 when there's a reason for
it; otherwise, we should prefer u32 for consistency.

Reference: Documentation/process/coding-style.rst
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Masahiro Yamada <yamada.masahiro@socionext.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/cper.h | 286 +++++++++++++++++++++----------------------
 1 file changed, 143 insertions(+), 143 deletions(-)

diff --git a/include/linux/cper.h b/include/linux/cper.h
index 2f361a96dc76..cc4980bb0f65 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -323,214 +323,214 @@ enum {
 /* Record Header, UEFI v2.7 sec N.2.1 */
 struct cper_record_header {
 	char	signature[CPER_SIG_SIZE];	/* must be CPER_SIG_RECORD */
-	__u16	revision;			/* must be CPER_RECORD_REV */
-	__u32	signature_end;			/* must be CPER_SIG_END */
-	__u16	section_count;
-	__u32	error_severity;
-	__u32	validation_bits;
-	__u32	record_length;
-	__u64	timestamp;
+	u16	revision;			/* must be CPER_RECORD_REV */
+	u32	signature_end;			/* must be CPER_SIG_END */
+	u16	section_count;
+	u32	error_severity;
+	u32	validation_bits;
+	u32	record_length;
+	u64	timestamp;
 	guid_t	platform_id;
 	guid_t	partition_id;
 	guid_t	creator_id;
 	guid_t	notification_type;
-	__u64	record_id;
-	__u32	flags;
-	__u64	persistence_information;
-	__u8	reserved[12];			/* must be zero */
+	u64	record_id;
+	u32	flags;
+	u64	persistence_information;
+	u8	reserved[12];			/* must be zero */
 };
 
 /* Section Descriptor, UEFI v2.7 sec N.2.2 */
 struct cper_section_descriptor {
-	__u32	section_offset;		/* Offset in bytes of the
+	u32	section_offset;		/* Offset in bytes of the
 					 *  section body from the base
 					 *  of the record header */
-	__u32	section_length;
-	__u16	revision;		/* must be CPER_RECORD_REV */
-	__u8	validation_bits;
-	__u8	reserved;		/* must be zero */
-	__u32	flags;
+	u32	section_length;
+	u16	revision;		/* must be CPER_RECORD_REV */
+	u8	validation_bits;
+	u8	reserved;		/* must be zero */
+	u32	flags;
 	guid_t	section_type;
 	guid_t	fru_id;
-	__u32	section_severity;
-	__u8	fru_text[20];
+	u32	section_severity;
+	u8	fru_text[20];
 };
 
 /* Generic Processor Error Section, UEFI v2.7 sec N.2.4.1 */
 struct cper_sec_proc_generic {
-	__u64	validation_bits;
-	__u8	proc_type;
-	__u8	proc_isa;
-	__u8	proc_error_type;
-	__u8	operation;
-	__u8	flags;
-	__u8	level;
-	__u16	reserved;
-	__u64	cpu_version;
+	u64	validation_bits;
+	u8	proc_type;
+	u8	proc_isa;
+	u8	proc_error_type;
+	u8	operation;
+	u8	flags;
+	u8	level;
+	u16	reserved;
+	u64	cpu_version;
 	char	cpu_brand[128];
-	__u64	proc_id;
-	__u64	target_addr;
-	__u64	requestor_id;
-	__u64	responder_id;
-	__u64	ip;
+	u64	proc_id;
+	u64	target_addr;
+	u64	requestor_id;
+	u64	responder_id;
+	u64	ip;
 };
 
 /* IA32/X64 Processor Error Section, UEFI v2.7 sec N.2.4.2 */
 struct cper_sec_proc_ia {
-	__u64	validation_bits;
-	__u64	lapic_id;
-	__u8	cpuid[48];
+	u64	validation_bits;
+	u64	lapic_id;
+	u8	cpuid[48];
 };
 
 /* IA32/X64 Processor Error Information Structure, UEFI v2.7 sec N.2.4.2.1 */
 struct cper_ia_err_info {
 	guid_t	err_type;
-	__u64	validation_bits;
-	__u64	check_info;
-	__u64	target_id;
-	__u64	requestor_id;
-	__u64	responder_id;
-	__u64	ip;
+	u64	validation_bits;
+	u64	check_info;
+	u64	target_id;
+	u64	requestor_id;
+	u64	responder_id;
+	u64	ip;
 };
 
 /* IA32/X64 Processor Context Information Structure, UEFI v2.7 sec N.2.4.2.2 */
 struct cper_ia_proc_ctx {
-	__u16	reg_ctx_type;
-	__u16	reg_arr_size;
-	__u32	msr_addr;
-	__u64	mm_reg_addr;
+	u16	reg_ctx_type;
+	u16	reg_arr_size;
+	u32	msr_addr;
+	u64	mm_reg_addr;
 };
 
 /* ARM Processor Error Section, UEFI v2.7 sec N.2.4.4 */
 struct cper_sec_proc_arm {
-	__u32	validation_bits;
-	__u16	err_info_num;		/* Number of Processor Error Info */
-	__u16	context_info_num;	/* Number of Processor Context Info Records*/
-	__u32	section_length;
-	__u8	affinity_level;
-	__u8	reserved[3];		/* must be zero */
-	__u64	mpidr;
-	__u64	midr;
-	__u32	running_state;		/* Bit 0 set - Processor running. PSCI = 0 */
-	__u32	psci_state;
+	u32	validation_bits;
+	u16	err_info_num;		/* Number of Processor Error Info */
+	u16	context_info_num;	/* Number of Processor Context Info Records*/
+	u32	section_length;
+	u8	affinity_level;
+	u8	reserved[3];		/* must be zero */
+	u64	mpidr;
+	u64	midr;
+	u32	running_state;		/* Bit 0 set - Processor running. PSCI = 0 */
+	u32	psci_state;
 };
 
 /* ARM Processor Error Information Structure, UEFI v2.7 sec N.2.4.4.1 */
 struct cper_arm_err_info {
-	__u8	version;
-	__u8	length;
-	__u16	validation_bits;
-	__u8	type;
-	__u16	multiple_error;
-	__u8	flags;
-	__u64	error_info;
-	__u64	virt_fault_addr;
-	__u64	physical_fault_addr;
+	u8	version;
+	u8	length;
+	u16	validation_bits;
+	u8	type;
+	u16	multiple_error;
+	u8	flags;
+	u64	error_info;
+	u64	virt_fault_addr;
+	u64	physical_fault_addr;
 };
 
 /* ARM Processor Context Information Structure, UEFI v2.7 sec N.2.4.4.2 */
 struct cper_arm_ctx_info {
-	__u16	version;
-	__u16	type;
-	__u32	size;
+	u16	version;
+	u16	type;
+	u32	size;
 };
 
 /* Old Memory Error Section, UEFI v2.1, v2.2 */
 struct cper_sec_mem_err_old {
-	__u64	validation_bits;
-	__u64	error_status;
-	__u64	physical_addr;
-	__u64	physical_addr_mask;
-	__u16	node;
-	__u16	card;
-	__u16	module;
-	__u16	bank;
-	__u16	device;
-	__u16	row;
-	__u16	column;
-	__u16	bit_pos;
-	__u64	requestor_id;
-	__u64	responder_id;
-	__u64	target_id;
-	__u8	error_type;
+	u64	validation_bits;
+	u64	error_status;
+	u64	physical_addr;
+	u64	physical_addr_mask;
+	u16	node;
+	u16	card;
+	u16	module;
+	u16	bank;
+	u16	device;
+	u16	row;
+	u16	column;
+	u16	bit_pos;
+	u64	requestor_id;
+	u64	responder_id;
+	u64	target_id;
+	u8	error_type;
 };
 
 /* Memory Error Section (UEFI >= v2.3), UEFI v2.7 sec N.2.5 */
 struct cper_sec_mem_err {
-	__u64	validation_bits;
-	__u64	error_status;
-	__u64	physical_addr;
-	__u64	physical_addr_mask;
-	__u16	node;
-	__u16	card;
-	__u16	module;
-	__u16	bank;
-	__u16	device;
-	__u16	row;
-	__u16	column;
-	__u16	bit_pos;
-	__u64	requestor_id;
-	__u64	responder_id;
-	__u64	target_id;
-	__u8	error_type;
-	__u8	reserved;
-	__u16	rank;
-	__u16	mem_array_handle;	/* "card handle" in UEFI 2.4 */
-	__u16	mem_dev_handle;		/* "module handle" in UEFI 2.4 */
+	u64	validation_bits;
+	u64	error_status;
+	u64	physical_addr;
+	u64	physical_addr_mask;
+	u16	node;
+	u16	card;
+	u16	module;
+	u16	bank;
+	u16	device;
+	u16	row;
+	u16	column;
+	u16	bit_pos;
+	u64	requestor_id;
+	u64	responder_id;
+	u64	target_id;
+	u8	error_type;
+	u8	reserved;
+	u16	rank;
+	u16	mem_array_handle;	/* "card handle" in UEFI 2.4 */
+	u16	mem_dev_handle;		/* "module handle" in UEFI 2.4 */
 };
 
 struct cper_mem_err_compact {
-	__u64	validation_bits;
-	__u16	node;
-	__u16	card;
-	__u16	module;
-	__u16	bank;
-	__u16	device;
-	__u16	row;
-	__u16	column;
-	__u16	bit_pos;
-	__u64	requestor_id;
-	__u64	responder_id;
-	__u64	target_id;
-	__u16	rank;
-	__u16	mem_array_handle;
-	__u16	mem_dev_handle;
+	u64	validation_bits;
+	u16	node;
+	u16	card;
+	u16	module;
+	u16	bank;
+	u16	device;
+	u16	row;
+	u16	column;
+	u16	bit_pos;
+	u64	requestor_id;
+	u64	responder_id;
+	u64	target_id;
+	u16	rank;
+	u16	mem_array_handle;
+	u16	mem_dev_handle;
 };
 
 /* PCI Express Error Section, UEFI v2.7 sec N.2.7 */
 struct cper_sec_pcie {
-	__u64		validation_bits;
-	__u32		port_type;
+	u64		validation_bits;
+	u32		port_type;
 	struct {
-		__u8	minor;
-		__u8	major;
-		__u8	reserved[2];
+		u8	minor;
+		u8	major;
+		u8	reserved[2];
 	}		version;
-	__u16		command;
-	__u16		status;
-	__u32		reserved;
+	u16		command;
+	u16		status;
+	u32		reserved;
 	struct {
-		__u16	vendor_id;
-		__u16	device_id;
-		__u8	class_code[3];
-		__u8	function;
-		__u8	device;
-		__u16	segment;
-		__u8	bus;
-		__u8	secondary_bus;
-		__u16	slot;
-		__u8	reserved;
+		u16	vendor_id;
+		u16	device_id;
+		u8	class_code[3];
+		u8	function;
+		u8	device;
+		u16	segment;
+		u8	bus;
+		u8	secondary_bus;
+		u16	slot;
+		u8	reserved;
 	}		device_id;
 	struct {
-		__u32	lower;
-		__u32	upper;
+		u32	lower;
+		u32	upper;
 	}		serial_number;
 	struct {
-		__u16	secondary_status;
-		__u16	control;
+		u16	secondary_status;
+		u16	control;
 	}		bridge;
-	__u8	capability[60];
-	__u8	aer_info[96];
+	u8	capability[60];
+	u8	aer_info[96];
 };
 
 /* Reset to default packing */
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] CPER: Remove unnecessary use of user-space types
  2019-03-25 18:14 ` [PATCH 4/4] CPER: Remove unnecessary use of user-space types helgaas
@ 2019-03-25 18:26   ` Bjorn Helgaas
  2019-03-25 19:13     ` Joe Perches
                       ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2019-03-25 18:26 UTC (permalink / raw)
  To: linux-pci, linux-kernel
  Cc: Masahiro Yamada, Greg Kroah-Hartman, Andrew Morton

On Mon, Mar 25, 2019 at 01:14:25PM -0500, helgaas@kernel.org wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> "__u32" and similar types are intended for things exported to user-space,
> including structs used in ioctls; see include/uapi/asm-generic/int-l64.h.
> 
> They are not needed for the CPER struct definitions, which not exported to
> user-space and not used in ioctls.  Replace them with the typical "u32" and
> similar types.  No functional change intended.
> 
> The reason for changing this is to remove the question of "why do we use
> __u32 here instead of u32?"  We should use __u32 when there's a reason for
> it; otherwise, we should prefer u32 for consistency.
> 
> Reference: Documentation/process/coding-style.rst
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Masahiro Yamada <yamada.masahiro@socionext.com>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Andrew Morton <akpm@linux-foundation.org>

I cc'd you folks because you were part of this conversation:

  https://lore.kernel.org/lkml/1526350925-14922-3-git-send-email-yamada.masahiro@socionext.com/T/#u

I *think* the conclusion there was that this sort of change makes
sense, but I want to make sure.  If it does make sense, I'm surprised
at how much stuff in include/linux/ still uses __u32 when it doesn't
appear to need it.

> ---
>  include/linux/cper.h | 286 +++++++++++++++++++++----------------------
>  1 file changed, 143 insertions(+), 143 deletions(-)
> 
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index 2f361a96dc76..cc4980bb0f65 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -323,214 +323,214 @@ enum {
>  /* Record Header, UEFI v2.7 sec N.2.1 */
>  struct cper_record_header {
>  	char	signature[CPER_SIG_SIZE];	/* must be CPER_SIG_RECORD */
> -	__u16	revision;			/* must be CPER_RECORD_REV */
> -	__u32	signature_end;			/* must be CPER_SIG_END */
> -	__u16	section_count;
> -	__u32	error_severity;
> -	__u32	validation_bits;
> -	__u32	record_length;
> -	__u64	timestamp;
> +	u16	revision;			/* must be CPER_RECORD_REV */
> +	u32	signature_end;			/* must be CPER_SIG_END */
> +	u16	section_count;
> +	u32	error_severity;
> +	u32	validation_bits;
> +	u32	record_length;
> +	u64	timestamp;
>  	guid_t	platform_id;
>  	guid_t	partition_id;
>  	guid_t	creator_id;
>  	guid_t	notification_type;
> -	__u64	record_id;
> -	__u32	flags;
> -	__u64	persistence_information;
> -	__u8	reserved[12];			/* must be zero */
> +	u64	record_id;
> +	u32	flags;
> +	u64	persistence_information;
> +	u8	reserved[12];			/* must be zero */
>  };
>  
>  /* Section Descriptor, UEFI v2.7 sec N.2.2 */
>  struct cper_section_descriptor {
> -	__u32	section_offset;		/* Offset in bytes of the
> +	u32	section_offset;		/* Offset in bytes of the
>  					 *  section body from the base
>  					 *  of the record header */
> -	__u32	section_length;
> -	__u16	revision;		/* must be CPER_RECORD_REV */
> -	__u8	validation_bits;
> -	__u8	reserved;		/* must be zero */
> -	__u32	flags;
> +	u32	section_length;
> +	u16	revision;		/* must be CPER_RECORD_REV */
> +	u8	validation_bits;
> +	u8	reserved;		/* must be zero */
> +	u32	flags;
>  	guid_t	section_type;
>  	guid_t	fru_id;
> -	__u32	section_severity;
> -	__u8	fru_text[20];
> +	u32	section_severity;
> +	u8	fru_text[20];
>  };
>  
>  /* Generic Processor Error Section, UEFI v2.7 sec N.2.4.1 */
>  struct cper_sec_proc_generic {
> -	__u64	validation_bits;
> -	__u8	proc_type;
> -	__u8	proc_isa;
> -	__u8	proc_error_type;
> -	__u8	operation;
> -	__u8	flags;
> -	__u8	level;
> -	__u16	reserved;
> -	__u64	cpu_version;
> +	u64	validation_bits;
> +	u8	proc_type;
> +	u8	proc_isa;
> +	u8	proc_error_type;
> +	u8	operation;
> +	u8	flags;
> +	u8	level;
> +	u16	reserved;
> +	u64	cpu_version;
>  	char	cpu_brand[128];
> -	__u64	proc_id;
> -	__u64	target_addr;
> -	__u64	requestor_id;
> -	__u64	responder_id;
> -	__u64	ip;
> +	u64	proc_id;
> +	u64	target_addr;
> +	u64	requestor_id;
> +	u64	responder_id;
> +	u64	ip;
>  };
>  
>  /* IA32/X64 Processor Error Section, UEFI v2.7 sec N.2.4.2 */
>  struct cper_sec_proc_ia {
> -	__u64	validation_bits;
> -	__u64	lapic_id;
> -	__u8	cpuid[48];
> +	u64	validation_bits;
> +	u64	lapic_id;
> +	u8	cpuid[48];
>  };
>  
>  /* IA32/X64 Processor Error Information Structure, UEFI v2.7 sec N.2.4.2.1 */
>  struct cper_ia_err_info {
>  	guid_t	err_type;
> -	__u64	validation_bits;
> -	__u64	check_info;
> -	__u64	target_id;
> -	__u64	requestor_id;
> -	__u64	responder_id;
> -	__u64	ip;
> +	u64	validation_bits;
> +	u64	check_info;
> +	u64	target_id;
> +	u64	requestor_id;
> +	u64	responder_id;
> +	u64	ip;
>  };
>  
>  /* IA32/X64 Processor Context Information Structure, UEFI v2.7 sec N.2.4.2.2 */
>  struct cper_ia_proc_ctx {
> -	__u16	reg_ctx_type;
> -	__u16	reg_arr_size;
> -	__u32	msr_addr;
> -	__u64	mm_reg_addr;
> +	u16	reg_ctx_type;
> +	u16	reg_arr_size;
> +	u32	msr_addr;
> +	u64	mm_reg_addr;
>  };
>  
>  /* ARM Processor Error Section, UEFI v2.7 sec N.2.4.4 */
>  struct cper_sec_proc_arm {
> -	__u32	validation_bits;
> -	__u16	err_info_num;		/* Number of Processor Error Info */
> -	__u16	context_info_num;	/* Number of Processor Context Info Records*/
> -	__u32	section_length;
> -	__u8	affinity_level;
> -	__u8	reserved[3];		/* must be zero */
> -	__u64	mpidr;
> -	__u64	midr;
> -	__u32	running_state;		/* Bit 0 set - Processor running. PSCI = 0 */
> -	__u32	psci_state;
> +	u32	validation_bits;
> +	u16	err_info_num;		/* Number of Processor Error Info */
> +	u16	context_info_num;	/* Number of Processor Context Info Records*/
> +	u32	section_length;
> +	u8	affinity_level;
> +	u8	reserved[3];		/* must be zero */
> +	u64	mpidr;
> +	u64	midr;
> +	u32	running_state;		/* Bit 0 set - Processor running. PSCI = 0 */
> +	u32	psci_state;
>  };
>  
>  /* ARM Processor Error Information Structure, UEFI v2.7 sec N.2.4.4.1 */
>  struct cper_arm_err_info {
> -	__u8	version;
> -	__u8	length;
> -	__u16	validation_bits;
> -	__u8	type;
> -	__u16	multiple_error;
> -	__u8	flags;
> -	__u64	error_info;
> -	__u64	virt_fault_addr;
> -	__u64	physical_fault_addr;
> +	u8	version;
> +	u8	length;
> +	u16	validation_bits;
> +	u8	type;
> +	u16	multiple_error;
> +	u8	flags;
> +	u64	error_info;
> +	u64	virt_fault_addr;
> +	u64	physical_fault_addr;
>  };
>  
>  /* ARM Processor Context Information Structure, UEFI v2.7 sec N.2.4.4.2 */
>  struct cper_arm_ctx_info {
> -	__u16	version;
> -	__u16	type;
> -	__u32	size;
> +	u16	version;
> +	u16	type;
> +	u32	size;
>  };
>  
>  /* Old Memory Error Section, UEFI v2.1, v2.2 */
>  struct cper_sec_mem_err_old {
> -	__u64	validation_bits;
> -	__u64	error_status;
> -	__u64	physical_addr;
> -	__u64	physical_addr_mask;
> -	__u16	node;
> -	__u16	card;
> -	__u16	module;
> -	__u16	bank;
> -	__u16	device;
> -	__u16	row;
> -	__u16	column;
> -	__u16	bit_pos;
> -	__u64	requestor_id;
> -	__u64	responder_id;
> -	__u64	target_id;
> -	__u8	error_type;
> +	u64	validation_bits;
> +	u64	error_status;
> +	u64	physical_addr;
> +	u64	physical_addr_mask;
> +	u16	node;
> +	u16	card;
> +	u16	module;
> +	u16	bank;
> +	u16	device;
> +	u16	row;
> +	u16	column;
> +	u16	bit_pos;
> +	u64	requestor_id;
> +	u64	responder_id;
> +	u64	target_id;
> +	u8	error_type;
>  };
>  
>  /* Memory Error Section (UEFI >= v2.3), UEFI v2.7 sec N.2.5 */
>  struct cper_sec_mem_err {
> -	__u64	validation_bits;
> -	__u64	error_status;
> -	__u64	physical_addr;
> -	__u64	physical_addr_mask;
> -	__u16	node;
> -	__u16	card;
> -	__u16	module;
> -	__u16	bank;
> -	__u16	device;
> -	__u16	row;
> -	__u16	column;
> -	__u16	bit_pos;
> -	__u64	requestor_id;
> -	__u64	responder_id;
> -	__u64	target_id;
> -	__u8	error_type;
> -	__u8	reserved;
> -	__u16	rank;
> -	__u16	mem_array_handle;	/* "card handle" in UEFI 2.4 */
> -	__u16	mem_dev_handle;		/* "module handle" in UEFI 2.4 */
> +	u64	validation_bits;
> +	u64	error_status;
> +	u64	physical_addr;
> +	u64	physical_addr_mask;
> +	u16	node;
> +	u16	card;
> +	u16	module;
> +	u16	bank;
> +	u16	device;
> +	u16	row;
> +	u16	column;
> +	u16	bit_pos;
> +	u64	requestor_id;
> +	u64	responder_id;
> +	u64	target_id;
> +	u8	error_type;
> +	u8	reserved;
> +	u16	rank;
> +	u16	mem_array_handle;	/* "card handle" in UEFI 2.4 */
> +	u16	mem_dev_handle;		/* "module handle" in UEFI 2.4 */
>  };
>  
>  struct cper_mem_err_compact {
> -	__u64	validation_bits;
> -	__u16	node;
> -	__u16	card;
> -	__u16	module;
> -	__u16	bank;
> -	__u16	device;
> -	__u16	row;
> -	__u16	column;
> -	__u16	bit_pos;
> -	__u64	requestor_id;
> -	__u64	responder_id;
> -	__u64	target_id;
> -	__u16	rank;
> -	__u16	mem_array_handle;
> -	__u16	mem_dev_handle;
> +	u64	validation_bits;
> +	u16	node;
> +	u16	card;
> +	u16	module;
> +	u16	bank;
> +	u16	device;
> +	u16	row;
> +	u16	column;
> +	u16	bit_pos;
> +	u64	requestor_id;
> +	u64	responder_id;
> +	u64	target_id;
> +	u16	rank;
> +	u16	mem_array_handle;
> +	u16	mem_dev_handle;
>  };
>  
>  /* PCI Express Error Section, UEFI v2.7 sec N.2.7 */
>  struct cper_sec_pcie {
> -	__u64		validation_bits;
> -	__u32		port_type;
> +	u64		validation_bits;
> +	u32		port_type;
>  	struct {
> -		__u8	minor;
> -		__u8	major;
> -		__u8	reserved[2];
> +		u8	minor;
> +		u8	major;
> +		u8	reserved[2];
>  	}		version;
> -	__u16		command;
> -	__u16		status;
> -	__u32		reserved;
> +	u16		command;
> +	u16		status;
> +	u32		reserved;
>  	struct {
> -		__u16	vendor_id;
> -		__u16	device_id;
> -		__u8	class_code[3];
> -		__u8	function;
> -		__u8	device;
> -		__u16	segment;
> -		__u8	bus;
> -		__u8	secondary_bus;
> -		__u16	slot;
> -		__u8	reserved;
> +		u16	vendor_id;
> +		u16	device_id;
> +		u8	class_code[3];
> +		u8	function;
> +		u8	device;
> +		u16	segment;
> +		u8	bus;
> +		u8	secondary_bus;
> +		u16	slot;
> +		u8	reserved;
>  	}		device_id;
>  	struct {
> -		__u32	lower;
> -		__u32	upper;
> +		u32	lower;
> +		u32	upper;
>  	}		serial_number;
>  	struct {
> -		__u16	secondary_status;
> -		__u16	control;
> +		u16	secondary_status;
> +		u16	control;
>  	}		bridge;
> -	__u8	capability[60];
> -	__u8	aer_info[96];
> +	u8	capability[60];
> +	u8	aer_info[96];
>  };
>  
>  /* Reset to default packing */
> -- 
> 2.21.0.392.gf8f6787159e-goog
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] CPER: Remove unnecessary use of user-space types
  2019-03-25 18:26   ` Bjorn Helgaas
@ 2019-03-25 19:13     ` Joe Perches
  2019-03-25 19:56     ` Greg Kroah-Hartman
  2019-03-30 12:52     ` Masahiro Yamada
  2 siblings, 0 replies; 12+ messages in thread
From: Joe Perches @ 2019-03-25 19:13 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, linux-kernel
  Cc: Masahiro Yamada, Greg Kroah-Hartman, Andrew Morton

On Mon, 2019-03-25 at 13:26 -0500, Bjorn Helgaas wrote:
> On Mon, Mar 25, 2019 at 01:14:25PM -0500, helgaas@kernel.org wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > "__u32" and similar types are intended for things exported to user-space,
> > including structs used in ioctls; see include/uapi/asm-generic/int-l64.h.
> > 
> > They are not needed for the CPER struct definitions, which not exported to
> > user-space and not used in ioctls.  Replace them with the typical "u32" and
> > similar types.  No functional change intended.
[]
> I *think* the conclusion there was that this sort of change makes
> sense, but I want to make sure.  If it does make sense, I'm surprised
> at how much stuff in include/linux/ still uses __u32 when it doesn't
> appear to need it.

It is a fairly large number.

$ git grep -w -P '__[us](?:8|16|32|64)' include/linux/ | wc -l
2318

It's a rather large number of patches though to make all the
actual function definitions match the declarations that are
spread around the kernel sources as well.

Whatever struct member uses of possible temporary assignments
could also be changed to match.

Perhaps it's a lot of churn for relatively little benefit
though I do find __ prefixes unsightly when unnecessary.

A gradual conversion would eventually help isolate uapi uses
in the kernel sources.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] CPER: Remove unnecessary use of user-space types
  2019-03-25 18:26   ` Bjorn Helgaas
  2019-03-25 19:13     ` Joe Perches
@ 2019-03-25 19:56     ` Greg Kroah-Hartman
  2019-03-30 12:52     ` Masahiro Yamada
  2 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2019-03-25 19:56 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, Masahiro Yamada, Andrew Morton

On Mon, Mar 25, 2019 at 01:26:08PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 25, 2019 at 01:14:25PM -0500, helgaas@kernel.org wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > "__u32" and similar types are intended for things exported to user-space,
> > including structs used in ioctls; see include/uapi/asm-generic/int-l64.h.
> > 
> > They are not needed for the CPER struct definitions, which not exported to
> > user-space and not used in ioctls.  Replace them with the typical "u32" and
> > similar types.  No functional change intended.
> > 
> > The reason for changing this is to remove the question of "why do we use
> > __u32 here instead of u32?"  We should use __u32 when there's a reason for
> > it; otherwise, we should prefer u32 for consistency.
> > 
> > Reference: Documentation/process/coding-style.rst
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Masahiro Yamada <yamada.masahiro@socionext.com>
> > CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > CC: Andrew Morton <akpm@linux-foundation.org>
> 
> I cc'd you folks because you were part of this conversation:
> 
>   https://lore.kernel.org/lkml/1526350925-14922-3-git-send-email-yamada.masahiro@socionext.com/T/#u
> 
> I *think* the conclusion there was that this sort of change makes
> sense, but I want to make sure.  If it does make sense, I'm surprised
> at how much stuff in include/linux/ still uses __u32 when it doesn't
> appear to need it.

People just cut/paste and don't think about it.  We used to have a bunch
of known structures that didn't use __u32 and friends as people didn't
realize it, so it doesn't surprise me that the other way is also the
case :(

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/4] PCI, CPER: Trivial cleanups
  2019-03-25 18:14 [PATCH 0/4] PCI, CPER: Trivial cleanups helgaas
                   ` (3 preceding siblings ...)
  2019-03-25 18:14 ` [PATCH 4/4] CPER: Remove unnecessary use of user-space types helgaas
@ 2019-03-27 13:56 ` Bjorn Helgaas
  4 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2019-03-27 13:56 UTC (permalink / raw)
  To: linux-pci, linux-kernel

On Mon, Mar 25, 2019 at 01:14:21PM -0500, helgaas@kernel.org wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Clean up whitespace, fix some typos, add some spec references, and convert
> unnecessary __u32 usage to u32.
> 
> Bjorn Helgaas (4):
>   PCI: Cleanup register definition width and whitespace
>   PCI: Fix comment typos
>   CPER: Add UEFI spec references
>   CPER: Remove unnecessary use of user-space types

These are on pci/trivial for v5.2.

>  drivers/pci/controller/dwc/pci-keystone.c |   2 +-
>  drivers/pci/controller/pci-host-generic.c |   2 +-
>  drivers/pci/controller/pcie-iproc-msi.c   |   2 +-
>  drivers/pci/pci.c                         | 328 +++++++++++----------
>  include/linux/cper.h                      | 336 +++++++++++-----------
>  include/uapi/linux/pci_regs.h             | 132 +++++----
>  6 files changed, 405 insertions(+), 397 deletions(-)
> 
> -- 
> 2.21.0.392.gf8f6787159e-goog
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] CPER: Remove unnecessary use of user-space types
  2019-03-25 18:26   ` Bjorn Helgaas
  2019-03-25 19:13     ` Joe Perches
  2019-03-25 19:56     ` Greg Kroah-Hartman
@ 2019-03-30 12:52     ` Masahiro Yamada
  2 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2019-03-30 12:52 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Linux Kernel Mailing List, Greg Kroah-Hartman, Andrew Morton

On Tue, Mar 26, 2019 at 3:26 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Mar 25, 2019 at 01:14:25PM -0500, helgaas@kernel.org wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > "__u32" and similar types are intended for things exported to user-space,
> > including structs used in ioctls; see include/uapi/asm-generic/int-l64.h.
> >
> > They are not needed for the CPER struct definitions, which not exported to
> > user-space and not used in ioctls.  Replace them with the typical "u32" and
> > similar types.  No functional change intended.
> >
> > The reason for changing this is to remove the question of "why do we use
> > __u32 here instead of u32?"  We should use __u32 when there's a reason for
> > it; otherwise, we should prefer u32 for consistency.
> >
> > Reference: Documentation/process/coding-style.rst
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Masahiro Yamada <yamada.masahiro@socionext.com>
> > CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > CC: Andrew Morton <akpm@linux-foundation.org>
>
> I cc'd you folks because you were part of this conversation:
>
>   https://lore.kernel.org/lkml/1526350925-14922-3-git-send-email-yamada.masahiro@socionext.com/T/#u
>
> I *think* the conclusion there was that this sort of change makes
> sense, but I want to make sure.  If it does make sense, I'm surprised
> at how much stuff in include/linux/ still uses __u32 when it doesn't
> appear to need it.


This patch looks good to me.

I still fail to understand Greg's comment in
the referred URL, though.


"__u32" and similar types are intended for things exported to user-space.

This is clearly documented.

Documentation/process/coding-style.rst:

 (e) Types safe for use in userspace.

     In certain structures which are visible to userspace, we cannot
     require C99 types and cannot use the ``u32`` form above. Thus, we
     use __u32 and similar types in all structures which are shared
     with userspace.



I'd be eager to see a document that suggests __u32 and similar types
in ioctl structures.




--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] PCI: Fix comment typos
  2019-03-25 18:14 ` [PATCH 2/4] PCI: Fix comment typos helgaas
@ 2019-03-30 17:56   ` Mukesh Ojha
  2019-04-01 13:43     ` Bjorn Helgaas
  0 siblings, 1 reply; 12+ messages in thread
From: Mukesh Ojha @ 2019-03-30 17:56 UTC (permalink / raw)
  To: helgaas, linux-pci, linux-kernel; +Cc: Bjorn Helgaas


On 3/25/2019 11:44 PM, helgaas@kernel.org wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Fix spelling errors and format function comments consistently.  Changes
> whitespace and comments only; no functional change intended.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh

> ---
>   drivers/pci/controller/dwc/pci-keystone.c |   2 +-
>   drivers/pci/controller/pci-host-generic.c |   2 +-
>   drivers/pci/controller/pcie-iproc-msi.c   |   2 +-
>   drivers/pci/pci.c                         | 328 +++++++++++-----------
>   4 files changed, 173 insertions(+), 161 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 14f2b0b4ed5e..9b4112095658 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -661,7 +661,7 @@ static int ks_pcie_get_irq_controller_info(struct keystone_pcie *ks_pcie,
>   			(legacy ? "legacy" : "MSI"), temp);
>   
>   	/*
> -	 * support upto max_host_irqs. In dt from index 0 to 3 (legacy) or 0 to
> +	 * support up to max_host_irqs. In DT from index 0 to 3 (legacy) or 0 to
>   	 * 7 (MSI)
>   	 */
>   	for (temp = 0; temp < max_host_irqs; temp++) {
> diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
> index dea3ec7592a2..75a2fb930d4b 100644
> --- a/drivers/pci/controller/pci-host-generic.c
> +++ b/drivers/pci/controller/pci-host-generic.c
> @@ -1,6 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /*
> - * Simple, generic PCI host controller driver targetting firmware-initialised
> + * Simple, generic PCI host controller driver targeting firmware-initialised
>    * systems and virtual machines (e.g. the PCI emulation provided by kvmtool).
>    *
>    * Copyright (C) 2014 ARM Limited
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index cb3401a931f8..0a3f61be5625 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -367,7 +367,7 @@ static void iproc_msi_handler(struct irq_desc *desc)
>   
>   		/*
>   		 * Now go read the tail pointer again to see if there are new
> -		 * oustanding events that came in during the above window.
> +		 * outstanding events that came in during the above window.
>   		 */
>   	} while (true);
>   
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7c1b362f599a..530eec3191e7 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -197,8 +197,8 @@ EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
>   
>   /**
>    * pci_dev_str_match_path - test if a path string matches a device
> - * @dev:    the PCI device to test
> - * @path:   string to match the device against
> + * @dev: the PCI device to test
> + * @path: string to match the device against
>    * @endptr: pointer to the string after the match
>    *
>    * Test if a string (typically from a kernel parameter) formatted as a
> @@ -280,8 +280,8 @@ static int pci_dev_str_match_path(struct pci_dev *dev, const char *path,
>   
>   /**
>    * pci_dev_str_match - test if a string matches a device
> - * @dev:    the PCI device to test
> - * @p:      string to match the device against
> + * @dev: the PCI device to test
> + * @p: string to match the device against
>    * @endptr: pointer to the string after the match
>    *
>    * Test if a string (typically from a kernel parameter) matches a specified
> @@ -341,7 +341,7 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>   	} else {
>   		/*
>   		 * PCI Bus, Device, Function IDs are specified
> -		 *  (optionally, may include a path of devfns following it)
> +		 * (optionally, may include a path of devfns following it)
>   		 */
>   		ret = pci_dev_str_match_path(dev, p, &p);
>   		if (ret < 0)
> @@ -425,7 +425,7 @@ static int __pci_bus_find_cap_start(struct pci_bus *bus,
>    * Tell if a device supports a given PCI capability.
>    * Returns the address of the requested capability structure within the
>    * device's PCI configuration space or 0 in case the device does not
> - * support it.  Possible values for @cap:
> + * support it.  Possible values for @cap include:
>    *
>    *  %PCI_CAP_ID_PM           Power Management
>    *  %PCI_CAP_ID_AGP          Accelerated Graphics Port
> @@ -450,11 +450,11 @@ EXPORT_SYMBOL(pci_find_capability);
>   
>   /**
>    * pci_bus_find_capability - query for devices' capabilities
> - * @bus:   the PCI bus to query
> + * @bus: the PCI bus to query
>    * @devfn: PCI device to query
> - * @cap:   capability code
> + * @cap: capability code
>    *
> - * Like pci_find_capability() but works for pci devices that do not have a
> + * Like pci_find_capability() but works for PCI devices that do not have a
>    * pci_dev structure set up yet.
>    *
>    * Returns the address of the requested capability structure within the
> @@ -535,7 +535,7 @@ EXPORT_SYMBOL_GPL(pci_find_next_ext_capability);
>    *
>    * Returns the address of the requested extended capability structure
>    * within the device's PCI configuration space or 0 if the device does
> - * not support it.  Possible values for @cap:
> + * not support it.  Possible values for @cap include:
>    *
>    *  %PCI_EXT_CAP_ID_ERR		Advanced Error Reporting
>    *  %PCI_EXT_CAP_ID_VC		Virtual Channel
> @@ -618,12 +618,13 @@ int pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
>   EXPORT_SYMBOL_GPL(pci_find_ht_capability);
>   
>   /**
> - * pci_find_parent_resource - return resource region of parent bus of given region
> + * pci_find_parent_resource - return resource region of parent bus of given
> + *			      region
>    * @dev: PCI device structure contains resources to be searched
>    * @res: child resource record for which parent is sought
>    *
> - *  For given resource region of given device, return the resource
> - *  region of parent bus the given region is contained in.
> + * For given resource region of given device, return the resource region of
> + * parent bus the given region is contained in.
>    */
>   struct resource *pci_find_parent_resource(const struct pci_dev *dev,
>   					  struct resource *res)
> @@ -800,7 +801,7 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
>   
>   /**
>    * pci_raw_set_power_state - Use PCI PM registers to set the power state of
> - *                           given PCI device
> + *			     given PCI device
>    * @dev: PCI device to handle.
>    * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
>    *
> @@ -826,7 +827,8 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
>   	if (state < PCI_D0 || state > PCI_D3hot)
>   		return -EINVAL;
>   
> -	/* Validate current state:
> +	/*
> +	 * Validate current state:
>   	 * Can enter D0 from any state, but if we can only go deeper
>   	 * to sleep if we're already in a low power state
>   	 */
> @@ -837,14 +839,15 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
>   		return -EINVAL;
>   	}
>   
> -	/* check if this device supports the desired state */
> +	/* Check if this device supports the desired state */
>   	if ((state == PCI_D1 && !dev->d1_support)
>   	   || (state == PCI_D2 && !dev->d2_support))
>   		return -EIO;
>   
>   	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
>   
> -	/* If we're (effectively) in D3, force entire word to 0.
> +	/*
> +	 * If we're (effectively) in D3, force entire word to 0.
>   	 * This doesn't affect PME_Status, disables PME_En, and
>   	 * sets PowerState to 0.
>   	 */
> @@ -867,11 +870,13 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
>   		break;
>   	}
>   
> -	/* enter specified state */
> +	/* Enter specified state */
>   	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
>   
> -	/* Mandatory power management transition delays */
> -	/* see PCI PM 1.1 5.6.1 table 18 */
> +	/*
> +	 * Mandatory power management transition delays; see PCI PM 1.1
> +	 * 5.6.1 table 18
> +	 */
>   	if (state == PCI_D3hot || dev->current_state == PCI_D3hot)
>   		pci_dev_d3_sleep(dev);
>   	else if (state == PCI_D2 || dev->current_state == PCI_D2)
> @@ -1085,16 +1090,18 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
>   {
>   	int error;
>   
> -	/* bound the state we're entering */
> +	/* Bound the state we're entering */
>   	if (state > PCI_D3cold)
>   		state = PCI_D3cold;
>   	else if (state < PCI_D0)
>   		state = PCI_D0;
>   	else if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
> +
>   		/*
> -		 * If the device or the parent bridge do not support PCI PM,
> -		 * ignore the request if we're doing anything other than putting
> -		 * it into D0 (which would only happen on boot).
> +		 * If the device or the parent bridge do not support PCI
> +		 * PM, ignore the request if we're doing anything other
> +		 * than putting it into D0 (which would only happen on
> +		 * boot).
>   		 */
>   		return 0;
>   
> @@ -1104,8 +1111,10 @@ int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
>   
>   	__pci_start_power_transition(dev, state);
>   
> -	/* This device is quirked not to be put into D3, so
> -	   don't put it in D3 */
> +	/*
> +	 * This device is quirked not to be put into D3, so don't put it in
> +	 * D3
> +	 */
>   	if (state >= PCI_D3hot && (dev->dev_flags & PCI_DEV_FLAGS_NO_D3))
>   		return 0;
>   
> @@ -1127,12 +1136,11 @@ EXPORT_SYMBOL(pci_set_power_state);
>    * pci_choose_state - Choose the power state of a PCI device
>    * @dev: PCI device to be suspended
>    * @state: target sleep state for the whole system. This is the value
> - *	that is passed to suspend() function.
> + *	   that is passed to suspend() function.
>    *
>    * Returns PCI power state suitable for given device and given system
>    * message.
>    */
> -
>   pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state)
>   {
>   	pci_power_t ret;
> @@ -1310,8 +1318,9 @@ static void pci_restore_ltr_state(struct pci_dev *dev)
>   }
>   
>   /**
> - * pci_save_state - save the PCI configuration space of a device before suspending
> - * @dev: - PCI device that we're dealing with
> + * pci_save_state - save the PCI configuration space of a device before
> + *		    suspending
> + * @dev: PCI device that we're dealing with
>    */
>   int pci_save_state(struct pci_dev *dev)
>   {
> @@ -1422,7 +1431,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
>   
>   /**
>    * pci_restore_state - Restore the saved state of a PCI device
> - * @dev: - PCI device that we're dealing with
> + * @dev: PCI device that we're dealing with
>    */
>   void pci_restore_state(struct pci_dev *dev)
>   {
> @@ -1599,8 +1608,8 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>    * pci_reenable_device - Resume abandoned device
>    * @dev: PCI device to be resumed
>    *
> - *  Note this function is a backend of pci_default_resume and is not supposed
> - *  to be called by normal code, write proper resume handler and use it instead.
> + * NOTE: This function is a backend of pci_default_resume() and is not supposed
> + * to be called by normal code, write proper resume handler and use it instead.
>    */
>   int pci_reenable_device(struct pci_dev *dev)
>   {
> @@ -1675,9 +1684,9 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
>    * pci_enable_device_io - Initialize a device for use with IO space
>    * @dev: PCI device to be initialized
>    *
> - *  Initialize device before it's used by a driver. Ask low-level code
> - *  to enable I/O resources. Wake up the device if it was suspended.
> - *  Beware, this function can fail.
> + * Initialize device before it's used by a driver. Ask low-level code
> + * to enable I/O resources. Wake up the device if it was suspended.
> + * Beware, this function can fail.
>    */
>   int pci_enable_device_io(struct pci_dev *dev)
>   {
> @@ -1689,9 +1698,9 @@ EXPORT_SYMBOL(pci_enable_device_io);
>    * pci_enable_device_mem - Initialize a device for use with Memory space
>    * @dev: PCI device to be initialized
>    *
> - *  Initialize device before it's used by a driver. Ask low-level code
> - *  to enable Memory resources. Wake up the device if it was suspended.
> - *  Beware, this function can fail.
> + * Initialize device before it's used by a driver. Ask low-level code
> + * to enable Memory resources. Wake up the device if it was suspended.
> + * Beware, this function can fail.
>    */
>   int pci_enable_device_mem(struct pci_dev *dev)
>   {
> @@ -1703,12 +1712,12 @@ EXPORT_SYMBOL(pci_enable_device_mem);
>    * pci_enable_device - Initialize device before it's used by a driver.
>    * @dev: PCI device to be initialized
>    *
> - *  Initialize device before it's used by a driver. Ask low-level code
> - *  to enable I/O and memory. Wake up the device if it was suspended.
> - *  Beware, this function can fail.
> + * Initialize device before it's used by a driver. Ask low-level code
> + * to enable I/O and memory. Wake up the device if it was suspended.
> + * Beware, this function can fail.
>    *
> - *  Note we don't actually enable the device many times if we call
> - *  this function repeatedly (we just increment the count).
> + * Note we don't actually enable the device many times if we call
> + * this function repeatedly (we just increment the count).
>    */
>   int pci_enable_device(struct pci_dev *dev)
>   {
> @@ -1717,8 +1726,8 @@ int pci_enable_device(struct pci_dev *dev)
>   EXPORT_SYMBOL(pci_enable_device);
>   
>   /*
> - * Managed PCI resources.  This manages device on/off, intx/msi/msix
> - * on/off and BAR regions.  pci_dev itself records msi/msix status, so
> + * Managed PCI resources.  This manages device on/off, INTx/MSI/MSI-X
> + * on/off and BAR regions.  pci_dev itself records MSI/MSI-X status, so
>    * there's no need to track it separately.  pci_devres is initialized
>    * when a device is enabled using managed PCI device enable interface.
>    */
> @@ -1836,7 +1845,8 @@ int __weak pcibios_add_device(struct pci_dev *dev)
>   }
>   
>   /**
> - * pcibios_release_device - provide arch specific hooks when releasing device dev
> + * pcibios_release_device - provide arch specific hooks when releasing
> + *			    device dev
>    * @dev: the PCI device being released
>    *
>    * Permits the platform to provide architecture specific functionality when
> @@ -1927,8 +1937,7 @@ EXPORT_SYMBOL(pci_disable_device);
>    * @dev: the PCIe device reset
>    * @state: Reset state to enter into
>    *
> - *
> - * Sets the PCIe reset state for the device. This is the default
> + * Set the PCIe reset state for the device. This is the default
>    * implementation. Architecture implementations can override this.
>    */
>   int __weak pcibios_set_pcie_reset_state(struct pci_dev *dev,
> @@ -1942,7 +1951,6 @@ int __weak pcibios_set_pcie_reset_state(struct pci_dev *dev,
>    * @dev: the PCIe device reset
>    * @state: Reset state to enter into
>    *
> - *
>    * Sets the PCI reset state for the device.
>    */
>   int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
> @@ -2339,7 +2347,8 @@ static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
>   }
>   
>   /**
> - * pci_prepare_to_sleep - prepare PCI device for system-wide transition into a sleep state
> + * pci_prepare_to_sleep - prepare PCI device for system-wide transition
> + *			  into a sleep state
>    * @dev: Device to handle.
>    *
>    * Choose the power state appropriate for the device depending on whether
> @@ -2367,7 +2376,8 @@ int pci_prepare_to_sleep(struct pci_dev *dev)
>   EXPORT_SYMBOL(pci_prepare_to_sleep);
>   
>   /**
> - * pci_back_from_sleep - turn PCI device on during system-wide transition into working state
> + * pci_back_from_sleep - turn PCI device on during system-wide transition
> + *			 into working state
>    * @dev: Device to handle.
>    *
>    * Disable device's system wake-up capability and put it into D0.
> @@ -3005,7 +3015,7 @@ static void pci_add_saved_cap(struct pci_dev *pci_dev,
>   
>   /**
>    * _pci_add_cap_save_buffer - allocate buffer for saving given
> - *                            capability registers
> + *			      capability registers
>    * @dev: the PCI device
>    * @cap: the capability to allocate the buffer for
>    * @extended: Standard or Extended capability ID
> @@ -3186,7 +3196,7 @@ static void pci_disable_acs_redir(struct pci_dev *dev)
>   }
>   
>   /**
> - * pci_std_enable_acs - enable ACS on devices using standard ACS capabilites
> + * pci_std_enable_acs - enable ACS on devices using standard ACS capabilities
>    * @dev: the PCI device
>    */
>   static void pci_std_enable_acs(struct pci_dev *dev)
> @@ -3609,13 +3619,14 @@ u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp)
>   EXPORT_SYMBOL_GPL(pci_common_swizzle);
>   
>   /**
> - *	pci_release_region - Release a PCI bar
> - *	@pdev: PCI device whose resources were previously reserved by pci_request_region
> - *	@bar: BAR to release
> + * pci_release_region - Release a PCI bar
> + * @pdev: PCI device whose resources were previously reserved by
> + *	  pci_request_region()
> + * @bar: BAR to release
>    *
> - *	Releases the PCI I/O and memory resources previously reserved by a
> - *	successful call to pci_request_region.  Call this function only
> - *	after all use of the PCI regions has ceased.
> + * Releases the PCI I/O and memory resources previously reserved by a
> + * successful call to pci_request_region().  Call this function only
> + * after all use of the PCI regions has ceased.
>    */
>   void pci_release_region(struct pci_dev *pdev, int bar)
>   {
> @@ -3637,23 +3648,23 @@ void pci_release_region(struct pci_dev *pdev, int bar)
>   EXPORT_SYMBOL(pci_release_region);
>   
>   /**
> - *	__pci_request_region - Reserved PCI I/O and memory resource
> - *	@pdev: PCI device whose resources are to be reserved
> - *	@bar: BAR to be reserved
> - *	@res_name: Name to be associated with resource.
> - *	@exclusive: whether the region access is exclusive or not
> + * __pci_request_region - Reserved PCI I/O and memory resource
> + * @pdev: PCI device whose resources are to be reserved
> + * @bar: BAR to be reserved
> + * @res_name: Name to be associated with resource.
> + * @exclusive: whether the region access is exclusive or not
>    *
> - *	Mark the PCI region associated with PCI device @pdev BR @bar as
> - *	being reserved by owner @res_name.  Do not access any
> - *	address inside the PCI regions unless this call returns
> - *	successfully.
> + * Mark the PCI region associated with PCI device @pdev BAR @bar as
> + * being reserved by owner @res_name.  Do not access any
> + * address inside the PCI regions unless this call returns
> + * successfully.
>    *
> - *	If @exclusive is set, then the region is marked so that userspace
> - *	is explicitly not allowed to map the resource via /dev/mem or
> - *	sysfs MMIO access.
> + * If @exclusive is set, then the region is marked so that userspace
> + * is explicitly not allowed to map the resource via /dev/mem or
> + * sysfs MMIO access.
>    *
> - *	Returns 0 on success, or %EBUSY on error.  A warning
> - *	message is also printed on failure.
> + * Returns 0 on success, or %EBUSY on error.  A warning
> + * message is also printed on failure.
>    */
>   static int __pci_request_region(struct pci_dev *pdev, int bar,
>   				const char *res_name, int exclusive)
> @@ -3687,18 +3698,18 @@ static int __pci_request_region(struct pci_dev *pdev, int bar,
>   }
>   
>   /**
> - *	pci_request_region - Reserve PCI I/O and memory resource
> - *	@pdev: PCI device whose resources are to be reserved
> - *	@bar: BAR to be reserved
> - *	@res_name: Name to be associated with resource
> + * pci_request_region - Reserve PCI I/O and memory resource
> + * @pdev: PCI device whose resources are to be reserved
> + * @bar: BAR to be reserved
> + * @res_name: Name to be associated with resource
>    *
> - *	Mark the PCI region associated with PCI device @pdev BAR @bar as
> - *	being reserved by owner @res_name.  Do not access any
> - *	address inside the PCI regions unless this call returns
> - *	successfully.
> + * Mark the PCI region associated with PCI device @pdev BAR @bar as
> + * being reserved by owner @res_name.  Do not access any
> + * address inside the PCI regions unless this call returns
> + * successfully.
>    *
> - *	Returns 0 on success, or %EBUSY on error.  A warning
> - *	message is also printed on failure.
> + * Returns 0 on success, or %EBUSY on error.  A warning
> + * message is also printed on failure.
>    */
>   int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
>   {
> @@ -3707,22 +3718,22 @@ int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
>   EXPORT_SYMBOL(pci_request_region);
>   
>   /**
> - *	pci_request_region_exclusive - Reserved PCI I/O and memory resource
> - *	@pdev: PCI device whose resources are to be reserved
> - *	@bar: BAR to be reserved
> - *	@res_name: Name to be associated with resource.
> + * pci_request_region_exclusive - Reserved PCI I/O and memory resource
> + * @pdev: PCI device whose resources are to be reserved
> + * @bar: BAR to be reserved
> + * @res_name: Name to be associated with resource.
>    *
> - *	Mark the PCI region associated with PCI device @pdev BR @bar as
> - *	being reserved by owner @res_name.  Do not access any
> - *	address inside the PCI regions unless this call returns
> - *	successfully.
> + * Mark the PCI region associated with PCI device @pdev BAR @bar as
> + * being reserved by owner @res_name.  Do not access any
> + * address inside the PCI regions unless this call returns
> + * successfully.
>    *
> - *	Returns 0 on success, or %EBUSY on error.  A warning
> - *	message is also printed on failure.
> + * Returns 0 on success, or %EBUSY on error.  A warning
> + * message is also printed on failure.
>    *
> - *	The key difference that _exclusive makes it that userspace is
> - *	explicitly not allowed to map the resource via /dev/mem or
> - *	sysfs.
> + * The key difference that _exclusive makes it that userspace is
> + * explicitly not allowed to map the resource via /dev/mem or
> + * sysfs.
>    */
>   int pci_request_region_exclusive(struct pci_dev *pdev, int bar,
>   				 const char *res_name)
> @@ -3791,12 +3802,13 @@ int pci_request_selected_regions_exclusive(struct pci_dev *pdev, int bars,
>   EXPORT_SYMBOL(pci_request_selected_regions_exclusive);
>   
>   /**
> - *	pci_release_regions - Release reserved PCI I/O and memory resources
> - *	@pdev: PCI device whose resources were previously reserved by pci_request_regions
> + * pci_release_regions - Release reserved PCI I/O and memory resources
> + * @pdev: PCI device whose resources were previously reserved by
> + *	  pci_request_regions()
>    *
> - *	Releases all PCI I/O and memory resources previously reserved by a
> - *	successful call to pci_request_regions.  Call this function only
> - *	after all use of the PCI regions has ceased.
> + * Releases all PCI I/O and memory resources previously reserved by a
> + * successful call to pci_request_regions().  Call this function only
> + * after all use of the PCI regions has ceased.
>    */
>   
>   void pci_release_regions(struct pci_dev *pdev)
> @@ -3806,17 +3818,17 @@ void pci_release_regions(struct pci_dev *pdev)
>   EXPORT_SYMBOL(pci_release_regions);
>   
>   /**
> - *	pci_request_regions - Reserved PCI I/O and memory resources
> - *	@pdev: PCI device whose resources are to be reserved
> - *	@res_name: Name to be associated with resource.
> + * pci_request_regions - Reserve PCI I/O and memory resources
> + * @pdev: PCI device whose resources are to be reserved
> + * @res_name: Name to be associated with resource.
>    *
> - *	Mark all PCI regions associated with PCI device @pdev as
> - *	being reserved by owner @res_name.  Do not access any
> - *	address inside the PCI regions unless this call returns
> - *	successfully.
> + * Mark all PCI regions associated with PCI device @pdev as
> + * being reserved by owner @res_name.  Do not access any
> + * address inside the PCI regions unless this call returns
> + * successfully.
>    *
> - *	Returns 0 on success, or %EBUSY on error.  A warning
> - *	message is also printed on failure.
> + * Returns 0 on success, or %EBUSY on error.  A warning
> + * message is also printed on failure.
>    */
>   int pci_request_regions(struct pci_dev *pdev, const char *res_name)
>   {
> @@ -3825,20 +3837,19 @@ int pci_request_regions(struct pci_dev *pdev, const char *res_name)
>   EXPORT_SYMBOL(pci_request_regions);
>   
>   /**
> - *	pci_request_regions_exclusive - Reserved PCI I/O and memory resources
> - *	@pdev: PCI device whose resources are to be reserved
> - *	@res_name: Name to be associated with resource.
> + * pci_request_regions_exclusive - Reserve PCI I/O and memory resources
> + * @pdev: PCI device whose resources are to be reserved
> + * @res_name: Name to be associated with resource.
>    *
> - *	Mark all PCI regions associated with PCI device @pdev as
> - *	being reserved by owner @res_name.  Do not access any
> - *	address inside the PCI regions unless this call returns
> - *	successfully.
> + * Mark all PCI regions associated with PCI device @pdev as being reserved
> + * by owner @res_name.  Do not access any address inside the PCI regions
> + * unless this call returns successfully.
>    *
> - *	pci_request_regions_exclusive() will mark the region so that
> - *	/dev/mem and the sysfs MMIO access will not be allowed.
> + * pci_request_regions_exclusive() will mark the region so that /dev/mem
> + * and the sysfs MMIO access will not be allowed.
>    *
> - *	Returns 0 on success, or %EBUSY on error.  A warning
> - *	message is also printed on failure.
> + * Returns 0 on success, or %EBUSY on error.  A warning message is also
> + * printed on failure.
>    */
>   int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
>   {
> @@ -3849,7 +3860,7 @@ EXPORT_SYMBOL(pci_request_regions_exclusive);
>   
>   /*
>    * Record the PCI IO range (expressed as CPU physical address + size).
> - * Return a negative value if an error has occured, zero otherwise
> + * Return a negative value if an error has occurred, zero otherwise
>    */
>   int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
>   			resource_size_t	size)
> @@ -3905,14 +3916,14 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
>   }
>   
>   /**
> - *	pci_remap_iospace - Remap the memory mapped I/O space
> - *	@res: Resource describing the I/O space
> - *	@phys_addr: physical address of range to be mapped
> + * pci_remap_iospace - Remap the memory mapped I/O space
> + * @res: Resource describing the I/O space
> + * @phys_addr: physical address of range to be mapped
>    *
> - *	Remap the memory mapped I/O space described by the @res
> - *	and the CPU physical address @phys_addr into virtual address space.
> - *	Only architectures that have memory mapped IO functions defined
> - *	(and the PCI_IOBASE value defined) should call this function.
> + * Remap the memory mapped I/O space described by the @res and the CPU
> + * physical address @phys_addr into virtual address space.  Only
> + * architectures that have memory mapped IO functions defined (and the
> + * PCI_IOBASE value defined) should call this function.
>    */
>   int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
>   {
> @@ -3928,8 +3939,10 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
>   	return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr,
>   				  pgprot_device(PAGE_KERNEL));
>   #else
> -	/* this architecture does not have memory mapped I/O space,
> -	   so this function should never be called */
> +	/*
> +	 * This architecture does not have memory mapped I/O space,
> +	 * so this function should never be called
> +	 */
>   	WARN_ONCE(1, "This architecture does not support memory mapped I/O\n");
>   	return -ENODEV;
>   #endif
> @@ -3937,12 +3950,12 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
>   EXPORT_SYMBOL(pci_remap_iospace);
>   
>   /**
> - *	pci_unmap_iospace - Unmap the memory mapped I/O space
> - *	@res: resource to be unmapped
> + * pci_unmap_iospace - Unmap the memory mapped I/O space
> + * @res: resource to be unmapped
>    *
> - *	Unmap the CPU virtual address @res from virtual address space.
> - *	Only architectures that have memory mapped IO functions defined
> - *	(and the PCI_IOBASE value defined) should call this function.
> + * Unmap the CPU virtual address @res from virtual address space.  Only
> + * architectures that have memory mapped IO functions defined (and the
> + * PCI_IOBASE value defined) should call this function.
>    */
>   void pci_unmap_iospace(struct resource *res)
>   {
> @@ -4288,7 +4301,7 @@ EXPORT_SYMBOL(pci_clear_mwi);
>    * @pdev: the PCI device to operate on
>    * @enable: boolean: whether to enable or disable PCI INTx
>    *
> - * Enables/disables PCI INTx for device dev
> + * Enables/disables PCI INTx for device @pdev
>    */
>   void pci_intx(struct pci_dev *pdev, int enable)
>   {
> @@ -4364,9 +4377,8 @@ static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask)
>    * pci_check_and_mask_intx - mask INTx on pending interrupt
>    * @dev: the PCI device to operate on
>    *
> - * Check if the device dev has its INTx line asserted, mask it and
> - * return true in that case. False is returned if no interrupt was
> - * pending.
> + * Check if the device dev has its INTx line asserted, mask it and return
> + * true in that case. False is returned if no interrupt was pending.
>    */
>   bool pci_check_and_mask_intx(struct pci_dev *dev)
>   {
> @@ -4378,9 +4390,9 @@ EXPORT_SYMBOL_GPL(pci_check_and_mask_intx);
>    * pci_check_and_unmask_intx - unmask INTx if no interrupt is pending
>    * @dev: the PCI device to operate on
>    *
> - * Check if the device dev has its INTx line asserted, unmask it if not
> - * and return true. False is returned and the mask remains active if
> - * there was still an interrupt pending.
> + * Check if the device dev has its INTx line asserted, unmask it if not and
> + * return true. False is returned and the mask remains active if there was
> + * still an interrupt pending.
>    */
>   bool pci_check_and_unmask_intx(struct pci_dev *dev)
>   {
> @@ -4389,7 +4401,7 @@ bool pci_check_and_unmask_intx(struct pci_dev *dev)
>   EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
>   
>   /**
> - * pci_wait_for_pending_transaction - waits for pending transaction
> + * pci_wait_for_pending_transaction - wait for pending transaction
>    * @dev: the PCI device to operate on
>    *
>    * Return 0 if transaction is pending 1 otherwise.
> @@ -4447,7 +4459,7 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
>   
>   /**
>    * pcie_has_flr - check if a device supports function level resets
> - * @dev:	device to check
> + * @dev: device to check
>    *
>    * Returns true if the device advertises support for PCIe function level
>    * resets.
> @@ -4466,7 +4478,7 @@ EXPORT_SYMBOL_GPL(pcie_has_flr);
>   
>   /**
>    * pcie_flr - initiate a PCIe function level reset
> - * @dev:	device to reset
> + * @dev: device to reset
>    *
>    * Initiate a function level reset on @dev.  The caller should ensure the
>    * device supports FLR before calling this function, e.g. by using the
> @@ -4810,6 +4822,7 @@ static void pci_dev_restore(struct pci_dev *dev)
>    *
>    * The device function is presumed to be unused and the caller is holding
>    * the device mutex lock when this function is called.
> + *
>    * Resetting the device will make the contents of PCI configuration space
>    * random, so any caller of this must be prepared to reinitialise the
>    * device including MSI, bus mastering, BARs, decoding IO and memory spaces,
> @@ -5373,8 +5386,8 @@ EXPORT_SYMBOL_GPL(pci_reset_bus);
>    * pcix_get_max_mmrbc - get PCI-X maximum designed memory read byte count
>    * @dev: PCI device to query
>    *
> - * Returns mmrbc: maximum designed memory read count in bytes
> - *    or appropriate error value.
> + * Returns mmrbc: maximum designed memory read count in bytes or
> + * appropriate error value.
>    */
>   int pcix_get_max_mmrbc(struct pci_dev *dev)
>   {
> @@ -5396,8 +5409,8 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
>    * pcix_get_mmrbc - get PCI-X maximum memory read byte count
>    * @dev: PCI device to query
>    *
> - * Returns mmrbc: maximum memory read count in bytes
> - *    or appropriate error value.
> + * Returns mmrbc: maximum memory read count in bytes or appropriate error
> + * value.
>    */
>   int pcix_get_mmrbc(struct pci_dev *dev)
>   {
> @@ -5421,7 +5434,7 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
>    * @mmrbc: maximum memory read count in bytes
>    *    valid values are 512, 1024, 2048, 4096
>    *
> - * If possible sets maximum memory read byte count, some bridges have erratas
> + * If possible sets maximum memory read byte count, some bridges have errata
>    * that prevent this.
>    */
>   int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
> @@ -5466,8 +5479,7 @@ EXPORT_SYMBOL(pcix_set_mmrbc);
>    * pcie_get_readrq - get PCI Express read request size
>    * @dev: PCI device to query
>    *
> - * Returns maximum memory read request in bytes
> - *    or appropriate error value.
> + * Returns maximum memory read request in bytes or appropriate error value.
>    */
>   int pcie_get_readrq(struct pci_dev *dev)
>   {
> @@ -5495,10 +5507,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>   		return -EINVAL;
>   
>   	/*
> -	 * If using the "performance" PCIe config, we clamp the
> -	 * read rq size to the max packet size to prevent the
> -	 * host bridge generating requests larger than we can
> -	 * cope with
> +	 * If using the "performance" PCIe config, we clamp the read rq
> +	 * size to the max packet size to keep the host bridge from
> +	 * generating requests larger than we can cope with.
>   	 */
>   	if (pcie_bus_config == PCIE_BUS_PERFORMANCE) {
>   		int mps = pcie_get_mps(dev);
> @@ -6144,6 +6155,7 @@ static int of_pci_bus_find_domain_nr(struct device *parent)
>   
>   	if (parent)
>   		domain = of_get_pci_domain_nr(parent->of_node);
> +
>   	/*
>   	 * Check DT domain and use_dt_domains values.
>   	 *

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] PCI: Fix comment typos
  2019-03-30 17:56   ` Mukesh Ojha
@ 2019-04-01 13:43     ` Bjorn Helgaas
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2019-04-01 13:43 UTC (permalink / raw)
  To: Mukesh Ojha; +Cc: linux-pci, linux-kernel

On Sat, Mar 30, 2019 at 11:26:23PM +0530, Mukesh Ojha wrote:
> On 3/25/2019 11:44 PM, helgaas@kernel.org wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > Fix spelling errors and format function comments consistently.  Changes
> > whitespace and comments only; no functional change intended.
> > 
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Added to the patch.  Reviewing this stuff is pretty tedious, so thanks a
lot for looking through this!

Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-04-01 13:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 18:14 [PATCH 0/4] PCI, CPER: Trivial cleanups helgaas
2019-03-25 18:14 ` [PATCH 1/4] PCI: Cleanup register definition width and whitespace helgaas
2019-03-25 18:14 ` [PATCH 2/4] PCI: Fix comment typos helgaas
2019-03-30 17:56   ` Mukesh Ojha
2019-04-01 13:43     ` Bjorn Helgaas
2019-03-25 18:14 ` [PATCH 3/4] CPER: Add UEFI spec references helgaas
2019-03-25 18:14 ` [PATCH 4/4] CPER: Remove unnecessary use of user-space types helgaas
2019-03-25 18:26   ` Bjorn Helgaas
2019-03-25 19:13     ` Joe Perches
2019-03-25 19:56     ` Greg Kroah-Hartman
2019-03-30 12:52     ` Masahiro Yamada
2019-03-27 13:56 ` [PATCH 0/4] PCI, CPER: Trivial cleanups Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).