[2/6] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros

Message ID	20220222163158.1666-3-pali@kernel.org
State	New
Headers	show Return-Path: <linux-pci-owner@vger.kernel.org> From: =?utf-8?q?Pali_Roh=C3=A1r?= <pali@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>, Andrew Lunn <andrew@lunn.ch>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, =?utf-8?q?Krzysztof_Wilczy?= =?utf-8?q?=C5=84ski?= <kw@linux.com>, =?utf-8?q?Marek_Beh=C3=BAn?= <kabel@kernel.org>, Russell King <rmk+kernel@armlinux.org.uk>, Gregory Clement <gregory.clement@bootlin.com> Cc: linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/6] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros Date: Tue, 22 Feb 2022 17:31:54 +0100 Message-Id: <20220222163158.1666-3-pali@kernel.org> In-Reply-To: <20220222163158.1666-1-pali@kernel.org> References: <20220222163158.1666-1-pali@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	PCI: mvebu: Slot support \| expand [0/6] PCI: mvebu: Slot support [1/6] PCI: Add PCI_EXP_SLTCTL_ASPL_DISABLE macro [2/6] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros [3/6] dt-bindings: Add 'slot-power-limit-milliwatt' PCIe port property [4/6] PCI: Add function for parsing 'slot-power-limit-milliwatt' DT property [5/6] PCI: mvebu: Add support for sending Set_Slot_Power_Limit message [6/6] ARM: dts: turris-omnia: Set PCIe slot-power-limit-milliwatt properties

Pali Rohár Feb. 22, 2022, 4:31 p.m. UTC

These macros allows to easily compose and extract Slot Power Limit and
Physical Slot Number values from Slot Capability Register.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 include/uapi/linux/pci_regs.h | 3 +++
 1 file changed, 3 insertions(+)

Bjorn Helgaas Feb. 24, 2022, 8:28 p.m. UTC | #1

On Tue, Feb 22, 2022 at 05:31:54PM +0100, Pali Rohár wrote:
> These macros allows to easily compose and extract Slot Power Limit and
> Physical Slot Number values from Slot Capability Register.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  include/uapi/linux/pci_regs.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 108f8523fa04..3fc9a4cac630 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -591,10 +591,13 @@
>  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
>  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
>  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */

Is there a way to use FIELD_PREP() and FIELD_GET() instead?  It seems
like that's what the cool kids are doing now.

At first I didn't really like them, but they do remove the need for
adding _SHIFT macros that have to be manually related to the other.

>  #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
> +#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
>  #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
>  #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
>  #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> +#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
>  #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
>  #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
>  #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
> -- 
> 2.20.1
>

Pali Rohár Feb. 25, 2022, 12:24 p.m. UTC | #2

On Thursday 24 February 2022 14:28:43 Bjorn Helgaas wrote:
> On Tue, Feb 22, 2022 at 05:31:54PM +0100, Pali Rohár wrote:
> > These macros allows to easily compose and extract Slot Power Limit and
> > Physical Slot Number values from Slot Capability Register.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >  include/uapi/linux/pci_regs.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index 108f8523fa04..3fc9a4cac630 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -591,10 +591,13 @@
> >  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
> >  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
> >  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> > +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
> 
> Is there a way to use FIELD_PREP() and FIELD_GET() instead?  It seems
> like that's what the cool kids are doing now.

This is possible too.

I have proposed a patch with _SHIFT macros as this is the way how are
other macros in this file defined and used.

> At first I didn't really like them, but they do remove the need for
> adding _SHIFT macros that have to be manually related to the other.
> 
> >  #define  PCI_EXP_SLTCAP_SPLS	0x00018000 /* Slot Power Limit Scale */
> > +#define  PCI_EXP_SLTCAP_SPLS_SHIFT	15 /* Slot Power Limit Scale shift */
> >  #define  PCI_EXP_SLTCAP_EIP	0x00020000 /* Electromechanical Interlock Present */
> >  #define  PCI_EXP_SLTCAP_NCCS	0x00040000 /* No Command Completed Support */
> >  #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> > +#define  PCI_EXP_SLTCAP_PSN_SHIFT	19 /* Physical Slot Number shift */
> >  #define PCI_EXP_SLTCTL		0x18	/* Slot Control */
> >  #define  PCI_EXP_SLTCTL_ABPE	0x0001	/* Attention Button Pressed Enable */
> >  #define  PCI_EXP_SLTCTL_PFDE	0x0002	/* Power Fault Detected Enable */
> > -- 
> > 2.20.1
> >

Bjorn Helgaas Feb. 25, 2022, 3:37 p.m. UTC | #3

