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 <[email protected]>
Signed-off-by: Mwesigwa Guma <[email protected]>
Signed-off-by: Joel Savitz <[email protected]>
---
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, ®);
+ 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);
+
+ 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 <[email protected]>");
+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
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 <[email protected]>
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/[email protected]
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
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 <[email protected]>
> Signed-off-by: Mwesigwa Guma <[email protected]>
> Signed-off-by: Joel Savitz <[email protected]>
> ---
> 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, ®);
> + 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);
> +
> + 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 <[email protected]>");
> +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
>
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