Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <6e6e478b-eb12-5d7b-e944-4e6c02e46d4c@gmail.com>
Date:   Mon, 8 Nov 2021 18:58:34 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH v2 2/5] platform: surface: Propagate ACPI Dependency
Content-Language: en-US
To:     Jarrett Schultz <jaschultzms@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Hans de Goede <hdegoede@redhat.com>,
        Mark Gross <mgross@linux.intel.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>
References: <20211108164449.3036210-1-jaschultz@microsoft.com>
 <20211108164449.3036210-3-jaschultz@microsoft.com>
From:   Maximilian Luz <luzmaximilian@gmail.com>
In-Reply-To: <20211108164449.3036210-3-jaschultz@microsoft.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/8/21 17:44, Jarrett Schultz wrote:> From: Jarrett Schultz <jaschultzMS@gmail.com>
> 
> 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>

Some remarks inline:

> ---
> 
> Changes in v2:
>   - Created to propagate ACPI dependency
> 
> ---
> 
>   drivers/platform/surface/Kconfig | 12 +++++++++++-

You also need to account for included Kconfigs, specifically:

     drivers/platform/surface/aggregator/Kconfig.

>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/surface/Kconfig b/drivers/platform/surface/Kconfig
> index 3105f651614f..0d3970e1d144 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
> @@ -18,6 +17,7 @@ if SURFACE_PLATFORMS
>   
>   config SURFACE3_WMI
>   	tristate "Surface 3 WMI Driver"
> +	depends on ACPI

This is redundant, you can drop that. ACPI_WMI already depends on ACPI.

>   	depends on ACPI_WMI
>   	depends on DMI
>   	depends on INPUT
> @@ -30,12 +30,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
> @@ -43,6 +45,7 @@ config SURFACE_3_POWER_OPREGION
>   
>   config SURFACE_ACPI_NOTIFY
>   	tristate "Surface ACPI Notify Driver"
> +	depends on ACPI

As mentioned above, you're missing aggregator/Kconfig. All you need to
do is add "depends on ACPI" to SURFACE_AGGREGATOR in that file. Then you
can drop the "depends on ACPI" for anything that depends on that.

Same holds for the couple of options depending on SURFACE_AGGREGATOR
below.

>   	depends on SURFACE_AGGREGATOR
>   	help
>   	  Surface ACPI Notify (SAN) driver for Microsoft Surface devices.
> @@ -62,6 +65,7 @@ config SURFACE_ACPI_NOTIFY
>   
>   config SURFACE_AGGREGATOR_CDEV
>   	tristate "Surface System Aggregator Module User-Space Interface"
> +	depends on ACPI
>   	depends on SURFACE_AGGREGATOR
>   	help
>   	  Provides a misc-device interface to the Surface System Aggregator
> @@ -79,6 +83,7 @@ config SURFACE_AGGREGATOR_CDEV
>   
>   config SURFACE_AGGREGATOR_REGISTRY
>   	tristate "Surface System Aggregator Module Device Registry"
> +	depends on ACPI
>   	depends on SURFACE_AGGREGATOR
>   	depends on SURFACE_AGGREGATOR_BUS
>   	help
> @@ -106,6 +111,7 @@ config SURFACE_AGGREGATOR_REGISTRY
>   
>   config SURFACE_DTX
>   	tristate "Surface DTX (Detachment System) Driver"
> +	depends on ACPI
>   	depends on SURFACE_AGGREGATOR
>   	depends on INPUT
>   	help
> @@ -126,6 +132,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 +142,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 +162,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 +185,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.
>