Re: [PATCH 2/2] platform: Facilitate the creation ofpseudo-platform buses

[Moffett, Kyle D]

On Aug 10, 2010, at 19:49, Patrick Pannuto wrote:
> As SOCs become more popular, the desire to quickly define a simple,
> but functional, bus type with only a few unique properties becomes
> desirable. As they become more complicated, the ability to nest these
> simple busses and otherwise orchestrate them to match the actual
> topology also becomes desirable.
> 
> EXAMPLE USAGE
> 
> /arch/ARCH/MY_ARCH/my_bus.c:
> 
> 	#include <linux/device.h>
> 	#include <linux/platform_device.h>
> 
> 	struct bus_type SOC_bus_type = {
> 		.name = "SOC-bus-type",
> 	};
> 	EXPORT_SYMBOL_GPL(SOC_bus_type);
> 
> 	struct platform_device SOC_bus1 = {
> 		.name	 = "SOC-bus1",
> 		.id		= -1,
> 		.dev.bus = &SOC_bus_type;
> 	};
> 	EXPORT_SYMBOL_GPL(SOC_bus1);
> 
> 	struct platform_device SOC_bus2 = {
> 		.name	 = "SOC-bus2",
> 		.id		= -2,
> 		.dev.bus = &SOC_bus_type;
> 	};
> 	EXPORT_SYMBOL_GPL(SOC_bus2);
> 
> 	static int __init SOC_bus_init(void)
> 	{
> 		int error;
> 
> 		error = pseudo_platform_bus_register(&SOC_bus_type);
> 		if (error)
> 			return error;
> 
> 		error = platform_device_register(&SOC_bus1);
> 		if (error)
> 			goto fail_bus1;
> 
> 		error = platform_device_register(&SOC_bus2);
> 		if (error)
> 			goto fail_bus2;
> 
> 		return error;
> 
> 		/* platform_device_unregister(&SOC_bus2); */
> fail_bus2:
> 		platform_device_unregister(&SOC_bus1);
> fail_bus1:
> 		pseudo_platform_bus_unregister(&SOC_bus_type);
> 
> 		return error;
> 	}
> 
> /drivers/my_driver.c:
> 	static struct platform_driver my_driver = {
> 		.driver = {
> 			.name	= "my-driver",
> 			.owner	= THIS_MODULE,
> 			.bus	= &SOC_bus_type,
> 		},
> 	};
> 
> /somewhere/my_device.c:
> 	static struct platform_device my_device = {
> 		.name		= "my-device",
> 		.id		= -1,
> 		.dev.bus	= &my_bus_type,
> 		.dev.parent	= &SOC_bus1.dev,
> 	};
> 
> This will build a device tree that mirrors the actual system:
> 
> /sys/bus
> |-- SOC-bus-type
> |   |-- devices
> |   |   |-- SOC_bus1 -> ../../../devices/SOC_bus1
> |   |   |-- SOC_bus2 -> ../../../devices/SOC_bus2
> |   |   |-- my-device -> ../../../devices/SOC_bus1/my-device
> |   |-- drivers
> |   |   |-- my-driver
> 
> /sys/devices
> |-- SOC_bus1
> |   |-- my-device
> |-- SOC_bus2
> 
> Driver can drive any device on the SOC, which is logical, without
> actually being registered on multiple /bus_types/, even though the
> devices may be on different /physical buses/ (which are actually
> just devices).

Hmm...

To me this seems like a really painful implementation of what the OpenFirmware-esque "Flattened Device Tree" does on many embedded systems.

For example, to build an equivalent device tree using an OpenFirmware FDT file, I'd just use this:

/dts-v1/;
/ {
	#address-cells = <1>;
	#size-cells = <1>;
	[...snip...]

	soc-bus-1@fc000000 {
		/* of_platform driver matches against this: */
		compatible = "my-company-name,soc-bus-type";

		/* Define base address and size of the bus */
		reg = <0xfc000000 0x01000000>;
		#address-cells = <1>;
		#size-cells = <1>;

		/* 
		 * Define logical memory mapping relative to the bus addr:
		 * First field is the relative base address for children,
		 * second field is the address in the parent's memory map,
		 * third field is the size of the range.
		 */
		ranges = <0x0 0xfc000000 0x01000000>;

		/* Now for sub-devices */
		my-device@0x10000 {
			compatible = "my-company-name,my-driver";
			reg = <0x10000 0x100>; /* Address and size */
		};
	};

	soc-bus-2@fd000000 {
		/* of_platform driver matches against this: */
		compatible = "my-company-name,soc-bus-type";

		/* Define base address and size of the bus */
		reg = <0xfd000000 0x01000000>;
		#address-cells = <1>;
		#size-cells = <1>;

		/* 
		 * Define logical memory mapping relative to the bus addr:
		 * First field is the relative base address for children,
		 * second field is the address in the parent's memory map,
		 * third field is the size of the range.
		 */
		ranges = <0x0 0xfd000000 0x01000000>;
	};
};

If you don't need to actually do anything special at the bus level, you can just:
	static struct of_device_id soc_bus_ids[] = {
		{ .compatible = "soc-bus-type", },
		{},
	};
	of_platform_bus_probe(NULL, &soc_bus_ids, NULL);

Any of_platform driver that matches something on one of those busses is automatically probed.  Alternatively, if you need special bus behavior:
	static struct of_device_id soc_bus_ids[] = {
		{ .compatible = "soc-bus-type", },
		{},
	};
	static struct of_platform_driver soc_bus_type = {
		.name = "soc-bus-type",
		.match_table = &soc_bus_ids,
		.owner = THIS_MODULE,
		.probe = mybus_probe,
		.remove = mybus_remove,
		.suspend = mybus_suspend,
		.resume = mybus_resume,
		.shutdown = mybus_shutdown,
	};

Then your .probe function actually registers a new OF bus.

The best part is... all devices registered as "of_platform" devices can be used to support many entirely different board models from the exact same kernel.

Fully commented and with actual physical addresses in, my FDT example is comparable to your sample code.  Furthermore, all of the error handling is automatically done, a bunch of device drivers are already ported over, and all the kinks regarding interrupts, etc are already taken care of.  I highly recommend taking a look to see if you can use the very nice existing OF bus code to solve your problem instead of writing yet another half-hard-coded platform bus type.

Cheers,
Kyle Moffett

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/