mbox series

[GIT,PULL,2/4] DaVinci defconfig updates for v5.4

Message ID 20190828151754.21023-2-nsekhar@ti.com
State New
Headers show
Series [GIT,PULL,1/4] DaVinci SoC updates for v5.4 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v5.4/defconfig

Message

Sekhar Nori Aug. 28, 2019, 3:17 p.m. UTC
The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:

  Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v5.4/defconfig

for you to fetch changes up to e869e44f2d82b9b4d35d58ceaeeadb0242bc634c:

  ARM: davinci_all_defconfig: enable GPIO backlight (2019-08-08 14:33:45 +0530)

----------------------------------------------------------------
Contains davinci_all_defconfig refresh using savedefconfig and a
patch to enable GPIO backlight.

----------------------------------------------------------------
Bartosz Golaszewski (2):
      ARM: davinci: refresh davinci_all_defconfig
      ARM: davinci_all_defconfig: enable GPIO backlight

 arch/arm/configs/davinci_all_defconfig | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

Comments

Arnd Bergmann Sept. 4, 2019, 2:51 p.m. UTC | #1
On Wed, Aug 28, 2019 at 5:18 PM Sekhar Nori <nsekhar@ti.com> wrote:
>
> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>
>   Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v5.4/defconfig
>
> for you to fetch changes up to e869e44f2d82b9b4d35d58ceaeeadb0242bc634c:
>
>   ARM: davinci_all_defconfig: enable GPIO backlight (2019-08-08 14:33:45 +0530)
>
> ----------------------------------------------------------------
> Contains davinci_all_defconfig refresh using savedefconfig and a
> patch to enable GPIO backlight.
>
> ----------------------------------------------------------------
> Bartosz Golaszewski (2):
>       ARM: davinci: refresh davinci_all_defconfig
>       ARM: davinci_all_defconfig: enable GPIO backlight

I'm generally not a fan of these 'refresh defconfig' patches when they
silently change options that may or may not be needed.

Some lines are just moved around but these ones
are completely removed:

-# CONFIG_IOSCHED_DEADLINE is not set
-# CONFIG_IOSCHED_CFQ is not set
-CONFIG_PREEMPT=y
-CONFIG_SND_SOC_TLV320AIC3X=m
-CONFIG_SND_SOC_DAVINCI_MCASP=m
-CONFIG_LEDS_TRIGGERS=y
-CONFIG_TI_EDMA=y
-# CONFIG_ARM_UNWIND is not set

I think most of these are ok, but I don't see any explanation
about why you turn off CONFIG_PREEMPT.

       Arnd
Sekhar Nori Sept. 5, 2019, 6:17 a.m. UTC | #2
Hi Arnd,

On 04/09/19 8:21 PM, Arnd Bergmann wrote:
> On Wed, Aug 28, 2019 at 5:18 PM Sekhar Nori <nsekhar@ti.com> wrote:
>>
>> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>>
>>   Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>>
>> are available in the Git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v5.4/defconfig
>>
>> for you to fetch changes up to e869e44f2d82b9b4d35d58ceaeeadb0242bc634c:
>>
>>   ARM: davinci_all_defconfig: enable GPIO backlight (2019-08-08 14:33:45 +0530)
>>
>> ----------------------------------------------------------------
>> Contains davinci_all_defconfig refresh using savedefconfig and a
>> patch to enable GPIO backlight.
>>
>> ----------------------------------------------------------------
>> Bartosz Golaszewski (2):
>>       ARM: davinci: refresh davinci_all_defconfig
>>       ARM: davinci_all_defconfig: enable GPIO backlight
> 
> I'm generally not a fan of these 'refresh defconfig' patches when they
> silently change options that may or may not be needed.

You are right, I should have watched the output more closely. Since this
is something someone doing 'make davinci_all_defconfig' already sees, I
tend to skip the actual patch output.

> 
> Some lines are just moved around but these ones
> are completely removed:
> 
> -# CONFIG_IOSCHED_DEADLINE is not set
> -# CONFIG_IOSCHED_CFQ is not set
> -CONFIG_PREEMPT=y
> -CONFIG_SND_SOC_TLV320AIC3X=m
> -CONFIG_SND_SOC_DAVINCI_MCASP=m
> -CONFIG_LEDS_TRIGGERS=y
> -CONFIG_TI_EDMA=y
> -# CONFIG_ARM_UNWIND is not set
> 
> I think most of these are ok, but I don't see any explanation
> about why you turn off CONFIG_PREEMPT.

