Received-SPF: pass (google.com: domain of linux-kernel+bounces-182044-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 17 May 2024 16:50:40 +0530
From: Siddharth Vadapalli <s-vadapalli@ti.com>
To: Onkarnarth <onkarnath.1@samsung.com>
CC: <bhelgaas@google.com>, <vigneshr@ti.com>, <s-vadapalli@ti.com>,
        <lpieralisi@kernel.org>, <kw@linux.com>, <robh@kernel.org>,
        <yue.wang@Amlogic.com>, <neil.armstrong@linaro.org>,
        <khilman@baylibre.com>, <jbrunet@baylibre.com>,
        <martin.blumenstingl@googlemail.com>, <thomas.petazzoni@bootlin.com>,
        <shawn.guo@linaro.org>, <lchuanhua@maxlinear.com>,
        <srikanth.thokala@intel.com>, <songxiaowei@hisilicon.com>,
        <wangbinghui@hisilicon.com>, <manivannan.sadhasivam@linaro.org>,
        <thierry.reding@gmail.com>, <jonathanh@nvidia.com>,
        <hayashi.kunihiko@socionext.com>, <mhiramat@kernel.org>,
        <pali@kernel.org>, <toan@os.amperecomputing.com>,
        <daire.mcnamara@microchip.com>, <conor.dooley@microchip.com>,
        <marek.vasut+renesas@gmail.com>, <shawn.lin@rock-chips.com>,
        <heiko@sntech.de>, <nirmal.patel@linux.intel.com>,
        <jonathan.derrick@linux.dev>, <kishon@kernel.org>, <jdmason@kudzu.us>,
        <dave.jiang@intel.com>, <rafael@kernel.org>, <lenb@kernel.org>,
        <mahesh@linux.ibm.com>, <oohall@gmail.com>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-omap@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        <linux-amlogic@lists.infradead.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-tegra@vger.kernel.org>, <r.thapliyal@samsung.com>,
        Maninder Singh <maninder1.s@samsung.com>
Subject: Re: [PATCH 1/1] PCI : Refactoring error log prints for better
 readability
Message-ID: <4ec3b167-9324-41d3-a086-74ca001b9042@ti.com>
References: <CGME20240517105941epcas5p3e8dbb97f19c9553bf9942ad146124806@epcas5p3.samsung.com>
 <20240517105923.2406246-1-onkarnath.1@samsung.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240517105923.2406246-1-onkarnath.1@samsung.com>

On Fri, May 17, 2024 at 04:29:23PM +0530, Onkarnarth wrote:
> From: Onkarnath <onkarnath.1@samsung.com>

I think that the $subject should be similar to:
https://patchwork.kernel.org/comment/25712288/
rather than the generic "refactoring error log".

> 
> As %pe is already introduced, it's better to use it in place of (%ld) or (%d) for
> printing error in logs. It will enhance readability of logs.
> 
> Error print style is more consistent now.
> 
> Co-developed-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
> ---
> Suggested by Bjorn Helgaas in below discussion
> https://patchwork.kernel.org/comment/25712288/

Since you have indicated that Bjorn has suggested it, shouldn't there
also be a "Suggested-by tag" in the commit message?

> 
>  drivers/pci/bus.c                             |   2 +-
>  drivers/pci/controller/dwc/pci-dra7xx.c       |   2 +-
>  drivers/pci/controller/dwc/pci-meson.c        |  16 +--
>  drivers/pci/controller/dwc/pcie-armada8k.c    |   4 +-
>  drivers/pci/controller/dwc/pcie-histb.c       |   6 +-
>  drivers/pci/controller/dwc/pcie-intel-gw.c    |  10 +-
>  drivers/pci/controller/dwc/pcie-keembay.c     |   2 +-
>  drivers/pci/controller/dwc/pcie-kirin.c       |   6 +-
>  drivers/pci/controller/dwc/pcie-qcom-ep.c     |  18 +--
>  drivers/pci/controller/dwc/pcie-qcom.c        |  18 +--
>  drivers/pci/controller/dwc/pcie-tegra194.c    | 132 +++++++++---------
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c |   2 +-
>  drivers/pci/controller/pci-aardvark.c         |   6 +-
>  drivers/pci/controller/pci-ftpci100.c         |   2 +-
>  drivers/pci/controller/pci-tegra.c            |  86 ++++++------
>  drivers/pci/controller/pci-xgene.c            |   4 +-
>  drivers/pci/controller/pcie-microchip-host.c  |   2 +-
>  drivers/pci/controller/pcie-rcar-host.c       |  14 +-
>  drivers/pci/controller/pcie-rockchip.c        |  34 ++---
>  drivers/pci/controller/vmd.c                  |   2 +-
>  drivers/pci/doe.c                             |   4 +-
>  drivers/pci/endpoint/functions/pci-epf-mhi.c  |   8 +-
>  drivers/pci/endpoint/functions/pci-epf-ntb.c  |   2 +-
>  drivers/pci/endpoint/functions/pci-epf-test.c |   4 +-
>  drivers/pci/endpoint/functions/pci-epf-vntb.c |   2 +-
>  drivers/pci/endpoint/pci-ep-cfs.c             |  12 +-
>  drivers/pci/endpoint/pci-epf-core.c           |  16 +--
>  drivers/pci/hotplug/acpiphp_core.c            |   2 +-
>  drivers/pci/hotplug/pciehp_core.c             |   8 +-
>  drivers/pci/hotplug/shpchp_core.c             |   4 +-
>  drivers/pci/of.c                              |   6 +-
>  drivers/pci/pci-driver.c                      |   4 +-
>  drivers/pci/pcie/dpc.c                        |   4 +-
>  drivers/pci/quirks.c                          |   2 +-
>  drivers/pci/setup-bus.c                       |   2 +-
>  drivers/pci/slot.c                            |   4 +-
>  drivers/pci/vgaarb.c                          |   2 +-
>  37 files changed, 227 insertions(+), 227 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 826b5016a101..dbc16cf5a246 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -351,7 +351,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>  	dev->match_driver = !dn || of_device_is_available(dn);
>  	retval = device_attach(&dev->dev);
>  	if (retval < 0 && retval != -EPROBE_DEFER)
> -		pci_warn(dev, "device attach failed (%d)\n", retval);
> +		pci_warn(dev, "device attach failed: %pe\n", ERR_PTR(retval));

Is there a reason behind dropping the parantheses?

>  
>  	pci_dev_assign_added(dev, true);
>  }
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index d2d17d37d3e0..79b6cc7f0287 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -801,7 +801,7 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
>  	reset = devm_gpiod_get_optional(dev, NULL, GPIOD_OUT_HIGH);
>  	if (IS_ERR(reset)) {
>  		ret = PTR_ERR(reset);
> -		dev_err(&pdev->dev, "gpio request failed, ret %d\n", ret);
> +		dev_err(&pdev->dev, "gpio request failed: %pe\n", ERR_PTR(ret));

Similar question as above regarding converting "failed, ret" to
"failed:". Is this a new convention that is expected to be followed,
where all errors are supposed to have "failed: %pe", rather than custom
statements? Please let me know if this has already been discussed
elsewhere.

[...]

Regards,
Siddharth.