| Message ID | 20200414102512.27506-1-pali@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: tegra: Fix reporting GPIO error value | expand |
On Tue, Apr 14, 2020 at 12:25:12PM +0200, Pali Rohár wrote: > Error code is stored in rp->reset_gpio and not in err variable. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pci/controller/pci-tegra.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index 0e03cef72840..378d5a8773c7 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -2314,8 +2314,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) > if (PTR_ERR(rp->reset_gpio) == -ENOENT) { > rp->reset_gpio = NULL; > } else { > - dev_err(dev, "failed to get reset GPIO: %d\n", > - err); > + dev_err(dev, "failed to get reset GPIO: %ld\n", > + PTR_ERR(rp->reset_gpio)); > return PTR_ERR(rp->reset_gpio); > } > } You can use %pe directly on the pointer for added benefit of translation of the error to a name. Best Regards, Michał Mirosław
On Tue, Apr 14, 2020 at 12:25:12PM +0200, Pali Rohár wrote: > Error code is stored in rp->reset_gpio and not in err variable. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pci/controller/pci-tegra.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) With or without Michał's suggestion: Acked-by: Thierry Reding <treding@nvidia.com>
On Tuesday 14 April 2020 13:31:04 Michał Mirosław wrote: > On Tue, Apr 14, 2020 at 12:25:12PM +0200, Pali Rohár wrote: > > Error code is stored in rp->reset_gpio and not in err variable. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > drivers/pci/controller/pci-tegra.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > > index 0e03cef72840..378d5a8773c7 100644 > > --- a/drivers/pci/controller/pci-tegra.c > > +++ b/drivers/pci/controller/pci-tegra.c > > @@ -2314,8 +2314,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) > > if (PTR_ERR(rp->reset_gpio) == -ENOENT) { > > rp->reset_gpio = NULL; > > } else { > > - dev_err(dev, "failed to get reset GPIO: %d\n", > > - err); > > + dev_err(dev, "failed to get reset GPIO: %ld\n", > > + PTR_ERR(rp->reset_gpio)); > > return PTR_ERR(rp->reset_gpio); > > } > > } > > You can use %pe directly on the pointer for added benefit of translation > of the error to a name. Well, I do not know what is the current preferred style of error messages. On lot of places I see just numeric error numbers. I did not wanted to change behavior of driver, I just fixed code to report correct error value. I do not have this hardware, so I cannot test this change. I just spotted this problem when I was looking at other PCI controller drivers how they issue PERST# signal to endpoint cards.
On Wed, Apr 15, 2020 at 01:17:36PM +0200, Pali Rohár wrote: > On Tuesday 14 April 2020 13:31:04 Michał Mirosław wrote: > > On Tue, Apr 14, 2020 at 12:25:12PM +0200, Pali Rohár wrote: > > > Error code is stored in rp->reset_gpio and not in err variable.a [...] > > > - dev_err(dev, "failed to get reset GPIO: %d\n", > > > - err); > > > + dev_err(dev, "failed to get reset GPIO: %ld\n", > > > + PTR_ERR(rp->reset_gpio)); > > > return PTR_ERR(rp->reset_gpio); > > > } > > > } > > > > You can use %pe directly on the pointer for added benefit of translation > > of the error to a name. > > Well, I do not know what is the current preferred style of error > messages. On lot of places I see just numeric error numbers. %pe is quite a recent addition to the kernel (since v5.3). Best Regards Michał Mirosław
On Tue, 14 Apr 2020 12:25:12 +0200, =?UTF-8?q?Pali=20Roh=C3=A1r?= wrote: > Error code is stored in rp->reset_gpio and not in err variable. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pci/controller/pci-tegra.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Acked-by: Rob Herring <robh@kernel.org>
On Tue, Apr 14, 2020 at 12:25:12PM +0200, Pali Rohár wrote: > Error code is stored in rp->reset_gpio and not in err variable. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pci/controller/pci-tegra.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Applied to pci/tegra, thanks. Lorenzo > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index 0e03cef72840..378d5a8773c7 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -2314,8 +2314,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) > if (PTR_ERR(rp->reset_gpio) == -ENOENT) { > rp->reset_gpio = NULL; > } else { > - dev_err(dev, "failed to get reset GPIO: %d\n", > - err); > + dev_err(dev, "failed to get reset GPIO: %ld\n", > + PTR_ERR(rp->reset_gpio)); > return PTR_ERR(rp->reset_gpio); > } > } > -- > 2.20.1 >
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 0e03cef72840..378d5a8773c7 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -2314,8 +2314,8 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) if (PTR_ERR(rp->reset_gpio) == -ENOENT) { rp->reset_gpio = NULL; } else { - dev_err(dev, "failed to get reset GPIO: %d\n", - err); + dev_err(dev, "failed to get reset GPIO: %ld\n", + PTR_ERR(rp->reset_gpio)); return PTR_ERR(rp->reset_gpio); } }
Error code is stored in rp->reset_gpio and not in err variable. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/controller/pci-tegra.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)