[PATCH v10 0/6] Raspberry Pi Sense HAT driver

[PATCH v10 0/6] 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 and communicate 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 the test folder in
this repo: https://github.com/underground-software/sensehat.git
	- sensehat_joystick_test logs the input events from the
	  joystick to the console
	- sensehat_display_test displays various solid colors on
	  the LED panel.
	- full_sensehat_test displays a single lit cell that can be
	  moved with the joystick. Pressing the joystick ends the
	  program.

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

Changes since v9:
	- The driver for the sensehat joystick was accepted upstream so
	  it is no longer included in this patchset.
	- Some of the people involved in the development of earlier
	  versions of these patches are no longer involved in the project
	  so they have been removed from the list of maintainers and the
	  patches.
	- The code is completely unchanged and so based on the discussions
	  from version 9 of the patches, it should be ready to be merged.

My appologies for the long delay between v9 and v10 especially given the
fact that there are no changes to the code. We appreciate the maintainers'
patience and guidance throughout all 10 versions of this sensehat patchset,
and we are excited for this driver to hopefully be fully accepted.

Best - Charlie

Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
Signed-off-by: Joel Slebodnick <jslebodn@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>

Charles Mirabile (5):
  drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
  drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
  dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
  MAINTAINERS: Add sensehat driver authors to MAINTAINERS
  DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

 .../raspberrypi,sensehat-display.yaml         |  26 +++
 .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
 .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
 MAINTAINERS                                   |  10 +
 drivers/auxdisplay/Kconfig                    |   8 +
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
 drivers/mfd/simple-mfd-i2c.c                  |   1 +
 sensehat.dtbs                                 |  52 +++++
 9 files changed, 391 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
 create mode 100644 drivers/auxdisplay/sensehat-display.c
 create mode 100644 sensehat.dtbs

-- 
2.31.1

[PATCH 1/5] drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c

[Charles Mirabile]

This patch adds the compatible string for the Sense HAT device to
the list of compatible strings in the simple_mfd_i2c driver so that
it can match against the device and load its children and their drivers

Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
---
 drivers/mfd/simple-mfd-i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index f4c8fc3ee463..c47f36581f3f 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -75,6 +75,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
 static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "kontron,sl28cpld" },
 	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
