All of lore.kernel.org
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH v3 2/2] dt: add custom device creation to platform bus scan
Date: Thu, 26 May 2011 13:43:22 -0500	[thread overview]
Message-ID: <4DDE9F4A.5050507@gmail.com> (raw)
In-Reply-To: <201105261511.23659.arnd@arndb.de>

Arnd,

On 05/26/2011 08:11 AM, Arnd Bergmann wrote:
> On Wednesday 25 May 2011, Rob Herring wrote:
>>
>> From: Rob Herring<rob.herring@calxeda.com>
>>
>> Add support to the platform bus scanning to call custom device creation
>> functions. This enables creation of non-platform devices like amba_bus.
>>
>> Cc: Jeremy Kerr<jeremy.kerr@canonical.com>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: linux at arm.linux.org.uk
>> Cc: arnd at arndb.de
>> Cc: Linus Walleij<linus.walleij@linaro.org>
>> Signed-off-by: Rob Herring<rob.herring@calxeda.com>
>
> This creates a confusing mix of match table entries: Normally,
> all entries in the match table are meant to identify child buses,
> but if I read your patch correctly, you now also need to match
> on the amba devices themselves, including the creation of
> platform devices for each child device node under an amba
> device.
>
We should only create devices for each matching bus and the immediate 
children of each bus. A child device of an amba device would be 
something like an i2c bus which we don't want to create devices for. Or 
am I missing something?

> I don't think that was the intention. Maybe we need to pass
> two match tables into of_platform_bus_probe() instead:
> one to identify the buses, and another one that is used
> to create the actual devices.
>
That was my original thinking too, but some reason I had concluded 1 
could get by with just 1 table. After more thought, I think you are 
right. In fact, I broke platform device creation with this patch. I need 
to be able to tell if no match means create a platform device (child of 
bus) or not (child of a device).

Here's an updated version with just the interesting hunk. I've tested it 
with a made up bus structure that looks something like this:

soc bus
  -plat dev
  -amba dev
  -sub-bus
    -plat dev
    -amba dev
  -plat dev

As of_platform_bus_probe is not recommended to be used by Grant, I only 
plan to add 2 match tables to of_platform_bus_populate.

@@ -234,18 +237,32 @@ static int of_platform_bus_create(struct 
device_node *bus,
  		return 0;
  	}

