[RFC PATCH v2 0/4] Raspberry Pi Sense HAT driver

[RFC PATCH v2 0/4] Raspberry Pi Sense HAT driver

[Charles Mirabile]

This patch series adds a set of drivers for operating the Sense HAT
peripheral device. This board is an add on for the Raspberry Pi that is
designed to connect using the GPIO connector via I2C.

It features:
	- a joystick
	- an 8x8 RGB LED matrix display
	- a whole bunch of environmental sensors with their own drivers
	(those are already in upstream Linux)

This is a refactor of the work of Serge Schneider, the author of a
version of this driver that is currently in the Raspberry Pi downstream
kernel. We modified his code to make it suitable for upstream Linux.

A couple of tests are available for the driver in this repo:
https://github.com/underground-software/sensehat/tree/master/tests
	- color_test displays various solid colors on the LED panel
	- framebuffer_test diplays a single lit cell that is
	  controllable via the arrow keys or the joystick

Known issue:
	- We are not sure how to integrate the device tree overlay for the
	Sense HAT into mainline Linux, or if we even should at this time.

Fortunately, most Linux distrubtions that can run on the Raspberry Pi
already include an appropriate device tree overlay since they often import
all of the overlays from the downstream Raspberry Pi kernel.

We have included a hypothetical overlay that would work for the sensehat
as a fourth patch in this series but we just put it in the root of the
source tree for the time being so that patch should not be merged unless a
consensus is reached that the overlay should be in the tree, and a proper
location is found to put it.

Suggestions are welcome and appreciated.

For more information about the Sense HAT, visit:
https://www.raspberrypi.org/products/sense-hat/

Changes since v1:
	- We included a device tree overlay for the Sense HAT
	- Based on Feedback from v1 we:
		- Changed the style of a few lines
		- Adding more error logging
		- Changed the compatible strings to use the more common
		"raspberrypi" instead of just "rpi"
		- Renamed everything from rpisense to sensehat

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>

Charles Mirabile (4):
  Add core driver and config
  Add joystick driver and config
  Add display driver and config
  Add sensehat device tree overlay

 arch/arm/boot/dts/sensehat.dtbs            |  51 +++++
 drivers/auxdisplay/Kconfig                 |   7 +
 drivers/auxdisplay/Makefile                |   1 +
 drivers/auxdisplay/sensehat-display.c      | 234 +++++++++++++++++++++
 drivers/input/joystick/Kconfig             |   8 +
 drivers/input/joystick/Makefile            |   1 +
 drivers/input/joystick/sensehat-joystick.c | 124 +++++++++++
 drivers/mfd/Kconfig                        |   8 +
 drivers/mfd/Makefile                       |   1 +
 drivers/mfd/sensehat-core.c                | 168 +++++++++++++++
 include/linux/mfd/sensehat.h               |  55 +++++
 11 files changed, 658 insertions(+)
 create mode 100644 arch/arm/boot/dts/sensehat.dtbs
 create mode 100644 drivers/auxdisplay/sensehat-display.c
 create mode 100644 drivers/input/joystick/sensehat-joystick.c
 create mode 100644 drivers/mfd/sensehat-core.c
 create mode 100644 include/linux/mfd/sensehat.h

-- 
2.27.0

[RFC PATCH v2 4/4] sensehat: Add device tree overlay (do not merge)

[Charles Mirabile]

This patch is an RFC. The overlay included here is suitable for the
Sense HAT but we are not sure where overlays like this should go, or if we
should even include it, because the Raspberry Pi kernel tree already
includes that device tree blob that this was based on and most
distributions for the Raspberry Pi include those blobs.

The overlay is just in the root of the source tree since we did not know
where to put it so this patch should not be merged in this state.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 sensehat.dtbs | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 sensehat.dtbs