+	{ .compatible = "raspberrypi,sensehat" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
-- 
2.31.1

Re: [PATCH 1/5] drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c

[Lee Jones]

On Tue, 23 Aug 2022, Charles Mirabile wrote:

> This patch adds the compatible string for the Sense HAT device to
> the list of compatible strings in the simple_mfd_i2c driver so that
> it can match against the device and load its children and their drivers
> 
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> ---
>  drivers/mfd/simple-mfd-i2c.c | 1 +
>  1 file changed, 1 insertion(+)

If you have not already done so, please resend, Cc'ing my kernel.org
address, as-per MAINTAINERS.  Thank you.

> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> index f4c8fc3ee463..c47f36581f3f 100644
> --- a/drivers/mfd/simple-mfd-i2c.c
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -75,6 +75,7 @@ static const struct simple_mfd_data silergy_sy7636a = {
>  static const struct of_device_id simple_mfd_i2c_of_match[] = {
>  	{ .compatible = "kontron,sl28cpld" },
>  	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
> +	{ .compatible = "raspberrypi,sensehat" },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

-- 
DEPRECATED: Please use lee@kernel.org

Re: [PATCH 1/5] drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On Tue, Aug 23, 2022 at 01:41:54PM -0400, Charles Mirabile wrote:
> This patch adds the compatible string for the Sense HAT device to
> the list of compatible strings in the simple_mfd_i2c driver so that
> it can match against the device and load its children and their drivers
> 

Better say "Add compatibe string for Sense HAT device. The device is
compatible with silergy,sy7636a."

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/5] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver

[Charles Mirabile]

This patch adds the driver for the 8x8 RGB LED matrix display
on the Sense HAT. It appears as a character device named sense-hat
in /dev/. That special file is 192 bytes large and contains 64
RGB triplets (3 bytes each) one for each pixel in row major order.

Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
---
 drivers/auxdisplay/Kconfig            |   8 +
 drivers/auxdisplay/Makefile           |   1 +
 drivers/auxdisplay/sensehat-display.c | 205 ++++++++++++++++++++++++++
 3 files changed, 214 insertions(+)
 create mode 100644 drivers/auxdisplay/sensehat-display.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 64012cda4d12..9bad1aade7a0 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -203,6 +203,14 @@ config ARM_CHARLCD
 	  line and the Linux version on the second line, but that's
 	  still useful.
 
+config SENSEHAT_DISPLAY
+	tristate "Raspberry Pi Sense HAT display driver"
+	depends on I2C
+	select MFD_SIMPLE_MFD_I2C
+	help
+	 This is a driver for the Raspberry Pi Sensehat 8x8 RBG-LED matrix
+	 you can access it as a misc device at /dev/sense-hat
+
 menuconfig PARPORT_PANEL
 	tristate "Parallel port LCD/Keypad Panel support"
 	depends on PARPORT
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index 6968ed4d3f0a..30b2b7934046 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33)		+= ht16k33.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.o
 obj-$(CONFIG_LINEDISP)		+= line-display.o
+obj-$(CONFIG_SENSEHAT_DISPLAY)	+= sensehat-display.o
diff --git a/drivers/auxdisplay/sensehat-display.c b/drivers/auxdisplay/sensehat-display.c
new file mode 100644
index 000000000000..08c0c4338f3e
--- /dev/null
+++ b/drivers/auxdisplay/sensehat-display.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Raspberry Pi Sense HAT 8x8 LED matrix display driver
+ * http://raspberrypi.org
+ *
+ * Copyright (C) 2015 Raspberry Pi
+ * Copyright (C) 2021 Charles Mirabile, Joel Savitz
+ *
+ * Original Author: Serge Schneider
+ * Revised for upstream Linux by: Charles Mirabile, Joel Savitz
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/miscdevice.h>
+#include <linux/regmap.h>
+#include <linux/property.h>
+
+#define DISPLAY_SMB_REG 0x00
+#define RGB_555_MASK 0x1f
+#define NUM_LEDS 8
+#define NUM_CHANNELS 3
+
+struct sensehat_display {
+	struct platform_device *pdev;
+	struct miscdevice mdev;
+	struct mutex rw_mtx;
+	u8 vmem[NUM_LEDS][NUM_LEDS][NUM_CHANNELS];
+	struct regmap *regmap;
+};
+
+#define VMEM_SIZE sizeof_field(struct sensehat_display, vmem)
+
+
+static int sensehat_update_display(struct sensehat_display *display)
+{
+	int i, j, k, ret;
+	u8 buff[NUM_LEDS][NUM_CHANNELS][NUM_LEDS];
+
+	for (i = 0; i < NUM_LEDS; ++i)
+		for (j = 0; j < NUM_LEDS; ++j)
+			for (k = 0; k < NUM_CHANNELS; ++k)
+				buff[i][k][j] =
+					display->vmem[i][j][k] & RGB_555_MASK;
+
+	ret = regmap_bulk_write(display->regmap, DISPLAY_SMB_REG, buff,
+				VMEM_SIZE);
+	if (ret < 0)
+		dev_err(&display->pdev->dev,
+			"Update to 8x8 LED matrix display failed");
+	return ret;
+}
+
+static loff_t sensehat_display_llseek(struct file *filp, loff_t offset,
+				      int whence)
+{
+	return fixed_size_llseek(filp, offset, whence, VMEM_SIZE);
+}
+
+static ssize_t sensehat_display_read(struct file *filp, char __user *buf,
+				     size_t count, loff_t *f_pos)
+{
+	struct sensehat_display *sensehat_display =
+		container_of(filp->private_data, struct sensehat_display, mdev);
+	ssize_t ret = -EFAULT;
+
+	if (*f_pos < 0 || *f_pos >= VMEM_SIZE)
+		return 0;
+	count = min_t(size_t, count, VMEM_SIZE - *f_pos);
+
+	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
+		return -ERESTARTSYS;
+	if (copy_to_user(buf, *f_pos + (u8 *)sensehat_display->vmem, count))
+		goto out;
+	*f_pos += count;
+	ret = count;
+out:
+	mutex_unlock(&sensehat_display->rw_mtx);
+	return ret;
+}
+
+static ssize_t sensehat_display_write(struct file *filp, const char __user *buf,
+				      size_t count, loff_t *f_pos)
+{
+	struct sensehat_display *sensehat_display =
+		container_of(filp->private_data, struct sensehat_display, mdev);
+	int ret = -EFAULT;
+
+	if (*f_pos < 0 || *f_pos >= VMEM_SIZE)
+		return -EFBIG;
+	count = min_t(size_t, count, VMEM_SIZE - *f_pos);
+
+	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
+		return -ERESTARTSYS;
+	if (copy_from_user(*f_pos + (u8 *)sensehat_display->vmem, buf, count))
+		goto out;
+	ret = sensehat_update_display(sensehat_display);
+	if (ret < 0) {
+		ret = -EIO;
+		goto out;
+	}
+	*f_pos += count;
+	ret = count;
+out:
+	mutex_unlock(&sensehat_display->rw_mtx);
+	return ret;
+}
+
+static const struct file_operations sensehat_display_fops = {
+	.owner = THIS_MODULE,
+	.llseek = sensehat_display_llseek,
+	.read = sensehat_display_read,
+	.write = sensehat_display_write,
+};
+
+static int sensehat_display_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	struct sensehat_display *sensehat_display =
+		devm_kmalloc(&pdev->dev, sizeof(*sensehat_display), GFP_KERNEL);
+	if (!sensehat_display)
+		return -ENOMEM;
+
+	sensehat_display->pdev = pdev;
+
+	dev_set_drvdata(&pdev->dev, sensehat_display);
+
+	sensehat_display->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!sensehat_display->regmap) {
+		dev_err(&pdev->dev,
+			"unable to get sensehat regmap");
+		return -ENODEV;
+	}
+
+	memset(sensehat_display->vmem, 0, VMEM_SIZE);
+
+	mutex_init(&sensehat_display->rw_mtx);
+
+	ret = sensehat_update_display(sensehat_display);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Could not communicate with sensehat");
+		return ret;
+	}
+
+	sensehat_display->mdev = (struct miscdevice){
+		.minor = MISC_DYNAMIC_MINOR,
+		.name = "sense-hat",
+		.mode = 0666,
+		.fops = &sensehat_display_fops,
+	};
+
+	ret = misc_register(&sensehat_display->mdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Could not register 8x8 LED matrix display.");
+		return ret;
+	}
+
+	dev_info(&pdev->dev,
+		 "8x8 LED matrix display registered with minor number %i",
+		 sensehat_display->mdev.minor);
+
+	return 0;
+}
+
+static int sensehat_display_remove(struct platform_device *pdev)
+{
+	struct sensehat_display *sensehat_display = dev_get_drvdata(&pdev->dev);
+
+	misc_deregister(&sensehat_display->mdev);
+	return 0;
+}
+
+static const struct of_device_id sensehat_display_device_id[] = {
+	{ .compatible = "raspberrypi,sensehat-display" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sensehat_display_device_id);
+
+static struct platform_driver sensehat_display_driver = {
+	.probe = sensehat_display_probe,
+	.remove = sensehat_display_remove,
+	.driver = {
+		.name = "sensehat-display",
+		.of_match_table = sensehat_display_device_id,
+	},
+};
+
+module_platform_driver(sensehat_display_driver);
+
+MODULE_DESCRIPTION("Raspberry Pi Sense HAT 8x8 LED matrix display driver");
+MODULE_AUTHOR("Charles Mirabile <cmirabil@redhat.com>");
+MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
+MODULE_LICENSE("GPL");
-- 
2.31.1

Re: [PATCH 2/5] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver

[Matthias Brugger]

On 23/08/2022 19:41, Charles Mirabile wrote:
> This patch adds the driver for the 8x8 RGB LED matrix display
> on the Sense HAT. It appears as a character device named sense-hat
> in /dev/. That special file is 192 bytes large and contains 64
> RGB triplets (3 bytes each) one for each pixel in row major order.
> 
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>

As this is a 8x8 pixel display, I think it makes no sense to implement this as a 
tiny DRM driver, so:

Reviewed-by: Matthias Brugger <mbrugger@suse.com>

> ---
>   drivers/auxdisplay/Kconfig            |   8 +
>   drivers/auxdisplay/Makefile           |   1 +
>   drivers/auxdisplay/sensehat-display.c | 205 ++++++++++++++++++++++++++
>   3 files changed, 214 insertions(+)
>   create mode 100644 drivers/auxdisplay/sensehat-display.c
> 
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index 64012cda4d12..9bad1aade7a0 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -203,6 +203,14 @@ config ARM_CHARLCD
>   	  line and the Linux version on the second line, but that's
>   	  still useful.
>   
> +config SENSEHAT_DISPLAY
> +	tristate "Raspberry Pi Sense HAT display driver"
> +	depends on I2C
> +	select MFD_SIMPLE_MFD_I2C
> +	help
> +	 This is a driver for the Raspberry Pi Sensehat 8x8 RBG-LED matrix
> +	 you can access it as a misc device at /dev/sense-hat
> +
>   menuconfig PARPORT_PANEL
>   	tristate "Parallel port LCD/Keypad Panel support"
>   	depends on PARPORT
> diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
> index 6968ed4d3f0a..30b2b7934046 100644
> --- a/drivers/auxdisplay/Makefile
> +++ b/drivers/auxdisplay/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33)		+= ht16k33.o
>   obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
>   obj-$(CONFIG_LCD2S)		+= lcd2s.o
>   obj-$(CONFIG_LINEDISP)		+= line-display.o
> +obj-$(CONFIG_SENSEHAT_DISPLAY)	+= sensehat-display.o
> diff --git a/drivers/auxdisplay/sensehat-display.c b/drivers/auxdisplay/sensehat-display.c
> new file mode 100644
> index 000000000000..08c0c4338f3e
> --- /dev/null
> +++ b/drivers/auxdisplay/sensehat-display.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Raspberry Pi Sense HAT 8x8 LED matrix display driver
> + * http://raspberrypi.org
> + *
> + * Copyright (C) 2015 Raspberry Pi
> + * Copyright (C) 2021 Charles Mirabile, Joel Savitz
> + *
> + * Original Author: Serge Schneider
> + * Revised for upstream Linux by: Charles Mirabile, Joel Savitz
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/miscdevice.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#define DISPLAY_SMB_REG 0x00
> +#define RGB_555_MASK 0x1f
> +#define NUM_LEDS 8
> +#define NUM_CHANNELS 3
> +
> +struct sensehat_display {
> +	struct platform_device *pdev;
> +	struct miscdevice mdev;
> +	struct mutex rw_mtx;
> +	u8 vmem[NUM_LEDS][NUM_LEDS][NUM_CHANNELS];
> +	struct regmap *regmap;
> +};
> +
> +#define VMEM_SIZE sizeof_field(struct sensehat_display, vmem)
> +
> +
> +static int sensehat_update_display(struct sensehat_display *display)
> +{
> +	int i, j, k, ret;
> +	u8 buff[NUM_LEDS][NUM_CHANNELS][NUM_LEDS];
> +
> +	for (i = 0; i < NUM_LEDS; ++i)
> +		for (j = 0; j < NUM_LEDS; ++j)
> +			for (k = 0; k < NUM_CHANNELS; ++k)
> +				buff[i][k][j] =
> +					display->vmem[i][j][k] & RGB_555_MASK;
> +
> +	ret = regmap_bulk_write(display->regmap, DISPLAY_SMB_REG, buff,
> +				VMEM_SIZE);
> +	if (ret < 0)
> +		dev_err(&display->pdev->dev,
> +			"Update to 8x8 LED matrix display failed");
> +	return ret;
> +}
> +
> +static loff_t sensehat_display_llseek(struct file *filp, loff_t offset,
> +				      int whence)
> +{
> +	return fixed_size_llseek(filp, offset, whence, VMEM_SIZE);
> +}
> +
> +static ssize_t sensehat_display_read(struct file *filp, char __user *buf,
> +				     size_t count, loff_t *f_pos)
> +{
> +	struct sensehat_display *sensehat_display =
> +		container_of(filp->private_data, struct sensehat_display, mdev);
> +	ssize_t ret = -EFAULT;
> +
> +	if (*f_pos < 0 || *f_pos >= VMEM_SIZE)
> +		return 0;
> +	count = min_t(size_t, count, VMEM_SIZE - *f_pos);
> +
> +	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
> +		return -ERESTARTSYS;
> +	if (copy_to_user(buf, *f_pos + (u8 *)sensehat_display->vmem, count))
> +		goto out;
> +	*f_pos += count;
> +	ret = count;
> +out:
> +	mutex_unlock(&sensehat_display->rw_mtx);
> +	return ret;
> +}
> +
> +static ssize_t sensehat_display_write(struct file *filp, const char __user *buf,
> +				      size_t count, loff_t *f_pos)
> +{
> +	struct sensehat_display *sensehat_display =
> +		container_of(filp->private_data, struct sensehat_display, mdev);
> +	int ret = -EFAULT;
> +
> +	if (*f_pos < 0 || *f_pos >= VMEM_SIZE)
> +		return -EFBIG;
> +	count = min_t(size_t, count, VMEM_SIZE - *f_pos);
> +
> +	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
> +		return -ERESTARTSYS;
> +	if (copy_from_user(*f_pos + (u8 *)sensehat_display->vmem, buf, count))
> +		goto out;
> +	ret = sensehat_update_display(sensehat_display);
> +	if (ret < 0) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +	*f_pos += count;
> +	ret = count;
> +out:
> +	mutex_unlock(&sensehat_display->rw_mtx);
> +	return ret;
> +}
> +
> +static const struct file_operations sensehat_display_fops = {
> +	.owner = THIS_MODULE,
> +	.llseek = sensehat_display_llseek,
> +	.read = sensehat_display_read,
> +	.write = sensehat_display_write,
> +};
> +
> +static int sensehat_display_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	struct sensehat_display *sensehat_display =
> +		devm_kmalloc(&pdev->dev, sizeof(*sensehat_display), GFP_KERNEL);
> +	if (!sensehat_display)
> +		return -ENOMEM;
> +
> +	sensehat_display->pdev = pdev;
> +
> +	dev_set_drvdata(&pdev->dev, sensehat_display);
> +
> +	sensehat_display->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!sensehat_display->regmap) {
> +		dev_err(&pdev->dev,
> +			"unable to get sensehat regmap");
> +		return -ENODEV;
> +	}
> +
> +	memset(sensehat_display->vmem, 0, VMEM_SIZE);
> +
> +	mutex_init(&sensehat_display->rw_mtx);
> +
> +	ret = sensehat_update_display(sensehat_display);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not communicate with sensehat");
> +		return ret;
> +	}
> +
> +	sensehat_display->mdev = (struct miscdevice){
> +		.minor = MISC_DYNAMIC_MINOR,
> +		.name = "sense-hat",
> +		.mode = 0666,
> +		.fops = &sensehat_display_fops,
> +	};
> +
> +	ret = misc_register(&sensehat_display->mdev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not register 8x8 LED matrix display.");
> +		return ret;
> +	}
> +
> +	dev_info(&pdev->dev,
> +		 "8x8 LED matrix display registered with minor number %i",
> +		 sensehat_display->mdev.minor);
> +
> +	return 0;
> +}
> +
> +static int sensehat_display_remove(struct platform_device *pdev)
> +{
> +	struct sensehat_display *sensehat_display = dev_get_drvdata(&pdev->dev);
> +
> +	misc_deregister(&sensehat_display->mdev);
> +	return 0;
> +}
> +
> +static const struct of_device_id sensehat_display_device_id[] = {
> +	{ .compatible = "raspberrypi,sensehat-display" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sensehat_display_device_id);
> +
> +static struct platform_driver sensehat_display_driver = {
> +	.probe = sensehat_display_probe,
> +	.remove = sensehat_display_remove,
> +	.driver = {
> +		.name = "sensehat-display",
> +		.of_match_table = sensehat_display_device_id,
> +	},
> +};
> +
> +module_platform_driver(sensehat_display_driver);
> +
> +MODULE_DESCRIPTION("Raspberry Pi Sense HAT 8x8 LED matrix display driver");
> +MODULE_AUTHOR("Charles Mirabile <cmirabil@redhat.com>");
> +MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
> +MODULE_LICENSE("GPL");

Re: [PATCH 2/5] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On Tue, Aug 23, 2022 at 01:41:55PM -0400, Charles Mirabile wrote:
> This patch adds the driver for the 8x8 RGB LED matrix display
> on the Sense HAT. It appears as a character device named sense-hat
> in /dev/. That special file is 192 bytes large and contains 64
> RGB triplets (3 bytes each) one for each pixel in row major order.
> 

Better say "Add driver for 8x8 RGB LED matrix display of Sense HAT. The
character device exposed is /dev/sense-hat, which is 192 bytes large and
contains 64 RGB triplets (3 byte each); one for each pixel in row major
order."

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/5] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver

