All of lore.kernel.org
 help / color / mirror / Atom feed
From: thierry.reding@gmail.com (Thierry Reding)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] soc: brcmstb: Only register SoC device on STB platforms
Date: Fri, 12 Jan 2018 13:58:28 +0100	[thread overview]
Message-ID: <20180112125828.GB19999@ulmo> (raw)
In-Reply-To: <973b2445-48ca-87a9-b4c2-21d8bfec76dd@arm.com>

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>

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: Brian Norris <computersforpeace@gmail.com>,
	Gregory Fong <gregory.0xf0@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	arm@kernel.org, bcm-kernel-feedback-list@broadcom.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] soc: brcmstb: Only register SoC device on STB platforms
Date: Fri, 12 Jan 2018 13:58:28 +0100	[thread overview]
Message-ID: <20180112125828.GB19999@ulmo> (raw)
In-Reply-To: <973b2445-48ca-87a9-b4c2-21d8bfec76dd@arm.com>

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-01-12 12:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-09 14:54 [PATCH] soc: brcmstb: Only register SoC device on STB platforms Thierry Reding
2018-01-09 14:54 ` Thierry Reding
2018-01-12  1:01 ` Olof Johansson
2018-01-12  1:01   ` Olof Johansson
2018-01-12  1:03   ` Florian Fainelli
2018-01-12  1:03     ` Florian Fainelli
2018-01-12 11:39 ` Sudeep Holla
2018-01-12 11:39   ` Sudeep Holla
2018-01-12 12:12   ` Sudeep Holla
2018-01-12 12:12     ` Sudeep Holla
2018-01-12 12:58     ` Thierry Reding [this message]
2018-01-12 12:58       ` Thierry Reding
2018-01-12 13:15       ` Thierry Reding
2018-01-12 13:15         ` Thierry Reding
2018-01-12 15:25         ` Sudeep Holla
2018-01-12 15:25           ` Sudeep Holla
2018-01-12 15:27       ` Sudeep Holla
2018-01-12 15:27         ` Sudeep Holla
2018-01-12 18:19         ` Olof Johansson
2018-01-12 18:19           ` Olof Johansson
2018-01-12 18:21           ` Olof Johansson
2018-01-12 18:21             ` Olof Johansson
2018-01-12 18:27             ` Florian Fainelli
2018-01-12 18:27               ` Florian Fainelli
2018-01-12 18:30               ` Olof Johansson
2018-01-12 18:30                 ` Olof Johansson
2018-01-12 18:34     ` Florian Fainelli
2018-01-12 18:34       ` Florian Fainelli
2018-01-12 12:56   ` Thierry Reding
2018-01-12 12:56     ` Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180112125828.GB19999@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.