Re: [PATCH] ACPI / GED: use late init to allow other drivers init

[Rafael J. Wysocki]

On Mon, May 15, 2017 at 4:36 AM, Sinan Kaya <okaya@xxxxxxxxxxxxxx> wrote:
> Hi Rafael,
>
> On 5/11/2017 10:52 AM, Rafael J. Wysocki wrote:
>>> OK. I'll reach out to Harb and let's see where the proposal goes.
>> It looks like this is about operation regions after all, however, so _DEP as is
>> should be sufficient here.
>>
>> There is some limited _DEP support in the ACPI layer, but we were missing
>> a way to represent those dependencies in the driver core.
>>
>> That can be done through device_link objects now, so we may be able to support
>> _DEP in a more meaningful way, but the cases when _DEP is used for something
>> different from operation regions in practice need to be treated with caution.
>>
>>
>
> _DEP could certainly help here. However, _DEP doesn't answer the loose binding question.
> If one driver is missing in one operating system, _GED driver will never load due
> to unsatisfied dependency. This forces us into all or none condition. We have operating
> systems that we need to support and do not have vendor I2C or GPIO drivers. That's why,
> we are hesitant to place _DEP into ACPI tables.

_DEP as defined in the spec should be fine still.

Quoting directly:

"_DEP evaluates to a package and designates device objects that OSPM
should assign a higher
priority in start ordering due to future operation region accesses.

To increase the likelihood that an SPB operation region handler is
available when needed, OSPM
needs to know in advance which methods will access it -- _DEP provides
OSPM with this
information. While the _DEP keyword may be used to determine start
ordering, only the _REG
method (Section 6.5.4) callbacks can be relied upon to determine
whether a region is accessible at a
given point in time."

So according to the above, _DEP is advisory only and you need _REG.

In theory, because I'm not sure if _REG actually works.

> The problem is that there is no concept of per event dependency. This could have helped
> us figure out if such an interrupt should be enabled or not.
>
> Another solution for operating regions is _REG if FW wants to ignore the event during
> boot. This is the one we are looking into at this moment for non-critical events.
>
> late_init proposed in this patch helps for built-in drivers such as GHES where we do
> not want to ignore events. Since GHES is not an actual device, this has to be solved
> in ACPI.

For built-in things it would be better to have explicit initialization
ordering, eg. initializing both GHES and GED from one initcall in the
right order.

Using different initcall levels for them is sort of OK, but then it is
not quite clear from the code what the specific ordering requirement
is, so please add a comment describing that at least.

Thanks,
Rafael