[Wander Lairson Costa]

On Tue, Aug 23, 2022 at 01:41:55PM -0400, Charles Mirabile wrote:
> This patch adds the driver for the 8x8 RGB LED matrix display
> on the Sense HAT. It appears as a character device named sense-hat
> in /dev/. That special file is 192 bytes large and contains 64
> RGB triplets (3 bytes each) one for each pixel in row major order.
> 
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> ---
>  drivers/auxdisplay/Kconfig            |   8 +
>  drivers/auxdisplay/Makefile           |   1 +
>  drivers/auxdisplay/sensehat-display.c | 205 ++++++++++++++++++++++++++
>  3 files changed, 214 insertions(+)
>  create mode 100644 drivers/auxdisplay/sensehat-display.c
> 
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index 64012cda4d12..9bad1aade7a0 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -203,6 +203,14 @@ config ARM_CHARLCD
>  	  line and the Linux version on the second line, but that's
>  	  still useful.
>  
> +config SENSEHAT_DISPLAY
> +	tristate "Raspberry Pi Sense HAT display driver"
> +	depends on I2C
> +	select MFD_SIMPLE_MFD_I2C
> +	help
> +	 This is a driver for the Raspberry Pi Sensehat 8x8 RBG-LED matrix
> +	 you can access it as a misc device at /dev/sense-hat
> +
>  menuconfig PARPORT_PANEL
>  	tristate "Parallel port LCD/Keypad Panel support"
>  	depends on PARPORT
> diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
> index 6968ed4d3f0a..30b2b7934046 100644
> --- a/drivers/auxdisplay/Makefile
> +++ b/drivers/auxdisplay/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33)		+= ht16k33.o
>  obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
>  obj-$(CONFIG_LCD2S)		+= lcd2s.o
>  obj-$(CONFIG_LINEDISP)		+= line-display.o
> +obj-$(CONFIG_SENSEHAT_DISPLAY)	+= sensehat-display.o
> diff --git a/drivers/auxdisplay/sensehat-display.c b/drivers/auxdisplay/sensehat-display.c
> new file mode 100644
> index 000000000000..08c0c4338f3e
> --- /dev/null
> +++ b/drivers/auxdisplay/sensehat-display.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Raspberry Pi Sense HAT 8x8 LED matrix display driver
> + * http://raspberrypi.org
> + *
> + * Copyright (C) 2015 Raspberry Pi
> + * Copyright (C) 2021 Charles Mirabile, Joel Savitz
> + *
> + * Original Author: Serge Schneider
> + * Revised for upstream Linux by: Charles Mirabile, Joel Savitz
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/miscdevice.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#define DISPLAY_SMB_REG 0x00
> +#define RGB_555_MASK 0x1f
> +#define NUM_LEDS 8
> +#define NUM_CHANNELS 3
> +
> +struct sensehat_display {
> +	struct platform_device *pdev;
> +	struct miscdevice mdev;
> +	struct mutex rw_mtx;

Calling this rw_mtx is misleading, since it is not a rw_mutex. We should
rename it to mtx.

> +	u8 vmem[NUM_LEDS][NUM_LEDS][NUM_CHANNELS];
> +	struct regmap *regmap;
> +};
> +
> +#define VMEM_SIZE sizeof_field(struct sensehat_display, vmem)
> +
> +
> +static int sensehat_update_display(struct sensehat_display *display)
> +{
> +	int i, j, k, ret;
> +	u8 buff[NUM_LEDS][NUM_CHANNELS][NUM_LEDS];
> +
> +	for (i = 0; i < NUM_LEDS; ++i)
> +		for (j = 0; j < NUM_LEDS; ++j)
> +			for (k = 0; k < NUM_CHANNELS; ++k)
> +				buff[i][k][j] =
> +					display->vmem[i][j][k] & RGB_555_MASK;
> +
> +	ret = regmap_bulk_write(display->regmap, DISPLAY_SMB_REG, buff,
> +				VMEM_SIZE);
> +	if (ret < 0)
> +		dev_err(&display->pdev->dev,
> +			"Update to 8x8 LED matrix display failed");
> +	return ret;
> +}
> +
> +static loff_t sensehat_display_llseek(struct file *filp, loff_t offset,
> +				      int whence)
> +{
> +	return fixed_size_llseek(filp, offset, whence, VMEM_SIZE);
> +}
> +
> +static ssize_t sensehat_display_read(struct file *filp, char __user *buf,
> +				     size_t count, loff_t *f_pos)
> +{
> +	struct sensehat_display *sensehat_display =
> +		container_of(filp->private_data, struct sensehat_display, mdev);
> +	ssize_t ret = -EFAULT;
> +
> +	if (*f_pos < 0 || *f_pos >= VMEM_SIZE)
> +		return 0;

Maybe it should return -EINVAL?

> +	count = min_t(size_t, count, VMEM_SIZE - *f_pos);
> +
> +	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
> +		return -ERESTARTSYS;
> +	if (copy_to_user(buf, *f_pos + (u8 *)sensehat_display->vmem, count))
> +		goto out;
> +	*f_pos += count;
> +	ret = count;
> +out:
> +	mutex_unlock(&sensehat_display->rw_mtx);
> +	return ret;
> +}
> +
> +static ssize_t sensehat_display_write(struct file *filp, const char __user *buf,
> +				      size_t count, loff_t *f_pos)
> +{
> +	struct sensehat_display *sensehat_display =
> +		container_of(filp->private_data, struct sensehat_display, mdev);
> +	int ret = -EFAULT;
> +
> +	if (*f_pos < 0 || *f_pos >= VMEM_SIZE)
> +		return -EFBIG;
> +	count = min_t(size_t, count, VMEM_SIZE - *f_pos);
> +
> +	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
> +		return -ERESTARTSYS;
> +	if (copy_from_user(*f_pos + (u8 *)sensehat_display->vmem, buf, count))
> +		goto out;
> +	ret = sensehat_update_display(sensehat_display);
> +	if (ret < 0) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +	*f_pos += count;
> +	ret = count;
> +out:
> +	mutex_unlock(&sensehat_display->rw_mtx);
> +	return ret;
> +}
> +
> +static const struct file_operations sensehat_display_fops = {
> +	.owner = THIS_MODULE,
> +	.llseek = sensehat_display_llseek,
> +	.read = sensehat_display_read,
> +	.write = sensehat_display_write,
> +};
> +
> +static int sensehat_display_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	struct sensehat_display *sensehat_display =
> +		devm_kmalloc(&pdev->dev, sizeof(*sensehat_display), GFP_KERNEL);
> +	if (!sensehat_display)
> +		return -ENOMEM;
> +
> +	sensehat_display->pdev = pdev;
> +
> +	dev_set_drvdata(&pdev->dev, sensehat_display);
> +
> +	sensehat_display->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!sensehat_display->regmap) {
> +		dev_err(&pdev->dev,
> +			"unable to get sensehat regmap");
> +		return -ENODEV;
> +	}
> +
> +	memset(sensehat_display->vmem, 0, VMEM_SIZE);
> +
> +	mutex_init(&sensehat_display->rw_mtx);
> +
> +	ret = sensehat_update_display(sensehat_display);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not communicate with sensehat");
> +		return ret;
> +	}
> +
> +	sensehat_display->mdev = (struct miscdevice){
> +		.minor = MISC_DYNAMIC_MINOR,
> +		.name = "sense-hat",
> +		.mode = 0666,
> +		.fops = &sensehat_display_fops,
> +	};
> +
> +	ret = misc_register(&sensehat_display->mdev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not register 8x8 LED matrix display.");
> +		return ret;
> +	}
> +
> +	dev_info(&pdev->dev,
> +		 "8x8 LED matrix display registered with minor number %i",
> +		 sensehat_display->mdev.minor);
> +
> +	return 0;
> +}
> +
> +static int sensehat_display_remove(struct platform_device *pdev)
> +{
> +	struct sensehat_display *sensehat_display = dev_get_drvdata(&pdev->dev);
> +
> +	misc_deregister(&sensehat_display->mdev);
> +	return 0;
> +}
> +
> +static const struct of_device_id sensehat_display_device_id[] = {
> +	{ .compatible = "raspberrypi,sensehat-display" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sensehat_display_device_id);
> +
> +static struct platform_driver sensehat_display_driver = {
> +	.probe = sensehat_display_probe,
> +	.remove = sensehat_display_remove,
> +	.driver = {
> +		.name = "sensehat-display",
> +		.of_match_table = sensehat_display_device_id,
> +	},
> +};
> +
> +module_platform_driver(sensehat_display_driver);
> +
> +MODULE_DESCRIPTION("Raspberry Pi Sense HAT 8x8 LED matrix display driver");
> +MODULE_AUTHOR("Charles Mirabile <cmirabil@redhat.com>");
> +MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.31.1
> 
> 

[PATCH 3/5] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema

[Charles Mirabile]

This patch adds the device tree bindings for the Sense HAT
and each of its children devices in yaml form.

Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../raspberrypi,sensehat-display.yaml         | 26 +++++++++
 .../input/raspberrypi,sensehat-joystick.yaml  | 31 ++++++++++
 .../bindings/mfd/raspberrypi,sensehat.yaml    | 57 +++++++++++++++++++
 3 files changed, 114 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..0cf59b0a519c
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
@@ -0,0 +1,26 @@
+# 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>
+  - 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..98b03878a570
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
@@ -0,0 +1,31 @@
+# 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>
+  - 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:
+    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..bc53834fbaa7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
@@ -0,0 +1,57 @@
+# 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>
+  - 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:
+    maxItems: 1
+
+  joystick:
+    $ref: /schemas/input/raspberrypi,sensehat-joystick.yaml#
+
+  display:
+    $ref: /schemas/auxdisplay/raspberrypi,sensehat-display.yaml#
+
+required:
+  - compatible
+  - reg
+  - joystick
+  - display
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      hat@46 {
+        compatible = "raspberrypi,sensehat";
+        reg = <0x46>;
+        display {
+          compatible = "raspberrypi,sensehat-display";
+        };
+        joystick {
+          compatible = "raspberrypi,sensehat-joystick";
+          interrupts = <23 IRQ_TYPE_EDGE_RISING>;
+        };
+      };
+    };
-- 
2.31.1

