Re: [PATCH] PCI: dwc: Describe DWC PCIe fields with full masks and FIELD_PREP()

[Bjorn Helgaas]

On Wed, Sep 30, 2026 at 09:38:42AM +0800, Hans Zhang wrote:
> Several DWC PCIe register fields are described with a shift macro next
> to their mask, or with only a shift and the field width implied by the
> value written.  Document the field position and width in a single full
> mask and use FIELD_PREP() to encode it.
> 
> No functional change intended.

> @@ -278,15 +275,14 @@
>  #define RAS_DES_EVENT_COUNTER_CONTROL	0x8
>  #define EVENT_COUNTER_ALL_CLEAR		0x3
>  #define EVENT_COUNTER_ENABLE_ALL	0x7
> -#define EVENT_COUNTER_ENABLE_SHIFT	2
> -#define EVENT_COUNTER_EVENT_SEL_MASK	GENMASK(7, 0)
> -#define EVENT_COUNTER_EVENT_SEL_SHIFT	16
> +#define EVENT_COUNTER_ENABLE_MASK	GENMASK(4, 2)
> +#define EVENT_COUNTER_EVENT_SEL_MASK	GENMASK(23, 16)
> +#define EVENT_COUNTER_GROUP_SEL_MASK	GENMASK(27, 24)
>  #define EVENT_COUNTER_EVENT_Tx_L0S	0x2
>  #define EVENT_COUNTER_EVENT_Rx_L0S	0x3
>  #define EVENT_COUNTER_EVENT_L1		0x5
>  #define EVENT_COUNTER_EVENT_L1_1	0x7
>  #define EVENT_COUNTER_EVENT_L1_2	0x8
> -#define EVENT_COUNTER_GROUP_SEL_SHIFT	24
>  #define EVENT_COUNTER_GROUP_5		0x5
>  
>  #define RAS_DES_EVENT_COUNTER_DATA	0xc
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 8c610212a180..285a38178da8 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -622,10 +622,10 @@ static inline u32 event_counter_prog(struct tegra_pcie_dw *pcie, u32 event)
>  
>  	val = dw_pcie_readl_dbi(&pcie->pci, pcie->ras_des_cap +
>  				RAS_DES_EVENT_COUNTER_CONTROL);
> -	val &= ~(EVENT_COUNTER_EVENT_SEL_MASK << EVENT_COUNTER_EVENT_SEL_SHIFT);
> -	val |= EVENT_COUNTER_GROUP_5 << EVENT_COUNTER_GROUP_SEL_SHIFT;
> -	val |= event << EVENT_COUNTER_EVENT_SEL_SHIFT;
> -	val |= EVENT_COUNTER_ENABLE_ALL << EVENT_COUNTER_ENABLE_SHIFT;
> +	val &= ~EVENT_COUNTER_EVENT_SEL_MASK;
> +	val |= FIELD_PREP(EVENT_COUNTER_GROUP_SEL_MASK, EVENT_COUNTER_GROUP_5);

Should EVENT_COUNTER_GROUP_SEL_MASK be cleared out before ORing
EVENT_COUNTER_GROUP_5 into it?

I see the previous code didn't clear it either, but I'm wondering if
this is a latent bug that should be fixed, probably with a separate
patch if so.

> +	val |= FIELD_PREP(EVENT_COUNTER_EVENT_SEL_MASK, event);
> +	val |= FIELD_PREP(EVENT_COUNTER_ENABLE_MASK, EVENT_COUNTER_ENABLE_ALL);
>  	dw_pcie_writel_dbi(&pcie->pci, pcie->ras_des_cap +
>  			   RAS_DES_EVENT_COUNTER_CONTROL, val);
>  	val = dw_pcie_readl_dbi(&pcie->pci, pcie->ras_des_cap +