Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Tomasz Figa <tomasz.figa@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Russell King <linux@armlinux.org.uk>,
	Kukjin Kim <kgene@kernel.org>,
	Vincent Sanders <vince@simtec.co.uk>,
	Simtec Linux Team <linux@simtec.co.uk>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"moderated list:SAMSUNG SOC CLOCK DRIVERS" 
	<linux-samsung-soc@vger.kernel.org>,
	linux-stm32@st-md-mailman.stormreply.com,
	patches@opensource.cirrus.com,
	"open list:COMMON CLK FRAMEWORK" <linux-clk@vger.kernel.org>,
	linux-watchdog@vger.kernel.org,
	Sergio Prado <sergio.prado@e-labworks.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Sylwester Nawrocki <snawrocki@kernel.org>,
	Cedric Roux <sed@free.fr>, Lihua Yao <ylhuajnu@outlook.com>
Subject: Re: [PATCH 5/7] ARM: samsung: Kill useless HAVE_S3C2410_WATCHDOG
Date: Wed, 29 Jul 2020 21:08:52 +0200
Message-ID: <20200729190852.GA5822@kozik-lap> (raw)
In-Reply-To: <CA+Ln22EcU+mhaYJYd3M-S0ZP-mA0bW4Qit-Kh6MyvwzxTRJ3GQ@mail.gmail.com>

On Wed, Jul 29, 2020 at 07:36:38PM +0200, Tomasz Figa wrote:
> 2020年7月29日(水) 19:02 Guenter Roeck <linux@roeck-us.net>:
> >
> > On Wed, Jul 29, 2020 at 06:09:40PM +0200, Krzysztof Kozlowski wrote:
> > > A separate Kconfig option HAVE_S3C2410_WATCHDOG for Samsung SoCs does
> > > not have sense, because:
> > > 1. All ARMv7 and ARMv8 Samsung SoCs have watchdog,
> > > 2. All architecture Kconfigs were selecting it (if WATCHDOG framework is
> > >    chosen),
> > > 3. HAVE_S3C2410_WATCHDOG is doing nothing except being a dependency of
> > >    actual Samsung SoC watchdog driver, which is enabled manually by
> > >    specific defconfigs.
> > >
> > > HAVE_S3C2410_WATCHDOG can be safely removed.
> > >
> >
> > That is not really correct. HAVE_S3C2410_WATCHDOG is used to ensure
> > that users can only enable S3C2410_WATCHDOG if the watchdog actually
> > exists in a system. With this change, it can be enabled for all
> > architectures and platforms.
> >
> > NACK.
> >
> > Guenter
> >
> 
> I'd side with Guenter on this. We better not flood users' screens with
> options that are not relevant to their hardware.
> 
> An alternative here could be making CONFIG_S3C2410_WATCHDOG depend on
> a general symbol for Samsung SoC support if there is such, but then,
> are we 100% sure that all the Samsung SoCs would actually have exactly
> this watchdog? If a new one shows up, one would have to bring back
> this HAVE_S3C2410_WATCHDOG symbol.

Ah, good points. Indeed for all of such SoC drivers we usually just
depend on architecture to limit the choices on other architectures.
In this case it would be:
    depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || ARCH_S5PV210 || COMPILE_TEST

I admit it is pretty long, but we already use this pattern. In shorter
version (less ARCH*) for all drivers, in full version also in:
drivers/iio/adc/Kconfig
drivers/gpu/drm/exynos/Kconfig

Have in mind that in general we follow the first approach and only three
drivers have still the HAVE_xxx option (also HAVE_S3C2410_I2C and
HAVE_S3C_RTC).

I can update therefore the "depends" while removing the
HAVE_S3C2410_WATCHDOG option or just keep it.

Best regards,
Krzysztof


  reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 16:09 [PATCH 0/7] ARM: samsung: Cleanup of various S3C bits Krzysztof Kozlowski
2020-07-29 16:09 ` [PATCH 1/7] clk: samsung: s3c64xx: Declare s3c64xx_clk_init() in shared header Krzysztof Kozlowski
2020-07-29 17:39   ` Tomasz Figa
2020-07-29 19:42   ` Stephen Boyd
2020-07-29 20:16     ` Krzysztof Kozlowski
2020-07-29 16:09 ` [PATCH 2/7] ARM: s3c64xx: Include header to fix -Wmissing-prototypes Krzysztof Kozlowski
2020-07-29 17:46   ` Tomasz Figa
2020-07-29 16:09 ` [PATCH 3/7] ARM: s3c: Remove plat-samsung/.../samsung-time.h Krzysztof Kozlowski
2020-07-29 17:49   ` Tomasz Figa
2020-07-29 19:02     ` Krzysztof Kozlowski
2020-07-29 16:09 ` [PATCH 4/7] ARM: samsung: Fix language typo Krzysztof Kozlowski
2020-07-29 16:09 ` [PATCH 5/7] ARM: samsung: Kill useless HAVE_S3C2410_WATCHDOG Krzysztof Kozlowski
2020-07-29 17:02   ` Guenter Roeck
2020-07-29 17:36     ` Tomasz Figa
2020-07-29 19:08       ` Krzysztof Kozlowski [this message]
2020-07-29 16:09 ` [PATCH 6/7] ARM: s3c64xx: Switch to generic watchdog driver reset Krzysztof Kozlowski
2020-07-29 17:33   ` Tomasz Figa
2020-07-29 19:15     ` Krzysztof Kozlowski
2020-07-29 19:25       ` Krzysztof Kozlowski
2020-07-30 11:34       ` Tomasz Figa
2020-07-30 11:45         ` Krzysztof Kozlowski
2020-07-29 16:09 ` [PATCH 7/7] ARM: s3c24xx: Fix missing system reset Krzysztof Kozlowski
2020-07-29 20:01 ` [PATCH 0/7] ARM: samsung: Cleanup of various S3C bits Arnd Bergmann
2020-07-29 20:15   ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200729190852.GA5822@kozik-lap \
    --to=krzk@kernel.org \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=cw00.choi@samsung.com \
    --cc=kgene@kernel.org \
    --cc=kyungmin.park@samsung.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linux@roeck-us.net \
    --cc=linux@simtec.co.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=mturquette@baylibre.com \
    --cc=patches@opensource.cirrus.com \
    --cc=s.nawrocki@samsung.com \
    --cc=sboyd@kernel.org \
    --cc=sed@free.fr \
    --cc=sergio.prado@e-labworks.com \
    --cc=snawrocki@kernel.org \
    --cc=tomasz.figa@gmail.com \
    --cc=vince@simtec.co.uk \
    --cc=will@kernel.org \
    --cc=wim@linux-watchdog.org \
    --cc=ylhuajnu@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git