| Message ID | 202303231145121987818@zte.com.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI: mt7621: Use dev_err_probe() | expand |
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
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);
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
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
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 >
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
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 >
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 > >
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);