Rob,
On 12/28/2015 11:29 AM, Wim Van Sebroeck wrote:
Hi Tim,
quoted
On Wed, Dec 2, 2015 at 12:54 PM, Tim Harvey [off-list ref] wrote:
quoted
On Wed, Dec 2, 2015 at 11:11 AM, Akshay Bhat [off-list ref] wrote:
quoted
On 11/06/2015 05:02 PM, Guenter Roeck wrote:
quoted
On Fri, Nov 06, 2015 at 11:53:42AM -0800, Tim Harvey wrote:
quoted
On Thu, Nov 5, 2015 at 2:23 PM, Guenter Roeck [off-list ref] wrote:
quoted
On Thu, Nov 05, 2015 at 04:19:21PM -0500, Akshay Bhat wrote:
quoted
From: Tim Harvey <tharvey@gateworks.com>
The IMX6 watchdog supports assertion of a signal (WDOG_B) which
can be pinmux'd to an external pin. This is typically used for boards
that
have PMIC's in control of the IMX6 power rails. In fact, failure to use
such an external reset on boards with external PMIC's can result in
various
hangs due to the IMX6 not being fully reset [1] as well as the board
failing
to reset because its PMIC has not been reset to provide adequate
voltage for
the CPU when coming out of reset at 800Mhz.
This uses a new device-tree property 'ext-reset-output' to indicate the
board has such a reset and to cause the watchdog to be configured to
assert
WDOG_B instead of an internal reset both on a watchdog timeout and in
system_restart.
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333689.html
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
.../devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 ++
drivers/watchdog/imx2_wdt.c | 20
++++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
- big-endian: If present the watchdog device's registers are
implemented
in big endian mode, otherwise in native mode(same with CPU), for
more
detail please see:
Documentation/devicetree/bindings/regmap/regmap.txt.
+- ext-reset-output: If present the watchdog device is configured to
assert its
Should that have a vendor prefix ? Also, not sure if "-output"
has any real value in the property name. "fsl,external-reset", maybe ?
Hi Guenter,
I don't see why a vendor prefix is necessary - its a feature of the
IMX6 watchdog supported by this driver to be able to trigger an
internal chip-level reset and/or an external signal that can be hooked
to additional hardware.
Sounded like vendor specific to me, but then I am not a devicetree
maintainer,
so I am not an authority on the subject.
Devicetree maintainers,
Any thoughts?
Tim,
After looking at all the other watchdog drivers, it does not appear that
there is any other processor which uses a similar feature. Since imx is the
only processor that appears to support this feature, it might make sense in
making this vendor specific. If in the future it is found more processors
support a similar functionality, it can be revisited and moved out from
being vendor specific?
I'm certainly no expert on device-tree policy. I understand your
point, but realize that the driver in question is imx2_wdt.c
(compatible = "fsl,imx21-wdt"). This is an IP block inside the silicon
of only Freescale chips, so its not like a future omap chip would be
using this driver - only fsl devices. So why would it need a 'vendor'
property any more than its other properties?
Regards,
Tim
Wim,
Does the lack of response mean overwhelming approval?
I haven't heard any valid complaints - what does it take to get this approved?
Regards,
Tim
I have no objections against the idea and the code itself.
But as Guenter pointed out: it would be handy to get feedback from the devicetree maintainers on the above discussion.
Kind regards,
Wim.
Any suggestions on whether a vendor specific prefix is necessary?
Thanks,
Akshay
Rob,
On Thu, Jan 28, 2016 at 6:28 PM, Akshay Bhat [off-list ref] wrote:
quoted
I have no objections against the idea and the code itself.
But as Guenter pointed out: it would be handy to get feedback from the
devicetree maintainers on the above discussion.
Kind regards,
Wim.
Any suggestions on whether a vendor specific prefix is necessary?
Hi Shawn,
On 02/28/2016 08:56 AM, Fabio Estevam wrote:
Rob,
On Thu, Jan 28, 2016 at 6:28 PM, Akshay Bhat [off-list ref] wrote:
quoted
quoted
I have no objections against the idea and the code itself.
But as Guenter pointed out: it would be handy to get feedback from the
devicetree maintainers on the above discussion.
Kind regards,
Wim.
Any suggestions on whether a vendor specific prefix is necessary?
Any comments?
Is this something you can help with, since you are the iMX
architecture/devicetree maintainer? Appreciate any feedback.
Thanks,
Akshay
On Mon, Mar 28, 2016 at 04:19:15PM -0400, Akshay Bhat wrote:
Hi Shawn,
On 02/28/2016 08:56 AM, Fabio Estevam wrote:
quoted
Rob,
On Thu, Jan 28, 2016 at 6:28 PM, Akshay Bhat [off-list ref] wrote:
quoted
quoted
I have no objections against the idea and the code itself.
But as Guenter pointed out: it would be handy to get feedback from the
devicetree maintainers on the above discussion.
Kind regards,
Wim.
Any suggestions on whether a vendor specific prefix is necessary?
Any comments?
Is this something you can help with, since you are the iMX
architecture/devicetree maintainer? Appreciate any feedback.
FWIW,
Acked-by: Shawn Guo <shawnguo@kernel.org>
Guenter,
I think it's reasonable to pick up the patch, if we have it copied to DT
maintainers and on the list for a long period of time, and do not see
any objection from anyone.
Shawn
On Wed, Mar 30, 2016 at 09:22:33AM +0800, Shawn Guo wrote:
On Mon, Mar 28, 2016 at 04:19:15PM -0400, Akshay Bhat wrote:
quoted
Hi Shawn,
On 02/28/2016 08:56 AM, Fabio Estevam wrote:
quoted
Rob,
On Thu, Jan 28, 2016 at 6:28 PM, Akshay Bhat [off-list ref] wrote:
quoted
quoted
I have no objections against the idea and the code itself.
But as Guenter pointed out: it would be handy to get feedback from the
devicetree maintainers on the above discussion.
Kind regards,
Wim.
Any suggestions on whether a vendor specific prefix is necessary?
Any comments?
Is this something you can help with, since you are the iMX
architecture/devicetree maintainer? Appreciate any feedback.
FWIW,
Acked-by: Shawn Guo <shawnguo@kernel.org>
Guenter,
I think it's reasonable to pick up the patch, if we have it copied to DT
maintainers and on the list for a long period of time, and do not see
any objection from anyone.
The question was if the property name should be ext-reset-output or
fsl,ext-reset-output. In my opinion, it should be fsl,ext-reset-output
because it is not a generic property. Tim disagrees.
So we have two options: Change the property name to fsl,ext-reset-output,
which I would accept, or wait for a devicetree maintainer to make a decision.
Guenter
On Wed, Mar 30, 2016 at 02:09:16PM -0700, Guenter Roeck wrote:
The question was if the property name should be ext-reset-output or
fsl,ext-reset-output. In my opinion, it should be fsl,ext-reset-output
because it is not a generic property. Tim disagrees.
So we have two options: Change the property name to fsl,ext-reset-output,
which I would accept, or wait for a devicetree maintainer to make a decision.
Guenter,
I agree with you on this point. Before everyone agrees that this is a
generic binding, we should have vendor prefix for the property.
Tim,
This is a small change which, IMO, shouldn't hold an useful patch from being
merged. Care to resend with the suggested change?
Shawn
From: Tim Harvey <tharvey@gateworks.com> Date: 2016-03-31 18:02:01
On Wed, Mar 30, 2016 at 6:57 PM, Shawn Guo [off-list ref] wrote:
On Wed, Mar 30, 2016 at 02:09:16PM -0700, Guenter Roeck wrote:
quoted
The question was if the property name should be ext-reset-output or
fsl,ext-reset-output. In my opinion, it should be fsl,ext-reset-output
because it is not a generic property. Tim disagrees.
Guenter,
My issue regarding the vendor prefix was not a hard dissagreement but
was more about me understanding the rational behind using vendor
prefixes. In this case the imx2_wdt driver 'is' a vendor specific
driver as its compatible strings are prefixed with 'fsl,' so isn't
'any' property added to devicetree/bindings/watchdog/fsl-imx-wdt.txt
inherently a vendor specific properly already? I assume that is why
the 'big-endian' property isn't 'fsl,big-endian'.
Maybe it's more about if it 'is' marked as a vendor specific property
its much easier to get approval and accepted by a vendor-specific
maintainer?
quoted
So we have two options: Change the property name to fsl,ext-reset-output,
which I would accept, or wait for a devicetree maintainer to make a decision.
Guenter,
I agree with you on this point. Before everyone agrees that this is a
generic binding, we should have vendor prefix for the property.
Tim,
This is a small change which, IMO, shouldn't hold an useful patch from being
merged. Care to resend with the suggested change?
Shawn
On Thu, Mar 31, 2016 at 11:01:58AM -0700, Tim Harvey wrote:
On Wed, Mar 30, 2016 at 6:57 PM, Shawn Guo [off-list ref] wrote:
quoted
On Wed, Mar 30, 2016 at 02:09:16PM -0700, Guenter Roeck wrote:
quoted
The question was if the property name should be ext-reset-output or
fsl,ext-reset-output. In my opinion, it should be fsl,ext-reset-output
because it is not a generic property. Tim disagrees.
Guenter,
My issue regarding the vendor prefix was not a hard dissagreement but
was more about me understanding the rational behind using vendor
prefixes. In this case the imx2_wdt driver 'is' a vendor specific
driver as its compatible strings are prefixed with 'fsl,' so isn't
'any' property added to devicetree/bindings/watchdog/fsl-imx-wdt.txt
inherently a vendor specific properly already? I assume that is why
the 'big-endian' property isn't 'fsl,big-endian'.
We should read it as that 'big-endian' is a generic property defined by
generic bindings - bindings/regmap/regmap.txt, and we just reference the
bindings in fsl-imx-wdt.txt.
Taking mmc bindings as example, bindings/mmc/mmc.txt defines generic
bindings, while bindings/mmc/fsl-imx-esdhc.txt defines i.MX vendor
specific properties, which should ideally have vendor prefix.
Shawn