From: Timothy Meade on
On Tue, Aug 3, 2010 at 8:17 PM, Patrick Pannuto <ppannuto(a)quicinc.com> wrote:
>>>>>
>>>>> � �struct platform_device sub_bus1 = {
>>>>> � � � � � �.name � � � � � = "sub_bus1",
>>>>> � � � � � �.id � � � � � � = -1,
>>>>> � � � � � �.dev.bus � � � �= &my_bus_type,
>>>>> � �}
>>>>> � �EXPORT_SYMBOL_GPL(sub_bus1);
>>>>
>>>> You really want a bus hanging off of a bus? �Normally you need a device
>>>> to do that, which is what I think you have here, but the naming is a bit
>>>> odd to me.
>>>>
>>>> What would you do with this "sub bus"? �It's just a device, but you are
>>>> wanting it to be around for something.
>>>>
>>>
>>> It's for power management stuff, basically, there are actual physical buses
>>> involved that can be completely powered off IFF all of their devices are
>>> not in use. Plus it actually matches bus topology this way.
>>
>> Then create a real bus hanging off of a device, not another device that
>> "acts" like a bus here, right? �Or am I missing the point?
>>
>
> The motivation for doing it this was is that one driver could drive
> devices on two different subbusses. �In the example, "my-driver" could
> drive a device on sub_bus1 AND sub_bus2 (if there were 2+ devices, one
> or more on each bus).
>
> From my understanding, this is not possible if they are actually
> different busses.

This could be quite useful on the Qualcomm devices where there are
many collections of "devices" that resemble a bus but cannot be
directly enumerated but are initialized by platform code or possibly
in future, a device tree. One such bus is made up of SMI devices that
are identified to the applications core by the modem firmware and can
be in the form of character devices (smd), network devices (rmnet) rpc
router channel and others, we also have to deal with the MDDI video
bus which represented a challenge when migrating the HTC Rhodium to
2.6.32 as each mdp device (and others in later kernels) are added to
the platform bus dynamically, though they don't appear similararly
group in sysfs due to not actaully being on a bus. While it would
have been possible to implement an mddi bus, the work would have
essentially recreated the platform bus with a new name. A simliar
challenge exists for exposing rpc endpoints to userspace as the
current codebase uses character devices rather than introducing a new
network protocol to the kernel, if such as bus was exposed through
sysfs a userspace daemon could easily bind a GPS library to the PDAPI
endpoint or similar features requiring less configuration to adapt to
new AMSS firmware or device name layout.

Thank you,
Timothy Meade
tmzt #htc-linux
facebook.com/HTCLinux
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Grant Likely on
On Tue, Aug 3, 2010 at 5:56 PM, Greg KH <gregkh(a)suse.de> wrote:
> On Tue, Aug 03, 2010 at 04:35:06PM -0700, Patrick Pannuto wrote:
>> @@ -539,12 +541,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
>> � � � �* if the probe was successful, and make sure any forced probes of
>> � � � �* new devices fail.
>> � � � �*/
>> - � � spin_lock(&platform_bus_type.p->klist_drivers.k_lock);
>> + � � spin_lock(&drv->driver.bus->p->klist_drivers.k_lock);
>> � � � drv->probe = NULL;
>> � � � if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list))
>> � � � � � � � retval = -ENODEV;
>> � � � drv->driver.probe = platform_drv_probe_fail;
>> - � � spin_unlock(&platform_bus_type.p->klist_drivers.k_lock);
>> + � � spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock);
>>
>> � � � if (code != retval)
>> � � � � � � � platform_driver_unregister(drv);
>
> I'm guessing that this chunk can be applied now, right?

