[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

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 <treding@nvidia.com>
---
 drivers/soc/bcm/brcmstb/common.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
index 781ada62d0a3..4fe1cb73b39a 100644
--- a/drivers/soc/bcm/brcmstb/common.c
+++ b/drivers/soc/bcm/brcmstb/common.c
@@ -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;
-- 
2.15.1

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Olof Johansson]

On Tue, Jan 09, 2018 at 03:54:09PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> 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 <treding@nvidia.com>
> ---


Thanks, applied.


-Olof

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Florian Fainelli]

On 01/11/2018 05:01 PM, Olof Johansson wrote:
> On Tue, Jan 09, 2018 at 03:54:09PM +0100, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> 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 <treding@nvidia.com>
>> ---
> 
> 
> Thanks, applied.

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

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Sudeep Holla]

On 09/01/18 14:54, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> 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 <treding@nvidia.com>
> ---
>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
> index 781ada62d0a3..4fe1cb73b39a 100644
> --- a/drivers/soc/bcm/brcmstb/common.c
> +++ b/drivers/soc/bcm/brcmstb/common.c
> @@ -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 ?

-- 
Regards,
Sudeep

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Sudeep Holla]

On 12/01/18 11:39, Sudeep Holla wrote:
> 
> 
> On 09/01/18 14:54, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> 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 <treding@nvidia.com>
>> ---
>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>> index 781ada62d0a3..4fe1cb73b39a 100644
>> --- a/drivers/soc/bcm/brcmstb/common.c
>> +++ b/drivers/soc/bcm/brcmstb/common.c
>> @@ -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 ?
> 

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 ?

-- 
Regards,
Sudeep

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Thierry Reding]

On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
> 
> 
> On 12/01/18 11:39, Sudeep Holla wrote:
> > 
> > 
> > On 09/01/18 14:54, Thierry Reding wrote:
> >> From: Thierry Reding <treding@nvidia.com>
> >>
> >> 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 <treding@nvidia.com>
> >> ---
> >>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
> >> index 781ada62d0a3..4fe1cb73b39a 100644
> >> --- a/drivers/soc/bcm/brcmstb/common.c
> >> +++ b/drivers/soc/bcm/brcmstb/common.c
> >> @@ -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 ?
> > 
> 
> 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>

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Thierry Reding]

On Fri, Jan 12, 2018 at 01:58:28PM +0100, Thierry Reding wrote:
> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
> > 
> > 
> > On 12/01/18 11:39, Sudeep Holla wrote:
> > > 
> > > 
> > > On 09/01/18 14:54, Thierry Reding wrote:
> > >> From: Thierry Reding <treding@nvidia.com>
> > >>
> > >> 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 <treding@nvidia.com>
> > >> ---
> > >>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
> > >>  1 file changed, 5 insertions(+)
> > >>
> > >> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
> > >> index 781ada62d0a3..4fe1cb73b39a 100644
> > >> --- a/drivers/soc/bcm/brcmstb/common.c
> > >> +++ b/drivers/soc/bcm/brcmstb/common.c
> > >> @@ -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 ?
> > > 
> > 
> > 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.

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.

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

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Sudeep Holla]

On 12/01/18 13:15, Thierry Reding wrote:
> On Fri, Jan 12, 2018 at 01:58:28PM +0100, Thierry Reding wrote:
>> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
>>>
>>>
>>> On 12/01/18 11:39, Sudeep Holla wrote:
>>>>
>>>>
>>>> On 09/01/18 14:54, Thierry Reding wrote:
>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>
>>>>> 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 <treding@nvidia.com>
>>>>> ---
>>>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>>>> @@ -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 ?
>>>>
>>>
>>> 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.
> 
> 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.

-- 
Regards,
Sudeep

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

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Sudeep Holla]

On 12/01/18 12:58, Thierry Reding wrote:
> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
>>
>>
>> On 12/01/18 11:39, Sudeep Holla wrote:
>>>
>>>
>>> On 09/01/18 14:54, Thierry Reding wrote:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> 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 <treding@nvidia.com>
>>>> ---
>>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>>> @@ -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 ?
>>>
>>
>> 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

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Olof Johansson]

