linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: mt7621: Use dev_err_probe()
@ 2023-03-23  3:45 ye.xingchen
  2023-03-23  6:23 ` Sergio Paracuellos
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: ye.xingchen @ 2023-03-23  3:45 UTC (permalink / raw)
  To: sergio.paracuellos
  Cc: lpieralisi, kw, robh, bhelgaas, matthias.bgg,
	angelogioacchino.delregno, linux-pci, linux-kernel,
	linux-arm-kernel, linux-mediatek

From: Ye Xingchen <ye.xingchen@zte.com.cn>

Replace the open-code with dev_err_probe() to simplify the code.

Signed-off-by: Ye Xingchen <ye.xingchen@zte.com.cn>
---
 drivers/pci/controller/pcie-mt7621.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
index 63a5f4463a9f..964de0e8c6a0 100644
--- a/drivers/pci/controller/pcie-mt7621.c
+++ b/drivers/pci/controller/pcie-mt7621.c
@@ -220,10 +220,9 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
 	}

 	port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
-	if (PTR_ERR(port->pcie_rst) == -EPROBE_DEFER) {
-		dev_err(dev, "failed to get pcie%d reset control\n", slot);
-		return PTR_ERR(port->pcie_rst);
-	}
+
+	return dev_err_probe(dev, PTR_ERR(port->pcie_rst),
+			     "failed to get pcie%d reset control\n", slot);

 	snprintf(name, sizeof(name), "pcie-phy%d", slot);
 	port->phy = devm_of_phy_get(dev, node, name);
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-03-23  3:45 [PATCH] PCI: mt7621: Use dev_err_probe() ye.xingchen
@ 2023-03-23  6:23 ` Sergio Paracuellos
  2023-04-03  4:50   ` Dan Carpenter
  2023-03-23  8:21 ` AngeloGioacchino Del Regno
  2023-04-03  4:41 ` Dan Carpenter
  2 siblings, 1 reply; 9+ messages in thread
From: Sergio Paracuellos @ 2023-03-23  6:23 UTC (permalink / raw)
  To: ye.xingchen
  Cc: lpieralisi, kw, robh, bhelgaas, matthias.bgg,
	angelogioacchino.delregno, linux-pci, linux-kernel,
	linux-arm-kernel, linux-mediatek

Hi,

On Thu, Mar 23, 2023 at 4:45 AM <ye.xingchen@zte.com.cn> wrote:
>
> From: Ye Xingchen <ye.xingchen@zte.com.cn>
>
> Replace the open-code with dev_err_probe() to simplify the code.
>
> Signed-off-by: Ye Xingchen <ye.xingchen@zte.com.cn>
> ---
>  drivers/pci/controller/pcie-mt7621.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
> index 63a5f4463a9f..964de0e8c6a0 100644
> --- a/drivers/pci/controller/pcie-mt7621.c
> +++ b/drivers/pci/controller/pcie-mt7621.c
> @@ -220,10 +220,9 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
>         }
>
>         port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
> -       if (PTR_ERR(port->pcie_rst) == -EPROBE_DEFER) {
> -               dev_err(dev, "failed to get pcie%d reset control\n", slot);
> -               return PTR_ERR(port->pcie_rst);
> -       }
> +
> +       return dev_err_probe(dev, PTR_ERR(port->pcie_rst),
> +                            "failed to get pcie%d reset control\n", slot);
>
>         snprintf(name, sizeof(name), "pcie-phy%d", slot);
>         port->phy = devm_of_phy_get(dev, node, name);

So this code is unreachable now. Please fix your tools.

> --
> 2.25.1

Also, this is not a probe(), so I don't see a point of using
dev_err_probe() here.

Thanks,
    Sergio Paracuellos

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-03-23  3:45 [PATCH] PCI: mt7621: Use dev_err_probe() ye.xingchen
  2023-03-23  6:23 ` Sergio Paracuellos
@ 2023-03-23  8:21 ` AngeloGioacchino Del Regno
  2023-04-03  4:41 ` Dan Carpenter
  2 siblings, 0 replies; 9+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-03-23  8:21 UTC (permalink / raw)
  To: ye.xingchen, sergio.paracuellos
  Cc: lpieralisi, kw, robh, bhelgaas, matthias.bgg, linux-pci,
	linux-kernel, linux-arm-kernel, linux-mediatek

