Re: [PATCH v10 2/4] PCI: mediatek: Add new method to get shared pcie-cfg base address and parse node

From: Chuanjia Liu
Date: Wed Jul 14 2021 - 06:30:15 EST


On Tue, 2021-07-13 at 13:32 +0200, Matthias Brugger wrote:
>
> On 11/06/2021 08:09, Chuanjia Liu wrote:
> > For the new dts format, add a new method to get
> > shared pcie-cfg base address and parse node.
> >
> > Signed-off-by: Chuanjia Liu <chuanjia.liu@xxxxxxxxxxxx>
> > Acked-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
>
> You missed the
> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> given in v8. Or were there any substantial changes in this patch?
>
Thanks for your review!

Only a small change,as shown below

if (err)
- goto error_put_node;
+ return err;
}
I have a description in the V9 version:
fix kernel-ci bot warning,In the scene of using new dts format,
when mtk_pcie_parse_port fails, of_node_put don't need to be called.

So I didn't add reviewed-by rob because I changed the patch。
> > ---
> > drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++-------
> > 1 file changed, 39 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 62a042e75d9a..950f577a2f44 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -14,6 +14,7 @@
> > #include <linux/irqchip/chained_irq.h>
> > #include <linux/irqdomain.h>
> > #include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> > #include <linux/msi.h>
> > #include <linux/module.h>
> > #include <linux/of_address.h>
> > @@ -23,6 +24,7 @@
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > #include <linux/reset.h>
> >
> > #include "../pci.h"
> > @@ -207,6 +209,7 @@ struct mtk_pcie_port {
> > * struct mtk_pcie - PCIe host information
> > * @dev: pointer to PCIe device
> > * @base: IO mapped register base
> > + * @cfg: IO mapped register map for PCIe config
> > * @free_ck: free-run reference clock
> > * @mem: non-prefetchable memory resource
> > * @ports: pointer to PCIe port information
> > @@ -215,6 +218,7 @@ struct mtk_pcie_port {
> > struct mtk_pcie {
> > struct device *dev;
> > void __iomem *base;
> > + struct regmap *cfg;
> > struct clk *free_ck;
> >
> > struct list_head ports;
> > @@ -650,7 +654,11 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
> > return err;
> > }
> >
> > - port->irq = platform_get_irq(pdev, port->slot);
> > + if (of_find_property(dev->of_node, "interrupt-names", NULL))
> > + port->irq = platform_get_irq_byname(pdev, "pcie_irq");
> > + else
> > + port->irq = platform_get_irq(pdev, port->slot);
> > +
>
> Do I understand that this is used for backwards compatibility with older DTS? I
> just wonder why we don't need to mandate
> interrupt-names = "pcie_irq"
> in the binding description.
Yes,this is used for backwards compatibility with older DTS。
If necessary, I will add the following in binding description.
- interrupt-names:Must include the following entries:
- "pcie_irq": The interrupt that is asserted when an MSI/INTX is
received

Best regards
Chuanjia
>
> Regards,
> Matthias