[1/2] tty: serial: samsung_tty: build it for any platform
diff mbox series

Message ID 20200220102628.3371996-1-gregkh@linuxfoundation.org
State Not Applicable
Headers show
Series
  • [1/2] tty: serial: samsung_tty: build it for any platform
Related show

Commit Message

Greg KH Feb. 20, 2020, 10:26 a.m. UTC
There is no need to tie this driver to only a specific SoC, or compile
test, so remove that dependancy from the Kconfig rules.

Cc: Kukjin Kim <kgene@kernel.org>
Cc: Donghoon Yu <hoony.yu@samsung.com>
Cc: Hyunki Koo <kkoos00@naver.com>
Cc: HYUN-KI KOO <hyunki00.koo@samsung.com>
Cc: Shinbeom Choi <sbeom.choi@samsung.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/Kconfig | 1 -
 1 file changed, 1 deletion(-)


base-commit: 11a48a5a18c63fd7621bb050228cebf13566e4d8

Comments

Krzysztof Kozlowski Feb. 20, 2020, 10:54 a.m. UTC | #1
On Thu, Feb 20, 2020 at 11:26:27AM +0100, Greg Kroah-Hartman wrote:
> There is no need to tie this driver to only a specific SoC, or compile
> test, so remove that dependancy from the Kconfig rules.
> 
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Donghoon Yu <hoony.yu@samsung.com>
> Cc: Hyunki Koo <kkoos00@naver.com>
> Cc: HYUN-KI KOO <hyunki00.koo@samsung.com>
> Cc: Shinbeom Choi <sbeom.choi@samsung.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/Kconfig | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
Bartlomiej Zolnierkiewicz Feb. 20, 2020, 12:13 p.m. UTC | #2
Hi Greg,

On 2/20/20 11:26 AM, Greg Kroah-Hartman wrote:
> There is no need to tie this driver to only a specific SoC, or compile
> test, so remove that dependancy from the Kconfig rules.

samsung_tty driver is hardware specific driver so why should we
build it for any platform?

This change seems to defeat the whole purpose behind COMPILE_TEST
config option (which allows us to build hardware-specific drivers
without needlessly presenting the user with tons of non-relevant
config options).

Please explain this change some more, are you planing to remove
COMPILE_TEST config option?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Donghoon Yu <hoony.yu@samsung.com>
> Cc: Hyunki Koo <kkoos00@naver.com>
> Cc: HYUN-KI KOO <hyunki00.koo@samsung.com>
> Cc: Shinbeom Choi <sbeom.choi@samsung.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 52eaac21ff9f..a310bd22f1e2 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -237,7 +237,6 @@ config SERIAL_CLPS711X_CONSOLE
>  
>  config SERIAL_SAMSUNG
>  	tristate "Samsung SoC serial support"
> -	depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST
>  	select SERIAL_CORE
>  	help
>  	  Support for the on-chip UARTs on the Samsung S3C24XX series CPUs,
> 
> base-commit: 11a48a5a18c63fd7621bb050228cebf13566e4d8
Geert Uytterhoeven Feb. 25, 2020, 8:52 a.m. UTC | #3
On Thu, Feb 20, 2020 at 1:13 PM Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> On 2/20/20 11:26 AM, Greg Kroah-Hartman wrote:
> > There is no need to tie this driver to only a specific SoC, or compile
> > test, so remove that dependancy from the Kconfig rules.
>
> samsung_tty driver is hardware specific driver so why should we
> build it for any platform?
>
> This change seems to defeat the whole purpose behind COMPILE_TEST
> config option (which allows us to build hardware-specific drivers
> without needlessly presenting the user with tons of non-relevant
> config options).
>
> Please explain this change some more, are you planing to remove
> COMPILE_TEST config option?

+1

I was just going to send a revert...

> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -237,7 +237,6 @@ config SERIAL_CLPS711X_CONSOLE
> >
> >  config SERIAL_SAMSUNG
> >       tristate "Samsung SoC serial support"
> > -     depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST
> >       select SERIAL_CORE
> >       help
> >         Support for the on-chip UARTs on the Samsung S3C24XX series CPUs,
> >
> > base-commit: 11a48a5a18c63fd7621bb050228cebf13566e4d8

Gr{oetje,eeting}s,

                        Geert
Greg KH Feb. 25, 2020, 8:41 p.m. UTC | #4
On Tue, Feb 25, 2020 at 09:52:38AM +0100, Geert Uytterhoeven wrote:
> On Thu, Feb 20, 2020 at 1:13 PM Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > On 2/20/20 11:26 AM, Greg Kroah-Hartman wrote:
> > > There is no need to tie this driver to only a specific SoC, or compile
> > > test, so remove that dependancy from the Kconfig rules.
> >
> > samsung_tty driver is hardware specific driver so why should we
> > build it for any platform?

Why not?

Seriously, this "only this one specific SoC is allowed to build this
driver" is crazy.  It prevents anyone from building a generic kernel
with drivers as a module which are loaded as needed.

That needs to be fixed, and removing this unneeded dependancy on this
driver allows it to be build for any system and then only loaded when
needed.

> > This change seems to defeat the whole purpose behind COMPILE_TEST
> > config option (which allows us to build hardware-specific drivers
> > without needlessly presenting the user with tons of non-relevant
> > config options).
> >
> > Please explain this change some more, are you planing to remove
> > COMPILE_TEST config option?

I want to get rid of this:

> > > -     depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST

We should not need PLAT_SAMSUNG or ARCH_EXYNOS at all, we should be able
to build an arm64 kernel for all platforms.

