On Monday 18 February 2013, Alexander Shiyan wrote: > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > index 4a7ed5a..3c0abcb 100644 > --- a/drivers/mfd/syscon.c > +++ b/drivers/mfd/syscon.c
Hi Alexander, > struct regmap *syscon_regmap_lookup_by_compatible(const char *s) > { > struct device_node *syscon_np; > struct regmap *regmap; > + struct syscon *syscon; > + struct device *dev; > > syscon_np = of_find_compatible_node(NULL, NULL, s); > - if (!syscon_np) > + if (syscon_np) { > + regmap = syscon_node_to_regmap(syscon_np); > + of_node_put(syscon_np); > + > + return regmap; > + } > + > + /* Fallback to search by id_entry.name string */ > + dev = driver_find_device(&syscon_driver.driver, NULL, (void *)s, > + syscon_match_id); > + if (!dev) > return ERR_PTR(-ENODEV); > > - regmap = syscon_node_to_regmap(syscon_np); > - of_node_put(syscon_np); > + syscon = dev_get_drvdata(dev); > > - return regmap; > + return syscon->regmap; > } > EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_compatible); Since you are not actually comparing the "compatible" property here, I would suggest adding another function here, something like syscon_regmap_lookup_by_pdevname() that you can use in device drivers that are not DT-enabled. I would also recommend enclosing that function in #ifdef CONFIG_ATAGS. Which code do you have in mind that would call this anyway? The changeset description does not really explain what ATAG boot support in syscon is good for. > + if (IS_ENABLED(CONFIG_OF) && np) { > + syscon->base = of_iomap(np, 0); > + if (!syscon->base) > + return -EADDRNOTAVAIL; > + > + res = &res_of; > + ret = of_address_to_resource(np, 0, res); > + if (ret) { > + iounmap(syscon->base); > + return ret; > + } > + } else { > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENOENT; > + > + if (!devm_request_mem_region(dev, res->start, > + resource_size(res), > + dev_name(&pdev->dev))) > + return -EBUSY; > + > + syscon->base = ioremap(res->start, resource_size(res)); > + if (!syscon->base) > + return -EADDRNOTAVAIL; > + } These two code paths look equivalent. Why not always use the second one? Also, you might want to convert this to devm_ioremap_resource() to simplify the code in the process. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/