diff mbox series

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

Message ID 20210126131239.8335-2-o.rempel@pengutronix.de
State New
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
-- 
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 |
Rob Herring (Arm) 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>;
+    };
+
+...