diff mbox series

[v3,10/13] pci: switch pci_conf_write8 to use pci_sbdf_t

Message ID 20190607092232.83179-11-roger.pau@citrix.com (mailing list archive)
State Superseded
Headers show
Series pci: expand usage of pci_sbdf_t | expand

Commit Message

Roger Pau Monné June 7, 2019, 9:22 a.m. UTC
This reduces the number of parameters of the function to two, and
simplifies some of the calling sites.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/cpu/amd.c       |  2 +-
 xen/arch/x86/x86_64/pci.c    | 21 ++++++++-------------
 xen/drivers/acpi/reboot.c    |  6 +++---
 xen/drivers/char/ehci-dbgp.c |  8 +++++---
 xen/drivers/vpci/vpci.c      |  9 ++++-----
 xen/include/xen/pci.h        |  4 +---
 6 files changed, 22 insertions(+), 28 deletions(-)

Comments

Jan Beulich June 17, 2019, 9:57 a.m. UTC | #1
>>> On 07.06.19 at 11:22, <roger.pau@citrix.com> wrote:
> --- a/xen/drivers/acpi/reboot.c
> +++ b/xen/drivers/acpi/reboot.c
> @@ -23,9 +23,9 @@ void acpi_reboot(void)
>  	case ACPI_ADR_SPACE_PCI_CONFIG:
>  		printk("Resetting with ACPI PCI RESET_REG.\n");
>  		/* Write the value that resets us. */
> -		pci_conf_write8(0, 0,
> -				(rr->address >> 32) & 31,
> -				(rr->address >> 16) & 7,
> +		pci_conf_write8(PCI_SBDF(0, 0,
> +					 (rr->address >> 32) & 31,
> +					 (rr->address >> 16) & 7),

Isn't it the case that the AND-ing by constants is now no longer
needed?

> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -263,8 +263,8 @@ static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
>           */
>          if ( reg & 1 )
>          {
> -            pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg,
> -                            data);
> +
> +            pci_conf_write8(sbdf, reg, data);
>              pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 1,
>                               data >> 8);

Please don't insert a blank line like this.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 37f60c0862..489d45fd25 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -426,7 +426,7 @@  static void disable_c1_ramping(void)
 		if (pmm7 == 0xFF)
 			break;
 		pmm7 &= 0xFC; /* clear pmm7[1:0] */
-		pci_conf_write8(0, 0, 0x18 + node, 0x3, 0x87, pmm7);
+		pci_conf_write8(PCI_SBDF(0, 0, 0x18 + node, 3), 0x87, pmm7);
 		printk ("AMD: Disabling C1 Clock Ramping Node #%x\n", node);
 	}
 }
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index b8b82a6fe7..eaa67b04f2 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -50,23 +50,18 @@  uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
     return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), 0, 4);
 }
 
-#undef PCI_CONF_ADDRESS
-#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
-    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
-
-void pci_conf_write8(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg, uint8_t data)
+void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data)
 {
-    if ( seg || reg > 255 )
-        pci_mmcfg_write(seg, bus, PCI_DEVFN(dev, func), reg, 1, data);
+    if ( sbdf.seg || reg > 255 )
+        pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, data);
     else
-    {
-        BUG_ON((bus > 255) || (dev > 31) || (func > 7));
-        pci_conf_write(PCI_CONF_ADDRESS(bus, dev, func, reg), reg & 3, 1, data);
-    }
+        pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1, data);
 }
 
+#undef PCI_CONF_ADDRESS
+#define PCI_CONF_ADDRESS(bus, dev, func, reg) \
+    (0x80000000 | (bus << 16) | (dev << 11) | (func << 8) | (reg & ~3))
+
 void pci_conf_write16(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint16_t data)
diff --git a/xen/drivers/acpi/reboot.c b/xen/drivers/acpi/reboot.c
index 72d06fd8e5..04dac4fe7d 100644
--- a/xen/drivers/acpi/reboot.c
+++ b/xen/drivers/acpi/reboot.c
@@ -23,9 +23,9 @@  void acpi_reboot(void)
 	case ACPI_ADR_SPACE_PCI_CONFIG:
 		printk("Resetting with ACPI PCI RESET_REG.\n");
 		/* Write the value that resets us. */
-		pci_conf_write8(0, 0,
-				(rr->address >> 32) & 31,
-				(rr->address >> 16) & 7,
+		pci_conf_write8(PCI_SBDF(0, 0,
+					 (rr->address >> 32) & 31,
+					 (rr->address >> 16) & 7),
 				(rr->address & 255),
 				reset_value);
 		break;
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 9b9025fb33..010fc3c5bc 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1048,7 +1048,8 @@  static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
     if ( (cap & 0xff) == 1 && (cap & EHCI_USBLEGSUP_BIOS) )
     {
         dbgp_printk("dbgp: BIOS handoff\n");
-        pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func, offset + 3, 1);
+        pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                        offset + 3, 1);
     }
 
     /* if boot firmware now owns EHCI, spin till it hands it over. */
@@ -1066,11 +1067,12 @@  static void ehci_dbgp_bios_handoff(struct ehci_dbgp *dbgp, u32 hcc_params)
         /* well, possibly buggy BIOS... try to shut it down,
          * and hope nothing goes too wrong */
         dbgp_printk("dbgp: BIOS handoff failed: %08x\n", cap);
-        pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func, offset + 2, 0);
+        pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
+                        offset + 2, 0);
     }
 
     /* just in case, always disable EHCI SMIs */
-    pci_conf_write8(0, dbgp->bus, dbgp->slot, dbgp->func,
+    pci_conf_write8(PCI_SBDF(0, dbgp->bus, dbgp->slot, dbgp->func),
                     offset + EHCI_USBLEGCTLSTS, 0);
 }
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 2106255863..53c5821c20 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -263,8 +263,8 @@  static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
          */
         if ( reg & 1 )
         {
-            pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg,
-                            data);
+
+            pci_conf_write8(sbdf, reg, data);
             pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 1,
                              data >> 8);
         }
@@ -272,8 +272,7 @@  static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
         {
             pci_conf_write16(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg,
                              data);
-            pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg + 2,
-                            data >> 16);
+            pci_conf_write8(sbdf, reg + 2, data >> 16);
         }
         break;
 
@@ -282,7 +281,7 @@  static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
         break;
 
     case 1:
-        pci_conf_write8(sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn, reg, data);
+        pci_conf_write8(sbdf, reg, data);
         break;
 
     default:
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 2a5705560e..3faa2a22d3 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -175,9 +175,7 @@  void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg);
 uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg);
 uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg);
-void pci_conf_write8(
-    unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
-    unsigned int reg, uint8_t data);
+void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data);
 void pci_conf_write16(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg, uint16_t data);