linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Raspberry Pi Sense HAT driver
@ 2021-10-29 21:55 Charles Mirabile
  2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Charles Mirabile @ 2021-10-29 21:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Charles Mirabile, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

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

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

Changes since v2:
	- We included a device tree binding in yaml form
	- Switched to regmap for all i2c communications
	- Fixed a few places where we had the old rpisense
	  name that we missed when renaming for v2

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 (5):
  drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
  drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick
    driver
  drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver
  Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device
    tree binding
  MAINTAINERS: Add sensehat driver authors to MAINTAINERS

 .../bindings/mfd/raspberrypi,sensehat.yaml    |  50 ++++
 MAINTAINERS                                   |  11 +
 drivers/auxdisplay/Kconfig                    |   7 +
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/sensehat-display.c         | 258 ++++++++++++++++++
 drivers/input/joystick/Kconfig                |   8 +
 drivers/input/joystick/Makefile               |   1 +
 drivers/input/joystick/sensehat-joystick.c    | 135 +++++++++
 drivers/mfd/Kconfig                           |   8 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/sensehat-core.c                   | 164 +++++++++++
 include/linux/mfd/sensehat.h                  |  64 +++++
 12 files changed, 708 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
 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.31.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
@ 2021-10-29 21:55 ` Charles Mirabile
  2021-10-30  8:02   ` kernel test robot
                     ` (2 more replies)
  2021-10-29 21:55 ` [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 18+ messages in thread
From: Charles Mirabile @ 2021-10-29 21:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Charles Mirabile, Lee Jones, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

This patch adds the core driver file, containing the regmap configuration
needed to communicate with the board over I2C. We also add the header
file shared by all three drivers, containing common data and definitions.
In addition, we add a config option to toggle compilation of the driver.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 drivers/mfd/Kconfig          |   8 ++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/sensehat-core.c  | 164 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/sensehat.h |  64 ++++++++++++++
 4 files changed, 237 insertions(+)
 create mode 100644 drivers/mfd/sensehat-core.c
 create mode 100644 include/linux/mfd/sensehat.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ca0edab91aeb..297ab2143ced 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -11,6 +11,14 @@ config MFD_CORE
 	select IRQ_DOMAIN
 	default n
 
+config MFD_SENSEHAT_CORE
+	tristate "Raspberry Pi Sense HAT core functions"
+	depends on I2C
+	select MFD_CORE
+	help
+	  This is the core driver for the Raspberry Pi Sense HAT. This provides
+	  the necessary functions to communicate with the hardware.
+
 config MFD_CS5535
 	tristate "AMD CS5535 and CS5536 southbridge core functions"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 2ba6646e874c..7347a040a4ac 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -264,6 +264,7 @@ obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
 obj-$(CONFIG_MFD_ROHM_BD957XMUF)	+= rohm-bd9576.o
 obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
+obj-$(CONFIG_MFD_SENSEHAT_CORE) += sensehat-core.o
 obj-$(CONFIG_MFD_ACER_A500_EC)	+= acer-ec-a500.o
 obj-$(CONFIG_MFD_QCOM_PM8008)	+= qcom-pm8008.o
 
diff --git a/drivers/mfd/sensehat-core.c b/drivers/mfd/sensehat-core.c
new file mode 100644
index 000000000000..fb6c89510ec0
--- /dev/null
+++ b/drivers/mfd/sensehat-core.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Raspberry Pi Sense HAT core driver
+ * http://raspberrypi.org
+ *
+ * Copyright (C) 2015 Raspberry Pi
+ * Copyright (C) 2021 Charles Mirabile, Mwesigwa Guma, Joel Savitz
+ *
+ * Original Author: Serge Schneider
+ * Revised for upstream Linux by: Charles Mirabile, Mwesigwa Guma, Joel Savitz
+ *
+ * This driver is based on wm8350 implementation and was refactored to use the
+ * misc device subsystem rather than the deprecated framebuffer subsystem.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include "sensehat.h"
+
+static struct platform_device *
+sensehat_client_dev_register(struct sensehat *sensehat, const char *name);
+
+static struct regmap_config sensehat_config;
+
+static int sensehat_probe(struct i2c_client *i2c,
+			       const struct i2c_device_id *id)
+{
+	int ret;
+	unsigned int reg;
+
+	struct sensehat *sensehat = devm_kzalloc(&i2c->dev, sizeof(*sensehat), GFP_KERNEL);
+
+	if (!sensehat)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, sensehat);
+	sensehat->dev = &i2c->dev;
+	sensehat->i2c_client = i2c;
+
+	sensehat->regmap = devm_regmap_init_i2c(sensehat->i2c_client, &sensehat_config);
+
+	if (IS_ERR(sensehat->regmap)) {
+		dev_err(sensehat->dev, "Failed to initialize sensehat regmap");
+		return PTR_ERR(sensehat->regmap);
+	}
+
+
+	ret = regmap_read(sensehat->regmap, SENSEHAT_WAI, &reg);
+	if (ret < 0) {
+		dev_err(sensehat->dev, "failed to read from device");
+		return ret;
+	}
+
+	if (reg != SENSEHAT_ID) {
+		dev_err(sensehat->dev, "expected device ID %i, got %i",
+			SENSEHAT_ID, ret);
+		return -EINVAL;
+	}
+
+	ret = regmap_read(sensehat->regmap, SENSEHAT_VER, &reg);
+	if (ret < 0) {
+		dev_err(sensehat->dev, "Unable to get sensehat firmware version");
+		return ret;
+	}
+
+	dev_info(sensehat->dev,
+		 "Raspberry Pi Sense HAT firmware version %i\n", reg);
+
+	sensehat->joystick.pdev = sensehat_client_dev_register(sensehat,
+							       "sensehat-joystick");
+
+	if (IS_ERR(sensehat->joystick.pdev)) {
+		dev_err(sensehat->dev, "failed to register sensehat-joystick");
+		return PTR_ERR(sensehat->joystick.pdev);
+	}
+
+	sensehat->display.pdev = sensehat_client_dev_register(sensehat,
+								  "sensehat-display");
+
+	if (IS_ERR(sensehat->display.pdev)) {
+		dev_err(sensehat->dev, "failed to register sensehat-display");
+		return PTR_ERR(sensehat->display.pdev);
+	}
+
+	return 0;
+}
+
+static struct platform_device *
+sensehat_client_dev_register(struct sensehat *sensehat, const char *name)
+{
+	long ret = -ENOMEM;
+	struct platform_device *pdev = platform_device_alloc(name, -1);
+
+	if (!pdev)
+		goto alloc_fail;
+
+	pdev->dev.parent = sensehat->dev;
+	platform_set_drvdata(pdev, sensehat);
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto add_fail;
+
+	ret = devm_add_action_or_reset(sensehat->dev,
+		(void *)platform_device_unregister, pdev);
+	if (ret)
+		goto alloc_fail;
+
+	return pdev;
+
+add_fail:
+	platform_device_put(pdev);
+alloc_fail:
+	return ERR_PTR(ret);
+}
+
+static bool sensehat_writeable_register(struct device *dev, unsigned int reg)
+{
+	return (reg >= SENSEHAT_DISPLAY &&
+		reg < SENSEHAT_DISPLAY + sizeof(sensehat_fb_t))
+		|| reg == SENSEHAT_EE_WP;
+}
+static bool sensehat_readable_register(struct device *dev, unsigned int reg)
+{
+	return (reg >= SENSEHAT_DISPLAY &&
+		reg < SENSEHAT_DISPLAY + sizeof(sensehat_fb_t))
+		|| reg == SENSEHAT_WAI || reg == SENSEHAT_VER
+		|| reg == SENSEHAT_KEYS || reg == SENSEHAT_EE_WP;
+}
+
+static struct regmap_config sensehat_config = {
+	.name = "sensehat",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.writeable_reg = sensehat_writeable_register,
+	.readable_reg = sensehat_readable_register,
+};
+
+static const struct i2c_device_id sensehat_i2c_id[] = {
+	{ "sensehat", 0 },
+	{ "rpi-sense", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sensehat_i2c_id);
+
+static struct i2c_driver sensehat_driver = {
+	.driver = {
+		   .name = "sensehat",
+	},
+	.probe = sensehat_probe,
+	.id_table = sensehat_i2c_id,
+};
+
+module_i2c_driver(sensehat_driver);
+
+MODULE_DESCRIPTION("Raspberry Pi Sense HAT core driver");
+MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/sensehat.h b/include/linux/mfd/sensehat.h
new file mode 100644
index 000000000000..7e2e97a43f90
--- /dev/null
+++ b/include/linux/mfd/sensehat.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Raspberry Pi Sense HAT core driver
+ * http://raspberrypi.org
+ *
+ * Copyright (C) 2015 Raspberry Pi
+ * Copyright (C) 2021 Charles Mirabile, Mwesigwa Guma, Joel Savitz
+ *
+ * Original Author: Serge Schneider
+ * Revised for upstream Linux by: Charles Mirabile, Mwesigwa Guma, Joel Savitz
+ */
+
+#ifndef __LINUX_MFD_SENSEHAT_H_
+#define __LINUX_MFD_SENSEHAT_H_
+#include <linux/miscdevice.h>
+
+//8x8 display with 3 color channels
+typedef u8 sensehat_fb_t[8][3][8];
+
+#define SENSEHAT_DISPLAY		0x00
+#define SENSEHAT_WAI			0xF0
+#define SENSEHAT_VER			0xF1
+#define SENSEHAT_KEYS			0xF2
+#define SENSEHAT_EE_WP			0xF3
+
+#define SENSEHAT_ID			's'
+
+#define SENSEDISP_IOC_MAGIC 0xF1
+
+#define SENSEDISP_IOGET_GAMMA _IO(SENSEDISP_IOC_MAGIC, 0)
+#define SENSEDISP_IOSET_GAMMA _IO(SENSEDISP_IOC_MAGIC, 1)
+#define SENSEDISP_IORESET_GAMMA _IO(SENSEDISP_IOC_MAGIC, 2)
+
+struct sensehat {
+	struct device *dev;
+	struct i2c_client *i2c_client;
+	struct regmap *regmap;
+
+	/* Client devices */
+	struct sensehat_joystick {
+		struct platform_device *pdev;
+		struct input_dev *keys_dev;
+		struct gpio_desc *keys_desc;
+		int keys_irq;
+	} joystick;
+
+	struct sensehat_display {
+		struct platform_device *pdev;
+		struct miscdevice mdev;
+		struct mutex rw_mtx;
+		u8 gamma[32];
+		struct {
+			u16 b:5, u:1, g:5, r:5;
+		} vmem[8][8];
+	} display;
+};
+
+enum gamma_preset {
+	GAMMA_DEFAULT = 0,
+	GAMMA_LOWLIGHT,
+	GAMMA_PRESET_COUNT,
+};
+
+#endif
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
  2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
@ 2021-10-29 21:55 ` Charles Mirabile
  2021-10-29 22:03   ` Randy Dunlap
  2021-10-30 14:02   ` kernel test robot
  2021-10-29 21:55 ` [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver Charles Mirabile
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Charles Mirabile @ 2021-10-29 21:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Charles Mirabile, Dmitry Torokhov, linux-input, Serge Schneider,
	Stefan Wahren, Nicolas Saenz Julienne, linux-rpi-kernel,
	fedora-rpi, Mwesigwa Guma, Joel Savitz

This patch implements support for the joystick.
It supports left/right/up/down/enter and is
attached via i2c and a gpio pin for irq.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 drivers/input/joystick/Kconfig             |   8 ++
 drivers/input/joystick/Makefile            |   1 +
 drivers/input/joystick/sensehat-joystick.c | 135 +++++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 drivers/input/joystick/sensehat-joystick.c

diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 3b23078bc7b5..d2f78353b74c 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -399,4 +399,12 @@ config JOYSTICK_N64
 	  Say Y here if you want enable support for the four
 	  built-in controller ports on the Nintendo 64 console.
 
+config JOYSTICK_SENSEHAT
+	tristate "Raspberry Pi Sense HAT joystick"
+	depends on GPIOLIB && INPUT
+	select MFD_SENSEHAT_CORE
+
+	help
+	  This is the joystick driver for the Raspberry Pi Sense HAT
+
 endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 5174b8aba2dd..39c8b5c6e5ae 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64)		+= n64joy.o
 obj-$(CONFIG_JOYSTICK_PSXPAD_SPI)	+= psxpad-spi.o
 obj-$(CONFIG_JOYSTICK_PXRC)		+= pxrc.o
 obj-$(CONFIG_JOYSTICK_QWIIC)		+= qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SENSEHAT)         += sensehat-joystick.o
 obj-$(CONFIG_JOYSTICK_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
 obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
diff --git a/drivers/input/joystick/sensehat-joystick.c b/drivers/input/joystick/sensehat-joystick.c
new file mode 100644
index 000000000000..2a25287b5863
--- /dev/null
+++ b/drivers/input/joystick/sensehat-joystick.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Raspberry Pi Sense HAT joystick driver
+ * http://raspberrypi.org
+ *
+ * Copyright (C) 2015 Raspberry Pi
+ * Copyright (C) 2021 Charles Mirabile, Mwesigwa Guma, Joel Savitz
+ *
+ * Original Author: Serge Schneider
+ * Revised for upstream Linux by: Charles Mirabile, Mwesigwa Guma, Joel Savitz
+ */
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "sensehat.h"
+
+int sensehat_get_joystick_state(struct sensehat *sensehat);
+
+static unsigned char keymap[] = {KEY_DOWN, KEY_RIGHT, KEY_UP, KEY_ENTER, KEY_LEFT,};
+
+static irqreturn_t sensehat_joystick_report(int n, void *cookie)
+{
+	int i;
+	static s32 prev_keys;
+	struct sensehat *sensehat = cookie;
+	struct sensehat_joystick *sensehat_joystick = &sensehat->joystick;
+	s32 keys = sensehat_get_joystick_state(sensehat);
+	s32 changes = keys ^ prev_keys;
+
+	prev_keys = keys;
+	for (i = 0; i < ARRAY_SIZE(keymap); ++i) {
+		if (changes & (1<<i)) {
+			input_report_key(sensehat_joystick->keys_dev,
+					 keymap[i], keys & (1<<i));
+		}
+	}
+	input_sync(sensehat_joystick->keys_dev);
+	return IRQ_HANDLED;
+}
+
+static int sensehat_joystick_probe(struct platform_device *pdev)
+{
+	int ret;
+	int i;
+	struct sensehat *sensehat = dev_get_drvdata(&pdev->dev);
+	struct sensehat_joystick *sensehat_joystick = &sensehat->joystick;
+
+	sensehat_joystick->keys_desc = devm_gpiod_get(&sensehat->i2c_client->dev,
+						"keys-int", GPIOD_IN);
+	if (IS_ERR(sensehat_joystick->keys_desc)) {
+		dev_warn(&pdev->dev, "Failed to get keys-int descriptor.\n");
+		return PTR_ERR(sensehat_joystick->keys_desc);
+	}
+
+
+	sensehat_joystick->keys_dev = devm_input_allocate_device(&pdev->dev);
+	if (!sensehat_joystick->keys_dev) {
+		dev_err(&pdev->dev, "Could not allocate input device.\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(keymap); i++) {
+		set_bit(keymap[i],
+			sensehat_joystick->keys_dev->keybit);
+	}
+
+	sensehat_joystick->keys_dev->name = "Raspberry Pi Sense HAT Joystick";
+	sensehat_joystick->keys_dev->phys = "rpi-sense-joy/input0";
+	sensehat_joystick->keys_dev->id.bustype = BUS_I2C;
+	sensehat_joystick->keys_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	sensehat_joystick->keys_dev->keycode = keymap;
+	sensehat_joystick->keys_dev->keycodesize = sizeof(unsigned char);
+	sensehat_joystick->keys_dev->keycodemax = ARRAY_SIZE(keymap);
+
+	ret = input_register_device(sensehat_joystick->keys_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not register input device.\n");
+		return ret;
+	}
+
+	ret = gpiod_direction_input(sensehat_joystick->keys_desc);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not set keys-int direction.\n");
+		return ret;
+	}
+
+	sensehat_joystick->keys_irq = gpiod_to_irq(sensehat_joystick->keys_desc);
+	if (sensehat_joystick->keys_irq < 0) {
+		dev_err(&pdev->dev, "Could not determine keys-int IRQ.\n");
+		return sensehat_joystick->keys_irq;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, sensehat_joystick->keys_irq,
+		NULL, sensehat_joystick_report, IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+		"keys", sensehat);
+
+	if (ret) {
+		dev_err(&pdev->dev, "IRQ request failed.\n");
+		return ret;
+	}
+	return 0;
+}
+
+int sensehat_get_joystick_state(struct sensehat *sensehat)
+{
+	unsigned int reg;
+	int ret = regmap_read(sensehat->regmap, SENSEHAT_KEYS, &reg);
+
+	return ret < 0 ? ret : reg;
+}
+
+static struct platform_device_id sensehat_joystick_device_id[] = {
+	{ .name = "sensehat-joystick" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, sensehat_joystick_device_id);
+
+static struct platform_driver sensehat_joystick_driver = {
+	.probe = sensehat_joystick_probe,
+	.driver = {
+		.name = "sensehat-joystick",
+	},
+};
+
+module_platform_driver(sensehat_joystick_driver);
+
+MODULE_DESCRIPTION("Raspberry Pi Sense HAT joystick driver");
+MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
+MODULE_LICENSE("GPL");
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
  2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
  2021-10-29 21:55 ` [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
@ 2021-10-29 21:55 ` Charles Mirabile
  2021-10-30 22:12   ` kernel test robot
  2021-11-03 16:47   ` Matthias Brugger
  2021-10-29 21:55 ` [PATCH 4/5] Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device tree binding Charles Mirabile
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Charles Mirabile @ 2021-10-29 21:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Charles Mirabile, Miguel Ojeda, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

This patch implements control of the 8x8 RGB LED matrix display.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 drivers/auxdisplay/Kconfig            |   7 +
 drivers/auxdisplay/Makefile           |   1 +
 drivers/auxdisplay/sensehat-display.c | 258 ++++++++++++++++++++++++++
 3 files changed, 266 insertions(+)
 create mode 100644 drivers/auxdisplay/sensehat-display.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 1509cb74705a..24f5d68cfbff 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -193,6 +193,13 @@ 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"
+	select MFD_SENSEHAT_CORE
+	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 307771027c89..7e9aa479586a 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_HD44780)		+= hd44780.o
 obj-$(CONFIG_HT16K33)		+= ht16k33.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.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..5980ad23fd4f
--- /dev/null
+++ b/drivers/auxdisplay/sensehat-display.c
@@ -0,0 +1,257 @@
+// 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, Mwesigwa Guma, Joel Savitz
+ *
+ * Original Author: Serge Schneider
+ * Revised for upstream Linux by: Charles Mirabile, Mwesigwa Guma, 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/regmap.h>
+
+#include "sensehat.h"
+
+#define GAMMA_SIZE sizeof_field(struct sensehat_display, gamma)
+#define VMEM_SIZE sizeof_field(struct sensehat_display, vmem)
+
+static int sensehat_update_display(struct sensehat *sensehat);
+
+static bool lowlight;
+module_param(lowlight, bool, 0);
+MODULE_PARM_DESC(lowlight, "Reduce LED matrix brightness to one third");
+
+static const u8 gamma_presets[][GAMMA_SIZE] = {
+	[GAMMA_DEFAULT] = {
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01,
+		0x02, 0x02, 0x03, 0x03, 0x04, 0x05, 0x06, 0x07,
+		0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0E, 0x0F, 0x11,
+		0x12, 0x14, 0x15, 0x17, 0x19, 0x1B, 0x1D, 0x1F,
+	},
+	[GAMMA_LOWLIGHT] = {
+		0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+		0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x02, 0x02,
+		0x03, 0x03, 0x03, 0x04, 0x04, 0x05, 0x05, 0x06,
+		0x06, 0x07, 0x07, 0x08, 0x08, 0x09, 0x0A, 0x0A,
+	},
+};
+
+static const struct file_operations sensehat_display_fops;
+
+static int sensehat_display_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	struct sensehat *sensehat = dev_get_drvdata(&pdev->dev);
+	struct sensehat_display *sensehat_display = &sensehat->display;
+
+	memcpy(sensehat_display->gamma, gamma_presets[lowlight], GAMMA_SIZE);
+
+	memset(sensehat_display->vmem, 0, VMEM_SIZE);
+
+	mutex_init(&sensehat_display->rw_mtx);
+
+	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.\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "8x8 LED matrix display registered with minor number %i",
+		sensehat_display->mdev.minor);
+
+	sensehat_update_display(sensehat);
+	return 0;
+}
+
+static int sensehat_display_remove(struct platform_device *pdev)
+{
+	struct sensehat *sensehat = dev_get_drvdata(&pdev->dev);
+	struct sensehat_display *sensehat_display = &sensehat->display;
+
+	misc_deregister(&sensehat_display->mdev);
+	return 0;
+}
+
+static loff_t sensehat_display_llseek(struct file *filp, loff_t pos, int whence)
+{
+	loff_t base;
+
+	switch (whence) {
+	case SEEK_SET:
+		base = 0;
+		break;
+	case SEEK_CUR:
+		base = filp->f_pos;
+		break;
+	case SEEK_END:
+		base = VMEM_SIZE;
+		break;
+	default:
+		return -EINVAL;
+	}
+	base += pos;
+	if (base < 0 || base >= VMEM_SIZE)
+		return -EINVAL;
+	filp->f_pos = base;
+	return base;
+}
+
+static ssize_t
+sensehat_display_read(struct file *filp, char __user *buf, size_t count, loff_t *f_pos)
+{
+	struct sensehat *sensehat = container_of(filp->private_data, struct sensehat, display.mdev);
+	struct sensehat_display *sensehat_display = &sensehat->display;
+	ssize_t retval = -EFAULT;
+
+	if (*f_pos >= VMEM_SIZE)
+		return 0;
+	if (*f_pos + count > VMEM_SIZE)
+		count = VMEM_SIZE - *f_pos;
+	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
+		return -ERESTARTSYS;
+	if (copy_to_user(buf, sensehat_display->vmem + *f_pos, count))
+		goto out;
+	*f_pos += count;
+	retval = count;
+out:
+	mutex_unlock(&sensehat_display->rw_mtx);
+	return retval;
+}
+
+static ssize_t
+sensehat_display_write(struct file *filp, const char __user *buf, size_t count, loff_t *f_pos)
+{
+	struct sensehat *sensehat = container_of(filp->private_data, struct sensehat, display.mdev);
+	struct sensehat_display *sensehat_display = &sensehat->display;
+	u8 temp[VMEM_SIZE];
+
+	if (*f_pos >= VMEM_SIZE)
+		return -EFBIG;
+	if (*f_pos + count > VMEM_SIZE)
+		count = VMEM_SIZE - *f_pos;
+	if (copy_from_user(temp, buf, count))
+		return -EFAULT;
+	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
+		return -ERESTARTSYS;
+	memcpy(sensehat_display->vmem + *f_pos, temp, count);
+	sensehat_update_display(sensehat);
+	*f_pos += count;
+	mutex_unlock(&sensehat_display->rw_mtx);
+	return count;
+}
+
+static long sensehat_display_ioctl(struct file *filp, unsigned int cmd,
+			     unsigned long arg)
+{
+	struct sensehat *sensehat = container_of(filp->private_data, struct sensehat, display.mdev);
+	struct sensehat_display *sensehat_display = &sensehat->display;
+	void __user *user_ptr = (void __user *)arg;
+	u8 temp[GAMMA_SIZE];
+	int ret;
+
+	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
+		return -ERESTARTSYS;
+	switch (cmd) {
+	case SENSEDISP_IOGET_GAMMA:
+		if (copy_to_user(user_ptr, sensehat_display->gamma, GAMMA_SIZE)) {
+			ret = -EFAULT;
+			goto out_unlock;
+		}
+		ret = 0;
+		goto out_unlock;
+	case SENSEDISP_IOSET_GAMMA:
+		if (copy_from_user(temp, user_ptr, GAMMA_SIZE)) {
+			ret = -EFAULT;
+			goto out_unlock;
+		}
+		ret = 0;
+		goto out_update;
+	case SENSEDISP_IORESET_GAMMA:
+		if (arg < GAMMA_DEFAULT || arg >= GAMMA_PRESET_COUNT) {
+			ret = -EINVAL;
+			goto out_unlock;
+		}
+		memcpy(temp, gamma_presets[arg], GAMMA_SIZE);
+		ret = 0;
+		goto out_update;
+	default:
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+out_update:
+	memcpy(sensehat_display->gamma, temp, GAMMA_SIZE);
+	sensehat_update_display(sensehat);
+out_unlock:
+	mutex_unlock(&sensehat_display->rw_mtx);
+	return ret;
+}
+
+int sensehat_update_display(struct sensehat *sensehat)
+{
+	int i, j, ret;
+	struct sensehat_display *display = &sensehat->display;
+	sensehat_fb_t pixel_data;
+
+	for (i = 0; i < 8; ++i) {
+		for (j = 0; j < 8; ++j) {
+			pixel_data[i][0][j] = display->gamma[display->vmem[i][j].r];
+			pixel_data[i][1][j] = display->gamma[display->vmem[i][j].g];
+			pixel_data[i][2][j] = display->gamma[display->vmem[i][j].b];
+		}
+	}
+
+	ret = regmap_bulk_write(sensehat->regmap, SENSEHAT_DISPLAY,
+				pixel_data, sizeof(pixel_data));
+	if (ret < 0)
+		dev_err(sensehat->dev, "Update to 8x8 LED matrix display failed");
+	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,
+	.unlocked_ioctl	= sensehat_display_ioctl,
+};
+
+static struct platform_device_id sensehat_display_device_id[] = {
+	{ .name = "sensehat-display" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, sensehat_display_device_id);
+
+static struct platform_driver sensehat_display_driver = {
+	.probe = sensehat_display_probe,
+	.remove = sensehat_display_remove,
+	.driver = {
+		.name = "sensehat-display",
+	},
+};
+
+module_platform_driver(sensehat_display_driver);
+
+MODULE_DESCRIPTION("Raspberry Pi Sense HAT 8x8 LED matrix display driver");
+MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
+MODULE_LICENSE("GPL");
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device tree binding
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
                   ` (2 preceding siblings ...)
  2021-10-29 21:55 ` [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver Charles Mirabile
@ 2021-10-29 21:55 ` Charles Mirabile
  2021-11-02 12:51   ` Rob Herring
  2021-10-29 21:55 ` [PATCH 5/5] MAINTAINERS: Add sensehat driver authors to MAINTAINERS Charles Mirabile
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Charles Mirabile @ 2021-10-29 21:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Charles Mirabile, Lee Jones, Rob Herring, devicetree,
	Serge Schneider, Stefan Wahren, Nicolas Saenz Julienne,
	linux-rpi-kernel, fedora-rpi, Mwesigwa Guma, Joel Savitz

This patch adds the device tree binding
for the Sense HAT in yaml form.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 .../bindings/mfd/raspberrypi,sensehat.yaml    | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml

diff --git a/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
new file mode 100644
index 000000000000..e00cd02a3752
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+$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:
+    oneOf:
+      - enum:
+        - raspberrypi,sensehat
+        - rpi,rpisense
+
+  reg:
+    items:
+      - description: i2c bus address
+
+  keys-int-gpios:
+    items:
+      - description: gpio pin for joystick interrupt
+
+required:
+  - compatible
+  - reg
+  - keys-int-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      sensehat@46 {
+        compatible = "raspberrypi,sensehat";
+        reg = <0x46>;
+        keys-int-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
+      };
+    };
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] MAINTAINERS: Add sensehat driver authors to MAINTAINERS
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
                   ` (3 preceding siblings ...)
  2021-10-29 21:55 ` [PATCH 4/5] Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device tree binding Charles Mirabile
