diff mbox series

[1/6] PCI: Add INTx Mechanism Messages macros

Message ID 20240130-pme_msg-v1-1-d52b0add5c7c@nxp.com
State New
Headers
Series PCI: dwc: Add common pme_turn_off message by using outbound iATU |

Commit Message

Frank Li Jan. 31, 2024, 12:45 a.m. UTC 
  From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Add "Message Routing" and "INTx Mechanism Messages" macros to enable
a PCIe driver to send messages for INTx Interrupt Signaling.

The "Message Routing" is from Table 2-17, and the "INTx Mechanism
Messages" is from Table 2-18 on the PCI Express Base Specification,
Rev. 4.0 Version 1.0.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/pci.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Bjorn Helgaas Jan. 31, 2024, 3:37 p.m. UTC | #1 
On Tue, Jan 30, 2024 at 07:45:26PM -0500, Frank Li wrote:
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Add "Message Routing" and "INTx Mechanism Messages" macros to enable
> a PCIe driver to send messages for INTx Interrupt Signaling.
> 
> The "Message Routing" is from Table 2-17, and the "INTx Mechanism
> Messages" is from Table 2-18 on the PCI Express Base Specification,
> Rev. 4.0 Version 1.0.

Please cite a newer spec revision, e.g., PCIe r6.0 or r6.1.

Also, please cite section numbers instead of table numbers because the
table numbers are hard to find (you can't navigate to them from
"Contents") and they change a lot between spec revisions.  "INTx
Mechanism Messages" is Table 2-21 in r6.0, but it's in sec 2.2.8.1 in
both r4.0 and r6.0.

> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

With these updates:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pci.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 2336a8d1edab2..fe42f5d10b010 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -22,6 +22,24 @@
>   */
>  #define PCIE_PME_TO_L2_TIMEOUT_US	10000
>  
> +/* Message Routing (r[2:0]) */

Add citation to the comment: "PCIe r6.0, sec 2.2.8"

> +#define PCI_MSG_TYPE_R_RC	0

I think I would prefix all these with "PCIE" instead of "PCI", since
they are specific to PCIe and we already use "PCIE" for some of the
PCIe-specific timeouts.

> +#define PCI_MSG_TYPE_R_ADDR	1
> +#define PCI_MSG_TYPE_R_ID	2
> +#define PCI_MSG_TYPE_R_BC	3
> +#define PCI_MSG_TYPE_R_LOCAL	4
> +#define PCI_MSG_TYPE_R_GATHER	5
> +
> +/* INTx Mechanism Messages */

Add "PCIe r6.0, sec 2.2.8.1"

> +#define PCI_MSG_CODE_ASSERT_INTA	0x20
> +#define PCI_MSG_CODE_ASSERT_INTB	0x21
> +#define PCI_MSG_CODE_ASSERT_INTC	0x22
> +#define PCI_MSG_CODE_ASSERT_INTD	0x23
> +#define PCI_MSG_CODE_DEASSERT_INTA	0x24
> +#define PCI_MSG_CODE_DEASSERT_INTB	0x25
> +#define PCI_MSG_CODE_DEASSERT_INTC	0x26
> +#define PCI_MSG_CODE_DEASSERT_INTD	0x27
> +
>  extern const unsigned char pcie_link_speed[];
>  extern bool pci_early_dump;
>  
> 
> -- 
> 2.34.1
>
Bjorn Helgaas Feb. 1, 2024, 5:53 p.m. UTC | #2 
On Wed, Jan 31, 2024 at 09:37:48AM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 30, 2024 at 07:45:26PM -0500, Frank Li wrote:
> ...

> With these updates:
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Sorry, I should have mentioned that there were more comments below.

> > ---
> >  drivers/pci/pci.h | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index 2336a8d1edab2..fe42f5d10b010 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -22,6 +22,24 @@
> >   */
> >  #define PCIE_PME_TO_L2_TIMEOUT_US	10000
> >  
> > +/* Message Routing (r[2:0]) */
> 
> Add citation to the comment: "PCIe r6.0, sec 2.2.8"
> 
> > +#define PCI_MSG_TYPE_R_RC	0
> 
> I think I would prefix all these with "PCIE" instead of "PCI", since
> they are specific to PCIe and we already use "PCIE" for some of the
> PCIe-specific timeouts.
> 
> > +#define PCI_MSG_TYPE_R_ADDR	1
> > +#define PCI_MSG_TYPE_R_ID	2
> > +#define PCI_MSG_TYPE_R_BC	3
> > +#define PCI_MSG_TYPE_R_LOCAL	4
> > +#define PCI_MSG_TYPE_R_GATHER	5
> > +
> > +/* INTx Mechanism Messages */
> 
> Add "PCIe r6.0, sec 2.2.8.1"
> 
> > +#define PCI_MSG_CODE_ASSERT_INTA	0x20
> > +#define PCI_MSG_CODE_ASSERT_INTB	0x21
> > +#define PCI_MSG_CODE_ASSERT_INTC	0x22
> > +#define PCI_MSG_CODE_ASSERT_INTD	0x23
> > +#define PCI_MSG_CODE_DEASSERT_INTA	0x24
> > +#define PCI_MSG_CODE_DEASSERT_INTB	0x25
> > +#define PCI_MSG_CODE_DEASSERT_INTC	0x26
> > +#define PCI_MSG_CODE_DEASSERT_INTD	0x27
> > +
> >  extern const unsigned char pcie_link_speed[];
> >  extern bool pci_early_dump;
> >  
> > 
> > -- 
> > 2.34.1
> >
diff mbox series

Patch

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2336a8d1edab2..fe42f5d10b010 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -22,6 +22,24 @@ 
  */
 #define PCIE_PME_TO_L2_TIMEOUT_US	10000
 
+/* Message Routing (r[2:0]) */
+#define PCI_MSG_TYPE_R_RC	0
+#define PCI_MSG_TYPE_R_ADDR	1
+#define PCI_MSG_TYPE_R_ID	2
+#define PCI_MSG_TYPE_R_BC	3
+#define PCI_MSG_TYPE_R_LOCAL	4
+#define PCI_MSG_TYPE_R_GATHER	5
+
+/* INTx Mechanism Messages */
+#define PCI_MSG_CODE_ASSERT_INTA	0x20
+#define PCI_MSG_CODE_ASSERT_INTB	0x21
+#define PCI_MSG_CODE_ASSERT_INTC	0x22
+#define PCI_MSG_CODE_ASSERT_INTD	0x23
+#define PCI_MSG_CODE_DEASSERT_INTA	0x24
+#define PCI_MSG_CODE_DEASSERT_INTB	0x25
+#define PCI_MSG_CODE_DEASSERT_INTC	0x26
+#define PCI_MSG_CODE_DEASSERT_INTD	0x27
+
 extern const unsigned char pcie_link_speed[];
 extern bool pci_early_dump;