On 10/18/2012 09:29 AM, Lars Poeschel wrote: > On Tuesday 16 October 2012 at 12:58:48, Lars-Peter Clausen wrote: >> On 10/16/2012 11:43 AM, Lars Poeschel wrote: >>> On Tuesday 16 October 2012 at 10:40:26, Lars-Peter Clausen wrote: >>>> On 10/12/2012 04:34 PM, Lars Poeschel wrote: >>>> Btw. I'm wondering why is the extra platform device required? Can't you >>>> not just use the usb device as the parent device for the mfd cells? >>> >>> This is what I first did, but this does not work. You can read about my >>> first thoughts why this is not working here: (To sum it up: The device >>> is housed in an usb_device, not a platform_device and This usb_device >>> has no mfd_cell member.) >>> >>> https://lkml.org/lkml/2012/9/28/327 >>> >>> As I got a bit more deeper I also noticed, that mfd_add_devices >>> (obviously) adds the devices "as childs" to the parent device. >>> mfd_remove_devices then removes ALL "child" devices from the parent, not >>> only those added by mfd_add_devices before. This does not work in the >>> case of the usb parent device, because it has other childs that the usb >>> layer added before (some endpoints and stuff). So I had to construct an >>> "empty" (in sense of childs) mock platform_device between the usb and >>> mfd. >> >> Ah, ok that makes sense. I was a bit confused, because there are other mfd >> drivers with for example i2c or spi devices as parents and these work fine, >> but I guess this is because non of them registers any additional child >> devices. I guess it makes sense to create a mfd cell device type and assign >> this type to newly created mfd cells and only unregister a device in >> mfd_remove_devices if it has the correct device type. >> >> E.g. something along the lines of: >> >> >> --- a/drivers/mfd/mfd-core.c >> +++ b/drivers/mfd/mfd-core.c >> @@ -21,6 +21,10 @@ >> #include <linux/irqdomain.h> >> #include <linux/of.h> >> >> +static struct device_type mfd_device_type = { >> + .name = "mfd-cell", >> +}; >> + >> int mfd_cell_enable(struct platform_device *pdev) >> { >> const struct mfd_cell *cell = mfd_get_cell(pdev); >> @@ -91,6 +95,7 @@ static int mfd_add_device(struct device *parent, int id, >> goto fail_device; >> >> pdev->dev.parent = parent; >> + pdev->dev.type = &mfd_device_type; >> >> if (parent->of_node && cell->of_compatible) { >> for_each_child_of_node(parent->of_node, np) { >> @@ -204,10 +209,16 @@ EXPORT_SYMBOL(mfd_add_devices); >> >> static int mfd_remove_devices_fn(struct device *dev, void *c) >> { >> - struct platform_device *pdev = to_platform_device(dev); >> - const struct mfd_cell *cell = mfd_get_cell(pdev); >> + struct platform_device *pdev; >> + const struct mfd_cell *cell; >> atomic_t **usage_count = c; >> >> + if (dev->type != &mfd_device_type) >> + return 0; >> + >> + pdev = to_platform_device(dev); >> + cell = mfd_get_cell(pdev); >> + >> /* find the base address of usage_count pointers (for freeing) */ >> if (!*usage_count || (cell->usage_count < *usage_count)) >> *usage_count = cell->usage_count; > > I thought about this and I am not fully happy with it: > If we add the mfd devices to the usb_interface parent they are at the same > level in the device tree as the usb endpoints and stuff. I would consider > this > logically wrong. > Is this something we should take care of ?
I wouldn't worry to much about it. If you use the the container platform device the container platform device would be at the same level as the usb endpoints. I did a quick search and it seams that other subsystems also register the child devices directly on the usb interface device. E.g. the media subsystem uses this a lot. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/