@ 2021-10-29 21:55 ` Charles Mirabile
  2021-10-31 16:38 ` [PATCH 0/5] Raspberry Pi Sense HAT driver Miguel Ojeda
  2021-11-20 10:55 ` Stefan Wahren
  6 siblings, 0 replies; 18+ messages in thread
From: Charles Mirabile @ 2021-10-29 21:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Charles Mirabile, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

This patch adds the driver authors to MAINAINERS.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
 MAINTAINERS | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3b79fd441dde..90fdd21d1db9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16882,6 +16882,17 @@ S:	Maintained
 F:	drivers/misc/phantom.c
 F:	include/uapi/linux/phantom.h
 
+SENSEHAT DRIVER
+M:	Charles Mirabile <cmirabil@redhat.com>
+M:	Mwesigwa Guma <mguma@redhat.com>
+M:	Joel Savitz <jsavitz@redhat.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
+F:	drivers/auxdisplay/sensehat-display.c
+F:	drivers/input/joystick/sensehat-joystick.c
+F:	drivers/mfd/sensehat-core.c
+F:	include/linux/mfd/sensehat.h
+
 SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER
 M:	Tomasz Duszynski <tomasz.duszynski@octakon.com>
 S:	Maintained
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver
  2021-10-29 21:55 ` [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
@ 2021-10-29 22:03   ` Randy Dunlap
  2021-10-30 14:02   ` kernel test robot
  1 sibling, 0 replies; 18+ messages in thread
From: Randy Dunlap @ 2021-10-29 22:03 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: Dmitry Torokhov, linux-input, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

On 10/29/21 2:55 PM, Charles Mirabile wrote:
> This patch implements support for the joystick.
> It supports left/right/up/down/enter and is
> attached via i2c and a gpio pin for irq.
> 
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>   drivers/input/joystick/Kconfig             |   8 ++
>   drivers/input/joystick/Makefile            |   1 +
>   drivers/input/joystick/sensehat-joystick.c | 135 +++++++++++++++++++++
>   3 files changed, 144 insertions(+)
>   create mode 100644 drivers/input/joystick/sensehat-joystick.c
> 
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 3b23078bc7b5..d2f78353b74c 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -399,4 +399,12 @@ config JOYSTICK_N64
>   	  Say Y here if you want enable support for the four
>   	  built-in controller ports on the Nintendo 64 console.
>   
> +config JOYSTICK_SENSEHAT
> +	tristate "Raspberry Pi Sense HAT joystick"
> +	depends on GPIOLIB && INPUT

I think also:
	depends on I2C

since this one:

> +	select MFD_SENSEHAT_CORE

also depends on I2C and 'select' does not follow any
dependency chains.

Same comment applies to patch 3/5 for SENSEHAT_DISPLAY.

> +
> +	help
> +	  This is the joystick driver for the Raspberry Pi Sense HAT
> +
>   endif


-- 
~Randy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
  2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
@ 2021-10-30  8:02   ` kernel test robot
  2021-10-30 17:20   ` Thomas Weißschuh
  2021-11-03 16:33   ` Matthias Brugger
  2 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-10-30  8:02 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: kbuild-all, Charles Mirabile, Lee Jones, Serge Schneider,
	Stefan Wahren, Nicolas Saenz Julienne, linux-rpi-kernel,
	fedora-rpi, Mwesigwa Guma, Joel Savitz

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

Hi Charles,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on dtor-input/next linux/master linus/master v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Charles-Mirabile/Raspberry-Pi-Sense-HAT-driver/20211030-055716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ae7af468c4994435ae01c89e2b8ef53275a6b3b6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Charles-Mirabile/Raspberry-Pi-Sense-HAT-driver/20211030-055716
        git checkout ae7af468c4994435ae01c89e2b8ef53275a6b3b6
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mfd/sensehat-core.c:24:10: fatal error: sensehat.h: No such file or directory
      24 | #include "sensehat.h"
         |          ^~~~~~~~~~~~
   compilation terminated.


vim +24 drivers/mfd/sensehat-core.c

  > 24	#include "sensehat.h"
    25	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69161 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver
  2021-10-29 21:55 ` [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
  2021-10-29 22:03   ` Randy Dunlap
@ 2021-10-30 14:02   ` kernel test robot
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-10-30 14:02 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: kbuild-all, Charles Mirabile, Dmitry Torokhov, linux-input,
	Serge Schneider, Stefan Wahren, Nicolas Saenz Julienne,
	linux-rpi-kernel, fedora-rpi, Mwesigwa Guma

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

Hi Charles,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on dtor-input/next linux/master linus/master v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Charles-Mirabile/Raspberry-Pi-Sense-HAT-driver/20211030-055716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0a53172779554b2a11dd6a8a372fd942e801bc60
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Charles-Mirabile/Raspberry-Pi-Sense-HAT-driver/20211030-055716
        git checkout 0a53172779554b2a11dd6a8a372fd942e801bc60
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/input/joystick/sensehat-joystick.c:21:10: fatal error: sensehat.h: No such file or directory
      21 | #include "sensehat.h"
         |          ^~~~~~~~~~~~
   compilation terminated.


vim +21 drivers/input/joystick/sensehat-joystick.c

    20	
  > 21	#include "sensehat.h"
    22	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69170 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
  2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
  2021-10-30  8:02   ` kernel test robot
@ 2021-10-30 17:20   ` Thomas Weißschuh
  2021-11-03 16:33   ` Matthias Brugger
  2 siblings, 0 replies; 18+ messages in thread
From: Thomas Weißschuh @ 2021-10-30 17:20 UTC (permalink / raw)
  To: Charles Mirabile
  Cc: linux-kernel, Lee Jones, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

On 2021-10-29 17:55-0400, Charles Mirabile wrote:
> +static int sensehat_probe(struct i2c_client *i2c,
> +			       const struct i2c_device_id *id)
> +{
> 	...
> +
> +	sensehat->joystick.pdev = sensehat_client_dev_register(sensehat,
> +							       "sensehat-joystick");

Will this not fail the complete driver load if the driver for this single
subdevice is not available?
For example if the Kconfig option has not been selected, the patch for the
driver is not applied or the user has blacklisted the respective module in
their system.

> +	if (IS_ERR(sensehat->joystick.pdev)) {
> +		dev_err(sensehat->dev, "failed to register sensehat-joystick");
> +		return PTR_ERR(sensehat->joystick.pdev);
> +	}
> +
> +	sensehat->display.pdev = sensehat_client_dev_register(sensehat,
> +								  "sensehat-display");

Same as above.

> +
> +	if (IS_ERR(sensehat->display.pdev)) {
> +		dev_err(sensehat->dev, "failed to register sensehat-display");
> +		return PTR_ERR(sensehat->display.pdev);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_device *
> +sensehat_client_dev_register(struct sensehat *sensehat, const char *name)
> +{
> +	long ret = -ENOMEM;
> +	struct platform_device *pdev = platform_device_alloc(name, -1);
> +
> +	if (!pdev)
> +		goto alloc_fail;
> +
> +	pdev->dev.parent = sensehat->dev;
> +	platform_set_drvdata(pdev, sensehat);
> +
> +	ret = platform_device_add(pdev);
> +	if (ret)
> +		goto add_fail;
> +
> +	ret = devm_add_action_or_reset(sensehat->dev,
> +		(void *)platform_device_unregister, pdev);
> +	if (ret)
> +		goto alloc_fail;
> +
> +	return pdev;
> +
> +add_fail:
> +	platform_device_put(pdev);
> +alloc_fail:
> +	return ERR_PTR(ret);
> +}

Thomas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver
  2021-10-29 21:55 ` [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver Charles Mirabile
@ 2021-10-30 22:12   ` kernel test robot
  2021-11-03 16:47   ` Matthias Brugger
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-10-30 22:12 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: kbuild-all, Charles Mirabile, Miguel Ojeda, Serge Schneider,
	Stefan Wahren, Nicolas Saenz Julienne, linux-rpi-kernel,
	fedora-rpi, Mwesigwa Guma, Joel Savitz

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

Hi Charles,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on dtor-input/next linux/master linus/master v5.15-rc7]
[cannot apply to next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Charles-Mirabile/Raspberry-Pi-Sense-HAT-driver/20211030-055716
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7bfd5f3b8be9618031188098c3e85930ea59844e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Charles-Mirabile/Raspberry-Pi-Sense-HAT-driver/20211030-055716
        git checkout 7bfd5f3b8be9618031188098c3e85930ea59844e
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/auxdisplay/sensehat-display.c:26:10: fatal error: sensehat.h: No such file or directory
      26 | #include "sensehat.h"
         |          ^~~~~~~~~~~~
   compilation terminated.


vim +26 drivers/auxdisplay/sensehat-display.c

    25	
  > 26	#include "sensehat.h"
    27	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69177 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Raspberry Pi Sense HAT driver
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
                   ` (4 preceding siblings ...)
  2021-10-29 21:55 ` [PATCH 5/5] MAINTAINERS: Add sensehat driver authors to MAINTAINERS Charles Mirabile
@ 2021-10-31 16:38 ` Miguel Ojeda
  2021-11-01 14:18   ` Joel Savitz
  2021-11-20 10:55 ` Stefan Wahren
  6 siblings, 1 reply; 18+ messages in thread
From: Miguel Ojeda @ 2021-10-31 16:38 UTC (permalink / raw)
  To: Charles Mirabile
  Cc: linux-kernel, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz

On Fri, Oct 29, 2021 at 11:58 PM Charles Mirabile <cmirabil@redhat.com> wrote:
>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>

Please note that you need to write `Co-developed-by: ...` (see
`Documentation/process/submitting-patches.rst`); otherwise, it looks
as if this was sent through several layers.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Raspberry Pi Sense HAT driver
  2021-10-31 16:38 ` [PATCH 0/5] Raspberry Pi Sense HAT driver Miguel Ojeda
@ 2021-11-01 14:18   ` Joel Savitz
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Savitz @ 2021-11-01 14:18 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Charles Mirabile, linux-kernel, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, Fedora RPi,
	Mwesigwa Guma

Thanks for the suggestion, we weren't sure what the right "Xxx-by:"
was for this circumstance so we will use this in subsequent patchsets.

Best,
Joel Savitz

Best,
Joel Savitz


On Sun, Oct 31, 2021 at 12:39 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Oct 29, 2021 at 11:58 PM Charles Mirabile <cmirabil@redhat.com> wrote:
> >
> > Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> > Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> > Signed-off-by: Joel Savitz <jsavitz@redhat.com>
>
> Please note that you need to write `Co-developed-by: ...` (see
> `Documentation/process/submitting-patches.rst`); otherwise, it looks
> as if this was sent through several layers.
>
> Cheers,
> Miguel
>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device tree binding
  2021-10-29 21:55 ` [PATCH 4/5] Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device tree binding Charles Mirabile
@ 2021-11-02 12:51   ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2021-11-02 12:51 UTC (permalink / raw)
  To: Charles Mirabile
  Cc: linux-kernel, Lee Jones, devicetree, Serge Schneider,
	Stefan Wahren, Nicolas Saenz Julienne, linux-rpi-kernel,
	fedora-rpi, Mwesigwa Guma, Joel Savitz

On Fri, Oct 29, 2021 at 05:55:15PM -0400, Charles Mirabile wrote:
> This patch adds the device tree binding
> for the Sense HAT in yaml form.

For the subject, follow the format of the subsystem (run git log 
--oneline) and no need to say 'binding' multiple times:

dt-bindings: mfd: Add Raspberry Pi Sense HAT schema

> 
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>  .../bindings/mfd/raspberrypi,sensehat.yaml    | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> new file mode 100644
> index 000000000000..e00cd02a3752
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +$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: |

'|' is not needed if there is no formatting to preserve.

> +  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:
> +    oneOf:
> +      - enum:

Don't need oneOf when there is only one entry.

> +        - raspberrypi,sensehat
> +        - rpi,rpisense

'rpi' is not a vendor prefix.

What's the fallback for anyways?

> +
> +  reg:
> +    items:
> +      - description: i2c bus address
> +
> +  keys-int-gpios:
> +    items:
> +      - description: gpio pin for joystick interrupt

For an interrupt, use 'interrupts'.

> +
> +required:
> +  - compatible
> +  - reg
> +  - keys-int-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      sensehat@46 {
> +        compatible = "raspberrypi,sensehat";
> +        reg = <0x46>;
> +        keys-int-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
> +      };
> +    };
> -- 
> 2.31.1
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
  2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
  2021-10-30  8:02   ` kernel test robot
  2021-10-30 17:20   ` Thomas Weißschuh
@ 2021-11-03 16:33   ` Matthias Brugger
  2021-11-03 16:53     ` nsaenzju
  2 siblings, 1 reply; 18+ messages in thread
From: Matthias Brugger @ 2021-11-03 16:33 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: Lee Jones, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz



On 29/10/2021 23:55, Charles Mirabile wrote:
> This patch adds the core driver file, containing the regmap configuration
> needed to communicate with the board over I2C. We also add the header
> file shared by all three drivers, containing common data and definitions.
> In addition, we add a config option to toggle compilation of the driver.
> 
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>   drivers/mfd/Kconfig          |   8 ++
>   drivers/mfd/Makefile         |   1 +
>   drivers/mfd/sensehat-core.c  | 164 +++++++++++++++++++++++++++++++++++
>   include/linux/mfd/sensehat.h |  64 ++++++++++++++
>   4 files changed, 237 insertions(+)
>   create mode 100644 drivers/mfd/sensehat-core.c
>   create mode 100644 include/linux/mfd/sensehat.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ca0edab91aeb..297ab2143ced 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -11,6 +11,14 @@ config MFD_CORE
>   	select IRQ_DOMAIN
>   	default n
>   
> +config MFD_SENSEHAT_CORE
> +	tristate "Raspberry Pi Sense HAT core functions"
> +	depends on I2C
> +	select MFD_CORE
> +	help
> +	  This is the core driver for the Raspberry Pi Sense HAT. This provides
> +	  the necessary functions to communicate with the hardware.
> +
>   config MFD_CS5535
>   	tristate "AMD CS5535 and CS5536 southbridge core functions"
>   	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 2ba6646e874c..7347a040a4ac 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -264,6 +264,7 @@ obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
>   obj-$(CONFIG_MFD_ROHM_BD957XMUF)	+= rohm-bd9576.o
>   obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>   obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
> +obj-$(CONFIG_MFD_SENSEHAT_CORE) += sensehat-core.o
>   obj-$(CONFIG_MFD_ACER_A500_EC)	+= acer-ec-a500.o
>   obj-$(CONFIG_MFD_QCOM_PM8008)	+= qcom-pm8008.o
>   
> diff --git a/drivers/mfd/sensehat-core.c b/drivers/mfd/sensehat-core.c
> new file mode 100644
> index 000000000000..fb6c89510ec0
> --- /dev/null
> +++ b/drivers/mfd/sensehat-core.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Raspberry Pi Sense HAT core driver
> + * http://raspberrypi.org
> + *
> + * Copyright (C) 2015 Raspberry Pi
> + * Copyright (C) 2021 Charles Mirabile, Mwesigwa Guma, Joel Savitz
> + *
> + * Original Author: Serge Schneider
> + * Revised for upstream Linux by: Charles Mirabile, Mwesigwa Guma, Joel Savitz
> + *
> + * This driver is based on wm8350 implementation and was refactored to use the
> + * misc device subsystem rather than the deprecated framebuffer subsystem.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include "sensehat.h"
> +
> +static struct platform_device *
> +sensehat_client_dev_register(struct sensehat *sensehat, const char *name);
> +
> +static struct regmap_config sensehat_config;
> +
> +static int sensehat_probe(struct i2c_client *i2c,
> +			       const struct i2c_device_id *id)
> +{
> +	int ret;
> +	unsigned int reg;
> +
> +	struct sensehat *sensehat = devm_kzalloc(&i2c->dev, sizeof(*sensehat), GFP_KERNEL);
> +
> +	if (!sensehat)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, sensehat);
> +	sensehat->dev = &i2c->dev;
> +	sensehat->i2c_client = i2c;
> +
> +	sensehat->regmap = devm_regmap_init_i2c(sensehat->i2c_client, &sensehat_config);
> +
> +	if (IS_ERR(sensehat->regmap)) {
> +		dev_err(sensehat->dev, "Failed to initialize sensehat regmap");
> +		return PTR_ERR(sensehat->regmap);
> +	}
> +
> +
> +	ret = regmap_read(sensehat->regmap, SENSEHAT_WAI, &reg);
> +	if (ret < 0) {
> +		dev_err(sensehat->dev, "failed to read from device");
> +		return ret;
> +	}
> +
> +	if (reg != SENSEHAT_ID) {
> +		dev_err(sensehat->dev, "expected device ID %i, got %i",
> +			SENSEHAT_ID, ret);
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_read(sensehat->regmap, SENSEHAT_VER, &reg);
> +	if (ret < 0) {
> +		dev_err(sensehat->dev, "Unable to get sensehat firmware version");
> +		return ret;
> +	}
> +
> +	dev_info(sensehat->dev,
> +		 "Raspberry Pi Sense HAT firmware version %i\n", reg);
> +
> +	sensehat->joystick.pdev = sensehat_client_dev_register(sensehat,
> +							       "sensehat-joystick");

Why don't you use devm_mfd_add_devices function together with mfd_cell?

> +
> +	if (IS_ERR(sensehat->joystick.pdev)) {
> +		dev_err(sensehat->dev, "failed to register sensehat-joystick");
> +		return PTR_ERR(sensehat->joystick.pdev);
> +	}
> +
> +	sensehat->display.pdev = sensehat_client_dev_register(sensehat,
> +								  "sensehat-display");
> +
> +	if (IS_ERR(sensehat->display.pdev)) {
> +		dev_err(sensehat->dev, "failed to register sensehat-display");
> +		return PTR_ERR(sensehat->display.pdev);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_device *
> +sensehat_client_dev_register(struct sensehat *sensehat, const char *name)
> +{
> +	long ret = -ENOMEM;
> +	struct platform_device *pdev = platform_device_alloc(name, -1);
> +
> +	if (!pdev)
> +		goto alloc_fail;
> +
> +	pdev->dev.parent = sensehat->dev;
> +	platform_set_drvdata(pdev, sensehat);
> +
> +	ret = platform_device_add(pdev);
> +	if (ret)
> +		goto add_fail;
> +
> +	ret = devm_add_action_or_reset(sensehat->dev,
> +		(void *)platform_device_unregister, pdev);
> +	if (ret)
> +		goto alloc_fail;
> +
> +	return pdev;
> +
> +add_fail:
> +	platform_device_put(pdev);
> +alloc_fail:
> +	return ERR_PTR(ret);
> +}
> +
> +static bool sensehat_writeable_register(struct device *dev, unsigned int reg)
> +{
> +	return (reg >= SENSEHAT_DISPLAY &&
> +		reg < SENSEHAT_DISPLAY + sizeof(sensehat_fb_t))
> +		|| reg == SENSEHAT_EE_WP;
> +}
> +static bool sensehat_readable_register(struct device *dev, unsigned int reg)
> +{
> +	return (reg >= SENSEHAT_DISPLAY &&
> +		reg < SENSEHAT_DISPLAY + sizeof(sensehat_fb_t))
> +		|| reg == SENSEHAT_WAI || reg == SENSEHAT_VER
> +		|| reg == SENSEHAT_KEYS || reg == SENSEHAT_EE_WP;
> +}

I wonder if we really need this. In the end this is tied to in kernel drivers. 
AFAIKS no userspace process can read. Without this we could move senhat_fb_t 
into the display driver as it's local to that driver.

> +
> +static struct regmap_config sensehat_config = {
> +	.name = "sensehat",
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.writeable_reg = sensehat_writeable_register,
> +	.readable_reg = sensehat_readable_register,
> +};
> +
> +static const struct i2c_device_id sensehat_i2c_id[] = {
> +	{ "sensehat", 0 },
> +	{ "rpi-sense", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sensehat_i2c_id);
> +
> +static struct i2c_driver sensehat_driver = {
> +	.driver = {
> +		   .name = "sensehat",
> +	},
> +	.probe = sensehat_probe,
> +	.id_table = sensehat_i2c_id,
> +};
> +
> +module_i2c_driver(sensehat_driver);
> +
> +MODULE_DESCRIPTION("Raspberry Pi Sense HAT core driver");
> +MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/sensehat.h b/include/linux/mfd/sensehat.h
> new file mode 100644
> index 000000000000..7e2e97a43f90
> --- /dev/null
> +++ b/include/linux/mfd/sensehat.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Raspberry Pi Sense HAT core driver
> + * http://raspberrypi.org
> + *
> + * Copyright (C) 2015 Raspberry Pi
> + * Copyright (C) 2021 Charles Mirabile, Mwesigwa Guma, Joel Savitz
> + *
> + * Original Author: Serge Schneider
> + * Revised for upstream Linux by: Charles Mirabile, Mwesigwa Guma, Joel Savitz
> + */
> +
> +#ifndef __LINUX_MFD_SENSEHAT_H_
> +#define __LINUX_MFD_SENSEHAT_H_
> +#include <linux/miscdevice.h>
> +
> +//8x8 display with 3 color channels
> +typedef u8 sensehat_fb_t[8][3][8];

I'm not a great fan of typedefs. AFAIK they are rarely used in kernel code.

> +
> +#define SENSEHAT_DISPLAY		0x00
> +#define SENSEHAT_WAI			0xF0
> +#define SENSEHAT_VER			0xF1
> +#define SENSEHAT_KEYS			0xF2
> +#define SENSEHAT_EE_WP			0xF3
> +
> +#define SENSEHAT_ID			's'
> +
> +#define SENSEDISP_IOC_MAGIC 0xF1
> +
> +#define SENSEDISP_IOGET_GAMMA _IO(SENSEDISP_IOC_MAGIC, 0)
> +#define SENSEDISP_IOSET_GAMMA _IO(SENSEDISP_IOC_MAGIC, 1)
> +#define SENSEDISP_IORESET_GAMMA _IO(SENSEDISP_IOC_MAGIC, 2)
> +
> +struct sensehat {
> +	struct device *dev;
> +	struct i2c_client *i2c_client;
> +	struct regmap *regmap;
> +
> +	/* Client devices */
> +	struct sensehat_joystick {
> +		struct platform_device *pdev;
> +		struct input_dev *keys_dev;
> +		struct gpio_desc *keys_desc;
> +		int keys_irq;
> +	} joystick;
> +
> +	struct sensehat_display {
> +		struct platform_device *pdev;
> +		struct miscdevice mdev;
> +		struct mutex rw_mtx;
> +		u8 gamma[32];
> +		struct {
> +			u16 b:5, u:1, g:5, r:5;
> +		} vmem[8][8];
> +	} display;
> +};
> +
> +enum gamma_preset {
> +	GAMMA_DEFAULT = 0,
> +	GAMMA_LOWLIGHT,
> +	GAMMA_PRESET_COUNT,
> +};

Please review, which of this defines really need to be in the header and can't 
be just local to the corresponding driver.

Apart from that, I'd appreciate if you could add me in CC in the next version of 
this patches. I also wonder why you didn't CC arm-linux-kernel mailinglist, in 
the end this is a board for RPi and hence Arm.

Regards,
Matthias

> +
> +#endif
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver
  2021-10-29 21:55 ` [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver Charles Mirabile
  2021-10-30 22:12   ` kernel test robot
@ 2021-11-03 16:47   ` Matthias Brugger
  1 sibling, 0 replies; 18+ messages in thread
From: Matthias Brugger @ 2021-11-03 16:47 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: Miguel Ojeda, Serge Schneider, Stefan Wahren,
	Nicolas Saenz Julienne, linux-rpi-kernel, fedora-rpi,
	Mwesigwa Guma, Joel Savitz



On 29/10/2021 23:55, Charles Mirabile wrote:
> This patch implements control of the 8x8 RGB LED matrix display.
> 
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
>   drivers/auxdisplay/Kconfig            |   7 +
>   drivers/auxdisplay/Makefile           |   1 +
>   drivers/auxdisplay/sensehat-display.c | 258 ++++++++++++++++++++++++++
>   3 files changed, 266 insertions(+)
>   create mode 100644 drivers/auxdisplay/sensehat-display.c
> 
[....]
> diff --git a/drivers/auxdisplay/sensehat-display.c b/drivers/auxdisplay/sensehat-display.c
> new file mode 100644
> index 000000000000..5980ad23fd4f
> --- /dev/null
> +++ b/drivers/auxdisplay/sensehat-display.c
[...]
> +
> +static ssize_t
> +sensehat_display_write(struct file *filp, const char __user *buf, size_t count, loff_t *f_pos)
> +{
> +	struct sensehat *sensehat = container_of(filp->private_data, struct sensehat, display.mdev);
> +	struct sensehat_display *sensehat_display = &sensehat->display;
> +	u8 temp[VMEM_SIZE];
> +
> +	if (*f_pos >= VMEM_SIZE)
> +		return -EFBIG;
> +	if (*f_pos + count > VMEM_SIZE)
> +		count = VMEM_SIZE - *f_pos;
> +	if (copy_from_user(temp, buf, count))
> +		return -EFAULT;
> +	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
> +		return -ERESTARTSYS;
> +	memcpy(sensehat_display->vmem + *f_pos, temp, count);

So we copy from buf into temp, from temp into vmem and then we 'transform' vmem 
into the representation of pixel_data.

As you are implementing the user-space interface, couldn't we just change the 
pixel representation there and get rid of the transformation?

> +	sensehat_update_display(sensehat);
> +	*f_pos += count;
> +	mutex_unlock(&sensehat_display->rw_mtx);
> +	return count;
> +}
> +
> +static long sensehat_display_ioctl(struct file *filp, unsigned int cmd,
> +			     unsigned long arg)
> +{
> +	struct sensehat *sensehat = container_of(filp->private_data, struct sensehat, display.mdev);
> +	struct sensehat_display *sensehat_display = &sensehat->display;
> +	void __user *user_ptr = (void __user *)arg;
> +	u8 temp[GAMMA_SIZE];
> +	int ret;
> +
> +	if (mutex_lock_interruptible(&sensehat_display->rw_mtx))
> +		return -ERESTARTSYS;
> +	switch (cmd) {
> +	case SENSEDISP_IOGET_GAMMA:
> +		if (copy_to_user(user_ptr, sensehat_display->gamma, GAMMA_SIZE)) {
> +			ret = -EFAULT;
> +			goto out_unlock;
> +		}
> +		ret = 0;
> +		goto out_unlock;
> +	case SENSEDISP_IOSET_GAMMA:
> +		if (copy_from_user(temp, user_ptr, GAMMA_SIZE)) {
> +			ret = -EFAULT;
> +			goto out_unlock;
> +		}
> +		ret = 0;
> +		goto out_update;

That's just a 'break;' correct?

Regards,
Matthias


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
  2021-11-03 16:33   ` Matthias Brugger
@ 2021-11-03 16:53     ` nsaenzju
  0 siblings, 0 replies; 18+ messages in thread
From: nsaenzju @ 2021-11-03 16:53 UTC (permalink / raw)
  To: Matthias Brugger, Charles Mirabile, linux-kernel
  Cc: Lee Jones, Serge Schneider, Stefan Wahren, linux-rpi-kernel,
	fedora-rpi, Mwesigwa Guma, Joel Savitz

On Wed, 2021-11-03 at 17:33 +0100, Matthias Brugger wrote:
> > +
> > +	sensehat->joystick.pdev = sensehat_client_dev_register(sensehat,
> > +							       "sensehat-joystick");
> 
> Why don't you use devm_mfd_add_devices function together with mfd_cell?

To complete Matthias' comment, why not use simple-mfd-i2c?

-- 
Nicolás Sáenz


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Raspberry Pi Sense HAT driver
  2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
                   ` (5 preceding siblings ...)
  2021-10-31 16:38 ` [PATCH 0/5] Raspberry Pi Sense HAT driver Miguel Ojeda
@ 2021-11-20 10:55 ` Stefan Wahren
  6 siblings, 0 replies; 18+ messages in thread
From: Stefan Wahren @ 2021-11-20 10:55 UTC (permalink / raw)
  To: Charles Mirabile, linux-kernel
  Cc: Serge Schneider, Nicolas Saenz Julienne, linux-rpi-kernel,
	fedora-rpi, Mwesigwa Guma, Joel Savitz

Hi,

Am 29.10.21 um 23:55 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 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
>
> For more information about the Sense HAT, visit:
> https://www.raspberrypi.org/products/sense-hat/
>
> Changes since v2:
> 	- We included a device tree binding in yaml form
> 	- Switched to regmap for all i2c communications
> 	- Fixed a few places where we had the old rpisense
> 	  name that we missed when renaming for v2

since this is V2 of the patch series, please mark this properly in all
subjects:

[PATCH V2 0/5] ...

Otherwise this is a nightmare for reviewers and maintainers.

Best regards


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-11-20 10:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 21:55 [PATCH 0/5] Raspberry Pi Sense HAT driver Charles Mirabile
2021-10-29 21:55 ` [PATCH 1/5] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
2021-10-30  8:02   ` kernel test robot
2021-10-30 17:20   ` Thomas Weißschuh
2021-11-03 16:33   ` Matthias Brugger
2021-11-03 16:53     ` nsaenzju
2021-10-29 21:55 ` [PATCH 2/5] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
2021-10-29 22:03   ` Randy Dunlap
2021-10-30 14:02   ` kernel test robot
2021-10-29 21:55 ` [PATCH 3/5] drivers/auxdisplay: senshat: Raspberry Pi Sense HAT display driver Charles Mirabile
2021-10-30 22:12   ` kernel test robot
2021-11-03 16:47   ` Matthias Brugger
2021-10-29 21:55 ` [PATCH 4/5] Documentation: bindings/mfd: sensehat: Raspberry Pi Sense HAT device tree binding Charles Mirabile
2021-11-02 12:51   ` Rob Herring
2021-10-29 21:55 ` [PATCH 5/5] MAINTAINERS: Add sensehat driver authors to MAINTAINERS Charles Mirabile
2021-10-31 16:38 ` [PATCH 0/5] Raspberry Pi Sense HAT driver Miguel Ojeda
2021-11-01 14:18   ` Joel Savitz
2021-11-20 10:55 ` Stefan Wahren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).