* [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
@ 2021-12-10 22:10 ` Charles Mirabile
2021-12-11 20:53 ` Thomas Weißschuh
` (2 more replies)
2021-12-10 22:10 ` [PATCH V5 2/6] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
` (5 subsequent siblings)
6 siblings, 3 replies; 15+ messages in thread
From: Charles Mirabile @ 2021-12-10 22:10 UTC (permalink / raw)
To: linux-kernel
Cc: Charles Mirabile, Lee Jones, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
linux-arm-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>
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
drivers/mfd/Kconfig | 8 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/sensehat-core.c | 157 +++++++++++++++++++++++++++++++++++
include/linux/mfd/sensehat.h | 51 ++++++++++++
4 files changed, 217 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 3fb480818599..e6de22f98c0e 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 0b1b629aef3e..2b012e3d497d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -263,6 +263,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..c5b6f4648d88
--- /dev/null
+++ b/drivers/mfd/sensehat-core.c
@@ -0,0 +1,157 @@
+// 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 <linux/mfd/sensehat.h>
+
+#define SENSEHAT_WAI 0xF0
+#define SENSEHAT_VER 0xF1
+
+#define SENSEHAT_ID 's'
+
+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, ®);
+ 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, ®);
+ 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);
+
+#ifdef CONFIG_JOYSTICK_SENSEHAT
+ 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);
+ }
+#endif
+#ifdef CONFIG_SENSEHAT_DISPLAY
+
+ 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);
+ }
+#endif
+
+ 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, PLATFORM_DEVID_AUTO);
+
+ 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 struct regmap_config sensehat_config = {
+ .name = "sensehat",
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static const struct i2c_device_id sensehat_i2c_id[] = {
+ { .name = "sensehat" },
+ {},
+};
+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..5d241dbed174
--- /dev/null
+++ b/include/linux/mfd/sensehat.h
@@ -0,0 +1,51 @@
+/* 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>
+
+#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;
+ } 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] 15+ messages in thread* Re: [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
2021-12-10 22:10 ` [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
@ 2021-12-11 20:53 ` Thomas Weißschuh
2021-12-12 11:43 ` Stefan Wahren
2021-12-13 11:49 ` Nicolas Saenz Julienne
2 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2021-12-11 20:53 UTC (permalink / raw)
To: Charles Mirabile
Cc: linux-kernel, Lee Jones, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
linux-arm-kernel, fedora-rpi, Mwesigwa Guma, Joel Savitz
Hi,
Currently this patch already contains some code that is only needed by the
patches adding the actual subdrivers.
I marked them below.
If you are resubmitting this code should be moved to the respective subdriver
patch.
On 2021-12-10 17:10-0500, 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>
> Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
> drivers/mfd/Kconfig | 8 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/sensehat-core.c | 157 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/sensehat.h | 51 ++++++++++++
> 4 files changed, 217 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 3fb480818599..e6de22f98c0e 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 0b1b629aef3e..2b012e3d497d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -263,6 +263,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..c5b6f4648d88
> --- /dev/null
> +++ b/drivers/mfd/sensehat-core.c
> @@ -0,0 +1,157 @@
> +// 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 <linux/mfd/sensehat.h>
> +
> +#define SENSEHAT_WAI 0xF0
> +#define SENSEHAT_VER 0xF1
> +
> +#define SENSEHAT_ID 's'
> +
> +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, ®);
> + 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, ®);
> + 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);
> +
> +#ifdef CONFIG_JOYSTICK_SENSEHAT
> + 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);
> + }
> +#endif
> +#ifdef CONFIG_SENSEHAT_DISPLAY
> +
> + 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);
> + }
> +#endif
These two ifdef blocks.
> +
> + 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, PLATFORM_DEVID_AUTO);
> +
> + 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 struct regmap_config sensehat_config = {
> + .name = "sensehat",
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static const struct i2c_device_id sensehat_i2c_id[] = {
> + { .name = "sensehat" },
> + {},
> +};
> +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..5d241dbed174
> --- /dev/null
> +++ b/include/linux/mfd/sensehat.h
> @@ -0,0 +1,51 @@
> +/* 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>
> +
> +#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)
These defines.
> +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;
> + } 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;
These two substructs.
> +};
> +
> +enum gamma_preset {
> + GAMMA_DEFAULT = 0,
> + GAMMA_LOWLIGHT,
> + GAMMA_PRESET_COUNT,
> +};
This enum.
> +
> +#endif
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
2021-12-10 22:10 ` [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
2021-12-11 20:53 ` Thomas Weißschuh
@ 2021-12-12 11:43 ` Stefan Wahren
2021-12-13 11:49 ` Nicolas Saenz Julienne
2 siblings, 0 replies; 15+ messages in thread
From: Stefan Wahren @ 2021-12-12 11:43 UTC (permalink / raw)
To: Charles Mirabile, linux-kernel
Cc: Lee Jones, Serge Schneider, Nicolas Saenz Julienne,
Mattias Brugger, linux-rpi-kernel, linux-arm-kernel, fedora-rpi,
Mwesigwa Guma, Joel Savitz
Am 10.12.21 um 23:10 schrieb Charles Mirabile:
> 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>
> Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
> drivers/mfd/Kconfig | 8 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/sensehat-core.c | 157 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/sensehat.h | 51 ++++++++++++
> 4 files changed, 217 insertions(+)
> create mode 100644 drivers/mfd/sensehat-core.c
> create mode 100644 include/linux/mfd/sensehat.h
>
...
> diff --git a/drivers/mfd/sensehat-core.c b/drivers/mfd/sensehat-core.c
> new file mode 100644
> index 000000000000..c5b6f4648d88
> --- /dev/null
> +++ b/drivers/mfd/sensehat-core.c
> @@ -0,0 +1,157 @@
> +// 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>
Is this header really necessary?
> +#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 <linux/mfd/sensehat.h>
> +
> +#define SENSEHAT_WAI 0xF0
> +#define SENSEHAT_VER 0xF1
> +
> +#define SENSEHAT_ID 's'
> +
> +static struct platform_device *
> +sensehat_client_dev_register(struct sensehat *sensehat, const char *name);
Please avoid this forward declaration
> +
> +static struct regmap_config sensehat_config;
Is there a specific reason, why the driver only supports one sense hat?
> +
> +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, ®);
> + if (ret < 0) {
> + dev_err(sensehat->dev, "failed to read from device");
I think the return code of regmap_read could be helpful in the log entry.
> + 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, ®);
> + 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);
> +
> +#ifdef CONFIG_JOYSTICK_SENSEHAT
> + 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);
> + }
> +#endif
Please replace the #ifdev blocks with IS_ENABLED() to increase compile
coverage
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver
2021-12-10 22:10 ` [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
2021-12-11 20:53 ` Thomas Weißschuh
2021-12-12 11:43 ` Stefan Wahren
@ 2021-12-13 11:49 ` Nicolas Saenz Julienne
2 siblings, 0 replies; 15+ messages in thread
From: Nicolas Saenz Julienne @ 2021-12-13 11:49 UTC (permalink / raw)
To: Charles Mirabile, linux-kernel
Cc: Lee Jones, Serge Schneider, Stefan Wahren, Mattias Brugger,
linux-rpi-kernel, linux-arm-kernel, fedora-rpi, Mwesigwa Guma,
Joel Savitz
Hi Charles,
On Fri, 2021-12-10 at 17:10 -0500, 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>
> Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
> drivers/mfd/Kconfig | 8 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/sensehat-core.c | 157 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/sensehat.h | 51 ++++++++++++
> 4 files changed, 217 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 3fb480818599..e6de22f98c0e 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 0b1b629aef3e..2b012e3d497d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -263,6 +263,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..c5b6f4648d88
> --- /dev/null
> +++ b/drivers/mfd/sensehat-core.c
> @@ -0,0 +1,157 @@
> +// 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 <linux/mfd/sensehat.h>
> +
> +#define SENSEHAT_WAI 0xF0
> +#define SENSEHAT_VER 0xF1
> +
> +#define SENSEHAT_ID 's'
> +
> +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, ®);
> + 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, ®);
> + 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);
I think both these sections and sensehat_client_dev_register() could be avoided
with a properly configured struct mfd_cell and a call to
devm_mfd_add_devices(). Actually, you're not making use of the mfd subsystem at
all in this driver, which is odd. For an example see 'drivers/mfd/mp2629.c'.
It's easy to get all these device registration operations wrong, so it's always
perfered to use the subsytem facilities when available.
> +
> +#ifdef CONFIG_JOYSTICK_SENSEHAT
> + 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);
> + }
> +#endif
> +#ifdef CONFIG_SENSEHAT_DISPLAY
> +
> + 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);
> + }
> +#endif
> +
> + 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, PLATFORM_DEVID_AUTO);
> +
> + 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 struct regmap_config sensehat_config = {
> + .name = "sensehat",
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static const struct i2c_device_id sensehat_i2c_id[] = {
> + { .name = "sensehat" },
> + {},
> +};
> +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");
--
Nicolás Sáenz
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V5 2/6] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
2021-12-10 22:10 ` [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
@ 2021-12-10 22:10 ` Charles Mirabile
2021-12-10 23:17 ` Dmitry Torokhov
2021-12-10 22:10 ` [PATCH V5 3/6] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver Charles Mirabile
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Charles Mirabile @ 2021-12-10 22:10 UTC (permalink / raw)
To: linux-kernel
Cc: Charles Mirabile, Dmitry Torokhov, linux-input, Serge Schneider,
Stefan Wahren, Nicolas Saenz Julienne, Mattias Brugger,
linux-rpi-kernel, linux-arm-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>
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
drivers/input/joystick/Kconfig | 8 ++
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/sensehat-joystick.c | 119 +++++++++++++++++++++
3 files changed, 128 insertions(+)
create mode 100644 drivers/input/joystick/sensehat-joystick.c
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 3b23078bc7b5..d5c5ffe13903 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 && I2C
+ 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..1f6bb09b4d1f
--- /dev/null
+++ b/drivers/input/joystick/sensehat-joystick.c
@@ -0,0 +1,119 @@
+// 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 <linux/mfd/sensehat.h>
+
+#define SENSEHAT_KEYS 0xF2
+
+static 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_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 = devm_request_threaded_irq(&pdev->dev, sensehat->i2c_client->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, ®);
+
+ 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] 15+ messages in thread* Re: [PATCH V5 2/6] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver
2021-12-10 22:10 ` [PATCH V5 2/6] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
@ 2021-12-10 23:17 ` Dmitry Torokhov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2021-12-10 23:17 UTC (permalink / raw)
To: Charles Mirabile
Cc: linux-kernel, linux-input, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
linux-arm-kernel, fedora-rpi, Mwesigwa Guma, Joel Savitz
Hi Charles,
On Fri, Dec 10, 2021 at 05:10:29PM -0500, 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.
This is not really a joystick, at least not in current incarnation, but
rather a set of keys. I'd also question whether the selected set is
right for the application, I'd look into BTN_DPAD_* maybe?
Or, if you want it to be a joystick, map ABS_X/ABS_Y.
>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
> drivers/input/joystick/Kconfig | 8 ++
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/sensehat-joystick.c | 119 +++++++++++++++++++++
> 3 files changed, 128 insertions(+)
> create mode 100644 drivers/input/joystick/sensehat-joystick.c
>
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 3b23078bc7b5..d5c5ffe13903 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 && I2C
I do not see I2C accessed directly in this driver, nor GPIO interface.
You also do not need to depend on INPUT explicitly here.
> + 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..1f6bb09b4d1f
> --- /dev/null
> +++ b/drivers/input/joystick/sensehat-joystick.c
> @@ -0,0 +1,119 @@
> +// 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>
Don't think you need this.
> +#include <linux/interrupt.h>
> +#include <linux/gpio/consumer.h>
Not needed.
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/sensehat.h>
> +
> +#define SENSEHAT_KEYS 0xF2
> +
> +static int sensehat_get_joystick_state(struct sensehat *sensehat);
> +
> +static unsigned char keymap[] = {
> + KEY_DOWN, KEY_RIGHT, KEY_UP, KEY_ENTER, KEY_LEFT,
> +};
Because you allowing manipulating keymap from userspace you need to make
a copy of it when setting up input device so that changes do would not
persist across bind/unbind.
This array needs to be marked const.
> +
> +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));
> + }
> + }
If you go with keys/buttons approach, then use
bitmap_xor()/for_each_set_bit() - see
drivers/input/keyboard/cypress-sf.c::cypress_sf_irq_handler() for
example of use.
> + input_sync(sensehat_joystick->keys_dev);
> + return IRQ_HANDLED;
> +}
> +
> +static int sensehat_joystick_probe(struct platform_device *pdev)
> +{
> + int ret;
Please call it "error";
> + int i;
> + struct sensehat *sensehat = dev_get_drvdata(&pdev->dev);
> + struct sensehat_joystick *sensehat_joystick = &sensehat->joystick;
This structure (struct sensehat_joystick) should probably be private to
this driver.
> +
> + 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 = devm_request_threaded_irq(&pdev->dev, sensehat->i2c_client->irq,
> + NULL, sensehat_joystick_report,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
Do not specify trigger type, whoever set up client's interrupt should
have set up appropriate trigger.
> + "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, ®);
> +
> + return ret < 0 ? ret : reg;
I do not see the point in having this wrapper.
> +}
> +
> +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
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V5 3/6] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
2021-12-10 22:10 ` [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
2021-12-10 22:10 ` [PATCH V5 2/6] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
@ 2021-12-10 22:10 ` Charles Mirabile
2021-12-11 18:38 ` Miguel Ojeda
2021-12-10 22:10 ` [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema Charles Mirabile
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Charles Mirabile @ 2021-12-10 22:10 UTC (permalink / raw)
To: linux-kernel
Cc: Charles Mirabile, Miguel Ojeda, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
linux-arm-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>
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
drivers/auxdisplay/Kconfig | 8 +
drivers/auxdisplay/Makefile | 1 +
drivers/auxdisplay/sensehat-display.c | 268 ++++++++++++++++++++++++++
3 files changed, 277 insertions(+)
create mode 100644 drivers/auxdisplay/sensehat-display.c
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 64012cda4d12..5b3a2feb34c9 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_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 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..ec88d64d41b6
--- /dev/null
+++ b/drivers/auxdisplay/sensehat-display.c
@@ -0,0 +1,268 @@
+// 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 <linux/mfd/sensehat.h>
+
+#define SENSEHAT_DISPLAY 0x00
+
+#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 = 0;
+ bool update = false;
+
+ 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;
+ break;
+ }
+ break;
+ case SENSEDISP_IOSET_GAMMA:
+ if (copy_from_user(temp, user_ptr, GAMMA_SIZE)) {
+ ret = -EFAULT;
+ break;
+ }
+ update = true;
+ break;
+ case SENSEDISP_IORESET_GAMMA:
+ if (arg < GAMMA_DEFAULT || arg >= GAMMA_PRESET_COUNT) {
+ ret = -EINVAL;
+ break;
+ }
+ memcpy(temp, gamma_presets[arg], GAMMA_SIZE);
+ update = true;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ if (update) {
+ memcpy(sensehat_display->gamma, temp, GAMMA_SIZE);
+ sensehat_update_display(sensehat);
+ }
+ 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;
+ u8 pixel_data[8][3][8];
+
+ 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] 15+ messages in thread* Re: [PATCH V5 3/6] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
2021-12-10 22:10 ` [PATCH V5 3/6] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver Charles Mirabile
@ 2021-12-11 18:38 ` Miguel Ojeda
0 siblings, 0 replies; 15+ messages in thread
From: Miguel Ojeda @ 2021-12-11 18:38 UTC (permalink / raw)
To: Charles Mirabile
Cc: linux-kernel, Miguel Ojeda, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
Linux ARM, fedora-rpi, Mwesigwa Guma, Joel Savitz
Hi Charles, Mwesigwa, Joel, Serge,
On Fri, Dec 10, 2021 at 11:11 PM Charles Mirabile <cmirabil@redhat.com> wrote:
>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
The "submitting author" should be the last one, i.e.:
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> +config SENSEHAT_DISPLAY
> + tristate "Raspberry pi Sense HAT display driver"
pi -> Pi
> +static int sensehat_update_display(struct sensehat *sensehat);
Can the function be directly defined instead?
> + if (*f_pos >= VMEM_SIZE)
> + return 0;
> + if (*f_pos + count > VMEM_SIZE)
> + count = VMEM_SIZE - *f_pos;
`min` / `min_t`?
> + if (ret < 0)
> + dev_err(sensehat->dev,
> + "Update to 8x8 LED matrix display failed");
Could this happen a lot of times? Is it expected to happen under some
condition or should never happen?
Cheers,
Miguel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
` (2 preceding siblings ...)
2021-12-10 22:10 ` [PATCH V5 3/6] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver Charles Mirabile
@ 2021-12-10 22:10 ` Charles Mirabile
2021-12-11 19:59 ` Rob Herring
2021-12-10 22:10 ` [PATCH V5 5/6] MAINTAINERS: Add sensehat driver authors to MAINTAINERS Charles Mirabile
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Charles Mirabile @ 2021-12-10 22:10 UTC (permalink / raw)
To: linux-kernel
Cc: Charles Mirabile, Lee Jones, Rob Herring, devicetree,
Serge Schneider, Stefan Wahren, Nicolas Saenz Julienne,
Mattias Brugger, linux-rpi-kernel, linux-arm-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>
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
.../bindings/mfd/raspberrypi,sensehat.yaml | 54 +++++++++++++++++++
1 file changed, 54 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..a57d1face50e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
@@ -0,0 +1,54 @@
+# 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:
+ const: raspberrypi,sensehat
+
+ reg:
+ items:
+ - description: i2c bus address
+
+ interrupts:
+ items:
+ - description: pin number for joystick interrupt
+
+ interrupt-parent:
+ items:
+ - description: gpio pin bank for interrupt pin
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-parent
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sensehat@46 {
+ compatible = "raspberrypi,sensehat";
+ reg = <0x46>;
+ interrupts = <23 GPIO_ACTIVE_HIGH>;
+ interrupt-parent = <&gpio>;
+ };
+ };
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
2021-12-10 22:10 ` [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema Charles Mirabile
@ 2021-12-11 19:59 ` Rob Herring
2021-12-13 17:09 ` Rob Herring
0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2021-12-11 19:59 UTC (permalink / raw)
To: Charles Mirabile
Cc: linux-kernel, Joel Savitz, Lee Jones, Mwesigwa Guma,
Nicolas Saenz Julienne, Serge Schneider, linux-arm-kernel,
linux-rpi-kernel, Mattias Brugger, fedora-rpi, Stefan Wahren,
devicetree, Rob Herring
On Fri, 10 Dec 2021 17:10:31 -0500, Charles Mirabile wrote:
> This patch adds the device tree binding
> for the Sense HAT in yaml form.
>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> ---
> .../bindings/mfd/raspberrypi,sensehat.yaml | 54 +++++++++++++++++++
> 1 file changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml:2:1: [error] missing document start "---" (document-start)
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml: properties:interrupt-parent: False schema does not allow {'items': [{'description': 'gpio pin bank for interrupt pin'}]}
from schema $id: http://devicetree.org/meta-schemas/interrupts.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml: ignoring, error in schema: properties: interrupt-parent
warning: no schema found in file: ./Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.example.dt.yaml:0:0: /example-0/i2c/sensehat@46: failed to match any schema with compatible: ['raspberrypi,sensehat']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1566669
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
2021-12-11 19:59 ` Rob Herring
@ 2021-12-13 17:09 ` Rob Herring
0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2021-12-13 17:09 UTC (permalink / raw)
To: Charles Mirabile
Cc: linux-kernel, Joel Savitz, Lee Jones, Mwesigwa Guma,
Nicolas Saenz Julienne, Serge Schneider, linux-arm-kernel,
linux-rpi-kernel, Mattias Brugger, fedora-rpi, Stefan Wahren,
devicetree
On Sat, Dec 11, 2021 at 01:59:31PM -0600, Rob Herring wrote:
> On Fri, 10 Dec 2021 17:10:31 -0500, Charles Mirabile wrote:
> > This patch adds the device tree binding
> > for the Sense HAT in yaml form.
> >
> > Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> > Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
> > Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
> > Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> > Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> > ---
> > .../bindings/mfd/raspberrypi,sensehat.yaml | 54 +++++++++++++++++++
> > 1 file changed, 54 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml:2:1: [error] missing document start "---" (document-start)
>
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml: properties:interrupt-parent: False schema does not allow {'items': [{'description': 'gpio pin bank for interrupt pin'}]}
> from schema $id: http://devicetree.org/meta-schemas/interrupts.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml: ignoring, error in schema: properties: interrupt-parent
> warning: no schema found in file: ./Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.example.dt.yaml:0:0: /example-0/i2c/sensehat@46: failed to match any schema with compatible: ['raspberrypi,sensehat']
'interrupt-parent' is not needed as it is always valid or could be in a
parent node.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V5 5/6] MAINTAINERS: Add sensehat driver authors to MAINTAINERS
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
` (3 preceding siblings ...)
2021-12-10 22:10 ` [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema Charles Mirabile
@ 2021-12-10 22:10 ` Charles Mirabile
2021-12-10 22:10 ` [PATCH V5 6/6] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4 Charles Mirabile
2021-12-13 11:29 ` [PATCH V5 0/6] Raspberry Pi Sense HAT driver Nicolas Saenz Julienne
6 siblings, 0 replies; 15+ messages in thread
From: Charles Mirabile @ 2021-12-10 22:10 UTC (permalink / raw)
To: linux-kernel
Cc: Charles Mirabile, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
linux-arm-kernel, fedora-rpi, Mwesigwa Guma, Joel Savitz
This patch adds the driver authors to MAINAINERS.
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
MAINTAINERS | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8691c531e297..8deb59e209e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17167,6 +17167,17 @@ 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: 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] 15+ messages in thread* [PATCH V5 6/6] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
` (4 preceding siblings ...)
2021-12-10 22:10 ` [PATCH V5 5/6] MAINTAINERS: Add sensehat driver authors to MAINTAINERS Charles Mirabile
@ 2021-12-10 22:10 ` Charles Mirabile
2021-12-13 11:29 ` [PATCH V5 0/6] Raspberry Pi Sense HAT driver Nicolas Saenz Julienne
6 siblings, 0 replies; 15+ messages in thread
From: Charles Mirabile @ 2021-12-10 22:10 UTC (permalink / raw)
To: linux-kernel
Cc: Charles Mirabile, Serge Schneider, Stefan Wahren,
Nicolas Saenz Julienne, Mattias Brugger, linux-rpi-kernel,
linux-arm-kernel, fedora-rpi, Mwesigwa Guma, Joel Savitz
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
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
Co-developed-by: Mwesigwa Guma <mguma@redhat.com>
Signed-off-by: Mwesigwa Guma <mguma@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
---
sensehat.dtbs | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 sensehat.dtbs
diff --git a/sensehat.dtbs b/sensehat.dtbs
new file mode 100644
index 000000000000..e087b7ba4a5a
--- /dev/null
+++ b/sensehat.dtbs
@@ -0,0 +1,44 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+ compatible = "brcm,bcm2835";
+};
+
+&i2c1 {
+ #address-cells = <0x01>;
+ #size-cells = <0x00>;
+ status = "okay";
+
+ sensehat@46 {
+ compatible = "raspberrypi,sensehat";
+ reg = <0x46>;
+ interrupts = <23 1>;
+ interrupt-parent = <&gpio>;
+ 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V5 0/6] Raspberry Pi Sense HAT driver
2021-12-10 22:10 [PATCH V5 0/6] Raspberry Pi Sense HAT driver Charles Mirabile
` (5 preceding siblings ...)
2021-12-10 22:10 ` [PATCH V5 6/6] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4 Charles Mirabile
@ 2021-12-13 11:29 ` Nicolas Saenz Julienne
6 siblings, 0 replies; 15+ messages in thread
From: Nicolas Saenz Julienne @ 2021-12-13 11:29 UTC (permalink / raw)
To: Charles Mirabile, linux-kernel
Cc: Serge Schneider, Stefan Wahren, Mattias Brugger,
linux-rpi-kernel, linux-arm-kernel, fedora-rpi, Mwesigwa Guma,
Joel Savitz
Hi Charles, Thanks for iterating on this.
On Fri, 2021-12-10 at 17:10 -0500, Charles Mirabile wrote:
> Suggestions from v4 not addressed yet:
> - Use mfd_cells for the joystick and display platform devices
> (as suggested by Matthias Brugger) or remove sensehat-core.c
> entirely and use simple-mfd-i2c instead (as suggested by
> Nicolás Sáenz Julienne).
> - while these suggestions might make great improvements
> for a version two of the driver they would require a
> large rewrite to move all of the complexity in core
> elsewhere. While certainly possible and something
> we want to iterate on and achieve eventually, we
> think saving it for down the road makes sense at this
> time.
Sorry for insisting, but I'm not convinced by this approach. The way you design
your driver affects its devicetree bindings, once we settle on those any
upgrade will have to be backwards compatible (i.e. an old DT would have to work
with the new implementation). This makes changing this further down the road
unlikely.
In terms of complexity, most of it is there due to not using the APIs and
facilities the mfd subsystem provides. See my comments in the driver.
Regards,
--
Nicolás Sáenz
^ permalink raw reply [flat|nested] 15+ messages in thread