All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Andy Yan <andy.yan@rock-chips.com>,
	robh+dt@kernel.org, heiko@sntech.de, john.stultz@linaro.org,
	arnd@arndb.de, bjorn.andersson@linaro.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	alexandre.belloni@free-electrons.com, dbaryshkov@gmail.com,
	sre@kernel.org, matthias.bgg@gmail.com, linux-pm@vger.kernel.org,
	mbrugger@suse.com, lorenzo.pieralisi@arm.com,
	moritz.fischer@ettus.com, richard@nod.at
Subject: Re: [PATCH v9 2/4] power: reset: add reboot mode driver
Date: Mon, 20 Jun 2016 10:40:02 -0400	[thread overview]
Message-ID: <20160620144002.GY12567@windriver.com> (raw)
In-Reply-To: <5767E22F.5030207@samsung.com>

[Re: [PATCH v9 2/4] power: reset: add reboot mode driver] On 20/06/2016 (Mon 14:31) Krzysztof Kozlowski wrote:

> On 06/20/2016 10:28 AM, Andy Yan wrote:
> > Hi Krzysztof:
> > 
> > On 2016年06月20日 16:09, Krzysztof Kozlowski wrote:
> >> On 06/20/2016 08:38 AM, Andy Yan wrote:

[...]

> >>> +
> >>> +config SYSCON_REBOOT_MODE
> >>> +    bool "Generic SYSCON regmap reboot mode driver"
> >> Why not tristate?
> > 
> >    I see many reset drivers in this directories  use bool, so I follow
> > them.

Andy - understood, but mistakes done in the past do not justify
repeating them again in the present.  OK, this is not strictly a mistake
in that it causes an error, but it isn't an ideal approach.

> 
> +Cc Paul,
> 
> I don't mind that although I don't see any particular objections for
> making it module-capable. In the same time I just reminded myself about
> Paul Gortmaker's long effort (like [1] [2]) about removing module
> capability from non-modular drivers.

Thanks -- it is nice to see that people are starting to add this to
their review checklist ; early on they were getting added faster than I
could remove them.  :-(   But I think we are making ground now.

For this case, I don't have any bias for it being built-in vs. being
modular, so long as the code is actually consistent with the Kconfig. 

For existing bool settings I just remove the modular references, since I
can't be extending the functionality to include a modular usage when I
can't test it or even be sure if a module has a sensible use case.

Paul.
--

> 
> Following his rationale, I think either this should be a tristate or the
> module stuff should be removed.
> 
> Best regards,
> Krzysztof
> 
> [1] https://lkml.org/lkml/2016/2/21/180
> [2] https://lkml.org/lkml/2016/6/13/682
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Paul Gortmaker <paul.gortmaker@windriver.com>
To: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Andy Yan <andy.yan@rock-chips.com>, <robh+dt@kernel.org>,
	<heiko@sntech.de>, <john.stultz@linaro.org>, <arnd@arndb.de>,
	<bjorn.andersson@linaro.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<alexandre.belloni@free-electrons.com>, <dbaryshkov@gmail.com>,
	<sre@kernel.org>, <matthias.bgg@gmail.com>,
	<linux-pm@vger.kernel.org>, <mbrugger@suse.com>,
	<lorenzo.pieralisi@arm.com>, <moritz.fischer@ettus.com>,
	<richard@nod.at>
Subject: Re: [PATCH v9 2/4] power: reset: add reboot mode driver
Date: Mon, 20 Jun 2016 10:40:02 -0400	[thread overview]
Message-ID: <20160620144002.GY12567@windriver.com> (raw)
In-Reply-To: <5767E22F.5030207@samsung.com>

[Re: [PATCH v9 2/4] power: reset: add reboot mode driver] On 20/06/2016 (Mon 14:31) Krzysztof Kozlowski wrote:

> On 06/20/2016 10:28 AM, Andy Yan wrote:
> > Hi Krzysztof:
> > 
> > On 2016年06月20日 16:09, Krzysztof Kozlowski wrote:
> >> On 06/20/2016 08:38 AM, Andy Yan wrote:

[...]

> >>> +
> >>> +config SYSCON_REBOOT_MODE
> >>> +    bool "Generic SYSCON regmap reboot mode driver"
> >> Why not tristate?
> > 
> >    I see many reset drivers in this directories  use bool, so I follow
> > them.

Andy - understood, but mistakes done in the past do not justify
repeating them again in the present.  OK, this is not strictly a mistake
in that it causes an error, but it isn't an ideal approach.

> 
> +Cc Paul,
> 
> I don't mind that although I don't see any particular objections for
> making it module-capable. In the same time I just reminded myself about
> Paul Gortmaker's long effort (like [1] [2]) about removing module
> capability from non-modular drivers.

Thanks -- it is nice to see that people are starting to add this to
their review checklist ; early on they were getting added faster than I
could remove them.  :-(   But I think we are making ground now.

For this case, I don't have any bias for it being built-in vs. being
modular, so long as the code is actually consistent with the Kconfig. 

For existing bool settings I just remove the modular references, since I
can't be extending the functionality to include a modular usage when I
can't test it or even be sure if a module has a sensible use case.

Paul.
--

> 
> Following his rationale, I think either this should be a tristate or the
> module stuff should be removed.
> 
> Best regards,
> Krzysztof
> 
> [1] https://lkml.org/lkml/2016/2/21/180
> [2] https://lkml.org/lkml/2016/6/13/682
> 
> 

  reply	other threads:[~2016-06-20 14:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-20  6:34 [PATCH v9 0/4] add reboot mode driver Andy Yan
2016-06-20  6:36 ` [PATCH v9 1/4] dt-bindings: power: reset: add document for reboot-mode driver Andy Yan
2016-06-20  6:38 ` [PATCH v9 2/4] power: reset: add reboot mode driver Andy Yan
2016-06-20  8:09   ` Krzysztof Kozlowski
2016-06-20  8:28     ` Andy Yan
2016-06-20 12:31       ` Krzysztof Kozlowski
2016-06-20 14:40         ` Paul Gortmaker [this message]
2016-06-20 14:40           ` Paul Gortmaker
2016-06-21  6:16           ` Andy Yan
     [not found]             ` <5768DBCB.4090903-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-06-21  9:32               ` Krzysztof Kozlowski
2016-06-21  9:32                 ` Krzysztof Kozlowski
2016-07-06 13:39   ` Krzysztof Kozlowski
2016-06-20  6:39 ` [PATCH v9 3/4] ARM: dts: rockchip: add syscon-reboot-mode DT node Andy Yan
2016-06-20 21:55   ` Bjorn Andersson
2016-06-21  6:08     ` Andy Yan
2016-06-20  6:40 ` [PATCH v9 4/4] ARM64: " Andy Yan

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=20160620144002.GY12567@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=andy.yan@rock-chips.com \
    --cc=arnd@arndb.de \
    --cc=bjorn.andersson@linaro.org \
    --cc=dbaryshkov@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=john.stultz@linaro.org \
    --cc=k.kozlowski@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=matthias.bgg@gmail.com \
    --cc=mbrugger@suse.com \
    --cc=moritz.fischer@ettus.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=sre@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.