soc/tegra: Register SoC device

PATCH New Thierry Reding 3 participants 7 messages Aug 17, 2017 – Aug 23, 2017

Subsystems (3)

Arm (32-bit) Arm sub-architectures Tegra architecture support

lore

Share & tools

lore

Thierry Reding

author

2017-08-17 14:42 UTC #0 lore raw

PATCH 1/1 soc/tegra: Register SoC device

From: Thierry Reding <[email protected]>

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.

Signed-off-by Thierry Reding <[email protected]>

---
 arch/arm/mach-tegra/tegra.c         | 29 +---------------------
 drivers/soc/tegra/Kconfig           |  5 ++++
 drivers/soc/tegra/fuse/fuse-tegra.c | 48 +++++++++++++++++++++++++++++++++++--
 include/soc/tegra/fuse.h            |  2 ++
 4 files changed, 54 insertions(+), 30 deletions(-)

arch/arm/mach-tegra/tegra.c +1 ~~-28~~

index 649e9e8c7bcc..02e712d2ea30 100644
@@ -84,35 +84,8 @@ static void __init tegra_dt_init_irq(void)
 
 static void __init tegra_dt_init(void)
 {
-	struct soc_device_attribute *soc_dev_attr;
-	struct soc_device *soc_dev;
-	struct device *parent = NULL;
+	struct device *parent = tegra_soc_device_register();
 
-	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
-	if (!soc_dev_attr)
-		goto out;
-
-	soc_dev_attr->family = kasprintf(GFP_KERNEL, "Tegra");
-	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d",
-					   tegra_sku_info.revision);
-	soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%u", tegra_get_chip_id());
-
-	soc_dev = soc_device_register(soc_dev_attr);
-	if (IS_ERR(soc_dev)) {
-		kfree(soc_dev_attr->family);
-		kfree(soc_dev_attr->revision);
-		kfree(soc_dev_attr->soc_id);
-		kfree(soc_dev_attr);
-		goto out;
-	}
-
-	parent = soc_device_to_device(soc_dev);
-
-	/*
-	 * Finished with the static registrations now; fill in the missing
-	 * devices
-	 */
-out:
 	of_platform_default_populate(NULL, NULL, parent);
 }

drivers/soc/tegra/Kconfig +5 -0

index 1beb7c347344..e9e277178c94 100644
@@ -107,6 +107,11 @@ config ARCH_TEGRA_186_SOC
 endif
 endif
 
+config SOC_TEGRA_FUSE
+	def_bool y
+	depends on ARCH_TEGRA
+	select SOC_BUS
+
 config SOC_TEGRA_FLOWCTRL
 	bool

drivers/soc/tegra/fuse/fuse-tegra.c +46 -2

68 lines not on this page · raw message

include/soc/tegra/fuse.h +2 -0

index b4c9219e7f95..9b6ea0c72117 100644
@@ -65,6 +65,8 @@ int tegra_fuse_readl(unsigned long offset, u32 *value);
 
 extern struct tegra_sku_info tegra_sku_info;
 
+struct device *tegra_soc_device_register(void);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __SOC_TEGRA_FUSE_H__ */

Signature · 1 line

2.13.3

Sudeep Holla

2017-08-22 10:15 UTC #1 lore raw

PATCH 1/1 soc/tegra: Register SoC device

Quoted 7 lines · Thierry Reding

From: Thierry Reding <[email protected]>
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

Signature · 2 lines

Regards,
Sudeep

Thierry Reding

author

2017-08-22 11:19 UTC #2 lore raw

PATCH 1/1 soc/tegra: Register SoC device

Quoted 22 lines · Sudeep Holla

Quoted 7 lines · Thierry Reding
From: Thierry Reding <[email protected]>
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

Quoted 7 lines

 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 ---

Quoted 1 line

From 5706cacc2af5e4ef138d1cd9e1269ca4947a447f Mon Sep 17 00:00:00 2001

From: Thierry Reding <[email protected]>
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 <[email protected]>

---
 drivers/soc/tegra/fuse/fuse-tegra.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/soc/tegra/fuse/fuse-tegra.c +8 -0

index 02686378dfc1..161522bd98ac 100644
@@ -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));

Signature · 9 lines

2.13.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170822/b05aad13/attachment.sig>

Sudeep Holla

2017-08-22 15:00 UTC #3 lore raw

PATCH 1/1 soc/tegra: Register SoC device

Quoted 31 lines · Thierry Reding

Quoted 22 lines · Sudeep Holla
Quoted 7 lines · Thierry Reding
From: Thierry Reding <[email protected]>
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
Quoted 7 lines
 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

Quoted 2 lines

Indeed. Does the below patch fix this?

Yes, it does.

Signature · 2 lines

Regards,
Sudeep

Arnd Bergmann

2017-08-23 08:49 UTC #4 lore raw

PATCH 1/1 soc/tegra: Register SoC device

In reply to Thierry Reding

Quoted 45 lines · Thierry Reding

Quoted 22 lines · Sudeep Holla
Quoted 7 lines · Thierry Reding
From: Thierry Reding <[email protected]>
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
Quoted 7 lines
 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 <[email protected]>
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.

Quoted 5 lines

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 <[email protected]>

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

Thierry Reding

author

2017-08-23 09:11 UTC #5 lore raw

PATCH 1/1 soc/tegra: Register SoC device

Quoted 50 lines · Arnd Bergmann

Quoted 48 lines · Thierry Reding
Quoted 22 lines · Sudeep Holla
Quoted 7 lines · Thierry Reding
From: Thierry Reding <[email protected]>
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
Quoted 7 lines
 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 <[email protected]>
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.

Quoted 8 lines

Quoted 2 lines
Fixes 8a46828e623c ("soc/tegra: Register SoC device")
Signed-off-by Thierry Reding <[email protected]>
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.

Quoted 3 lines

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170823/2a8f8a38/attachment.sig>

Arnd Bergmann

2017-08-23 09:21 UTC #6 lore raw

PATCH 1/1 soc/tegra: Register SoC device

Quoted 54 lines · Thierry Reding

Quoted 53 lines · Arnd Bergmann
Quoted 50 lines · Thierry Reding
Quoted 22 lines · Sudeep Holla
Quoted 7 lines · Thierry Reding
From: Thierry Reding <[email protected]>
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
Quoted 7 lines
 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 <[email protected]>
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 <[email protected]>

Quoted 8 lines

Quoted 5 lines
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.

Quoted 1 line

I'll be sending out both patches right away.

Thanks!

Quoted 11 lines

Quoted 3 lines
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