| Message ID | 20240916163748.2223815-1-thippesw@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for CPM5 controller 1 | expand |
On Mon, Sep 16, 2024 at 10:07:48PM +0530, Thippeswamy Havalige wrote: For some reason, this patch is not threaded and not part of the series: [PATCH v2 0/2] Add support for CPM5 controller 1 > This patch adds support for the Xilinx Versal CPM5 Root Port Controller 1. s/patch/commit Once this patch gets merged, it will become a commit. > The key difference between Controller 0 and Controller 1 lies in the > platform-specific error interrupt bits, which are located at different > register offsets. > > To handle these differences, a variant structure is introduced that holds > the following platform-specific details: > The variant structure is already present in the driver. Hence not introduced in *this* patch. > - Interrupt status register offset (ir_status) > - Interrupt enable register offset (ir_enable) > - Miscellaneous interrupt values (ir_misc_value) > > The driver differentiates between Controller 0 and Controller 1 using the > compatible string in the device tree. This ensures that the appropriate > register offsets are used for each controller, allowing for correct > handling of platform-specific interrupts and initialization. > > Signed-off-by: Thippeswamy Havalige <thippesw@amd.com> > --- > changes in v2: > -------------- > 1. Introduced new constants for Controller 1. > 2. Extended the xilinx_cpm_variant structure to support > a. ir_status, > b. ir_enable, and > c. ir_misc_value for different controllers. > 3. Updated IRQ handling and initialization to use the variant structure. > 4. Added a new device tree match entry for Controller 1. > --- > drivers/pci/controller/pcie-xilinx-cpm.c | 47 ++++++++++++++++++------ > 1 file changed, 36 insertions(+), 11 deletions(-) > > diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c > index a0f5e1d67b04..b783fff27c9d 100644 > --- a/drivers/pci/controller/pcie-xilinx-cpm.c > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c > @@ -30,11 +30,14 @@ > #define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C > #define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340 > #define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348 > -#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1) > +#define XILINX_CPM_PCIE0_MISC_IR_LOCAL BIT(1) > +#define XILINX_CPM_PCIE1_MISC_IR_LOCAL BIT(2) > > -#define XILINX_CPM_PCIE_IR_STATUS 0x000002A0 > -#define XILINX_CPM_PCIE_IR_ENABLE 0x000002A8 > -#define XILINX_CPM_PCIE_IR_LOCAL BIT(0) > +#define XILINX_CPM_PCIE0_IR_STATUS 0x000002A0 > +#define XILINX_CPM_PCIE1_IR_STATUS 0x000002B4 > +#define XILINX_CPM_PCIE0_IR_ENABLE 0x000002A8 > +#define XILINX_CPM_PCIE1_IR_ENABLE 0x000002BC > +#define XILINX_CPM_PCIE_IR_LOCAL BIT(0) > > #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x) > > @@ -80,6 +83,7 @@ > enum xilinx_cpm_version { > CPM, > CPM5, > + CPM5_HOST1, > }; > > /** > @@ -88,6 +92,9 @@ enum xilinx_cpm_version { > */ > struct xilinx_cpm_variant { > enum xilinx_cpm_version version; > + u32 ir_status; > + u32 ir_enable; > + u32 ir_misc_value; Kdoc comments missing for these members. > }; > > /** > @@ -269,6 +276,7 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) > { > struct xilinx_cpm_pcie *port = irq_desc_get_handler_data(desc); > struct irq_chip *chip = irq_desc_get_chip(desc); > + const struct xilinx_cpm_variant *variant = port->variant; > unsigned long val; > int i; > > @@ -279,11 +287,11 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) > generic_handle_domain_irq(port->cpm_domain, i); > pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR); > > - if (port->variant->version == CPM5) { > - val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS); > + if (variant->ir_status) { > + val = readl_relaxed(port->cpm_base + variant->ir_status); > if (val) > writel_relaxed(val, port->cpm_base + > - XILINX_CPM_PCIE_IR_STATUS); > + variant->ir_status); > } > > /* > @@ -465,6 +473,8 @@ static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie *port) > */ > static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) > { > + const struct xilinx_cpm_variant *variant = port->variant; > + > if (cpm_pcie_link_up(port)) > dev_info(port->dev, "PCIe Link is UP\n"); > else > @@ -483,15 +493,15 @@ static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) > * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to > * CPM SLCR block. > */ > - writel(XILINX_CPM_PCIE_MISC_IR_LOCAL, > + writel(variant->ir_misc_value, > port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE); > > - if (port->variant->version == CPM5) { > + if (variant->ir_enable) { > writel(XILINX_CPM_PCIE_IR_LOCAL, > - port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE); > + port->cpm_base + variant->ir_enable); > } > > - /* Enable the Bridge enable bit */ > + /* Set Bridge enable bit */ This changes doesn't belong to this patch. > pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) | > XILINX_CPM_PCIE_REG_RPSC_BEN, > XILINX_CPM_PCIE_REG_RPSC); > @@ -609,10 +619,21 @@ static int xilinx_cpm_pcie_probe(struct platform_device *pdev) > > static const struct xilinx_cpm_variant cpm_host = { > .version = CPM, > + .ir_misc_value = XILINX_CPM_PCIE0_MISC_IR_LOCAL, > }; > > static const struct xilinx_cpm_variant cpm5_host = { > .version = CPM5, > + .ir_misc_value = XILINX_CPM_PCIE0_MISC_IR_LOCAL, > + .ir_status = XILINX_CPM_PCIE0_IR_STATUS, > + .ir_enable = XILINX_CPM_PCIE0_IR_ENABLE, > +}; > + > +static const struct xilinx_cpm_variant cpm5_host1 = { > + .version = CPM5_HOST1, > + .ir_misc_value = XILINX_CPM_PCIE1_MISC_IR_LOCAL, > + .ir_status = XILINX_CPM_PCIE1_IR_STATUS, > + .ir_enable = XILINX_CPM_PCIE1_IR_ENABLE, > }; > > static const struct of_device_id xilinx_cpm_pcie_of_match[] = { > @@ -624,6 +645,10 @@ static const struct of_device_id xilinx_cpm_pcie_of_match[] = { > .compatible = "xlnx,versal-cpm5-host", > .data = &cpm5_host, > }, > + { > + .compatible = "xlnx,versal-cpm5-host1-1", This doesn't look like a valid compatible name. Please use the compatible as per the IP version. - Mani
Hi Manivannan Sadhasivam, > -----Original Message----- > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Sent: Tuesday, September 17, 2024 8:40 PM > To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com> > Cc: robh@kernel.org; linux-pci@vger.kernel.org; bhelgaas@google.com; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; devicetree@vger.kernel.org; Gogada, Bharat Kumar > <bharat.kumar.gogada@amd.com>; Simek, Michal <michal.simek@amd.com>; > lpieralisi@kernel.org; kw@linux.com > Subject: Re: [PATCH v2 2/2] PCI: xilinx-cpm: Add support for Versal CPM5 Root > Port controller 1 > > On Mon, Sep 16, 2024 at 10:07:48PM +0530, Thippeswamy Havalige wrote: > > For some reason, this patch is not threaded and not part of the series: > [PATCH v2 0/2] Add support for CPM5 controller 1 > > > This patch adds support for the Xilinx Versal CPM5 Root Port Controller 1. > > s/patch/commit > > Once this patch gets merged, it will become a commit. Thanks for review, will update this in next patch. > > > The key difference between Controller 0 and Controller 1 lies in the > > platform-specific error interrupt bits, which are located at different > > register offsets. > > > > To handle these differences, a variant structure is introduced that > > holds the following platform-specific details: > > > > The variant structure is already present in the driver. Hence not introduced in *this* > patch. Thanks for review, will update this in next patch. > > - Interrupt status register offset (ir_status) > > - Interrupt enable register offset (ir_enable) > > - Miscellaneous interrupt values (ir_misc_value) > > > > The driver differentiates between Controller 0 and Controller 1 using > > the compatible string in the device tree. This ensures that the > > appropriate register offsets are used for each controller, allowing > > for correct handling of platform-specific interrupts and initialization. > > > > Signed-off-by: Thippeswamy Havalige <thippesw@amd.com> > > --- > > changes in v2: > > -------------- > > 1. Introduced new constants for Controller 1. > > 2. Extended the xilinx_cpm_variant structure to support > > a. ir_status, > > b. ir_enable, and > > c. ir_misc_value for different controllers. > > 3. Updated IRQ handling and initialization to use the variant structure. > > 4. Added a new device tree match entry for Controller 1. > > --- > > drivers/pci/controller/pcie-xilinx-cpm.c | 47 > > ++++++++++++++++++------ > > 1 file changed, 36 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c > > b/drivers/pci/controller/pcie-xilinx-cpm.c > > index a0f5e1d67b04..b783fff27c9d 100644 > > --- a/drivers/pci/controller/pcie-xilinx-cpm.c > > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c > > @@ -30,11 +30,14 @@ > > #define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C > > #define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340 > > #define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348 > > -#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1) > > +#define XILINX_CPM_PCIE0_MISC_IR_LOCAL BIT(1) > > +#define XILINX_CPM_PCIE1_MISC_IR_LOCAL BIT(2) > > > > -#define XILINX_CPM_PCIE_IR_STATUS 0x000002A0 > > -#define XILINX_CPM_PCIE_IR_ENABLE 0x000002A8 > > -#define XILINX_CPM_PCIE_IR_LOCAL BIT(0) > > +#define XILINX_CPM_PCIE0_IR_STATUS 0x000002A0 > > +#define XILINX_CPM_PCIE1_IR_STATUS 0x000002B4 > > +#define XILINX_CPM_PCIE0_IR_ENABLE 0x000002A8 > > +#define XILINX_CPM_PCIE1_IR_ENABLE 0x000002BC > > +#define XILINX_CPM_PCIE_IR_LOCAL BIT(0) > > > > #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x) > > > > @@ -80,6 +83,7 @@ > > enum xilinx_cpm_version { > > CPM, > > CPM5, > > + CPM5_HOST1, > > }; > > > > /** > > @@ -88,6 +92,9 @@ enum xilinx_cpm_version { > > */ > > struct xilinx_cpm_variant { > > enum xilinx_cpm_version version; > > + u32 ir_status; > > + u32 ir_enable; > > + u32 ir_misc_value; > > Kdoc comments missing for these members. Thanks for review, will update this in next patch. > > > }; > > > > /** > > @@ -269,6 +276,7 @@ static void xilinx_cpm_pcie_event_flow(struct > > irq_desc *desc) { > > struct xilinx_cpm_pcie *port = irq_desc_get_handler_data(desc); > > struct irq_chip *chip = irq_desc_get_chip(desc); > > + const struct xilinx_cpm_variant *variant = port->variant; > > unsigned long val; > > int i; > > > > @@ -279,11 +287,11 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc > *desc) > > generic_handle_domain_irq(port->cpm_domain, i); > > pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR); > > > > - if (port->variant->version == CPM5) { > > - val = readl_relaxed(port->cpm_base + > XILINX_CPM_PCIE_IR_STATUS); > > + if (variant->ir_status) { > > + val = readl_relaxed(port->cpm_base + variant->ir_status); > > if (val) > > writel_relaxed(val, port->cpm_base + > > - XILINX_CPM_PCIE_IR_STATUS); > > + variant->ir_status); > > } > > > > /* > > @@ -465,6 +473,8 @@ static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie > *port) > > */ > > static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) > > { > > + const struct xilinx_cpm_variant *variant = port->variant; > > + > > if (cpm_pcie_link_up(port)) > > dev_info(port->dev, "PCIe Link is UP\n"); > > else > > @@ -483,15 +493,15 @@ static void xilinx_cpm_pcie_init_port(struct > xilinx_cpm_pcie *port) > > * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to > > * CPM SLCR block. > > */ > > - writel(XILINX_CPM_PCIE_MISC_IR_LOCAL, > > + writel(variant->ir_misc_value, > > port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE); > > > > - if (port->variant->version == CPM5) { > > + if (variant->ir_enable) { > > writel(XILINX_CPM_PCIE_IR_LOCAL, > > - port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE); > > + port->cpm_base + variant->ir_enable); > > } > > > > - /* Enable the Bridge enable bit */ > > + /* Set Bridge enable bit */ > > This changes doesn't belong to this patch. Thanks for review, will update this in next patch. > > > pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) | > > XILINX_CPM_PCIE_REG_RPSC_BEN, > > XILINX_CPM_PCIE_REG_RPSC); > > @@ -609,10 +619,21 @@ static int xilinx_cpm_pcie_probe(struct > > platform_device *pdev) > > > > static const struct xilinx_cpm_variant cpm_host = { > > .version = CPM, > > + .ir_misc_value = XILINX_CPM_PCIE0_MISC_IR_LOCAL, > > }; > > > > static const struct xilinx_cpm_variant cpm5_host = { > > .version = CPM5, > > + .ir_misc_value = XILINX_CPM_PCIE0_MISC_IR_LOCAL, > > + .ir_status = XILINX_CPM_PCIE0_IR_STATUS, > > + .ir_enable = XILINX_CPM_PCIE0_IR_ENABLE, }; > > + > > +static const struct xilinx_cpm_variant cpm5_host1 = { > > + .version = CPM5_HOST1, > > + .ir_misc_value = XILINX_CPM_PCIE1_MISC_IR_LOCAL, > > + .ir_status = XILINX_CPM_PCIE1_IR_STATUS, > > + .ir_enable = XILINX_CPM_PCIE1_IR_ENABLE, > > }; > > > > static const struct of_device_id xilinx_cpm_pcie_of_match[] = { @@ > > -624,6 +645,10 @@ static const struct of_device_id xilinx_cpm_pcie_of_match[] > = { > > .compatible = "xlnx,versal-cpm5-host", > > .data = &cpm5_host, > > }, > > + { > > + .compatible = "xlnx,versal-cpm5-host1-1", > > This doesn't look like a valid compatible name. Please use the compatible as per > the IP version. Thanks, Here we don't have any IP versioning for CPM5, so I ll make compatible string as xlnx,versal-cpm5-host1 as updated to Krzysztof Kozlowski. > > - Mani > > > -- > மணிவண்ணன் சதாசிவம்
diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c index a0f5e1d67b04..b783fff27c9d 100644 --- a/drivers/pci/controller/pcie-xilinx-cpm.c +++ b/drivers/pci/controller/pcie-xilinx-cpm.c @@ -30,11 +30,14 @@ #define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C #define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340 #define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348 -#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1) +#define XILINX_CPM_PCIE0_MISC_IR_LOCAL BIT(1) +#define XILINX_CPM_PCIE1_MISC_IR_LOCAL BIT(2) -#define XILINX_CPM_PCIE_IR_STATUS 0x000002A0 -#define XILINX_CPM_PCIE_IR_ENABLE 0x000002A8 -#define XILINX_CPM_PCIE_IR_LOCAL BIT(0) +#define XILINX_CPM_PCIE0_IR_STATUS 0x000002A0 +#define XILINX_CPM_PCIE1_IR_STATUS 0x000002B4 +#define XILINX_CPM_PCIE0_IR_ENABLE 0x000002A8 +#define XILINX_CPM_PCIE1_IR_ENABLE 0x000002BC +#define XILINX_CPM_PCIE_IR_LOCAL BIT(0) #define IMR(x) BIT(XILINX_PCIE_INTR_ ##x) @@ -80,6 +83,7 @@ enum xilinx_cpm_version { CPM, CPM5, + CPM5_HOST1, }; /** @@ -88,6 +92,9 @@ enum xilinx_cpm_version { */ struct xilinx_cpm_variant { enum xilinx_cpm_version version; + u32 ir_status; + u32 ir_enable; + u32 ir_misc_value; }; /** @@ -269,6 +276,7 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) { struct xilinx_cpm_pcie *port = irq_desc_get_handler_data(desc); struct irq_chip *chip = irq_desc_get_chip(desc); + const struct xilinx_cpm_variant *variant = port->variant; unsigned long val; int i; @@ -279,11 +287,11 @@ static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) generic_handle_domain_irq(port->cpm_domain, i); pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR); - if (port->variant->version == CPM5) { - val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_IR_STATUS); + if (variant->ir_status) { + val = readl_relaxed(port->cpm_base + variant->ir_status); if (val) writel_relaxed(val, port->cpm_base + - XILINX_CPM_PCIE_IR_STATUS); + variant->ir_status); } /* @@ -465,6 +473,8 @@ static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie *port) */ static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) { + const struct xilinx_cpm_variant *variant = port->variant; + if (cpm_pcie_link_up(port)) dev_info(port->dev, "PCIe Link is UP\n"); else @@ -483,15 +493,15 @@ static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie *port) * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to * CPM SLCR block. */ - writel(XILINX_CPM_PCIE_MISC_IR_LOCAL, + writel(variant->ir_misc_value, port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE); - if (port->variant->version == CPM5) { + if (variant->ir_enable) { writel(XILINX_CPM_PCIE_IR_LOCAL, - port->cpm_base + XILINX_CPM_PCIE_IR_ENABLE); + port->cpm_base + variant->ir_enable); } - /* Enable the Bridge enable bit */ + /* Set Bridge enable bit */ pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) | XILINX_CPM_PCIE_REG_RPSC_BEN, XILINX_CPM_PCIE_REG_RPSC); @@ -609,10 +619,21 @@ static int xilinx_cpm_pcie_probe(struct platform_device *pdev) static const struct xilinx_cpm_variant cpm_host = { .version = CPM, + .ir_misc_value = XILINX_CPM_PCIE0_MISC_IR_LOCAL, }; static const struct xilinx_cpm_variant cpm5_host = { .version = CPM5, + .ir_misc_value = XILINX_CPM_PCIE0_MISC_IR_LOCAL, + .ir_status = XILINX_CPM_PCIE0_IR_STATUS, + .ir_enable = XILINX_CPM_PCIE0_IR_ENABLE, +}; + +static const struct xilinx_cpm_variant cpm5_host1 = { + .version = CPM5_HOST1, + .ir_misc_value = XILINX_CPM_PCIE1_MISC_IR_LOCAL, + .ir_status = XILINX_CPM_PCIE1_IR_STATUS, + .ir_enable = XILINX_CPM_PCIE1_IR_ENABLE, }; static const struct of_device_id xilinx_cpm_pcie_of_match[] = { @@ -624,6 +645,10 @@ static const struct of_device_id xilinx_cpm_pcie_of_match[] = { .compatible = "xlnx,versal-cpm5-host", .data = &cpm5_host, }, + { + .compatible = "xlnx,versal-cpm5-host1-1", + .data = &cpm5_host1, + }, {} };
This patch adds support for the Xilinx Versal CPM5 Root Port Controller 1. The key difference between Controller 0 and Controller 1 lies in the platform-specific error interrupt bits, which are located at different register offsets. To handle these differences, a variant structure is introduced that holds the following platform-specific details: - Interrupt status register offset (ir_status) - Interrupt enable register offset (ir_enable) - Miscellaneous interrupt values (ir_misc_value) The driver differentiates between Controller 0 and Controller 1 using the compatible string in the device tree. This ensures that the appropriate register offsets are used for each controller, allowing for correct handling of platform-specific interrupts and initialization. Signed-off-by: Thippeswamy Havalige <thippesw@amd.com> --- changes in v2: -------------- 1. Introduced new constants for Controller 1. 2. Extended the xilinx_cpm_variant structure to support a. ir_status, b. ir_enable, and c. ir_misc_value for different controllers. 3. Updated IRQ handling and initialization to use the variant structure. 4. Added a new device tree match entry for Controller 1. --- drivers/pci/controller/pcie-xilinx-cpm.c | 47 ++++++++++++++++++------ 1 file changed, 36 insertions(+), 11 deletions(-)