Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210108093610.28595-1-Zhiqiang.Hou@nxp.com> <20210108093610.28595-2-Zhiqiang.Hou@nxp.com>
In-Reply-To: <20210108093610.28595-2-Zhiqiang.Hou@nxp.com>
From:   Li Yang <leoyang.li@nxp.com>
Date:   Thu, 28 Jan 2021 17:22:02 -0600
Message-ID: <CADRPPNQr4xGH0On94rxr69VYeZAQEYQgt=CosVqEDi91cfO_tw@mail.gmail.com>
Subject: Re: [PATCHv3 1/7] PCI: dwc: Fix a bug of the case dw_pci->ops is NULL
To:     Zhiqiang Hou <Zhiqiang.Hou@nxp.com>,
        Bjorn Helgaas <bhelgaas@google.com>
Cc:     linux-pci@vger.kernel.org,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Rob Herring <robh+dt@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>, gustavo.pimentel@synopsys.com,
        Minghuan Lian <minghuan.Lian@nxp.com>,
        Mingkai Hu <mingkai.hu@nxp.com>, Roy Zang <roy.zang@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 8, 2021 at 3:29 AM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote:
>
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>
> The dw_pci->ops may be a NULL, and fix it by adding one more check.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

Hi Bjorn,

This is causing many layerscape platforms to fail to boot.  Can you
help to pick up this patch?  Or are you actually preferring other
solutions like creating dummy ops when it is not needed?

Regards,
Leo

> ---
> V3:
>  - Rebased against the latest code base
>
>  drivers/pci/controller/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/controller/dwc/pcie-designware.c      | 14 +++++++-------
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 516b151e0ef3..0413284fdd93 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -429,7 +429,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>         dw_pcie_setup_rc(pp);
>         dw_pcie_msi_init(pp);
>
> -       if (!dw_pcie_link_up(pci) && pci->ops->start_link) {
> +       if (!dw_pcie_link_up(pci) && pci->ops && pci->ops->start_link) {
>                 ret = pci->ops->start_link(pci);
>                 if (ret)
>                         goto err_free_msi;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 645fa1892375..cf895c12f71f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -141,7 +141,7 @@ u32 dw_pcie_read_dbi(struct dw_pcie *pci, u32 reg, size_t size)
>         int ret;
>         u32 val;
>
> -       if (pci->ops->read_dbi)
> +       if (pci->ops && pci->ops->read_dbi)
>                 return pci->ops->read_dbi(pci, pci->dbi_base, reg, size);
>
>         ret = dw_pcie_read(pci->dbi_base + reg, size, &val);
> @@ -156,7 +156,7 @@ void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
>  {
>         int ret;
>
> -       if (pci->ops->write_dbi) {
> +       if (pci->ops && pci->ops->write_dbi) {
>                 pci->ops->write_dbi(pci, pci->dbi_base, reg, size, val);
>                 return;
>         }
> @@ -171,7 +171,7 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val)
>  {
>         int ret;
>
> -       if (pci->ops->write_dbi2) {
> +       if (pci->ops && pci->ops->write_dbi2) {
>                 pci->ops->write_dbi2(pci, pci->dbi_base2, reg, size, val);
>                 return;
>         }
> @@ -186,7 +186,7 @@ static u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg)
>         int ret;
>         u32 val;
>
> -       if (pci->ops->read_dbi)
> +       if (pci->ops && pci->ops->read_dbi)
>                 return pci->ops->read_dbi(pci, pci->atu_base, reg, 4);
>
>         ret = dw_pcie_read(pci->atu_base + reg, 4, &val);
> @@ -200,7 +200,7 @@ static void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val)
>  {
>         int ret;
>
> -       if (pci->ops->write_dbi) {
> +       if (pci->ops && pci->ops->write_dbi) {
>                 pci->ops->write_dbi(pci, pci->atu_base, reg, 4, val);
>                 return;
>         }
> @@ -273,7 +273,7 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  {
>         u32 retries, val;
>
> -       if (pci->ops->cpu_addr_fixup)
> +       if (pci->ops && pci->ops->cpu_addr_fixup)
>                 cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
>
>         if (pci->iatu_unroll_enabled) {
> @@ -481,7 +481,7 @@ int dw_pcie_link_up(struct dw_pcie *pci)
>  {
>         u32 val;
>
> -       if (pci->ops->link_up)
> +       if (pci->ops && pci->ops->link_up)
>                 return pci->ops->link_up(pci);
>
>         val = readl(pci->dbi_base + PCIE_PORT_DEBUG1);
> --
> 2.17.1
>