Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [RFC PATCH v2 07/16] gpio: Add support for unified device properties interface | Date | Tue, 23 Sep 2014 18:26:07 +0200 |
| |
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); return ERR_PTR(-ENXIO); }
would keep either side of it relatively simple, by leaving out the indirect function calls and new for_each_available_child_of_node() macro.
How many other users of fw_dev_node do you have at the moment?
Arnd
| |