| Message ID | 20211202191630.12450-1-jaschultz@microsoft.com |
|---|---|
| Headers | show |
| Series | platform: surface: Introduce Surface XBL Driver | expand |
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Friday, December 3, 2021 1:59 AM > To: Jarrett Schultz <jaschultzms@gmail.com>; Rob Herring > <robh+dt@kernel.org>; Andy Gross <agross@kernel.org>; Bjorn Andersson > <bjorn.andersson@linaro.org>; Mark Gross <markgross@kernel.org>; > Maximilian Luz <luzmaximilian@gmail.com> > Cc: linux-arm-msm@vger.kernel.org; platform-driver-x86@vger.kernel.org; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Felipe Balbi > <balbi@kernel.org>; Jarrett Schultz <jaschultz@microsoft.com> > Subject: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI > Dependency > > Hi Jarett, > > On 12/2/21 20:16, Jarrett Schultz wrote: > > Since the Surface XBL Driver does not depend on ACPI, the > > platform/surface directory as a whole no longer depends on ACPI. With > > respect to this, the ACPI dependency is moved into each config that > > depends on ACPI individually. > > > > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > > I think I will already merge this patch into the pdx86 tree: > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k > ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fpdx86%2Fplatform- > drivers- > x86.git%2F&data=04%7C01%7Cjaschultz%40microsoft.com%7C0ab6fcc6 > 4a5c4fd8657308d9b64391dd%7C72f988bf86f141af91ab2d7cd011db47%7C0 > %7C0%7C637741223627024908%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000& > amp;sdata=fEszuAgBLL3g2Z9Lh3DPQ%2BlzrWZR3o6aUst6fDmLOrE%3D& > ;reserved=0 > > While we are waiting for the rest of the series to get hashed out. > > But as already pointed out by Trilok Soni your From: and Signed-off-by email > addresses don't match. > > I can fix up the From to match the Signed-off-by while I apply this, but before > I do that I wanted to check with you that setting both to "Jarrett Schultz > <jaschultz@microsoft.com>" is the right thing to do ? > > Regards, > > Hans > > Hans, Yes, that is the correct email. Still trying to get all the kinks worked out, I appreciate your patience. Thank you, Jarrett > > > > > > > > --- > > > > Changes in v3: > > - Further propagated ACPI dependecy to SURFACE_AGGREGATOR > > > > --- > > > > Changes in v2: > > - Created to propagate ACPI dependency > > --- > > drivers/platform/surface/Kconfig | 7 ++++++- > > drivers/platform/surface/aggregator/Kconfig | 1 + > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/platform/surface/Kconfig > > b/drivers/platform/surface/Kconfig > > index 3105f651614f..5f0578e25f71 100644 > > --- a/drivers/platform/surface/Kconfig > > +++ b/drivers/platform/surface/Kconfig > > @@ -5,7 +5,6 @@ > > > > menuconfig SURFACE_PLATFORMS > > bool "Microsoft Surface Platform-Specific Device Drivers" > > - depends on ACPI > > default y > > help > > Say Y here to get to see options for platform-specific device > > drivers @@ -30,12 +29,14 @@ config SURFACE3_WMI > > > > config SURFACE_3_BUTTON > > tristate "Power/home/volume buttons driver for Microsoft Surface 3 > tablet" > > + depends on ACPI > > depends on KEYBOARD_GPIO && I2C > > help > > This driver handles the power/home/volume buttons on the > Microsoft Surface 3 tablet. > > > > config SURFACE_3_POWER_OPREGION > > tristate "Surface 3 battery platform operation region support" > > + depends on ACPI > > depends on I2C > > help > > This driver provides support for ACPI operation @@ -126,6 +127,7 > > @@ config SURFACE_DTX > > > > config SURFACE_GPE > > tristate "Surface GPE/Lid Support Driver" > > + depends on ACPI > > depends on DMI > > help > > This driver marks the GPEs related to the ACPI lid device found on > > @@ -135,6 +137,7 @@ config SURFACE_GPE > > > > config SURFACE_HOTPLUG > > tristate "Surface Hot-Plug Driver" > > + depends on ACPI > > depends on GPIOLIB > > help > > Driver for out-of-band hot-plug event signaling on Microsoft > > Surface @@ -154,6 +157,7 @@ config SURFACE_HOTPLUG > > > > config SURFACE_PLATFORM_PROFILE > > tristate "Surface Platform Profile Driver" > > + depends on ACPI > > depends on SURFACE_AGGREGATOR_REGISTRY > > select ACPI_PLATFORM_PROFILE > > help > > @@ -176,6 +180,7 @@ config SURFACE_PLATFORM_PROFILE > > > > config SURFACE_PRO3_BUTTON > > tristate "Power/home/volume buttons driver for Microsoft Surface > Pro 3/4 tablet" > > + depends on ACPI > > depends on INPUT > > help > > This driver handles the power/home/volume buttons on the > Microsoft Surface Pro 3/4 tablet. > > diff --git a/drivers/platform/surface/aggregator/Kconfig > > b/drivers/platform/surface/aggregator/Kconfig > > index fd6dc452f3e8..cab020324256 100644 > > --- a/drivers/platform/surface/aggregator/Kconfig > > +++ b/drivers/platform/surface/aggregator/Kconfig > > @@ -4,6 +4,7 @@ > > menuconfig SURFACE_AGGREGATOR > > tristate "Microsoft Surface System Aggregator Module Subsystem > and Drivers" > > depends on SERIAL_DEV_BUS > > + depends on ACPI > > select CRC_CCITT > > help > > The Surface System Aggregator Module (Surface SAM or SSAM) is an > >
Hi, On 12/3/21 18:34, Jarrett Schultz wrote: > > >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: Friday, December 3, 2021 1:59 AM >> To: Jarrett Schultz <jaschultzms@gmail.com>; Rob Herring >> <robh+dt@kernel.org>; Andy Gross <agross@kernel.org>; Bjorn Andersson >> <bjorn.andersson@linaro.org>; Mark Gross <markgross@kernel.org>; >> Maximilian Luz <luzmaximilian@gmail.com> >> Cc: linux-arm-msm@vger.kernel.org; platform-driver-x86@vger.kernel.org; >> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Felipe Balbi >> <balbi@kernel.org>; Jarrett Schultz <jaschultz@microsoft.com> >> Subject: [EXTERNAL] Re: [PATCH 2/5] platform: surface: Propagate ACPI >> Dependency >> >> Hi Jarett, >> >> On 12/2/21 20:16, Jarrett Schultz wrote: >>> Since the Surface XBL Driver does not depend on ACPI, the >>> platform/surface directory as a whole no longer depends on ACPI. With >>> respect to this, the ACPI dependency is moved into each config that >>> depends on ACPI individually. >>> >>> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> >> >> I think I will already merge this patch into the pdx86 tree: >> >> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k >> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fpdx86%2Fplatform- >> drivers- >> x86.git%2F&data=04%7C01%7Cjaschultz%40microsoft.com%7C0ab6fcc6 >> 4a5c4fd8657308d9b64391dd%7C72f988bf86f141af91ab2d7cd011db47%7C0 >> %7C0%7C637741223627024908%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC >> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000& >> amp;sdata=fEszuAgBLL3g2Z9Lh3DPQ%2BlzrWZR3o6aUst6fDmLOrE%3D& >> ;reserved=0 >> >> While we are waiting for the rest of the series to get hashed out. >> >> But as already pointed out by Trilok Soni your From: and Signed-off-by email >> addresses don't match. >> >> I can fix up the From to match the Signed-off-by while I apply this, but before >> I do that I wanted to check with you that setting both to "Jarrett Schultz >> <jaschultz@microsoft.com>" is the right thing to do ? >> >> Regards, >> >> Hans >> >> > > Hans, > > Yes, that is the correct email. Still trying to get all the kinks worked out, I appreciate your patience. Ok, I've merged this patch now, so you can drop it from the next version of the series. Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans >>> --- >>> >>> Changes in v3: >>> - Further propagated ACPI dependecy to SURFACE_AGGREGATOR >>> >>> --- >>> >>> Changes in v2: >>> - Created to propagate ACPI dependency >>> --- >>> drivers/platform/surface/Kconfig | 7 ++++++- >>> drivers/platform/surface/aggregator/Kconfig | 1 + >>> 2 files changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/platform/surface/Kconfig >>> b/drivers/platform/surface/Kconfig >>> index 3105f651614f..5f0578e25f71 100644 >>> --- a/drivers/platform/surface/Kconfig >>> +++ b/drivers/platform/surface/Kconfig >>> @@ -5,7 +5,6 @@ >>> >>> menuconfig SURFACE_PLATFORMS >>> bool "Microsoft Surface Platform-Specific Device Drivers" >>> - depends on ACPI >>> default y >>> help >>> Say Y here to get to see options for platform-specific device >>> drivers @@ -30,12 +29,14 @@ config SURFACE3_WMI >>> >>> config SURFACE_3_BUTTON >>> tristate "Power/home/volume buttons driver for Microsoft Surface 3 >> tablet" >>> + depends on ACPI >>> depends on KEYBOARD_GPIO && I2C >>> help >>> This driver handles the power/home/volume buttons on the >> Microsoft Surface 3 tablet. >>> >>> config SURFACE_3_POWER_OPREGION >>> tristate "Surface 3 battery platform operation region support" >>> + depends on ACPI >>> depends on I2C >>> help >>> This driver provides support for ACPI operation @@ -126,6 +127,7 >>> @@ config SURFACE_DTX >>> >>> config SURFACE_GPE >>> tristate "Surface GPE/Lid Support Driver" >>> + depends on ACPI >>> depends on DMI >>> help >>> This driver marks the GPEs related to the ACPI lid device found on >>> @@ -135,6 +137,7 @@ config SURFACE_GPE >>> >>> config SURFACE_HOTPLUG >>> tristate "Surface Hot-Plug Driver" >>> + depends on ACPI >>> depends on GPIOLIB >>> help >>> Driver for out-of-band hot-plug event signaling on Microsoft >>> Surface @@ -154,6 +157,7 @@ config SURFACE_HOTPLUG >>> >>> config SURFACE_PLATFORM_PROFILE >>> tristate "Surface Platform Profile Driver" >>> + depends on ACPI >>> depends on SURFACE_AGGREGATOR_REGISTRY >>> select ACPI_PLATFORM_PROFILE >>> help >>> @@ -176,6 +180,7 @@ config SURFACE_PLATFORM_PROFILE >>> >>> config SURFACE_PRO3_BUTTON >>> tristate "Power/home/volume buttons driver for Microsoft Surface >> Pro 3/4 tablet" >>> + depends on ACPI >>> depends on INPUT >>> help >>> This driver handles the power/home/volume buttons on the >> Microsoft Surface Pro 3/4 tablet. >>> diff --git a/drivers/platform/surface/aggregator/Kconfig >>> b/drivers/platform/surface/aggregator/Kconfig >>> index fd6dc452f3e8..cab020324256 100644 >>> --- a/drivers/platform/surface/aggregator/Kconfig >>> +++ b/drivers/platform/surface/aggregator/Kconfig >>> @@ -4,6 +4,7 @@ >>> menuconfig SURFACE_AGGREGATOR >>> tristate "Microsoft Surface System Aggregator Module Subsystem >> and Drivers" >>> depends on SERIAL_DEV_BUS >>> + depends on ACPI >>> select CRC_CCITT >>> help >>> The Surface System Aggregator Module (Surface SAM or SSAM) is an >>> >
Hi, Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Jarrett, > > On Mon, Dec 6, 2021 at 4:03 PM Jarrett Schultz <jaschultzms@gmail.com> wrote: >> Since the Surface XBL Driver does not depend on ACPI, the >> platform/surface directory as a whole no longer depends on ACPI. With >> respect to this, the ACPI dependency is moved into each config that depends >> on ACPI individually. >> >> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > > Thanks for your patch, which is now commit 272479928172edf0 ("platform: > surface: Propagate ACPI Dependency"). > >> --- a/drivers/platform/surface/Kconfig >> +++ b/drivers/platform/surface/Kconfig >> @@ -5,7 +5,6 @@ >> >> menuconfig SURFACE_PLATFORMS >> bool "Microsoft Surface Platform-Specific Device Drivers" >> - depends on ACPI >> default y >> help >> Say Y here to get to see options for platform-specific device drivers > > Without any dependency, all users configuring a kernel are now asked > about this. Is there any other platform dependency that can be used > instead? there's probably no symbol that would be true for x86 and arm64 while being false for everything else. Any ideas? In any case, what's the problem of being asked about a new symbol? That happens all the time whenever new drivers are merged, right?
Hi Felipe, On Fri, Jan 14, 2022 at 7:21 AM Felipe Balbi <balbi@kernel.org> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > > On Mon, Dec 6, 2021 at 4:03 PM Jarrett Schultz <jaschultzms@gmail.com> wrote: > >> Since the Surface XBL Driver does not depend on ACPI, the > >> platform/surface directory as a whole no longer depends on ACPI. With > >> respect to this, the ACPI dependency is moved into each config that depends > >> on ACPI individually. > >> > >> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > > > > Thanks for your patch, which is now commit 272479928172edf0 ("platform: > > surface: Propagate ACPI Dependency"). > > > >> --- a/drivers/platform/surface/Kconfig > >> +++ b/drivers/platform/surface/Kconfig > >> @@ -5,7 +5,6 @@ > >> > >> menuconfig SURFACE_PLATFORMS > >> bool "Microsoft Surface Platform-Specific Device Drivers" > >> - depends on ACPI > >> default y > >> help > >> Say Y here to get to see options for platform-specific device drivers > > > > Without any dependency, all users configuring a kernel are now asked > > about this. Is there any other platform dependency that can be used > > instead? > > there's probably no symbol that would be true for x86 and arm64 while > being false for everything else. Any ideas? depends on ARM64 || X86 || COMPILE_TEST? > In any case, what's the problem of being asked about a new symbol? That > happens all the time whenever new drivers are merged, right? In se there is no problem with being asked about a new symbol. The problem is the sheer number of symbols, and irrelevant questions (e.g. this one, on !x86 and !arm64). Time spent on adding proper dependencies is IMHO well-spent, as it will save time for everyone who configures his own kernel. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi, On 1/14/22 09:29, Geert Uytterhoeven wrote: > Hi Felipe, > > On Fri, Jan 14, 2022 at 7:21 AM Felipe Balbi <balbi@kernel.org> wrote: >> Geert Uytterhoeven <geert@linux-m68k.org> writes: >>> On Mon, Dec 6, 2021 at 4:03 PM Jarrett Schultz <jaschultzms@gmail.com> wrote: >>>> Since the Surface XBL Driver does not depend on ACPI, the >>>> platform/surface directory as a whole no longer depends on ACPI. With >>>> respect to this, the ACPI dependency is moved into each config that depends >>>> on ACPI individually. >>>> >>>> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> >>> >>> Thanks for your patch, which is now commit 272479928172edf0 ("platform: >>> surface: Propagate ACPI Dependency"). >>> >>>> --- a/drivers/platform/surface/Kconfig >>>> +++ b/drivers/platform/surface/Kconfig >>>> @@ -5,7 +5,6 @@ >>>> >>>> menuconfig SURFACE_PLATFORMS >>>> bool "Microsoft Surface Platform-Specific Device Drivers" >>>> - depends on ACPI >>>> default y >>>> help >>>> Say Y here to get to see options for platform-specific device drivers >>> >>> Without any dependency, all users configuring a kernel are now asked >>> about this. Is there any other platform dependency that can be used >>> instead? >> >> there's probably no symbol that would be true for x86 and arm64 while >> being false for everything else. Any ideas? > > depends on ARM64 || X86 || COMPILE_TEST? That sounds reasonable to me, I would be happy to take a patch for that. Regards, Hans
Hi, Hans de Goede <hdegoede@redhat.com> writes: > Hi, > > On 1/14/22 09:29, Geert Uytterhoeven wrote: >> Hi Felipe, >> >> On Fri, Jan 14, 2022 at 7:21 AM Felipe Balbi <balbi@kernel.org> wrote: >>> Geert Uytterhoeven <geert@linux-m68k.org> writes: >>>> On Mon, Dec 6, 2021 at 4:03 PM Jarrett Schultz <jaschultzms@gmail.com> wrote: >>>>> Since the Surface XBL Driver does not depend on ACPI, the >>>>> platform/surface directory as a whole no longer depends on ACPI. With >>>>> respect to this, the ACPI dependency is moved into each config that depends >>>>> on ACPI individually. >>>>> >>>>> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> >>>> >>>> Thanks for your patch, which is now commit 272479928172edf0 ("platform: >>>> surface: Propagate ACPI Dependency"). >>>> >>>>> --- a/drivers/platform/surface/Kconfig >>>>> +++ b/drivers/platform/surface/Kconfig >>>>> @@ -5,7 +5,6 @@ >>>>> >>>>> menuconfig SURFACE_PLATFORMS >>>>> bool "Microsoft Surface Platform-Specific Device Drivers" >>>>> - depends on ACPI >>>>> default y >>>>> help >>>>> Say Y here to get to see options for platform-specific device drivers >>>> >>>> Without any dependency, all users configuring a kernel are now asked >>>> about this. Is there any other platform dependency that can be used >>>> instead? >>> >>> there's probably no symbol that would be true for x86 and arm64 while >>> being false for everything else. Any ideas? >> >> depends on ARM64 || X86 || COMPILE_TEST? > > That sounds reasonable to me, I would be happy to take a patch for that. fair enough, let's see what Jarrett replies
Hi, > Felipe Balbi <balbi@kernel.org> writes: > > Hi, > > Hans de Goede <hdegoede@redhat.com> writes: > > Hi, > > > > On 1/14/22 09:29, Geert Uytterhoeven wrote: > >> Hi Felipe, > >> > >> On Fri, Jan 14, 2022 at 7:21 AM Felipe Balbi <balbi@kernel.org> wrote: > >>> Geert Uytterhoeven <geert@linux-m68k.org> writes: > >>>> On Mon, Dec 6, 2021 at 4:03 PM Jarrett Schultz > <jaschultzms@gmail.com> wrote: > >>>>> Since the Surface XBL Driver does not depend on ACPI, the > >>>>> platform/surface directory as a whole no longer depends on ACPI. > >>>>> With respect to this, the ACPI dependency is moved into each > >>>>> config that depends on ACPI individually. > >>>>> > >>>>> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> > >>>> > >>>> Thanks for your patch, which is now commit 272479928172edf0 > ("platform: > >>>> surface: Propagate ACPI Dependency"). > >>>> > >>>>> --- a/drivers/platform/surface/Kconfig > >>>>> +++ b/drivers/platform/surface/Kconfig > >>>>> @@ -5,7 +5,6 @@ > >>>>> > >>>>> menuconfig SURFACE_PLATFORMS > >>>>> bool "Microsoft Surface Platform-Specific Device Drivers" > >>>>> - depends on ACPI > >>>>> default y > >>>>> help > >>>>> Say Y here to get to see options for platform-specific > >>>>> device drivers > >>>> > >>>> Without any dependency, all users configuring a kernel are now > >>>> asked about this. Is there any other platform dependency that can > >>>> be used instead? > >>> > >>> there's probably no symbol that would be true for x86 and arm64 > >>> while being false for everything else. Any ideas? > >> > >> depends on ARM64 || X86 || COMPILE_TEST? > > > > That sounds reasonable to me, I would be happy to take a patch for that. > > fair enough, let's see what Jarrett replies > > -- > Balbi Sounds good to me, I'll include this in the next patch version. Thank you for pointing this out. -Jarrett
Introduce the Surface Extensible Boot Loader driver for the Surface Duo. Exposes information about the driver to user space via sysfs. Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com> --- Changes in v3: - For the yaml documentation: * Updated description * Fixed examples * Updated 'required' field - Further propogated ACPI dependency in Kconfigs - Updated sysfs several binding descriptions - Renamed files to conform to naming conventions --- Changes in v2: - Per Maximilian, added patch 2: propagated ACPI dependency from the directory as a whole to each individual driver - For the yaml documentation: * Removed json-schema dependence * Elaborated on description of driver * Updated example - Changed target KernelVersion in sysfs documentation - Updated MAINTAINER changes to be properly applied across patches - For the driver itself, * Added types.h inclusion and removed unused inclusions * Minor updates to code and acronym style * Remove __packed attribute on driver struct * Use .dev_groups for sysfs - Added more in-depth description of driver in Kconfig - Modified dts to reference a newly added section in sm8150.dtsi --- Jarrett Schultz (5): dt-bindings: platform: microsoft: Document surface xbl platform: surface: Propagate ACPI Dependency platform: surface: Add surface xbl arm64: dts: qcom: sm8150: Add imem section arm64: dts: qcom: surface-duo: Add surface xbl .../ABI/testing/sysfs-platform-surface-xbl | 79 ++++++++ .../platform/microsoft/surface-xbl.yaml | 69 +++++++ MAINTAINERS | 9 + .../dts/qcom/sm8150-microsoft-surface-duo.dts | 10 + arch/arm64/boot/dts/qcom/sm8150.dtsi | 8 + drivers/platform/surface/Kconfig | 19 +- drivers/platform/surface/Makefile | 1 + drivers/platform/surface/aggregator/Kconfig | 1 + drivers/platform/surface/surface_xbl.c | 186 ++++++++++++++++++ 9 files changed, 381 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/sysfs-platform-surface-xbl create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml create mode 100644 drivers/platform/surface/surface_xbl.c