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
Date: Wed, 27 May 2020 21:20:05 +0800 [thread overview]
Message-ID: <20200527132005.GA7143@nuc8i5> (raw)
In-Reply-To: <1d7703d5c29dc9371ace3645377d0ddd9c89be30.camel@amazon.com>
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
next prev parent reply other threads:[~2020-05-27 13:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-26 15:09 [PATCH v1] PCI: controller: Remove duplicate error message Dejin Zheng
2020-05-26 18:22 ` Chocron, Jonathan
2020-05-27 13:20 ` Dejin Zheng [this message]
2020-06-02 7:44 ` Chocron, Jonathan
2020-06-02 15:01 ` Rob Herring
2020-07-06 15:58 ` Lorenzo Pieralisi
2020-07-11 7:47 ` Dejin Zheng
2020-07-15 16:45 ` Lorenzo Pieralisi
2020-07-17 13:44 ` Dejin Zheng
2020-06-01 22:13 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200527132005.GA7143@nuc8i5 \
--to=zhengdejin5@gmail.com \
--cc=bhelgaas@google.com \
--cc=jonnyc@amazon.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=pratyush.anand@gmail.com \
--cc=robh@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
--cc=tjoseph@cadence.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).