soc: brcmstb: Only register SoC device on STB platforms

PATCH Applied Thierry Reding 4 participants 15 messages Jan 9, 2018 – Jan 12, 2018 Broadcom BCM7XXX Arm architecture

lore

Share & tools

lore

Thierry Reding

author

2018-01-09 14:54 UTC #0 lore raw

PATCH 1/1 soc: brcmstb: Only register SoC device on STB platforms

From: Thierry Reding <[email protected]>

After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.

This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.

To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.

Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")

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

---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/soc/bcm/brcmstb/common.c +5 -0

index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
 		return -ENOMEM;

Signature · 1 line

2.15.1

Olof Johansson

Applied

2018-01-12 01:01 UTC #1 lore raw

Quoted 13 lines · Thierry Reding

From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.

Quoted 7 lines

To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---

Thanks, applied.

-Olof

Florian Fainelli

2018-01-12 01:03 UTC #2 lore raw

Quoted 18 lines · Olof Johansson

Quoted 17 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.

Quoted 6 lines

Quoted 3 lines
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
Thanks, applied.

Thanks for picking that up, I was just going to submit it.

Signature · 1 line

Florian

Sudeep Holla

2018-01-12 11:39 UTC #3 lore raw

In reply to Thierry Reding

Quoted 28 lines · Thierry Reding

From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +0 -0
index 781ada62d0a3..4fe1cb73b39a 100644

drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98

Quoted 10 lines

drivers/soc/bcm/brcmstb/common.c +5 -0

@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+

missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?

Signature · 2 lines

Regards,
Sudeep

Sudeep Holla

2018-01-12 12:12 UTC #4 lore raw

Quoted 37 lines · Sudeep Holla

Quoted 34 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +1 -0
index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 

Quoted 7 lines

drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 4 lines
drivers/soc/bcm/brcmstb/common.c +4 -0
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?

Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?

Signature · 2 lines

Regards,
Sudeep

Thierry Reding

author

2018-01-12 12:58 UTC #5 lore raw

Quoted 48 lines · Sudeep Holla

Quoted 37 lines · Sudeep Holla
Quoted 34 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +1 -0
index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
Quoted 7 lines
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 4 lines
drivers/soc/bcm/brcmstb/common.c +4 -0
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?

Quoted 3 lines

Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?

I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.

That said, perhaps an even better thing would be to return 0 in order to avoid marking this as failure, since it really isn't an error if this happens.

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/20180112/b8257be1/attachment.sig>

Thierry Reding

author

2018-01-12 13:15 UTC #6 lore raw

Quoted 53 lines · Thierry Reding

Quoted 48 lines · Sudeep Holla
Quoted 37 lines · Sudeep Holla
Quoted 34 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +1 -0
index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
Quoted 7 lines
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 4 lines
drivers/soc/bcm/brcmstb/common.c +4 -0
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?
Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?

Quoted 7 lines

I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.
That said, perhaps an even better thing would be to return 0 in order to avoid marking this as failure, since it really isn't an error if this happens.

Oh, I see the errors you mentioned now. They're in the biuctrl code, which I hadn't noticed before since they don't cause any weird behaviour other than the error messages in the boot log. Let me fix that up while I'm on it.

Sudeep Holla

2018-01-12 15:25 UTC #7 lore raw

Quoted 58 lines · Thierry Reding

Quoted 57 lines · Thierry Reding
Quoted 48 lines · Sudeep Holla
Quoted 37 lines · Sudeep Holla
Quoted 34 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +1 -0
index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
Quoted 7 lines
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 4 lines
drivers/soc/bcm/brcmstb/common.c +4 -0
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?
Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?
I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.

Quoted 8 lines

Quoted 3 lines
That said, perhaps an even better thing would be to return 0 in order to avoid marking this as failure, since it really isn't an error if this happens.
Oh, I see the errors you mentioned now. They're in the biuctrl code, which I hadn't noticed before since they don't cause any weird behaviour other than the error messages in the boot log. Let me fix that up while I'm on it.

Sorry for missing context. I posted a patch[1] to fix the error messages I mentioned, but Florian directed me to his patch instead and hence the above question was for him as your patch was addressing a different issue.

