On 21.12.2018 13.43, Jonathan Hunter wrote:
On 14/12/2018 08:43, Timo Alho wrote:
On cases where device tree entries for fuse and clock provider are in
different order, fuse driver needs to defer probing. This leads to
freeing incorrect IO base address as the fuse->base variable gets
overwritten once during first probe invocation. This leads to
following spew during boot:
[ 3.082285] Trying to vfree() nonexistent vm area (00000000cfe8fd94)
[ 3.082308] WARNING: CPU: 5 PID: 126 at /hdd/l4t/kernel/stable/mm/vmalloc.c:1511 __vunmap+0xcc/0xd8
[ 3.082318] Modules linked in:
[ 3.082330] CPU: 5 PID: 126 Comm: kworker/5:1 Tainted: G S 4.19.7-tegra-gce119d3 #1
[ 3.082340] Hardware name: quill (DT)
[ 3.082353] Workqueue: events deferred_probe_work_func
[ 3.082364] pstate: 40000005 (nZcv daif -PAN -UAO)
[ 3.082372] pc : __vunmap+0xcc/0xd8
[ 3.082379] lr : __vunmap+0xcc/0xd8
[ 3.082385] sp : ffff00000a1d3b60
[ 3.082391] x29: ffff00000a1d3b60 x28: 0000000000000000
[ 3.082402] x27: 0000000000000000 x26: ffff000008e8b610
[ 3.082413] x25: 0000000000000000 x24: 0000000000000009
[ 3.082423] x23: ffff000009221a90 x22: ffff000009f6d000
[ 3.082432] x21: 0000000000000000 x20: 0000000000000000
[ 3.082442] x19: ffff000009f6d000 x18: ffffffffffffffff
[ 3.082452] x17: 0000000000000000 x16: 0000000000000000
[ 3.082462] x15: ffff0000091396c8 x14: 0720072007200720
[ 3.082471] x13: 0720072007200720 x12: 0720072907340739
[ 3.082481] x11: 0764076607380765 x10: 0766076307300730
[ 3.082491] x9 : 0730073007300730 x8 : 0730073007280720
[ 3.082501] x7 : 0761076507720761 x6 : 0000000000000102
[ 3.082510] x5 : 0000000000000000 x4 : 0000000000000000
[ 3.082519] x3 : ffffffffffffffff x2 : ffff000009150ff8
[ 3.082528] x1 : 3d95b1429fff5200 x0 : 0000000000000000
[ 3.082538] Call trace:
[ 3.082545] __vunmap+0xcc/0xd8
[ 3.082552] vunmap+0x24/0x30
[ 3.082561] __iounmap+0x2c/0x38
[ 3.082569] tegra_fuse_probe+0xc8/0x118
[ 3.082577] platform_drv_probe+0x50/0xa0
[ 3.082585] really_probe+0x1b0/0x288
[ 3.082593] driver_probe_device+0x58/0x100
[ 3.082601] __device_attach_driver+0x98/0xf0
[ 3.082609] bus_for_each_drv+0x64/0xc8
[ 3.082616] __device_attach+0xd8/0x130
[ 3.082624] device_initial_probe+0x10/0x18
[ 3.082631] bus_probe_device+0x90/0x98
[ 3.082638] deferred_probe_work_func+0x74/0xb0
[ 3.082649] process_one_work+0x1e0/0x318
[ 3.082656] worker_thread+0x228/0x450
[ 3.082664] kthread+0x128/0x130
[ 3.082672] ret_from_fork+0x10/0x18
[ 3.082678] ---[ end trace 0810fe6ba772c1c7 ]---
To fix, define a separate struct member for early use of IO base
address.
Signed-off-by: Timo Alho <talho@xxxxxxxxxx>
---
drivers/soc/tegra/fuse/fuse-tegra.c | 9 +++++----
drivers/soc/tegra/fuse/fuse.h | 1 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c
index a33ee8e..c614b19 100644
--- a/drivers/soc/tegra/fuse/fuse-tegra.c
+++ b/drivers/soc/tegra/fuse/fuse-tegra.c
@@ -129,7 +129,6 @@ static const struct of_device_id tegra_fuse_match[] = {
static int tegra_fuse_probe(struct platform_device *pdev)
{
- void __iomem *base = fuse->base;
struct resource *res;
int err;
@@ -161,7 +160,8 @@ static int tegra_fuse_probe(struct platform_device *pdev)
return -ENODEV;
/* release the early I/O memory mapping */
- iounmap(base);
+ iounmap(fuse->base_early);
+ fuse->base_early = NULL;
return 0;
}
@@ -327,11 +327,12 @@ static int __init tegra_init_fuse(void)
}
}
- fuse->base = ioremap_nocache(regs.start, resource_size(®s));
- if (!fuse->base) {
+ fuse->base_early = ioremap_nocache(regs.start, resource_size(®s));
+ if (!fuse->base_early) {
pr_err("failed to map FUSE registers\n");
return -ENXIO;
}
+ fuse->base = fuse->base_early;
fuse->soc->init(fuse);
diff --git a/drivers/soc/tegra/fuse/fuse.h b/drivers/soc/tegra/fuse/fuse.h
index f355b9d..fafd635 100644
--- a/drivers/soc/tegra/fuse/fuse.h
+++ b/drivers/soc/tegra/fuse/fuse.h
@@ -41,6 +41,7 @@ struct tegra_fuse_soc {
struct tegra_fuse {
struct device *dev;
void __iomem *base;
+ void __iomem *base_early;
phys_addr_t phys;
struct clk *clk;
Does this break functions like tegra30_fuse_read_early() that use on
fuse->base and not fuse->base_early?
It does not, as 'base_early' is only used to store the address that was
early mapped. 'base' is always used when accessing registers.
Could we not just restore 'fuse->base' to 'base' in the error paths of
tegra_fuse_probe()?
Cheers
Jon
Well, yes. And that would make more sense as well. So I'll make that
change and post a new version.
BR,
Timo