Yes, I agree. This chunk makes sense independently of the rest of the patch.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Grant Likely on
On Tue, Aug 3, 2010 at 6:17 PM, Patrick Pannuto <ppannuto(a)quicinc.com> wrote:
>>>>>
>>>>> � �struct platform_device sub_bus1 = {
>>>>> � � � � � �.name � � � � � = "sub_bus1",
>>>>> � � � � � �.id � � � � � � = -1,
>>>>> � � � � � �.dev.bus � � � �= &my_bus_type,
>>>>> � �}
>>>>> � �EXPORT_SYMBOL_GPL(sub_bus1);
>>>>
>>>> You really want a bus hanging off of a bus? �Normally you need a device
>>>> to do that, which is what I think you have here, but the naming is a bit
>>>> odd to me.
>>>>
>>>> What would you do with this "sub bus"? �It's just a device, but you are
>>>> wanting it to be around for something.
>>>>
>>>
>>> It's for power management stuff, basically, there are actual physical buses
>>> involved that can be completely powered off IFF all of their devices are
>>> not in use. Plus it actually matches bus topology this way.
>>
>> Then create a real bus hanging off of a device, not another device that
>> "acts" like a bus here, right? �Or am I missing the point?
>>
>
> The motivation for doing it this was is that one driver could drive
> devices on two different subbusses. �In the example, "my-driver" could
> drive a device on sub_bus1 AND sub_bus2 (if there were 2+ devices, one
> or more on each bus).
>
> From my understanding, this is not possible if they are actually
> different busses.

It sounds an awful lot like 2 separate concepts are being conflated
here. In Linux terminology, we have two things:

/bus types/ describe the common behavour of similarly attached things.
/physical or logical busses/ are where devices are actually attached
registered to the system.

These are not the same thing.

The primary thing that a bus-type provides is the set of device driver
that can be bound to the device plus some common functionality.
Almost every struct device has the 'struct bus_type bus' field set so
that it can be bound against a device driver when it is registered.

