Re: [Ksummit-discuss] [TECH TOPIC] Driver model/resources, ACPI, DT, etc (sigh)

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Monday, May 26, 2014 11:19:26 PM Linus Walleij wrote:
> On Mon, May 19, 2014 at 1:56 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Saturday, May 17, 2014 02:24:23 PM Mark Brown wrote:
> 
> >> That's much less invasive than making new APIs.
> >
> > But for drivers that call of_get_something() directly I don't see any other
> > sensible way forward.  Otherwise each of them will need to do something like
> >
> > if (dev->of_node)
> >         use of_get_X()
> > else if (ACPI_COMPANION(dev))
> >         use acpi_get_X()
> > else
> >         use black magic
> 
> So there is currently net/rfkill/rfkill-gpio.c,
> and there is a patch to add device tree support out there:
> http://marc.info/?l=devicetree&m=139754665715639&w=2
> 
> And after that it looks like that:
> 
> static int rfkill_gpio_acpi_probe(struct device *dev,
>                                   struct rfkill_gpio_data *rfkill)
> {
>         const struct acpi_device_id *id;
> 
>         id = acpi_match_device(dev->driver->acpi_match_table, dev);
>         if (!id)
>                 return -ENODEV;
> 
>         rfkill->name = dev_name(dev);
>         rfkill->type = (unsigned)id->driver_data;
> 
>         return 0;
> }
> 
> static int rfkill_gpio_dt_probe(struct device *dev, struct
> rfkill_gpio_data *rfkill)
> {
>         struct device_node * np = dev->of_node;
> 
>         rfkill->name = np->name;
>         of_property_read_string(np, "rfkill-name", &rfkill->name);
>         of_property_read_u32(np, "rfkill-type", &rfkill->type);
> 
>         return 0;
> }
> 
> static int rfkill_gpio_probe(struct platform_device *pdev)
> {
> (...)
>         if (ACPI_HANDLE(&pdev->dev)) {
>                 ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
>                 if (ret)
>                         return ret;
>         } else if (&pdev->dev.of_node) {
>                ret = rfkill_gpio_dt_probe(&pdev->dev, rfkill);
>                if (ret)
>                        return ret;
>         } else if (pdata) {
>                 rfkill->name = pdata->name;
>                 rfkill->type = pdata->type;
>         } else {
>                 return -ENODEV;
>         }
> }
> 
> Whopee-do. The semantic differences are not so subtle. In
> one case the type is defined in the driver itself by ACPI's match
> data, in the DT case it is explicitly provided in a node in the DT
> itself. DT can also explicitly name the rfkill, whereas ACPI will
> just use the device name.
> 
> What should we do? How will the interface look? I mean for real.

This is talking about *current* ACPI, which may be different from future
ACPI (where I mean 5.1).  And future ACPI may be able to do the same thing
as DT does.  So why don't we get back to that when ACPI 5.1 is out
(which should be some time in the summer)?

> This is the discussion we need to have. Admittedly I do not see anything
> clean enough :-(
> 
> And this is a DEAD SIMPLE example.
> 
> And even if there can be something clean and nice in the probe path,
> we still have this:
> 
> #ifdef CONFIG_ACPI
> static const struct acpi_device_id rfkill_acpi_match[] = {
>         { "BCM2E1A", RFKILL_TYPE_BLUETOOTH },
>         { "BCM2E39", RFKILL_TYPE_BLUETOOTH },
>         { "BCM2E3D", RFKILL_TYPE_BLUETOOTH },
>         { "BCM4752", RFKILL_TYPE_GPS },
>         { "LNV4752", RFKILL_TYPE_GPS },
>         { },
> };
> #endif
> #ifdef CONFIG_OF
> static const struct of_device_id rfkill_of_match[] = {
>        { .compatible = "rfkill-gpio", },
>        {},
> };
> #endif
> (...)
>                 .acpi_match_table = ACPI_PTR(rfkill_acpi_match),
>                 .of_match_table = of_match_ptr(rfkill_of_match),
> (...)
> 
> This stuff we don't even talk about unifiing right?

We haven't been talking about that so far.

Rafael