thanks,

greg k-h
Geert Uytterhoeven Feb. 25, 2020, 9:22 p.m. UTC | #5
Hi Greg,

On Tue, Feb 25, 2020 at 9:41 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Feb 25, 2020 at 09:52:38AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Feb 20, 2020 at 1:13 PM Bartlomiej Zolnierkiewicz
> > <b.zolnierkie@samsung.com> wrote:
> > > On 2/20/20 11:26 AM, Greg Kroah-Hartman wrote:
> > > > There is no need to tie this driver to only a specific SoC, or compile
> > > > test, so remove that dependancy from the Kconfig rules.
> > >
> > > samsung_tty driver is hardware specific driver so why should we
> > > build it for any platform?
>
> Why not?

Because this driver won't bind to a device anyway, when the kernel is
configured without Samsung SoC support.  It will just bloat the kernel,
and asking this question is a silly waste of time for anyone building a
(non-generic) kernel for a non-Samsung SoC.

> Seriously, this "only this one specific SoC is allowed to build this
> driver" is crazy.  It prevents anyone from building a generic kernel
> with drivers as a module which are loaded as needed.

A generic kernel will include Samsung SoC support, hence PLAT_SAMSUNG
or ARCH_EXYNOS will be enabled.

> That needs to be fixed, and removing this unneeded dependancy on this
> driver allows it to be build for any system and then only loaded when
> needed.

It can only be loaded on a Samsung system, which requires PLAT_SAMSUNG
or ARCH_EXYNOS anyway.
It's not like a Samsung serial device can be plugged into your PC's PCI
bus or so, it only exists on Samsung SoCs.

> > > This change seems to defeat the whole purpose behind COMPILE_TEST
> > > config option (which allows us to build hardware-specific drivers
> > > without needlessly presenting the user with tons of non-relevant
> > > config options).
> > >
> > > Please explain this change some more, are you planing to remove
> > > COMPILE_TEST config option?
>
> I want to get rid of this:

IMHO we need _more_ of these dependencies, to avoid all these silly questions
when they don't make sense.

> > > > -     depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST
>
> We should not need PLAT_SAMSUNG or ARCH_EXYNOS at all, we should be able
> to build an arm64 kernel for all platforms.

An arm64 kernel for all platforms will have ARCH_EXYNOS enabled.

Gr{oetje,eeting}s,

                        Geert
Bartlomiej Zolnierkiewicz Feb. 26, 2020, 10:11 a.m. UTC | #6
On 2/25/20 10:22 PM, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Tue, Feb 25, 2020 at 9:41 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Tue, Feb 25, 2020 at 09:52:38AM +0100, Geert Uytterhoeven wrote:
>>> On Thu, Feb 20, 2020 at 1:13 PM Bartlomiej Zolnierkiewicz
>>> <b.zolnierkie@samsung.com> wrote:
>>>> On 2/20/20 11:26 AM, Greg Kroah-Hartman wrote:
>>>>> There is no need to tie this driver to only a specific SoC, or compile
>>>>> test, so remove that dependancy from the Kconfig rules.
>>>>
>>>> samsung_tty driver is hardware specific driver so why should we
>>>> build it for any platform?
>>
>> Why not?
> 
> Because this driver won't bind to a device anyway, when the kernel is
> configured without Samsung SoC support.  It will just bloat the kernel,
> and asking this question is a silly waste of time for anyone building a
> (non-generic) kernel for a non-Samsung SoC.
> 
>> Seriously, this "only this one specific SoC is allowed to build this
>> driver" is crazy.  It prevents anyone from building a generic kernel
>> with drivers as a module which are loaded as needed.
> 
> A generic kernel will include Samsung SoC support, hence PLAT_SAMSUNG
> or ARCH_EXYNOS will be enabled.
> 
>> That needs to be fixed, and removing this unneeded dependancy on this
>> driver allows it to be build for any system and then only loaded when
>> needed.
> 
> It can only be loaded on a Samsung system, which requires PLAT_SAMSUNG
> or ARCH_EXYNOS anyway.
> It's not like a Samsung serial device can be plugged into your PC's PCI
> bus or so, it only exists on Samsung SoCs.
> 
>>>> This change seems to defeat the whole purpose behind COMPILE_TEST
>>>> config option (which allows us to build hardware-specific drivers
>>>> without needlessly presenting the user with tons of non-relevant
>>>> config options).
>>>>
>>>> Please explain this change some more, are you planing to remove
>>>> COMPILE_TEST config option?
>>
>> I want to get rid of this:
> 
> IMHO we need _more_ of these dependencies, to avoid all these silly questions
> when they don't make sense.
> 
>>>>> -     depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST
>>
>> We should not need PLAT_SAMSUNG or ARCH_EXYNOS at all, we should be able
>> to build an arm64 kernel for all platforms.
> 
> An arm64 kernel for all platforms will have ARCH_EXYNOS enabled.

+1 on all comments from Geert

IMHO this change should be reverted (it doesn't fix anything and
only makes kernel configuration harder).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Patch
diff mbox series

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 52eaac21ff9f..a310bd22f1e2 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -237,7 +237,6 @@  config SERIAL_CLPS711X_CONSOLE
 
 config SERIAL_SAMSUNG
 	tristate "Samsung SoC serial support"
-	depends on PLAT_SAMSUNG || ARCH_EXYNOS || COMPILE_TEST
 	select SERIAL_CORE
 	help
 	  Support for the on-chip UARTs on the Samsung S3C24XX series CPUs,