-	dev = of_platform_device_create(bus, NULL, parent);
-	if (!dev || !of_match_node(matches, bus))
-		return 0;
-
-	for_each_child_of_node(bus, child) {
-		pr_debug("   create child: %s\n", child->full_name);
-		rc = of_platform_bus_create(child, matches, &dev->dev, strict);
-		if (rc) {
-			of_node_put(child);
-			break;
+	id = of_match_node(bus_matches, bus);
+	if (id) {
+		dev = of_platform_device_create(bus, NULL, parent);
+		if (!dev)
+			return 0;
+		for_each_child_of_node(bus, child) {
+			pr_debug("   create child: %s\n", child->full_name);
+			rc = of_platform_bus_create(child, bus_matches,
+						    dev_matches, dev, strict);
+			if (rc) {
+				of_node_put(child);
+				break;
+			}
  		}
+		return rc;
  	}
+
+	id = of_match_node(dev_matches, bus);
+	mdata = id ? id->data : NULL;
+	if (id && mdata && mdata->dev_create)
+		dev = mdata->dev_create(bus, parent);
+	else
+		dev = of_platform_device_create(bus, NULL, parent);
+	if (!dev)
+		return 0;
+
  	return rc;
  }


Rob

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [RFC PATCH v3 2/2] dt: add custom device creation to platform bus scan
Date: Thu, 26 May 2011 13:43:22 -0500	[thread overview]
Message-ID: <4DDE9F4A.5050507@gmail.com> (raw)
In-Reply-To: <201105261511.23659.arnd-r2nGTMty4D4@public.gmane.org>

Arnd,

On 05/26/2011 08:11 AM, Arnd Bergmann wrote:
> On Wednesday 25 May 2011, Rob Herring wrote:
>>
>> From: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>
>> Add support to the platform bus scanning to call custom device creation
>> functions. This enables creation of non-platform devices like amba_bus.
>>
>> Cc: Jeremy Kerr<jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> Cc: Grant Likely<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Cc: linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org
>> Cc: arnd-r2nGTMty4D4@public.gmane.org
>> Cc: Linus Walleij<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>
> This creates a confusing mix of match table entries: Normally,
> all entries in the match table are meant to identify child buses,
> but if I read your patch correctly, you now also need to match
> on the amba devices themselves, including the creation of
> platform devices for each child device node under an amba
> device.
>
We should only create devices for each matching bus and the immediate 
children of each bus. A child device of an amba device would be 
something like an i2c bus which we don't want to create devices for. Or 
am I missing something?

> I don't think that was the intention. Maybe we need to pass
> two match tables into of_platform_bus_probe() instead:
> one to identify the buses, and another one that is used
> to create the actual devices.
>
That was my original thinking too, but some reason I had concluded 1 
could get by with just 1 table. After more thought, I think you are 
right. In fact, I broke platform device creation with this patch. I need 
to be able to tell if no match means create a platform device (child of 
bus) or not (child of a device).

Here's an updated version with just the interesting hunk. I've tested it 
with a made up bus structure that looks something like this:

soc bus
  -plat dev
  -amba dev
  -sub-bus
    -plat dev
    -amba dev
  -plat dev

As of_platform_bus_probe is not recommended to be used by Grant, I only 
plan to add 2 match tables to of_platform_bus_populate.

@@ -234,18 +237,32 @@ static int of_platform_bus_create(struct 
device_node *bus,
  		return 0;
  	}

-	dev = of_platform_device_create(bus, NULL, parent);
-	if (!dev || !of_match_node(matches, bus))
-		return 0;
-
-	for_each_child_of_node(bus, child) {
-		pr_debug("   create child: %s\n", child->full_name);
-		rc = of_platform_bus_create(child, matches, &dev->dev, strict);
-		if (rc) {
-			of_node_put(child);
-			break;
+	id = of_match_node(bus_matches, bus);
+	if (id) {
+		dev = of_platform_device_create(bus, NULL, parent);
+		if (!dev)
+			return 0;
+		for_each_child_of_node(bus, child) {
+			pr_debug("   create child: %s\n", child->full_name);
+			rc = of_platform_bus_create(child, bus_matches,
+						    dev_matches, dev, strict);
+			if (rc) {
+				of_node_put(child);
+				break;
+			}
  		}
+		return rc;
  	}
+
+	id = of_match_node(dev_matches, bus);
+	mdata = id ? id->data : NULL;
+	if (id && mdata && mdata->dev_create)
+		dev = mdata->dev_create(bus, parent);
+	else
+		dev = of_platform_device_create(bus, NULL, parent);
+	if (!dev)
+		return 0;
+
  	return rc;
  }


Rob

  reply	other threads:[~2011-05-26 18:43 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-25 21:31 [RFC PATCH v3 0/2] amba bus device tree probing Rob Herring
2011-05-25 21:31 ` Rob Herring
2011-05-25 21:31 ` [RFC PATCH v3 1/2] drivers/amba: create devices from device tree Rob Herring
2011-05-25 21:31   ` Rob Herring
2011-05-26  7:41   ` Linus Walleij
2011-05-26  7:41     ` Linus Walleij
2011-05-25 21:31 ` [RFC PATCH v3 2/2] dt: add custom device creation to platform bus scan Rob Herring
2011-05-25 21:31   ` Rob Herring
2011-05-26 13:11   ` Arnd Bergmann
2011-05-26 13:11     ` Arnd Bergmann
2011-05-26 18:43     ` Rob Herring [this message]
2011-05-26 18:43       ` Rob Herring
2011-05-27 12:06       ` Arnd Bergmann
2011-05-27 12:06         ` Arnd Bergmann
2011-06-01 16:52         ` Rob Herring
2011-06-01 16:52           ` Rob Herring
2011-06-01 16:58           ` Grant Likely
2011-06-01 16:58             ` Grant Likely
2011-06-01 21:29             ` Arnd Bergmann
2011-06-01 21:29               ` Arnd Bergmann
2011-06-08  3:12               ` Rob Herring
2011-06-08  3:12                 ` Rob Herring
2011-06-08  6:16                 ` Arnd Bergmann
2011-06-08  6:16                   ` Arnd Bergmann

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=4DDE9F4A.5050507@gmail.com \
    --to=robherring2@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.