Re: [PATCH 3/5] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema

[Matthias Brugger]

On 23/08/2022 19:41, Charles Mirabile wrote:
> This patch adds the device tree bindings for the Sense HAT
> and each of its children devices in yaml form.
> 
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

When doing a v11, maybe decide if you want to split this patch up. This way 
every maintainer (MFD, auxdisplay, input) can take his part of the binding 
description through his tree. Otherwise you will need Acked-by tags from them 
and one maintainer to volunteer, maybe provide a stable tag... Having it split 
up seems easier to me.

Regards,
Matthias

> ---
>   .../raspberrypi,sensehat-display.yaml         | 26 +++++++++
>   .../input/raspberrypi,sensehat-joystick.yaml  | 31 ++++++++++
>   .../bindings/mfd/raspberrypi,sensehat.yaml    | 57 +++++++++++++++++++
>   3 files changed, 114 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..0cf59b0a519c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
> @@ -0,0 +1,26 @@
> +# 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>
> +  - 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..98b03878a570
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
> @@ -0,0 +1,31 @@
> +# 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>
> +  - 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:
> +    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..bc53834fbaa7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> @@ -0,0 +1,57 @@
> +# 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>
> +  - 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:
> +    maxItems: 1
> +
> +  joystick:
> +    $ref: /schemas/input/raspberrypi,sensehat-joystick.yaml#
> +
> +  display:
> +    $ref: /schemas/auxdisplay/raspberrypi,sensehat-display.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - joystick
> +  - display
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      hat@46 {
> +        compatible = "raspberrypi,sensehat";
> +        reg = <0x46>;
> +        display {
> +          compatible = "raspberrypi,sensehat-display";
> +        };
> +        joystick {
> +          compatible = "raspberrypi,sensehat-joystick";
> +          interrupts = <23 IRQ_TYPE_EDGE_RISING>;
> +        };
> +      };
> +    };

