Hi Bjorn,
Thanks, will update all your review comments and resend patch.
Regards,
Thippeswamy H
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Tuesday, July 23, 2024 3:45 AM
> To: Havalige, Thippeswamy <thippeswamy.havalige@xxxxxxx>
> Cc: lpieralisi@xxxxxxxxxx; kw@xxxxxxxxx; robh@xxxxxxxxxx;
> bhelgaas@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> pci@xxxxxxxxxxxxxxx; Havalige, Thippeswamy
> <thippeswamy.havalige@xxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> Simek, Michal <michal.simek@xxxxxxx>
> Subject: Re: [PATCH v2 2/2] PCI: xilinx-xdma: Add Xilinx QDMA Root Port
> driver
>
> On Mon, Jul 22, 2024 at 11:55:58AM +0530, Thippeswamy Havalige wrote:
> > Add support for Xilinx QDMA Soft IP core as Root Port.
> >
> > The versal prime devices support QDMA soft IP module in
> > programmable logic.
>
> Capitalize brand names.
>
> > The integrated QDMA Soft IP block has integrated bridge function that
> > can act as PCIe Root Port.
>
> Rewrap to fill 75 columns.
>
> > +#define QDMA_BRIDGE_BASE_OFF 0xCD8
>
> Other #defines in this file user lower-case hex; please match them.
>
> > static inline u32 pcie_read(struct pl_dma_pcie *port, u32 reg)
> > {
> > - return readl(port->reg_base + reg);
> > + if (port->variant->version == XDMA)
> > + return readl(port->reg_base + reg);
> > + else
> > + return readl(port->reg_base + reg +
> QDMA_BRIDGE_BASE_OFF);
> > }
> >
> > static inline void pcie_write(struct pl_dma_pcie *port, u32 val, u32 reg)
> > {
> > - writel(val, port->reg_base + reg);
> > + if (port->variant->version == XDMA)
> > + writel(val, port->reg_base + reg);
> > + else
> > + writel(val, port->reg_base + reg +
> QDMA_BRIDGE_BASE_OFF);
> > }
> >
> > static inline bool xilinx_pl_dma_pcie_link_up(struct pl_dma_pcie *port)
> > @@ -173,7 +198,10 @@ static void __iomem
> *xilinx_pl_dma_pcie_map_bus(struct pci_bus *bus,
> > if (!xilinx_pl_dma_pcie_valid_device(bus, devfn))
> > return NULL;
> >
> > - return port->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn,
> where);
> > + if (port->variant->version == XDMA)
> > + return port->reg_base + PCIE_ECAM_OFFSET(bus->number,
> devfn, where);
> > + else
> > + return port->cfg_base + PCIE_ECAM_OFFSET(bus->number,
> devfn, where);
>
> If you rework the variant tests above to use
> "if (port->variant->version == QDMA)" instead, they will match the one
> below, and you won't need to touch the existing code at all, e.g.,
>
> + if (port->variant->version == QDMA)
> + return port->cfg_base + PCIE_ECAM_OFFSET(bus->number, devfn,
> where);
>
> return port->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn, where);
>
> > }
> >
> > /* PCIe operations */
> > @@ -731,6 +759,15 @@ static int xilinx_pl_dma_pcie_parse_dt(struct
> pl_dma_pcie *port,
> >
> > port->reg_base = port->cfg->win;
> >
> > + if (port->variant->version == QDMA) {
> > + port->cfg_base = port->cfg->win;
> > + res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM, "breg");
> > + port->reg_base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(port->reg_base))
> > + return PTR_ERR(port->reg_base);
> > + port->phys_reg_base = res->start;
> > + }