Signature · 4 lines

Regards,
Sudeep

[1] https://marc.info/?l=linux-kernel&m=151568158127806&w=2

Sudeep Holla

2018-01-12 15:27 UTC #8 lore raw

In reply to Thierry Reding

Quoted 49 lines · Thierry Reding

Quoted 48 lines · Sudeep Holla
Quoted 44 lines · Sudeep Holla
Quoted 28 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +0 -0
index 781ada62d0a3..4fe1cb73b39a 100644
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 10 lines
drivers/soc/bcm/brcmstb/common.c +5 -0
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?

Quoted 8 lines

Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?
I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.

Sorry for missing the context, I was referring [1]

Signature · 4 lines

Regards,
Sudeep

[1] [1] https://marc.info/?l=linux-kernel&m=151568158127806&w=2

Olof Johansson

2018-01-12 18:19 UTC #9 lore raw

Quoted 61 lines · Sudeep Holla

Quoted 49 lines · Thierry Reding
Quoted 48 lines · Sudeep Holla
Quoted 44 lines · Sudeep Holla
Quoted 28 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +0 -0
index 781ada62d0a3..4fe1cb73b39a 100644
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 10 lines
drivers/soc/bcm/brcmstb/common.c +5 -0
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
   struct soc_device_attribute *soc_dev_attr;
+  struct device_node *sun_top_ctrl;
   struct soc_device *soc_dev;
 
+  sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+  if (!sun_top_ctrl)
+          return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?
Quoted 8 lines
Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?
I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.

Quoted 7 lines

Sorry for missing the context, I was referring [1]
--
Regards,
Sudeep
[1] [1] https://marc.info/?l=linux-kernel&m=151568158127806&w=2

Do we have an incremental patch coming, or should I revert this?

-Olof

Olof Johansson

2018-01-12 18:21 UTC #10 lore raw

Quoted 64 lines · Olof Johansson

Quoted 63 lines · Sudeep Holla
Quoted 49 lines · Thierry Reding
Quoted 48 lines · Sudeep Holla
Quoted 44 lines · Sudeep Holla
Quoted 28 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +0 -0
index 781ada62d0a3..4fe1cb73b39a 100644
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 10 lines
drivers/soc/bcm/brcmstb/common.c +5 -0
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
   struct soc_device_attribute *soc_dev_attr;
+  struct device_node *sun_top_ctrl;
   struct soc_device *soc_dev;
 
+  sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+  if (!sun_top_ctrl)
+          return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?
Quoted 8 lines
Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?
I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.
Sorry for missing the context, I was referring [1]

Quoted 8 lines

Quoted 5 lines
--
Regards,
Sudeep
[1] [1] https://marc.info/?l=linux-kernel&m=151568158127806&w=2
Do we have an incremental patch coming, or should I revert this?

Oh, d'oh. Just noticed the 3-patch series.

Acks from the involved parties would be appreciated before I apply.

-Olof

Florian Fainelli

2018-01-12 18:27 UTC #11 lore raw

Quoted 69 lines · Olof Johansson

Quoted 68 lines · Olof Johansson
Quoted 67 lines · Sudeep Holla
Quoted 49 lines · Thierry Reding
Quoted 48 lines · Sudeep Holla
Quoted 44 lines · Sudeep Holla
Quoted 28 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +0 -0
index 781ada62d0a3..4fe1cb73b39a 100644
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 10 lines
drivers/soc/bcm/brcmstb/common.c +5 -0
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
   struct soc_device_attribute *soc_dev_attr;
+  struct device_node *sun_top_ctrl;
   struct soc_device *soc_dev;
 
+  sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+  if (!sun_top_ctrl)
+          return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?
Quoted 8 lines
Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?
I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.
Sorry for missing the context, I was referring [1]
--
Regards,
Sudeep

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
[1] [1] https://marc.info/?l=linux-kernel&m=151568158127806&w=2
Do we have an incremental patch coming, or should I revert this?
Oh, d'oh. Just noticed the 3-patch series.
Acks from the involved parties would be appreciated before I apply.

Please revert, I will submit a simpler fix which is a combination of Thierry's and Sudeep's patches. Sorry about the mess.

Signature · 1 line

Florian