Il 23/03/23 04:45, ye.xingchen@zte.com.cn ha scritto:
> From: Ye Xingchen <ye.xingchen@zte.com.cn>
> 
> Replace the open-code with dev_err_probe() to simplify the code.
> 
> Signed-off-by: Ye Xingchen <ye.xingchen@zte.com.cn>
> ---
>   drivers/pci/controller/pcie-mt7621.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
> index 63a5f4463a9f..964de0e8c6a0 100644
> --- a/drivers/pci/controller/pcie-mt7621.c
> +++ b/drivers/pci/controller/pcie-mt7621.c
> @@ -220,10 +220,9 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
>   	}
> 
>   	port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
> -	if (PTR_ERR(port->pcie_rst) == -EPROBE_DEFER) {
> -		dev_err(dev, "failed to get pcie%d reset control\n", slot);
> -		return PTR_ERR(port->pcie_rst);
> -	}
> +
> +	return dev_err_probe(dev, PTR_ERR(port->pcie_rst),
> +			     "failed to get pcie%d reset control\n", slot);

That's breaking this function. You're unconditionally returning.

I think that this is fine as it is, but if you really want to use dev_err_probe()
here, this could be...

ret = dev_err_probe(dev, PTR_ERR(port->pcie_rst), "failed ...." ...);
if (ret)
	return ret;

Regards,
Angelo

> 
>   	snprintf(name, sizeof(name), "pcie-phy%d", slot);
>   	port->phy = devm_of_phy_get(dev, node, name);



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-03-23  3:45 [PATCH] PCI: mt7621: Use dev_err_probe() ye.xingchen
  2023-03-23  6:23 ` Sergio Paracuellos
  2023-03-23  8:21 ` AngeloGioacchino Del Regno
@ 2023-04-03  4:41 ` Dan Carpenter
  2023-04-03  5:05   ` Sergio Paracuellos
  2 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2023-04-03  4:41 UTC (permalink / raw)
  To: oe-kbuild, ye.xingchen, sergio.paracuellos
  Cc: lkp, oe-kbuild-all, lpieralisi, kw, robh, bhelgaas, matthias.bgg,
	angelogioacchino.delregno, linux-pci, linux-kernel,
	linux-arm-kernel, linux-mediatek