So this came because with commit a50a3f4b6a31 ("sched/rt, Kconfig:
Introduce CONFIG_PREEMPT_RT") PREEMPT lost its prompt string. As
suggested by that commit, davinci_all_defconfig should be enabling
CONFIG_PREEMPT_LL=y to retain functionality.

I can respin the branch with the preempt fix first and then the
defconfig refresh. Or I can skip the refresh altogether.

Thanks,
Sekhar
Arnd Bergmann Sept. 5, 2019, 9:15 a.m. UTC | #3
On Thu, Sep 5, 2019 at 8:18 AM Sekhar Nori <nsekhar@ti.com> wrote:
> On 04/09/19 8:21 PM, Arnd Bergmann wrote:
> > On Wed, Aug 28, 2019 at 5:18 PM Sekhar Nori <nsekhar@ti.com> wrote:
> > Some lines are just moved around but these ones
> > are completely removed:
> >
> > -# CONFIG_IOSCHED_DEADLINE is not set
> > -# CONFIG_IOSCHED_CFQ is not set
> > -CONFIG_PREEMPT=y
> > -CONFIG_SND_SOC_TLV320AIC3X=m
> > -CONFIG_SND_SOC_DAVINCI_MCASP=m
> > -CONFIG_LEDS_TRIGGERS=y
> > -CONFIG_TI_EDMA=y
> > -# CONFIG_ARM_UNWIND is not set
> >
> > I think most of these are ok, but I don't see any explanation
> > about why you turn off CONFIG_PREEMPT.
>
> So this came because with commit a50a3f4b6a31 ("sched/rt, Kconfig:
> Introduce CONFIG_PREEMPT_RT") PREEMPT lost its prompt string. As
> suggested by that commit, davinci_all_defconfig should be enabling
> CONFIG_PREEMPT_LL=y to retain functionality.

This got changed back with b8d3349803ba ("sched/rt, Kconfig: Unbreak
def/oldconfig with CONFIG_PREEMPT=y")

> I can respin the branch with the preempt fix first and then the
> defconfig refresh. Or I can skip the refresh altogether.

I think the best way to do these is to split up the refresh into two
or more patches:

- one patch to reorder the options that just moved around
- one patch that removes lines that are no longer needed,
  explaining why there is no replacement
- separate patches for things that are now called something
  else, or got replaced with a different option.

        Arnd
Peter Ujfalusi Sept. 6, 2019, 6:31 a.m. UTC | #4
On 04/09/2019 17.51, Arnd Bergmann wrote:
> On Wed, Aug 28, 2019 at 5:18 PM Sekhar Nori <nsekhar@ti.com> wrote:
>>
>> The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b:
>>
>>   Linus 5.3-rc1 (2019-07-21 14:05:38 -0700)
>>
>> are available in the Git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v5.4/defconfig
>>
>> for you to fetch changes up to e869e44f2d82b9b4d35d58ceaeeadb0242bc634c:
>>
>>   ARM: davinci_all_defconfig: enable GPIO backlight (2019-08-08 14:33:45 +0530)
>>
>> ----------------------------------------------------------------
>> Contains davinci_all_defconfig refresh using savedefconfig and a
>> patch to enable GPIO backlight.
>>
>> ----------------------------------------------------------------
>> Bartosz Golaszewski (2):
>>       ARM: davinci: refresh davinci_all_defconfig
>>       ARM: davinci_all_defconfig: enable GPIO backlight
> 
> I'm generally not a fan of these 'refresh defconfig' patches when they
> silently change options that may or may not be needed.
> 
> Some lines are just moved around but these ones
> are completely removed:
> 
> -# CONFIG_IOSCHED_DEADLINE is not set
> -# CONFIG_IOSCHED_CFQ is not set
> -CONFIG_PREEMPT=y
> -CONFIG_SND_SOC_TLV320AIC3X=m
> -CONFIG_SND_SOC_DAVINCI_MCASP=m
> -CONFIG_LEDS_TRIGGERS=y
> -CONFIG_TI_EDMA=y

EDMA is kind of needed on daVinci, no?
aic3x and McASP can be good if you want audio support on the boards...

> -# CONFIG_ARM_UNWIND is not set
> 
> I think most of these are ok, but I don't see any explanation
> about why you turn off CONFIG_PREEMPT.
> 
>        Arnd
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Arnd Bergmann Sept. 6, 2019, 8:46 a.m. UTC | #5
On Fri, Sep 6, 2019 at 8:31 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> On 04/09/2019 17.51, Arnd Bergmann wrote:
> > On Wed, Aug 28, 2019 at 5:18 PM Sekhar Nori <nsekhar@ti.com> wrote:
> >
> > -# CONFIG_IOSCHED_DEADLINE is not set
> > -# CONFIG_IOSCHED_CFQ is not set
> > -CONFIG_PREEMPT=y
> > -CONFIG_SND_SOC_TLV320AIC3X=m
> > -CONFIG_SND_SOC_DAVINCI_MCASP=m
> > -CONFIG_LEDS_TRIGGERS=y
> > -CONFIG_TI_EDMA=y
>
> EDMA is kind of needed on daVinci, no?
> aic3x and McASP can be good if you want audio support on the boards...

That was my thought as well, but after looking closer I found
that they are enabled implicitly. However, I really prefer not having
to take a closer look when I get a pull request, hence the request
to split up these patches into obvious ones that explain
what is going on.

        Arnd