Re: [PATCH 3/5] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Tue, Aug 23, 2022 at 01:41:56PM -0400, Charles Mirabile wrote:
> This patch adds the device tree bindings for the Sense HAT
> and each of its children devices in yaml form.

Better say "Add DT bindings for Sense HAT and its subdrivers."

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 4/5] MAINTAINERS: Add sensehat driver authors to MAINTAINERS

[Charles Mirabile]

This patch adds the driver authors to MAINAINERS.

Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 96f47a7865d6..3af3238a1535 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18344,6 +18344,16 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-chemical-sunrise-co2
 F:	Documentation/devicetree/bindings/iio/chemical/senseair,sunrise.yaml
 F:	drivers/iio/chemical/sunrise_co2.c
 
+SENSEHAT DRIVER
+M:	Charles Mirabile <cmirabil@redhat.com>
+M:	Joel Savitz <jsavitz@redhat.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
+F:	Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
+F:	Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
+F:	drivers/auxdisplay/sensehat-display.c
+F:	drivers/input/joystick/sensehat-joystick.c
+
 SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER
 M:	Tomasz Duszynski <tomasz.duszynski@octakon.com>
 S:	Maintained
-- 
2.31.1

Re: [PATCH 4/5] MAINTAINERS: Add sensehat driver authors to MAINTAINERS

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]