Hi,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ye-xingchen-zte-com-cn/PCI-mt7621-Use-dev_err_probe/20230323-114623
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/202303231145121987818%40zte.com.cn
patch subject: [PATCH] PCI: mt7621: Use dev_err_probe()
config: s390-randconfig-m031-20230329 (https://download.01.org/0day-ci/archive/20230401/202304010325.2OPFvIm3-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304010325.2OPFvIm3-lkp@intel.com/

smatch warnings:
drivers/pci/controller/pcie-mt7621.c:224 mt7621_pcie_parse_port() warn: passing zero to 'PTR_ERR'
drivers/pci/controller/pcie-mt7621.c:227 mt7621_pcie_parse_port() warn: ignoring unreachable code.

vim +/PTR_ERR +224 drivers/pci/controller/pcie-mt7621.c

ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  198  static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  199  				  struct device_node *node,
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  200  				  int slot)
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  201  {
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  202  	struct mt7621_pcie_port *port;
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  203  	struct device *dev = pcie->dev;
fab6710e4c51f4 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-04-13  204  	struct platform_device *pdev = to_platform_device(dev);
61f9bde6ea578f drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2019-01-04  205  	char name[10];
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  206  	int err;
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  207  
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  208  	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  209  	if (!port)
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  210  		return -ENOMEM;
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  211  
108b2f2a972454 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-11-23  212  	port->base = devm_platform_ioremap_resource(pdev, slot + 1);
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  213  	if (IS_ERR(port->base))
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  214  		return PTR_ERR(port->base);
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  215  
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  216  	port->clk = devm_get_clk_from_child(dev, node, NULL);
cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  217  	if (IS_ERR(port->clk)) {
cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  218  		dev_err(dev, "failed to get pcie%d clock\n", slot);
cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  219  		return PTR_ERR(port->clk);
cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  220  	}
cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  221  
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  222  	port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
9873bac812f262 drivers/pci/controller/pcie-mt7621.c    Ye Xingchen        2023-03-23  223  
9873bac812f262 drivers/pci/controller/pcie-mt7621.c    Ye Xingchen        2023-03-23 @224  	return dev_err_probe(dev, PTR_ERR(port->pcie_rst),
                                                                                                                          ^^^^^^^^^^^^^^^^^^^^^^^

9873bac812f262 drivers/pci/controller/pcie-mt7621.c    Ye Xingchen        2023-03-23  225  			     "failed to get pcie%d reset control\n", slot);
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  226  
61f9bde6ea578f drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2019-01-04 @227  	snprintf(name, sizeof(name), "pcie-phy%d", slot);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  228  	port->phy = devm_of_phy_get(dev, node, name);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  229  	if (IS_ERR(port->phy)) {
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  230  		dev_err(dev, "failed to get pcie-phy%d\n", slot);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  231  		err = PTR_ERR(port->phy);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  232  		goto remove_reset;
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  233  	}
61f9bde6ea578f drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2019-01-04  234  
b27e35f91c75cf drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-13  235  	port->gpio_rst = devm_gpiod_get_index_optional(dev, "reset", slot,
b27e35f91c75cf drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-13  236  						       GPIOD_OUT_LOW);
825c6f470c62da drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-20  237  	if (IS_ERR(port->gpio_rst)) {
2bdd5238e756aa drivers/pci/controller/pcie-mt7621.c    Sergio Paracuellos 2021-09-22  238  		dev_err(dev, "failed to get GPIO for PCIe%d\n", slot);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  239  		err = PTR_ERR(port->gpio_rst);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  240  		goto remove_reset;
825c6f470c62da drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-20  241  	}
b27e35f91c75cf drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-13  242  
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  243  	port->slot = slot;
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  244  	port->pcie = pcie;
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  245  
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  246  	INIT_LIST_HEAD(&port->list);
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  247  	list_add_tail(&port->list, &pcie->ports);
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  248  
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  249  	return 0;
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  250  
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  251  remove_reset:
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  252  	reset_control_put(port->pcie_rst);
2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  253  	return err;
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  254  }
ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  255  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-03-23  6:23 ` Sergio Paracuellos
@ 2023-04-03  4:50   ` Dan Carpenter
  2023-04-03  7:12     ` Sergio Paracuellos
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2023-04-03  4:50 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: ye.xingchen, lpieralisi, kw, robh, bhelgaas, matthias.bgg,
	angelogioacchino.delregno, linux-pci, linux-kernel,
	linux-arm-kernel, linux-mediatek

On Thu, Mar 23, 2023 at 07:23:26AM +0100, Sergio Paracuellos wrote:
> Hi,
> 
> On Thu, Mar 23, 2023 at 4:45 AM <ye.xingchen@zte.com.cn> wrote:
> >
> > From: Ye Xingchen <ye.xingchen@zte.com.cn>
> >
> > Replace the open-code with dev_err_probe() to simplify the code.
> >
> > Signed-off-by: Ye Xingchen <ye.xingchen@zte.com.cn>
> > ---
> >  drivers/pci/controller/pcie-mt7621.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
> > index 63a5f4463a9f..964de0e8c6a0 100644
> > --- a/drivers/pci/controller/pcie-mt7621.c
> > +++ b/drivers/pci/controller/pcie-mt7621.c
> > @@ -220,10 +220,9 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
> >         }
> >
> >         port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
> > -       if (PTR_ERR(port->pcie_rst) == -EPROBE_DEFER) {

So the theory here is that -EPROBE_DEFER is recoverable but other errors
are not so we just ignore them?  Error pointers will trigger a WARN() in
mt7621_control_assert/deassert().


> > --
> > 2.25.1
> 
> Also, this is not a probe(), so I don't see a point of using
> dev_err_probe() here.

It's a weird thing to return -EPROBE_DEFER from something which is not
a probe function...  Someone told me I should write a Smatch check for
it but I never got around to doing that.

In this case, I guess the user is supposed to see the error message and
manually fix the probe order?  The dev_err_probe() will change the error
message into a debug message so the user will not see it and will not be
able to fix it.  So using dev_err_probe() will break things for the
user.

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-04-03  4:41 ` Dan Carpenter
@ 2023-04-03  5:05   ` Sergio Paracuellos
  2023-04-03  6:09     ` Dan Carpenter
  0 siblings, 1 reply; 9+ messages in thread
