Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
On Thu, Aug 5, 2010 at 7:25 PM, Patrick Pannuto ppann...@codeaurora.org wrote:
On 08/05/2010 04:16 PM, Grant Likely wrote:
The relevant question before going down this path is, Is the
omap/sh/other-soc behaviour something fundamentally different from the
platform bus? Or is it something complementary that would be better
handled with a notifier or some orthogonal method of adding new
behaviour?
I don't have a problem with multiple platform_bus instances using the
same code (I did suggest it after all), but I do worry about muddying
the Linux device model or making it overly complex. Binding single
drivers to multiple device types could be messy.
Cheers,
g.
From your other email, the distinction of /bus_types/ versus /physical
or logical buses/ was very valuable (thanks). I am becoming less
convinced that I need this infrastructure or the ability to create
pseudo-platform buses. Let me outline some thoughts below, which I
think at least solves my problems ( ;) ), and if some of the OMAP folks
and Alan could chime in as to whether they can apply something similar,
or if they still have need of creating actual new bus types?
As we are exploring all of this, let me put a concrete (ish) scenario
out there to discuss:
SUB_BUS1---DEVICE1
/
CPU ---
\
SUB_BUS2---DEVICE2
DEVICE1 and DEVICE2 are the same device (say, usb host controller, or
whatever), and they should be driven by the same driver. However,
physically they are attached to different buses.
SUB_BUS1 and SUB_BUS2 are *different* and let's say require a different
suspend method. If I understand correctly, the right way to build the
topology would be:
struct device_type SUB_BUS1_type = {
.name = sub-bus1-type,
.pm = sub_bus1_pm_ops;
};
struct platform_device SUB_BUS1 = {
.init_name = sub-bus1,
.dev.type = sub_bus1_type,
};
struct platform_device DEVICE1 = {
.name = device1,
.dev.parent = SUB_BUS1.dev,
};
platform_device_register(SUB_BUS1);
platform_device_register(DEVICE1);
[analogous for *2]
Not quite. The device_type is intended to collect similar, but
different things (ie, all keyboards, regardless of how they are
attached to the system). Trying to capture the device-specific
behavour really belongs in the specific device driver attached to the
SUB_BUS* device. In fact, the way you've constructed your example,
SUB_BUS1 and SUB_BUS2 don't really need to be platform_devices at all;
you could have used a plain struct device because in this example you
aren't binding them to a device driver).
That being said, because each bus *does* have custom behaviour, it
probably does make sense to either bind each to a device driver, or to
do something to each child device that changes the PM behaviour. I
haven't dug into the problem domain deep enough to give you absolute
answers, but the devres approach looks promising, as does creating a
derived platform_bus_type (although I'm not fond of sharing device
drivers between multiple bus_types). Another option might be giving
the parent struct device some runtime PM operations that it performs
per-child. I'm just throwing out ideas here though.
which would yield:
/sys/bus/platform/devices/
|
|-SUB_BUS1
| |
| |-DEVICE1
|
|-SUB_BUS2
| |
| |-DEVICE2
You're looking in the wrong place. Look in /sys/devices instead of
/sys/bus... (see comments below)
Which is the correct topology, and logically provides all the correct
interfaces. The only thing that becomes confusing now, is that SUB_BUS*
is *actually* a physically different bus, yet it's not in /sys/bus;
although I suppose this is actually how the world works presently (you
don't see an individual entry in /sys/bus for each usb host controller,
which is technically defining a sub-bus...)
/sys/bus is for bus_types, not bus instances. All USB devices will be
listed in /sys/bus/usb/devices regardless of the bus instance that
they attached to.
However, look carefully at the files in /sys/bus/*/devices. Notice
that they are all symlinks? Notice that the all of them point to
directories in /sys/devices? /sys/bus tells you which devices are
registered against each bus type, but /sys/devices is the actual
structure. Always look in /sys/devices when you want to know the
topology of the system.
Cheers,
g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
Magnus Damm magnus.d...@gmail.com writes:
On Thu, Aug 5, 2010 at 7:14 AM, Patrick Pannuto ppann...@codeaurora.org
wrote:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
RFC: http://lkml.org/lkml/2010/8/3/496
Patch is unchanged from the RFC. Reviews seemed generally positive
and it seemed this was desired functionality.
Thanks for your patch, it's really nice to see work done in this area!
I'd like to see something like this merged in the not so distant
future. At this point I'm not so concerned about the details, so I'll
restrict myself to this:
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus = my_bus_type,
},
};
I would really prefer not to have the bus type in the here. I
understand it's needed at this point, but I wonder if it's possible to
adjust the device-driver matching for platform devices to allow any
type of pseudo-platform bus_type.
I totally agree here. Keeping the drivers ignorant of the bus (or SoC)
they are on will make them much more portable.
Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
[snip]
Which will allow the same driver to easily to used on either
the platform bus or the newly defined bus type.
Except it requires a re-compile.
Rather than doing this at compile time, it would be better to support
legacy devices at runtime. You could handle this by simply registering
the driver on the custom bus and the platform_bus and let the bus
matching code handle it. Then, the same binary would work on both
legacy and updated SoCs.
Can you safely register a driver on more than one bus? I didn't think
that was safe -- normally it's impossible since you're calling
struct BUS_TYPE_driver mydriver;
BUS_TYPE_driver_register(mydriver)
but now we have multiple bus types that are all actually platform type;
still,
at a minimum you would need:
struct platform_driver mydrvier1 = {
.driver.bus = sub_bus1,
};
struct platform_driver mydrvier2 = {
.driver.bus = sub_bus2,
};
which would all point to the same driver functions, yet the respective
devices
attached for the same driver would be on different buses. I fear this might
confuse some drivers. I don't think dynamic bus assignment is this easy
In short: I do not believe the same driver can be registered on multiple
different buses -- if this is wrong, please correct me.
It is possible, and currently done in powerpc land where some
drivers handle devices on the platform_bus and the custom OF bus.
However, as noted by Magnus, what we really need here is a way for
drivers to not care at all what kind of bus they are on. There are an
increasing number of drivers that are re-used not just across different
SoCs in the same family, but across totally different SoCs (e.g. drivers
for hardware shared between TI OMAP and TI DaVinci, or SH and SH-Mobile/ARM)
I will start trying to work on this
Up to here, this looks exactly what I wrote in thread referenced
above.
It is, you just went on vacation :)
Ah, OK. The changelog was missing credits to that affect, but I was
more concerned that you hadn't seen my example and didn't want to be
duplicating work.
will fix.
[snip]
if you call it second then they will all already be well-defined and
thus not overwritten.
Right, they will not be overwritten, but you'll be left with a mostly
empty dev_pm_ops on the custom bus.
IOW, Most of these custom busses will only want to customize a small
subset of the dev_pm_ops methods (e.g. only the runtime PM methods.) If
you setup your sparsly populated custom dev_pm_ops and then call
platform_bus_type_init() second, dev_pm_ops on the new buswill have *only*
your custom fields, and none of the defaults from platform_dev_pm_ops.
So, what I was getting at is that it should probably be clearer to the
users of platform_bus_type_init() that any customization of dev_pm_ops
should be done after.
I understand what you're saying now, and I can fix this as well.
If you would like to lead this effort, please do so; I did not mean to step
on your toes, it's just that this is an issue for me as well.
No worries there, my toes are fine. :)
Good :)
You had indicated that you were going on vacation for a month and I
had not seen any more follow-up on this issue, so I forged ahead.
Great, I'm glad you forged ahead. There is definitely a broader need
for something like this, and I have no personal attachment to the code.
I have no problems with you continuing the work (in fact, I'd prefer it.
I have lots of other things to catch up on after my vacation.)
In the future though, it's common (and kind) to note the original author
in the changelog when basing a patch on previous work. Something like
originally written by... or based on the work of... etc.
Ok, I can do that; that was the intention of the original inspiration from
line at the beginning. Is there a more formal way of indicating this in the
next version of the patch? Should I add you as a From: or an Author:?
-Pat
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
On 08/04/2010 07:32 PM, Magnus Damm wrote:
On Thu, Aug 5, 2010 at 7:14 AM, Patrick Pannuto ppann...@codeaurora.org
wrote:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
RFC: http://lkml.org/lkml/2010/8/3/496
Patch is unchanged from the RFC. Reviews seemed generally positive
and it seemed this was desired functionality.
Thanks for your patch, it's really nice to see work done in this area!
I'd like to see something like this merged in the not so distant
future. At this point I'm not so concerned about the details, so I'll
restrict myself to this:
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus= my_bus_type,
},
};
I would really prefer not to have the bus type in the here. I
understand it's needed at this point, but I wonder if it's possible to
adjust the device-driver matching for platform devices to allow any
type of pseudo-platform bus_type.
The reason why I'd like to avoid having the bus type in the driver is
that I'd like to reuse the platform driver across multiple
architectures and buses. For instance, on the SH architecture and
So would I :). That's where this was all heading eventually, I was just
originally doing it in two passes. I have some ideas for how to do this
and will try to send out a patchset either today or tomorrow.
SH-Mobile ARM we have SoCs with SCIF hardware blocks driven by the
sh-sci.c serial driver. The sh-sci.c platform driver supports a wide
range of different SCI(F)(A)(B) hardware blocks, and on any given SoC
there is a mix of SCIF blocks spread out on different buses.
At this point our SH platform drivers are unaware where their driver
instanced are located on the SoC. The I/O address and IRQs are
assigned via struct resource and clocks are managed through clkdev. I
believe that adding the bus type in the driver will violate this
abstraction and make it more difficult to just instantiate a driver
somewhere on the SoC.
/somewhere/my_device.c
static struct platform_device my_device = {
.name = my-device,
.id = -1,
.dev.bus= my_bus_type,
.dev.parent = sub_bus_1.dev,
};
This I don't mind at all. Actually, this is the place where the
topology should be defined IMO.
Agreed.
Cheers,
/ magnus
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
Patrick Pannuto ppann...@codeaurora.org writes: You had indicated that you were going on vacation for a month and I had not seen any more follow-up on this issue, so I forged ahead. Great, I'm glad you forged ahead. There is definitely a broader need for something like this, and I have no personal attachment to the code. I have no problems with you continuing the work (in fact, I'd prefer it. I have lots of other things to catch up on after my vacation.) In the future though, it's common (and kind) to note the original author in the changelog when basing a patch on previous work. Something like originally written by... or based on the work of... etc. Ok, I can do that; that was the intention of the original inspiration from line at the beginning. Maybe we need an 'Inspired-by:' tag. :) Is there a more formal way of indicating this in the next version of the patch? Should I add you as a From: or an Author:? I don't know if there is an official way of doing this. I typically add something like based on the idea/code originally proposed/written by John Doe in the changelog, and then add Cc: John Doe ... in the changelog too, but AFAIK, none of this is formalized. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
On Thu, Aug 5, 2010 at 9:57 AM, Kevin Hilman
khil...@deeprootsystems.com wrote:
Patrick Pannuto ppann...@codeaurora.org writes:
On 08/04/2010 05:16 PM, Kevin Hilman wrote:
Patrick Pannuto ppann...@codeaurora.org writes:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
Also, later in that thread I also wrote[1] what seems to be the core of
what you've done here: namely, allow platform_devices and
platform_drivers to to be used on custom busses. Patch is at the end of
this mail with a more focused changelog. As Greg suggested in his reply
to your first version, this part could be merged today, and the
platform_bus_init stuff could be added later, after some more review.
Some comments below...
I can split this into 2 patches.
Yes, I think that would be better.
Was your patch sent to linux-kernel or just linux-omap? I'm not on
linux-omap...
That thread was on linux-arm-kernel and linux-omap
[snip]
Which will allow the same driver to easily to used on either
the platform bus or the newly defined bus type.
Except it requires a re-compile.
Rather than doing this at compile time, it would be better to support
legacy devices at runtime. You could handle this by simply registering
the driver on the custom bus and the platform_bus and let the bus
matching code handle it. Then, the same binary would work on both
legacy and updated SoCs.
Can you safely register a driver on more than one bus? I didn't think
that was safe -- normally it's impossible since you're calling
struct BUS_TYPE_driver mydriver;
BUS_TYPE_driver_register(mydriver)
but now we have multiple bus types that are all actually platform type;
still,
at a minimum you would need:
struct platform_driver mydrvier1 = {
.driver.bus = sub_bus1,
};
struct platform_driver mydrvier2 = {
.driver.bus = sub_bus2,
};
which would all point to the same driver functions, yet the respective
devices
attached for the same driver would be on different buses. I fear this might
confuse some drivers. I don't think dynamic bus assignment is this easy
In short: I do not believe the same driver can be registered on multiple
different buses -- if this is wrong, please correct me.
It is possible, and currently done in powerpc land where some
drivers handle devices on the platform_bus and the custom OF bus.
As of now, the of_platform_bus_type has been removed. It was a bad
idea because it tried to encode non-bus-specific information into
something that was just a clone of the platform_bus. Drivers that
worked on both had to be bound to both busses. I do actually have
code that automatically registers a driver on more than one bus, but
it is rather a hack and was only a temporary measure.
The relevant question before going down this path is, Is the
omap/sh/other-soc behaviour something fundamentally different from the
platform bus? Or is it something complementary that would be better
handled with a notifier or some orthogonal method of adding new
behaviour?
I don't have a problem with multiple platform_bus instances using the
same code (I did suggest it after all), but I do worry about muddying
the Linux device model or making it overly complex. Binding single
drivers to multiple device types could be messy.
Cheers,
g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
On 08/05/2010 04:16 PM, Grant Likely wrote:
On Thu, Aug 5, 2010 at 9:57 AM, Kevin Hilman
khil...@deeprootsystems.com wrote:
Patrick Pannuto ppann...@codeaurora.org writes:
On 08/04/2010 05:16 PM, Kevin Hilman wrote:
Patrick Pannuto ppann...@codeaurora.org writes:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
Also, later in that thread I also wrote[1] what seems to be the core of
what you've done here: namely, allow platform_devices and
platform_drivers to to be used on custom busses. Patch is at the end of
this mail with a more focused changelog. As Greg suggested in his reply
to your first version, this part could be merged today, and the
platform_bus_init stuff could be added later, after some more review.
Some comments below...
I can split this into 2 patches.
Yes, I think that would be better.
Was your patch sent to linux-kernel or just linux-omap? I'm not on
linux-omap...
That thread was on linux-arm-kernel and linux-omap
[snip]
Which will allow the same driver to easily to used on either
the platform bus or the newly defined bus type.
Except it requires a re-compile.
Rather than doing this at compile time, it would be better to support
legacy devices at runtime. You could handle this by simply registering
the driver on the custom bus and the platform_bus and let the bus
matching code handle it. Then, the same binary would work on both
legacy and updated SoCs.
Can you safely register a driver on more than one bus? I didn't think
that was safe -- normally it's impossible since you're calling
struct BUS_TYPE_driver mydriver;
BUS_TYPE_driver_register(mydriver)
but now we have multiple bus types that are all actually platform type;
still,
at a minimum you would need:
struct platform_driver mydrvier1 = {
.driver.bus = sub_bus1,
};
struct platform_driver mydrvier2 = {
.driver.bus = sub_bus2,
};
which would all point to the same driver functions, yet the respective
devices
attached for the same driver would be on different buses. I fear this
might
confuse some drivers. I don't think dynamic bus assignment is this easy
In short: I do not believe the same driver can be registered on multiple
different buses -- if this is wrong, please correct me.
It is possible, and currently done in powerpc land where some
drivers handle devices on the platform_bus and the custom OF bus.
As of now, the of_platform_bus_type has been removed. It was a bad
idea because it tried to encode non-bus-specific information into
something that was just a clone of the platform_bus. Drivers that
worked on both had to be bound to both busses. I do actually have
code that automatically registers a driver on more than one bus, but
it is rather a hack and was only a temporary measure.
The relevant question before going down this path is, Is the
omap/sh/other-soc behaviour something fundamentally different from the
platform bus? Or is it something complementary that would be better
handled with a notifier or some orthogonal method of adding new
behaviour?
I don't have a problem with multiple platform_bus instances using the
same code (I did suggest it after all), but I do worry about muddying
the Linux device model or making it overly complex. Binding single
drivers to multiple device types could be messy.
Cheers,
g.
From your other email, the distinction of /bus_types/ versus /physical
or logical buses/ was very valuable (thanks). I am becoming less
convinced that I need this infrastructure or the ability to create
pseudo-platform buses. Let me outline some thoughts below, which I
think at least solves my problems ( ;) ), and if some of the OMAP folks
and Alan could chime in as to whether they can apply something similar,
or if they still have need of creating actual new bus types?
As we are exploring all of this, let me put a concrete (ish) scenario
out there to discuss:
SUB_BUS1---DEVICE1
/
CPU ---
\
SUB_BUS2---DEVICE2
DEVICE1 and DEVICE2 are the same device (say, usb host controller, or
whatever), and they should be driven by the same driver. However,
physically they are attached to different buses.
SUB_BUS1 and SUB_BUS2 are *different* and let's say require a different
suspend method. If I understand correctly, the right way to build the
topology would be:
struct device_type SUB_BUS1_type = {
.name = sub-bus1-type,
.pm = sub_bus1_pm_ops;
};
struct platform_device SUB_BUS1 = {
.init_name = sub-bus1,
.dev.type = sub_bus1_type,
};
struct platform_device DEVICE1 = {
.name = device1,
.dev.parent = SUB_BUS1.dev,
};
platform_device_register(SUB_BUS1);
platform_device_register(DEVICE1);
[analogous for *2]
which would yield:
/sys/bus/platform/devices/
|
|-SUB_BUS1
| |
| |-DEVICE1
|
|-SUB_BUS2
| |
| |-DEVICE2
Which is the
[PATCH] platform: Facilitate the creation of pseduo-platform busses
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
RFC: http://lkml.org/lkml/2010/8/3/496
Patch is unchanged from the RFC. Reviews seemed generally positive
and it seemed this was desired functionality.
INTRO
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 my_bus_type = {
.name = mybus,
};
EXPORT_SYMBOL_GPL(my_bus_type);
struct platform_device sub_bus1 = {
.name = sub_bus1,
.id = -1,
.dev.bus= my_bus_type,
}
EXPORT_SYMBOL_GPL(sub_bus1);
struct platform_device sub_bus2 = {
.name = sub_bus2,
.id = -1,
.dev.bus= my_bus_type,
}
EXPORT_SYMBOL_GPL(sub_bus2);
static int __init my_bus_init(void)
{
int error;
platform_bus_type_init(my_bus_type);
error = bus_register(my_bus_type);
if (error)
return error;
error = platform_device_register(sub_bus1);
if (error)
goto fail_sub_bus1;
error = platform_device_register(sub_bus2);
if (error)
goto fail_sub_bus2;
return error;
fail_sub_bus2:
platform_device_unregister(sub_bus1);
fail_sub_bus2:
bus_unregister(my_bus_type);
return error;
}
postcore_initcall(my_bus_init);
EXPORT_SYMBOL_GPL(my_bus_init);
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus= my_bus_type,
},
};
/somewhere/my_device.c
static struct platform_device my_device = {
.name = my-device,
.id = -1,
.dev.bus= my_bus_type,
.dev.parent = sub_bus_1.dev,
};
Notice that for a device / driver, only 3 lines were added to
switch from the platform bus to the new my_bus. This is
especially valuable if we consider supporting a legacy SOCs
and new SOCs where the same driver is used, but may need to
be on either the platform bus or the new my_bus. The above
code then becomes:
(possibly in a header)
#ifdef CONFIG_MY_BUS
#define MY_BUS_TYPE my_bus_type
#else
#define MY_BUS_TYPE NULL
#endif
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus= MY_BUS_TYPE,
},
};
Which will allow the same driver to easily to used on either
the platform bus or the newly defined bus type.
This will build a device tree that mirrors the actual configuration:
/sys/bus
|-- my_bus
| |-- devices
| | |-- sub_bus1 - ../../../devices/platform/sub_bus1
| | |-- sub_bus2 - ../../../devices/platform/sub_bus2
| | |-- my-device - ../../../devices/platform/sub_bus1/my-device
| |-- drivers
| | |-- my-driver
Signed-off-by: Patrick Pannuto ppann...@codeaurora.org
---
drivers/base/platform.c | 30 ++
include/linux/platform_device.h |2 ++
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4d99c8b..c86be03 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -241,7 +241,8 @@ int platform_device_add(struct platform_device *pdev)
if (!pdev-dev.parent)
pdev-dev.parent = platform_bus;
- pdev-dev.bus = platform_bus_type;
+ if (!pdev-dev.bus)
+ pdev-dev.bus = platform_bus_type;
if (pdev-id != -1)
dev_set_name(pdev-dev, %s.%d, pdev-name, pdev-id);
@@ -482,7 +483,8 @@ static void platform_drv_shutdown(struct device *_dev)
*/
int platform_driver_register(struct platform_driver *drv)
{
- drv-driver.bus = platform_bus_type;
+ if (!drv-driver.bus)
+ drv-driver.bus = platform_bus_type;
if (drv-probe)
drv-driver.probe = platform_drv_probe;
if (drv-remove)
@@ -539,12 +541,12 @@ int __init_or_module
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
Patrick Pannuto ppann...@codeaurora.org writes:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
Also, later in that thread I also wrote[1] what seems to be the core of
what you've done here: namely, allow platform_devices and
platform_drivers to to be used on custom busses. Patch is at the end of
this mail with a more focused changelog. As Greg suggested in his reply
to your first version, this part could be merged today, and the
platform_bus_init stuff could be added later, after some more review.
Some comments below...
INTRO
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 my_bus_type = {
.name = mybus,
};
EXPORT_SYMBOL_GPL(my_bus_type);
struct platform_device sub_bus1 = {
.name = sub_bus1,
.id = -1,
.dev.bus= my_bus_type,
}
EXPORT_SYMBOL_GPL(sub_bus1);
struct platform_device sub_bus2 = {
.name = sub_bus2,
.id = -1,
.dev.bus= my_bus_type,
}
EXPORT_SYMBOL_GPL(sub_bus2);
static int __init my_bus_init(void)
{
int error;
platform_bus_type_init(my_bus_type);
error = bus_register(my_bus_type);
if (error)
return error;
error = platform_device_register(sub_bus1);
if (error)
goto fail_sub_bus1;
error = platform_device_register(sub_bus2);
if (error)
goto fail_sub_bus2;
return error;
fail_sub_bus2:
platform_device_unregister(sub_bus1);
fail_sub_bus2:
bus_unregister(my_bus_type);
return error;
}
postcore_initcall(my_bus_init);
EXPORT_SYMBOL_GPL(my_bus_init);
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus= my_bus_type,
},
};
/somewhere/my_device.c
static struct platform_device my_device = {
.name = my-device,
.id = -1,
.dev.bus= my_bus_type,
.dev.parent = sub_bus_1.dev,
};
Notice that for a device / driver, only 3 lines were added to
switch from the platform bus to the new my_bus. This is
especially valuable if we consider supporting a legacy SOCs
and new SOCs where the same driver is used, but may need to
be on either the platform bus or the new my_bus. The above
code then becomes:
(possibly in a header)
#ifdef CONFIG_MY_BUS
#define MY_BUS_TYPE my_bus_type
#else
#define MY_BUS_TYPE NULL
#endif
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus= MY_BUS_TYPE,
},
};
Which will allow the same driver to easily to used on either
the platform bus or the newly defined bus type.
Except it requires a re-compile.
Rather than doing this at compile time, it would be better to support
legacy devices at runtime. You could handle this by simply registering
the driver on the custom bus and the platform_bus and let the bus
matching code handle it. Then, the same binary would work on both
legacy and updated SoCs.
This will build a device tree that mirrors the actual configuration:
/sys/bus
|-- my_bus
| |-- devices
| | |-- sub_bus1 - ../../../devices/platform/sub_bus1
| | |-- sub_bus2 - ../../../devices/platform/sub_bus2
| | |-- my-device - ../../../devices/platform/sub_bus1/my-device
| |-- drivers
| | |-- my-driver
Signed-off-by: Patrick Pannuto ppann...@codeaurora.org
---
drivers/base/platform.c | 30 ++
include/linux/platform_device.h |2 ++
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4d99c8b..c86be03 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -241,7 +241,8 @@ int platform_device_add(struct platform_device *pdev)
if (!pdev-dev.parent)
pdev-dev.parent =
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
On 08/04/2010 05:16 PM, Kevin Hilman wrote:
Patrick Pannuto ppann...@codeaurora.org writes:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
Also, later in that thread I also wrote[1] what seems to be the core of
what you've done here: namely, allow platform_devices and
platform_drivers to to be used on custom busses. Patch is at the end of
this mail with a more focused changelog. As Greg suggested in his reply
to your first version, this part could be merged today, and the
platform_bus_init stuff could be added later, after some more review.
Some comments below...
I can split this into 2 patches.
Was your patch sent to linux-kernel or just linux-omap? I'm not on linux-omap...
[snip]
Which will allow the same driver to easily to used on either
the platform bus or the newly defined bus type.
Except it requires a re-compile.
Rather than doing this at compile time, it would be better to support
legacy devices at runtime. You could handle this by simply registering
the driver on the custom bus and the platform_bus and let the bus
matching code handle it. Then, the same binary would work on both
legacy and updated SoCs.
Can you safely register a driver on more than one bus? I didn't think
that was safe -- normally it's impossible since you're calling
struct BUS_TYPE_driver mydriver;
BUS_TYPE_driver_register(mydriver)
but now we have multiple bus types that are all actually platform type; still,
at a minimum you would need:
struct platform_driver mydrvier1 = {
.driver.bus = sub_bus1,
};
struct platform_driver mydrvier2 = {
.driver.bus = sub_bus2,
};
which would all point to the same driver functions, yet the respective devices
attached for the same driver would be on different buses. I fear this might
confuse some drivers. I don't think dynamic bus assignment is this easy
In short: I do not believe the same driver can be registered on multiple
different buses -- if this is wrong, please correct me.
Up to here, this looks exactly what I wrote in thread referenced above.
It is, you just went on vacation :)
if (code != retval)
platform_driver_unregister(drv);
@@ -1017,6 +1019,26 @@ struct bus_type platform_bus_type = {
};
EXPORT_SYMBOL_GPL(platform_bus_type);
+/** platform_bus_type_init - fill in a pseudo-platform-bus
+ * @bus: foriegn bus type
+ *
+ * This init is basically a selective memcpy that
+ * won't overwrite any user-defined attributes and
+ * only copies things that platform bus defines anyway
+ */
minor nit: kernel doc style has wrong indentation
will fix
+void platform_bus_type_init(struct bus_type *bus)
+{
+if (!bus-dev_attrs)
+bus-dev_attrs = platform_bus_type.dev_attrs;
+if (!bus-match)
+bus-match = platform_bus_type.match;
+if (!bus-uevent)
+bus-uevent = platform_bus_type.uevent;
+if (!bus-pm)
+bus-pm = platform_bus_type.pm;
+}
+EXPORT_SYMBOL_GPL(platform_bus_type_init);
With this approach, you should note in the comments/changelog that
any selective customization of the bus PM methods must be done after
calling platform_bus_type_init().
No they don't. If you call platform_bus_type_init first then you'll
just overwrite them with new values; if you call it second then they
will all already be well-defined and thus not overwritten.
Also, You've left out the legacy PM methods here. That implies that
moving a driver from the platform_bus to the custom bus is not entirely
transparent. If the driver still has legacy PM methods, it would stop
working on the custom bus.
While this is good motivation for converting a driver to dev_pm_ops, at
a minimum it should be clear in the changelog that the derivative busses
do not support legacy PM methods. However, since it's quite easy to do,
and you want the derivative busses to be *exactly* like the platform bus
except where explicitly changed, I'd suggest you also check/copy the
legacy PM methods.
In addition, you've missed several fields in 'struct bus_type'
(bus_attr, drv_attr, p, etc.) Without digging deeper into the driver
core, I'm not sure if they are all needed at init time, but it should be
clear in the comments why they can be excluded.
I copied everything that was defined for platform_bus_type:
struct bus_type platform_bus_type = {
.name = platform,
.dev_attrs = platform_dev_attrs,
.match = platform_match,
.uevent = platform_uevent,
.pm = platform_dev_pm_ops,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
struct bus_type {
const char *name;
struct bus_attribute*bus_attrs;
struct device_attribute *dev_attrs;
struct driver_attribute *drv_attrs;
int (*match)(struct device *dev, struct
Re: [PATCH] platform: Facilitate the creation of pseduo-platform busses
On Thu, Aug 5, 2010 at 7:14 AM, Patrick Pannuto ppann...@codeaurora.org wrote:
Inspiration for this comes from:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
RFC: http://lkml.org/lkml/2010/8/3/496
Patch is unchanged from the RFC. Reviews seemed generally positive
and it seemed this was desired functionality.
Thanks for your patch, it's really nice to see work done in this area!
I'd like to see something like this merged in the not so distant
future. At this point I'm not so concerned about the details, so I'll
restrict myself to this:
/drivers/my_driver.c
static struct platform_driver my_driver = {
.driver = {
.name = my-driver,
.owner = THIS_MODULE,
.bus = my_bus_type,
},
};
I would really prefer not to have the bus type in the here. I
understand it's needed at this point, but I wonder if it's possible to
adjust the device-driver matching for platform devices to allow any
type of pseudo-platform bus_type.
The reason why I'd like to avoid having the bus type in the driver is
that I'd like to reuse the platform driver across multiple
architectures and buses. For instance, on the SH architecture and
SH-Mobile ARM we have SoCs with SCIF hardware blocks driven by the
sh-sci.c serial driver. The sh-sci.c platform driver supports a wide
range of different SCI(F)(A)(B) hardware blocks, and on any given SoC
there is a mix of SCIF blocks spread out on different buses.
At this point our SH platform drivers are unaware where their driver
instanced are located on the SoC. The I/O address and IRQs are
assigned via struct resource and clocks are managed through clkdev. I
believe that adding the bus type in the driver will violate this
abstraction and make it more difficult to just instantiate a driver
somewhere on the SoC.
/somewhere/my_device.c
static struct platform_device my_device = {
.name = my-device,
.id = -1,
.dev.bus = my_bus_type,
.dev.parent = sub_bus_1.dev,
};
This I don't mind at all. Actually, this is the place where the
topology should be defined IMO.
Cheers,
/ magnus
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
