linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To: Mark Brown <broonie@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: linux-kernel@vger.kernel.org, Dong Aisheng <aisheng.dong@nxp.com>,
	Fabio Estevam <festevam@gmail.com>,
	Shawn Guo <shawnguo@kernel.org>, Stefan Agner <stefan@agner.ch>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-arm-kernel@lists.infradead.org,
	Lee Jones <lee.jones@linaro.org>, Arnd Bergmann <arnd@arndb.de>
Subject: Duplicate calls to regmap_debugfs_init() through regmap_attach_dev()
Date: Mon, 19 Jul 2021 15:53:38 +0200	[thread overview]
Message-ID: <eb27b79ce46bde0202a4e7b047a3aaec8338fb6d.camel@ew.tq-group.com> (raw)

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


             reply	other threads:[~2021-07-19 13:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-19 13:53 Matthias Schiffer [this message]
2021-07-23  7:34 ` Duplicate calls to regmap_debugfs_init() through regmap_attach_dev() Matthias Schiffer
2021-07-23 14:06 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eb27b79ce46bde0202a4e7b047a3aaec8338fb6d.camel@ew.tq-group.com \
    --to=matthias.schiffer@ew.tq-group.com \
    --cc=aisheng.dong@nxp.com \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=stefan@agner.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).