Messages in this thread | | | Subject | Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs | From | Scott Wood <> | Date | Tue, 16 Sep 2014 14:58:14 -0500 |
| |
On Tue, 2014-09-16 at 14:28 -0500, Kim Phillips wrote: > On Mon, 15 Sep 2014 23:31:21 -0500 > Scott Wood <scottwood@freescale.com> wrote: > > > On Mon, 2014-09-15 at 18:44 -0500, Kim Phillips wrote: > > > On Thu, 11 Sep 2014 12:34:21 -0500 > > > "J. German Rivera" <German.Rivera@freescale.com> wrote: > > > > > > > diff --git a/drivers/bus/fsl-mc/fsl_mc_sys.c b/drivers/bus/fsl-mc/fsl_mc_sys.c > > > > > > > +/** > > > > + * Map an MC portal in the kernel virtual address space > > > > + */ > > > > +static int map_mc_portal(phys_addr_t mc_portal_phys_addr, > > > > + uint32_t mc_portal_size, > > > > + void __iomem **new_mc_portal_virt_addr) > > > > +{ > > > > + void __iomem *mc_portal_virt_addr = NULL; > > > > + struct resource *res = NULL; > > > > + int error = -EINVAL; > > > > + > > > > + res = > > > > + request_mem_region(mc_portal_phys_addr, mc_portal_size, > > > > + "mc_portal"); > > > > + if (res == NULL) { > > > > + pr_err("request_mem_region() failed for MC portal %#llx\n", > > > > + mc_portal_phys_addr); > > > > + error = -EBUSY; > > > > + goto error; > > > > + } > > > > + > > > > + mc_portal_virt_addr = ioremap_nocache(mc_portal_phys_addr, > > > > + mc_portal_size); > > > > + if (mc_portal_virt_addr == NULL) { > > > > + pr_err("ioremap_nocache() failed for MC portal %#llx\n", > > > > + mc_portal_phys_addr); > > > > + error = -EFAULT; > > > > + goto error; > > > > + } > > > > + > > > > + *new_mc_portal_virt_addr = mc_portal_virt_addr; > > > > + return 0; > > > > +error: > > > > + if (mc_portal_virt_addr != NULL) > > > > + iounmap(mc_portal_virt_addr); > > > > + > > > > + if (res != NULL) > > > > + release_mem_region(mc_portal_phys_addr, mc_portal_size); > > > > + > > > > + return error; > > > > +} > > > > > > unnecessary initializations, bad error codes (both should be > > > -ENOMEM), > > > > Why should the first one be -ENOMEM? It's not allocating memory, but > > rather reserving I/O space. > > I was going with what most of the drivers are already doing, but I > see EFAULT is 'Bad address', which, you're right, is probably more > appropriate.
EFAULT is for userspace addresses that fault.
I don't see consistency on what other drivers use after request_mem_region() -- some use ENOMEM, some ENODEV, some EBUSY... and probably others.
I'd probably go with ENXIO or EBUSY.
-Scott
| |