Olof Johansson

2018-01-12 18:30 UTC #12 lore raw

Quoted 73 lines · Florian Fainelli

Quoted 72 lines · Olof Johansson
Quoted 71 lines · Olof Johansson
Quoted 68 lines · Sudeep Holla
Quoted 49 lines · Thierry Reding
Quoted 48 lines · Sudeep Holla
Quoted 44 lines · Sudeep Holla
Quoted 28 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +0 -0
index 781ada62d0a3..4fe1cb73b39a 100644
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 10 lines
drivers/soc/bcm/brcmstb/common.c +5 -0
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
   struct soc_device_attribute *soc_dev_attr;
+  struct device_node *sun_top_ctrl;
   struct soc_device *soc_dev;
 
+  sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+  if (!sun_top_ctrl)
+          return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?
Quoted 8 lines
Quoted 3 lines
Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?
I'm not sure I understand. There's no way we can prevent the early initcall from running. The point here is to prevent it from running code that shouldn't be run on a platform.
Sorry for missing the context, I was referring [1]
--
Regards,
Sudeep
[1] [1] https://marc.info/?l=linux-kernel&m=151568158127806&w=2

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Do we have an incremental patch coming, or should I revert this?
Oh, d'oh. Just noticed the 3-patch series.
Acks from the involved parties would be appreciated before I apply.
Please revert, I will submit a simpler fix which is a combination of Thierry's and Sudeep's patches. Sorry about the mess.

Done.

-Olof

Florian Fainelli

2018-01-12 18:34 UTC #13 lore raw

In reply to Sudeep Holla

Quoted 48 lines · Sudeep Holla

Quoted 37 lines · Sudeep Holla
Quoted 34 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +1 -0
index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 
Quoted 7 lines
drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 4 lines
drivers/soc/bcm/brcmstb/common.c +4 -0
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?

Quoted 3 lines

Further, I still the error messags on my Juno with this patch applied. I fail to see how this patch prevents brcmstb_biuctrl_init which is early_initcall in drivers/soc/bcm/brcmstb/biuctrl.c getting called ?

I read your patch too quickly, yours is needed, and so is Thierry's, I will send an incremental fix to Olof shortly, sorry about the mess.

Signature · 1 line

Florian

Thierry Reding

author

2018-01-12 12:56 UTC #14 lore raw

In reply to Sudeep Holla

Quoted 37 lines · Sudeep Holla

Quoted 34 lines · Thierry Reding
From: Thierry Reding <[email protected]>
After moving the SoC device initialization to an early initcall in commit f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall"), the Broadcom STB SoC device is registered on all platforms if support for the device is enabled in the kernel configuration.
This causes an additional SoC device to appear on platforms that already register a native one. In case of Tegra the STB SoC device is registered as soc0 (with totally meaningless content in the sysfs attributes) and causes various scripts and programs to fail because they don't know how to parse that data.
To fix this, duplicate the check from brcmstb_soc_device_early_init() that already prevents the code from doing anything nonsensical on non- STB platforms.
Fixes f780429adfbc ("soc: brcmstb: biuctrl: Move to early_initcall")
Signed-off-by Thierry Reding <[email protected]>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/bcm/brcmstb/common.c +1 -0
index 781ada62d0a3..4fe1cb73b39a 100644
@@ -89,8 +89,13 @@ early_initcall(brcmstb_soc_device_early_init);
 static int __init brcmstb_soc_device_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *sun_top_ctrl;
 	struct soc_device *soc_dev;
 

Quoted 6 lines

drivers/soc/bcm/brcmstb/common.c early_initcall(brcmstb_soc_device_early_init); · line 98
Quoted 4 lines
drivers/soc/bcm/brcmstb/common.c +4 -0
+	sun_top_ctrl = of_find_matching_node(NULL, sun_top_ctrl_match);
+	if (!sun_top_ctrl)
+		return -ENODEV;
+
missing of_node_put(sun_top_ctrl) ? or am I missing to see that elsewhere ?

Hm... you're right. I was blindly copying this from brcmstb_soc_device_early() which doesn't have the of_node_put() either. Let me fix this up with another couple of patches.

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/20180112/4a4e350e/attachment-0001.sig>