Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1596285 > unrolled thread

[PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device Geert Uytterhoeven <geert+renesas@glider.be> - 2017-03-09 19:20 +0100
    Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single  SoC device Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-13 13:50 +0100
      Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single  SoC device Arnd Bergmann <arnd@arndb.de> - 2017-03-13 14:10 +0100
        Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single  SoC device Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-13 14:20 +0100
          Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single  SoC device Arnd Bergmann <arnd@arndb.de> - 2017-03-13 14:30 +0100
            Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single  SoC device Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-13 14:50 +0100
    Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single  SoC device Arnd Bergmann <arnd@arndb.de> - 2017-03-13 13:50 +0100

#1596285 — [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

commit 1da1b3628df34a2a ("base: soc: Early register bus when needed")
added support for early registration of SoC devices from a
core_initcall().  However, some drivers need to check the SoC revision
from an early_initcall(), which is even earlier.

While registering the SoC bus and device, and using soc_device_match(),
from an early_initcall() do work, the "soc" directory and the "soc0"
file end up wrongly in the sysfs root, as the "bus" resp. "devices"
directories haven't been created yet.

To fix this, allow to register a single SoC device early on.
As long as the SoC bus isn't registered, soc_device_match() just
matches against this early device.
When the SoC bus is registered later, the early device is registered for
real.

Note that soc_device_register() returns NULL (no error, but also not a
valid pointer) when registering an early device.  Hence platform devices
cannot be instantiated as children of the "soc0" node representing an
early SoC device.  This should not be an issue, as that practice has
been deprecated for new platforms.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Is there a better way to fix this?
While I believe all current users register a single SoC device, we may
want to register multiple SoC devices in the future?

soc_device_match() itself is completely independent from sysfs, but the
sysfs operations are tied intimately to the driver core.

The "bus" and "devices" directories are created from do_basic_setup() ->
driver_init() -> devices_init() / buses_init(), which runs in between
do_pre_smp_initcalls() (early_initcall()) and do_initcalls().
---
 drivers/base/soc.c | 50 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index 50033081834a9ccd..909dedae4c4e1d48 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -109,15 +109,18 @@ static void soc_release(struct device *dev)
 	kfree(soc_dev);
 }
 
+static struct soc_device_attribute *early_soc_dev_attr;
+
 struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr)
 {
 	struct soc_device *soc_dev;
 	int ret;
 
 	if (!soc_bus_type.p) {
-		ret = bus_register(&soc_bus_type);
-		if (ret)
-			goto out1;
+		if (early_soc_dev_attr)
+			return ERR_PTR(-EBUSY);
+		early_soc_dev_attr = soc_dev_attr;
+		return NULL;
 	}
 
 	soc_dev = kzalloc(sizeof(*soc_dev), GFP_KERNEL);
@@ -159,45 +162,53 @@ void soc_device_unregister(struct soc_device *soc_dev)
 	ida_simple_remove(&soc_ida, soc_dev->soc_dev_num);
 
 	device_unregister(&soc_dev->dev);
+	early_soc_dev_attr = NULL;
 }
 
 static int __init soc_bus_register(void)
 {
-	if (soc_bus_type.p)
-		return 0;
+	int ret;
 
-	return bus_register(&soc_bus_type);
+	ret = bus_register(&soc_bus_type);
+	if (ret)
+		return ret;
+
+	if (early_soc_dev_attr)
+		return PTR_ERR(soc_device_register(early_soc_dev_attr));
+
+	return 0;
 }
 core_initcall(soc_bus_register);
 
-static int soc_device_match_one(struct device *dev, void *arg)
+static int soc_device_match_attr(const struct soc_device_attribute *attr,
+				 const struct soc_device_attribute *match)
 {
-	struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
-	const struct soc_device_attribute *match = arg;
-
 	if (match->machine &&
-	    (!soc_dev->attr->machine ||
-	     !glob_match(match->machine, soc_dev->attr->machine)))
+	    (!attr->machine || !glob_match(match->machine, attr->machine)))
 		return 0;
 
 	if (match->family &&
-	    (!soc_dev->attr->family ||
-	     !glob_match(match->family, soc_dev->attr->family)))
+	    (!attr->family || !glob_match(match->family, attr->family)))
 		return 0;
 
 	if (match->revision &&
-	    (!soc_dev->attr->revision ||
-	     !glob_match(match->revision, soc_dev->attr->revision)))
+	    (!attr->revision || !glob_match(match->revision, attr->revision)))
 		return 0;
 
 	if (match->soc_id &&
-	    (!soc_dev->attr->soc_id ||
-	     !glob_match(match->soc_id, soc_dev->attr->soc_id)))
+	    (!attr->soc_id || !glob_match(match->soc_id, attr->soc_id)))
 		return 0;
 
 	return 1;
 }
 
