diff mbox series

[v2,1/3] kconfig: add dependencies of POWER_RESET for mips malta

Message ID 1c17f017d6c837ef887d08bd2f85102df3fbc17c.1693535514.git.tanyuan@tinylab.org (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series Add dependencies of POWER_RESET for MIPS Malta, x86, and PowerMac | expand

Commit Message

Yuan Tan Sept. 1, 2023, 2:42 a.m. UTC
MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set
for convenience.

Suggested-by: Zhangjin Wu <falcon@tinylab.org>
Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
---
 arch/mips/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

Comments

Philippe Mathieu-Daudé Sept. 4, 2023, 7:40 a.m. UTC | #1
Hi,

On 1/9/23 04:42, Yuan Tan wrote:
> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set
> for convenience.
> 
> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
> ---
>   arch/mips/Kconfig | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index bc8421859006..13bacbd05125 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -547,6 +547,9 @@ config MIPS_MALTA
>   	select MIPS_L1_CACHE_SHIFT_6
>   	select MIPS_MSC
>   	select PCI_GT64XXX_PCI0
> +	select PCI if POWER_RESET
> +	select PCI_QUIRKS if POWER_RESET
> +	select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>   	select SMP_UP if SMP
>   	select SWAP_IO_SPACE
>   	select SYS_HAS_CPU_MIPS32_R1

Shouldn't we also update the _defconfig files?
Yuan Tan Sept. 4, 2023, 9:24 a.m. UTC | #2
Hi,

On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote:
> Hi,
>
> On 1/9/23 04:42, Yuan Tan wrote:
>> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
>> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set
>> for convenience.
>>
>> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>> ---
>>   arch/mips/Kconfig | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index bc8421859006..13bacbd05125 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -547,6 +547,9 @@ config MIPS_MALTA
>>       select MIPS_L1_CACHE_SHIFT_6
>>       select MIPS_MSC
>>       select PCI_GT64XXX_PCI0
>> +    select PCI if POWER_RESET
>> +    select PCI_QUIRKS if POWER_RESET
>> +    select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>>       select SMP_UP if SMP
>>       select SWAP_IO_SPACE
>>       select SYS_HAS_CPU_MIPS32_R1
>
> Shouldn't we also update the _defconfig files?
>
Sorry, in my last email, I forgot to reply to all. So I am now resending 
this email.

In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already been 
set and PCI_QUIRKS is also selected by FSL_PCI [=n].

So shutdown and reboot with malta_defconfig is working and there is no 
need to update the malta_defconfig 🙂
Philippe Mathieu-Daudé Sept. 4, 2023, 10:51 a.m. UTC | #3
On 4/9/23 11:24, Yuan Tan wrote:
> Hi,
> 
> On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 1/9/23 04:42, Yuan Tan wrote:
>>> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
>>> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set
>>> for convenience.
>>>
>>> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
>>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>>> ---
>>>   arch/mips/Kconfig | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>> index bc8421859006..13bacbd05125 100644
>>> --- a/arch/mips/Kconfig
>>> +++ b/arch/mips/Kconfig
>>> @@ -547,6 +547,9 @@ config MIPS_MALTA
>>>       select MIPS_L1_CACHE_SHIFT_6
>>>       select MIPS_MSC
>>>       select PCI_GT64XXX_PCI0
>>> +    select PCI if POWER_RESET
>>> +    select PCI_QUIRKS if POWER_RESET
>>> +    select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>>>       select SMP_UP if SMP
>>>       select SWAP_IO_SPACE
>>>       select SYS_HAS_CPU_MIPS32_R1
>>
>> Shouldn't we also update the _defconfig files?
>>
> Sorry, in my last email, I forgot to reply to all. So I am now resending 
> this email.
> 
> In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already been 
> set and PCI_QUIRKS is also selected by FSL_PCI [=n].
> 
> So shutdown and reboot with malta_defconfig is working and there is no 
> need to update the malta_defconfig 🙂

Since the dependency is now enforced by Kconfig, the defconfig can
be simplified:

--- a/arch/mips/configs/malta_defconfig
+++ b/arch/mips/configs/malta_defconfig
@@ -306,3 +306,2 @@ CONFIG_SERIAL_8250_CONSOLE=y
  CONFIG_POWER_RESET=y
-CONFIG_POWER_RESET_PIIX4_POWEROFF=y
  CONFIG_POWER_RESET_SYSCON=y

But maybe we don't care, I don't know.
Christophe Leroy Sept. 4, 2023, 10:58 a.m. UTC | #4
Le 04/09/2023 à 12:51, Philippe Mathieu-Daudé a écrit :
> On 4/9/23 11:24, Yuan Tan wrote:
>> Hi,
>>
>> On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>>
>>> On 1/9/23 04:42, Yuan Tan wrote:
>>>> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
>>>> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set
>>>> for convenience.
>>>>
>>>> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
>>>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>>>> ---
>>>>   arch/mips/Kconfig | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>> index bc8421859006..13bacbd05125 100644
>>>> --- a/arch/mips/Kconfig
>>>> +++ b/arch/mips/Kconfig
>>>> @@ -547,6 +547,9 @@ config MIPS_MALTA
>>>>       select MIPS_L1_CACHE_SHIFT_6
>>>>       select MIPS_MSC
>>>>       select PCI_GT64XXX_PCI0
>>>> +    select PCI if POWER_RESET
>>>> +    select PCI_QUIRKS if POWER_RESET
>>>> +    select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>>>>       select SMP_UP if SMP
>>>>       select SWAP_IO_SPACE
>>>>       select SYS_HAS_CPU_MIPS32_R1
>>>
>>> Shouldn't we also update the _defconfig files?
>>>
>> Sorry, in my last email, I forgot to reply to all. So I am now 
>> resending this email.
>>
>> In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already 
>> been set and PCI_QUIRKS is also selected by FSL_PCI [=n].
>>
>> So shutdown and reboot with malta_defconfig is working and there is no 
>> need to update the malta_defconfig 🙂
> 
> Since the dependency is now enforced by Kconfig, the defconfig can
> be simplified:
> 
> --- a/arch/mips/configs/malta_defconfig
> +++ b/arch/mips/configs/malta_defconfig
> @@ -306,3 +306,2 @@ CONFIG_SERIAL_8250_CONSOLE=y
>   CONFIG_POWER_RESET=y
> -CONFIG_POWER_RESET_PIIX4_POWEROFF=y
>   CONFIG_POWER_RESET_SYSCON=y
> 
> But maybe we don't care, I don't know.

I understand from what you say that you update malta_defconfig manually ?

defconfigs shouldn't be updated manually.

Once you have the new .config you should use "make savedefconfig" then 
replace your file by the newly generated defconfig file.

Christophe
Yuan Tan Sept. 4, 2023, 5:40 p.m. UTC | #5
On 9/4/2023 6:58 PM, Christophe Leroy wrote:
>
> Le 04/09/2023 à 12:51, Philippe Mathieu-Daudé a écrit :
>> On 4/9/23 11:24, Yuan Tan wrote:
>>> Hi,
>>>
>>> On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote:
>>>> Hi,
>>>>
>>>> On 1/9/23 04:42, Yuan Tan wrote:
>>>>> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
>>>>> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is set
>>>>> for convenience.
>>>>>
>>>>> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
>>>>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>>>>> ---
>>>>>    arch/mips/Kconfig | 3 +++
>>>>>    1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>>> index bc8421859006..13bacbd05125 100644
>>>>> --- a/arch/mips/Kconfig
>>>>> +++ b/arch/mips/Kconfig
>>>>> @@ -547,6 +547,9 @@ config MIPS_MALTA
>>>>>        select MIPS_L1_CACHE_SHIFT_6
>>>>>        select MIPS_MSC
>>>>>        select PCI_GT64XXX_PCI0
>>>>> +    select PCI if POWER_RESET
>>>>> +    select PCI_QUIRKS if POWER_RESET
>>>>> +    select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>>>>>        select SMP_UP if SMP
>>>>>        select SWAP_IO_SPACE
>>>>>        select SYS_HAS_CPU_MIPS32_R1
>>>> Shouldn't we also update the _defconfig files?
>>>>
>>> Sorry, in my last email, I forgot to reply to all. So I am now
>>> resending this email.
>>>
>>> In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already
>>> been set and PCI_QUIRKS is also selected by FSL_PCI [=n].
>>>
>>> So shutdown and reboot with malta_defconfig is working and there is no
>>> need to update the malta_defconfig 🙂
>> Since the dependency is now enforced by Kconfig, the defconfig can
>> be simplified:
>>
>> --- a/arch/mips/configs/malta_defconfig
>> +++ b/arch/mips/configs/malta_defconfig
>> @@ -306,3 +306,2 @@ CONFIG_SERIAL_8250_CONSOLE=y
>>    CONFIG_POWER_RESET=y
>> -CONFIG_POWER_RESET_PIIX4_POWEROFF=y
>>    CONFIG_POWER_RESET_SYSCON=y
>>
>> But maybe we don't care, I don't know.
> I understand from what you say that you update malta_defconfig manually ?
>
> defconfigs shouldn't be updated manually.
>
> Once you have the new .config you should use "make savedefconfig" then
> replace your file by the newly generated defconfig file.
>
> Christophe

To do so, I just unset CONFIG_POWER_RESET and set it again in 
menuconfig, then "make savedefconfig". The POWER_RESET part is simplified.

  CONFIG_POWER_RESET=y
-CONFIG_POWER_RESET_PIIX4_POWEROFF=y
-CONFIG_POWER_RESET_SYSCON=y

  However, I found that there's other changes in this new 
malta_defconfig, for example

CONFIG_NLS_KOI8_U=m CONFIG_CRYPTO_CRYPTD=m -CONFIG_CRYPTO_LRW=m 
-CONFIG_CRYPTO_PCBC=m -CONFIG_CRYPTO_HMAC=y -CONFIG_CRYPTO_XCBC=m 
-CONFIG_CRYPTO_MD4=m -CONFIG_CRYPTO_SHA512=m -CONFIG_CRYPTO_WP512=m 
-CONFIG_CRYPTO_ANUBIS=m CONFIG_CRYPTO_BLOWFISH=m CONFIG_CRYPTO_CAMELLIA=m

Should I import all these changes in a commit? Or only POWER_RESET part.
Philippe Mathieu-Daudé Sept. 4, 2023, 7:08 p.m. UTC | #6
On 4/9/23 19:40, Yuan Tan wrote:
> 
> On 9/4/2023 6:58 PM, Christophe Leroy wrote:
>>
>> Le 04/09/2023 à 12:51, Philippe Mathieu-Daudé a écrit :
>>> On 4/9/23 11:24, Yuan Tan wrote:
>>>> Hi,
>>>>
>>>> On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote:
>>>>> Hi,
>>>>>
>>>>> On 1/9/23 04:42, Yuan Tan wrote:
>>>>>> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
>>>>>> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET 
>>>>>> is set
>>>>>> for convenience.
>>>>>>
>>>>>> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
>>>>>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>>>>>> ---
>>>>>>    arch/mips/Kconfig | 3 +++
>>>>>>    1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>>>> index bc8421859006..13bacbd05125 100644
>>>>>> --- a/arch/mips/Kconfig
>>>>>> +++ b/arch/mips/Kconfig
>>>>>> @@ -547,6 +547,9 @@ config MIPS_MALTA
>>>>>>        select MIPS_L1_CACHE_SHIFT_6
>>>>>>        select MIPS_MSC
>>>>>>        select PCI_GT64XXX_PCI0
>>>>>> +    select PCI if POWER_RESET
>>>>>> +    select PCI_QUIRKS if POWER_RESET
>>>>>> +    select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>>>>>>        select SMP_UP if SMP
>>>>>>        select SWAP_IO_SPACE
>>>>>>        select SYS_HAS_CPU_MIPS32_R1
>>>>> Shouldn't we also update the _defconfig files?
>>>>>
>>>> Sorry, in my last email, I forgot to reply to all. So I am now
>>>> resending this email.
>>>>
>>>> In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already
>>>> been set and PCI_QUIRKS is also selected by FSL_PCI [=n].
>>>>
>>>> So shutdown and reboot with malta_defconfig is working and there is no
>>>> need to update the malta_defconfig 🙂
>>> Since the dependency is now enforced by Kconfig, the defconfig can
>>> be simplified:
>>>
>>> --- a/arch/mips/configs/malta_defconfig
>>> +++ b/arch/mips/configs/malta_defconfig
>>> @@ -306,3 +306,2 @@ CONFIG_SERIAL_8250_CONSOLE=y
>>>    CONFIG_POWER_RESET=y
>>> -CONFIG_POWER_RESET_PIIX4_POWEROFF=y
>>>    CONFIG_POWER_RESET_SYSCON=y
>>>
>>> But maybe we don't care, I don't know.
>> I understand from what you say that you update malta_defconfig manually ?
>>
>> defconfigs shouldn't be updated manually.
>>
>> Once you have the new .config you should use "make savedefconfig" then
>> replace your file by the newly generated defconfig file.
>>
>> Christophe
> 
> To do so, I just unset CONFIG_POWER_RESET and set it again in 
> menuconfig, then "make savedefconfig". The POWER_RESET part is simplified.
> 
>   CONFIG_POWER_RESET=y
> -CONFIG_POWER_RESET_PIIX4_POWEROFF=y
> -CONFIG_POWER_RESET_SYSCON=y
> 
>   However, I found that there's other changes in this new 
> malta_defconfig, for example
> 
> CONFIG_NLS_KOI8_U=m CONFIG_CRYPTO_CRYPTD=m -CONFIG_CRYPTO_LRW=m 
> -CONFIG_CRYPTO_PCBC=m -CONFIG_CRYPTO_HMAC=y -CONFIG_CRYPTO_XCBC=m 
> -CONFIG_CRYPTO_MD4=m -CONFIG_CRYPTO_SHA512=m -CONFIG_CRYPTO_WP512=m 
> -CONFIG_CRYPTO_ANUBIS=m CONFIG_CRYPTO_BLOWFISH=m CONFIG_CRYPTO_CAMELLIA=m
> 
> Should I import all these changes in a commit? Or only POWER_RESET part.

I'd first update the defconfigs with mainline (as a cleanup)
then apply your series on top, re-running 'make savedefconfig'
you should get only the changes relevant to your work.
Yuan Tan Sept. 14, 2023, 7:28 a.m. UTC | #7
On 9/4/2023 6:51 PM, Philippe Mathieu-Daudé wrote:
> On 4/9/23 11:24, Yuan Tan wrote:
>> Hi,
>>
>> On 9/4/2023 3:40 PM, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>>
>>> On 1/9/23 04:42, Yuan Tan wrote:
>>>> MIPS Malta's power off depends on PCI, PCI_QUIRKS, and
>>>> POWER_RESET_PIIX4_POWEROFF to work. Enable them when POWER_RESET is 
>>>> set
>>>> for convenience.
>>>>
>>>> Suggested-by: Zhangjin Wu <falcon@tinylab.org>
>>>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>>>> ---
>>>>   arch/mips/Kconfig | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>> index bc8421859006..13bacbd05125 100644
>>>> --- a/arch/mips/Kconfig
>>>> +++ b/arch/mips/Kconfig
>>>> @@ -547,6 +547,9 @@ config MIPS_MALTA
>>>>       select MIPS_L1_CACHE_SHIFT_6
>>>>       select MIPS_MSC
>>>>       select PCI_GT64XXX_PCI0
>>>> +    select PCI if POWER_RESET
>>>> +    select PCI_QUIRKS if POWER_RESET
>>>> +    select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
>>>>       select SMP_UP if SMP
>>>>       select SWAP_IO_SPACE
>>>>       select SYS_HAS_CPU_MIPS32_R1
>>>
>>> Shouldn't we also update the _defconfig files?
>>>
>> Sorry, in my last email, I forgot to reply to all. So I am now 
>> resending this email.
>>
>> In malta_defconfig, PCI and POWER_RESET_PIIX4_POWEROFF have already 
>> been set and PCI_QUIRKS is also selected by FSL_PCI [=n].
>>
>> So shutdown and reboot with malta_defconfig is working and there is 
>> no need to update the malta_defconfig 🙂
>
> Since the dependency is now enforced by Kconfig, the defconfig can
> be simplified:
>
> --- a/arch/mips/configs/malta_defconfig
> +++ b/arch/mips/configs/malta_defconfig
> @@ -306,3 +306,2 @@ CONFIG_SERIAL_8250_CONSOLE=y
>  CONFIG_POWER_RESET=y
> -CONFIG_POWER_RESET_PIIX4_POWEROFF=y
>  CONFIG_POWER_RESET_SYSCON=y
>
> But maybe we don't care, I don't know.
After testing, I found that "savedefconfig" will automatically generate 
the simplified configuration.

As I have to use "savedefconfig" on the latest branch of the three 
architectures, in v3, I will send a separate patch for each architecture.

Thanks to your advice.
diff mbox series

Patch

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index bc8421859006..13bacbd05125 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -547,6 +547,9 @@  config MIPS_MALTA
 	select MIPS_L1_CACHE_SHIFT_6
 	select MIPS_MSC
 	select PCI_GT64XXX_PCI0
+	select PCI if POWER_RESET
+	select PCI_QUIRKS if POWER_RESET
+	select POWER_RESET_PIIX4_POWEROFF if POWER_RESET
 	select SMP_UP if SMP
 	select SWAP_IO_SPACE
 	select SYS_HAS_CPU_MIPS32_R1