On Tue, Aug 23, 2022 at 01:41:57PM -0400, Charles Mirabile wrote:
> This patch adds the driver authors to MAINAINERS.

Better say "Add MAINTAINERS entry for Sense HAT driver."

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 5/5] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

[Charles Mirabile]

This patch shold not be merged - dtbs files are not stored in the
kernel tree. We just provide this file so the code can be tested.

This overlay is suitable for testing the driver, it can be compiled with
dtc and put in the /boot/overlays/ folder then specified in config.txt
by putting the lines:

dtoverlay=		#suppress loading of default overlay for HAT
dtoverlay=sensehat	#load custom overlay

at the beginning before any other lines in config.txt

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

diff --git a/sensehat.dtbs b/sensehat.dtbs
new file mode 100644
index 000000000000..9e5a6d9229b1
--- /dev/null
+++ b/sensehat.dtbs
@@ -0,0 +1,52 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	compatible = "brcm,bcm2835";
+};
+
+&i2c1 {
+	#address-cells = <0x01>;
+	#size-cells = <0x00>;
+	status = "okay";
+
+	sensehat@46 {
+		compatible = "raspberrypi,sensehat";
+		reg = <0x46>;
+		interrupt-parent = <&gpio>;
+		status = "okay";
+		display {
+			compatible = "raspberrypi,sensehat-display";
+			status = "okay";
+		};
+		joystick {
+			compatible = "raspberrypi,sensehat-joystick";
+			interrupts = <23 1>;
+			status = "okay";
+		};
+	};
+
+	lsm9ds1-magn@1c {
+		compatible = "st,lsm9ds1-magn";
+		reg = <0x1c>;
+		status = "okay";
+	};
+
+	lsm9ds1-accel@6a {
+		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";
+	};
+};
-- 
2.31.1

Re: [PATCH 5/5] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Tue, Aug 23, 2022 at 01:41:58PM -0400, Charles Mirabile wrote:
> This patch shold not be merged - dtbs files are not stored in the
> kernel tree. We just provide this file so the code can be tested.
> 

Hmm, if this is DO NOT MERGE patch due to build artifacts (in this case
dtbs), then how testing of this driver can be done with just DT source
(like other drivers)? Does providing custom overlay needed for testing
it?

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 5/5] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

[Matthias Brugger]

On 06/09/2022 04:52, Bagas Sanjaya wrote:
> On Tue, Aug 23, 2022 at 01:41:58PM -0400, Charles Mirabile wrote:
>> This patch shold not be merged - dtbs files are not stored in the
>> kernel tree. We just provide this file so the code can be tested.
>>
> 
> Hmm, if this is DO NOT MERGE patch due to build artifacts (in this case
> dtbs), then how testing of this driver can be done with just DT source
> (like other drivers)? Does providing custom overlay needed for testing
> it?
> 

As it's a extension board to the RaspberryPi the DTs are not included into the 
Linux kernel. Instead an overlay is build which can be loaded to the base DT via 
a special mechanism of the RPi FW.

Adding this file helps anybody who wants to test the drivers to do so without 
any hustle.

Regards,
Matthias

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Stefan Wahren]

Hi Charles,

+ Maxime & Noralf

Am 23.08.22 um 19:41 schrieb 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 and communicate via I2C.
thanks for sending a new version of this series.
>
> 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 the test folder in
> this repo: https://github.com/underground-software/sensehat.git
> 	- sensehat_joystick_test logs the input events from the
> 	  joystick to the console
> 	- sensehat_display_test displays various solid colors on
> 	  the LED panel.
> 	- full_sensehat_test displays a single lit cell that can be
> 	  moved with the joystick. Pressing the joystick ends the
> 	  program.
>
> For more information about the Sense HAT, visit:
> https://www.raspberrypi.org/products/sense-hat/
>
> Changes since v9:
> 	- The driver for the sensehat joystick was accepted upstream so
> 	  it is no longer included in this patchset.
> 	- Some of the people involved in the development of earlier
> 	  versions of these patches are no longer involved in the project
> 	  so they have been removed from the list of maintainers and the
> 	  patches.
> 	- The code is completely unchanged and so based on the discussions
> 	  from version 9 of the patches, it should be ready to be merged.
>
> My appologies for the long delay between v9 and v10 especially given the
> fact that there are no changes to the code. We appreciate the maintainers'
> patience and guidance throughout all 10 versions of this sensehat patchset,
> and we are excited for this driver to hopefully be fully accepted.
>
> Best - Charlie
>
> Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> Signed-off-by: Joel Slebodnick <jslebodn@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>
>
> Charles Mirabile (5):
>    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
AFAIK Lee Jones has a new kernel.org address the old linaro address 
shouldn't be used anymore. We don't get any further without him.
>    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
We don't have an Ack for this display driver. Maybe we should ask Maxime 
Ripard and Noralf Trønnes especially Pavel Machek had concerns about the 
right subsystem?
>    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
>    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
>    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

In case you want to send a new version it would be nice to enable the 
driver in bcm2835_defconfig for better test coverage. But this is not a 
show stopper.

