Re: [PATCH] regulator: core: Avoid debugfs: Directory ... already present! error
On Fri, 22 Jan 2021 19:32:50 +0100, Hans de Goede wrote: > Sometimes regulator_get() gets called twice for the same supply on the > same device. This may happen e.g. when a framework / library is used > which uses the regulator; and the driver itself also needs to enable > the regulator in some cases where the framework will not enable it. > > Commit ff268b56ce8c ("regulator: core: Don't spew backtraces on > duplicate sysfs") already takes care of the backtrace which would > trigger when creating a duplicate consumer symlink under > /sys/class/regulator/regulator.%d in this scenario. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [1/1] regulator: core: Avoid debugfs: Directory ... already present! error commit: dbe954d8f1635f949a1d9a5d6e6fb749ae022b47 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH] regulator: core: Avoid debugfs: Directory ... already present! error
Hi, On 1/23/21 11:29 AM, Charles Keepax wrote: > On Fri, Jan 22, 2021 at 07:32:50PM +0100, Hans de Goede wrote: >> Sometimes regulator_get() gets called twice for the same supply on the >> same device. This may happen e.g. when a framework / library is used >> which uses the regulator; and the driver itself also needs to enable >> the regulator in some cases where the framework will not enable it. >> >> Commit ff268b56ce8c ("regulator: core: Don't spew backtraces on >> duplicate sysfs") already takes care of the backtrace which would >> trigger when creating a duplicate consumer symlink under >> /sys/class/regulator/regulator.%d in this scenario. >> >> Commit c33d442328f5 ("debugfs: make error message a bit more verbose") >> causes a new error to get logged in this scenario: >> >> [ 26.938425] debugfs: Directory 'wm5102-codec-MICVDD' with parent >> 'spi-WM510204:00-MICVDD' already present! >> >> There is no _nowarn variant of debugfs_create_dir(), but we can detect >> and avoid this problem by checking the return value of the earlier >> sysfs_create_link_nowarn() call. >> >> Add a check for the earlier sysfs_create_link_nowarn() failing with >> -EEXIST and skip the debugfs_create_dir() call in that case, avoiding >> this error getting logged. >> >> Fixes: c33d442328f5 ("debugfs: make error message a bit more verbose") >> Cc: Charles Keepax >> Signed-off-by: Hans de Goede >> --- > > Reviewed-by: Charles Keepax > > Thanks, > Charles > >> -int err; >> +int err = 0; >> >> @@ -1663,8 +1663,8 @@ static struct regulator *create_regulator(struct >> regulator_dev *rdev, >> >> -regulator->debugfs = debugfs_create_dir(supply_name, >> -rdev->debugfs); >> +if (err != -EEXIST) >> +regulator->debugfs = debugfs_create_dir(supply_name, >> rdev->debugfs); > > There is a slight oddity here in that if this regulator has > no struct device we will still get the warning. However, I > am totally not clear on when/why a regulator might not have a > dev, and am fairly sure it isn't common. So my vote would be > to cross that bridge if we ever come to it. Yes, I expect the combination of having 2 consumers which both get the regulator with a NULL device pointer to be very rare and hopefully it does not happen at all. Regards, Hans
Re: [PATCH] regulator: core: Avoid debugfs: Directory ... already present! error
On Fri, Jan 22, 2021 at 07:32:50PM +0100, Hans de Goede wrote: > Sometimes regulator_get() gets called twice for the same supply on the > same device. This may happen e.g. when a framework / library is used > which uses the regulator; and the driver itself also needs to enable > the regulator in some cases where the framework will not enable it. > > Commit ff268b56ce8c ("regulator: core: Don't spew backtraces on > duplicate sysfs") already takes care of the backtrace which would > trigger when creating a duplicate consumer symlink under > /sys/class/regulator/regulator.%d in this scenario. > > Commit c33d442328f5 ("debugfs: make error message a bit more verbose") > causes a new error to get logged in this scenario: > > [ 26.938425] debugfs: Directory 'wm5102-codec-MICVDD' with parent > 'spi-WM510204:00-MICVDD' already present! > > There is no _nowarn variant of debugfs_create_dir(), but we can detect > and avoid this problem by checking the return value of the earlier > sysfs_create_link_nowarn() call. > > Add a check for the earlier sysfs_create_link_nowarn() failing with > -EEXIST and skip the debugfs_create_dir() call in that case, avoiding > this error getting logged. > > Fixes: c33d442328f5 ("debugfs: make error message a bit more verbose") > Cc: Charles Keepax > Signed-off-by: Hans de Goede > --- Reviewed-by: Charles Keepax Thanks, Charles > - int err; > + int err = 0; > > @@ -1663,8 +1663,8 @@ static struct regulator *create_regulator(struct > regulator_dev *rdev, > > - regulator->debugfs = debugfs_create_dir(supply_name, > - rdev->debugfs); > + if (err != -EEXIST) > + regulator->debugfs = debugfs_create_dir(supply_name, > rdev->debugfs); There is a slight oddity here in that if this regulator has no struct device we will still get the warning. However, I am totally not clear on when/why a regulator might not have a dev, and am fairly sure it isn't common. So my vote would be to cross that bridge if we ever come to it. Thanks, Charles
[PATCH] regulator: core: Avoid debugfs: Directory ... already present! error
Sometimes regulator_get() gets called twice for the same supply on the same device. This may happen e.g. when a framework / library is used which uses the regulator; and the driver itself also needs to enable the regulator in some cases where the framework will not enable it. Commit ff268b56ce8c ("regulator: core: Don't spew backtraces on duplicate sysfs") already takes care of the backtrace which would trigger when creating a duplicate consumer symlink under /sys/class/regulator/regulator.%d in this scenario. Commit c33d442328f5 ("debugfs: make error message a bit more verbose") causes a new error to get logged in this scenario: [ 26.938425] debugfs: Directory 'wm5102-codec-MICVDD' with parent 'spi-WM510204:00-MICVDD' already present! There is no _nowarn variant of debugfs_create_dir(), but we can detect and avoid this problem by checking the return value of the earlier sysfs_create_link_nowarn() call. Add a check for the earlier sysfs_create_link_nowarn() failing with -EEXIST and skip the debugfs_create_dir() call in that case, avoiding this error getting logged. Fixes: c33d442328f5 ("debugfs: make error message a bit more verbose") Cc: Charles Keepax Signed-off-by: Hans de Goede --- drivers/regulator/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index ca03d8e70bd1..75ec6f334506 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1617,7 +1617,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, const char *supply_name) { struct regulator *regulator; - int err; + int err = 0; if (dev) { char buf[REG_STR_SIZE]; @@ -1663,8 +1663,8 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, } } - regulator->debugfs = debugfs_create_dir(supply_name, - rdev->debugfs); + if (err != -EEXIST) + regulator->debugfs = debugfs_create_dir(supply_name, rdev->debugfs); if (!regulator->debugfs) { rdev_dbg(rdev, "Failed to create debugfs directory\n"); } else { -- 2.28.0