Messages in this thread | | | Subject | Re: [RFC PATCH 1/3] drivers/mfd: rpisense: Raspberry Pi senseHAT core driver | From | Stefan Wahren <> | Date | Sat, 7 Aug 2021 11:53:22 +0200 |
| |
Am 07.08.21 um 02:27 schrieb Joel Savitz: > This patch adds the core driver file, containing methods 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 | 10 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/rpisense-core.c | 170 +++++++++++++++++++++++++++++++++++ > include/linux/mfd/rpisense.h | 55 ++++++++++++ > 4 files changed, 236 insertions(+) > create mode 100644 drivers/mfd/rpisense-core.c > create mode 100644 include/linux/mfd/rpisense.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 6a3fd2d75f96..614de080dee6 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -11,6 +11,16 @@ config MFD_CORE > select IRQ_DOMAIN > default n > > +config MFD_RPISENSE RPI is not used before in config options. I suggest MFG_RASPBERRYPI_SENSE > + tristate "Raspberry Pi Sense HAT driver" > + depends on I2C && GPIOLIB > + select MFD_CORE > + help > + This is the driver for the Raspberry Pi Sense HAT. This provides > + the necessary functions to communicate with the hardware as well > + as a joystick and display interface. Linux communicates with the > + hardwire using the GPIO pins via the I2C protocol. > + > 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 8116c19d5fd4..76f9a9221241 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_RPISENSE) += rpisense-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/rpisense-core.c b/drivers/mfd/rpisense-core.c > new file mode 100644 > index 000000000000..69e3051a4be0 > --- /dev/null > +++ b/drivers/mfd/rpisense-core.c > @@ -0,0 +1,170 @@ > +// 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/mfd/rpisense.h> > + > +#define RPISENSE_DISPLAY 0x00 > +#define RPISENSE_WAI 0xF0 > +#define RPISENSE_VER 0xF1 > +#define RPISENSE_KEYS 0xF2 > +#define RPISENSE_EE_WP 0xF3 > + > +#define RPISENSE_ID 's' > + > +static struct platform_device * > +rpisense_client_dev_register(struct rpisense *rpisense, const char *name); > + > +static int rpisense_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + int ret; > + > + struct rpisense *rpisense = devm_kzalloc(&i2c->dev, sizeof(*rpisense), GFP_KERNEL); > + > + if (rpisense == NULL) > + return -ENOMEM; The more common style is: if (!rpisense) > + > + i2c_set_clientdata(i2c, rpisense); > + rpisense->dev = &i2c->dev; > + rpisense->i2c_client = i2c; > + > + > + ret = i2c_smbus_read_byte_data(rpisense->i2c_client, RPISENSE_WAI); > + if (ret < 0) > + return ret; > + > + if (ret != RPISENSE_ID) > + return -EINVAL; > + > + ret = i2c_smbus_read_byte_data(rpisense->i2c_client, RPISENSE_VER); > + if (ret < 0) > + return ret; Not sure about all the error cases, but i think the last two deserve a log entry. > + > + dev_info(rpisense->dev, > + "Raspberry Pi Sense HAT firmware version %i\n", ret); > + > + rpisense->joystick.pdev = rpisense_client_dev_register(rpisense, > + "rpi-sense-js"); > + > + if (IS_ERR(rpisense->joystick.pdev)) { > + dev_err(rpisense->dev, "failed to register rpisense-js"); > + return PTR_ERR(rpisense->joystick.pdev); > + } > + > + rpisense->display.pdev = rpisense_client_dev_register(rpisense, > + "rpi-sense-fb"); > + > + if (IS_ERR(rpisense->display.pdev)) { > + dev_err(rpisense->dev, "failed to register rpisense-fb"); > + return PTR_ERR(rpisense->display.pdev); > + } > + > + return 0; > +} > + > +static struct platform_device * > +rpisense_client_dev_register(struct rpisense *rpisense, const char *name) > +{ > + long ret = -ENOMEM; > + struct platform_device *pdev = platform_device_alloc(name, -1); > + > + if (pdev == NULL) > + goto alloc_fail; > + > + pdev->dev.parent = rpisense->dev; > + platform_set_drvdata(pdev, rpisense); > + > + ret = platform_device_add(pdev); > + if (ret != 0) Similiar as above: if (ret) > + goto add_fail; > + > + ret = devm_add_action_or_reset(rpisense->dev, > + (void *)platform_device_unregister, pdev); > + if (ret != 0) > + goto alloc_fail; > + > + return pdev; > + > +add_fail: > + platform_device_put(pdev); > +alloc_fail: > + return ERR_PTR(ret); > +} > + > +int rpisense_get_joystick_state(struct rpisense *rpisense) > +{ > + int ret = i2c_smbus_read_byte_data(rpisense->i2c_client, RPISENSE_KEYS); > + > + return ret < 0 ? ret : ret & 0x1f; > +} > +EXPORT_SYMBOL_GPL(rpisense_get_joystick_state); > + > +int rpisense_update_display(struct rpisense *rpisense) > +{ > + int i, j, ret; > + struct rpisense_display *display = &rpisense->display; > + struct {u8 reg, pixel_data[8][3][8]; } msg; > + > + msg.reg = RPISENSE_DISPLAY; > + for (i = 0; i < 8; ++i) { > + for (j = 0; j < 8; ++j) { > + msg.pixel_data[i][0][j] = display->gamma[display->vmem[i][j].r]; > + msg.pixel_data[i][1][j] = display->gamma[display->vmem[i][j].g]; > + msg.pixel_data[i][2][j] = display->gamma[display->vmem[i][j].b]; > + } > + } > + > + ret = i2c_master_send(rpisense->i2c_client, (u8 *)&msg, sizeof(msg)); > + if (ret < 0) > + dev_err(rpisense->dev, "Update to 8x8 LED matrix display failed"); > + return ret; > +} > +EXPORT_SYMBOL_GPL(rpisense_update_display); > + > +static const struct i2c_device_id rpisense_i2c_id[] = { > + { "rpi-sense", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, rpisense_i2c_id); > + > +#ifdef CONFIG_OF > +static const struct of_device_id rpisense_core_id[] = { > + { .compatible = "rpi,rpi-sense" },
The vendor is raspberrypi, which is already listed in Documentation <https://elixir.bootlin.com/linux/latest/source/Documentation>/devicetree <https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree>/bindings <https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings>/vendor-prefixes.yaml <https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/vendor-prefixes.yaml>
Not sure about the compatible. It must be added to the bindings, but unsure this should go to Documentation <https://elixir.bootlin.com/linux/latest/source/Documentation>/devicetree <https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree>/bindings <https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings>/trivial-devices.yaml <https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/trivial-devices.yaml> or a separate YAML file.
Best regards
| |