| Message ID | 20170822111922.GA15965@ulmo |
|---|---|
| State | Superseded |
| Headers | show |
On 22/08/17 12:19, Thierry Reding wrote: > On Tue, Aug 22, 2017 at 11:15:21AM +0100, Sudeep Holla wrote: >> >> >> On 17/08/17 15:42, Thierry Reding wrote: >>> From: Thierry Reding <treding@nvidia.com> >>> >>> Move this code from arch/arm/mach-tegra and make it common among 32-bit >>> and 64-bit Tegra SoCs. This is slightly complicated by the fact that on >>> 32-bit Tegra, the SoC device is used as the parent for all devices that >>> are instantiated from device tree. >>> >> >> This seem to be in linux-next and causing the below splat on my platform >> which is not Tegra :) >> >> WARNING: .... at drivers/soc/tegra/fuse/tegra-apbmisc.c:48 >> tegra_get_chip_id+0x30/0x40 >> Modules linked in: >> CPU: 2 PID: 1 Comm: swapper/0 Not tainted >> 4.13.0-rc6-next-20170822-00008-g52a8e57512ae #13 >> task: ffff8009768a0000 task.stack: ffff000008038000 >> PC is at tegra_get_chip_id+0x30/0x40 >> LR is at tegra_get_chip_id+0x30/0x40 >> tegra_get_chip_id+0x30/0x40 >> tegra_soc_device_register+0x68/0xd0 >> tegra_init_soc+0x10/0x44 >> do_one_initcall+0x38/0x120 >> kernel_init_freeable+0x184/0x224 >> kernel_init+0x10/0x100 >> ret_from_fork+0x10/0x18 > > Indeed. Does the below patch fix this? > Yes, it does.
On Tue, Aug 22, 2017 at 1:19 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Tue, Aug 22, 2017 at 11:15:21AM +0100, Sudeep Holla wrote: >> >> >> On 17/08/17 15:42, Thierry Reding wrote: >> > From: Thierry Reding <treding@nvidia.com> >> > >> > Move this code from arch/arm/mach-tegra and make it common among 32-bit >> > and 64-bit Tegra SoCs. This is slightly complicated by the fact that on >> > 32-bit Tegra, the SoC device is used as the parent for all devices that >> > are instantiated from device tree. >> > >> >> This seem to be in linux-next and causing the below splat on my platform >> which is not Tegra :) >> >> WARNING: .... at drivers/soc/tegra/fuse/tegra-apbmisc.c:48 >> tegra_get_chip_id+0x30/0x40 >> Modules linked in: >> CPU: 2 PID: 1 Comm: swapper/0 Not tainted >> 4.13.0-rc6-next-20170822-00008-g52a8e57512ae #13 >> task: ffff8009768a0000 task.stack: ffff000008038000 >> PC is at tegra_get_chip_id+0x30/0x40 >> LR is at tegra_get_chip_id+0x30/0x40 >> tegra_get_chip_id+0x30/0x40 >> tegra_soc_device_register+0x68/0xd0 >> tegra_init_soc+0x10/0x44 >> do_one_initcall+0x38/0x120 >> kernel_init_freeable+0x184/0x224 >> kernel_init+0x10/0x100 >> ret_from_fork+0x10/0x18 > > Indeed. Does the below patch fix this? > > Thierry > > --- >8 --- > From 5706cacc2af5e4ef138d1cd9e1269ca4947a447f Mon Sep 17 00:00:00 2001 > From: Thierry Reding <treding@nvidia.com> > Date: Tue, 22 Aug 2017 13:15:18 +0200 > Subject: [PATCH] soc/tegra: Restrict SoC device registration to Tegra > > Commit 8a46828e623c ("soc/tegra: Register SoC device") added an initcall > to register the SoC device on Tegra. However, that code is unrestricted > and will run on all platforms, causing unwanted warnings. > > Fix this by first checking that we're running on hardware that supports > the fuses block that we use to provide SoC information. > > Fixes: 8a46828e623c ("soc/tegra: Register SoC device") > Signed-off-by: Thierry Reding <treding@nvidia.com> I just found the same thing by inspection after running into drivers/soc/tegra/fuse/fuse-tegra.c:360:0: error: expected ';' at end of input Please fix that as well. Lastly, I believe that tegra_init_fuse() has the same bug, it just doesn't lead to an Oops, just to a harmless warning message, but please add another check there, or combine the two checks in some form. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 23, 2017 at 10:49:36AM +0200, Arnd Bergmann wrote: > On Tue, Aug 22, 2017 at 1:19 PM, Thierry Reding > <thierry.reding@gmail.com> wrote: > > On Tue, Aug 22, 2017 at 11:15:21AM +0100, Sudeep Holla wrote: > >> > >> > >> On 17/08/17 15:42, Thierry Reding wrote: > >> > From: Thierry Reding <treding@nvidia.com> > >> > > >> > Move this code from arch/arm/mach-tegra and make it common among 32-bit > >> > and 64-bit Tegra SoCs. This is slightly complicated by the fact that on > >> > 32-bit Tegra, the SoC device is used as the parent for all devices that > >> > are instantiated from device tree. > >> > > >> > >> This seem to be in linux-next and causing the below splat on my platform > >> which is not Tegra :) > >> > >> WARNING: .... at drivers/soc/tegra/fuse/tegra-apbmisc.c:48 > >> tegra_get_chip_id+0x30/0x40 > >> Modules linked in: > >> CPU: 2 PID: 1 Comm: swapper/0 Not tainted > >> 4.13.0-rc6-next-20170822-00008-g52a8e57512ae #13 > >> task: ffff8009768a0000 task.stack: ffff000008038000 > >> PC is at tegra_get_chip_id+0x30/0x40 > >> LR is at tegra_get_chip_id+0x30/0x40 > >> tegra_get_chip_id+0x30/0x40 > >> tegra_soc_device_register+0x68/0xd0 > >> tegra_init_soc+0x10/0x44 > >> do_one_initcall+0x38/0x120 > >> kernel_init_freeable+0x184/0x224 > >> kernel_init+0x10/0x100 > >> ret_from_fork+0x10/0x18 > > > > Indeed. Does the below patch fix this? > > > > Thierry > > > > --- >8 --- > > From 5706cacc2af5e4ef138d1cd9e1269ca4947a447f Mon Sep 17 00:00:00 2001 > > From: Thierry Reding <treding@nvidia.com> > > Date: Tue, 22 Aug 2017 13:15:18 +0200 > > Subject: [PATCH] soc/tegra: Restrict SoC device registration to Tegra > > > > Commit 8a46828e623c ("soc/tegra: Register SoC device") added an initcall > > to register the SoC device on Tegra. However, that code is unrestricted > > and will run on all platforms, causing unwanted warnings. > > > > Fix this by first checking that we're running on hardware that supports > > the fuses block that we use to provide SoC information. > > > > Fixes: 8a46828e623c ("soc/tegra: Register SoC device") > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > I just found the same thing by inspection after running into > > drivers/soc/tegra/fuse/fuse-tegra.c:360:0: error: expected ';' at end of input > > Please fix that as well. Done, though it's strange that I didn't see those errors. Are those compiler errors or sparse errors? I'll be sending out both patches right away. > Lastly, I believe that tegra_init_fuse() has the same bug, it just doesn't lead > to an Oops, just to a harmless warning message, but please add another > check there, or combine the two checks in some form. I don't think that's the case. The of_find_matching_node_and_match() should catch all non-Tegra cases, and the backward-compatibility fallback has an extra check on the SoC compatible. See the comment on lines 295-298. That code is fairly old and I haven't seen anybody bring up any errors on non-Tegra platforms for that particular piece. Thanks, Thierry
On Wed, Aug 23, 2017 at 11:11 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Wed, Aug 23, 2017 at 10:49:36AM +0200, Arnd Bergmann wrote: >> On Tue, Aug 22, 2017 at 1:19 PM, Thierry Reding >> <thierry.reding@gmail.com> wrote: >> > On Tue, Aug 22, 2017 at 11:15:21AM +0100, Sudeep Holla wrote: >> >> >> >> >> >> On 17/08/17 15:42, Thierry Reding wrote: >> >> > From: Thierry Reding <treding@nvidia.com> >> >> > >> >> > Move this code from arch/arm/mach-tegra and make it common among 32-bit >> >> > and 64-bit Tegra SoCs. This is slightly complicated by the fact that on >> >> > 32-bit Tegra, the SoC device is used as the parent for all devices that >> >> > are instantiated from device tree. >> >> > >> >> >> >> This seem to be in linux-next and causing the below splat on my platform >> >> which is not Tegra :) >> >> >> >> WARNING: .... at drivers/soc/tegra/fuse/tegra-apbmisc.c:48 >> >> tegra_get_chip_id+0x30/0x40 >> >> Modules linked in: >> >> CPU: 2 PID: 1 Comm: swapper/0 Not tainted >> >> 4.13.0-rc6-next-20170822-00008-g52a8e57512ae #13 >> >> task: ffff8009768a0000 task.stack: ffff000008038000 >> >> PC is at tegra_get_chip_id+0x30/0x40 >> >> LR is at tegra_get_chip_id+0x30/0x40 >> >> tegra_get_chip_id+0x30/0x40 >> >> tegra_soc_device_register+0x68/0xd0 >> >> tegra_init_soc+0x10/0x44 >> >> do_one_initcall+0x38/0x120 >> >> kernel_init_freeable+0x184/0x224 >> >> kernel_init+0x10/0x100 >> >> ret_from_fork+0x10/0x18 >> > >> > Indeed. Does the below patch fix this? >> > >> > Thierry >> > >> > --- >8 --- >> > From 5706cacc2af5e4ef138d1cd9e1269ca4947a447f Mon Sep 17 00:00:00 2001 >> > From: Thierry Reding <treding@nvidia.com> >> > Date: Tue, 22 Aug 2017 13:15:18 +0200 >> > Subject: [PATCH] soc/tegra: Restrict SoC device registration to Tegra >> > >> > Commit 8a46828e623c ("soc/tegra: Register SoC device") added an initcall >> > to register the SoC device on Tegra. However, that code is unrestricted >> > and will run on all platforms, causing unwanted warnings. >> > >> > Fix this by first checking that we're running on hardware that supports >> > the fuses block that we use to provide SoC information. >> > >> > Fixes: 8a46828e623c ("soc/tegra: Register SoC device") >> > Signed-off-by: Thierry Reding <treding@nvidia.com> >> >> I just found the same thing by inspection after running into >> >> drivers/soc/tegra/fuse/fuse-tegra.c:360:0: error: expected ';' at end of input >> >> Please fix that as well. > > Done, though it's strange that I didn't see those errors. Are those > compiler errors or sparse errors? It's from the compiler. I just upgraded to gcc-8, which might explain it, or it could be configuration (module/built-in) dependent. > I'll be sending out both patches right away. Thanks! >> Lastly, I believe that tegra_init_fuse() has the same bug, it just doesn't lead >> to an Oops, just to a harmless warning message, but please add another >> check there, or combine the two checks in some form. > > I don't think that's the case. The of_find_matching_node_and_match() > should catch all non-Tegra cases, and the backward-compatibility > fallback has an extra check on the SoC compatible. See the comment on > lines 295-298. > > That code is fairly old and I haven't seen anybody bring up any errors > on non-Tegra platforms for that particular piece. My mistake, I see how it works now. Thanks for checking. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c index 02686378dfc1..161522bd98ac 100644 --- a/drivers/soc/tegra/fuse/fuse-tegra.c +++ b/drivers/soc/tegra/fuse/fuse-tegra.c @@ -349,8 +349,16 @@ early_initcall(tegra_init_fuse); #ifdef CONFIG_ARM64 static int __init tegra_init_soc(void) { + struct device_node *np; struct device *soc; + /* make sure we're running on Tegra */ + np = of_find_matching_node(NULL, tegra_fuse_match); + if (!np) + return 0; + + of_node_put(np); + soc = tegra_soc_device_register(); if (IS_ERR(soc)) { pr_err("failed to register SoC device: %ld\n", PTR_ERR(soc));