On 9/23/14, 9:26, "Arnd Bergmann" <a...@arndb.de> wrote: >On Tuesday 23 September 2014 18:25:01 Rafael J. Wysocki wrote: >> The problem is iteration over child nodes of a given one where there >> may not be struct device objects. >> >> For example (from patch [2/16]): >> >> +int acpi_for_each_child_node(struct acpi_device *adev, >> + int (*fn)(struct fw_dev_node *fdn, void >>*data), >> + void *data) >> +{ >> + struct acpi_device *child; >> + int ret = 0; >> + >> + list_for_each_entry(child, &adev->children, node) { >> + struct fw_dev_node fdn = { .acpi_node = child, }; >> + >> + ret = fn(&fdn, data); >> + if (ret) >> + break; >> + } >> + return ret; >> +} >> >> and then fn() can be made work for both DTs and ACPI. Without this we'd >> need to have two versions of fn(), one for DTs and one for ACPI (and >>possibly >> more for some other FW protocols), which isn't necessary in general (and >> duplicates code etc.). >> >> That actually is used by some patches down in the series (eg. [10/16]). >> > >Ok, I understand what you are doing now. > >Looking at the example you point to >(http://www.spinics.net/lists/devicetree/msg49502.html), I still feel >that this is adding more abstraction than what is good for us, and >I'd be happier with an implementation of gpio_leds_create() that >has a bit more duplication and less abstraction. > >The important part should be that the driver-side interface is >sensible, other than that an implementation like > >static struct gpio_leds_priv *gpio_leds_create(struct platform_device >*pdev) >{ > if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) > return gpio_leds_create_of(pdev); > else if (IS_ENABLED(CONFIG_ACPI)) > return gpio_leds_create_of(acpi);
Arnd, I think you meant: Return gpio_leds_create_acpi(pdev) ? This is what we did early on to prototype this concept, but the problem with this approach we duplicate all of the creation code, which leads to maintenance errors, and is inconsistent with the goals of the _DSD which is to reuse the same schemas for ACPI and FDT. If we have separate pdata creation functions anyway, we are leaving much of the advantage of the common schema on the table. Namely the ability to reuse drivers relatively easily across firmware implementations. We don't want driver authors to have to care if it's ACPI or FDT. We would have preferred to have deprecated the of property interface in favor of the new generic device_property interface, but Grant specifically requested that we update drivers individually rather than all at once, which means we can't just kill the OF interface. We agreed to that, somewhat reluctantly as it adds more work in updating the drivers over time which will slow adoption, but I understand the desire not to make large sweeping changes due to the risk of breaking things inadvertently as we cannot expect to be able to test all of them. That said, I don't want to forget that the goal is to use the common interface over time as we convert individual drivers, and using the common interface means we need a common iterator function and that we not have fw implementation specific pdata create functions. -- Darren Hart Intel Open Source Technology Center -- 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/