From: Sergio Paracuellos @ 2023-04-03  5:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, ye.xingchen, lkp, oe-kbuild-all, lpieralisi, kw, robh,
	bhelgaas, matthias.bgg, angelogioacchino.delregno, linux-pci,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Mon, Apr 3, 2023 at 6:41 AM Dan Carpenter <error27@gmail.com> wrote:
>
> Hi,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/ye-xingchen-zte-com-cn/PCI-mt7621-Use-dev_err_probe/20230323-114623
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
> patch link:    https://lore.kernel.org/r/202303231145121987818%40zte.com.cn

So, I already replied to this proposed patch clearly saying that this
makes the rest of the code unreachable, so it is a clear NAK.
Why is this applied to the intel-lab-lkp tree? Just to be able to test
the changes?

Thanks,
    Sergio Paracuellos

> patch subject: [PATCH] PCI: mt7621: Use dev_err_probe()
> config: s390-randconfig-m031-20230329 (https://download.01.org/0day-ci/archive/20230401/202304010325.2OPFvIm3-lkp@intel.com/config)
> compiler: s390-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202304010325.2OPFvIm3-lkp@intel.com/
>
> smatch warnings:
> drivers/pci/controller/pcie-mt7621.c:224 mt7621_pcie_parse_port() warn: passing zero to 'PTR_ERR'
> drivers/pci/controller/pcie-mt7621.c:227 mt7621_pcie_parse_port() warn: ignoring unreachable code.
>
> vim +/PTR_ERR +224 drivers/pci/controller/pcie-mt7621.c
>
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  198  static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  199                                 struct device_node *node,
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  200                                 int slot)
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  201  {
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  202       struct mt7621_pcie_port *port;
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  203       struct device *dev = pcie->dev;
> fab6710e4c51f4 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-04-13  204       struct platform_device *pdev = to_platform_device(dev);
> 61f9bde6ea578f drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2019-01-04  205       char name[10];
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  206       int err;
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  207
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  208       port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  209       if (!port)
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  210               return -ENOMEM;
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  211
> 108b2f2a972454 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-11-23  212       port->base = devm_platform_ioremap_resource(pdev, slot + 1);
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  213       if (IS_ERR(port->base))
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  214               return PTR_ERR(port->base);
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  215
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  216       port->clk = devm_get_clk_from_child(dev, node, NULL);
> cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  217       if (IS_ERR(port->clk)) {
> cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  218               dev_err(dev, "failed to get pcie%d clock\n", slot);
> cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  219               return PTR_ERR(port->clk);
> cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  220       }
> cc4e864a5ce4c1 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-05-05  221
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  222       port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
> 9873bac812f262 drivers/pci/controller/pcie-mt7621.c    Ye Xingchen        2023-03-23  223
> 9873bac812f262 drivers/pci/controller/pcie-mt7621.c    Ye Xingchen        2023-03-23 @224       return dev_err_probe(dev, PTR_ERR(port->pcie_rst),
>                                                                                                                           ^^^^^^^^^^^^^^^^^^^^^^^
>
> 9873bac812f262 drivers/pci/controller/pcie-mt7621.c    Ye Xingchen        2023-03-23  225                            "failed to get pcie%d reset control\n", slot);
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  226
> 61f9bde6ea578f drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2019-01-04 @227       snprintf(name, sizeof(name), "pcie-phy%d", slot);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  228       port->phy = devm_of_phy_get(dev, node, name);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  229       if (IS_ERR(port->phy)) {
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  230               dev_err(dev, "failed to get pcie-phy%d\n", slot);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  231               err = PTR_ERR(port->phy);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  232               goto remove_reset;
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  233       }
> 61f9bde6ea578f drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2019-01-04  234
> b27e35f91c75cf drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-13  235       port->gpio_rst = devm_gpiod_get_index_optional(dev, "reset", slot,
> b27e35f91c75cf drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-13  236                                                      GPIOD_OUT_LOW);
> 825c6f470c62da drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-20  237       if (IS_ERR(port->gpio_rst)) {
> 2bdd5238e756aa drivers/pci/controller/pcie-mt7621.c    Sergio Paracuellos 2021-09-22  238               dev_err(dev, "failed to get GPIO for PCIe%d\n", slot);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  239               err = PTR_ERR(port->gpio_rst);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  240               goto remove_reset;
> 825c6f470c62da drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-20  241       }
> b27e35f91c75cf drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2020-03-13  242
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  243       port->slot = slot;
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  244       port->pcie = pcie;
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  245
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  246       INIT_LIST_HEAD(&port->list);
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  247       list_add_tail(&port->list, &pcie->ports);
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  248
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  249       return 0;
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  250
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  251  remove_reset:
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  252       reset_control_put(port->pcie_rst);
> 2d3d288f0eaf10 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2021-06-07  253       return err;
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  254  }
> ad9c87e129d139 drivers/staging/mt7621-pci/pci-mt7621.c Sergio Paracuellos 2018-11-04  255
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-04-03  5:05   ` Sergio Paracuellos
@ 2023-04-03  6:09     ` Dan Carpenter
  2023-04-03  7:12       ` Sergio Paracuellos
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2023-04-03  6:09 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: oe-kbuild, ye.xingchen, lkp, oe-kbuild-all, lpieralisi, kw, robh,
	bhelgaas, matthias.bgg, angelogioacchino.delregno, linux-pci,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Mon, Apr 03, 2023 at 07:05:56AM +0200, Sergio Paracuellos wrote:
> On Mon, Apr 3, 2023 at 6:41 AM Dan Carpenter <error27@gmail.com> wrote:
> >
> > Hi,
> >
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/ye-xingchen-zte-com-cn/PCI-mt7621-Use-dev_err_probe/20230323-114623
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
> > patch link:    https://lore.kernel.org/r/202303231145121987818%40zte.com.cn
> 
> So, I already replied to this proposed patch clearly saying that this
> makes the rest of the code unreachable, so it is a clear NAK.
> Why is this applied to the intel-lab-lkp tree? Just to be able to test
> the changes?
> 

These emails are automatically generated by kbuild-bot.  I don't know
how kbuild-bot internals work.  I just review some of the Smatch related
warnings and hit forward or ignore them.

Normally, I don't look at the context outside of the email but to be
honest, I was curious enough about this one that I looked it up on the
list.  I knew it was NAKed but I set the email anyway hoping that maybe
people would see the extra Smatch warning and be encouraged to run
Smatch on their code in the future to avoid potential embarrassment.

regards,
dan carpenter



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-04-03  4:50   ` Dan Carpenter
@ 2023-04-03  7:12     ` Sergio Paracuellos
  0 siblings, 0 replies; 9+ messages in thread
From: Sergio Paracuellos @ 2023-04-03  7:12 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: ye.xingchen, lpieralisi, kw, robh, bhelgaas, matthias.bgg,
	angelogioacchino.delregno, linux-pci, linux-kernel,
	linux-arm-kernel, linux-mediatek

On Mon, Apr 3, 2023 at 6:50 AM Dan Carpenter <error27@gmail.com> wrote:
>
> On Thu, Mar 23, 2023 at 07:23:26AM +0100, Sergio Paracuellos wrote:
> > Hi,
> >
> > On Thu, Mar 23, 2023 at 4:45 AM <ye.xingchen@zte.com.cn> wrote:
> > >
> > > From: Ye Xingchen <ye.xingchen@zte.com.cn>
> > >
> > > Replace the open-code with dev_err_probe() to simplify the code.
> > >
> > > Signed-off-by: Ye Xingchen <ye.xingchen@zte.com.cn>
> > > ---
> > >  drivers/pci/controller/pcie-mt7621.c | 7 +++----
> > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
> > > index 63a5f4463a9f..964de0e8c6a0 100644
> > > --- a/drivers/pci/controller/pcie-mt7621.c
> > > +++ b/drivers/pci/controller/pcie-mt7621.c
> > > @@ -220,10 +220,9 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
> > >         }
> > >
> > >         port->pcie_rst = of_reset_control_get_exclusive(node, NULL);
> > > -       if (PTR_ERR(port->pcie_rst) == -EPROBE_DEFER) {
>
> So the theory here is that -EPROBE_DEFER is recoverable but other errors
> are not so we just ignore them?  Error pointers will trigger a WARN() in
> mt7621_control_assert/deassert().
>
>
> > > --
> > > 2.25.1
> >
> > Also, this is not a probe(), so I don't see a point of using
> > dev_err_probe() here.
>
> It's a weird thing to return -EPROBE_DEFER from something which is not
> a probe function...  Someone told me I should write a Smatch check for
> it but I never got around to doing that.

I don't remember clearly why this was returned in the first instance.
I think I just took the idea from pcie-mediatek driver for arm64 SoCs
platforms here:

https://elixir.bootlin.com/linux/v6.3-rc5/source/drivers/pci/controller/pcie-mediatek.c#L967

Thanks,
    Sergio Paracuellos
>
> In this case, I guess the user is supposed to see the error message and
> manually fix the probe order?  The dev_err_probe() will change the error
> message into a debug message so the user will not see it and will not be
> able to fix it.  So using dev_err_probe() will break things for the
> user.
>
> regards,
> dan carpenter
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: mt7621: Use dev_err_probe()
  2023-04-03  6:09     ` Dan Carpenter