On Fri, Feb 25, 2022 at 01:24:51PM +0100, Pali Rohár wrote:
> On Thursday 24 February 2022 14:28:43 Bjorn Helgaas wrote:
> > On Tue, Feb 22, 2022 at 05:31:54PM +0100, Pali Rohár wrote:
> > > These macros allows to easily compose and extract Slot Power Limit and
> > > Physical Slot Number values from Slot Capability Register.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > ---
> > >  include/uapi/linux/pci_regs.h | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > > index 108f8523fa04..3fc9a4cac630 100644
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -591,10 +591,13 @@
> > >  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
> > >  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
> > >  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> > > +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */
> > 
> > Is there a way to use FIELD_PREP() and FIELD_GET() instead?  It seems
> > like that's what the cool kids are doing now.
> 
> This is possible too.
> 
> I have proposed a patch with _SHIFT macros as this is the way how are
> other macros in this file defined and used.

Yes, it's a mix.  For some recent additions, I've resisted adding the
_SHIFT macros on the theory that they clutter the file, they never
change, and the main point of the #defines is readability and so
grep/tags/etc can find things.

There are a *few* users of FIELD_PREP() and FIELD_GET():

  git grep -E "FIELD_(GET|PREP)\(PCI_EXP"

and I'm inclined to go that direction in the future.  What do you
think?

Bjorn

Marek Behún Feb. 25, 2022, 5:22 p.m. UTC | #4

On Fri, 25 Feb 2022 09:37:56 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Fri, Feb 25, 2022 at 01:24:51PM +0100, Pali Rohár wrote:
> > On Thursday 24 February 2022 14:28:43 Bjorn Helgaas wrote:  
> > > On Tue, Feb 22, 2022 at 05:31:54PM +0100, Pali Rohár wrote:  
> > > > These macros allows to easily compose and extract Slot Power Limit and
> > > > Physical Slot Number values from Slot Capability Register.
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > > ---
> > > >  include/uapi/linux/pci_regs.h | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > > > index 108f8523fa04..3fc9a4cac630 100644
> > > > --- a/include/uapi/linux/pci_regs.h
> > > > +++ b/include/uapi/linux/pci_regs.h
> > > > @@ -591,10 +591,13 @@
> > > >  #define  PCI_EXP_SLTCAP_HPS	0x00000020 /* Hot-Plug Surprise */
> > > >  #define  PCI_EXP_SLTCAP_HPC	0x00000040 /* Hot-Plug Capable */
> > > >  #define  PCI_EXP_SLTCAP_SPLV	0x00007f80 /* Slot Power Limit Value */
> > > > +#define  PCI_EXP_SLTCAP_SPLV_SHIFT	7  /* Slot Power Limit Value shift */  
> > > 
> > > Is there a way to use FIELD_PREP() and FIELD_GET() instead?  It seems
> > > like that's what the cool kids are doing now.  
> > 
> > This is possible too.
> > 
> > I have proposed a patch with _SHIFT macros as this is the way how are
> > other macros in this file defined and used.  
> 
> Yes, it's a mix.  For some recent additions, I've resisted adding the
> _SHIFT macros on the theory that they clutter the file, they never
> change, and the main point of the #defines is readability and so
> grep/tags/etc can find things.
> 
> There are a *few* users of FIELD_PREP() and FIELD_GET():
> 
>   git grep -E "FIELD_(GET|PREP)\(PCI_EXP"
> 
> and I'm inclined to go that direction in the future.  What do you
> think?

I am also pro that direction.

Would you also like to convert current usages in the .c driver files?

We can't remove the existing macros since they are in UAPI, but we can
convert drivers so that they don't use _SHIFT macros.

Marek

Bjorn Helgaas Feb. 25, 2022, 5:51 p.m. UTC | #5

On Fri, Feb 25, 2022 at 06:22:16PM +0100, Marek Behún wrote:
> On Fri, 25 Feb 2022 09:37:56 -0600
> Bjorn Helgaas <helgaas@kernel.org> wrote:

> > Yes, it's a mix.  For some recent additions, I've resisted adding the
> > _SHIFT macros on the theory that they clutter the file, they never
> > change, and the main point of the #defines is readability and so
> > grep/tags/etc can find things.
> > 
> > There are a *few* users of FIELD_PREP() and FIELD_GET():
> > 
> >   git grep -E "FIELD_(GET|PREP)\(PCI_EXP"
> > 
> > and I'm inclined to go that direction in the future.  What do you
> > think?
> 
> I am also pro that direction.
> 
> Would you also like to convert current usages in the .c driver files?
> 
> We can't remove the existing macros since they are in UAPI, but we can
> convert drivers so that they don't use _SHIFT macros.

Certainly not a high priority, but it actually looks like there aren't
*that* many uses in drivers/pci/, so I'd be OK with converting them.

Bjorn

[2/6] PCI: Add PCI_EXP_SLTCAP_*_SHIFT macros

Commit Message

Comments

Patch