We are now allowing to register debugfs for syscon regmap, and not
having a valid name will end up using "dummy" to create debugfs dir.
Fixes: 9b947a13e7f6 ("regmap: use debugfs even when no device")
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/mfd/syscon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 7eaa40bc703f..250d22f40c84 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -109,6 +109,7 @@ static struct syscon *of_syscon_register(struct device_node *np)
syscon_config.reg_stride = reg_io_width;
syscon_config.val_bits = reg_io_width * 8;
syscon_config.max_register = resource_size(&res) - reg_io_width;
+ syscon_config.name = of_node_full_name(np);
regmap = regmap_init_mmio(NULL, base, &syscon_config);
if (IS_ERR(regmap)) {
--
2.11.0
Use map->debugfs_name to store allocated debugfs name, so it would be
freed in regmap_debugfs_exit().
Kmemleak reported:
unreferenced object 0xffffffc0cf78cf00 (size 128):
comm "swapper/0", pid 1, jiffies 4294669168 (age 89.152s)
hex dump (first 32 bytes):
64 75 6d 6d 79 32 33 00 00 00 00 00 00 00 00 00 dummy23.........
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<0000000027429160>] kmemleak_alloc+0x58/0x8c
[<0000000081861ddf>] __kmalloc_track_caller+0x1dc/0x2bc
[<00000000258a341f>] kvasprintf+0xd0/0x168
[<00000000f3243f27>] kasprintf+0xac/0xd8
[<000000005f585642>] regmap_debugfs_init+0x1a0/0x3b0
[<000000002c08b110>] __regmap_init+0x12b8/0x135c
[<00000000f64bcddb>] __regmap_init_mmio_clk+0x70/0x84
[<000000009c8f06b5>] of_syscon_register+0x278/0x378
[<0000000087e6c121>] syscon_node_to_regmap+0x80/0xb0
Fixes: a430ab205d29 ("regmap: debugfs: Disambiguate dummy debugfs file name")
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/base/regmap/regmap-debugfs.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index e3e7b91cc421..5479a183248f 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -570,15 +570,15 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
devname, name);
name = map->debugfs_name;
+ } else if (!map->dev) {
+ map->debugfs_name = kasprintf(GFP_KERNEL, "%s%d",
+ devname, dummy_index);
+ name = map->debugfs_name;
+ dummy_index++;
} else {
name = devname;
}
- if (!strcmp(name, "dummy")) {
- name = kasprintf(GFP_KERNEL, "dummy%d", dummy_index);
- dummy_index++;
- }
-
map->debugfs = debugfs_create_dir(name, regmap_debugfs_root);
if (!map->debugfs) {
dev_warn(map->dev, "Failed to create debugfs directory\n");
--
2.11.0
Free map->debugfs_name when debugfs_create_dir() failed to avoid memory
leak.
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/base/regmap/regmap-debugfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 5479a183248f..55e862a81e82 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -582,6 +582,9 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
map->debugfs = debugfs_create_dir(name, regmap_debugfs_root);
if (!map->debugfs) {
dev_warn(map->dev, "Failed to create debugfs directory\n");
+
+ kfree(map->debugfs_name);
+ map->debugfs_name = NULL;
return;
}
--
2.11.0
On Tue, Mar 06, 2018 at 07:04:02PM +0800, Jeffy Chen wrote:
> Use map->debugfs_name to store allocated debugfs name, so it would be
> freed in regmap_debugfs_exit().
I'm missing patch 1 in this series and I think this collides with a fix
I already have locally.
+CC Mark.
even this is already fixed by a430ab205d29 ("regmap: debugfs:
Disambiguate dummy debugfs file name")
but maybe we can still have this for a better debugfs name?
On 03/06/2018 07:04 PM, Jeffy Chen wrote:
> We are now allowing to register debugfs for syscon regmap, and not
> having a valid name will end up using "dummy" to create debugfs dir.
>
> Fixes: 9b947a13e7f6 ("regmap: use debugfs even when no device")
> Signed-off-by: Jeffy Chen <[email protected]>
> ---
>
> drivers/mfd/syscon.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 7eaa40bc703f..250d22f40c84 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -109,6 +109,7 @@ static struct syscon *of_syscon_register(struct device_node *np)
> syscon_config.reg_stride = reg_io_width;
> syscon_config.val_bits = reg_io_width * 8;
> syscon_config.max_register = resource_size(&res) - reg_io_width;
> + syscon_config.name = of_node_full_name(np);
>
> regmap = regmap_init_mmio(NULL, base, &syscon_config);
> if (IS_ERR(regmap)) {
>
On Tue, Mar 06, 2018 at 10:58:26PM +0800, JeffyChen wrote:
> even this is already fixed by a430ab205d29 ("regmap: debugfs: Disambiguate
> dummy debugfs file name")
> but maybe we can still have this for a better debugfs name?
That's why the facility is there.
Please don't top post, reply in line with needed context. This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.
Hi Mark,
On 03/07/2018 06:20 PM, Mark Brown wrote:
> On Tue, Mar 06, 2018 at 10:58:26PM +0800, JeffyChen wrote:
>
>> even this is already fixed by a430ab205d29 ("regmap: debugfs: Disambiguate
>> dummy debugfs file name")
>
>> but maybe we can still have this for a better debugfs name?
>
> That's why the facility is there.
>
> Please don't top post, reply in line with needed context. This allows
> readers to readily follow the flow of conversation and understand what
> you are talking about and also helps ensure that everything in the
> discussion is being addressed.
>
ok, sorry for that :)
recently we are now allowing to register debugfs for syscon regmap, then
i hit a lot of:
(NULL device *): Failed to create debugfs directory
so i sent this patch to fix that by using the of_node name to create
debugfs.
but later i found we have:
commit a430ab205d29e7d1537b220fcf989b8080d8267f
Author: Fabio Estevam <[email protected]>
Date: Mon Mar 5 15:52:09 2018 -0300
regmap: debugfs: Disambiguate dummy debugfs file name
which would fix that in another way by naming dummy0, dummy1, dummy2.
even if the issue is fixed, i wonder could we still have this patch to
make it more readable:
+++ b/drivers/mfd/syscon.c
@@ -109,6 +109,7 @@ static struct syscon *of_syscon_register(struct
device_node *np)
syscon_config.reg_stride = reg_io_width;
syscon_config.val_bits = reg_io_width * 8;
syscon_config.max_register = resource_size(&res) - reg_io_width;
+ syscon_config.name = of_node_full_name(np);
and i've sent 2 more patches to fix the memleak there:
https://patchwork.kernel.org/patch/10261409
https://patchwork.kernel.org/patch/10261411
and sorry again, i forgot to add a cover letter, and i should CC you the
whole series.
Hi Mark,
Thanks for your reply.
On 03/06/2018 08:59 PM, Mark Brown wrote:
> On Tue, Mar 06, 2018 at 07:04:02PM +0800, Jeffy Chen wrote:
>> Use map->debugfs_name to store allocated debugfs name, so it would be
>> freed in regmap_debugfs_exit().
>
> I'm missing patch 1 in this series and I think this collides with a fix
> I already have locally.
>
sorry for that, i should CC you the whole seires.
the patch 1 should be:
https://patchwork.kernel.org/patch/10261413
On Wed, Mar 07, 2018 at 06:50:08PM +0800, JeffyChen wrote:
> so i sent this patch to fix that by using the of_node name to create
> debugfs.
> but later i found we have:
> commit a430ab205d29e7d1537b220fcf989b8080d8267f
> Author: Fabio Estevam <[email protected]>
> Date: Mon Mar 5 15:52:09 2018 -0300
> regmap: debugfs: Disambiguate dummy debugfs file name
> which would fix that in another way by naming dummy0, dummy1, dummy2.
> even if the issue is fixed, i wonder could we still have this patch to make
> it more readable:
Yes, like I say I think that's a good idea anyway - that sort of usage
is the reason we've got the facility to set an explicit name.
The patch
regmap: debugfs: Free map->debugfs_name when debugfs_create_dir() failed
has been applied to the regmap tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
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
From 17cf46cfe975f1dd04db6bd38398923846512d49 Mon Sep 17 00:00:00 2001
From: Jeffy Chen <[email protected]>
Date: Tue, 6 Mar 2018 19:04:03 +0800
Subject: [PATCH] regmap: debugfs: Free map->debugfs_name when
debugfs_create_dir() failed
Free map->debugfs_name when debugfs_create_dir() failed to avoid memory
leak.
Signed-off-by: Jeffy Chen <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/base/regmap/regmap-debugfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index dd3a16894e3c..c84f5ceb015a 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -584,6 +584,9 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
map->debugfs = debugfs_create_dir(name, regmap_debugfs_root);
if (!map->debugfs) {
dev_warn(map->dev, "Failed to create debugfs directory\n");
+
+ kfree(map->debugfs_name);
+ map->debugfs_name = NULL;
return;
}
--
2.16.2
On Wed, 07 Mar 2018, Mark Brown wrote:
> On Wed, Mar 07, 2018 at 06:50:08PM +0800, JeffyChen wrote:
>
> > so i sent this patch to fix that by using the of_node name to create
> > debugfs.
>
> > but later i found we have:
> > commit a430ab205d29e7d1537b220fcf989b8080d8267f
> > Author: Fabio Estevam <[email protected]>
> > Date: Mon Mar 5 15:52:09 2018 -0300
>
> > regmap: debugfs: Disambiguate dummy debugfs file name
>
> > which would fix that in another way by naming dummy0, dummy1, dummy2.
>
> > even if the issue is fixed, i wonder could we still have this patch to make
> > it more readable:
>
> Yes, like I say I think that's a good idea anyway - that sort of usage
> is the reason we've got the facility to set an explicit name.
Is that an Ack?
--
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, Mar 07, 2018 at 04:54:05PM +0000, Lee Jones wrote:
> On Wed, 07 Mar 2018, Mark Brown wrote:
> > On Wed, Mar 07, 2018 at 06:50:08PM +0800, JeffyChen wrote:
> > Yes, like I say I think that's a good idea anyway - that sort of usage
> > is the reason we've got the facility to set an explicit name.
> Is that an Ack?
No, it looked like the change was only for MFD?
On Wed, 07 Mar 2018, Mark Brown wrote:
> On Wed, Mar 07, 2018 at 04:54:05PM +0000, Lee Jones wrote:
> > On Wed, 07 Mar 2018, Mark Brown wrote:
> > > On Wed, Mar 07, 2018 at 06:50:08PM +0800, JeffyChen wrote:
>
> > > Yes, like I say I think that's a good idea anyway - that sort of usage
> > > is the reason we've got the facility to set an explicit name.
>
> > Is that an Ack?
>
> No, it looked like the change was only for MFD?
Right, but you can still Review/Ack patches from other subsystems.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Mon, Mar 12, 2018 at 02:07:42PM +0000, Lee Jones wrote:
> On Wed, 07 Mar 2018, Mark Brown wrote:
> > On Wed, Mar 07, 2018 at 04:54:05PM +0000, Lee Jones wrote:
> > > Is that an Ack?
> > No, it looked like the change was only for MFD?
> Right, but you can still Review/Ack patches from other subsystems.
If I want to ack something I'll do so explicitly; I've not looked to see
if what's being filled in here makes sense or not.