+static int soc_device_match_one(struct device *dev, void *arg)
+{
+	struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
+
+	return soc_device_match_attr(soc_dev->attr, arg);
+}
+
 /*
  * soc_device_match - identify the SoC in the machine
  * @matches: zero-terminated array of possible matches
@@ -230,6 +241,9 @@ const struct soc_device_attribute *soc_device_match(
 			break;
 		ret = bus_for_each_dev(&soc_bus_type, NULL, (void *)matches,
 				       soc_device_match_one);
+		if (ret < 0 && early_soc_dev_attr)
+			ret = soc_device_match_attr(early_soc_dev_attr,
+						    matches);
 		if (ret < 0)
 			return NULL;
 		if (!ret)
-- 
2.7.4

[toc] | [next] | [standalone]

#1599305 — Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

Hi Arnd,

On Mon, Mar 13, 2017 at 1:41 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>> commit 1da1b3628df34a2a ("base: soc: Early register bus when needed")
>> added support for early registration of SoC devices from a
>> core_initcall().  However, some drivers need to check the SoC revision
>> from an early_initcall(), which is even earlier.
>>
>> While registering the SoC bus and device, and using soc_device_match(),
>> from an early_initcall() do work, the "soc" directory and the "soc0"
>> file end up wrongly in the sysfs root, as the "bus" resp. "devices"
>> directories haven't been created yet.
>>
>> To fix this, allow to register a single SoC device early on.
>> As long as the SoC bus isn't registered, soc_device_match() just
>> matches against this early device.
>> When the SoC bus is registered later, the early device is registered for
>> real.
>>
>> Note that soc_device_register() returns NULL (no error, but also not a
>> valid pointer) when registering an early device.  Hence platform devices
>> cannot be instantiated as children of the "soc0" node representing an
>> early SoC device.  This should not be an issue, as that practice has
>> been deprecated for new platforms.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Is there a better way to fix this?
>> While I believe all current users register a single SoC device, we may
>> want to register multiple SoC devices in the future?
>>
>> soc_device_match() itself is completely independent from sysfs, but the
>> sysfs operations are tied intimately to the driver core.
>>
>> The "bus" and "devices" directories are created from do_basic_setup() ->
>> driver_init() -> devices_init() / buses_init(), which runs in between
>> do_pre_smp_initcalls() (early_initcall()) and do_initcalls().
>
> I'd prefer to not have to do the early registration at all and have fewer
> special cases. Can you list a specific example that requires this?

The specific example is the Renesas R-Car SYSC driver, which manages PM
Domains and thus needs to be initialized from an early_initcall.

> If we want to do early registration, then your implementation seems
> fine to me.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1599337 — Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

On Mon, Mar 13, 2017 at 1:46 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Mar 13, 2017 at 1:41 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
>>
>> I'd prefer to not have to do the early registration at all and have fewer
>> special cases. Can you list a specific example that requires this?
>
> The specific example is the Renesas R-Car SYSC driver, which manages PM
> Domains and thus needs to be initialized from an early_initcall.

Ok, and what prevents us from using information in DT to detect which
variant we have? Is this a case of absolutely having to know the exact
hardware revision at the time of initialization, or is it just to simplify the
implementation of the SYSC driver?

     Arnd

[toc] | [prev] | [next] | [standalone]

#1599339 — Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

Hi Arnd,

On Mon, Mar 13, 2017 at 2:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Mar 13, 2017 at 1:46 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Mar 13, 2017 at 1:41 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
>>> I'd prefer to not have to do the early registration at all and have fewer
>>> special cases. Can you list a specific example that requires this?
>>
>> The specific example is the Renesas R-Car SYSC driver, which manages PM
>> Domains and thus needs to be initialized from an early_initcall.
>
> Ok, and what prevents us from using information in DT to detect which
> variant we have? Is this a case of absolutely having to know the exact
> hardware revision at the time of initialization, or is it just to simplify the
> implementation of the SYSC driver?

The former.
Preproduction versions of R-Car H3 have an additional power area, which no
longer exists on H3 ES2.0.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1599357 — Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

On Mon, Mar 13, 2017 at 2:13 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Arnd,
>
> On Mon, Mar 13, 2017 at 2:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Mar 13, 2017 at 1:46 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Mon, Mar 13, 2017 at 1:41 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
>>>> I'd prefer to not have to do the early registration at all and have fewer
>>>> special cases. Can you list a specific example that requires this?
>>>
>>> The specific example is the Renesas R-Car SYSC driver, which manages PM
>>> Domains and thus needs to be initialized from an early_initcall.
>>
>> Ok, and what prevents us from using information in DT to detect which
>> variant we have? Is this a case of absolutely having to know the exact
>> hardware revision at the time of initialization, or is it just to simplify the
>> implementation of the SYSC driver?
>
> The former.
> Preproduction versions of R-Car H3 have an additional power area, which no
> longer exists on H3 ES2.0.

Ok. I'm still not happy about adding the workaround, but this seems like
a reasonable requirement, assuming that the preproduction versions of R-Car H3
are important enough to you that supporting them in mainline helps you
get your work done better.

Please add the explanation to the changelog, along with my

Acked-by: Arnd Bergmann <arnd@arndb.de>

[toc] | [prev] | [next] | [standalone]

#1599377 — Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

Hi Arnd,

On Mon, Mar 13, 2017 at 2:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Mar 13, 2017 at 2:13 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Mar 13, 2017 at 2:06 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Mon, Mar 13, 2017 at 1:46 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Mon, Mar 13, 2017 at 1:41 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>> On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
>>>>> I'd prefer to not have to do the early registration at all and have fewer
>>>>> special cases. Can you list a specific example that requires this?
>>>>
>>>> The specific example is the Renesas R-Car SYSC driver, which manages PM
>>>> Domains and thus needs to be initialized from an early_initcall.
>>>
>>> Ok, and what prevents us from using information in DT to detect which
>>> variant we have? Is this a case of absolutely having to know the exact
>>> hardware revision at the time of initialization, or is it just to simplify the
>>> implementation of the SYSC driver?
>>
>> The former.
>> Preproduction versions of R-Car H3 have an additional power area, which no
>> longer exists on H3 ES2.0.
>
> Ok. I'm still not happy about adding the workaround, but this seems like
> a reasonable requirement, assuming that the preproduction versions of R-Car H3
> are important enough to you that supporting them in mainline helps you
> get your work done better.

That's indeed our motivation.  Currently we all have preproduction SoCs, with
limited (remote) access to R-Car H3 ES2.0.

The goal is to:
  1. Support both the ES1.x and ES2.0 SoC revisions in a single binary
     for now,
  2. Make it clear which code supports ES1.x, so it can easily be identified
     and removed later, when production SoCs are deemed ubiquitous.

> Please add the explanation to the changelog, along with my
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Will do, thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1599311 — Re: [PATCH 2/2] [RFC] base: soc: Allow early registration of a single SoC device

On Thu, Mar 9, 2017 at 7:18 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> commit 1da1b3628df34a2a ("base: soc: Early register bus when needed")
> added support for early registration of SoC devices from a
> core_initcall().  However, some drivers need to check the SoC revision
> from an early_initcall(), which is even earlier.
>
> While registering the SoC bus and device, and using soc_device_match(),
> from an early_initcall() do work, the "soc" directory and the "soc0"
> file end up wrongly in the sysfs root, as the "bus" resp. "devices"
> directories haven't been created yet.
>
> To fix this, allow to register a single SoC device early on.
> As long as the SoC bus isn't registered, soc_device_match() just
> matches against this early device.
> When the SoC bus is registered later, the early device is registered for
> real.
>
> Note that soc_device_register() returns NULL (no error, but also not a
> valid pointer) when registering an early device.  Hence platform devices
> cannot be instantiated as children of the "soc0" node representing an
> early SoC device.  This should not be an issue, as that practice has
> been deprecated for new platforms.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Is there a better way to fix this?
> While I believe all current users register a single SoC device, we may
> want to register multiple SoC devices in the future?
>
> soc_device_match() itself is completely independent from sysfs, but the
> sysfs operations are tied intimately to the driver core.
>
> The "bus" and "devices" directories are created from do_basic_setup() ->
> driver_init() -> devices_init() / buses_init(), which runs in between
> do_pre_smp_initcalls() (early_initcall()) and do_initcalls().

I'd prefer to not have to do the early registration at all and have fewer
special cases. Can you list a specific example that requires this?

If we want to do early registration, then your implementation seems
fine to me.

     Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web