diff mbox series

[QUERY] Is there a reason pci_quirk_enable_intel_rp_mpc_acs() uses pci_write_config_word()

Message ID 67958fbe-fa61-4ea4-8040-c6b8d0313d57@oracle.com
State New
Headers show
Series [QUERY] Is there a reason pci_quirk_enable_intel_rp_mpc_acs() uses pci_write_config_word() | expand

Commit Message

ALOK TIWARI Oct. 14, 2025, 5:27 a.m. UTC 
Hi,

function pci_quirk_enable_intel_rp_mpc_acs() in
drivers/pci/quirks.c, I noticed that the code reads a 32-bit value from
INTEL_MPC_REG using pci_read_config_dword(), but writes it back using
pci_write_config_word().

The relevant lines are:

     pci_read_config_dword(dev, INTEL_MPC_REG, &mpc);
     if (!(mpc & INTEL_MPC_REG_IRBNCE)) {
         pci_info(dev, "Enabling MPC IRBNCE\n");
         mpc |= INTEL_MPC_REG_IRBNCE;
         pci_write_config_word(dev, INTEL_MPC_REG, mpc);
     }

Given that:
/* Miscellaneous Port Configuration register */
#define INTEL_MPC_REG 0xd8
/* MPC: Invalid Receive Bus Number Check Enable */
#define INTEL_MPC_REG_IRBNCE (1 << 26)

the IRBNCE bit is in the upper 16 bits of this 32-bit register.
It seems that using pci_write_config_word() would not actually update 
that bit.

is there a specific hardware reason for using a 16-bit write?

Thanks,
Alok

---

Comments

Alex Williamson Oct. 14, 2025, 7:56 p.m. UTC | #1 
On Tue, 14 Oct 2025 10:57:39 +0530
ALOK TIWARI <alok.a.tiwari@oracle.com> wrote:

> Hi,
> 
> function pci_quirk_enable_intel_rp_mpc_acs() in
> drivers/pci/quirks.c, I noticed that the code reads a 32-bit value from
> INTEL_MPC_REG using pci_read_config_dword(), but writes it back using
> pci_write_config_word().
> 
> The relevant lines are:
> 
>      pci_read_config_dword(dev, INTEL_MPC_REG, &mpc);
>      if (!(mpc & INTEL_MPC_REG_IRBNCE)) {
>          pci_info(dev, "Enabling MPC IRBNCE\n");
>          mpc |= INTEL_MPC_REG_IRBNCE;
>          pci_write_config_word(dev, INTEL_MPC_REG, mpc);
>      }
> 
> Given that:
> /* Miscellaneous Port Configuration register */
> #define INTEL_MPC_REG 0xd8
> /* MPC: Invalid Receive Bus Number Check Enable */
> #define INTEL_MPC_REG_IRBNCE (1 << 26)
> 
> the IRBNCE bit is in the upper 16 bits of this 32-bit register.
> It seems that using pci_write_config_word() would not actually update 
> that bit.
> 
> is there a specific hardware reason for using a 16-bit write?

It looks like a typo to me.  Please post a formal fix.  Thanks,

Alex

> ---
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 214ed060ca1b..1bd6e70058b5 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5312,7 +5312,7 @@ static void 
> pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev)
>          if (!(mpc & INTEL_MPC_REG_IRBNCE)) {
>                  pci_info(dev, "Enabling MPC IRBNCE\n");
>                  mpc |= INTEL_MPC_REG_IRBNCE;
> -               pci_write_config_word(dev, INTEL_MPC_REG, mpc);
> +               pci_write_config_dword(dev, INTEL_MPC_REG, mpc);
>          }
>   }
>
diff mbox series

Patch

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 214ed060ca1b..1bd6e70058b5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5312,7 +5312,7 @@  static void 
pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev)
         if (!(mpc & INTEL_MPC_REG_IRBNCE)) {
                 pci_info(dev, "Enabling MPC IRBNCE\n");
                 mpc |= INTEL_MPC_REG_IRBNCE;
-               pci_write_config_word(dev, INTEL_MPC_REG, mpc);
+               pci_write_config_dword(dev, INTEL_MPC_REG, mpc);
         }
  }