Messages in this thread | | | Subject | Re: Duplicate calls to regmap_debugfs_init() through regmap_attach_dev() | From | Matthias Schiffer <> | Date | Fri, 23 Jul 2021 09:34:26 +0200 |
| |
On Mon, 2021-07-19 at 15:53 +0200, Matthias Schiffer wrote: > Hi everyone, > > I hope I got the right list of maintainers for this issue, which seems > to be rooted in the interaction between regmap, syscon and pinctrl-imx. > > With recent kernels (observed on v5.10.y, but the code doesn't look > significantly different on master/next) I've seen the following message > on boot on i.MX6UL SoCs: > > > debugfs: Directory 'dummy-iomuxc-gpr@20e4000' with parent 'regmap' already present! > > I've tracked this down to this piece of code in the pinctrl-imx driver: > > > gpr = syscon_regmap_lookup_by_compatible(info->gpr_compatible); > > if (!IS_ERR(gpr)) > > regmap_attach_dev(&pdev->dev, gpr, &config); > > __regmap_init() (called by syscon_regmap_lookup_by_compatible()) has: > > > if (dev) { > > ret = regmap_attach_dev(dev, map, config); > > if (ret != 0) > > goto err_regcache; > > } else { > > regmap_debugfs_init(map); > > } > > As dev is NULL in this call, regmap_debugfs_init() will be called. > > pinctrl-imx then calls regmap_attach_dev(), which calls > regmap_debugfs_init() again. Unless I'm missing something, this is very > problematic: regmap_debugfs_init() does a lot more than just adding > debugfs files - it also initializes list heads and mutices in the > regmap structure. > > It seems to me that there is no correct way to use regmap_attach_dev() > from outside of __regmap_init(). In particular on a syscon regmap that > may be shared between different drivers, setting map->dev looks wrong > to me. > > The total number of drivers that call regmap_attach_dev() is very low > (I count 5), but all of them use it on a syscon regmap. Some of them > perform further operations on the regmap as if they owned it, like > modifying the cache configuration. > > While not directly related, could anyone tell me why the locking around > syscon_list in the syscon driver is correct (or if it is in fact > incorrect)? It looks to me like two tasks might call > device_node_get_regmap() at the same time, leading to two concurrent > constructions of the same syscon regmap. > > Kind regards, > Matthias
Another question regarding the syscon driver: Does the syscon platform device still have any use after bdb0066df96e ("mfd: syscon: Decouple syscon interface from platform devices")? All exported syscon functions use the regmaps stored in the global "syscon_list", which are compeletely independent of the devices handled by syscon_probe().
As the syscon platform_driver doesn't do anything, it seems to me like that part could just be removed, leaving only the handling of shared regmaps. Maybe that code should live under drivers/base/regmap instead of drivers/mfd?
| |