Re: [PATCH v2] platform/chrome: Add Wilco EC Event Handling

[Nick Crews]

Bump so this doesn't get lost :)

On Mon, Mar 4, 2019 at 4:54 PM Nick Crews <ncrews@xxxxxxxxxxxx> wrote:
>
> Hi Enric,
>
> On Tue, Feb 26, 2019 at 8:04 AM Enric Balletbo Serra
> <eballetbo@xxxxxxxxx> wrote:
> >
> > Hi Nick,
> >
> > Missatge de Nick Crews <ncrews@xxxxxxxxxxxx> del dia dl., 25 de febr.
> > 2019 a les 20:13:
> > >
> > > This patch is meant to be applied on top of the current
> > > for-next top of tree in the chrome/platform repo, at
> > > https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
> > >
> > > The Wilco Embedded Controller can return extended events that
> > > are not handled by standard ACPI objects. These events can
> > > contain information about changes in EC controlled features,
> > > such as the charger or battery getting plugged/unplugged.
> > >
> >
> > To understand better this I think would be good to know what kind of
> > events or which specific events do you expect to handle through this
> > interface. Because if these events are like the ones you described
> > above (charger/battery plugged/unplugged) this should go again through
> > the power-supply subsystem and use the standard interface for that.
>
> I didn't fully understand what was going on here either, so I had to figure
> it out myself as well :)
>
> I had a bad description for what sort of events this driver will be used for.
> There are two general classes of events that are generated by the EC and
> sent to the OS over ACPI: Standard ones (such as charger/battery/lid stuff
> as you mention) and the special ones that we care about. These special events
> include errors and notifications from the optional charging/display
> dock accessory,
> for instance "The cable you connected with is not supported". The
> standard ones are
> indeed handled through the standard interfaces. This interface also
> gets notified
> for a few of the standard events such as the battery getting plugged
> and unplugged,
> and that's why I was getting confused.
>
> You can see in some of the coreboot code where ACPI determines whether
> or not to forward these special "QS events" to this driver or to the
> standard ACPI system:
> https://review.coreboot.org/cgit/coreboot.git/tree/src/ec/google/wilco/acpi/event.asl#n80
>
> >
> > I suspect that what you want here, is different kernel drivers
> > accessing this event interface. E.g a wilco_ec_charger hooked to this
> > event interface that is able to get the event when the battery is
> > charging or not charging or charged. Maybe something similar to MKBP
> > events in cros-ec? Is that what you want?
>
> This is not what we want, I should have explained the use case in my
> original message.
> The point of this driver is to allow telemetry and diagnostics management for
> enterprise uses. A telemetry daemon will be running on the device
> waiting for events,
> (such as "The video port on the dock is not working.") and will report
> them to a cloud service.
> An example daemon that will use this is here:
> https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1390115
>
> With these explanations, do you now think that we should keep this structure,
> and not move to the power_supply subsystem?
>
> Thanks for your thoughts,
> Nick