Best regards

>
>   .../raspberrypi,sensehat-display.yaml         |  26 +++
>   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
>   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
>   MAINTAINERS                                   |  10 +
>   drivers/auxdisplay/Kconfig                    |   8 +
>   drivers/auxdisplay/Makefile                   |   1 +
>   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
>   drivers/mfd/simple-mfd-i2c.c                  |   1 +
>   sensehat.dtbs                                 |  52 +++++
>   9 files changed, 391 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
>   create mode 100644 drivers/auxdisplay/sensehat-display.c
>   create mode 100644 sensehat.dtbs
>

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Charles Mirabile]

On Tue, Aug 23, 2022 at 5:30 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>
> Hi Charles,
>
> + Maxime & Noralf
>
> Am 23.08.22 um 19:41 schrieb 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 and communicate via I2C.
> thanks for sending a new version of this series.
> >
> > 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 the test folder in
> > this repo: https://github.com/underground-software/sensehat.git
> >       - sensehat_joystick_test logs the input events from the
> >         joystick to the console
> >       - sensehat_display_test displays various solid colors on
> >         the LED panel.
> >       - full_sensehat_test displays a single lit cell that can be
> >         moved with the joystick. Pressing the joystick ends the
> >         program.
> >
> > For more information about the Sense HAT, visit:
> > https://www.raspberrypi.org/products/sense-hat/
> >
> > Changes since v9:
> >       - The driver for the sensehat joystick was accepted upstream so
> >         it is no longer included in this patchset.
> >       - Some of the people involved in the development of earlier
> >         versions of these patches are no longer involved in the project
> >         so they have been removed from the list of maintainers and the
> >         patches.
> >       - The code is completely unchanged and so based on the discussions
> >         from version 9 of the patches, it should be ready to be merged.
> >
> > My appologies for the long delay between v9 and v10 especially given the
> > fact that there are no changes to the code. We appreciate the maintainers'
> > patience and guidance throughout all 10 versions of this sensehat patchset,
> > and we are excited for this driver to hopefully be fully accepted.
> >
> > Best - Charlie
> >
> > Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> > Signed-off-by: Joel Slebodnick <jslebodn@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>
> >
> > Charles Mirabile (5):
> >    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
> AFAIK Lee Jones has a new kernel.org address the old linaro address
> shouldn't be used anymore. We don't get any further without him.
oops - my bad. Copied the old one from the previous CC list, but I see
the new one was being output properly by get_maintainer.pl, I just
wasn't paying enough attention
> >    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
> We don't have an Ack for this display driver. Maybe we should ask Maxime
> Ripard and Noralf Trønnes especially Pavel Machek had concerns about the
> right subsystem?
I just looked back at the comments from Pavel on v9. I had meant to
reply to him, and I actually drafted something but it got lost in the
shuffle and I never sent it (also my bad).
The driver is an fbdev in the raspberry pi tree, and that is where we
started when upstreaming, but right from the outset we were told that
it couldn't be an fbdev since that subsystem is being deprecated
the replacement is the DRM subsystem, but (as Matthias Brugger
suggests) that is probably overkill for an 8x8 display. Does that
settle the subsystem question?
Also, Is Matthias's review acceptable, or do we need someone else's
ack? It was my understanding based on his comments on v9 that Miguel
Ojeda was also basically ready to ack it, but I don't want to speak
for him.
> >    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
> >    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
> >    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
>
> In case you want to send a new version it would be nice to enable the
> driver in bcm2835_defconfig for better test coverage. But this is not a
> show stopper.
Since I am going to have to send a v11 to fix Lee's email address I
will look into this. Is it as simple as adding another patch that puts
CONFIG_SENSEHAT_DISPLAY=y into the next version?
Presumably I would also want to enable CONFIG_JOYSTICK_SENSEHAT right?
Do I have to do anything special since both of those options have
decencies (namely CONFIG_I2C for both and CONFIG_INPUT for the latter,
and both select MFD_SIMPLE_MFD_I2C)?
>
> Best regards
>
> >
> >   .../raspberrypi,sensehat-display.yaml         |  26 +++
> >   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
> >   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
> >   MAINTAINERS                                   |  10 +
> >   drivers/auxdisplay/Kconfig                    |   8 +
> >   drivers/auxdisplay/Makefile                   |   1 +
> >   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
> >   drivers/mfd/simple-mfd-i2c.c                  |   1 +
> >   sensehat.dtbs                                 |  52 +++++
> >   9 files changed, 391 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
> >   create mode 100644 drivers/auxdisplay/sensehat-display.c
> >   create mode 100644 sensehat.dtbs
> >
>

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

On Wed, Aug 24, 2022 at 08:06:59AM -0400, Charles Mirabile wrote:
> On Tue, Aug 23, 2022 at 5:30 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > >    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
> > We don't have an Ack for this display driver. Maybe we should ask Maxime
> > Ripard and Noralf Trønnes especially Pavel Machek had concerns about the
> > right subsystem?
>
> I just looked back at the comments from Pavel on v9. I had meant to
> reply to him, and I actually drafted something but it got lost in the
> shuffle and I never sent it (also my bad).
>
> The driver is an fbdev in the raspberry pi tree, and that is where we
> started when upstreaming, but right from the outset we were told that
> it couldn't be an fbdev since that subsystem is being deprecated
> the replacement is the DRM subsystem

Indeed

> but (as Matthias Brugger suggests) that is probably overkill for an
> 8x8 display. Does that settle the subsystem question?

If you were aiming for an fbdev driver, a KMS driver these days would be
notably simpler.

We've added simpler and simpler devices, such as the SSD1306 recently,
so I can't see anything wrong with using KMS either.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Stefan Wahren]

Am 24.08.22 um 14:06 schrieb Charles Mirabile:
>> In case you want to send a new version it would be nice to enable the
>> driver in bcm2835_defconfig for better test coverage. But this is not a
>> show stopper.
> Since I am going to have to send a v11 to fix Lee's email address I
> will look into this. Is it as simple as adding another patch that puts
> CONFIG_SENSEHAT_DISPLAY=y into the next version?
> Presumably I would also want to enable CONFIG_JOYSTICK_SENSEHAT right?
> Do I have to do anything special since both of those options have
> decencies (namely CONFIG_I2C for both and CONFIG_INPUT for the latter,
> and both select MFD_SIMPLE_MFD_I2C)?

make bcm2835_defconfig

enable both driver via make menuconfig

make savedefconfig

mv defconfig arch/arm/configs/bcm2835_defconfig.2

diff -u arch/arm/configs/bcm2835_defconfig 
arch/arm/configs/bcm2835_defconfig.2 > config.patch