@ 2023-04-03  7:12       ` Sergio Paracuellos
  0 siblings, 0 replies; 9+ messages in thread
From: Sergio Paracuellos @ 2023-04-03  7:12 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, ye.xingchen, lkp, oe-kbuild-all, lpieralisi, kw, robh,
	bhelgaas, matthias.bgg, angelogioacchino.delregno, linux-pci,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Mon, Apr 3, 2023 at 8:11 AM Dan Carpenter <error27@gmail.com> wrote:
>
> On Mon, Apr 03, 2023 at 07:05:56AM +0200, Sergio Paracuellos wrote:
> > On Mon, Apr 3, 2023 at 6:41 AM Dan Carpenter <error27@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > >
> > > url:    https://github.com/intel-lab-lkp/linux/commits/ye-xingchen-zte-com-cn/PCI-mt7621-Use-dev_err_probe/20230323-114623
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
> > > patch link:    https://lore.kernel.org/r/202303231145121987818%40zte.com.cn
> >
> > So, I already replied to this proposed patch clearly saying that this
> > makes the rest of the code unreachable, so it is a clear NAK.
> > Why is this applied to the intel-lab-lkp tree? Just to be able to test
> > the changes?
> >
>
> These emails are automatically generated by kbuild-bot.  I don't know
> how kbuild-bot internals work.  I just review some of the Smatch related
> warnings and hit forward or ignore them.
>
> Normally, I don't look at the context outside of the email but to be
> honest, I was curious enough about this one that I looked it up on the
> list.  I knew it was NAKed but I set the email anyway hoping that maybe
> people would see the extra Smatch warning and be encouraged to run
> Smatch on their code in the future to avoid potential embarrassment.

I see :). Thanks for the explanation, Dan.

Best regards,
    Sergio Paracuellos
>
> regards,
> dan carpenter
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-03  7:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-23  3:45 [PATCH] PCI: mt7621: Use dev_err_probe() ye.xingchen
2023-03-23  6:23 ` Sergio Paracuellos
2023-04-03  4:50   ` Dan Carpenter
2023-04-03  7:12     ` Sergio Paracuellos
2023-03-23  8:21 ` AngeloGioacchino Del Regno
2023-04-03  4:41 ` Dan Carpenter
2023-04-03  5:05   ` Sergio Paracuellos
2023-04-03  6:09     ` Dan Carpenter
2023-04-03  7:12       ` Sergio Paracuellos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).