diff mbox series

[v4,1/2] dt-bindings: counter: add pulse-counter binding

Message ID 20210126131239.8335-2-o.rempel@pengutronix.de (mailing list archive)
State Superseded, archived
Headers show
Series add support for GPIO based counter | expand

Commit Message

Oleksij Rempel Jan. 26, 2021, 1:12 p.m. UTC
Add binding for the pulse counter node

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../bindings/counter/pulse-counter.yaml       | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/counter/pulse-counter.yaml

Comments

Linus Walleij Jan. 28, 2021, 8:17 a.m. UTC | #1
Hi Oleksij,

thanks for your patch!

On Tue, Jan 26, 2021 at 2:15 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Add binding for the pulse counter node
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
(...)

> +properties:
> +  compatible:
> +    const: virtual,pulse-counter

What is so virtual about this? The device seems very real.
However it is certainly a GPIO counter.

I would call it "gpio-counter" simply.

Define:
  $nodename:
     pattern: "^counter(@.*)?$"

> +    counter-0 {

counter@0 {

> +    counter-1 {

counter@1 {

Thanks!
Linus Walleij
Oleksij Rempel Jan. 28, 2021, 1:39 p.m. UTC | #2
On Thu, Jan 28, 2021 at 09:17:23AM +0100, Linus Walleij wrote:
> Hi Oleksij,
> 
> thanks for your patch!
> 
> On Tue, Jan 26, 2021 at 2:15 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > Add binding for the pulse counter node
> >
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> (...)
> 
> > +properties:
> > +  compatible:
> > +    const: virtual,pulse-counter
> 
> What is so virtual about this? The device seems very real.

Currently there are two ways:
1. use "virtual" or "linux" vendor. Same as "virtual,mdio-gpio"
2. Extend the list of "not vendor" prefixes in the prefixes list:
   Documentation/devicetree/bindings/vendor-prefixes.yaml

Since both ways seems to be valid, i personally prefer to use existing
prefix instead of maintaining the vendor-prefixes.yaml

@Rob, what do you prefer?

> However it is certainly a GPIO counter.

This was my first implementation. @Jonathan you suggest to use GPIO-free
way, can you and Linus please decide what is the way to go.

I personally can imagine that this driver can be attached to any IRQ
source, including drivers/iio/trigger/iio-trig-sysfs.c

> I would call it "gpio-counter" simply.
> 
> Define:
>   $nodename:
>      pattern: "^counter(@.*)?$"
> 
> > +    counter-0 {
> 
> counter@0 {
> 
> > +    counter-1 {
> 
> counter@1 {

In this case the dtc compiler will say:
/counter@0: node has a unit name, but no reg property

Regards,
Oleksij
Rob Herring Feb. 5, 2021, 11:34 p.m. UTC | #3
On Thu, Jan 28, 2021 at 02:39:22PM +0100, Oleksij Rempel wrote:
> On Thu, Jan 28, 2021 at 09:17:23AM +0100, Linus Walleij wrote:
> > Hi Oleksij,
> > 
> > thanks for your patch!
> > 
> > On Tue, Jan 26, 2021 at 2:15 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > 
> > > Add binding for the pulse counter node
> > >
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > (...)
> > 
> > > +properties:
> > > +  compatible:
> > > +    const: virtual,pulse-counter
> > 
> > What is so virtual about this? The device seems very real.
> 
> Currently there are two ways:
> 1. use "virtual" or "linux" vendor. Same as "virtual,mdio-gpio"

virtual is used by exactly one case. linux for a few more, mostly 
linux,spdif-dit and extcon (deprecated).

> 2. Extend the list of "not vendor" prefixes in the prefixes list:
>    Documentation/devicetree/bindings/vendor-prefixes.yaml

Pretty sure that says 'DON'T ADD MORE'. Maybe I forgot to scream it.

> 
> Since both ways seems to be valid, i personally prefer to use existing
> prefix instead of maintaining the vendor-prefixes.yaml
> 
> @Rob, what do you prefer?

For vendorless bindings, no vendor prefix! 'gpio-counter' if only gpio 
interfaced. No idea what other options would be.

> 
> > However it is certainly a GPIO counter.
> 
> This was my first implementation. @Jonathan you suggest to use GPIO-free
> way, can you and Linus please decide what is the way to go.
> 
> I personally can imagine that this driver can be attached to any IRQ
> source, including drivers/iio/trigger/iio-trig-sysfs.c
> 
> > I would call it "gpio-counter" simply.
> > 
> > Define:
> >   $nodename:
> >      pattern: "^counter(@.*)?$"
> > 
> > > +    counter-0 {
> > 
> > counter@0 {
> > 
> > > +    counter-1 {
> > 
> > counter@1 {
> 
> In this case the dtc compiler will say:
> /counter@0: node has a unit name, but no reg property

counter-0 then.

> 
> Regards,
> Oleksij
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/counter/pulse-counter.yaml b/Documentation/devicetree/bindings/counter/pulse-counter.yaml
new file mode 100644
index 000000000000..8a82091edd65
--- /dev/null
+++ b/Documentation/devicetree/bindings/counter/pulse-counter.yaml
@@ -0,0 +1,52 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/counter/pulse-counter.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Pulse counter
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+  A generic pulse counter to measure pulse frequency. It was developed and used
+  for agricultural devices to measure rotation speed of wheels or other tools.
+  Since the direction of rotation is not important, only one signal line is
+  needed.
+
+properties:
+  compatible:
+    const: virtual,pulse-counter
+
+  interrupts:
+    maxItems: 1
+
+  gpios:
+    description: Optional diagnostic interface to measure signal level
+    maxItems: 1
+
+required:
+  - compatible
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    counter-0 {
+        compatible = "virtual,pulse-counter";
+        interrupts-extended = <&gpio 0 IRQ_TYPE_EDGE_RISING>;
+    };
+
+    counter-1 {
+        compatible = "virtual,pulse-counter";
+        interrupts-extended = <&gpio 1 IRQ_TYPE_EDGE_RISING>;
+        gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+    };
+
+...