manually add only the relevant changes from config.patch in 
arch/arm/configs/bcm2835_defconfig

generate the patch as usual

goal is to have simple patch which only contains the 2 driver changes 
without any clean-up changes

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Matthias Brugger]

On 23/08/2022 19:41, Charles Mirabile wrote:
> 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 and communicate 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 the test folder in
> this repo: https://github.com/underground-software/sensehat.git
> 	- sensehat_joystick_test logs the input events from the
> 	  joystick to the console
> 	- sensehat_display_test displays various solid colors on
> 	  the LED panel.
> 	- full_sensehat_test displays a single lit cell that can be
> 	  moved with the joystick. Pressing the joystick ends the
> 	  program.
> 
> For more information about the Sense HAT, visit:
> https://www.raspberrypi.org/products/sense-hat/
> 
> Changes since v9:
> 	- The driver for the sensehat joystick was accepted upstream so
> 	  it is no longer included in this patchset.
> 	- Some of the people involved in the development of earlier
> 	  versions of these patches are no longer involved in the project
> 	  so they have been removed from the list of maintainers and the
> 	  patches.

I'd would expect the patches to honor their involvement in the past, but if they 
are fine from being dropped, I will not object...

Regards,
Matthias

> 	- The code is completely unchanged and so based on the discussions
> 	  from version 9 of the patches, it should be ready to be merged.
> 
> My appologies for the long delay between v9 and v10 especially given the
> fact that there are no changes to the code. We appreciate the maintainers'
> patience and guidance throughout all 10 versions of this sensehat patchset,
> and we are excited for this driver to hopefully be fully accepted.
> 
> Best - Charlie
> 
> Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> Signed-off-by: Joel Slebodnick <jslebodn@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>
> 
> Charles Mirabile (5):
>    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
>    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
>    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
>    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
>    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
> 
>   .../raspberrypi,sensehat-display.yaml         |  26 +++
>   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
>   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
>   MAINTAINERS                                   |  10 +
>   drivers/auxdisplay/Kconfig                    |   8 +
>   drivers/auxdisplay/Makefile                   |   1 +
>   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
>   drivers/mfd/simple-mfd-i2c.c                  |   1 +
>   sensehat.dtbs                                 |  52 +++++
>   9 files changed, 391 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
>   create mode 100644 drivers/auxdisplay/sensehat-display.c
>   create mode 100644 sensehat.dtbs
> 

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Charles Mirabile]

On Wed, Aug 24, 2022 at 7:28 AM Matthias Brugger <mbrugger@suse.com> wrote:
>
>
>
> On 23/08/2022 19:41, Charles Mirabile wrote:
> > 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 and communicate 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 the test folder in
> > this repo: https://github.com/underground-software/sensehat.git
> >       - sensehat_joystick_test logs the input events from the
> >         joystick to the console
> >       - sensehat_display_test displays various solid colors on
> >         the LED panel.
> >       - full_sensehat_test displays a single lit cell that can be
> >         moved with the joystick. Pressing the joystick ends the
> >         program.
> >
> > For more information about the Sense HAT, visit:
> > https://www.raspberrypi.org/products/sense-hat/
> >
> > Changes since v9:
> >       - The driver for the sensehat joystick was accepted upstream so
> >         it is no longer included in this patchset.
> >       - Some of the people involved in the development of earlier
> >         versions of these patches are no longer involved in the project
> >         so they have been removed from the list of maintainers and the
> >         patches.
>
> I'd would expect the patches to honor their involvement in the past, but if they
> are fine from being dropped, I will not object...
Since I am sending a v11 I will try to get in touch with them and see
if they want
to be involved and find proper emails (they no longer work at Red Hat).
I will also look into how much / to which parts of the code they
contributed and see
where it is appropriate to include them if they do want to be
included. I should have
done this before sending v10, but I didn't have the proper emails and
wasn't sure
about the correct etiquette.
>
> Regards,
> Matthias
>
> >       - The code is completely unchanged and so based on the discussions
> >         from version 9 of the patches, it should be ready to be merged.
> >
> > My appologies for the long delay between v9 and v10 especially given the
> > fact that there are no changes to the code. We appreciate the maintainers'
> > patience and guidance throughout all 10 versions of this sensehat patchset,
> > and we are excited for this driver to hopefully be fully accepted.
> >
> > Best - Charlie
> >
> > Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> > Signed-off-by: Joel Slebodnick <jslebodn@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>
> >
> > Charles Mirabile (5):
> >    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
> >    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
> >    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
> >    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
> >    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
> >
> >   .../raspberrypi,sensehat-display.yaml         |  26 +++
> >   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
> >   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
> >   MAINTAINERS                                   |  10 +
> >   drivers/auxdisplay/Kconfig                    |   8 +
> >   drivers/auxdisplay/Makefile                   |   1 +
> >   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
> >   drivers/mfd/simple-mfd-i2c.c                  |   1 +
> >   sensehat.dtbs                                 |  52 +++++
> >   9 files changed, 391 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
> >   create mode 100644 drivers/auxdisplay/sensehat-display.c
> >   create mode 100644 sensehat.dtbs
> >
>

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On Tue, Aug 23, 2022 at 01:41:53PM -0400, Charles Mirabile wrote:
> 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 and communicate via I2C.
> 

Better say "Add a set of drivers for Sense HAT peripheral add-on device
for Raspberry Pi. The device is connected using GPIO connector and
communicate via I2C".

> 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.
> 

s/We modified his code/The driver code is modified/

Also, I see that description of individual patches is written in
descriptive mood. However, it is preferable to write in imperative
instead (see "Describe your changes" in
Documentation/process/submitting-patches.rst). The specific suggested
wordings are in reply to each patch.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v10 0/6] Raspberry Pi Sense HAT driver

[Matthias Brugger]

On 06/09/2022 04:21, Bagas Sanjaya wrote:
> On Tue, Aug 23, 2022 at 01:41:53PM -0400, Charles Mirabile wrote:
>> 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 and communicate via I2C.
>>
> 
> Better say "Add a set of drivers for Sense HAT peripheral add-on device
> for Raspberry Pi. The device is connected using GPIO connector and
> communicate via I2C".
> 

The cover letter won't be part of the git log, so as long as it's understandable 
it's OK. It makes no sense to review the wording...

Regards,
Matthias

>> 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.
>>
> 
> s/We modified his code/The driver code is modified/
> 
> Also, I see that description of individual patches is written in
> descriptive mood. However, it is preferable to write in imperative
> instead (see "Describe your changes" in
> Documentation/process/submitting-patches.rst). The specific suggested
> wordings are in reply to each patch.
> 
> Thanks.
>