mbox series

[v2,0/3] ARM: qcom: defconfig: socinfo + cleanup

Message ID 20220721155356.248319-1-krzysztof.kozlowski@linaro.org
Headers show
Series ARM: qcom: defconfig: socinfo + cleanup | expand

Message

Krzysztof Kozlowski July 21, 2022, 3:53 p.m. UTC
Hi,

v1 from June [1] was did not hit Qualcomm patchwork due to missing maintainers
entry.

Resending with changes:
Patch 1: no changes
Patch 2: new patch
Patch 3: resending although recently Arnd posted global rework.

[1] https://lore.kernel.org/all/20220623110535.177326-1-krzysztof.kozlowski@linaro.org/

Best regards,
Krzysztof

Krzysztof Kozlowski (3):
  ARM: qcom_defconfig: enable socinfo driver
  ARM: qcom: include defconfig in MAINTAINERS
  ARM: qcom_defconfig: order items with savedefconfig

 MAINTAINERS                     |  1 +
 arch/arm/configs/qcom_defconfig | 57 +++++++++++++++++----------------
 2 files changed, 30 insertions(+), 28 deletions(-)

Comments

Luca Weiss July 23, 2022, 6:17 p.m. UTC | #1
On Samstag, 23. Juli 2022 19:36:17 CEST Krzysztof Kozlowski wrote:
> On 23/07/2022 11:58, Luca Weiss wrote:
> > See also
> > https://lore.kernel.org/linux-arm-msm/20191104210943.101393-1-luca@z3ntu.x
> > yz/ (never applied for some reason)
> 
> Mentioned patch is incorrect so should not be applied - it removes at
> least TMPFS which is not desired. I did not check other removed symbols.

For this example: TMPFS is still enabled after this, it's selected by other 
options, like DRM or COMMON_CLK.

Imo not doing this just hides the brokeness as options wouldn't get selected 
anyways when you do "make qcom_defconfig". Savedefconfig afterwards just puts 
reality into the defconfig file. And yes, if some option gets lost then some 
dependency for it probably needs to get enabled as well and this should get 
fixed.

Regards
Luca

> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski July 23, 2022, 6:44 p.m. UTC | #2
On 23/07/2022 20:17, Luca Weiss wrote:
> On Samstag, 23. Juli 2022 19:36:17 CEST Krzysztof Kozlowski wrote:
>> On 23/07/2022 11:58, Luca Weiss wrote:
>>> See also
>>> https://lore.kernel.org/linux-arm-msm/20191104210943.101393-1-luca@z3ntu.x
>>> yz/ (never applied for some reason)
>>
>> Mentioned patch is incorrect so should not be applied - it removes at
>> least TMPFS which is not desired. I did not check other removed symbols.
> 
> For this example: TMPFS is still enabled after this, it's selected by other 
> options, like DRM or COMMON_CLK.

I know, it does not matter. We had this case (with DEBUGFS and probably
others) and the decision was - user visible symbols must no be removed
by savedefconfig.

> 
> Imo not doing this just hides the brokeness as options wouldn't get selected 
> anyways when you do "make qcom_defconfig". Savedefconfig afterwards just puts 
> reality into the defconfig file. And yes, if some option gets lost then some 
> dependency for it probably needs to get enabled as well and this should get 
> fixed.

But dependencies are no being enabled, because expectation is that all
user-visible options are selected by defconfig.


Best regards,
Krzysztof
Luca Weiss July 23, 2022, 6:52 p.m. UTC | #3
On Samstag, 23. Juli 2022 20:44:08 CEST Krzysztof Kozlowski wrote:
> On 23/07/2022 20:17, Luca Weiss wrote:
> > On Samstag, 23. Juli 2022 19:36:17 CEST Krzysztof Kozlowski wrote:
> >> On 23/07/2022 11:58, Luca Weiss wrote:
> >>> See also
> >>> https://lore.kernel.org/linux-arm-msm/20191104210943.101393-1-luca@z3ntu
> >>> .x
> >>> yz/ (never applied for some reason)
> >> 
> >> Mentioned patch is incorrect so should not be applied - it removes at
> >> least TMPFS which is not desired. I did not check other removed symbols.
> > 
> > For this example: TMPFS is still enabled after this, it's selected by
> > other
> > options, like DRM or COMMON_CLK.
> 
> I know, it does not matter. We had this case (with DEBUGFS and probably
> others) and the decision was - user visible symbols must no be removed
> by savedefconfig.

So savedefconfig is "broken" (not doing the correct thing) then or what? Sounds 
like a topic for kconfig maintainers?

> 
> > Imo not doing this just hides the brokeness as options wouldn't get
> > selected anyways when you do "make qcom_defconfig". Savedefconfig
> > afterwards just puts reality into the defconfig file. And yes, if some
> > option gets lost then some dependency for it probably needs to get
> > enabled as well and this should get fixed.
> 
> But dependencies are no being enabled, because expectation is that all
> user-visible options are selected by defconfig.
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski July 23, 2022, 8:56 p.m. UTC | #4
On 23/07/2022 20:52, Luca Weiss wrote:
> On Samstag, 23. Juli 2022 20:44:08 CEST Krzysztof Kozlowski wrote:
>> On 23/07/2022 20:17, Luca Weiss wrote:
>>> On Samstag, 23. Juli 2022 19:36:17 CEST Krzysztof Kozlowski wrote:
>>>> On 23/07/2022 11:58, Luca Weiss wrote:
>>>>> See also
>>>>> https://lore.kernel.org/linux-arm-msm/20191104210943.101393-1-luca@z3ntu
>>>>> .x
>>>>> yz/ (never applied for some reason)
>>>>
>>>> Mentioned patch is incorrect so should not be applied - it removes at
>>>> least TMPFS which is not desired. I did not check other removed symbols.
>>>
>>> For this example: TMPFS is still enabled after this, it's selected by
>>> other
>>> options, like DRM or COMMON_CLK.
>>
>> I know, it does not matter. We had this case (with DEBUGFS and probably
>> others) and the decision was - user visible symbols must no be removed
>> by savedefconfig.
> 
> So savedefconfig is "broken" (not doing the correct thing) then or what? Sounds 
> like a topic for kconfig maintainers?

I agree.

Best regards,
Krzysztof