From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Sinan Kaya <okaya@codeaurora.org>
Cc: Lukas Wunner <lukas@wunner.de>,
ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
Timur Tabi <timur@codeaurora.org>, Len Brown <lenb@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ACPI / GED: use late init to allow other drivers init
Date: Thu, 11 May 2017 02:46:06 +0200 [thread overview]
Message-ID: <1874042.5bWzSJ73x0@aspire.rjw.lan> (raw)
In-Reply-To: <3f2c5bcc-3499-d5ef-6955-778979d25b08@codeaurora.org>
Sorry for the delay.
On Tuesday, April 25, 2017 12:24:19 PM Sinan Kaya wrote:
> On 4/25/2017 3:01 AM, Lukas Wunner wrote:
> > On Sat, Apr 22, 2017 at 12:48 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> >> On 4/21/2017 6:43 PM, Rafael J. Wysocki wrote:
> >>> +late_initcall(ged_init);
> >>> Does this fix the problem?
> >>>
> >>> What about if the module in question is loaded after running
> >>> late_initcalls?
> >>
> >> This fixed the issue for me where I had dependencies for QUP I2C driver
> >> and GHES drivers. Both of them are modules and get probed via normal
> >> module execution path.
> >>
> >> However, I'm open to improvements. Do you have a better suggestion?
> >> I can try to add some _DEP stuff if it is present, but I remember Linux
> >> doesn't like _DEP stuff too much.
> >
> > Would it be possible to solve this by just returning -EPROBE_DEFER from the
> > ->probe hook if the devices you depend on are not bound yet?
> >
>
> I'm not sure.
>
> > Alternatively, would it be possible to solve it with a struct device_link?
>
> I wasn't aware of device_link concept. This is something that I will keep in
> my mind when I'm dealing with producer/consumer problems with known device
> driver instances. It looked very useful.
>
> Here is how the overall relationship between drivers.
>
> | GED | <---> | Platform specific ACPI AML | <----> Vendor GPIO
> <----> Vendor I2C
> <----> ACPI GHES
> <----> Some other driver
>
> The problem with Generic Event Device (GED) is that it produces event
> notification facility to the platform specific AML code and GED doesn't
> have any idea about the consumers of these interrupts or what platform AML
> does.
>
> GED only sees the interrupts that it needs to register and
> knows the ASL code it needs to execute when that interrupt happens.
>
> It is possible for AML code not to use any of these drivers or require
> some arbitrary driver as well as vendor specific drivers. It is totally
> up to the firmware to decide what to do with this event.
>
> My proposal was to require platform AML code to indicate the dependencies
> between GED and drivers on the right side of the picture via _DEP as this
> cannot be done via normal kernel mechanisms.
Something like _DEP would be needed.
However, _DEP as specified is only about operation region dependencies, which
doesn't seem to be applicable here.
That said, _DEP is used for general dependecies by firmware already, but it
would at least be good to send a proposal for a spec update regarding that
before mandating using _DEP for GED.
> This approach might work in general. However, it also has its own caveats.
>
> All of these drivers on the right side are unrelated to each other. Some
> operating system can implement a subset of these drivers.
>
> If I include the dependencies, GED will never load for partial driver situations.
> This is also a deal breaker.
_DEP doesn't mean a hard dependency AFAICS. It is about ordering, not about
presence, at least as specified currently.
> Why would you break some other feature if your OS doesn't support RAS as an
> example?
>
> Given all these lose bindings and no driver association, where do we go
> from here?
>
> I consider GED as a light version of Embedded controller (EC) implementation.
No, it is not.
It is more of a generalization of the GPE/SCI mechanism in order to make it
possible to cover things different from GPIO (which already is covered by
_AEI).
> How is this problem solved for EC as it has the same problem?
It doesn't. The EC relies on the GPE/SCI mechanism to be there and that is
always present.
Thanks,
Rafael
next prev parent reply other threads:[~2017-05-11 0:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-21 22:28 [PATCH] ACPI / GED: use late init to allow other drivers init Sinan Kaya
2017-04-21 22:43 ` Rafael J. Wysocki
2017-04-21 22:48 ` Sinan Kaya
2017-04-21 22:48 ` Sinan Kaya
2017-04-24 23:01 ` Rafael J. Wysocki
2017-04-24 23:33 ` Sinan Kaya
2017-04-25 7:01 ` Lukas Wunner
2017-04-25 16:24 ` Sinan Kaya
2017-04-28 2:32 ` Sinan Kaya
2017-05-11 0:58 ` Rafael J. Wysocki
2017-05-11 1:38 ` Sinan Kaya
2017-05-11 0:46 ` Rafael J. Wysocki [this message]
2017-05-11 13:43 ` Sinan Kaya
2017-05-11 14:52 ` Rafael J. Wysocki
2017-05-15 2:36 ` Sinan Kaya
2017-05-15 10:59 ` Rafael J. Wysocki
2017-05-21 15:51 ` Sinan Kaya
2017-04-25 1:43 ` Sinan Kaya
2017-04-25 11:03 ` Rafael J. Wysocki
2017-04-25 16:25 ` Sinan Kaya
2017-04-24 22:49 ` Baicar, Tyler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1874042.5bWzSJ73x0@aspire.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=okaya@codeaurora.org \
--cc=timur@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).