Dear Jingoo Han, On Tue, 17 Sep 2013 09:52:06 +0900, Jingoo Han wrote: > 'Just returning NULL when of_address_to_resource() fails > instead of trying to propagate the error' looks better. > > Then, how about the following? > > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -728,7 +728,7 @@ mvebu_pcie_map_registers(struct platform_device *pdev, > > ret = of_address_to_resource(np, 0, ®s); > if (ret) > - return ERR_PTR(ret); > + return NULL; > > return devm_ioremap_resource(&pdev->dev, ®s); > } > @@ -874,7 +874,7 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev) > } > > port->base = mvebu_pcie_map_registers(pdev, child, port); > - if (IS_ERR(port->base)) { > + if (!port->base) { > dev_err(&pdev->dev, "PCIe%d.%d: cannot map registers\n", > port->port, port->lane); > port->base = NULL; Yes, I believe it makes more sense since we were anyway to using the error code returned by of_address_to_ressource(). However, you can get rid of the port->base = NULL assignment in the if () { ... } code now. With that fixed, you can add my: Acked-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html