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?
Could we not just restore 'fuse->base' to 'base' in the error paths of
tegra_fuse_probe()?
Cheers
Jon
--
nvpublic
