* [PATCH v6 2/2] drm/panel: Add Ilitek ILI9488 controller driver
From: Igor Reznichenko @ 2026-06-17 14:32 UTC (permalink / raw)
To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Lad Prabhakar,
Manivannan Sadhasivam, Kael D'Alcamo, Kever Yang
Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20260617143245.372182-1-igor@reznichenko.net>
Add support for Ilitek ILI9488 DSI controller which is used in
FocusLCDs E35GH-I-MW800-CB 320x480 MIPI DSI panel. The mode timing
was adjusted after STM32MP157 testing. The previous 14.256 MHz mode
worked on AM62P, but was awkward for STM32 DSI/LTDC clock synthesis.
Signed-off-by: Igor Reznichenko <igor@reznichenko.net>
---
MAINTAINERS | 6 +
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9488.c | 289 +++++++++++++++++++
4 files changed, 305 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9488.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ac87c217ab1f..987635948cde 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8040,6 +8040,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
F: drivers/gpu/drm/tiny/ili9486.c
+DRM DRIVER FOR ILITEK ILI9488 PANELS
+M: Igor Reznichenko <igor@reznichenko.net>
+S: Maintained
+F: Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
+F: drivers/gpu/drm/panel/panel-ilitek-ili9488.c
+
DRM DRIVER FOR ILITEK ILI9805 PANELS
M: Michael Trimarchi <michael@amarulasolutions.com>
S: Maintained
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 7450b27622a2..37987e47f3f7 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -284,6 +284,15 @@ config DRM_PANEL_ILITEK_ILI9341
QVGA (240x320) RGB panels. support serial & parallel rgb
interface.
+config DRM_PANEL_ILITEK_ILI9488
+ tristate "Ilitek ILI9488-based panels"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y if you want to enable support for panels based on the
+ Ilitek ILI9488 controller.
+
config DRM_PANEL_ILITEK_ILI9805
tristate "Ilitek ILI9805-based panels"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index c2c5cf817116..a1344a0a3fd0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
obj-$(CONFIG_DRM_PANEL_HYDIS_HV101HD1) += panel-hydis-hv101hd1.o
obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
+obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9488) += panel-ilitek-ili9488.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9805) += panel-ilitek-ili9805.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E_CORE) += panel-ilitek-ili9806e-core.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E_DSI) += panel-ilitek-ili9806e-dsi.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9488.c b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
new file mode 100644
index 000000000000..7302766034fc
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+#include <video/mipi_display.h>
+
+struct ili9488_desc {
+ const struct drm_display_mode *display_mode;
+ unsigned long mode_flags;
+ enum mipi_dsi_pixel_format format;
+ unsigned int lanes;
+ unsigned int bpc;
+ void (*init_sequence)(struct mipi_dsi_multi_context *ctx);
+};
+
+struct ili9488 {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct gpio_desc *reset;
+ struct regulator_bulk_data supplies[2];
+ const struct ili9488_desc *desc;
+ enum drm_panel_orientation orientation;
+};
+
+static const char * const regulator_names[] = {
+ "vci",
+ "iovcc",
+};
+
+static void e35gh_i_mw800cb_init(struct mipi_dsi_multi_context *ctx)
+{
+ /* Gamma control 1,2 */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xE0, 0x00, 0x10, 0x14, 0x01, 0x0E, 0x04, 0x33,
+ 0x56, 0x48, 0x03, 0x0C, 0x0B, 0x2B, 0x34, 0x0F);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xE1, 0x00, 0x12, 0x18, 0x05, 0x12, 0x06, 0x40,
+ 0x34, 0x57, 0x06, 0x10, 0x0C, 0x3B, 0x3F, 0x0F);
+ /* Power control 1,2 */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xC0, 0x0F, 0x0C);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xC1, 0x41);
+ /* VCOM Control */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xC5, 0x00, 0x25, 0x80);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0x36, 0x48);
+ /* Interface pixel format 18bpp */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0x3A, 0x66);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB0, 0x00);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB1, 0xA0);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB4, 0x02);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB6, 0x02, 0x02, 0x3B);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xE9, 0x00);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xF7, 0xA9, 0x51, 0x2C, 0x82);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0x21);
+}
+
+static const struct drm_display_mode e35gh_i_mw800cb_display_mode = {
+ .clock = 14400,
+
+ .hdisplay = 320,
+ .hsync_start = 320 + 60,
+ .hsync_end = 320 + 60 + 20,
+ .htotal = 320 + 60 + 20 + 42,
+
+ .vdisplay = 480,
+ .vsync_start = 480 + 20,
+ .vsync_end = 480 + 20 + 10,
+ .vtotal = 480 + 20 + 10 + 33,
+
+ .width_mm = 48,
+ .height_mm = 73,
+
+ .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+ .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+};
+
+static inline struct ili9488 *panel_to_ili9488(struct drm_panel *panel)
+{
+ return container_of(panel, struct ili9488, panel);
+}
+
+static int ili9488_power_on(struct ili9488 *ili)
+{
+ struct mipi_dsi_device *dsi = ili->dsi;
+ int ret;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(ili->supplies), ili->supplies);
+ if (ret < 0) {
+ dev_err(&dsi->dev, "regulator bulk enable failed: %d\n", ret);
+ return ret;
+ }
+
+ gpiod_set_value_cansleep(ili->reset, 0);
+ usleep_range(1000, 5000);
+ gpiod_set_value_cansleep(ili->reset, 1);
+ usleep_range(1000, 5000);
+ gpiod_set_value_cansleep(ili->reset, 0);
+ usleep_range(5000, 10000);
+
+ return 0;
+}
+
+static int ili9488_power_off(struct ili9488 *ili)
+{
+ struct mipi_dsi_device *dsi = ili->dsi;
+ int ret;
+
+ gpiod_set_value_cansleep(ili->reset, 1);
+
+ ret = regulator_bulk_disable(ARRAY_SIZE(ili->supplies), ili->supplies);
+ if (ret)
+ dev_err(&dsi->dev, "regulator bulk disable failed: %d\n", ret);
+
+ return ret;
+}
+
+static int ili9488_activate(struct ili9488 *ili)
+{
+ struct mipi_dsi_multi_context ctx = { .dsi = ili->dsi };
+
+ if (ili->desc->init_sequence)
+ ili->desc->init_sequence(&ctx);
+
+ mipi_dsi_dcs_exit_sleep_mode_multi(&ctx);
+ mipi_dsi_msleep(&ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&ctx);
+
+ return ctx.accum_err;
+}
+
+static int ili9488_prepare(struct drm_panel *panel)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+ int ret;
+
+ ret = ili9488_power_on(ili);
+ if (ret)
+ return ret;
+
+ ret = ili9488_activate(ili);
+ if (ret) {
+ ili9488_power_off(ili);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ili9488_deactivate(struct ili9488 *ili)
+{
+ struct mipi_dsi_multi_context ctx = { .dsi = ili->dsi };
+
+ mipi_dsi_dcs_set_display_off_multi(&ctx);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&ctx);
+ mipi_dsi_msleep(&ctx, 120);
+
+ return ctx.accum_err;
+}
+
+static int ili9488_unprepare(struct drm_panel *panel)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+ struct mipi_dsi_device *dsi = ili->dsi;
+ int ret;
+
+ ili9488_deactivate(ili);
+ ret = ili9488_power_off(ili);
+ if (ret < 0)
+ dev_err(&dsi->dev, "power off failed: %d\n", ret);
+
+ return ret;
+}
+
+static int ili9488_get_modes(struct drm_panel *panel, struct drm_connector *connector)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+ const struct drm_display_mode *mode = ili->desc->display_mode;
+
+ connector->display_info.bpc = ili->desc->bpc;
+
+ return drm_connector_helper_get_modes_fixed(connector, mode);
+}
+
+static enum drm_panel_orientation ili9488_get_orientation(struct drm_panel *panel)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+
+ return ili->orientation;
+}
+
+static const struct drm_panel_funcs ili9488_funcs = {
+ .prepare = ili9488_prepare,
+ .unprepare = ili9488_unprepare,
+ .get_modes = ili9488_get_modes,
+ .get_orientation = ili9488_get_orientation,
+};
+
+static int ili9488_dsi_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct ili9488 *ili;
+ int i, ret;
+
+ ili = devm_drm_panel_alloc(dev, struct ili9488, panel, &ili9488_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ if (IS_ERR(ili))
+ return PTR_ERR(ili);
+
+ ili->desc = device_get_match_data(dev);
+ ili->dsi = dsi;
+
+ dsi->mode_flags = ili->desc->mode_flags;
+ dsi->format = ili->desc->format;
+ dsi->lanes = ili->desc->lanes;
+
+ ili->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(ili->reset))
+ return dev_err_probe(dev, PTR_ERR(ili->reset),
+ "failed to get reset-gpios\n");
+
+ for (i = 0; i < ARRAY_SIZE(ili->supplies); i++)
+ ili->supplies[i].supply = regulator_names[i];
+
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ili->supplies),
+ ili->supplies);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+ ret = of_drm_get_panel_orientation(dev->of_node, &ili->orientation);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get orientation\n");
+
+ ret = drm_panel_of_backlight(&ili->panel);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get backlight\n");
+
+ ili->panel.prepare_prev_first = true;
+
+ ret = devm_drm_panel_add(dev, &ili->panel);
+ if (ret)
+ return ret;
+
+ ret = devm_mipi_dsi_attach(dev, dsi);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to attach to DSI host\n");
+
+ return 0;
+}
+
+static const struct ili9488_desc e35gh_i_mw800cb_desc = {
+ .init_sequence = e35gh_i_mw800cb_init,
+ .display_mode = &e35gh_i_mw800cb_display_mode,
+ .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS,
+ .format = MIPI_DSI_FMT_RGB666_PACKED,
+ .lanes = 1,
+ .bpc = 6,
+};
+
+static const struct of_device_id ili9488_of_match[] = {
+ { .compatible = "focuslcds,e35gh-i-mw800cb", .data = &e35gh_i_mw800cb_desc },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, ili9488_of_match);
+
+static struct mipi_dsi_driver ili9488_dsi_driver = {
+ .probe = ili9488_dsi_probe,
+ .driver = {
+ .name = "ili9488-dsi",
+ .of_match_table = ili9488_of_match,
+ },
+};
+module_mipi_dsi_driver(ili9488_dsi_driver);
+
+MODULE_AUTHOR("Igor Reznichenko <igor@reznichenko.net>");
+MODULE_DESCRIPTION("Ilitek ILI9488 Controller Driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v6 2/2] drm/panel: Add Ilitek ILI9488 controller driver
From: sashiko-bot @ 2026-06-17 14:44 UTC (permalink / raw)
To: Igor Reznichenko; +Cc: dri-devel, devicetree, robh, conor+dt
In-Reply-To: <20260617143245.372182-3-igor@reznichenko.net>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for the result of device_get_match_data() leads to a kernel oops.
- [High] Undefined function call and missing DRM panel unregistration leading to Use-After-Free.
--
commit c22f05cd630ecc7dace66102729e2ea4cdfd93fc
Author: Igor Reznichenko <igor@reznichenko.net>
drm/panel: Add Ilitek ILI9488 controller driver
This commit adds support for the Ilitek ILI9488 DSI controller.
It introduces power sequencing and DSI initialization tailored
for the FocusLCDs E35GH-I-MW800-CB panel.
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9488.c b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
> new file mode 100644
> index 0000000000000..7302766034fc9
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
[ ... ]
> +static int ili9488_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = &dsi->dev;
> + struct ili9488 *ili;
> + int i, ret;
> +
> + ili = devm_drm_panel_alloc(dev, struct ili9488, panel, &ili9488_funcs,
> + DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(ili))
> + return PTR_ERR(ili);
> +
> + ili->desc = device_get_match_data(dev);
> + ili->dsi = dsi;
> +
> + dsi->mode_flags = ili->desc->mode_flags;
[Severity: High]
Can this dereference a NULL pointer if the device is instantiated without
matching firmware data?
The function device_get_match_data() can return NULL, for example via sysfs
or string-based matching. Verifying the return value before accessing
ili->desc->mode_flags would prevent a potential NULL pointer dereference.
[ ... ]
> + ret = drm_panel_of_backlight(&ili->panel);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get backlight\n");
> +
> + ili->panel.prepare_prev_first = true;
> +
> + ret = devm_drm_panel_add(dev, &ili->panel);
[Severity: High]
Does the devm_drm_panel_add() function exist in the kernel, or will this
cause a build failure?
Additionally, if this call is replaced with drm_panel_add(), the driver
lacks a corresponding .remove() callback or a devres action to unregister
the panel with drm_panel_remove(). Since the panel structure is allocated
with devm_drm_panel_alloc() and freed on driver unbind, failing to unregister
the panel will leave it on the DRM core's global panel list, leading to a
Use-After-Free when the list is later accessed.
> + if (ret)
> + return ret;
> +
> + ret = devm_mipi_dsi_attach(dev, dsi);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to attach to DSI host\n");
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617143245.372182-1-igor@reznichenko.net?part=2
^ permalink raw reply
* Re: (subset) [PATCH v2 2/2] leds: pwm-multicolor: Introduce default-intensity property
From: Lee Jones @ 2026-06-17 14:47 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jacek Anaszewski, Sven Schwermer, Jonas Rebmann
Cc: Pavel Machek, linux-leds, devicetree, linux-kernel, kernel
In-Reply-To: <20260605-multicolor-default-v2-2-ed07271df6b0@pengutronix.de>
On Fri, 05 Jun 2026 09:27:48 +0200, Jonas Rebmann wrote:
> Like all LED drivers, pwm-multicolor supports turning on an LED on boot
> by setting linux,default-trigger, e.g. to "default-on". pwm-multicolor
> however scales the brightness of the color-component sub-LEDs with their
> individual intensity value. Since these intensities are
> zero-initialized, on boot a trigger is invisible until colors are set
> from userspace.
>
> [...]
Applied, thanks!
[2/2] leds: pwm-multicolor: Introduce default-intensity property
commit: 69dd3b9d8681085eceeb35459a12d67b848372d4
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
From: Frank Li @ 2026-06-17 14:50 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, linux-i3c, robh, conor+dt, Frank.Li, devicetree
In-Reply-To: <20260617111744.1A0971F000E9@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:17:43AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Concurrent regulator operations race on the shared device-level protection register, leading to silent hardware configuration failures.
> - [Medium] The driver module will not auto-load because it lacks a MODULE_ALIAS or platform_device_id table.
> - [Medium] If regmap_read or the unlock sequence fails in the guard constructor, the driver proceeds with the regulator operation on locked registers.
> - [Low] Typo in regulator config assignment assigns dev->of_node to itself instead of rcfg.of_node.
> --
>
> commit e68081b58005a2422d441a5611dac29f8cffe2f4
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
>
> The NXP P3H2x4x family integrates on-die regulators alongside I3C hub
> functionality. This driver registers the regulators using the MFD
> framework and exposes them via the regulator subsystem.
>
> > --- /dev/null
> > +++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
>
> [ ... ]
>
> > +static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev,
> > + struct p3h2x4x_reg_state *state)
> > +{
> > + state->restore = false;
> > +
> > + if (regmap_read(rdev->regmap,
> > + P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + &state->orig))
> > + return;
>
> [Severity: Medium]
> If regmap_read() fails here, the function returns early and swallows the
> error since the guard constructor returns void. Will this cause the caller
> to proceed with regulator operations on locked hardware, potentially
> resulting in silent hardware configuration failures?
>
> If the device ACKs the transaction despite ignoring the write, wouldn't
> the driver incorrectly report success to the regulator core?
>
> > +
> > + if (state->orig != P3H2X4X_REGISTERS_UNLOCK_CODE) {
> > + regmap_write(rdev->regmap,
> > + P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + P3H2X4X_REGISTERS_UNLOCK_CODE);
> > + state->restore = true;
> > + }
> > +}
> > +
> > +static void p3h2x4x_reg_guard_exit(struct regulator_dev *rdev,
> > + struct p3h2x4x_reg_state *state)
> > +{
> > + if (state->restore)
> > + regmap_write(rdev->regmap,
> > + P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + state->orig);
> > +}
> > +
> > +DEFINE_LOCK_GUARD_1(p3h2x4x_reg, struct regulator_dev,
> > + p3h2x4x_reg_guard_enter(_T->lock, &_T->state),
> > + p3h2x4x_reg_guard_exit(_T->lock, &_T->state),
> > + struct p3h2x4x_reg_state state);
>
> [Severity: High]
> Does this guard provide mutual exclusion across the four different regulators
> on the same device?
>
> The regmap lock protects individual I/O operations, but since there is no
> device-level lock held across the unlock/modify/lock sequence, can concurrent
> operations on different regulators interleave?
Look like make sense, need use mutex to protect this sequency.
>
> For example, could one thread re-lock the device in p3h2x4x_reg_guard_exit()
> while another thread is just about to write its configuration, causing the
> hardware write to be silently ignored? Is a shared device-level mutex needed
> to serialize these sequences?
>
> [ ... ]
>
> > +static int p3h2x4x_regulator_probe(struct platform_device *pdev)
> > +{
> > + struct p3h2x4x_dev *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
> > + struct p3h2x4x_regulator_dev *p3h2x4x_regulator;
> > + struct regulator_config rcfg = { };
> > + struct device *dev = &pdev->dev;
> > + struct regulator_dev *rdev;
> > + int i;
> > +
> > + p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
> > + if (!p3h2x4x_regulator)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, p3h2x4x_regulator);
> > +
> > + p3h2x4x_regulator->regmap = p3h2x4x->regmap;
> > + device_set_of_node_from_dev(dev, dev->parent);
> > +
> > + rcfg.dev = dev;
> > + rcfg.dev->of_node = dev->of_node;
>
> [Severity: Low]
> Should this be rcfg.of_node = dev->of_node? Since rcfg.dev is assigned to dev
> on the previous line, this just assigns dev->of_node to itself.
Although it is small problem, look likes not necesary to set of_node here.
It'd better to fix it at next version.
Frank
>
> > + rcfg.regmap = p3h2x4x_regulator->regmap;
> > + rcfg.driver_data = p3h2x4x_regulator;
>
> [ ... ]
>
> > +static struct platform_driver p3h2x4x_regulator_driver = {
> > + .driver = {
> > + .name = "p3h2x4x-regulator",
> > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > + },
> > + .probe = p3h2x4x_regulator_probe,
> > +};
> > +module_platform_driver(p3h2x4x_regulator_driver);
> > +
> > +MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@nxp.com>");
> > +MODULE_AUTHOR("Vikash Bansal <vikash.bansal@nxp.com>");
> > +MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
> > +MODULE_DESCRIPTION("NXP P3H2X4X I3C HUB Regulator driver");
> > +MODULE_LICENSE("GPL");
>
> [Severity: Medium]
> Since the driver is registered as a platform driver with the name
> "p3h2x4x-regulator" but the module is compiled as
> "p3h2840_i3c_hub_regulator.ko", will this module fail to auto-load?
>
> Without a MODULE_ALIAS("platform:p3h2x4x-regulator") or an exported
> platform_device_id table, udev may not be able to load the module when the
> MFD core creates the child platform device.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=4
^ permalink raw reply
* Re: [PATCH v8 2/5] iio: adc: add Versal SysMon driver
From: Andy Shevchenko @ 2026-06-17 14:52 UTC (permalink / raw)
To: Salih Erim
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260616131559.3029543-3-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:56PM +0100, Salih Erim wrote:
> Add the core driver and MMIO platform driver for the AMD/Xilinx Versal
> System Monitor (SysMon) block.
>
> The SysMon block resides in the platform management controller (PMC) and
> provides on-chip voltage and temperature monitoring through a 10-bit,
> 200 kSPS ADC. It can monitor up to 160 voltage channels and 64
> temperature satellites distributed across the SoC, with a consistent
> sample rate of 8 kSPS per channel regardless of how many channels are
> enabled.
>
> The hardware also provides four aggregate temperature registers that
> are always present regardless of the device tree configuration: the
> current max and min across all active satellites, and the peak and
> trough values recorded since the last hardware reset.
>
> The driver is split into two compilation units:
> - versal-sysmon-core: Channel parsing, IIO registration, read_raw
> - versal-sysmon: MMIO platform driver with custom regmap accessors
>
> Voltage results are stored in a 19-bit modified floating-point format
> and converted to millivolts. Temperature results are stored in Q8.7
> signed fixed-point Celsius format and converted to millicelsius.
>
> The MMIO regmap backend uses a custom reg_write accessor that
> automatically unlocks the NPI (NoC programming interface) lock
> register before each write, as required by the hardware. The regmap
> is configured with fast_io since the underlying MMIO accessors are
> safe to call from atomic context.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
with a caveat of using namespaced exports (see below).
...
> +EXPORT_SYMBOL_GPL(devm_versal_sysmon_core_probe);
Please, also use symbol namespace.
...
> +static int sysmon_mmio_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
Make it a single line. It will be more readable.
static int sysmon_mmio_reg_read(void *context, unsigned int reg, unsigned int *val)
...
> +static int sysmon_mmio_reg_write(void *context, unsigned int reg,
> + unsigned int val)
In the similar way.
static int sysmon_mmio_reg_write(void *context, unsigned int reg, unsigned int val)
In both cases it's only 83 characters.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
From: Frank Li @ 2026-06-17 14:52 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, robh, conor+dt, devicetree, linux-i3c, Frank.Li
In-Reply-To: <20260617111331.7AB671F000E9@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:13:31AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] The binding documents GPIO capability for target ports but omits the standard GPIO provider properties, violating subsystem guidelines.
> - [Medium] The binding fails to include compatible strings for the 4-port variants and unconditionally allows up to 8 ports.
> - [Low] The unit address in the I3C example does not match its `reg` property.
> --
>
> commit af3b77595fece3e15864b37b9b683be835a0e1eb
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
>
> This commit adds device tree bindings for the NXP P3H2x4x multi-port I3C hub
> family. It supports connecting a host via I3C/I2C/SMBus to multiple downstream
> peripherals across up to 8 ports.
>
> > diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> > new file mode 100644
> > index 0000000000000..c080eeb0eeaa8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> > @@ -0,0 +1,291 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2025 NXP
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i3c/nxp,p3h2840.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP P3H2X4X I3C HUB
> > +
> > +maintainers:
> > + - Aman Kumar Pandey <aman.kumarpandey@nxp.com>
> > + - Vikash Bansal <vikash.bansal@nxp.com>
> > + - Lakshay Piplani <lakshay.piplani@nxp.com>
> > +
> > +description: |
> > + P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841) is a family of multiport I3C
> > + hub devices that connect to:-
> > + 1. A host CPU via I3C/I2C/SMBus bus on upstream side and connect to multiple
> > + peripheral devices on the downstream side.
> > + 2. Have two Controller Ports which can support either
> > + I2C/SMBus or I3C buses and connect to a CPU, BMC or SOC.
> > + 3. P3H2840/ P3H2841 are 8 port I3C hub with eight I3C/I2C Target Port.
> > + 4. P3H2440/ P3H2441 are 4 port I3C hub with four I3C/I2C Target Port.
> > + Target ports can be configured as I2C/SMBus, I3C or GPIO and connect to
> > + peripherals.
>
> [Severity: Medium]
> Since the description advertises that target ports can be configured as GPIO,
> should this binding include the standard GPIO provider properties like
> gpio-controller and #gpio-cells?
>
> Because unevaluatedProperties is false and there are no child node patterns
> defined for GPIO ports in patternProperties, won't adding a GPIO child node
> fail schema validation?
>
> > +
> > +properties:
> > + compatible:
> > + const: nxp,p3h2840
>
> [Severity: Medium]
> The commit message and description state the binding supports the P3H2440
> and P3H2441 4-port hubs alongside the 8-port P3H2840/P3H2841.
>
> Should compatible strings be included for the 4-port variants here as well?
>
> [ ... ]
>
> > +patternProperties:
> > + "^i3c@[0-7]$":
> > + type: object
> > + $ref: /schemas/i3c/i3c.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + reg:
> > + description:
> > + The I3C HUB Target Port number.
> > + maximum: 7
>
> [Severity: Medium]
> Does unconditionally allowing a maximum port index of 7 cause issues for the
> 4-port variants mentioned in the description?
>
> It seems this allows invalid configurations that define non-existent ports
> (4-7) for the 4-port hardware.
>
> [ ... ]
>
> > +examples:
> > + - |
> > + i3c {
> > + #address-cells = <3>;
> > + #size-cells = <0>;
> > +
> > + hub@70,236153000c2 {
> > + reg = <0x70 0x236 0x3000c2>;
>
> [Severity: Low]
> Does the unit address in this example match its reg property?
>
> The reg property combines to a provisioned ID of 0x236003000c2, which appears
> to differ from 236153000c2 in the unit address. Could this mismatch trigger a
> simple_bus_reg warning during make dt_binding_check?
I remember I3C mask 0x15, it'd better kept node name consisent with reg
Frank
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v8 3/5] iio: adc: versal-sysmon: add I2C driver
From: Andy Shevchenko @ 2026-06-17 14:53 UTC (permalink / raw)
To: Salih Erim
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260616131559.3029543-4-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:57PM +0100, Salih Erim wrote:
> Add an I2C transport driver for the Versal SysMon block. The SysMon
> provides an I2C slave interface that allows an external master to
> read voltage and temperature measurements through the same register
> map used by the MMIO path.
>
> The I2C command frame is an 8-byte structure containing a 4-byte data
> payload, a 2-byte register offset, and a 1-byte instruction field.
> Read operations send the frame with a read instruction, then receive
> a 4-byte response containing the register value.
>
> Events are not supported on the I2C path because there is no
> interrupt line and the I2C regmap backend cannot be called from
> atomic context.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] dt-bindings: sound: add toshiba,apb-dummy-codec binding
From: Pablo D. Bergamasco @ 2026-06-17 14:55 UTC (permalink / raw)
To: krzk
Cc: broonie, conor+dt, danpablo, devicetree, krzk+dt, lgirdwood,
linux-kernel, linux-sound, mgreer, robh, vaibhav.sr
In-Reply-To: <20260617-marvellous-cunning-curassow-0113ab@quoll>
On Wed, Jun 17, 2026, Krzysztof Kozlowski wrote:
> Nope. We don't take bindings for staging. Isn't this documented in
> staging docs already?
I checked drivers/staging/greybus/Documentation/ and found only
firmware and sysfs documentation. There is no existing DT binding
documentation for the "toshiba,apb-dummy-codec" compatible string.
Should I add the binding documentation within the staging directory
itself, or is there a preferred approach for staging drivers?
Best regards,
Pablo D. Bergamasco
^ permalink raw reply
* [PATCH v4 00/10] Input: cap11xx - Add support for CAP1114
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
Patches 1-4 perform driver cleanup and DT binding tweaks.
Patches 5-6 add reset-gpios support for CAP11xx.
Patches 7-10 add support for CAP1114.
Changes in v4:
- Revert cleanup operation for CAP11XX_REG_SENSOR_DELTA.
- Move the LED unit-address update operation to the CAP1114 support patch.
- Limit the CAP1114 LED unit-address range from 0x0 to 0xa.
- Link to v3:
https://lore.kernel.org/all/20260615142103.352163-1-jerrysteve1101@gmail.com/
Changes in v3:
- Simplified the logic of the reset pin operation.
- Adjust linux,keycodes configuration for CAP11xx.
- Drop unnecessary CAP11XX_REG_SENSOR_THRESH(8).
- Checks for the presence of microchip,calib-sensitivity and
microchip,signal-guard properties before processing them.
- Link to v2:
https://lore.kernel.org/all/20260612072237.1177304-1-jerrysteve1101@gmail.com/
Changes in v2:
- Drop LED property tweaks, keep only reg changes and node regex
update in DT bindings.
- Split microchip,cap1126 LED reg constraints into a separate patch.
- Replace usleep_range() with msleep() for 500 ms delay during
reset pin handling.
- Add missing <linux/delay.h> for usleep_range() and msleep().
- Add CAP1114 to unsupported enum for microchip,signal-guard and
microchip,calib-sensitivity
- Add constraint for linux,keycodes to support CAP1114.
- When reading CAP1114 button status, mask STATUS1 to bits 0-5
and OR with STATUS2.
- Adjust code style.
- Link to v1:
https://lore.kernel.org/all/20260606150458.250606-1-jerrysteve1101@gmail.com
Jun Yan (10):
Input: cap11xx - clean up duplicate log and add probe error logs
Input: cap11xx - remove unused register macros
dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED
reg range
dt-bindings: input: microchip,cap11xx: Add microchip,cap1126 LED reg
constraints
dt-bindings: input: microchip,cap11xx: Add reset-gpios property
Input: cap11xx - add reset gpio support
Input: cap11xx - refactor code for better CAP1114 support.
Input: cap11xx - guard unsupported DT properties before parsing
dt-bindings: input: microchip,cap11xx: Add CAP1114 support
Input: cap11xx - add support for CAP1114
.../bindings/input/microchip,cap11xx.yaml | 90 +++++-
drivers/input/keyboard/cap11xx.c | 274 +++++++++++-------
2 files changed, 254 insertions(+), 110 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH v4 02/10] Input: cap11xx - remove unused register macros
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Remove unused register address macros and unused definitions in
the cap11xx_reg_defaults array and cap11xx_volatile_reg.
This cleanup reduces code clutter and makes the driver easier to
maintain without affecting functionality.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 51 --------------------------------
1 file changed, 51 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 485d8ba97723..fae26f035186 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -20,53 +20,24 @@
#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
#define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
#define CAP11XX_REG_MAIN_CONTROL_DLSEEP BIT(4)
-#define CAP11XX_REG_GENERAL_STATUS 0x02
#define CAP11XX_REG_SENSOR_INPUT 0x03
-#define CAP11XX_REG_NOISE_FLAG_STATUS 0x0a
#define CAP11XX_REG_SENOR_DELTA(X) (0x10 + (X))
#define CAP11XX_REG_SENSITIVITY_CONTROL 0x1f
#define CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK 0x70
-#define CAP11XX_REG_CONFIG 0x20
-#define CAP11XX_REG_SENSOR_ENABLE 0x21
-#define CAP11XX_REG_SENSOR_CONFIG 0x22
-#define CAP11XX_REG_SENSOR_CONFIG2 0x23
-#define CAP11XX_REG_SAMPLING_CONFIG 0x24
-#define CAP11XX_REG_CALIBRATION 0x26
-#define CAP11XX_REG_INT_ENABLE 0x27
#define CAP11XX_REG_REPEAT_RATE 0x28
#define CAP11XX_REG_SIGNAL_GUARD_ENABLE 0x29
-#define CAP11XX_REG_MT_CONFIG 0x2a
-#define CAP11XX_REG_MT_PATTERN_CONFIG 0x2b
-#define CAP11XX_REG_MT_PATTERN 0x2d
-#define CAP11XX_REG_RECALIB_CONFIG 0x2f
#define CAP11XX_REG_SENSOR_THRESH(X) (0x30 + (X))
-#define CAP11XX_REG_SENSOR_NOISE_THRESH 0x38
-#define CAP11XX_REG_STANDBY_CHANNEL 0x40
-#define CAP11XX_REG_STANDBY_CONFIG 0x41
-#define CAP11XX_REG_STANDBY_SENSITIVITY 0x42
-#define CAP11XX_REG_STANDBY_THRESH 0x43
#define CAP11XX_REG_CONFIG2 0x44
#define CAP11XX_REG_CONFIG2_ALT_POL BIT(6)
-#define CAP11XX_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
-#define CAP11XX_REG_LED_POLARITY 0x73
#define CAP11XX_REG_LED_OUTPUT_CONTROL 0x74
#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG 0x80
#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2 0x81
-
-#define CAP11XX_REG_LED_DUTY_CYCLE_1 0x90
-#define CAP11XX_REG_LED_DUTY_CYCLE_2 0x91
-#define CAP11XX_REG_LED_DUTY_CYCLE_3 0x92
#define CAP11XX_REG_LED_DUTY_CYCLE_4 0x93
-#define CAP11XX_REG_LED_DUTY_MIN_MASK (0x0f)
-#define CAP11XX_REG_LED_DUTY_MIN_MASK_SHIFT (0)
#define CAP11XX_REG_LED_DUTY_MAX_MASK (0xf0)
#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT (4)
#define CAP11XX_REG_LED_DUTY_MAX_VALUE (15)
-#define CAP11XX_REG_SENSOR_CALIB (0xb1 + (X))
-#define CAP11XX_REG_SENSOR_CALIB_LSB1 0xb9
-#define CAP11XX_REG_SENSOR_CALIB_LSB2 0xba
#define CAP11XX_REG_PRODUCT_ID 0xfd
#define CAP11XX_REG_MANUFACTURER_ID 0xfe
#define CAP11XX_REG_REVISION 0xff
@@ -111,37 +82,15 @@ struct cap11xx_hw_model {
static const struct reg_default cap11xx_reg_defaults[] = {
{ CAP11XX_REG_MAIN_CONTROL, 0x00 },
- { CAP11XX_REG_GENERAL_STATUS, 0x00 },
- { CAP11XX_REG_SENSOR_INPUT, 0x00 },
- { CAP11XX_REG_NOISE_FLAG_STATUS, 0x00 },
{ CAP11XX_REG_SENSITIVITY_CONTROL, 0x2f },
- { CAP11XX_REG_CONFIG, 0x20 },
- { CAP11XX_REG_SENSOR_ENABLE, 0x3f },
- { CAP11XX_REG_SENSOR_CONFIG, 0xa4 },
- { CAP11XX_REG_SENSOR_CONFIG2, 0x07 },
- { CAP11XX_REG_SAMPLING_CONFIG, 0x39 },
- { CAP11XX_REG_CALIBRATION, 0x00 },
- { CAP11XX_REG_INT_ENABLE, 0x3f },
{ CAP11XX_REG_REPEAT_RATE, 0x3f },
- { CAP11XX_REG_MT_CONFIG, 0x80 },
- { CAP11XX_REG_MT_PATTERN_CONFIG, 0x00 },
- { CAP11XX_REG_MT_PATTERN, 0x3f },
- { CAP11XX_REG_RECALIB_CONFIG, 0x8a },
{ CAP11XX_REG_SENSOR_THRESH(0), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(1), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(2), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
- { CAP11XX_REG_SENSOR_NOISE_THRESH, 0x01 },
- { CAP11XX_REG_STANDBY_CHANNEL, 0x00 },
- { CAP11XX_REG_STANDBY_CONFIG, 0x39 },
- { CAP11XX_REG_STANDBY_SENSITIVITY, 0x02 },
- { CAP11XX_REG_STANDBY_THRESH, 0x40 },
{ CAP11XX_REG_CONFIG2, 0x40 },
- { CAP11XX_REG_LED_POLARITY, 0x00 },
- { CAP11XX_REG_SENSOR_CALIB_LSB1, 0x00 },
- { CAP11XX_REG_SENSOR_CALIB_LSB2, 0x00 },
};
static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
--
2.54.0
^ permalink raw reply related
* [PATCH v4 03/10] dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED reg range
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
- Add datasheet links for all supported CAP11xx variants.
- Update LED node regex and replace enum constraints with minimum/maximum
for LED reg ranges in preparation for CAP1114 support.
CAP1114 has 11 LED channels. minimum/maximum constraints are easier to
maintain than long enum lists when expanding channel count later.
Drop unnecessary led unit-address pattern.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 7ade03f1b32b..eabf06a1163e 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -10,6 +10,15 @@ description: |
The Microchip CAP1xxx Family of RightTouchTM multiple-channel capacitive
touch controllers and LED drivers. The device communication via I2C only.
+ For more product information please see the links below:
+ CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
+ CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.pdf
+ CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.pdf
+ CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.pdf
+ CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.pdf
+ CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.pdf
+ CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.pdf
+
maintainers:
- Rob Herring <robh@kernel.org>
@@ -131,7 +140,9 @@ patternProperties:
properties:
reg:
- enum: [0, 1, 2, 3, 4, 5, 6, 7]
+ description: LED channel number
+ minimum: 0
+ maximum: 7
label: true
@@ -158,7 +169,7 @@ allOf:
- microchip,cap1298
then:
patternProperties:
- "^led@[0-7]$": false
+ "^led@": false
- if:
properties:
--
2.54.0
^ permalink raw reply related
* [PATCH v4 04/10] dt-bindings: input: microchip,cap11xx: Add microchip,cap1126 LED reg constraints
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Apply per-chip LED channel limits:
- CAP1126: max 2 channels (0-1)
- CAP1188: max 8 channels (0-7)
- CAP1106, CAP12xx: no LED support
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index eabf06a1163e..798035e942af 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -171,6 +171,19 @@ allOf:
patternProperties:
"^led@": false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1126
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 1
+
- if:
properties:
compatible:
--
2.54.0
^ permalink raw reply related
* [PATCH v4 05/10] dt-bindings: input: microchip,cap11xx: Add reset-gpios property
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Add support for the optional reset-gpios property to describe
the active-high reset pin for CAP1126/CAP1188 devices.
Driving the GPIO high asserts reset and deep sleep, while driving
it low releases reset for normal operation.
Restrict this property to be available only on CAP1126 and CAP1188
chips, as other CAP11xx variants do not have a hardware reset pin.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 798035e942af..b97e5b2735f1 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -49,6 +49,13 @@ properties:
device's ALERT#/CM_IRQ# pin is connected to.
The device only has one interrupt source.
+ reset-gpios:
+ description: |
+ GPIO connected to the active-high RESET pin of the chip;
+ driving it high asserts reset and deep sleep, while driving
+ it low releases reset for normal operation.
+ maxItems: 1
+
autorepeat:
description: |
Enables the Linux input system's autorepeat feature on the input device.
@@ -157,6 +164,20 @@ patternProperties:
allOf:
- $ref: input.yaml
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1106
+ - microchip,cap1203
+ - microchip,cap1206
+ - microchip,cap1293
+ - microchip,cap1298
+ then:
+ properties:
+ reset-gpios: false
+
- if:
properties:
compatible:
@@ -207,6 +228,8 @@ additionalProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
+
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -228,6 +251,8 @@ examples:
<109>, /* KEY_PAGEDOWN */
<104>; /* KEY_PAGEUP */
+ reset-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+
#address-cells = <1>;
#size-cells = <0>;
--
2.54.0
^ permalink raw reply related
* [PATCH v4 06/10] Input: cap11xx - add reset gpio support
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Some CAP11xx devices (CAP1126/CAP1188) have a dedicated RESET pin.
Add hardware reset operation to improve device reliability and
ensure proper initialization on probe.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index fae26f035186..1db4a9090705 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -5,6 +5,7 @@
* (c) 2014 Daniel Mack <linux@zonque.org>
*/
+#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/interrupt.h>
@@ -44,6 +45,9 @@
#define CAP11XX_MANUFACTURER_ID 0x5d
+#define CAP11XX_T_RST_FILT_MIN_US 10000
+#define CAP11XX_T_RST_ON_MIN_MS 400
+
#ifdef CONFIG_LEDS_CLASS
struct cap11xx_led {
struct cap11xx_priv *priv;
@@ -56,6 +60,7 @@ struct cap11xx_priv {
struct regmap *regmap;
struct device *dev;
struct input_dev *idev;
+ struct gpio_desc *reset_gpio;
const struct cap11xx_hw_model *model;
struct cap11xx_led *leds;
@@ -459,6 +464,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
+ priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
+ "Failed to get 'reset' GPIO\n");
+
+ if (priv->reset_gpio) {
+ usleep_range(CAP11XX_T_RST_FILT_MIN_US, CAP11XX_T_RST_FILT_MIN_US * 2);
+ gpiod_set_value_cansleep(priv->reset_gpio, 0);
+ msleep(CAP11XX_T_RST_ON_MIN_MS);
+ }
+
error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
if (error)
return dev_err_probe(dev, error, "Failed to read product ID\n");
--
2.54.0
^ permalink raw reply related
* [PATCH v4 07/10] Input: cap11xx - refactor code for better CAP1114 support.
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Extend cap11xx_hw_model structure to support CAP1114 with
different register offsets and hardware characteristics:
- led_output_control_reg_base: different address on CAP1114
- sensor_input_reg_base: different address on CAP1114
- num_sensor_thresholds: separate value from num_channels for CAP1114
- has_repeat_en: repeat enable support, disabled by default on CAP1114
Include linux/bits.h, update the register operations related to LEDs.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 73 +++++++++++++++++++++++---------
1 file changed, 53 insertions(+), 20 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 1db4a9090705..0f19ee036e78 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -5,6 +5,7 @@
* (c) 2014 Daniel Mack <linux@zonque.org>
*/
+#include <linux/bits.h>
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -36,7 +37,6 @@
#define CAP11XX_REG_LED_DUTY_CYCLE_4 0x93
#define CAP11XX_REG_LED_DUTY_MAX_MASK (0xf0)
-#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT (4)
#define CAP11XX_REG_LED_DUTY_MAX_VALUE (15)
#define CAP11XX_REG_PRODUCT_ID 0xfd
@@ -77,10 +77,14 @@ struct cap11xx_priv {
struct cap11xx_hw_model {
u8 product_id;
+ u8 led_output_control_reg_base;
+ u8 sensor_input_reg_base;
unsigned int num_channels;
unsigned int num_leds;
+ unsigned int num_sensor_thresholds;
bool has_gain;
bool has_irq_config;
+ bool has_repeat_en;
bool has_sensitivity_control;
bool has_signal_guard;
};
@@ -211,8 +215,8 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
}
if (!of_property_read_u32_array(node, "microchip,input-threshold",
- priv->thresholds, priv->model->num_channels)) {
- for (i = 0; i < priv->model->num_channels; i++) {
+ priv->thresholds, priv->model->num_sensor_thresholds)) {
+ for (i = 0; i < priv->model->num_sensor_thresholds; i++) {
if (priv->thresholds[i] > 127) {
dev_err(dev, "Invalid input-threshold value %u\n",
priv->thresholds[i]);
@@ -286,10 +290,12 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
of_property_read_u32_array(node, "linux,keycodes",
priv->keycodes, priv->model->num_channels);
- /* Disable autorepeat. The Linux input system has its own handling. */
- error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
- if (error)
- return error;
+ if (priv->model->has_repeat_en) {
+ /* Disable autorepeat. The Linux input system has its own handling. */
+ error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+ if (error)
+ return error;
+ }
return 0;
}
@@ -308,7 +314,7 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
if (ret < 0)
goto out;
- ret = regmap_read(priv->regmap, CAP11XX_REG_SENSOR_INPUT, &status);
+ ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base, &status);
if (ret < 0)
goto out;
@@ -362,7 +368,7 @@ static int cap11xx_led_set(struct led_classdev *cdev,
* 0 (OFF) and 1 (ON).
*/
return regmap_update_bits(priv->regmap,
- CAP11XX_REG_LED_OUTPUT_CONTROL,
+ priv->model->led_output_control_reg_base,
BIT(led->reg),
value ? BIT(led->reg) : 0);
}
@@ -374,6 +380,7 @@ static int cap11xx_init_leds(struct device *dev,
struct cap11xx_led *led;
int cnt = of_get_child_count(node);
int error;
+ u32 duty_val;
if (!num_leds || !cnt)
return 0;
@@ -387,15 +394,18 @@ static int cap11xx_init_leds(struct device *dev,
priv->leds = led;
+ /* Set all LEDs to off */
error = regmap_update_bits(priv->regmap,
- CAP11XX_REG_LED_OUTPUT_CONTROL, 0xff, 0);
+ priv->model->led_output_control_reg_base,
+ GENMASK(min(num_leds, 8) - 1, 0), 0);
if (error)
return error;
+ duty_val = FIELD_PREP(CAP11XX_REG_LED_DUTY_MAX_MASK,
+ CAP11XX_REG_LED_DUTY_MAX_VALUE);
+
error = regmap_update_bits(priv->regmap, CAP11XX_REG_LED_DUTY_CYCLE_4,
- CAP11XX_REG_LED_DUTY_MAX_MASK,
- CAP11XX_REG_LED_DUTY_MAX_VALUE <<
- CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT);
+ CAP11XX_REG_LED_DUTY_MAX_MASK, duty_val);
if (error)
return error;
@@ -561,41 +571,64 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
}
static const struct cap11xx_hw_model cap1106_model = {
- .product_id = 0x55, .num_channels = 6, .num_leds = 0,
+ .product_id = 0x55,
+ .num_channels = 6, .num_leds = 0, .num_sensor_thresholds = 6,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
.has_irq_config = true,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1126_model = {
- .product_id = 0x53, .num_channels = 6, .num_leds = 2,
+ .product_id = 0x53,
+ .num_channels = 6, .num_leds = 2, .num_sensor_thresholds = 6,
+ .led_output_control_reg_base = CAP11XX_REG_LED_OUTPUT_CONTROL,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
.has_irq_config = true,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1188_model = {
- .product_id = 0x50, .num_channels = 8, .num_leds = 8,
+ .product_id = 0x50,
+ .num_channels = 8, .num_leds = 8, .num_sensor_thresholds = 8,
+ .led_output_control_reg_base = CAP11XX_REG_LED_OUTPUT_CONTROL,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
.has_irq_config = true,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1203_model = {
- .product_id = 0x6d, .num_channels = 3, .num_leds = 0,
+ .product_id = 0x6d,
+ .num_channels = 3, .num_leds = 0, .num_sensor_thresholds = 3,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1206_model = {
- .product_id = 0x67, .num_channels = 6, .num_leds = 0,
+ .product_id = 0x67,
+ .num_channels = 6, .num_leds = 0, .num_sensor_thresholds = 6,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1293_model = {
- .product_id = 0x6f, .num_channels = 3, .num_leds = 0,
+ .product_id = 0x6f,
+ .num_channels = 3, .num_leds = 0, .num_sensor_thresholds = 3,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
+ .has_repeat_en = true,
.has_sensitivity_control = true,
.has_signal_guard = true,
};
static const struct cap11xx_hw_model cap1298_model = {
- .product_id = 0x71, .num_channels = 8, .num_leds = 0,
+ .product_id = 0x71,
+ .num_channels = 8, .num_leds = 0, .num_sensor_thresholds = 8,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
+ .has_repeat_en = true,
.has_sensitivity_control = true,
.has_signal_guard = true,
};
--
2.54.0
^ permalink raw reply related
* [PATCH v4 01/10] Input: cap11xx - clean up duplicate log and add probe error logs
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Duplicated device detection log exists at line 537 and line 542,
which brings redundant kernel print messages. Drop one redundant
log entry to clean up dmesg output.
Meanwhile add missing error logs when I2C communication fails
during driver probe(), helping debug.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 2447c1ae2166..485d8ba97723 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -512,7 +512,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
if (error)
- return error;
+ return dev_err_probe(dev, error, "Failed to read product ID\n");
if (val != cap->product_id) {
dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
@@ -522,7 +522,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
error = regmap_read(priv->regmap, CAP11XX_REG_MANUFACTURER_ID, &val);
if (error)
- return error;
+ return dev_err_probe(dev, error, "Failed to read manufacturer ID\n");
if (val != CAP11XX_MANUFACTURER_ID) {
dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
@@ -531,11 +531,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
}
error = regmap_read(priv->regmap, CAP11XX_REG_REVISION, &rev);
- if (error < 0)
- return error;
-
- dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
- id->name, rev);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to read revision\n");
priv->model = cap;
--
2.54.0
^ permalink raw reply related
* [PATCH v4 08/10] Input: cap11xx - guard unsupported DT properties before parsing
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Check of_property_present() before parsing microchip,calib-sensitivity
and microchip,signal-guard, so that models which do not support these
properties (e.g. CAP1114) skip the parsing entirely.
This prevents a potential buffer overflow in calib_sensitivities[8] and
signal_guard_inputs_mask when a model with more than 8 channels
(CAP1114 has 14) would otherwise call of_property_read_u32_array()
with num_channels as the element count.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 52 +++++++++++++++++---------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 0f19ee036e78..275eb79a7193 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -231,10 +231,13 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
}
}
- if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
- priv->calib_sensitivities,
- priv->model->num_channels)) {
- if (priv->model->has_sensitivity_control) {
+ if (of_property_present(node, "microchip,calib-sensitivity")) {
+ if (!priv->model->has_sensitivity_control) {
+ dev_warn(dev,
+ "This model doesn't support 'calib-sensitivity'\n");
+ } else if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
+ priv->calib_sensitivities,
+ priv->model->num_channels)) {
for (i = 0; i < priv->model->num_channels; i++) {
if (!is_power_of_2(priv->calib_sensitivities[i]) ||
priv->calib_sensitivities[i] > 4) {
@@ -254,32 +257,31 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
if (error)
return error;
}
- } else {
- dev_warn(dev,
- "This model doesn't support 'calib-sensitivity'\n");
}
}
- for (i = 0; i < priv->model->num_channels; i++) {
- if (!of_property_read_u32_index(node, "microchip,signal-guard",
- i, &u32_val)) {
- if (u32_val > 1)
- return -EINVAL;
- if (u32_val)
- priv->signal_guard_inputs_mask |= 0x01 << i;
- }
- }
-
- if (priv->signal_guard_inputs_mask) {
- if (priv->model->has_signal_guard) {
- error = regmap_write(priv->regmap,
- CAP11XX_REG_SIGNAL_GUARD_ENABLE,
- priv->signal_guard_inputs_mask);
- if (error)
- return error;
- } else {
+ if (of_property_present(node, "microchip,signal-guard")) {
+ if (!priv->model->has_signal_guard) {
dev_warn(dev,
"This model doesn't support 'signal-guard'\n");
+ } else {
+ for (i = 0; i < priv->model->num_channels; i++) {
+ if (!of_property_read_u32_index(node, "microchip,signal-guard",
+ i, &u32_val)) {
+ if (u32_val > 1)
+ return -EINVAL;
+ if (u32_val)
+ priv->signal_guard_inputs_mask |= 0x01 << i;
+ }
+ }
+
+ if (priv->signal_guard_inputs_mask) {
+ error = regmap_write(priv->regmap,
+ CAP11XX_REG_SIGNAL_GUARD_ENABLE,
+ priv->signal_guard_inputs_mask);
+ if (error)
+ return error;
+ }
}
}
--
2.54.0
^ permalink raw reply related
* [PATCH v4 09/10] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
Add the compatible string for CAP1114, add its datasheet URL,
update the maximum of LED channel reg, and add constraint for
linux,keycodes.
Previously, the LED reg property had a default maximum of 7 for CAP1188.
With the addition of CAP1114, the default maximum is now 11.
An if-then constraint is added to limit the LED count for CAP1188.
Update description for microchip,input-threshold: CAP1114 only provides
eight threshold entries, which does not match its total channel count.
CAP1114 does not support microchip,signal-guard and
microchip,calib-sensitivity.
Add CAP1114 to the unsupported enum list.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 39 +++++++++++++++++--
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index b97e5b2735f1..2a37ac252c37 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -12,6 +12,7 @@ description: |
For more product information please see the links below:
CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
+ CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.pdf
CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.pdf
CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.pdf
CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.pdf
@@ -26,6 +27,7 @@ properties:
compatible:
enum:
- microchip,cap1106
+ - microchip,cap1114
- microchip,cap1126
- microchip,cap1188
- microchip,cap1203
@@ -62,7 +64,7 @@ properties:
linux,keycodes:
minItems: 3
- maxItems: 8
+ maxItems: 14
description: |
Specifies an array of numeric keycode values to
be used for the channels. If this property is
@@ -122,6 +124,8 @@ properties:
is required for a touch to be registered, making the touch sensor less
sensitive.
The number of entries must correspond to the number of channels.
+ CAP1114 is an exception where channels 8~14 reuse the eighth entry's
+ threshold, so counts differ.
microchip,calib-sensitivity:
$ref: /schemas/types.yaml#/definitions/uint32-array
@@ -140,7 +144,7 @@ properties:
The number of entries must correspond to the number of channels.
patternProperties:
- "^led@[0-7]$":
+ "^led@[0-9a]$":
type: object
description: CAP11xx LEDs
$ref: /schemas/leds/common.yaml#
@@ -149,7 +153,7 @@ patternProperties:
reg:
description: LED channel number
minimum: 0
- maximum: 7
+ maximum: 10
label: true
@@ -178,6 +182,21 @@ allOf:
properties:
reset-gpios: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1114
+ then:
+ properties:
+ linux,keycodes:
+ minItems: 14
+ else:
+ properties:
+ linux,keycodes:
+ maxItems: 8
+
- if:
properties:
compatible:
@@ -205,12 +224,26 @@ allOf:
reg:
maximum: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1188
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 7
+
- if:
properties:
compatible:
contains:
enum:
- microchip,cap1106
+ - microchip,cap1114
- microchip,cap1126
- microchip,cap1188
- microchip,cap1203
--
2.54.0
^ permalink raw reply related
* [PATCH v4 10/10] Input: cap11xx - add support for CAP1114
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
The CAP1114 uses two control registers for LED output management and
requires two button status registers for touch input state reporting.
By default, channels CS8~CS14 operate as a single grouped block.
Set the corresponding register enable bit to enable these channels as
independent touch inputs. Note these channels share the input threshold
of the eighth entry, causing num_sensor_thresholds to differ from
num_channels.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 73 ++++++++++++++++++++++++++++++--
1 file changed, 69 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 275eb79a7193..865c58533eb9 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -18,6 +18,12 @@
#include <linux/gpio/consumer.h>
#include <linux/bitfield.h>
+#define CAP1114_REG_BUTTON_STATUS1 0x03
+#define CAP1114_REG_BUTTON_STATUS2 0x04
+#define CAP1114_REG_CONFIG2 0x40
+#define CAP1114_REG_CONFIG2_VOL_UP_DOWN BIT(1)
+#define CAP1114_REG_LED_OUTPUT_CONTROL1 0x73
+
#define CAP11XX_REG_MAIN_CONTROL 0x00
#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
#define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
@@ -83,6 +89,7 @@ struct cap11xx_hw_model {
unsigned int num_leds;
unsigned int num_sensor_thresholds;
bool has_gain;
+ bool has_grouped_sensors;
bool has_irq_config;
bool has_repeat_en;
bool has_sensitivity_control;
@@ -99,6 +106,8 @@ static const struct reg_default cap11xx_reg_defaults[] = {
{ CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(6), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(7), 0x40 },
{ CAP11XX_REG_CONFIG2, 0x40 },
};
@@ -107,6 +116,12 @@ static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
switch (reg) {
case CAP11XX_REG_MAIN_CONTROL:
case CAP11XX_REG_SENSOR_INPUT:
+ /*
+ * CAP1114_REG_BUTTON_STATUS1 (CAP11XX_REG_SENSOR_INPUT) and
+ * CAP1114_REG_BUTTON_STATUS2 is volatile for the CAP1114,
+ * which supports more than 8 touch channels.
+ */
+ case CAP1114_REG_BUTTON_STATUS2:
case CAP11XX_REG_SENOR_DELTA(0):
case CAP11XX_REG_SENOR_DELTA(1):
case CAP11XX_REG_SENOR_DELTA(2):
@@ -292,6 +307,17 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
of_property_read_u32_array(node, "linux,keycodes",
priv->keycodes, priv->model->num_channels);
+ /*
+ * CAP1114 needs dedicated configuration to split
+ * grouped sensors into independent inputs.
+ */
+ if (priv->model->has_grouped_sensors) {
+ error = regmap_set_bits(priv->regmap, CAP1114_REG_CONFIG2,
+ CAP1114_REG_CONFIG2_VOL_UP_DOWN);
+ if (error)
+ return error;
+ }
+
if (priv->model->has_repeat_en) {
/* Disable autorepeat. The Linux input system has its own handling. */
error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
@@ -320,6 +346,21 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
if (ret < 0)
goto out;
+ if (priv->model->num_channels > 8) {
+ unsigned int status2;
+
+ ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base + 1, &status2);
+ if (ret < 0)
+ goto out;
+
+ /*
+ * CAP1114 STATUS1 register only contains data for the first 6 channels.
+ * the remaining channels is stored in STATUS2.
+ */
+ status &= GENMASK(5, 0);
+ status |= FIELD_PREP(GENMASK(13, 6), status2);
+ }
+
for (i = 0; i < priv->idev->keycodemax; i++)
input_report_key(priv->idev, priv->keycodes[i],
status & (1 << i));
@@ -369,10 +410,16 @@ static int cap11xx_led_set(struct led_classdev *cdev,
* limitation. Brightness levels per LED are either
* 0 (OFF) and 1 (ON).
*/
- return regmap_update_bits(priv->regmap,
- priv->model->led_output_control_reg_base,
- BIT(led->reg),
- value ? BIT(led->reg) : 0);
+ if (led->reg >= 8)
+ return regmap_update_bits(priv->regmap,
+ priv->model->led_output_control_reg_base + 1,
+ BIT(led->reg - 8),
+ value ? BIT(led->reg - 8) : 0);
+ else
+ return regmap_update_bits(priv->regmap,
+ priv->model->led_output_control_reg_base,
+ BIT(led->reg),
+ value ? BIT(led->reg) : 0);
}
static int cap11xx_init_leds(struct device *dev,
@@ -403,6 +450,14 @@ static int cap11xx_init_leds(struct device *dev,
if (error)
return error;
+ if (num_leds > 8) {
+ error = regmap_update_bits(priv->regmap,
+ priv->model->led_output_control_reg_base + 1,
+ GENMASK(num_leds - 8 - 1, 0), 0);
+ if (error)
+ return error;
+ }
+
duty_val = FIELD_PREP(CAP11XX_REG_LED_DUTY_MAX_MASK,
CAP11XX_REG_LED_DUTY_MAX_VALUE);
@@ -581,6 +636,14 @@ static const struct cap11xx_hw_model cap1106_model = {
.has_repeat_en = true,
};
+static const struct cap11xx_hw_model cap1114_model = {
+ .product_id = 0x3a,
+ .num_channels = 14, .num_leds = 11, .num_sensor_thresholds = 8,
+ .led_output_control_reg_base = CAP1114_REG_LED_OUTPUT_CONTROL1,
+ .sensor_input_reg_base = CAP1114_REG_BUTTON_STATUS1,
+ .has_grouped_sensors = true,
+};
+
static const struct cap11xx_hw_model cap1126_model = {
.product_id = 0x53,
.num_channels = 6, .num_leds = 2, .num_sensor_thresholds = 6,
@@ -637,6 +700,7 @@ static const struct cap11xx_hw_model cap1298_model = {
static const struct of_device_id cap11xx_dt_ids[] = {
{ .compatible = "microchip,cap1106", .data = &cap1106_model },
+ { .compatible = "microchip,cap1114", .data = &cap1114_model },
{ .compatible = "microchip,cap1126", .data = &cap1126_model },
{ .compatible = "microchip,cap1188", .data = &cap1188_model },
{ .compatible = "microchip,cap1203", .data = &cap1203_model },
@@ -649,6 +713,7 @@ MODULE_DEVICE_TABLE(of, cap11xx_dt_ids);
static const struct i2c_device_id cap11xx_i2c_ids[] = {
{ .name = "cap1106", .driver_data = (kernel_ulong_t)&cap1106_model },
+ { .name = "cap1114", .driver_data = (kernel_ulong_t)&cap1114_model },
{ .name = "cap1126", .driver_data = (kernel_ulong_t)&cap1126_model },
{ .name = "cap1188", .driver_data = (kernel_ulong_t)&cap1188_model },
{ .name = "cap1203", .driver_data = (kernel_ulong_t)&cap1203_model },
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v5 10/16] media: iris: add minimal GET_PROPERTY implementation
From: Vishnu Reddy @ 2026-06-17 15:04 UTC (permalink / raw)
To: Dmitry Baryshkov, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-media, linux-arm-msm, linux-kernel, devicetree,
Dikshita Agarwal
In-Reply-To: <20260616-iris-ar50lt-v5-10-583b42770b6a@oss.qualcomm.com>
On 6/16/2026 5:34 AM, Dmitry Baryshkov wrote:
> AR50Lt with the Gen1 firmware requires host to read
> HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS property, otherwise it doesn't
> update internal data and fails the HFI_CMD_SESSION_LOAD_RESOURCES
> command. Implement minimal support for querying the properties from the
> firmware.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 +
> .../platform/qcom/iris/iris_hfi_gen1_command.c | 21 +++++++++++++++++++++
> .../platform/qcom/iris/iris_hfi_gen1_defines.h | 15 +++++++++++++++
> .../platform/qcom/iris/iris_hfi_gen1_response.c | 6 ++++++
> 4 files changed, 43 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> index a27447eb2519..16099f9a25b6 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> @@ -121,6 +121,7 @@ struct iris_hfi_session_ops {
> int (*session_set_property)(struct iris_inst *inst,
> u32 packet_type, u32 flag, u32 plane, u32 payload_type,
> void *payload, u32 payload_size);
> + int (*session_get_property)(struct iris_inst *inst, u32 packet_type);
> int (*session_open)(struct iris_inst *inst);
> int (*session_start)(struct iris_inst *inst, u32 plane);
> int (*session_queue_buf)(struct iris_inst *inst, struct iris_buffer *buffer);
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index 7674b47ad6c4..99e82e5510ab 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -1117,10 +1117,31 @@ static int iris_hfi_gen1_session_set_config_params(struct iris_inst *inst, u32 p
> return 0;
> }
>
> +static int iris_hfi_gen1_session_get_property(struct iris_inst *inst, u32 packet_type)
> +{
> + struct hfi_session_get_property_pkt pkt;
> + int ret;
> +
> + pkt.shdr.hdr.size = sizeof(pkt);
> + pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_GET_PROPERTY;
> + pkt.shdr.session_id = inst->session_id;
> + pkt.num_properties = 1;
> + pkt.data = packet_type;
> +
> + reinit_completion(&inst->completion);
> +
> + ret = iris_hfi_queue_cmd_write(inst->core, &pkt, pkt.shdr.hdr.size);
> + if (ret)
> + return ret;
> +
> + return iris_wait_for_session_response(inst, false);
> +}
> +
> static const struct iris_hfi_session_ops iris_hfi_gen1_session_ops = {
> .session_open = iris_hfi_gen1_session_open,
> .session_set_config_params = iris_hfi_gen1_session_set_config_params,
> .session_set_property = iris_hfi_gen1_session_set_property,
> + .session_get_property = iris_hfi_gen1_session_get_property,
> .session_start = iris_hfi_gen1_session_start,
> .session_queue_buf = iris_hfi_gen1_session_queue_buffer,
> .session_release_buf = iris_hfi_gen1_session_unset_buffers,
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> index 0e4dee192384..bb495a1d2623 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> @@ -35,6 +35,7 @@
> #define HFI_CMD_SESSION_EMPTY_BUFFER 0x211004
> #define HFI_CMD_SESSION_FILL_BUFFER 0x211005
> #define HFI_CMD_SESSION_FLUSH 0x211008
> +#define HFI_CMD_SESSION_GET_PROPERTY 0x211009
> #define HFI_CMD_SESSION_RELEASE_BUFFERS 0x21100b
> #define HFI_CMD_SESSION_RELEASE_RESOURCES 0x21100c
> #define HFI_CMD_SESSION_CONTINUE 0x21100d
> @@ -113,6 +114,7 @@
> #define HFI_MSG_SESSION_FLUSH 0x221006
> #define HFI_MSG_SESSION_EMPTY_BUFFER 0x221007
> #define HFI_MSG_SESSION_FILL_BUFFER 0x221008
> +#define HFI_MSG_SESSION_PROPERTY_INFO 0x221009
> #define HFI_MSG_SESSION_RELEASE_RESOURCES 0x22100a
> #define HFI_MSG_SESSION_RELEASE_BUFFERS 0x22100c
>
> @@ -205,6 +207,12 @@ struct hfi_session_set_property_pkt {
> u32 data[];
> };
>
> +struct hfi_session_get_property_pkt {
> + struct hfi_session_hdr_pkt shdr;
> + u32 num_properties;
> + u32 data;
> +};
> +
> struct hfi_sys_pc_prep_pkt {
> struct hfi_pkt_hdr hdr;
> };
> @@ -574,6 +582,13 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
> u32 data[];
> };
>
> +struct hfi_msg_session_property_info_pkt {
> + struct hfi_session_hdr_pkt shdr;
> + u32 num_properties;
> + u32 property;
> + u8 data[];
> +};
> +
> struct hfi_msg_session_release_buffers_done_pkt {
> struct hfi_msg_session_hdr_pkt shdr;
> u32 num_buffers;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index bfd7495bf44f..23fc7194b1e3 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> @@ -591,6 +591,10 @@ static const struct iris_hfi_gen1_response_pkt_info pkt_infos[] = {
> .pkt = HFI_MSG_SESSION_RELEASE_BUFFERS,
> .pkt_sz = sizeof(struct hfi_msg_session_release_buffers_done_pkt),
> },
> + {
> + .pkt = HFI_MSG_SESSION_PROPERTY_INFO,
> + .pkt_sz = sizeof(struct hfi_msg_session_property_info_pkt),
> + },
> };
>
> static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response)
> @@ -652,6 +656,8 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> iris_hfi_gen1_session_etb_done(inst, hdr);
> } else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
> iris_hfi_gen1_session_ftb_done(inst, hdr);
> + } else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> + complete(&inst->completion);
> } else {
> struct hfi_msg_session_hdr_pkt *shdr;
>
Reviewed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v8 4/6] leds: is31fl32xx: Add powerdown pin for hardware shutdown mode
From: Lee Jones @ 2026-06-17 15:09 UTC (permalink / raw)
To: Jun Yan
Cc: linux-kernel, devicetree, linux-leds, robh, krzk+dt, conor+dt,
luccafachinetti, pzalewski, daniel
In-Reply-To: <20260607152002.446617-5-jerrysteve1101@gmail.com>
/* Sashiko Automation: Issues Found (2 Findings) */
Please consider these.
On Sun, 07 Jun 2026, Jun Yan wrote:
> The IS31FL32XX series features an SDB shutdown pin. Driving it low
> (active low) places the chip into hardware shutdown mode for power
> saving, while all register contents are preserved and registers are
> not reset.
>
> Deassert/assert the pin accordingly during probe() and remove() to
> manage hardware shutdown mode.
>
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> drivers/leds/leds-is31fl32xx.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index 6c8d6b833260..b34fcef2c173 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
> @@ -10,6 +10,7 @@
> */
>
> #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> #include <linux/leds.h>
> @@ -60,6 +61,7 @@ struct is31fl32xx_led_data {
> struct is31fl32xx_priv {
> const struct is31fl32xx_chipdef *cdef;
> struct i2c_client *client;
> + struct gpio_desc *powerdown_gpio;
> unsigned int num_leds;
> struct is31fl32xx_led_data leds[];
> };
> @@ -421,6 +423,15 @@ static int is31fl32xx_parse_dt(struct device *dev,
> const struct is31fl32xx_chipdef *cdef = priv->cdef;
> int ret = 0;
>
> + /*
> + * Driving this GPIO line low takes the chip out of shutdown,
> + * as it is flagged as GPIO_ACTIVE_LOW in provider (such as the device tree).
> + */
> + priv->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->powerdown_gpio))
> + return dev_err_probe(dev, PTR_ERR(priv->powerdown_gpio),
> + "Failed to get 'powerdown' GPIO\n");
> +
[Severity: Medium]
Does this leave the chip actively powered on if probe fails later?
The powerdown gpio is acquired and driven logically low with GPIOD_OUT_LOW,
which wakes the chip from hardware shutdown.
If a subsequent initialization step in probe() fails (for example, if
is31fl32xx_init_regs() returns an error), probe() will return an error
without explicitly reverting the GPIO to its shutdown state.
The devres framework automatically frees the GPIO descriptor, but it does
not revert its physical output level. Will this leave the device powered
on indefinitely instead of returning it to its low-power standby state?
> if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
> of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
>
> @@ -609,6 +620,9 @@ static void is31fl32xx_remove(struct i2c_client *client)
> if (ret)
> dev_err(&client->dev, "Failed to reset registers on removal (%pe)\n",
> ERR_PTR(ret));
> +
> + if (priv->powerdown_gpio)
> + gpiod_set_value_cansleep(priv->powerdown_gpio, 1);
[Severity: Medium]
Could this cause an I2C error cascade during driver unbind due to the teardown
order?
The driver registers LEDs using devm_led_classdev_register_ext(), which
means the LED class devices are unregistered by devres after the driver's
remove() callback completes.
When this asserts the powerdown_gpio, it places the chip into hardware
shutdown mode. Subsequently, devres invokes led_classdev_unregister() which
attempts to gracefully turn off each LED:
drivers/leds/led-class.c:led_classdev_unregister() {
...
if (!(led_cdev->flags & LED_RETAIN_AT_SHUTDOWN))
led_set_brightness(led_cdev, LED_OFF);
...
}
This triggers an I2C write to the chip via is31fl32xx_brightness_set().
Because the hardware is already in hardware shutdown mode, will these
I2C writes fail and result in errors?
> }
>
> /*
> --
> 2.54.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v8 5/6] leds: is31fl32xx: Fix missing brightness_steps for is31fl3236
From: Lee Jones @ 2026-06-17 15:10 UTC (permalink / raw)
To: Jun Yan
Cc: linux-kernel, devicetree, linux-leds, robh, krzk+dt, conor+dt,
luccafachinetti, pzalewski, daniel
In-Reply-To: <20260607152002.446617-6-jerrysteve1101@gmail.com>
/* Sashiko Automation: Issues Found (5 Findings) */
And these.
On Sun, 07 Jun 2026, Jun Yan wrote:
> Add missing brightness_steps for is31fl3236 to fix brightness control.
>
> Fixes: a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> drivers/leds/leds-is31fl32xx.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index b34fcef2c173..cbda7edbfb48 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
> @@ -492,6 +492,7 @@ static const struct is31fl32xx_chipdef is31fl3236_cdef = {
> .pwm_register_base = 0x01,
> .led_control_register_base = 0x26,
> .enable_bits_per_led_control_register = 1,
> + .brightness_steps = 256,
> };
>
> static const struct is31fl32xx_chipdef is31fl3236a_cdef = {
While reviewing this file, I also noticed a few other regressions in the same
driver:
drivers/leds/leds-is31fl32xx.c:is31fl3293_reset() {
...
for (i = 0; i < priv->num_leds; i++) {
struct is31fl32xx_led_data *led_data = &priv->leds[i];
int current_level_reg = IS31FL3293_CL_REG + led_data->channel - 1;
int microamp = max(led_data->max_microamp, IS31FL3293_MAX_MICROAMP);
int current_level = (microamp * 0xff) / IS31FL3293_MAX_MICROAMP;
ret = is31fl32xx_write(priv, current_level_reg, current_level);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but does this calculation handle the
current limit bounds correctly?
It appears that max(led_data->max_microamp, IS31FL3293_MAX_MICROAMP) forces
microamp to be at least 20,000 (the value of IS31FL3293_MAX_MICROAMP). If a
device tree requires a lower safety limit (e.g., 5000), it seems it would be
ignored.
Conversely, if the device tree specifies a value larger than 20,000 (e.g.,
30,000), the calculated current_level evaluates to 382. When passed to
is31fl32xx_write(), which takes a u8 parameter, won't this overflow and
truncate to 126, silently programming a drastically lower current limit?
drivers/leds/leds-is31fl32xx.c:is31fl32xx_probe() {
...
ret = is31fl32xx_parse_dt(dev, priv);
if (ret)
return ret;
ret = is31fl32xx_init_regs(priv);
...
}
[Severity: High]
This is a pre-existing issue, but could this ordering lead to a race condition
with userspace?
is31fl32xx_parse_dt() calls devm_led_classdev_register_ext(), which
immediately publishes the LED devices. The LED subsystem might apply
default-state triggers, or userspace might set the brightness, which sends
PWM writes to the device over I2C.
However, the hardware registers are only initialized and reset in
is31fl32xx_init_regs() afterwards. Won't this reset wipe out any brightness
state configured during or immediately after LED registration? Should the
hardware be fully initialized before device registration?
drivers/leds/leds-is31fl32xx.c:
...
static const struct is31fl32xx_chipdef is31fl3293_cdef = {
.channels = 3,
.shutdown_reg = IS31FL32XX_REG_NONE,
.pwm_update_reg = 0x28,
.pwm_update_value = 0xc5,
.global_control_reg = IS31FL32XX_REG_NONE,
.reset_reg = IS31FL32XX_REG_NONE,
.pwm_register_base = 0x19,
.led_control_register_base = IS31FL32XX_REG_NONE,
.brightness_steps = 4096,
.reset_func = is31fl3293_reset,
.sw_shutdown_func = is31fl3293_software_shutdown,
};
...
[Severity: High]
Is is31fl3293_cdef missing the initialization for output_frequency_setting_reg?
Since this field isn't set to IS31FL32XX_REG_NONE, it is implicitly initialized
to 0x00. During probe, is31fl32xx_init_regs() checks if this field is
!= IS31FL32XX_REG_NONE (which is 0xFF). Since 0x00 != 0xFF, if the DT
contains the issi,22khz-pwm property, the driver will erroneously write 0x01
to register 0x00.
For the IS31FL3293, register 0x00 is undefined. Can this silently corrupt the
chip state?
drivers/leds/leds-is31fl32xx.c:is31fl32xx_remove() {
...
static void is31fl32xx_remove(struct i2c_client *client)
{
struct is31fl32xx_priv *priv = i2c_get_clientdata(client);
int ret;
ret = is31fl32xx_reset_regs(priv);
...
if (priv->powerdown_gpio)
gpiod_set_value_cansleep(priv->powerdown_gpio, 1);
}
...
[Severity: High]
Will the device acknowledge I2C writes after being powered down during removal?
In is31fl32xx_remove(), the driver asserts powerdown_gpio to place the chip
into a hardware shutdown mode. After remove() completes, devres automatically
unregisters the LED class devices that were allocated in is31fl32xx_parse_dt().
During unregistration, the LED subsystem turns off the LEDs by calling
is31fl32xx_brightness_set(), which attempts to write to the chip over I2C.
Because the chip is already physically powered down, won't this result in I2C
timeouts or EREMOTEIO errors on every driver removal?
> --
> 2.54.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v4 2/2] hw_random: timeriomem-rng: add configurable read width and data mask
From: Thomas Gleixner @ 2026-06-17 15:11 UTC (permalink / raw)
To: Jad Keskes, Krzysztof Kozlowski
Cc: Olivia Mackall, Herbert Xu, Rob Herring, Conor Dooley,
Alexander Clouter, linux-crypto, devicetree, linux-kernel,
Jad Keskes
In-Reply-To: <20260617114642.1911191-1-inasj268@gmail.com>
Jad!
On Wed, Jun 17 2026 at 12:46, Jad Keskes wrote:
> The TODO for supporting read sizes other than 32 bits and masking has
> been sitting in this driver since 2009. Implement it.
>
> Add reg-io-width (1, 2, or 4 bytes) and mask support. The read loop
> dispatches on width using readb/readw/readl so a configured 1-byte
> access doesn't trigger a bus error on hardware that rejects 32-bit
> reads to that address. The mask is ANDed with the value before storing.
>
> These are platform properties, not runtime policy -- width depends on
> SoC integration, mask reflects which output bits carry entropy.
>
> The alignment check in probe is updated to verify the resource is
> aligned to the configured width instead of hardcoding 4-byte alignment.
So this is the 4th version of the same thing within 24 hours and without
any explanation what the difference between v1/2/3/4 is.
Please stop this frenzy and send out new versions only if there is a
good and documented reason. Otherwise give people the time to review
your patch. All of this is documented in Documentation/process.
^ permalink raw reply
* Re: [PATCH v4 3/5] arm64: dts: qcom: sm8550-hdk: add SPMI ADC channels and thermal nodes
From: sashiko-bot @ 2026-06-17 15:13 UTC (permalink / raw)
To: Neil Armstrong; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260617-topic-sm8x50-adc5-gen3-v4-3-4af9251731f1@linaro.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Device tree compilation fails because the patch uses parameterized macros (e.g., `ADC5_GEN3_DIE_TEMP(1)` and `ADC5_GEN3_AMUX1_THM_100K_PU(1)`) that are undefined. The required local header `qcom-adc5-gen3.h` (included via `#include "qcom-adc5-gen3.h"` in `pmk8550.dtsi` in patch 1 of the series) is completely missing from the tree and was not added by any patch in this series. Furthermore, the similarly named kernel-internal header `include/linux/iio/adc/qcom-adc5-gen3-common.h` defines these as flat integers (e.g., `#define ADC5_GEN3_DIE_TEMP 0x03`), not parameterized macros, so even if included it would cause DTC syntax errors (e.g., `0x03(1)`).
--
commit ab5e68d3e2e92bec8b24c3b38a79b453baff7e21
Author: Neil Armstrong <neil.armstrong@linaro.org>
arm64: dts: qcom: sm8550-hdk: add SPMI ADC channels and thermal nodes
Add the SPMI ADC channels on the PMK8550 SPMI5 ADC3 for the other PMICS
on the system. The thermal nodes are sorted by the sensor channel to be
coherent with the system thermal nodes ordering.
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> index ee13e6136a825..f19419f63e36b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> @@ -280,6 +280,92 @@ platform {
> };
> };
>
> + thermal-zones {
> + skin-thermal {
> + thermal-sensors = <&pmk8550_vadc ADC5_GEN3_AMUX1_THM_100K_PU(1)>;
[Severity: Low]
Does this compile? It appears the parameterized macro
ADC5_GEN3_AMUX1_THM_100K_PU(1) is undefined because the required header
is missing from the tree.
[ ... ]
> @@ -1102,27 +1188,218 @@ led@3 {
> };
> };
>
> +&pm8550_temp_alarm {
> + io-channels = <&pmk8550_vadc ADC5_GEN3_DIE_TEMP(1)>;
[Severity: Low]
Similarly here, ADC5_GEN3_DIE_TEMP(1) seems to be missing its definition.
Even if include/linux/iio/adc/qcom-adc5-gen3-common.h were included, it
defines ADC5_GEN3_DIE_TEMP as a flat integer (0x03) rather than a macro
accepting parameters, which would cause a DTC syntax error.
> + io-channel-names = "thermal";
> +};
[ ... ]
> +&pmk8550_vadc {
> + /* PM8550 Channel nodes */
> + channel@100 {
> + reg = <ADC5_GEN3_REF_GND(1)>;
[Severity: Low]
Are these macros defined anywhere in the tree? The ADC channel definitions
throughout this file use these parameterized macros, which appear to rely
on a missing header.
> + label = "pm8550_offset_ref";
> + qcom,pre-scaling = <1 1>;
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-topic-sm8x50-adc5-gen3-v4-0-4af9251731f1@linaro.org?part=3
^ permalink raw reply
* [PATCH v2] dt-bindings: arm: qcom: Document Hawi SoC and its reference boards
From: Mukesh Ojha @ 2026-06-17 15:16 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Mukesh Ojha
Add Hawi SoC and its reference boards to the Qualcomm binding.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
Changes in v2: https://lore.kernel.org/lkml/20260617080147.1657632-1-mukesh.ojha@oss.qualcomm.com/
- Fixed the position of the Documentation.
- Corrected the commit text.
Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 50cc18a6ec5e..e229c0097e6f 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -303,6 +303,11 @@ properties:
- xiaomi,sagit
- const: qcom,msm8998
+ - items:
+ - enum:
+ - qcom,hawi-mtp
+ - const: qcom,hawi
+
- items:
- enum:
- 8dev,jalapeno
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox