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;
Date:   Wed, 27 May 2020 21:20:05 +0800
From:   Dejin Zheng <zhengdejin5@gmail.com>
To:     "Chocron, Jonathan" <jonnyc@amazon.com>
Cc:     "lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
        "thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
        "pratyush.anand@gmail.com" <pratyush.anand@gmail.com>,
        "robh@kernel.org" <robh@kernel.org>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "bhelgaas@google.com" <bhelgaas@google.com>,
        "tjoseph@cadence.com" <tjoseph@cadence.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] PCI: controller: Remove duplicate error message
Message-ID: <20200527132005.GA7143@nuc8i5>
References: <20200526150954.4729-1-zhengdejin5@gmail.com>
 <1d7703d5c29dc9371ace3645377d0ddd9c89be30.camel@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1d7703d5c29dc9371ace3645377d0ddd9c89be30.camel@amazon.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, May 26, 2020 at 06:22:56PM +0000, Chocron, Jonathan wrote:
> On Tue, 2020-05-26 at 23:09 +0800, Dejin Zheng wrote:
> > CAUTION: This email originated from outside of the organization. Do
> > not click links or open attachments unless you can confirm the sender
> > and know the content is safe.
> > 
> > 
> > 
> > It will print an error message by itself when
> > devm_pci_remap_cfg_resource() goes wrong. so remove the duplicate
> > error message.
> > 
> 
> It seems like that in the first error case in
> devm_pci_remap_cfg_resource(), the print will be less indicative. Could
> you please share an example print log with the duplicate print?
>
Hi Jonathan:

Thank you very much for using your precious time to review my patch.

I did not have this log and just found it by review codes. the function
of devm_pci_remap_cfg_resource() is designed to handle error messages by
itself. and Its recommended usage is as follows in the function description
	
	base = devm_pci_remap_cfg_resource(&pdev->dev, res);
	if (IS_ERR(base))
		return PTR_ERR(base);

In fact, I think its error handling is clear enough, It just goes wrong
in three places, as follows:

void __iomem *devm_pci_remap_cfg_resource(struct device *dev,
                                          struct resource *res)
{
        resource_size_t size;
        const char *name;
        void __iomem *dest_ptr;

        BUG_ON(!dev);

        if (!res || resource_type(res) != IORESOURCE_MEM) {
                dev_err(dev, "invalid resource\n");
                return IOMEM_ERR_PTR(-EINVAL);
        }

        size = resource_size(res);
        name = res->name ?: dev_name(dev);

        if (!devm_request_mem_region(dev, res->start, size, name)) {
                dev_err(dev, "can't request region for resource %pR\n", res);
                return IOMEM_ERR_PTR(-EBUSY);
        }

        dest_ptr = devm_pci_remap_cfgspace(dev, res->start, size);
        if (!dest_ptr) {
                dev_err(dev, "ioremap failed for resource %pR\n", res);
                devm_release_mem_region(dev, res->start, size);
                dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
        }

        return dest_ptr;
}

BR,
Dejin

> Thanks,
>    Jonathan