On Fri, Jan 12, 2018 at 7:27 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 12/01/18 12:58, Thierry Reding wrote:
>> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
>>>
>>>
>>> On 12/01/18 11:39, Sudeep Holla wrote:
>>>>
>>>>
>>>> On 09/01/18 14:54, Thierry Reding wrote:
>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>
>>>>> 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 <treding@nvidia.com>
>>>>> ---
>>>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>>>> @@ -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 ?
>>>>
>>>
>>> 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


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


-Olof

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Olof Johansson]

On Fri, Jan 12, 2018 at 10:19 AM, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Jan 12, 2018 at 7:27 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 12/01/18 12:58, Thierry Reding wrote:
>>> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
>>>>
>>>>
>>>> On 12/01/18 11:39, Sudeep Holla wrote:
>>>>>
>>>>>
>>>>> On 09/01/18 14:54, Thierry Reding wrote:
>>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>>
>>>>>> 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 <treding@nvidia.com>
>>>>>> ---
>>>>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>>>>  1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>>>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>>>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>>>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>>>>> @@ -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 ?
>>>>>
>>>>
>>>> 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
>
>
> 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

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Florian Fainelli]

On 01/12/2018 10:21 AM, Olof Johansson wrote:
> On Fri, Jan 12, 2018 at 10:19 AM, Olof Johansson <olof@lixom.net> wrote:
>> On Fri, Jan 12, 2018 at 7:27 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>>
>>> On 12/01/18 12:58, Thierry Reding wrote:
>>>> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
>>>>>
>>>>>
>>>>> On 12/01/18 11:39, Sudeep Holla wrote:
>>>>>>
>>>>>>
>>>>>> On 09/01/18 14:54, Thierry Reding wrote:
>>>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>>>
>>>>>>> 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 <treding@nvidia.com>
>>>>>>> ---
>>>>>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>>>>>  1 file changed, 5 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>>>>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>>>>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>>>>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>>>>>> @@ -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 ?
>>>>>>
>>>>>
>>>>> 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
>>
>>
>> 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.
-- 
Florian

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Olof Johansson]

On Fri, Jan 12, 2018 at 10:27 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 01/12/2018 10:21 AM, Olof Johansson wrote:
>> On Fri, Jan 12, 2018 at 10:19 AM, Olof Johansson <olof@lixom.net> wrote:
>>> On Fri, Jan 12, 2018 at 7:27 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>>
>>>> On 12/01/18 12:58, Thierry Reding wrote:
>>>>> On Fri, Jan 12, 2018 at 12:12:11PM +0000, Sudeep Holla wrote:
>>>>>>
>>>>>>
>>>>>> On 12/01/18 11:39, Sudeep Holla wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 09/01/18 14:54, Thierry Reding wrote:
>>>>>>>> From: Thierry Reding <treding@nvidia.com>
>>>>>>>>
>>>>>>>> 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 <treding@nvidia.com>
>>>>>>>> ---
>>>>>>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>>>>>>  1 file changed, 5 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>>>>>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>>>>>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>>>>>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>>>>>>> @@ -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 ?
>>>>>>>
>>>>>>
>>>>>> 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
>>>
>>>
>>> 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

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Florian Fainelli]

On 01/12/2018 04:12 AM, Sudeep Holla wrote:
> 
> 
> On 12/01/18 11:39, Sudeep Holla wrote:
>>
>>
>> On 09/01/18 14:54, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> 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 <treding@nvidia.com>
>>> ---
>>>  drivers/soc/bcm/brcmstb/common.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
>>> index 781ada62d0a3..4fe1cb73b39a 100644
>>> --- a/drivers/soc/bcm/brcmstb/common.c
>>> +++ b/drivers/soc/bcm/brcmstb/common.c
>>> @@ -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 ?
>>
> 
> 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.
-- 
Florian

[PATCH] soc: brcmstb: Only register SoC device on STB platforms

[Thierry Reding]

On Fri, Jan 12, 2018 at 11:39:30AM +0000, Sudeep Holla wrote:
> 
> 
> On 09/01/18 14:54, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > 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 <treding@nvidia.com>
> > ---
> >  drivers/soc/bcm/brcmstb/common.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
> > index 781ada62d0a3..4fe1cb73b39a 100644
> > --- a/drivers/soc/bcm/brcmstb/common.c
> > +++ b/drivers/soc/bcm/brcmstb/common.c
> > @@ -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 ?

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>