Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190225190957.40425-1-ncrews@chromium.org> <CAFqH_52NG0Zmc=dtZa=6o7kJc0fdueLqTaQiZdtM=2tBer+6vA@mail.gmail.com>
 <CAHX4x84gmrvnGMSE9Geb=8s+Htao+Z=Zp=rR0GpCV+FP5Ax+QQ@mail.gmail.com>
In-Reply-To: <CAHX4x84gmrvnGMSE9Geb=8s+Htao+Z=Zp=rR0GpCV+FP5Ax+QQ@mail.gmail.com>
From:   Nick Crews <ncrews@chromium.org>
Date:   Mon, 11 Mar 2019 12:08:57 -0600
Message-ID: <CAHX4x84UE_TD4kSny3JABTzORWrirSNsFb7GkdF=Rqcj9uHEkw@mail.gmail.com>
Subject: Re: [PATCH v2] platform/chrome: Add Wilco EC Event Handling
To:     Enric Balletbo Serra <eballetbo@gmail.com>
Cc:     Benson Leung <bleung@chromium.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Duncan Laurie <dlaurie@chromium.org>,
        Oleh Lamzin <lamzin@google.com>,
        Bartosz Fabianowski <bartfab@google.com>,
        Daniel Erat <derat@google.com>,
        Guenter Roeck <groeck@google.com>,
        Dmitry Torokhov <dtor@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Bump so this doesn't get lost :)

On Mon, Mar 4, 2019 at 4:54 PM Nick Crews <ncrews@chromium.org> wrote:
>
> Hi Enric,
>
> On Tue, Feb 26, 2019 at 8:04 AM Enric Balletbo Serra
> <eballetbo@gmail.com> wrote:
> >
> > Hi Nick,
> >
> > Missatge de Nick Crews <ncrews@chromium.org> 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