> > ... > >> >> >> >> 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, > >> >> >> > >> >> >> Yes, i also think like that. > >> >> > > >> >> > In this case we should provide two paths for drivers which can work as with DT > >> >> > and without DT. > >> >> > >> >> Yes. > >> > > >> > I still think the universal procedure is better for the driver. > >> > > >> > >> Why? > >> I did not see your reply on my other comments on the problems of using universal > >> procedure? > >> Please let me know if you think they're not issues. > > > > Yes, I do not see a problem here. > > I will try to show the code. > > > > In the driver: > > struct regmap *r; > > r = syscon_regmap_lookup_by_compatible("my_super_syscon"); > > > > For DT case: > > syscon@123456 { > > compatible = "my_super_syscon", "syscon"; > > ... > > }; > > > > For non-DT case: > > struct platform_device_id id = { .name = "my_super_syscon", }; > > struct platform_device syscon_pdev = { > > .name = "syscon", > > .id_entry = &id, > > This is really strange to me and i've never seen such using. > Usually the id_table is provided by the driver and the match process then will > set the correct id_entry for the platform device once it matches. > Please see the platform_bus match process: drivers/base/platform.c Yes, this is non-standard usage. Currently we do not have platform_device_id entries for a "syscon" driver, so this field is untouched for a platform device and we can use it. At least this works, but, of course, more experts should fix me on this question if I think incorrect. > > > ... > > }; > > platform_device_register(&syscon_pdev); > > > > Do I understand what you mean? > > > > My understanding for non-dt case is something like: > In client driver: > struct regmap *r; > r = syscon_regmap_lookup_by_pdevname("my_super_syscon"); > > In board code: > struct platform_device syscon_pdev = { > .name = "my_super_syscon", > ... > }; > platform_device_register(&syscon_pdev); > > In syscon driver: > static struct platform_device_id syscon_device_ids[] = { > { > .name = "my_super_syscon", > }, { > /* sentinel */ > } > }; > MODULE_DEVICE_TABLE(platform, syscon_device_ids); > > static struct platform_driver syscon_driver = { > .driver = { > .name = "syscon", > .owner = THIS_MODULE, > .of_match_table = of_syscon_match, > }, > .id_table = syscon_device_ids, > .probe = syscon_probe, > .remove = syscon_remove, > }; > > One problem is that every user needs to add their syscon compatible device > support(platform_device_id) in syscon driver first before they can use it. > But it looks to me just like how other driver generally does. I agree, this is a more proper way, but in this case we should create a big table here with records for each device... > Arnd, > Do you think this is an issue? --- {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I