Every struct device also has a 'struct device *parent' field. The
parent field denotes where the device lives in the Linux device model
(see /sys/devices/*), but it says *nothing* about what the bus type of
device it is. Remember I mentioned physical/logical busses earlier?
Well the core Linux device model doesn't really have a concept of bus
instances, it only knows about devices and bus types. The .parent
field is the closest thing the device model has to a bus instance, but
the model places no constraints on the bus type of each child 'struct
device'.

(caveat: many bus_types do have a concept of bus instances; but that
is specific to the bus_type. The core device model doesn't care.
Also, in practical terms, yes most sibling devices will also be of the
same bus_type, but it is not a given, and it is not enforced.)

For example, a system can have many physical pci busses, but all the
devices hanging off those busses are still just registered on
pci_bus_type. The parent of those devices, however, reflects the
physical bus the device is attached to.

In fact, the bus type has absolutely no bearing on where a device
actually lives in the Linux driver model. A common misunderstanding
is that all platform devices live under /sys/devices/platform.

This also is not true.

By default the platform bus will put devices without a parent into
/sys/devices/platform, but if parent is set, then the device will be
registered where the parent pointer tells it to be registered. It is
totally okay to have the following structure (off the top of my head):

system_bus (platform_bus_type, registers a bunch of child devices)
|--pci_bus (platform_bus_type; implements a pci host bridge)
| |--usb host controller 1 (pci_bus_type, implements a usb bus instance)
| |--usb host controller 2 (pci_bus_type, implements a usb bus instance)
| |--fpga (pci_bus_type, implements a bunch of devices)
| |--spi controller (platform_bus_type)
| |--custom peripheral (platform_bus_type)
|--uart (platform_bus_type)
|--uart (platform_bus_type)
|--ethernet (platform_bus_type)
|--external bus (platform_bus_type; represents and external local bus)
| |--i2c controller (platform_bus_type)
| | |--eeprom (i2c_bus_type)
| | |--temperature sensor (i2c_bus_type)
|--gpio controller (platform_bus_type)

Platform devices can show up just about anywhere in the tree.

(On that point Greg, what is the reason for even having the
/sys/devices/platform/ parent? Why not just let the platform devices
sit at the root of the device tree? In the OF case (granted, I'm
biased) all of the platform_device registrations reflect the actual
device hierarchy expressed in the device tree data.)

So, the *only* reason to have a different bus_type is if the actual
behaviour of the bus is different. A different bus type wouldn't even
cause a different topology if doing so wouldn't make sense. For
example in the OMAP case, the topology might look like:

system_bus (platform_bus_type, registers a bunch of child devices)
| ...
|--uart (omap_bus_type)
|--uart (omap_bus_type)
|--ethernet (platform_bus_type)

See? Devices of different types can often be siblings at the same
device layer. (note: not *always* true. PCI busses for instance
always expect to only host pci_devices and visa-versa, but for simple
platform-like busses it may be true).

Now, having gone on this whole long tirade, it looks like having
separate platform bus types may not be the best approach after all.
Magnus and Kevin are exploring using devres to handle implementing the
system-specific behaviour. It is certainly an area for more research
and experimental implementations.

g.

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Grant Likely on
On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh(a)suse.de> wrote:
> On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote:
>> (On that point Greg, what is the reason for even having the
>> /sys/devices/platform/ parent? �Why not just let the platform devices
>> sit at the root of the device tree? �In the OF case (granted, I'm
>> biased) all of the platform_device registrations reflect the actual
>> device hierarchy expressed in the device tree data.)
>
> If we sat them at the "root", there would be a bunch of them there. �I
> don't know, we could drop the parent, I guess whoever created the
> platform device oh so long ago, decided that it would look nicer to be
> in this type of structure.

Personally I'd rather see a meaningful structure used here. Maybe
having them all in the root will encourage people to find realistic
parents for their platform devices. :-) Why don't I float a patch to
remove this and see if anybody freaks out. Should I wrap it with a
CONFIG_ so that it can be configurable for a release or to, or just
make it unconditional?

>> Now, having gone on this whole long tirade, it looks like having
>> separate platform bus types may not be the best approach after all.
>
> I totally agree, and thanks for the detailed explaination, it saved me
> from having to write up the same thing :)

:-)

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Grant Likely on
On Fri, Aug 6, 2010 at 5:46 PM, Greg KH <gregkh(a)suse.de> wrote:
> On Fri, Aug 06, 2010 at 09:12:27AM -0600, Grant Likely wrote:
>> On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh(a)suse.de> wrote:
>> > On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote:
>> >> (On that point Greg, what is the reason for even having the
>> >> /sys/devices/platform/ parent? �Why not just let the platform devices
>> >> sit at the root of the device tree? �In the OF case (granted, I'm
>> >> biased) all of the platform_device registrations reflect the actual
>> >> device hierarchy expressed in the device tree data.)
>> >
>> > If we sat them at the "root", there would be a bunch of them there. �I
>> > don't know, we could drop the parent, I guess whoever created the
>> > platform device oh so long ago, decided that it would look nicer to be
>> > in this type of structure.
>>
>> Personally I'd rather see a meaningful structure used here. �Maybe
>> having them all in the root will encourage people to find realistic
>> parents for their platform devices. �:-)
>
> That would be nice, but take your "standard" PC today:
> � � � �> ls /sys/devices/platform/
> � � � �Fixed MDIO bus.0 �i8042 �pcspkr �power �serial8250 �uevent vesafb.0
>
> There are tty devices below the serial port, which is nice to see, but
> the others? �I don't know what type of bus they would be on, do you?

Does it matter? On my PC I count 7 devices. On my servers, I see the
same. I don't personally don't see any disadvantage to having them at
the root.

However, looking at them:

"Fixed MDIO bus" is actually a complete hack that I'm going to try and
get rid of.
i8042 is keyboard/mouse that probably lives on the southbridge. I
imagine hanging off the PCI bus in the ISA IO range.
ditto pcspkr
ditto serial8250
ditto vesafb

I wouldn't have any problem modifying those specific drivers to
register under something like /sys/devices/legacy, but I don't really
think it is in any way necessary.

I suppose one *could* try to figure out the actual PCI topology that
leads to these devices, but that gets into trying to guess what the
BIOS set up to decide where to register those things. Probably a lot
of error-prone work for absolutely no benefit. :-)

However, on the embedded side I'm seeing a lot of devices show up
under /sys/devices/platform. Embedded is generally different. We
*know* what the bus topology is. However, it seems to me that many
developers are under the mistaken assumption that platform devices
must live in /sys/devices/platform, so they don't bother reflecting
the topology in the device model and it seems to be affecting how
embedded PM is being approached (my opinion). Removing the "struct
device platform_bus" may reduce the confusion.

>> Why don't I float a patch to remove this and see if anybody freaks
>> out. �Should I wrap it with a CONFIG_ so that it can be configurable
>> for a release or to, or just make it unconditional?
>
> If you can figure out a structure for the desktop/server machines, sure,
> I say just always enable it :)

I've got a patch in my tree now. I'll play with it a bit before
posting it for RFC.

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