diff --git a/sensehat.dtbs b/sensehat.dtbs
new file mode 100644
index 000000000000..3f1c12c8f75f
--- /dev/null
+++ b/sensehat.dtbs
@@ -0,0 +1,51 @@
+/dts-v1/;
+
+/ {
+	compatible = "brcm,bcm2835";
+
+	fragment@0 {
+		target = <0xffffffff>;
+
+		__overlay__ {
+			#address-cells = <0x01>;
+			#size-cells = <0x00>;
+			status = "okay";
+
+			sensehat@46 {
+				compatible = "raspberrypi,sensehat";
+				reg = <0x46>;
+				keys-int-gpios = <0xffffffff 0x17 0x01>;
+				status = "okay";
+			};
+
+			lsm9ds1-magn@1c {
+				compatible = "st,lsm9ds1-magn";
+				reg = <0x1c>;
+				status = "okay";
+			};
+
+			lsm9ds1-accel6a {
+				compatible = "st,lsm9ds1-accel";
+				reg = <0x6a>;
+				status = "okay";
+			};
+
+			lps25h-press@5c {
+				compatible = "st,lps25h-press";
+				reg = <0x5c>;
+				status = "okay";
+			};
+
+			hts221-humid@5f {
+				compatible = "st,hts221-humid\0st,hts221";
+				reg = <0x5f>;
+				status = "okay";
+			};
+		};
+	};
+
+	__fixups__ {
+		i2c1 = "/fragment@0:target:0";
+		gpio = "/fragment@0/__overlay__/sensehat@46:keys-int-gpios:0";
+	};
+};
-- 
2.27.0

Re: [RFC PATCH v2 4/4] sensehat: Add device tree overlay (do not merge)

[nsaenzju]

Hi Charles,

On Fri, 2021-08-20 at 14:08 -0400, Charles Mirabile wrote:
> This patch is an RFC. The overlay included here is suitable for the
> Sense HAT but we are not sure where overlays like this should go, or if we
> should even include it, because the Raspberry Pi kernel tree already
> includes that device tree blob that this was based on and most
> distributions for the Raspberry Pi include those blobs.
> 
> The overlay is just in the root of the source tree since we did not know
> where to put it so this patch should not be merged in this state.
> 
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---

Once we get this merged, and since upstream doesn't yet mantain overlays, I
suggest you merge this overlay in the downstream rpi kernel repo, who actively
maintain them. AFAIK a number of distros use that repo as the base for their
devicetrees.

That said, I think providing a devicetree binding for this driver is a must for
it to be merged (in yaml form).

Regards,
Nicolas

Re: [RFC PATCH v2 4/4] sensehat: Add device tree overlay (do not merge)

[Matthias Brugger]

On 20/08/2021 20:08, Charles Mirabile wrote:
> This patch is an RFC. The overlay included here is suitable for the
> Sense HAT but we are not sure where overlays like this should go, or if we
> should even include it, because the Raspberry Pi kernel tree already
> includes that device tree blob that this was based on and most
> distributions for the Raspberry Pi include those blobs.
> 
> The overlay is just in the root of the source tree since we did not know
> where to put it so this patch should not be merged in this state.
> 
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>   sensehat.dtbs | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
>   create mode 100644 sensehat.dtbs
> 
> diff --git a/sensehat.dtbs b/sensehat.dtbs
> new file mode 100644
> index 000000000000..3f1c12c8f75f
> --- /dev/null
> +++ b/sensehat.dtbs
> @@ -0,0 +1,51 @@
> +/dts-v1/;
> +
> +/ {
> +	compatible = "brcm,bcm2835";
> +
> +	fragment@0 {
> +		target = <0xffffffff>;
> +
> +		__overlay__ {
> +			#address-cells = <0x01>;
> +			#size-cells = <0x00>;
> +			status = "okay";
> +
> +			sensehat@46 {
> +				compatible = "raspberrypi,sensehat";
> +				reg = <0x46>;
> +				keys-int-gpios = <0xffffffff 0x17 0x01>;
> +				status = "okay";
> +			};
> +
> +			lsm9ds1-magn@1c {
> +				compatible = "st,lsm9ds1-magn";
> +				reg = <0x1c>;
> +				status = "okay";
> +			};
> +
> +			lsm9ds1-accel6a {
> +				compatible = "st,lsm9ds1-accel";
> +				reg = <0x6a>;
> +				status = "okay";
> +			};
> +
> +			lps25h-press@5c {
> +				compatible = "st,lps25h-press";
> +				reg = <0x5c>;
> +				status = "okay";
> +			};
> +
> +			hts221-humid@5f {
> +				compatible = "st,hts221-humid\0st,hts221";

Isn't "st,hts221" good enough?

> +				reg = <0x5f>;
> +				status = "okay";
> +			};
> +		};
> +	};
> +
> +	__fixups__ {
> +		i2c1 = "/fragment@0:target:0";
> +		gpio = "/fragment@0/__overlay__/sensehat@46:keys-int-gpios:0";
> +	};
> +};
> 

As Nicolas said, yaml bindings are a must. The overlay can be added as for 
convenience to the series but it should be made clear that it is not supposed to 
be upstreamed.

Regards,
Matthias