lkml.org 
[lkml]   [2020]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 0/9] Add support for Microsoft Surface System Aggregator Module
    From
    Date
    On 9/23/20 9:43 PM, Arnd Bergmann wrote:
    > On Wed, Sep 23, 2020 at 5:43 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
    >>
    >> On 9/23/20 5:30 PM, Arnd Bergmann wrote:
    >>> On Wed, Sep 23, 2020 at 5:15 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
    >>>>
    >>>> Hello,
    >>>>
    >>>> The Surface System Aggregator Module (we'll refer to it as Surface
    >>>> Aggregator or SAM below) is an embedded controller (EC) found on various
    >>>> Microsoft Surface devices. Specifically, all 4th and later generation
    >>>> Surface devices, i.e. Surface Pro 4, Surface Book 1 and later, with the
    >>>> exception of the Surface Go series and the Surface Duo. Notably, it
    >>>> seems like this EC can also be found on the ARM-based Surface Pro X [1].
    >>>
    >>> I think this should go to drivers/platform/x86 or drivers/platform/surface/
    >>> along with other laptop vendor specific code rather than drivers/misc/.
    >>
    >> I initially had this under drivers/platform/x86. There are two main
    >> reasons I changed that: First, I think it's a bit too big for
    >> platform/x86 given that it basically introduces a new subsystem. At this
    >> point it's really less of "a couple of odd devices here and there" and
    >> more of a bus-type thing. Second, with the possibility of future support
    >> for ARM devices (Pro X, Pro X 2 which is rumored to come out soon), I
    >> thought that platform/x86 would not be a good fit.
    >
    > I don't see that as a strong reason against it. As you write yourself, the
    > driver won't work on the arm machines without major changes anyway,
    > and even if it does, it fits much better with the rest of it.

    Sorry, I should have written that a bit more clearly. I don't see any
    reason why these drivers would not work on an ARM device such as the Pro
    X right now, assuming that it boots via ACPI and the serial device it
    loads against is fully functional.

    The reason (at least as far as I know) it currently hasn't been tested
    is that a) there aren't a lot of people around attempting to run Linux
    on the currently only ARM device with that and b) it's currently blocked
    by a reason unrelated to this driver itself, specifically that the
    serial controller isn't being set up and thus the core driver doesn't
    have a device it can attach to. My information may be outdated though
    and is pretty much exclusively based on
    https://github.com/Sonicadvance1/linux/issues/7.

    > If you are worried about the size of the directory,
    > drivers/platform/x86/surface/
    > would also work.

    This was the alternative I'd have considered without ARM devices.

    >> I'd be happy to move this to platform/surface though, if that's
    >> considered a better fit and you're okay with me adding that. Would make
    >> sense given that there's already a platform/chrome, which, as far as I
    >> can tell, also seems to be mainly focused on EC support.
    >
    > Yes, I think the main question is how much overlap you see functionally
    > between this driver and the others in drivers/platform/x86.

    I think that the Pro X likely won't be the last ARM Surface device with
    a SAM EC. Further, the subsystem is going to grow, and platform/x86
    seems more like a collection of, if at all, loosely connected drivers,
    which might give off the wrong impression. In my mind, this is just a
    bit more comparable to platform/chrome than the rest of platform/x86. I
    don't think I'm really qualified to make the decision on that though,
    that's just my opinion.

    Here's an overview of other drivers that I hopefully at some point get
    in good enough shape, which are part of this subsystem/dependent on the
    EC API introduced here:

    - A device registry / device hub for devices that are connected to the
    EC but can't be detected via ACPI.

    - A dedicated battery driver for 7th generation devices (where the
    battery isn't hanled via the ACPI shim).

    - A driver properly handling clipboard detachment on the Surface Books.

    - A driver for HID input/transport on the Surface Laptops and Surface
    Book 3.

    - A driver for allowing users to set the performance/cooling mode via
    sysfs.

    - Possibly a driver improving hot-plug handling of the discrete GPU in
    the Surface Book base.

    And also some stuff that hasn't been written yet:

    - A dedicated driver for temperature sensors handled via the EC on 7th
    generation devices (also handled via the ACPI shim on previous
    generations).

    - Possibly a driver for real-time-clock access on 7th generation
    devices (it has yet to be tested if that interface is still
    around/required on those devices; that's also a thing handled via
    the ACPI shim on previous generations).

    I doubt that those client drivers will be exclusive to x86, and I could
    see (current and future) ARM devices using SAM based battery,
    keyboard/HID, and performance mode drivers (which will likely also
    require the device registry, because for some reason MS doesn't want to
    describe those devices in ACPI on the newer generations any more...).
    All of those should work as-is on ARM (or at least after the
    corresponding device entries have been added to the device registry),
    modulo bugs of course.

    I hope this all gives a better overview of the form this may eventually
    take on and helps you in your decision. I'd be completely happy to move
    it to either, platform/surface or platform/x86/surface, whatever the
    consensus is. I'd very much like to keep the client drivers all
    contained to one sub-directory, though, and not scattered all over
    platform/x86/surface_*.c. Again that's more of a personal preference
    though :)

    Thanks,
    Max

    \
     
     \ /
      Last update: 2020-09-24 01:29    [W:4.428 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site