Subject: Re: [PATCH] platform/x86: x86-android-tablets: select PMIC_OPREGION
 to resolve a link error of intel_soc_pmic_exec_mipi_pmic_seq_element
To:     Hans de Goede <hdegoede@redhat.com>, markgross@kernel.org
Cc:     platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230310192915.2700069-1-trix@redhat.com>
 <e3c00116-5283-8c76-7414-66f36fb575fa@redhat.com>
From:   Tom Rix <trix@redhat.com>
Message-ID: <b8fa13a6-5fd7-13e6-812f-2e31b64b1eaf@redhat.com>
Date:   Sat, 11 Mar 2023 06:16:33 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <e3c00116-5283-8c76-7414-66f36fb575fa@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Precedence: bulk


On 3/11/23 3:12 AM, Hans de Goede wrote:
> Hi,
>
> On 3/10/23 20:29, Tom Rix wrote:
>> A rand config causes this link error
>> drivers/platform/x86/x86-android-tablets/lenovo.o: In function `lenovo_yt3_init':
>> lenovo.c:(.init.text+0x6c): undefined reference to `intel_soc_pmic_exec_mipi_pmic_seq_element'
>>
>> The rand config has
>> CONFIG_X86_ANDROID_TABLETS=y
>> CONFIG_PMIC_OPREGION=n
>>
>> PMIC_OPREGION should be selected to build the
>> intel_soc_pmic_exec_mipi_pmic_seq_element symbol.
>>
>> Signed-off-by: Tom Rix <trix@redhat.com>
> Thanks, but this is a dup with:
>
> https://patchwork.kernel.org/project/platform-driver-x86/patch/20230309094035.18736-1-hdegoede@redhat.com/
>
> I just did not get around to pushing that out to pdx86/for-next yet.
> I have just pushed this to pdx86/for-next to avoid further duplicate fixes.
>
> FWIW, since all other users of PMIC_OPREGION use depends on, not select this
> should have been a depends on too. Unless Kconfig symbols are explicitly
> intended to be selected (typically these are hidden Kconfig symbols (1))
> then you should usually use depends on. Mixing depends on and select leads
> to Kconfig circular dependencies errors.

Ok, thanks for the pointer.

Tom

>
> Regards,
>
> Hans
>
>
> 1) A hidden Kconfig symbol is one which is e.g. just 'bool' not
> 'bool "Description of feature here"' these typically are libraries /
>
> helpers.
>
>
>
>
>> ---
>>   drivers/platform/x86/x86-android-tablets/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/platform/x86/x86-android-tablets/Kconfig b/drivers/platform/x86/x86-android-tablets/Kconfig
>> index 2b3daca5380b..65980229d783 100644
>> --- a/drivers/platform/x86/x86-android-tablets/Kconfig
>> +++ b/drivers/platform/x86/x86-android-tablets/Kconfig
>> @@ -6,6 +6,7 @@
>>   config X86_ANDROID_TABLETS
>>   	tristate "X86 Android tablet support"
>>   	depends on I2C && SPI && SERIAL_DEV_BUS && ACPI && EFI && GPIOLIB
>> +	select PMIC_OPREGION
>>   	help
>>   	  X86 tablets which ship with Android as (part of) the factory image
>>   	  typically have various problems with their DSDTs. The factory kernels