1. Patchew
  2. linux
  3. [PATCH v8 0/6] Raspberry Pi Sense HAT driver
  4. View patch

[PATCH v8 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema

Charles Mirabile posted 6 patches 3 years, 8 months ago
There is a newer version of this series
[PATCH v8 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
Posted by Charles Mirabile 3 years, 8 months ago 
This patch adds the device tree bindings for the Sense HAT
and each of its children devices in yaml form.

Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
---
 .../raspberrypi,sensehat-display.yaml         | 27 ++++++++
 .../input/raspberrypi,sensehat-joystick.yaml  | 33 +++++++++
 .../bindings/mfd/raspberrypi,sensehat.yaml    | 69 +++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
 create mode 100644 Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml b/Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
new file mode 100644
index 000000000000..5e41d6b7817d
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/raspberrypi,sensehat-display.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raspberry Pi Sensehat Display
+
+maintainers:
+  - Charles Mirabile <cmirabil@redhat.com>
+  - Mwesigwa Guma <mguma@redhat.com>
+  - Joel Savitz <jsavitz@redhat.com>
+
+description:
+  This device is part of the sensehat multi function device.
+  For more information see ../mfd/raspberrypi,sensehat.yaml.
+
+  This device features a programmable 8x8 RGB LED matrix.
+
+properties:
+  compatible:
+    const: raspberrypi,sensehat-display
+
+required:
+  - compatible
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml b/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
new file mode 100644
index 000000000000..c97cd1d8eac6
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/raspberrypi,sensehat-joystick.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raspberry Pi Sensehat Joystick
+
+maintainers:
+  - Charles Mirabile <cmirabil@redhat.com>
+  - Mwesigwa Guma <mguma@redhat.com>
+  - Joel Savitz <jsavitz@redhat.com>
+
+description:
+  This device is part of the sensehat multi function device.
+  For more information see ../mfd/raspberrypi,sensehat.yaml.
+
+  This device features a five button joystick (up, down,left,
+  right, click)
+
+properties:
+  compatible:
+    const: raspberrypi,sensehat-joystick
+
+  interrupts:
+    items:
+      - description: pin number for joystick interrupt
+
+required:
+  - compatible
+  - interrupts
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
new file mode 100644
index 000000000000..2484ec91b430
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/raspberrypi,sensehat.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raspberry Pi Sensehat
+
+maintainers:
+  - Charles Mirabile <cmirabil@redhat.com>
+  - Mwesigwa Guma <mguma@redhat.com>
+  - Joel Savitz <jsavitz@redhat.com>
+
+description:
+  The Raspberry Pi Sensehat is an addon board originally developed
+  for the Raspberry Pi that has a joystick and an 8x8 RGB LED display
+  as well as several environmental sensors. It connects via i2c and
+  a gpio for irq.
+
+properties:
+  compatible:
+    const: raspberrypi,sensehat
+
+  reg:
+    items:
+      - description: i2c device address
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "joystick":
+    $ref: ../input/raspberrypi,sensehat-joystick.yaml
+
+  "display":
+    $ref: ../auxdisplay/raspberrypi,sensehat-display.yaml
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - joystick
+  - display
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      sensehat@46 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "raspberrypi,sensehat";
+        reg = <0x46>;
+        display {
+          compatible = "raspberrypi,sensehat-display";
+        };
+        joystick {
+          compatible = "raspberrypi,sensehat-joystick";
+          interrupts = <23 GPIO_ACTIVE_HIGH>;
+        };
+      };
+    };
-- 
2.31.1
Re: [PATCH v8 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
Posted by Krzysztof Kozlowski 3 years, 8 months ago 
On 12/04/2022 22:13, Charles Mirabile wrote:
> This patch adds the device tree bindings for the Sense HAT
> and each of its children devices in yaml form.
>

Thank you for your patch. There is something to discuss/improve.

> +description:
> +  This device is part of the sensehat multi function device.
> +  For more information see ../mfd/raspberrypi,sensehat.yaml.
> +
> +  This device features a programmable 8x8 RGB LED matrix.
> +
> +properties:
> +  compatible:
> +    const: raspberrypi,sensehat-display

This binding is practically empty, so I wonder what's is purpose? Do you
plan to grow it? If this was already explained, sorry for bringing it up
again... :)

> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml b/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
> new file mode 100644
> index 000000000000..c97cd1d8eac6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/raspberrypi,sensehat-joystick.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Raspberry Pi Sensehat Joystick
> +
> +maintainers:
> +  - Charles Mirabile <cmirabil@redhat.com>
> +  - Mwesigwa Guma <mguma@redhat.com>
> +  - Joel Savitz <jsavitz@redhat.com>
> +
> +description:
> +  This device is part of the sensehat multi function device.
> +  For more information see ../mfd/raspberrypi,sensehat.yaml.
> +
> +  This device features a five button joystick (up, down,left,
> +  right, click)
> +
> +properties:
> +  compatible:
> +    const: raspberrypi,sensehat-joystick
> +
> +  interrupts:
> +    items:
> +      - description: pin number for joystick interrupt

Description is obvious, so just "maxItems: 1"

> +
> +required:
> +  - compatible
> +  - interrupts
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> new file mode 100644
> index 000000000000..2484ec91b430
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/raspberrypi,sensehat.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Raspberry Pi Sensehat
> +
> +maintainers:
> +  - Charles Mirabile <cmirabil@redhat.com>
> +  - Mwesigwa Guma <mguma@redhat.com>
> +  - Joel Savitz <jsavitz@redhat.com>
> +
> +description:
> +  The Raspberry Pi Sensehat is an addon board originally developed
> +  for the Raspberry Pi that has a joystick and an 8x8 RGB LED display
> +  as well as several environmental sensors. It connects via i2c and
> +  a gpio for irq.
> +
> +properties:
> +  compatible:
> +    const: raspberrypi,sensehat
> +
> +  reg:
> +    items:
> +      - description: i2c device address

Description is obvious, so just "maxItems: 1"

> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

Why do you need these? You are not allowing any children with unit
addresses.

> +
> +  "joystick":
> +    $ref: ../input/raspberrypi,sensehat-joystick.yaml

Full path, so "/schemas/input/raspberrypi,sensehat-joystick.yaml#"

> +
> +  "display":
> +    $ref: ../auxdisplay/raspberrypi,sensehat-display.yaml

The same.

> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - joystick
> +  - display
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      sensehat@46 {

Generic node names please, so "hat".

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "raspberrypi,sensehat";
> +        reg = <0x46>;

Could you put compatible and then reg at beginning of properties (before
address/size)? It is more common convention and these are more important
properties.

> +        display {
> +          compatible = "raspberrypi,sensehat-display";
> +        };
> +        joystick {
> +          compatible = "raspberrypi,sensehat-joystick";
> +          interrupts = <23 GPIO_ACTIVE_HIGH>;

This does not look like proper interrupt flag. Wrong constant.

> +        };
> +      };
> +    };


Best regards,
Krzysztof
Re: [PATCH v8 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
Posted by Krzysztof Kozlowski 3 years, 8 months ago 
On 13/04/2022 15:38, Krzysztof Kozlowski wrote:
> On 12/04/2022 22:13, Charles Mirabile wrote:
>> This patch adds the device tree bindings for the Sense HAT
>> and each of its children devices in yaml form.
>>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> +description:
>> +  This device is part of the sensehat multi function device.
>> +  For more information see ../mfd/raspberrypi,sensehat.yaml.
>> +
>> +  This device features a programmable 8x8 RGB LED matrix.
>> +
>> +properties:
>> +  compatible:
>> +    const: raspberrypi,sensehat-display
> 
> This binding is practically empty, so I wonder what's is purpose? Do you
> plan to grow it? If this was already explained, sorry for bringing it up
> again... :)
> 

BTW, I see now you got review from Rob. What changed here that you did
not include it? It is expected to document such case in patch changelog,
so the reviewer will know he wasn't ignored. :)

Best regards,
Krzysztof

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.