Received-SPF: pass (google.com: domain of linux-kernel+bounces-169648-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <4bb43e89-c387-4219-9051-421d700f332e@redhat.com>
Date: Mon, 6 May 2024 12:18:05 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5] platform/x86: dell-laptop: Implement platform_profile
To: Lyndon Sanche <lsanche@lyndeno.ca>, kernel test robot <lkp@intel.com>
Cc: Paul Gazzillo <paul@pgazz.com>,
 Necip Fazil Yildiran <fazilyildiran@gmail.com>,
 oe-kbuild-all@lists.linux.dev, mario.limonciello@amd.com, pali@kernel.org,
 W_Armin@gmx.de, srinivas.pandruvada@linux.intel.com,
 ilpo.jarvinen@linux.intel.com, Matthew Garrett <mjg59@srcf.ucam.org>,
 Jonathan Corbet <corbet@lwn.net>, Heiner Kallweit <hkallweit1@gmail.com>,
 Vegard Nossum <vegard.nossum@oracle.com>,
 platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
 Dell.Client.Kernel@dell.com
References: <20240501215829.4991-2-lsanche@lyndeno.ca>
 <202405031851.NYy0ZB02-lkp@intel.com> <A9SXCS.IUN31UTTT9GM2@lyndeno.ca>
Content-Language: en-US
From: Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <A9SXCS.IUN31UTTT9GM2@lyndeno.ca>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hi Lyndon,

Thank you for your patch!

On 5/4/24 3:03 AM, Lyndon Sanche wrote:
> 
> 
> On Fri, May 3 2024 at 06:19:18 PM +08:00:00, kernel test robot <lkp@intel.com> wrote:
>> Hi Lyndon,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on linus/master]
>> [also build test WARNING on v6.9-rc6 next-20240503]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Lyndon-Sanche/platform-x86-dell-laptop-Implement-platform_profile/20240502-060146
>> base:   linus/master
>> patch link:    https://lore.kernel.org/r/20240501215829.4991-2-lsanche%40lyndeno.ca
>> patch subject: [PATCH v5] platform/x86: dell-laptop: Implement platform_profile
>> config: i386-kismet-CONFIG_ACPI_PLATFORM_PROFILE-CONFIG_DELL_LAPTOP-0-0 (https://download.01.org/0day-ci/archive/20240503/202405031851.NYy0ZB02-lkp@intel.com/config)
>> reproduce: (https://download.01.org/0day-ci/archive/20240503/202405031851.NYy0ZB02-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202405031851.NYy0ZB02-lkp@intel.com/
>>
>> kismet warnings: (new ones prefixed by >>)
>>>>  kismet: WARNING: unmet direct dependencies detected for ACPI_PLATFORM_PROFILE when selected by DELL_LAPTOP
>>    WARNING: unmet direct dependencies detected for ACPI_PLATFORM_PROFILE
>>      Depends on [n]: ACPI [=n]
>>      Selected by [y]:
>>      - DELL_LAPTOP [=y] && X86_PLATFORM_DEVICES [=y] && X86_PLATFORM_DRIVERS_DELL [=y] && DMI [=y] && BACKLIGHT_CLASS_DEVICE [=y] && (ACPI_VIDEO [=n] || ACPI_VIDEO [=n]=n) && (RFKILL [=n] || RFKILL [=n]=n) && (DELL_WMI [=n] || DELL_WMI [=n]=n) && SERIO_I8042 [=y] && DELL_SMBIOS [=y]
>>
>> -- 
>> 0-DAY CI Kernel Test Service
>> https://github.com/intel/lkp-tests/wiki
> 
> I will try reproducing this test on my machine, to avoid spamming the mailing list with the same error over and over.

No need to reproduce this. When you select something in Kconfig you must ensure
that the item doing the selecting depends on all the dependencies of what you
are selecting.

IOW if you add this change to your next version then that should fix this:

diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index bd9f445974cc..d18fbc6a5fbf 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -47,6 +47,7 @@ config DCDBAS
 config DELL_LAPTOP
 	tristate "Dell Laptop Extras"
 	default m
+	depends on ACPI
 	depends on DMI
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on ACPI_VIDEO || ACPI_VIDEO = n

And please also address Armin's remark about making sure that failure
to initialize platform_profile support should not cause the entire driver
to fail to probe.

I see that Armin suggests to check da_supported_commands for this,
this is a good idea but atm this is private to dell-smbios-base. So
you will first need to do a small preparation patch adding a small:

bool dell_laptop_check_supported_cmds(struct calling_interface_buffer *buffer)
{
	return da_supported_commands & (1 << buffer->cmd_class);
}
EXPORT_SYMBOL_GPL(dell_laptop_check_supported_cmds):

helper for this.

If this check fails (returns false) make the code not register
the platform_profile() while allowing probe() to continue / succeed,
please do not log anything in this case (or use dev_dbg())

If this check succeeds but subsequent dell_smbios_call()'s
fail during probe, then it is ok to log an error but please
still let probe() continue / succeed (without registering
a platform_profile handler).

Regards,

Hans