| Message ID | 20200921054821.26071-1-qiang.zhao@nxp.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2,1/3] dt-bindings: rtc: pcf2127: Add bindings for nxp,pcf2127 | expand |
Hi, You forgot to copy the watchdog maintainers, I think such a property should be discussed with them. Note that I'm still convinced this is not a complete solution, see: https://lore.kernel.org/linux-rtc/20200716181816.GF3428@piout.net/ On 21/09/2020 13:48:19+0800, Qiang Zhao wrote: > From: Zhao Qiang <qiang.zhao@nxp.com> > > Add bindings for nxp,pcf2127 > > Signed-off-by: Zhao Qiang <qiang.zhao@nxp.com> > --- > Changes for v2: > - modify the format to yaml > - add compitable "nxp,pca2129" > > .../devicetree/bindings/rtc/nxp,pcf2127.yaml | 41 ++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml > new file mode 100644 > index 0000000..226a0b2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/rtc/nxp,pcf2127.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: PCF RTCs > + > +maintainers: > + - Qiang Zhao <qiang.zhao@nxp.com> > + > +allOf: > + - $ref: "rtc.yaml#" > + > +properties: > + compatible: > + enum: > + - nxp,pcf2127 > + - nxp,pcf2129 > + - nxp,pca2129 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + no-watchdog: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + With this property, the device will not registered as a watchdog device. > + > + start-year: true > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +... > -- > 2.7.4 >
On 21/09/2020 13:48:19+0800, Qiang Zhao wrote: > -----Original Message----- > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > Sent: 2020年9月23日 17:45 > To: Qiang Zhao <qiang.zhao@nxp.com> > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck > <linux@roeck-us.net>; linux-watchdog@vger.kernel.org; > a.zummo@towertech.it; robh+dt@kernel.org; linux-rtc@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for > nxp,pcf2127 > > Hi, > > You forgot to copy the watchdog maintainers, I think such a property should be > discussed with them. > > Note that I'm still convinced this is not a complete solution, see: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern > el.org%2Flinux-rtc%2F20200716181816.GF3428%40piout.net%2F&data= > 02%7C01%7Cqiang.zhao%40nxp.com%7Cb71f79a044b0493d6d4f08d85fa551c > b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637364510931174 > 355&sdata=%2BOxrzB8RIuxM9Let5slhfCVmMm6PMNoERDeHC9%2Fdxng > %3D&reserved=0 > Yes, you are right, There is not a fundamental solution. However it somewhat avoid this situation at least. And if without this issue, is it correct to register a rtc device as watchdog no matter it is used as watchdog on the board? Every time Linux are booted up, watchdog device should be configured to the right one manually. So the patch are useful, even though it is not for the issue. What should we do to really resolve this issue? Best Regards Qiang Zhao
Hello, On Thu, Sep 24, 2020 at 03:20:33AM +0000, Qiang Zhao wrote: > On 21/09/2020 13:48:19+0800, Qiang Zhao wrote: > > > -----Original Message----- > > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Sent: 2020年9月23日 17:45 > > To: Qiang Zhao <qiang.zhao@nxp.com> > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck > > <linux@roeck-us.net>; linux-watchdog@vger.kernel.org; > > a.zummo@towertech.it; robh+dt@kernel.org; linux-rtc@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Uwe Kleine-König > > <u.kleine-koenig@pengutronix.de> > > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for > > nxp,pcf2127 > > > > Hi, > > > > You forgot to copy the watchdog maintainers, I think such a property should be > > discussed with them. > > > > Note that I'm still convinced this is not a complete solution, see: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern > > el.org%2Flinux-rtc%2F20200716181816.GF3428%40piout.net%2F&data= > > 02%7C01%7Cqiang.zhao%40nxp.com%7Cb71f79a044b0493d6d4f08d85fa551c > > b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637364510931174 > > 355&sdata=%2BOxrzB8RIuxM9Let5slhfCVmMm6PMNoERDeHC9%2Fdxng > > %3D&reserved=0 haha > Yes, you are right, There is not a fundamental solution. > However it somewhat avoid this situation at least. > > And if without this issue, > is it correct to register a rtc device as watchdog no matter it is used as watchdog on the board? > Every time Linux are booted up, watchdog device should be configured to the right one manually. > So the patch are useful, even though it is not for the issue. > > What should we do to really resolve this issue? I still think we need a kernel solution here. I would expect that most assembled pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST output connected to something that resets the machine). So my favoured solution would be a positive property like: has-watchdog; or something similar. In my eyes this is definitely something we want to specify in the device tree because it is a relevant hardware property. I consider it a bug to give a watchdog device to userspace that isn't functional. Best regards Uwe
On Thu, Sep 24, 2020 at 15:05AM +0000, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: 2020年9月24日 15:05 > To: Qiang Zhao <qiang.zhao@nxp.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Wim Van Sebroeck > <wim@linux-watchdog.org>; Guenter Roeck <linux@roeck-us.net>; > linux-watchdog@vger.kernel.org; a.zummo@towertech.it; robh+dt@kernel.org; > linux-rtc@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings for > nxp,pcf2127 > > Hello, > > On Thu, Sep 24, 2020 at 03:20:33AM +0000, Qiang Zhao wrote: > > On 21/09/2020 13:48:19+0800, Qiang Zhao wrote: > > > > > -----Original Message----- > > > From: Alexandre Belloni <alexandre.belloni@bootlin.com> > > > Sent: 2020年9月23日 17:45 > > > To: Qiang Zhao <qiang.zhao@nxp.com> > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck > > > <linux@roeck-us.net>; linux-watchdog@vger.kernel.org; > > > a.zummo@towertech.it; robh+dt@kernel.org; linux-rtc@vger.kernel.org; > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Uwe > > > Kleine-König <u.kleine-koenig@pengutronix.de> > > > Subject: Re: [Patch v2 1/3] dt-bindings: rtc: pcf2127: Add bindings > > > for > > > nxp,pcf2127 > > > > > > Hi, > > > > > > You forgot to copy the watchdog maintainers, I think such a property > > > should be discussed with them. > > > > > > Note that I'm still convinced this is not a complete solution, see: > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo > > > re.kern > > > > el.org%2Flinux-rtc%2F20200716181816.GF3428%40piout.net%2F&data= > > > > 02%7C01%7Cqiang.zhao%40nxp.com%7Cb71f79a044b0493d6d4f08d85fa551c > > > > b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637364510931174 > > > > 355&sdata=%2BOxrzB8RIuxM9Let5slhfCVmMm6PMNoERDeHC9%2Fdxng > > > %3D&reserved=0 > > haha > > > Yes, you are right, There is not a fundamental solution. > > However it somewhat avoid this situation at least. > > > > And if without this issue, > > is it correct to register a rtc device as watchdog no matter it is used as > watchdog on the board? > > Every time Linux are booted up, watchdog device should be configured to the > right one manually. > > So the patch are useful, even though it is not for the issue. > > > > What should we do to really resolve this issue? > > I still think we need a kernel solution here. I would expect that most assembled > pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST output > connected to something that resets the machine). > > So my favoured solution would be a positive property like: > > has-watchdog; > > or something similar. In my eyes this is definitely something we want to specify > in the device tree because it is a relevant hardware property. > I consider it a bug to give a watchdog device to userspace that isn't functional. > > Best regards > Uwe I strongly agree with you! It should be positive property. However, we couldn't identify which board are using pcf2127 as watchdog, So we are unable to modify the boards' dts to correct (watchdog or not) in this patchset. I noticed that only LS series platforms and imx6 have pcf2127 node, as far as I know, the LS platforms don't use it as watchdog, But I am not sure about imx6 > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | https://www.pengutronix.de/ | Best Regards Qiang Zhao
Hi, On 24/09/2020 07:23:18+0000, Qiang Zhao wrote: > > > Yes, you are right, There is not a fundamental solution. > > > However it somewhat avoid this situation at least. > > > > > > And if without this issue, > > > is it correct to register a rtc device as watchdog no matter it is used as > > watchdog on the board? > > > Every time Linux are booted up, watchdog device should be configured to the > > right one manually. > > > So the patch are useful, even though it is not for the issue. > > > > > > What should we do to really resolve this issue? > > > > I still think we need a kernel solution here. I would expect that most assembled > > pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST output > > connected to something that resets the machine). > > > > So my favoured solution would be a positive property like: > > > > has-watchdog; > > > > or something similar. In my eyes this is definitely something we want to specify > > in the device tree because it is a relevant hardware property. > > I consider it a bug to give a watchdog device to userspace that isn't functional. > > > > Best regards > > Uwe > > I strongly agree with you! It should be positive property. > However, we couldn't identify which board are using pcf2127 as watchdog, > So we are unable to modify the boards' dts to correct (watchdog or not) in this patchset. > > I noticed that only LS series platforms and imx6 have pcf2127 node, as far as I know, the LS platforms don't use it as watchdog, > But I am not sure about imx6 > I don't think there is any user upstream and it is recent engouh that we can probably make that a positive property. Bruno, is it ok for you? you are the only know user of the feature.
Den tor. 24. sep. 2020 kl. 09.47 skrev Alexandre Belloni <alexandre.belloni@bootlin.com>: > > Hi, > > On 24/09/2020 07:23:18+0000, Qiang Zhao wrote: > > > > Yes, you are right, There is not a fundamental solution. > > > > However it somewhat avoid this situation at least. > > > > > > > > And if without this issue, > > > > is it correct to register a rtc device as watchdog no matter it is used as > > > watchdog on the board? > > > > Every time Linux are booted up, watchdog device should be configured to the > > > right one manually. > > > > So the patch are useful, even though it is not for the issue. > > > > > > > > What should we do to really resolve this issue? > > > > > > I still think we need a kernel solution here. I would expect that most assembled > > > pcf2127 chips are unable to act as a watchdog (i.e. don't have the RST output > > > connected to something that resets the machine). > > > > > > So my favoured solution would be a positive property like: > > > > > > has-watchdog; > > > > > > or something similar. In my eyes this is definitely something we want to specify > > > in the device tree because it is a relevant hardware property. > > > I consider it a bug to give a watchdog device to userspace that isn't functional. > > > > > > Best regards > > > Uwe > > > > I strongly agree with you! It should be positive property. > > However, we couldn't identify which board are using pcf2127 as watchdog, > > So we are unable to modify the boards' dts to correct (watchdog or not) in this patchset. > > > > I noticed that only LS series platforms and imx6 have pcf2127 node, as far as I know, the LS platforms don't use it as watchdog, > > But I am not sure about imx6 > > > > I don't think there is any user upstream and it is recent engouh that we > can probably make that a positive property. > > Bruno, is it ok for you? you are the only know user of the feature. Hi This seems like an okay solution to me. I have a patch series on the way with a new product dts[1] that is using the watchdog feature in the pcf2127 chip. I know that the watchdog feature is used by other products with out-of-tree dts, but I will make sure to give them a heads up. /Bruno [1] https://lore.kernel.org/linux-arm-kernel/20200923154024.11417-2-bruno.thomsen@gmail.com/
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml new file mode 100644 index 0000000..226a0b2 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rtc/nxp,pcf2127.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: PCF RTCs + +maintainers: + - Qiang Zhao <qiang.zhao@nxp.com> + +allOf: + - $ref: "rtc.yaml#" + +properties: + compatible: + enum: + - nxp,pcf2127 + - nxp,pcf2129 + - nxp,pca2129 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + no-watchdog: + $ref: /schemas/types.yaml#/definitions/flag + description: + With this property, the device will not registered as a watchdog device. + + start-year: true + +required: + - compatible + - reg + +additionalProperties: false + +...