* Re: [PATCH 1/2] arm64: dts: qcom: kodiak: Sort pinctrl subnodes by pins
From: Dmitry Baryshkov @ 2026-06-12 13:21 UTC (permalink / raw)
To: Luca Weiss
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, ~postmarketos/upstreaming, phone-devel,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260612-kodiak-cam-mclk-v1-1-fd294ff003a2@fairphone.com>
On Fri, Jun 12, 2026 at 12:55:51PM +0200, Luca Weiss wrote:
> As documented in the "Devicetree Sources (DTS) Coding Style" document,
> pinctrl subnodes should be sorted by the pins property. Do this once for
> kodiak.dtsi so that future additions can be added at the right places.
>
> No functional change intended, verified with dtx_diff.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> arch/arm64/boot/dts/qcom/kodiak.dtsi | 1382 +++++++++++++++++-----------------
> 1 file changed, 691 insertions(+), 691 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH v5 4/9] dt-bindings: bluetooth: qcom: Add NVMEM BD address cell
From: Loic Poulain @ 2026-06-12 13:20 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain, Bartosz Golaszewski
In-Reply-To: <20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com>
Add support for an NVMEM cell provider for "local-bd-address",
allowing the Bluetooth stack to retrieve controller's BD address
from non-volatile storage such as an EEPROM or an eMMC partition.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
.../devicetree/bindings/net/bluetooth/qcom,bluetooth-common.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/bluetooth/qcom,bluetooth-common.yaml b/Documentation/devicetree/bindings/net/bluetooth/qcom,bluetooth-common.yaml
index c8e9c55c1afb4c8e05ba2dae41ce2db4194b4a0f..7cb28f30c9af032082f23311f2fc89a32f266f17 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qcom,bluetooth-common.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qcom,bluetooth-common.yaml
@@ -22,4 +22,13 @@ properties:
description:
boot firmware is incorrectly passing the address in big-endian order
+ nvmem-cells:
+ maxItems: 1
+ description:
+ Nvmem data cell that contains a 6 byte BD address with the most
+ significant byte first (big-endian).
+
+ nvmem-cell-names:
+ const: local-bd-address
+
additionalProperties: true
--
2.34.1
^ permalink raw reply related
* [PATCH v5 5/9] block: implement NVMEM provider
From: Loic Poulain @ 2026-06-12 13:20 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain
In-Reply-To: <20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com>
From: Daniel Golle <daniel@makrotopia.org>
On embedded devices using an eMMC it is common that one or more partitions
on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM
data. Allow referencing the partition in device tree for the kernel and
Wi-Fi drivers accessing it via the NVMEM layer.
For now, NVMEM is only registered for the whole disk block device, as the
OF node is currently only associated to it.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
block/Kconfig | 9 ++++
block/Makefile | 1 +
block/blk-nvmem.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++
block/blk.h | 8 ++++
block/genhd.c | 4 ++
include/linux/blk_types.h | 3 ++
include/linux/blkdev.h | 1 +
7 files changed, 135 insertions(+)
diff --git a/block/Kconfig b/block/Kconfig
index 15027963472d7b40e27b9097a5993c457b5b3054..0b33747e16dc33473683706f75c92bdf8b648f7c 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -209,6 +209,15 @@ config BLK_INLINE_ENCRYPTION_FALLBACK
by falling back to the kernel crypto API when inline
encryption hardware is not present.
+config BLK_NVMEM
+ bool "Block device NVMEM provider"
+ depends on OF
+ depends on NVMEM
+ help
+ Allow block devices (or partitions) to act as NVMEM providers,
+ typically used with eMMC to store MAC addresses or Wi-Fi
+ calibration data on embedded devices.
+
source "block/partitions/Kconfig"
config BLK_PM
diff --git a/block/Makefile b/block/Makefile
index 7dce2e44276c4274c11a0a61121c83d9c43d6e0c..d7ac389e71902bc091a8800ea266190a43b3e63d 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -36,3 +36,4 @@ obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o blk-crypto-profile.o \
blk-crypto-sysfs.o
obj-$(CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK) += blk-crypto-fallback.o
obj-$(CONFIG_BLOCK_HOLDER_DEPRECATED) += holder.o
+obj-$(CONFIG_BLK_NVMEM) += blk-nvmem.o
diff --git a/block/blk-nvmem.c b/block/blk-nvmem.c
new file mode 100644
index 0000000000000000000000000000000000000000..c005f059d9fe56242ebaef9905673dff902b5686
--- /dev/null
+++ b/block/blk-nvmem.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * block device NVMEM provider
+ *
+ * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * Useful on devices using a partition on an eMMC for MAC addresses or
+ * Wi-Fi calibration EEPROM data.
+ */
+
+#include <linux/file.h>
+#include <linux/nvmem-provider.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/pagemap.h>
+#include <linux/property.h>
+
+#include "blk.h"
+
+static int blk_nvmem_reg_read(void *priv, unsigned int from, void *val, size_t bytes)
+{
+ blk_mode_t mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES;
+ dev_t devt = (dev_t)(uintptr_t)priv;
+ size_t bytes_left = bytes;
+ loff_t pos = from;
+ int ret = 0;
+
+ struct file *bdev_file __free(fput) = bdev_file_open_by_dev(devt, mode, priv, NULL);
+ if (IS_ERR(bdev_file))
+ return PTR_ERR(bdev_file);
+
+ while (bytes_left) {
+ pgoff_t f_index = pos >> PAGE_SHIFT;
+ struct folio *folio;
+ size_t folio_off;
+ size_t to_read;
+
+ folio = read_mapping_folio(bdev_file->f_mapping, f_index, NULL);
+ if (IS_ERR(folio)) {
+ ret = PTR_ERR(folio);
+ break;
+ }
+
+ folio_off = offset_in_folio(folio, pos);
+ to_read = min(bytes_left, folio_size(folio) - folio_off);
+ memcpy_from_folio(val, folio, folio_off, to_read);
+ pos += to_read;
+ bytes_left -= to_read;
+ val += to_read;
+ folio_put(folio);
+ }
+
+ return ret;
+}
+
+void blk_nvmem_add(struct block_device *bdev)
+{
+ struct device *dev = &bdev->bd_device;
+ struct nvmem_config config = {};
+
+ /* skip devices which do not have a device tree node */
+ if (!dev_of_node(dev))
+ return;
+
+ /* skip devices without an nvmem layout defined */
+ struct device_node *child __free(device_node) =
+ of_get_child_by_name(dev_of_node(dev), "nvmem-layout");
+ if (!child)
+ return;
+
+ /*
+ * skip block device too large to be represented as NVMEM devices,
+ * the NVMEM reg_read callback uses an unsigned int offset
+ */
+ if (bdev_nr_bytes(bdev) > UINT_MAX) {
+ dev_warn(dev, "block device too large to be an NVMEM provider\n");
+ return;
+ }
+
+ config.id = NVMEM_DEVID_NONE;
+ config.dev = dev;
+ config.name = dev_name(dev);
+ config.owner = THIS_MODULE;
+ config.priv = (void *)(uintptr_t)dev->devt;
+ config.reg_read = blk_nvmem_reg_read;
+ config.size = bdev_nr_bytes(bdev);
+ config.word_size = 1;
+ config.stride = 1;
+ config.read_only = true;
+ config.root_only = true;
+ config.ignore_wp = true;
+ config.of_node = to_of_node(dev->fwnode);
+
+ bdev->bd_nvmem = nvmem_register(&config);
+ if (IS_ERR(bdev->bd_nvmem)) {
+ dev_err_probe(dev, PTR_ERR(bdev->bd_nvmem),
+ "Failed to register NVMEM device\n");
+ bdev->bd_nvmem = NULL;
+ }
+}
+
+void blk_nvmem_del(struct block_device *bdev)
+{
+ if (bdev->bd_nvmem)
+ nvmem_unregister(bdev->bd_nvmem);
+
+ bdev->bd_nvmem = NULL;
+}
diff --git a/block/blk.h b/block/blk.h
index ec4674cdf2ead4fd259ff5fc42401f591e684ee9..cd3c7ca723391c40be56f1dd4810e641b7c8a2b3 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -757,4 +757,12 @@ static inline void blk_debugfs_unlock(struct request_queue *q,
memalloc_noio_restore(memflags);
}
+#ifdef CONFIG_BLK_NVMEM
+void blk_nvmem_add(struct block_device *bdev);
+void blk_nvmem_del(struct block_device *bdev);
+#else
+static inline void blk_nvmem_add(struct block_device *bdev) {}
+static inline void blk_nvmem_del(struct block_device *bdev) {}
+#endif
+
#endif /* BLK_INTERNAL_H */
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e95ae9134309679a7c6a937f5b7db8..1b2382de6fb30c1e5f60f45c04dc03ed3bf5d5f2 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -421,6 +421,8 @@ static void add_disk_final(struct gendisk *disk)
*/
dev_set_uevent_suppress(ddev, 0);
disk_uevent(disk, KOBJ_ADD);
+
+ blk_nvmem_add(disk->part0);
}
blk_apply_bdi_limits(disk->bdi, &disk->queue->limits);
@@ -704,6 +706,8 @@ static void __del_gendisk(struct gendisk *disk)
disk_del_events(disk);
+ blk_nvmem_del(disk->part0);
+
/*
* Prevent new openers by unlinked the bdev inode.
*/
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 8808ee76e73c09e0ceaac41ba59e86fb0c4efc64..ace6f59b860d0813665b2f62a1c03a1f4be94059 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -73,6 +73,9 @@ struct block_device {
int bd_writers;
#ifdef CONFIG_SECURITY
void *bd_security;
+#endif
+#ifdef CONFIG_BLK_NVMEM
+ struct nvmem_device *bd_nvmem;
#endif
/*
* keep this out-of-line as it's both big and not needed in the fast
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 890128cdea1ce66863c5baa36f3b336ec4550807..f15d2b5bf9e4fd2368b8a70416a978e22c0d4333 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -30,6 +30,7 @@
struct module;
struct request_queue;
+struct nvmem_device;
struct elevator_queue;
struct blk_trace;
struct request;
--
2.34.1
^ permalink raw reply related
* [PATCH v5 6/9] net: of_net: Add of_get_nvmem_eui48() helper for EUI-48 lookup
From: Loic Poulain @ 2026-06-12 13:20 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain, Bartosz Golaszewski
In-Reply-To: <20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com>
Factor out the common NVMEM EUI-48 retrieval logic from
of_get_mac_address_nvmem() into a new of_get_nvmem_eui48() helper that
accepts the NVMEM cell name as a parameter. This allows other subsystems
(e.g. Bluetooth) to reuse the same lookup-validate-copy pattern with a
different cell name, without duplicating code.
of_get_mac_address_nvmem() is updated to call of_get_nvmem_eui48() with
"mac-address", preserving its existing behavior.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
include/linux/of_net.h | 7 +++++++
net/core/of_net.c | 49 +++++++++++++++++++++++++++++++++++++------------
2 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index d88715a0b3a52f87af23d47791bea3baf5be5200..7854ba555d9a55f3d020a37fe00a27ae52e0e5dc 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -15,6 +15,7 @@ struct net_device;
extern int of_get_phy_mode(struct device_node *np, phy_interface_t *interface);
extern int of_get_mac_address(struct device_node *np, u8 *mac);
extern int of_get_mac_address_nvmem(struct device_node *np, u8 *mac);
+int of_get_nvmem_eui48(struct device_node *np, const char *cell_name, u8 *addr);
int of_get_ethdev_address(struct device_node *np, struct net_device *dev);
extern struct net_device *of_find_net_device_by_node(struct device_node *np);
#else
@@ -34,6 +35,12 @@ static inline int of_get_mac_address_nvmem(struct device_node *np, u8 *mac)
return -ENODEV;
}
+static inline int of_get_nvmem_eui48(struct device_node *np,
+ const char *cell_name, u8 *addr)
+{
+ return -ENODEV;
+}
+
static inline int of_get_ethdev_address(struct device_node *np, struct net_device *dev)
{
return -ENODEV;
diff --git a/net/core/of_net.c b/net/core/of_net.c
index 93ea425b9248a23f4f95a336e9cdbf0053248e32..11c1acca151266ac9287457b4050a54b08e2b5f5 100644
--- a/net/core/of_net.c
+++ b/net/core/of_net.c
@@ -61,9 +61,7 @@ static int of_get_mac_addr(struct device_node *np, const char *name, u8 *addr)
int of_get_mac_address_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
- struct nvmem_cell *cell;
- const void *mac;
- size_t len;
+ u8 mac[ETH_ALEN] __aligned(sizeof(u16));
int ret;
/* Try lookup by device first, there might be a nvmem_cell_lookup
@@ -75,27 +73,54 @@ int of_get_mac_address_nvmem(struct device_node *np, u8 *addr)
return ret;
}
- cell = of_nvmem_cell_get(np, "mac-address");
+ ret = of_get_nvmem_eui48(np, "mac-address", mac);
+ if (ret)
+ return ret;
+
+ if (!is_valid_ether_addr(mac))
+ return -EINVAL;
+
+ ether_addr_copy(addr, mac);
+ return 0;
+}
+EXPORT_SYMBOL(of_get_mac_address_nvmem);
+
+/**
+ * of_get_nvmem_eui48 - Read a 6-byte EUI-48 address from a named NVMEM cell.
+ * @np: Device node to look up the NVMEM cell from.
+ * @cell_name: Name of the NVMEM cell (e.g. "mac-address", "local-bd-address").
+ * @addr: Output buffer for the 6-byte address.
+ *
+ * Reads the named NVMEM cell and validates that it contains a non-zero 6-byte
+ * address. Returns 0 on success, negative errno on failure.
+ */
+int of_get_nvmem_eui48(struct device_node *np, const char *cell_name, u8 *addr)
+{
+ struct nvmem_cell *cell;
+ const void *eui48;
+ size_t len;
+
+ cell = of_nvmem_cell_get(np, cell_name);
if (IS_ERR(cell))
return PTR_ERR(cell);
- mac = nvmem_cell_read(cell, &len);
+ eui48 = nvmem_cell_read(cell, &len);
nvmem_cell_put(cell);
- if (IS_ERR(mac))
- return PTR_ERR(mac);
+ if (IS_ERR(eui48))
+ return PTR_ERR(eui48);
- if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
- kfree(mac);
+ if (len != ETH_ALEN || !memchr_inv(eui48, 0, ETH_ALEN)) {
+ kfree(eui48);
return -EINVAL;
}
- memcpy(addr, mac, ETH_ALEN);
- kfree(mac);
+ memcpy(addr, eui48, ETH_ALEN);
+ kfree(eui48);
return 0;
}
-EXPORT_SYMBOL(of_get_mac_address_nvmem);
+EXPORT_SYMBOL_GPL(of_get_nvmem_eui48);
/**
* of_get_mac_address()
--
2.34.1
^ permalink raw reply related
* [PATCH v3 7/8] media: platform: neoisp: Add debugfs support
From: Antoine Bouyer @ 2026-06-12 13:20 UTC (permalink / raw)
To: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern
Cc: linux-media, linux-kernel, devicetree, imx, ai.luthra, paul.elder,
geert, sakari.ailus, hverkuil+cisco, Antoine Bouyer
In-Reply-To: <20260612132039.2089051-1-antoine.bouyer@nxp.com>
Add debugfs entries to dump ISP registers, and some internal memory
regions used to store Vignetting, DRC global and DRC local coefficients.
Debug mode is activated with the `enable_debugfs` module's parameter, to
avoid runtime suspend which blocks register access when IP is not active,
so we can capture an ISP snapshot after a frame is decoded.
Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
---
drivers/media/platform/nxp/neoisp/Makefile | 2 +
drivers/media/platform/nxp/neoisp/neoisp.h | 16 +
.../platform/nxp/neoisp/neoisp_debugfs.c | 495 ++++++++++++++++++
.../media/platform/nxp/neoisp/neoisp_main.c | 19 +
4 files changed, 532 insertions(+)
create mode 100644 drivers/media/platform/nxp/neoisp/neoisp_debugfs.c
diff --git a/drivers/media/platform/nxp/neoisp/Makefile b/drivers/media/platform/nxp/neoisp/Makefile
index 7652df785e98..c68e216980dc 100644
--- a/drivers/media/platform/nxp/neoisp/Makefile
+++ b/drivers/media/platform/nxp/neoisp/Makefile
@@ -4,3 +4,5 @@ obj-$(CONFIG_VIDEO_NXP_NEOISP) += neoisp.o
neoisp-objs := neoisp_ctx.o \
neoisp_main.o
+
+neoisp-$(CONFIG_DEBUG_FS) += neoisp_debugfs.o
diff --git a/drivers/media/platform/nxp/neoisp/neoisp.h b/drivers/media/platform/nxp/neoisp/neoisp.h
index a777625974fe..d8ddf057eb4d 100644
--- a/drivers/media/platform/nxp/neoisp/neoisp.h
+++ b/drivers/media/platform/nxp/neoisp/neoisp.h
@@ -9,6 +9,7 @@
#define __NXP_NEOISP_H
#include <linux/bits.h>
+#include <linux/debugfs.h>
#include <linux/media/nxp/nxp_neoisp.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
@@ -223,8 +224,23 @@ struct neoisp_dev_s {
dma_addr_t dummy_dma;
u32 dummy_size;
struct neoisp_context_s *context;
+ struct dentry *debugfs_entry;
+ struct debugfs_regset32 *regset;
};
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+void neoisp_debugfs_init(struct neoisp_dev_s *neoispd);
+void neoisp_debugfs_exit(struct neoisp_dev_s *neoispd);
+#else
+static inline void neoisp_debugfs_init(struct neoisp_dev_s *neoispd)
+{
+}
+
+static inline void neoisp_debugfs_exit(struct neoisp_dev_s *neoispd)
+{
+}
+#endif
+
static inline int neoisp_node_link_is_enabled(struct neoisp_node_s *node)
{
return (node->intf_link->flags & MEDIA_LNK_FL_ENABLED);
diff --git a/drivers/media/platform/nxp/neoisp/neoisp_debugfs.c b/drivers/media/platform/nxp/neoisp/neoisp_debugfs.c
new file mode 100644
index 000000000000..e730569184b3
--- /dev/null
+++ b/drivers/media/platform/nxp/neoisp/neoisp_debugfs.c
@@ -0,0 +1,495 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NEOISP debugfs definition
+ *
+ * Copyright 2024-2026 NXP
+ */
+
+#include <linux/debugfs.h>
+
+#include "neoisp.h"
+#include "neoisp_ctx.h"
+#include "neoisp_regs.h"
+
+#define NEOISP_DFS_REG(reg) {.name = #reg, .offset = reg}
+
+static const struct debugfs_reg32 neoisp_dfs_regs[] = {
+ NEOISP_DFS_REG(NEO_PIPE_CONF_SOFT_RESET),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_BUS_TXPARAM),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_REG_XFR_DIS),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_CSI_CTRL),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_FRAME_NUM),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_REG_SHD_CTRL),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_REG_SHD_CMD),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_TRIG_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_INT_EN0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_INT_STAT0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_CSI_STAT),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_IMG_CONF_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_IMG_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_IMG0_IN_ADDR_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_IMG1_IN_ADDR_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_OUTCH0_ADDR_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_OUTCH1_ADDR_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_OUTIR_ADDR_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_IMG0_IN_LS_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_IMG1_IN_LS_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_OUTCH0_LS_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_OUTCH1_LS_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_OUTIR_LS_CAM0),
+ NEOISP_DFS_REG(NEO_PIPE_CONF_SKIP_CTRL0),
+ NEOISP_DFS_REG(NEO_HC_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_POINT1_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_POINT2_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_POINT3_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_POINT4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_OFFSET0_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_OFFSET1_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_OFFSET2_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_OFFSET3_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_OFFSET4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_RATIO01_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_RATIO23_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_RATIO4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_NPOINT0_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_NPOINT1_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_NPOINT2_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_NPOINT3_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS0_KNEE_NPOINT4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_POINT1_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_POINT2_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_POINT3_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_POINT4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_OFFSET0_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_OFFSET1_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_OFFSET2_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_OFFSET3_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_OFFSET4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_RATIO01_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_RATIO23_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_RATIO4_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_NPOINT0_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_NPOINT1_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_NPOINT2_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_NPOINT3_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_DECOMPRESS1_KNEE_NPOINT4_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB0_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB0_R_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB0_GR_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB0_GB_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB0_B_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB1_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB1_R_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB1_GR_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB1_GB_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB1_B_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB2_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB2_R_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB2_GR_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB2_GB_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_OB_WB2_B_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_GAIN_OFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_GAIN_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_GAIN_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_LUMA_TH_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_LUMA_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_DOWNSCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_UPSCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_POST_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_GAIN_OFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_GAIN_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_GAIN_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_LUMA_TH_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_LUMA_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_DOWNSCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_UPSCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_POST_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_HDR_MERGE_S_LINE_NUM_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_ROI_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_ROI_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_REDGAIN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_BLUEGAIN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_POINT1_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_POINT2_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_HOFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_VOFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_POINT1_SLOPE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_POINT2_SLOPE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_LUMA_TH_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CSC_MAT0_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CSC_MAT1_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CSC_MAT2_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CSC_MAT3_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CSC_MAT4_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_R_GR_OFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GB_B_OFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CNT_WHITE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMRL_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMRH_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMGL_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMGH_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMBL_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMBH_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMRGL_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMRGH_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMBGL_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_SUMBGH_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_STAT_BLK_SIZE0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_STAT_CURR_BLK_Y0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI0_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI0_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI0_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI0_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI0_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI1_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI1_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI1_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI1_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI1_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI2_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI2_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI2_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI2_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI2_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI3_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI3_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI3_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI3_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI3_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI4_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI4_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI4_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI4_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI4_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI5_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI5_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI5_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI5_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI5_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI6_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI6_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI6_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI6_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI6_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI7_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI7_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI7_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI7_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI7_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI8_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI8_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI8_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI8_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI8_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI9_POS_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI9_PIXCNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI9_SUMRED_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI9_SUMGREEN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_CROI9_SUMBLUE_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GR_AVG_IN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GB_AVG_IN_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GR_GB_CNT_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GR_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GB_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GR2_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GB2_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_COLOR_TEMP_GRGB_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CCM0_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CCM1_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CCM2_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CCM0_TH_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CCM1_TH_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_CCM2_TH_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_ROI0_POS_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_ROI0_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_ROI1_POS_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_ROI1_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_HIST0_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_HIST0_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_HIST1_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_RGBIR_HIST1_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_ROI0_POS_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_ROI0_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_ROI1_POS_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_ROI1_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST0_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST0_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST1_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST1_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST2_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST2_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST3_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_STAT_HIST3_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_POINT1_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_POINT2_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_POINT3_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_POINT4_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_OFFSET0_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_OFFSET1_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_OFFSET2_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_OFFSET3_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_OFFSET4_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_RATIO01_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_RATIO23_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_RATIO4_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_NPOINT0_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_NPOINT1_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_NPOINT2_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_NPOINT3_CAM0),
+ NEOISP_DFS_REG(NEO_IR_COMPRESS_KNEE_NPOINT4_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YPEAK_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YEDGE_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YEDGE_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YEDGES_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YEDGES_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YEDGEA_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YEDGEA_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YLUMA_X_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YLUMA_Y_TH_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YLUMA_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_YALPHA_GAIN_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CPEAK_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CEDGE_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CEDGE_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CEDGES_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CEDGES_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CEDGEA_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CEDGEA_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CLUMA_X_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CLUMA_Y_TH_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CLUMA_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_CALPHA_GAIN_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_EDGE_STAT_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_EDGES_STAT_CAM0),
+ NEOISP_DFS_REG(NEO_BNR_STRETCH_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_CONF_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_STEPY_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_STEPX_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_C_LINE_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_C_ROW_CAM0),
+ NEOISP_DFS_REG(NEO_VIGNETTING_BLK_C_FRACY_CAM0),
+ NEOISP_DFS_REG(NEO_IDBG1_LINE_NUM),
+ NEOISP_DFS_REG(NEO_IDBG1_CURR_LINE_NUM),
+ NEOISP_DFS_REG(NEO_IDBG1_IMA),
+ NEOISP_DFS_REG(NEO_IDBG1_IMD),
+ NEOISP_DFS_REG(NEO_IDBG1_DONE_STAT),
+ NEOISP_DFS_REG(NEO_DEMOSAIC_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_DEMOSAIC_ACTIVITY_CTL_CAM0),
+ NEOISP_DFS_REG(NEO_DEMOSAIC_DYNAMICS_CTL0_CAM0),
+ NEOISP_DFS_REG(NEO_DEMOSAIC_DYNAMICS_CTL2_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_GAIN_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_MAT0_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_MAT1_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_MAT2_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_MAT3_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_MAT4_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_MAT5_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_OFFSET0_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_OFFSET1_CAM0),
+ NEOISP_DFS_REG(NEO_RGB_TO_YUV_OFFSET2_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_ROI0_POS_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_ROI0_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_ROI1_POS_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_ROI1_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_GROI_SUM_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_GBL_GAIN_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_LCL_BLK_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_LCL_STRETCH_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_LCL_BLK_STEPY_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_LCL_BLK_STEPX_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_LCL_SUM_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_ALPHA_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_GROI0_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_GROI1_SUM_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_STAT_BLK_Y_CAM0),
+ NEOISP_DFS_REG(NEO_DRC_CURR_YFRACT_CAM0),
+ NEOISP_DFS_REG(NEO_NR_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_NR_BLEND_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_NR_BLEND_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_NR_EDGECNT_CAM0),
+ NEOISP_DFS_REG(NEO_DF_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_DF_TH_SCALE_CAM0),
+ NEOISP_DFS_REG(NEO_DF_BLEND_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_DF_BLEND_TH0_CAM0),
+ NEOISP_DFS_REG(NEO_DF_EDGECNT_CAM0),
+ NEOISP_DFS_REG(NEO_EE_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_EE_CORING_CAM0),
+ NEOISP_DFS_REG(NEO_EE_CLIP_CAM0),
+ NEOISP_DFS_REG(NEO_EE_MASKGAIN_CAM0),
+ NEOISP_DFS_REG(NEO_EE_EDGECNT_CAM0),
+ NEOISP_DFS_REG(NEO_CCONVMED_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_CAS_GAIN_CAM0),
+ NEOISP_DFS_REG(NEO_CAS_CORR_CAM0),
+ NEOISP_DFS_REG(NEO_CAS_OFFSET_CAM0),
+ NEOISP_DFS_REG(NEO_PACKETIZER_CH0_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_PACKETIZER_CH12_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_PACKETIZER_PACK_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IMAT0_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IMAT1_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IMAT2_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IMAT3_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IMAT4_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IMAT5_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IOFFSET0_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IOFFSET1_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_IOFFSET2_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OMAT0_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OMAT1_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OMAT2_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OMAT3_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OMAT4_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OMAT5_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OOFFSET0_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OOFFSET1_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_OOFFSET2_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_GAMMA0_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_GAMMA1_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_GAMMA2_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_BLKLVL0_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_BLKLVL1_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_BLKLVL2_CTRL_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_LOWTH_CTRL01_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_LOWTH_CTRL2_CAM0),
+ NEOISP_DFS_REG(NEO_GCM_MAT_CONFG_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI0_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI0_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI1_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI1_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI2_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI2_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI3_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI3_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI4_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI4_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI5_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI5_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI6_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI6_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI7_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI7_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI8_POS_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI8_SIZE_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL0_COEFFS0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL0_COEFFS1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL0_COEFFS2_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL0_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL1_COEFFS0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL1_COEFFS1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL1_COEFFS2_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_FIL1_SHIFT_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI0_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI0_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI1_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI1_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI2_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI2_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI3_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI3_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI4_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI4_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI5_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI5_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI6_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI6_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI7_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI7_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI8_SUM0_CAM0),
+ NEOISP_DFS_REG(NEO_AUTOFOCUS_ROI8_SUM1_CAM0),
+ NEOISP_DFS_REG(NEO_IDBG2_LINE_NUM),
+ NEOISP_DFS_REG(NEO_IDBG2_CURR_LINE_NUM),
+ NEOISP_DFS_REG(NEO_IDBG2_IMA),
+ NEOISP_DFS_REG(NEO_IDBG2_IMD),
+ NEOISP_DFS_REG(NEO_IDBG2_DONE_STAT),
+};
+
+/* Structure to store word when reading memory */
+union udata_t {
+ u8 byte[4];
+ u16 half[2];
+ u32 word;
+};
+
+static inline int
+neoisp_dump_memory(struct seq_file *m, enum isp_block_map_e map, int wsize)
+{
+ struct neoisp_dev_s *neoispd = m->private;
+ union udata_t data;
+ u32 addr;
+ u32 *src = (u32 *)neoispd->local_mem;
+ u32 offset = ISP_GET_OFF(map) / sizeof(u32);
+ u32 size = ISP_GET_SZ(map) / sizeof(u32);
+ int i, j;
+
+ for (i = 0; i < size; i++) {
+ addr = offset * sizeof(u32);
+ data.word = src[offset++];
+
+ if (wsize == sizeof(u8)) {
+ for (j = 0; j < ARRAY_SIZE(data.byte); j++)
+ seq_printf(m, "%#x: %#04x\n",
+ addr + (j * wsize), data.byte[j]);
+ }
+
+ if (wsize == sizeof(u16)) {
+ for (j = 0; j < ARRAY_SIZE(data.half); j++)
+ seq_printf(m, "%#x: %#06x\n",
+ addr + (j * wsize), data.half[j]);
+ }
+ }
+
+ return 0;
+}
+
+static int neoisp_dump_vignetting_show(struct seq_file *m, void *private)
+{
+ return neoisp_dump_memory(m, NEO_VIGNETTING_TABLE_MAP, sizeof(u16));
+}
+DEFINE_SHOW_ATTRIBUTE(neoisp_dump_vignetting);
+
+static int neoisp_dump_drc_global_show(struct seq_file *m, void *private)
+{
+ return neoisp_dump_memory(m, NEO_DRC_GLOBAL_TONEMAP_MAP, sizeof(u16));
+}
+DEFINE_SHOW_ATTRIBUTE(neoisp_dump_drc_global);
+
+static int neoisp_dump_drc_local_show(struct seq_file *m, void *private)
+{
+ return neoisp_dump_memory(m, NEO_DRC_LOCAL_TONEMAP_MAP, sizeof(u8));
+}
+DEFINE_SHOW_ATTRIBUTE(neoisp_dump_drc_local);
+
+void neoisp_debugfs_init(struct neoisp_dev_s *neoispd)
+{
+ neoispd->regset = devm_kzalloc(neoispd->dev, sizeof(*neoispd->regset), GFP_KERNEL);
+ if (!neoispd->regset)
+ return;
+
+ neoispd->regset->regs = neoisp_dfs_regs;
+ neoispd->regset->nregs = ARRAY_SIZE(neoisp_dfs_regs);
+ neoispd->regset->base = neoispd->mmio;
+
+ neoispd->debugfs_entry = debugfs_create_dir(dev_name(neoispd->dev), NULL);
+
+ debugfs_create_regset32("registers", 0400, neoispd->debugfs_entry, neoispd->regset);
+
+ debugfs_create_file("vignetting", 0400, neoispd->debugfs_entry, neoispd,
+ &neoisp_dump_vignetting_fops);
+ debugfs_create_file("drc_global", 0400, neoispd->debugfs_entry, neoispd,
+ &neoisp_dump_drc_global_fops);
+ debugfs_create_file("drc_local", 0400, neoispd->debugfs_entry, neoispd,
+ &neoisp_dump_drc_local_fops);
+}
+
+void neoisp_debugfs_exit(struct neoisp_dev_s *neoispd)
+{
+ debugfs_remove_recursive(neoispd->debugfs_entry);
+}
diff --git a/drivers/media/platform/nxp/neoisp/neoisp_main.c b/drivers/media/platform/nxp/neoisp/neoisp_main.c
index b08995403c59..ca61af8cf66e 100644
--- a/drivers/media/platform/nxp/neoisp/neoisp_main.c
+++ b/drivers/media/platform/nxp/neoisp/neoisp_main.c
@@ -9,6 +9,7 @@
*/
#include <linux/clk.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -41,6 +42,10 @@ static int standalone_mdev;
module_param_named(standalone_mdev, standalone_mdev, uint, 0644);
MODULE_PARM_DESC(standalone_mdev, " Create standalone neoisp media device, default is 0 (off)");
+static int enable_debugfs;
+module_param_named(enable_debugfs, enable_debugfs, uint, 0644);
+MODULE_PARM_DESC(enable_debugfs, " Turn on/off debugfs, default is 0 (off)");
+
static inline bool node_desc_is_output(const struct neoisp_node_desc_s *desc)
{
return desc->buf_type == V4L2_BUF_TYPE_META_OUTPUT ||
@@ -1766,9 +1771,18 @@ static int neoisp_probe(struct platform_device *pdev)
neoisp_init_hw(neoispd);
neoisp_set_default_context(neoispd);
+ if (enable_debugfs) {
+ neoisp_debugfs_init(neoispd);
+ /* Increase pm_runtime counter to prevent suspend */
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ goto err_pm_runtime_suspend;
+ }
+
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
+ dev_dbg(dev, "probe: done (%d) debugfs (%x)\n", ret, enable_debugfs);
return 0;
err_pm_runtime_suspend:
@@ -1785,6 +1799,11 @@ static void neoisp_remove(struct platform_device *pdev)
{
struct neoisp_dev_s *neoispd = platform_get_drvdata(pdev);
+ if (neoispd->regset) {
+ neoisp_debugfs_exit(neoispd);
+ pm_runtime_put(neoispd->dev);
+ }
+
neoisp_destroy_devices(neoispd);
if (standalone_mdev)
--
2.53.0
^ permalink raw reply related
* [PATCH v5 7/9] Bluetooth: hci_sync: Add NVMEM-backed BD address retrieval
From: Loic Poulain @ 2026-06-12 13:20 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain, Bartosz Golaszewski
In-Reply-To: <20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com>
Some devices store the Bluetooth BD address in non-volatile
memory, which can be accessed through the NVMEM framework.
Similar to Ethernet or WiFi MAC addresses, add support for
reading the BD address from a 'local-bd-address' NVMEM cell.
As with the device-tree provided BD address, add a quirk to
indicate whether a device or platform should attempt to read
the address from NVMEM when no valid in-chip address is present.
Also add a quirk to indicate if the address is stored in
big-endian byte order.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
include/net/bluetooth/hci.h | 18 ++++++++++++++++++
net/bluetooth/hci_sync.c | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 572b1c620c5d653a1fe10b26c1b0ba33e8f4968f..7686466d1109253b0d75edeb5f6a99fb98ce4cc6 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -164,6 +164,24 @@ enum {
*/
HCI_QUIRK_BDADDR_PROPERTY_BROKEN,
+ /* When this quirk is set, the public Bluetooth address
+ * initially reported by HCI Read BD Address command
+ * is considered invalid. The public BD Address can be
+ * retrieved via a 'local-bd-address' NVMEM cell.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_USE_BDADDR_NVMEM,
+
+ /* When this quirk is set, the Bluetooth Device Address provided by
+ * the 'local-bd-address' NVMEM is stored in big-endian order.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BDADDR_NVMEM_BE,
+
/* When this quirk is set, the duplicate filtering during
* scanning is based on Bluetooth devices addresses. To allow
* RSSI based updates, restart scanning if needed.
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index fd3aacdea512a37c22b9a2be90c89ddca4b4d99f..589ccdfa26c1281d6eb979370523fff0d7920302 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -7,6 +7,7 @@
*/
#include <linux/property.h>
+#include <linux/of_net.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -3588,6 +3589,37 @@ int hci_powered_update_sync(struct hci_dev *hdev)
return 0;
}
+/**
+ * hci_dev_get_bd_addr_from_nvmem - Get the Bluetooth Device Address
+ * (BD_ADDR) for a HCI device from
+ * an NVMEM cell.
+ * @hdev: The HCI device
+ *
+ * Search for 'local-bd-address' NVMEM cell in the device firmware node.
+ *
+ * All-zero BD addresses are rejected (unprovisioned).
+ */
+static int hci_dev_get_bd_addr_from_nvmem(struct hci_dev *hdev)
+{
+ struct device_node *np = dev_of_node(hdev->dev.parent);
+ u8 ba[sizeof(bdaddr_t)];
+ int err;
+
+ if (!np)
+ return -ENODEV;
+
+ err = of_get_nvmem_eui48(np, "local-bd-address", ba);
+ if (err)
+ return err;
+
+ if (hci_test_quirk(hdev, HCI_QUIRK_BDADDR_NVMEM_BE))
+ baswap(&hdev->public_addr, (bdaddr_t *)ba);
+ else
+ bacpy(&hdev->public_addr, (bdaddr_t *)ba);
+
+ return 0;
+}
+
/**
* hci_dev_get_bd_addr_from_property - Get the Bluetooth Device Address
* (BD_ADDR) for a HCI device from
@@ -5042,12 +5074,17 @@ static int hci_dev_setup_sync(struct hci_dev *hdev)
* its setup callback.
*/
invalid_bdaddr = hci_test_quirk(hdev, HCI_QUIRK_INVALID_BDADDR) ||
- hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY);
+ hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY) ||
+ hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_NVMEM);
if (!ret) {
if (hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY) &&
!bacmp(&hdev->public_addr, BDADDR_ANY))
hci_dev_get_bd_addr_from_property(hdev);
+ if (hci_test_quirk(hdev, HCI_QUIRK_USE_BDADDR_NVMEM) &&
+ !bacmp(&hdev->public_addr, BDADDR_ANY))
+ hci_dev_get_bd_addr_from_nvmem(hdev);
+
if (invalid_bdaddr && bacmp(&hdev->public_addr, BDADDR_ANY) &&
hdev->set_bdaddr) {
ret = hdev->set_bdaddr(hdev, &hdev->public_addr);
--
2.34.1
^ permalink raw reply related
* [PATCH v5 8/9] Bluetooth: qca: Set NVMEM BD address quirks when address is invalid
From: Loic Poulain @ 2026-06-12 13:21 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain, Bartosz Golaszewski
In-Reply-To: <20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com>
When the controller BD address is invalid (zero or default),
set the NVMEM quirks to allow retrieving the address from a
'local-bd-address' NVMEM cell. The BD address is often stored
alongside the WiFi MAC address in big-endian format, so also
set the big-endian quirk.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
drivers/bluetooth/btqca.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index dda76365726f0bfe0e80e05fe04859fa4f0592e1..df33eacfd29fa680f393f90215150743e6001d5b 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -721,8 +721,11 @@ static int qca_check_bdaddr(struct hci_dev *hdev, const struct qca_fw_config *co
}
bda = (struct hci_rp_read_bd_addr *)skb->data;
- if (!bacmp(&bda->bdaddr, &config->bdaddr))
+ if (!bacmp(&bda->bdaddr, &config->bdaddr)) {
hci_set_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY);
+ hci_set_quirk(hdev, HCI_QUIRK_USE_BDADDR_NVMEM);
+ hci_set_quirk(hdev, HCI_QUIRK_BDADDR_NVMEM_BE);
+ }
kfree_skb(skb);
--
2.34.1
^ permalink raw reply related
* [PATCH v3 8/8] arm64: dts: freescale: imx95: Add NXP neoisp device tree node
From: Antoine Bouyer @ 2026-06-12 13:20 UTC (permalink / raw)
To: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern
Cc: linux-media, linux-kernel, devicetree, imx, ai.luthra, paul.elder,
geert, sakari.ailus, hverkuil+cisco, Antoine Bouyer
In-Reply-To: <20260612132039.2089051-1-antoine.bouyer@nxp.com>
Add neoisp device tree node to imx95.dtsi and enable it by default in
19x19 evk board.
Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
---
arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts | 4 ++++
arch/arm64/boot/dts/freescale/imx95.dtsi | 11 +++++++++++
2 files changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
index 2e463bc7c601..306593585c74 100644
--- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
@@ -511,6 +511,10 @@ &mu7 {
status = "okay";
};
+&neoisp0 {
+ status = "okay";
+};
+
&netcmix_blk_ctrl {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index d6c549c16047..5543a6cb1250 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -1867,6 +1867,17 @@ pmu@49252000 {
};
};
+ neoisp0: isp@4ae00000 {
+ compatible = "nxp,imx95-neoisp";
+ reg = <0x0 0x4ae00000 0x0 0x8000>,
+ <0x0 0x4afe0000 0x0 0x10000>;
+ interrupts = <GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&scmi_clk IMX95_CLK_CAMCM0>;
+ clock-names = "camcm0";
+ power-domains = <&scmi_devpd IMX95_PD_CAMERA>;
+ status = "disabled";
+ };
+
usb3: usb@4c010010 {
compatible = "fsl,imx95-dwc3", "fsl,imx8mp-dwc3";
reg = <0x0 0x4c010010 0x0 0x04>,
--
2.53.0
^ permalink raw reply related
* [PATCH v5 9/9] arm64: dts: qcom: arduino-imola: Describe NVMEM layout for WiFi/BT addresses
From: Loic Poulain @ 2026-06-12 13:21 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Jens Axboe, Johannes Berg,
Jeff Johnson, Bartosz Golaszewski, Marcel Holtmann,
Luiz Augusto von Dentz, Balakrishna Godavarthi, Rocky Liao,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Srinivas Kandagatla, Andrew Lunn, Heiner Kallweit,
Russell King, Saravana Kannan
Cc: linux-mmc, devicetree, linux-kernel, linux-arm-msm, linux-block,
linux-wireless, ath10k, linux-bluetooth, netdev, daniel,
Loic Poulain, Konrad Dybcio, Bartosz Golaszewski
In-Reply-To: <20260612-block-as-nvmem-v5-0-95e0b30fff90@oss.qualcomm.com>
On Arduino Uno-Q, the eMMC boot1 partition is factory provisioned
with device-specific information such as the WiFi MAC address
and the Bluetooth BD address. This partition can serve as an
alternative to additional non-volatile memory, such as a
dedicated EEPROM.
The eMMC boot partitions are typically good candidates, as they
are relatively small, read-only by default (and can be enforced
as hardware read-only), and are not affected by board reflashing
procedures, which generally target the eMMC user or GP partitions.
Describe the corresponding nvmem-layout for the WiFi and Bluetooth
addresses, and point the WiFi and Bluetooth nodes to the appropriate
NVMEM cells to retrieve them.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
index bf088fa9807f040f0c8f405f9111b01790b09377..128c7a7e76b5b089044745f5d6407d6391055fc2 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
@@ -409,7 +409,40 @@ &sdhc_1 {
no-sdio;
no-sd;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
status = "okay";
+
+ card@0 {
+ compatible = "mmc-card";
+ reg = <0>;
+
+ partitions-boot1 {
+ compatible = "fixed-partitions";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ nvmem-layout {
+ compatible = "fixed-layout";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ wifi_mac_addr: mac-addr@4400 {
+ compatible = "mac-base";
+ reg = <0x4400 0x6>;
+ #nvmem-cell-cells = <1>;
+ };
+
+ bd_addr: bd-addr@5400 {
+ compatible = "mac-base";
+ reg = <0x5400 0x6>;
+ #nvmem-cell-cells = <1>;
+ };
+ };
+ };
+ };
};
&spi5 {
@@ -512,6 +545,9 @@ bluetooth {
vddch0-supply = <&pm4125_l22>;
enable-gpios = <&tlmm 87 GPIO_ACTIVE_HIGH>;
max-speed = <3000000>;
+
+ nvmem-cells = <&bd_addr 0>;
+ nvmem-cell-names = "local-bd-address";
};
};
@@ -557,6 +593,9 @@ &wifi {
qcom,ath10k-calibration-variant = "ArduinoImola";
firmware-name = "qcm2290";
+ nvmem-cells = <&wifi_mac_addr 0>;
+ nvmem-cell-names = "mac-address";
+
status = "okay";
};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] arm64: dts: qcom: qcm6490-fairphone-fp5: Add AW88261 amplifiers
From: Dmitry Baryshkov @ 2026-06-12 13:22 UTC (permalink / raw)
To: Luca Weiss
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Val Packett, Bharadwaj Raju, Bhushan Shah,
~postmarketos/upstreaming, phone-devel, linux-arm-msm, devicetree,
linux-kernel, Konrad Dybcio
In-Reply-To: <20260612-fp5-aw88261-v2-1-f7ef7d060170@fairphone.com>
On Fri, Jun 12, 2026 at 02:52:43PM +0200, Luca Weiss wrote:
> Add nodes for the two AW88261 amplifiers, for the top and bottom
> speakers of this phone. Hook them up to the sound card.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Co-developed-by: Bharadwaj Raju <bharadwaj.raju@machinesoul.in>
> Signed-off-by: Bharadwaj Raju <bharadwaj.raju@machinesoul.in>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> Changes in v2:
> - Remove awinic,sync-flag for both amps since it's actually not needed
> (Bhushan, off-list)
> - Remove RFC prefix
> - Pick up tags
> - Link to v1: https://patch.msgid.link/20260522-fp5-aw88261-v1-1-20e412eb4c4e@fairphone.com
> ---
> arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 57 +++++++++++++++++++++-
> 1 file changed, 55 insertions(+), 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH net-next v2 0/2] net: pse-pd: add Realtek/Broadcom PSE MCU support
From: Jonas Jelonek @ 2026-06-12 13:29 UTC (permalink / raw)
To: Oleksij Rempel, Kory Maincent, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: netdev, devicetree, linux-kernel, Daniel Golle, Bjørn Mork,
Jonas Jelonek
This series adds a PSE-PD driver for the microcontroller (MCU) that fronts
the PSE silicon on a range of managed switches, together with its DT
binding.
Hardware model
==============
These boards do not expose the PSE chips to the host directly. A small
microcontroller sits on an I2C/SMBus or UART bus and manages one or more PSE
chips behind it; the host CPU only ever talks to that MCU, using a fixed
12-byte request/response protocol with a trailing checksum. The PSE silicon
never appears on the bus.
The same protocol family is used by MCUs fronting Realtek PSE chips
(RTL8238B, RTL8239, RTL8239C) and Broadcom PSE chips (BCM59111, BCM59121),
diverging in opcode numbering and a few response layouts. The driver
abstracts that behind a per-dialect opcode table and parser hooks, selected
by the compatible. The specific PSE chip behind the MCU is detected at
runtime and only influences per-chip constants (power scaling and the
per-port cap).
Why the compatible names the protocol, not the chip
===================================================
The compatibles are "realtek,pse-mcu-rtk" and "realtek,pse-mcu-bcm". This is
a deliberate choice and the part most likely to raise questions, so the
reasoning up front.
The node names the protocol dialect, not a part:
- The DT node describes the MCU, not a PSE chip: the PSE chips are behind
the MCU and never appear on the bus, so naming the node after one (e.g.
"realtek,rtl8239") would describe hardware that isn't at that address.
- The PSE chips are, in principle, usable without this MCU (host-driven
directly) - different hardware with a different programming model that
would warrant its own binding. Claiming the PSE-chip compatibles here
would collide with that.
- Naming the MCU silicon is equally wrong: these are ordinary
general-purpose microcontrollers (GigaDevice, Nuvoton, ...) that vary
across boards and are not dedicated to this application.
- What is fixed, and all the driver needs at DT-parse time, is the
protocol dialect, so the compatible encodes exactly that. The two
dialects share one protocol family and one binding, kept in a single
"realtek" vendor namespace because this MCU front-end is found almost
exclusively on Realtek-based switches; a "-rtk"/"-bcm" suffix selects
the dialect. This follows the "google,cros-ec-*" pattern: a compatible
for a firmware/protocol interface implemented by varying
microcontrollers.
One compatible per dialect spans both transports:
- The 12-byte wire protocol is identical over I2C/SMBus and UART; only the
plumbing differs (SMBus vs native framing on I2C, baud rate on UART),
and the transport is already expressed structurally by the node's parent
bus (i2c@... vs serial@...). A "-i2c"/"-uart" suffix would only
duplicate that, for a protocol that does not change across transports.
- This is the multi-transport model used by e.g. "bosch,bmi160" (one
compatible, separate i2c and spi drivers binding it), rather than the
cros-ec model of per-transport compatibles - cros-ec splits because its
on-wire framing genuinely differs per bus, which is not the case here.
The binding documents both points as well.
Testing
=======
- Linksys LGS328MPCv2 (RTL8238B, I2C)
- Zyxel GS1900-10HP A1 (BCM59121, UART)
- Zyxel GS1900-10HP B1 (RTL8238B, UART)
- Zyxel XMG1915-10EP (RTL8239C, UART)
- Zyxel XS1930-12HP (RTL8239, SMBus)
---
v1 -> v2:
- all points flagged by Sashiko addressed:
- uart: drop frame overflow (return count, not the stored length) so
serdev retains no leftover bytes that would misalign the next response
- uart: guard rx_buf/rx_len with a spinlock to close a data race between
the async receive_buf callback and send/recv
- i2c: return terminal MCU error opcodes (0xfd/0xfe) to the core
immediately instead of polling to the 1 s timeout
- core: cap BCM59121 at 30 W (802.3at) — the basic 8-bit set command
can't program the advertised 60 W (it silently clamped to 51 W)
v1: https://lore.kernel.org/netdev/20260608205758.1830521-1-jelonek.jonas@gmail.com/
---
Jonas Jelonek (2):
dt-bindings: net: pse-pd: add bindings for Realtek/Broadcom PSE MCU
net: pse-pd: add Realtek/Broadcom PSE MCU driver
.../bindings/net/pse-pd/realtek,pse-mcu.yaml | 154 +++
MAINTAINERS | 7 +
drivers/net/pse-pd/Kconfig | 28 +
drivers/net/pse-pd/Makefile | 3 +
drivers/net/pse-pd/realtek-pse-core.c | 1007 +++++++++++++++++
drivers/net/pse-pd/realtek-pse-i2c.c | 164 +++
drivers/net/pse-pd/realtek-pse-uart.c | 156 +++
drivers/net/pse-pd/realtek-pse.h | 87 ++
8 files changed, 1606 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu.yaml
create mode 100644 drivers/net/pse-pd/realtek-pse-core.c
create mode 100644 drivers/net/pse-pd/realtek-pse-i2c.c
create mode 100644 drivers/net/pse-pd/realtek-pse-uart.c
create mode 100644 drivers/net/pse-pd/realtek-pse.h
base-commit: f6033078a9e671e3c8b83d387b91591a6f6a54e7
--
2.51.0
^ permalink raw reply
* [PATCH net-next v2 1/2] dt-bindings: net: pse-pd: add bindings for Realtek/Broadcom PSE MCU
From: Jonas Jelonek @ 2026-06-12 13:29 UTC (permalink / raw)
To: Oleksij Rempel, Kory Maincent, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: netdev, devicetree, linux-kernel, Daniel Golle, Bjørn Mork,
Jonas Jelonek
In-Reply-To: <20260612132944.460646-1-jelonek.jonas@gmail.com>
Add a binding for the microcontroller (MCU) that fronts the PSE silicon
on a range of managed switches. The host talks only to the MCU, over
I2C/SMBus or UART, using a fixed message-based protocol; the PSE chips
behind it never appear on the bus.
The compatible identifies the PSE-MCU protocol dialect
(realtek,pse-mcu-rtk or realtek,pse-mcu-bcm), not a specific part: the
node describes the MCU - whose silicon is a general-purpose
microcontroller that varies across boards - and the 'realtek' vendor
prefix reflects the platform these MCUs are found on (Realtek-based PoE
switches), following the google,cros-ec-* pattern rather than naming the
MCU silicon. The '-rtk'/'-bcm' suffix selects the Realtek or Broadcom
dialect within that one family. The specific PSE chip is detected at
runtime and is not described here.
A single compatible per dialect covers both the I2C/SMBus and UART
attachments: the wire protocol is identical across them and the transport
is expressed by the node's parent bus, so it is not encoded in the
compatible.
Both dialects share one protocol family and one device tree contract, so
they are documented in a single binding under one vendor prefix. The
'realtek' prefix is used because this MCU front-end is found almost
exclusively on Realtek-based switches; the Broadcom dialect is expressed
as the realtek,pse-mcu-bcm compatible within the same family.
Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
---
.../bindings/net/pse-pd/realtek,pse-mcu.yaml | 154 ++++++++++++++++++
1 file changed, 154 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu.yaml
diff --git a/Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu.yaml b/Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu.yaml
new file mode 100644
index 000000000000..2fb729dcb41f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pse-pd/realtek,pse-mcu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek/Broadcom PSE MCU
+
+maintainers:
+ - Jonas Jelonek <jelonek.jonas@gmail.com>
+
+description: |
+ Microcontroller (MCU) that fronts the PSE hardware on switches using
+ Realtek (RTL8238B, RTL8239, RTL8239C) or Broadcom (BCM59111, BCM59121)
+ PSE chips. The MCU exposes a small message-based protocol over either
+ I2C/SMBus or UART; the actual PSE silicon is not accessed directly. The
+ Realtek and Broadcom variants share this device tree contract but use
+ different protocol opcodes, selected by the compatible.
+
+ The compatible identifies the PSE-MCU protocol dialect, not a specific
+ part. The device described here is the MCU, whose own silicon varies
+ across boards and is incidental to the protocol. The MCU is not
+ made by Realtek or Broadcom; the 'realtek' vendor prefix reflects the
+ platform these MCUs are found on (Realtek-based PoE switches) and the
+ '-rtk'/'-bcm' suffix selects the Realtek or Broadcom protocol dialect.
+ The specific PSE chip behind the MCU is not described in the device
+ tree either; it is detected at runtime by querying the MCU.
+
+ A single compatible per dialect covers both the I2C/SMBus and UART
+ attachments: the wire protocol is identical across them and the
+ transport is already expressed by the node's parent bus, so it is not
+ encoded in the compatible. Transport-specific properties differ
+ accordingly - the I2C attachment carries 'reg' (and, for Realtek,
+ 'realtek,i2c-protocol'), while the UART attachment carries the serial
+ peripheral properties such as 'current-speed'.
+
+properties:
+ compatible:
+ enum:
+ - realtek,pse-mcu-rtk
+ - realtek,pse-mcu-bcm
+
+ reg:
+ maxItems: 1
+
+ power-supply:
+ description: Regulator supplying the PoE power rail.
+
+ enable-gpios:
+ maxItems: 1
+
+ realtek,i2c-protocol:
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ i2c, smbus ]
+ description: |
+ Wire framing the MCU firmware expects on the I2C bus. "smbus" means
+ reads carry a leading command byte (0x00) and a repeated start; "i2c"
+ means bare 12-byte writes and reads with no command prefix. Only
+ applies to the Realtek I2C attachment.
+
+required:
+ - compatible
+
+allOf:
+ - $ref: pse-controller.yaml#
+ - $ref: /schemas/serial/serial-peripheral-props.yaml#
+ # The I2C attachment (identified by 'reg') cannot carry serial bus props.
+ - if:
+ required: [reg]
+ then:
+ properties:
+ current-speed: false
+ max-speed: false
+ # 'realtek,i2c-protocol' is meaningful only for the Realtek I2C attachment;
+ # the Broadcom variant and any UART attachment must not carry it.
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: realtek,pse-mcu-rtk
+ required: [reg]
+ then:
+ required:
+ - realtek,i2c-protocol
+ else:
+ properties:
+ "realtek,i2c-protocol": false
+
+unevaluatedProperties: false
+
+examples:
+ # Realtek PSE chip, I2C attachment (SMBus framing).
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-pse@20 {
+ compatible = "realtek,pse-mcu-rtk";
+ reg = <0x20>;
+ realtek,i2c-protocol = "smbus";
+
+ pse-pis {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pse-pi@0 {
+ reg = <0>;
+ #pse-cells = <0>;
+ };
+ };
+ };
+ };
+
+ # Broadcom PSE chip, I2C attachment.
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-pse@20 {
+ compatible = "realtek,pse-mcu-bcm";
+ reg = <0x20>;
+
+ pse-pis {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pse-pi@0 {
+ reg = <0>;
+ #pse-cells = <0>;
+ };
+ };
+ };
+ };
+
+ # Realtek PSE chip, UART attachment.
+ - |
+ serial {
+ ethernet-pse {
+ compatible = "realtek,pse-mcu-rtk";
+ current-speed = <115200>;
+
+ pse-pis {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pse-pi@0 {
+ reg = <0>;
+ #pse-cells = <0>;
+ };
+ };
+ };
+ };
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 2/2] net: pse-pd: add Realtek/Broadcom PSE MCU driver
From: Jonas Jelonek @ 2026-06-12 13:29 UTC (permalink / raw)
To: Oleksij Rempel, Kory Maincent, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: netdev, devicetree, linux-kernel, Daniel Golle, Bjørn Mork,
Jonas Jelonek
In-Reply-To: <20260612132944.460646-1-jelonek.jonas@gmail.com>
A range of PoE switches use a small microcontroller on the PCB to front
the actual PSE silicon. The host CPU talks to that MCU over I2C/SMBus or
UART using a fixed 12-byte request/response protocol with a trailing
checksum; the PSE chips are managed by the MCU and are not accessed
directly. The same protocol family is spoken by Realtek and Broadcom PSE
MCUs, diverging in opcode numbering and a few response layouts, which the
driver abstracts behind a per-dialect opcode table and parser hooks
selected by the compatible. The specific PSE chip behind the MCU is
detected at runtime and only influences per-chip constants (power scaling
and the per-port cap).
The driver is split into a shared core and two transport modules:
- PSE_REALTEK: protocol, message framing, dialect machinery, and the
pse_controller_ops glue.
- PSE_REALTEK_I2C / PSE_REALTEK_UART: transport modules registering the
MCU on an I2C bus or a serdev port respectively.
The realtek-* file names and PSE_REALTEK* symbols reflect the platform
this setup appears on rather than vendor scope: this MCU front-end is
found almost exclusively on Realtek-based switches. Broadcom PSE MCUs
speak the same protocol family and are supported by the same shared core
through the dialect abstraction, so the realtek-* naming is kept rather
than splitting the code or renaming to a vendor-neutral scheme. For the
same reason the device tree compatibles live under one vendor prefix,
realtek,pse-mcu-rtk and realtek,pse-mcu-bcm, with the suffix selecting the
dialect.
Power budgeting is left to the MCU firmware; the driver advertises
PSE_BUDGET_EVAL_STRAT_DYNAMIC (controller-managed budget) accordingly.
Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
---
MAINTAINERS | 7 +
drivers/net/pse-pd/Kconfig | 28 +
drivers/net/pse-pd/Makefile | 3 +
drivers/net/pse-pd/realtek-pse-core.c | 1007 +++++++++++++++++++++++++
drivers/net/pse-pd/realtek-pse-i2c.c | 164 ++++
drivers/net/pse-pd/realtek-pse-uart.c | 156 ++++
drivers/net/pse-pd/realtek-pse.h | 87 +++
7 files changed, 1452 insertions(+)
create mode 100644 drivers/net/pse-pd/realtek-pse-core.c
create mode 100644 drivers/net/pse-pd/realtek-pse-i2c.c
create mode 100644 drivers/net/pse-pd/realtek-pse-uart.c
create mode 100644 drivers/net/pse-pd/realtek-pse.h
diff --git a/MAINTAINERS b/MAINTAINERS
index cc1dde0c9067..3cddfb1dd594 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22484,6 +22484,13 @@ S: Maintained
F: include/sound/rt*.h
F: sound/soc/codecs/rt*
+REALTEK/BROADCOM PSE MCU DRIVER
+M: Jonas Jelonek <jelonek.jonas@gmail.com>
+L: netdev@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/net/pse-pd/realtek,pse-mcu.yaml
+F: drivers/net/pse-pd/realtek-pse*
+
REALTEK OTTO WATCHDOG
M: Sander Vanheule <sander@svanheule.net>
L: linux-watchdog@vger.kernel.org
diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
index 7ef29657ee5d..b065b19db126 100644
--- a/drivers/net/pse-pd/Kconfig
+++ b/drivers/net/pse-pd/Kconfig
@@ -13,6 +13,34 @@ menuconfig PSE_CONTROLLER
if PSE_CONTROLLER
+config PSE_REALTEK
+ tristate
+ help
+ Shared core for the Realtek/Broadcom PSE MCU driver. This is
+ selected automatically by the transport options below.
+
+config PSE_REALTEK_I2C
+ tristate "Realtek/Broadcom PSE MCU driver (I2C transport)"
+ depends on I2C
+ select PSE_REALTEK
+ help
+ Driver for the microcontroller (MCU) that fronts the PSE
+ hardware on switches with Realtek or Broadcom PSE chips, attached
+ via I2C/SMBus. The MCU exposes a message-based protocol; the actual
+ PSE silicon is not accessed directly. To compile this driver as a
+ module, choose M here: the module will be called realtek-pse-i2c.
+
+config PSE_REALTEK_UART
+ tristate "Realtek/Broadcom PSE MCU driver (UART transport)"
+ depends on SERIAL_DEV_BUS
+ select PSE_REALTEK
+ help
+ Driver for the microcontroller (MCU) that fronts the PSE
+ hardware on switches with Realtek or Broadcom PSE chips, attached
+ via UART. The MCU exposes a message-based protocol; the actual PSE
+ silicon is not accessed directly. To compile this driver as a
+ module, choose M here: the module will be called realtek-pse-uart.
+
config PSE_REGULATOR
tristate "Regulator based PSE controller"
help
diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
index cc78f7ea7f5f..ad6ebf50fd56 100644
--- a/drivers/net/pse-pd/Makefile
+++ b/drivers/net/pse-pd/Makefile
@@ -3,6 +3,9 @@
obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
+obj-$(CONFIG_PSE_REALTEK) += realtek-pse-core.o
+obj-$(CONFIG_PSE_REALTEK_I2C) += realtek-pse-i2c.o
+obj-$(CONFIG_PSE_REALTEK_UART) += realtek-pse-uart.o
obj-$(CONFIG_PSE_REGULATOR) += pse_regulator.o
obj-$(CONFIG_PSE_PD692X0) += pd692x0.o
obj-$(CONFIG_PSE_SI3474) += si3474.o
diff --git a/drivers/net/pse-pd/realtek-pse-core.c b/drivers/net/pse-pd/realtek-pse-core.c
new file mode 100644
index 000000000000..3a601cfb6280
--- /dev/null
+++ b/drivers/net/pse-pd/realtek-pse-core.c
@@ -0,0 +1,1007 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for the microcontroller (MCU) fronting Realtek or Broadcom PSE
+ * chips. Both vendors' MCUs speak a closely related 12-byte fixed-frame
+ * management protocol; this driver covers both via a per-dialect opcode
+ * table and response parsers.
+ *
+ * Many PoE switch designs put a dedicated microcontroller in front of the
+ * actual PSE silicon: the host CPU talks to the MCU over I2C/SMBus or
+ * UART, and the MCU in turn manages the PSE chips on the board. The MCU
+ * speaks a small message-based protocol (12-byte fixed-size frames; opcode
+ * + arg + 9 payload bytes + checksum). The PSE chips themselves are not
+ * accessed directly; everything goes through MCU commands.
+ *
+ * This driver targets that architecture for the Realtek-family protocol.
+ * Two dialects are supported: Realtek MCUs managing RTL823x/RTL8239* PSE
+ * chips, and Broadcom MCUs managing BCM590xx PSE chips. The two share
+ * frame format and a sum-mod-256 checksum but diverge on opcode numbers
+ * and on a few response layouts; this is handled by the per-dialect
+ * opcode table and parser hooks.
+ *
+ * Out of scope: PSE chips that are interfaced directly from the host
+ * without a management MCU, MCU designs that speak an unrelated protocol
+ * family, and "dumb PSE" modes where no host control is wired up at all.
+ * Those, if and when they show up in the kernel, belong in separate
+ * drivers under drivers/net/pse-pd/.
+ *
+ * This core module implements the protocol, decoding/encoding of MCU
+ * responses, and the pse_controller_ops integration. Transport modules
+ * (realtek-pse-i2c, realtek-pse-uart) provide the send/recv callbacks.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/jiffies.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/pse-pd/pse.h>
+#include <linux/unaligned.h>
+
+#include "realtek-pse.h"
+
+#define RTPSE_DEVICE_ID_RTL8238B 0x0138
+#define RTPSE_DEVICE_ID_RTL8239 0x0039
+#define RTPSE_DEVICE_ID_RTL8239C 0x0139
+#define RTPSE_DEVICE_ID_BCM59111 0xe111
+#define RTPSE_DEVICE_ID_BCM59121 0xe121
+
+#define RTPSE_PORT_STS_DISABLED 0x00
+#define RTPSE_PORT_STS_SEARCHING 0x01
+#define RTPSE_PORT_STS_DELIVERING 0x02
+#define RTPSE_PORT_STS_FAULT 0x04
+#define RTPSE_PORT_STS_REQUESTING 0x06
+
+/* RTPSE_PORT_SET_POWER_LIMIT_TYPE values */
+#define RTPSE_PORT_PW_LIMIT_TYPE_USER 0x02
+
+#define RTPSE_MAX_PORTS 48
+#define RTPSE_PORT_MAX_PRIORITY 3
+
+enum rtpse_cmd {
+ RTPSE_CMD_MCU_SET_GLOBAL_STATE,
+ RTPSE_CMD_MCU_GET_SYSTEM_INFO,
+ RTPSE_CMD_MCU_GET_EXT_CONFIG,
+
+ RTPSE_CMD_PORT_ENABLE,
+ RTPSE_CMD_PORT_SET_POWER_LIMIT_TYPE,
+ RTPSE_CMD_PORT_SET_POWER_LIMIT,
+ RTPSE_CMD_PORT_SET_POWER_LIMIT_EXT,
+ RTPSE_CMD_PORT_SET_PRIORITY,
+ RTPSE_CMD_PORT_GET_STATUS,
+ RTPSE_CMD_PORT_GET_POWER_STATS,
+ RTPSE_CMD_PORT_GET_CONFIG,
+ RTPSE_CMD_PORT_GET_EXT_CONFIG,
+
+ RTPSE_NUM_CMDS,
+};
+
+struct rtpse_opcode {
+ u8 op;
+ bool valid;
+};
+
+/* Shorthand for the designated-initializer entries in dialect opcode tables. */
+#define RTPSE_OP(opc) { .op = (opc), .valid = true }
+
+/* Forward-declared so dialects can supply response parsers (defined below). */
+struct rtpse_mcu_info;
+struct rtpse_port_status;
+
+struct rtpse_mcu_dialect {
+ struct rtpse_opcode opcode[RTPSE_NUM_CMDS];
+
+ /*
+ * Response parsers. Each dialect must supply its own; the core calls
+ * these unconditionally rather than carrying a default that would
+ * silently mis-decode bytes from a dialect that forgot to set them.
+ */
+ int (*parse_system_info)(const u8 *payload, struct rtpse_mcu_info *info);
+ int (*parse_port_class)(const struct rtpse_port_status *status);
+ const char *(*mcu_type_str)(unsigned int mcu_type);
+};
+
+/*
+ * Per-compatible match data: selected by the DT/I2C compatible, it bundles
+ * the protocol dialect with attachment quirks that the exact MCU silicon
+ * does not determine (only its firmware protocol and the host bus do).
+ */
+struct rtpse_match_data {
+ const struct rtpse_mcu_dialect *dialect;
+ /* I2C framing must come from DT (realtek,i2c-protocol); else SMBus. */
+ bool i2c_proto_dt_required;
+};
+
+struct rtpse_chip_info {
+ const char *name;
+ u16 device_id;
+ u32 max_mW_per_port;
+ enum rtpse_cmd pw_set_cmd; /* command used by set_pw_limit */
+ u32 pw_set_lsb_mW; /* LSB of pw_set_cmd value, in mW */
+ u32 pw_read_lsb_mW; /* LSB of ext_config.max_power read-back, in mW */
+};
+
+/* Parsed MCU response structures (decoded from rtpse_mcu_msg replies) */
+
+struct rtpse_mcu_info {
+ u8 max_ports;
+ bool system_enable;
+ u16 device_id;
+ u8 sw_ver;
+ u8 mcu_type;
+ u8 config_status;
+ u8 ext_ver;
+};
+
+struct rtpse_mcu_ext_config {
+ u8 uvlo;
+ u8 ovlo;
+ bool prealloc_enable;
+ u8 num_of_pses;
+};
+
+struct rtpse_port_status {
+ u8 sts1;
+ u8 sts2;
+ u8 sts3;
+};
+
+struct rtpse_port_measurement {
+ u16 voltage_raw; /* 64.45mV/LSB */
+ u16 current_raw; /* 1mA/LSB */
+ u16 temperature_raw; /* T(mC) = 1250 * (220 - raw) */
+ u16 power_raw; /* 100mW/LSB */
+};
+
+struct rtpse_port_config {
+ bool enable;
+ u8 function_mode;
+ u8 detection_type;
+ u8 cls_type;
+ u8 disconnect_type;
+ u8 pair_type;
+};
+
+struct rtpse_port_ext_config {
+ u8 inrush_mode;
+ u8 limit_type;
+ u8 max_power;
+ u8 priority;
+ u8 chip_addr;
+ u8 channel;
+};
+
+static const struct rtpse_chip_info rtl8238b_info = {
+ .device_id = RTPSE_DEVICE_ID_RTL8238B,
+ .max_mW_per_port = 30000,
+ .name = "RTL8238B",
+ .pw_read_lsb_mW = 200,
+ .pw_set_cmd = RTPSE_CMD_PORT_SET_POWER_LIMIT,
+ .pw_set_lsb_mW = 200,
+};
+
+static const struct rtpse_chip_info rtl8239_info = {
+ .device_id = RTPSE_DEVICE_ID_RTL8239,
+ .max_mW_per_port = 90000,
+ .name = "RTL8239",
+ .pw_read_lsb_mW = 400,
+ .pw_set_cmd = RTPSE_CMD_PORT_SET_POWER_LIMIT_EXT,
+ .pw_set_lsb_mW = 400,
+};
+
+static const struct rtpse_chip_info rtl8239c_info = {
+ .device_id = RTPSE_DEVICE_ID_RTL8239C,
+ .max_mW_per_port = 90000,
+ .name = "RTL8239C",
+ .pw_read_lsb_mW = 400,
+ .pw_set_cmd = RTPSE_CMD_PORT_SET_POWER_LIMIT_EXT,
+ .pw_set_lsb_mW = 400,
+};
+
+static const struct rtpse_chip_info bcm59111_info = {
+ .device_id = RTPSE_DEVICE_ID_BCM59111,
+ .max_mW_per_port = 30000,
+ .name = "BCM59111",
+ .pw_read_lsb_mW = 200,
+ .pw_set_cmd = RTPSE_CMD_PORT_SET_POWER_LIMIT,
+ .pw_set_lsb_mW = 200,
+};
+
+static const struct rtpse_chip_info bcm59121_info = {
+ .device_id = RTPSE_DEVICE_ID_BCM59121,
+ /*
+ * BCM59121 is a 60W Type-3 part, but known boards run it at 802.3at
+ * and the BCM dialect has only the 8-bit/0.2W set command (<=51W);
+ * cap at the 30W the hardware actually offers.
+ */
+ .max_mW_per_port = 30000,
+ .name = "BCM59121",
+ .pw_read_lsb_mW = 200,
+ .pw_set_cmd = RTPSE_CMD_PORT_SET_POWER_LIMIT,
+ .pw_set_lsb_mW = 200,
+};
+
+/* Helpers and basic functions */
+
+static inline struct rtpse_ctrl *to_rtpse_ctrl(struct pse_controller_dev *pcdev)
+{
+ return container_of(pcdev, struct rtpse_ctrl, pcdev);
+}
+
+bool rtpse_needs_i2c_proto(const struct rtpse_match_data *match)
+{
+ return match->i2c_proto_dt_required;
+}
+EXPORT_SYMBOL_GPL(rtpse_needs_i2c_proto);
+
+static inline void rtpse_mcu_msg_init(struct rtpse_mcu_msg *msg, u8 opcode)
+{
+ memset(msg, 0xff, sizeof(*msg));
+ msg->opcode = opcode;
+}
+
+static u8 rtpse_checksum(const u8 *buf, size_t len)
+{
+ u8 sum = 0;
+
+ while (len--)
+ sum += *buf++;
+ return sum;
+}
+
+static int rtpse_do_xfer(struct rtpse_ctrl *pse, struct rtpse_mcu_msg *req,
+ struct rtpse_mcu_msg *resp)
+{
+ int ret;
+
+ req->checksum = rtpse_checksum((u8 *)req, RTPSE_MCU_MSG_SIZE - 1);
+
+ scoped_guard(mutex, &pse->mutex) {
+ ret = pse->transport->send(pse, req);
+ if (ret)
+ return ret;
+
+ /*
+ * The MCU needs a fixed amount of time between receiving a request
+ * and having the response ready, regardless of how the bytes get to
+ * us. Pace the transaction here so each transport can keep its recv
+ * path simple: a single bounded wait rather than a generic retry.
+ */
+ msleep(RTPSE_MCU_RESPONSE_MS);
+
+ memset(resp, 0, sizeof(*resp));
+ ret = pse->transport->recv(pse, req, resp);
+ if (ret)
+ return ret;
+ }
+
+ /*
+ * Explicit MCU error opcodes (observed on the BCM dialect; harmless
+ * to check for RTL too). Catch these before the generic opcode/CRC
+ * mismatch path so callers see a meaningful errno.
+ */
+ switch (resp->opcode) {
+ case RTPSE_OPCODE_INCOMPLETE: return -EBADE;
+ case RTPSE_OPCODE_BAD_CSUM: return -EBADMSG;
+ case RTPSE_OPCODE_NOT_READY: return -EAGAIN;
+ }
+
+ if (resp->opcode != req->opcode ||
+ resp->checksum != rtpse_checksum((u8 *)resp, RTPSE_MCU_MSG_SIZE - 1))
+ return -EBADMSG;
+
+ return 0;
+}
+
+static int rtpse_port_query(struct rtpse_ctrl *pse, unsigned int port, u8 opcode,
+ struct rtpse_mcu_msg *resp)
+{
+ struct rtpse_mcu_msg req;
+ int ret;
+
+ rtpse_mcu_msg_init(&req, opcode);
+ req.payload[0] = port;
+
+ ret = rtpse_do_xfer(pse, &req, resp);
+ if (ret)
+ return ret;
+
+ if (resp->payload[0] != port)
+ return -EIO;
+
+ return 0;
+}
+
+static int rtpse_port_cmd(struct rtpse_ctrl *pse, unsigned int port, u8 opcode, u8 arg)
+{
+ struct rtpse_mcu_msg req, resp;
+ int ret;
+
+ rtpse_mcu_msg_init(&req, opcode);
+ req.payload[0] = port;
+ req.payload[1] = arg;
+
+ ret = rtpse_do_xfer(pse, &req, &resp);
+ if (ret)
+ return ret;
+
+ if (resp.payload[0] != port || resp.payload[1] != 0)
+ return -EIO;
+
+ return 0;
+}
+
+/* Global operations */
+
+static int rtpse_mcu_get_info(struct rtpse_ctrl *pse, struct rtpse_mcu_info *info)
+{
+ struct rtpse_mcu_msg req, resp;
+ const struct rtpse_opcode *opc;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_MCU_GET_SYSTEM_INFO];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ rtpse_mcu_msg_init(&req, opc->op);
+ ret = rtpse_do_xfer(pse, &req, &resp);
+ if (ret)
+ return ret;
+
+ return pse->dialect->parse_system_info(resp.payload, info);
+}
+
+static int rtpse_mcu_get_ext_config(struct rtpse_ctrl *pse, struct rtpse_mcu_ext_config *config)
+{
+ struct rtpse_mcu_msg req, resp;
+ const struct rtpse_opcode *opc;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_MCU_GET_EXT_CONFIG];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ rtpse_mcu_msg_init(&req, opc->op);
+ ret = rtpse_do_xfer(pse, &req, &resp);
+ if (ret)
+ return ret;
+
+ config->uvlo = resp.payload[0];
+ config->ovlo = resp.payload[5];
+ config->prealloc_enable = (resp.payload[1] == 0x1);
+ config->num_of_pses = resp.payload[6];
+
+ return 0;
+}
+
+static int rtpse_set_global_state(struct rtpse_ctrl *pse, bool enable)
+{
+ struct rtpse_mcu_msg req, resp;
+ const struct rtpse_opcode *opc;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_MCU_SET_GLOBAL_STATE];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ rtpse_mcu_msg_init(&req, opc->op);
+ req.payload[0] = enable ? 0x1 : 0x0;
+
+ ret = rtpse_do_xfer(pse, &req, &resp);
+ if (ret)
+ return ret;
+
+ return (resp.payload[0] == 0x0) ? 0 : -EIO;
+}
+
+/* Port operations */
+
+static int rtpse_port_get_status(struct rtpse_ctrl *pse, unsigned int port,
+ struct rtpse_port_status *status)
+{
+ const struct rtpse_opcode *opc;
+ struct rtpse_mcu_msg resp;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_PORT_GET_STATUS];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ ret = rtpse_port_query(pse, port, opc->op, &resp);
+ if (ret)
+ return ret;
+
+ status->sts1 = resp.payload[1];
+ status->sts2 = resp.payload[2];
+ status->sts3 = resp.payload[3];
+
+ return 0;
+}
+
+static int rtpse_port_get_measurement(struct rtpse_ctrl *pse, unsigned int port,
+ struct rtpse_port_measurement *measurement)
+{
+ const struct rtpse_opcode *opc;
+ struct rtpse_mcu_msg resp;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_PORT_GET_POWER_STATS];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ ret = rtpse_port_query(pse, port, opc->op, &resp);
+ if (ret)
+ return ret;
+
+ measurement->voltage_raw = get_unaligned_be16(&resp.payload[1]);
+ measurement->current_raw = get_unaligned_be16(&resp.payload[3]);
+ measurement->temperature_raw = get_unaligned_be16(&resp.payload[5]);
+ measurement->power_raw = get_unaligned_be16(&resp.payload[7]);
+
+ return 0;
+}
+
+static int rtpse_port_get_config(struct rtpse_ctrl *pse, unsigned int port,
+ struct rtpse_port_config *config)
+{
+ const struct rtpse_opcode *opc;
+ struct rtpse_mcu_msg resp;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_PORT_GET_CONFIG];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ ret = rtpse_port_query(pse, port, opc->op, &resp);
+ if (ret)
+ return ret;
+
+ config->enable = (resp.payload[1] == 1);
+ config->function_mode = resp.payload[2];
+ config->detection_type = resp.payload[3];
+ config->cls_type = resp.payload[4];
+ config->disconnect_type = resp.payload[5];
+ config->pair_type = resp.payload[6];
+
+ return 0;
+}
+
+static int rtpse_port_get_ext_config(struct rtpse_ctrl *pse, unsigned int port,
+ struct rtpse_port_ext_config *config)
+{
+ const struct rtpse_opcode *opc;
+ struct rtpse_mcu_msg resp;
+ int ret;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_PORT_GET_EXT_CONFIG];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ ret = rtpse_port_query(pse, port, opc->op, &resp);
+ if (ret)
+ return ret;
+
+ config->inrush_mode = resp.payload[1];
+ config->limit_type = resp.payload[2];
+ config->max_power = resp.payload[3];
+ config->priority = resp.payload[4];
+ config->chip_addr = resp.payload[5];
+ config->channel = resp.payload[6];
+
+ return 0;
+}
+
+static int rtpse_port_set_state(struct rtpse_ctrl *pse, unsigned int port, bool enable)
+{
+ const struct rtpse_opcode *opc;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_PORT_ENABLE];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ return rtpse_port_cmd(pse, port, opc->op, enable ? 0x1 : 0x0);
+}
+
+/*
+ * PSE controller ops
+ *
+ * @id is the PSE PI index from DT, used directly as the MCU port number.
+ * This assumes 0-based, contiguous MCU port addressing. Boards whose PSE
+ * outputs are wired to the chip at an offset would need a PI-index ->
+ * MCU-port mapping here.
+ */
+
+static int rtpse_port_get_admin_state(struct pse_controller_dev *pcdev, int id,
+ struct pse_admin_state *admin_state)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_config config;
+ int ret;
+
+ ret = rtpse_port_get_config(pse, id, &config);
+ if (ret)
+ return ret;
+
+ admin_state->c33_admin_state = config.enable ? ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED :
+ ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED;
+ return 0;
+}
+
+static int rtpse_port_get_pw_status(struct pse_controller_dev *pcdev, int id,
+ struct pse_pw_status *pw_status)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_status status;
+ int ret;
+
+ ret = rtpse_port_get_status(pse, id, &status);
+ if (ret)
+ return ret;
+
+ switch (status.sts1) {
+ case RTPSE_PORT_STS_DISABLED:
+ pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_DISABLED;
+ break;
+ case RTPSE_PORT_STS_SEARCHING:
+ case RTPSE_PORT_STS_REQUESTING:
+ pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_SEARCHING;
+ break;
+ case RTPSE_PORT_STS_DELIVERING:
+ pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_DELIVERING;
+ break;
+ case RTPSE_PORT_STS_FAULT:
+ pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_FAULT;
+ break;
+ default:
+ pw_status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_UNKNOWN;
+ break;
+ }
+
+ return 0;
+}
+
+static int rtpse_port_get_pw_class(struct pse_controller_dev *pcdev, int id)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_status status;
+ int ret;
+
+ ret = rtpse_port_get_status(pse, id, &status);
+ if (ret)
+ return ret;
+
+ /*
+ * sts2 carries detection+classification only when sts1 is not a
+ * fault state; in fault states it encodes the fault type instead.
+ * Treat the two reserved sts1 codes (0x3, 0x5) as faults too, since
+ * the datasheet hints at "other fault" beyond the explicit 0x4.
+ */
+ switch (status.sts1) {
+ case RTPSE_PORT_STS_DISABLED:
+ case RTPSE_PORT_STS_SEARCHING:
+ case RTPSE_PORT_STS_DELIVERING:
+ case RTPSE_PORT_STS_REQUESTING:
+ return pse->dialect->parse_port_class(&status);
+ default:
+ return 0;
+ }
+}
+
+static int rtpse_port_get_actual_pw(struct pse_controller_dev *pcdev, int id)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_measurement measurement;
+ int ret;
+
+ ret = rtpse_port_get_measurement(pse, id, &measurement);
+ if (ret)
+ return ret;
+
+ /* 100mW per LSB */
+ return measurement.power_raw * 100U;
+}
+
+static int rtpse_port_get_voltage(struct pse_controller_dev *pcdev, int id)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_measurement measurement;
+ int ret;
+ u32 uV;
+
+ ret = rtpse_port_get_measurement(pse, id, &measurement);
+ if (ret)
+ return ret;
+
+ /* 64.45mV per LSB */
+ uV = (u32)measurement.voltage_raw * 64450U;
+ return min_t(u32, uV, INT_MAX);
+}
+
+static int rtpse_port_enable(struct pse_controller_dev *pcdev, int id)
+{
+ return rtpse_port_set_state(to_rtpse_ctrl(pcdev), id, true);
+}
+
+static int rtpse_port_disable(struct pse_controller_dev *pcdev, int id)
+{
+ return rtpse_port_set_state(to_rtpse_ctrl(pcdev), id, false);
+}
+
+static int rtpse_port_get_pw_limit(struct pse_controller_dev *pcdev, int id)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_ext_config config;
+ int ret;
+
+ ret = rtpse_port_get_ext_config(pse, id, &config);
+ if (ret)
+ return ret;
+
+ return config.max_power * pse->chip->pw_read_lsb_mW;
+}
+
+static int rtpse_port_set_pw_limit(struct pse_controller_dev *pcdev, int id, int max_mW)
+{
+ const struct rtpse_opcode *type_opc, *val_opc;
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ const struct rtpse_chip_info *chip = pse->chip;
+ unsigned int prg_val;
+ int ret;
+
+ if (max_mW < 0 || max_mW > chip->max_mW_per_port)
+ return -ERANGE;
+
+ type_opc = &pse->dialect->opcode[RTPSE_CMD_PORT_SET_POWER_LIMIT_TYPE];
+ val_opc = &pse->dialect->opcode[chip->pw_set_cmd];
+ if (!type_opc->valid || !val_opc->valid)
+ return -EOPNOTSUPP;
+
+ /*
+ * Switch the port to user-defined limit mode first, then program the
+ * limit value. If the second cmd fails, the port is left in
+ * user-defined mode but with the previous limit value; the next
+ * successful set_pw_limit call recovers it.
+ */
+ ret = rtpse_port_cmd(pse, id, type_opc->op, RTPSE_PORT_PW_LIMIT_TYPE_USER);
+ if (ret)
+ return ret;
+
+ prg_val = min_t(unsigned int, max_mW / chip->pw_set_lsb_mW, 0xff);
+
+ return rtpse_port_cmd(pse, id, val_opc->op, prg_val);
+}
+
+static int rtpse_port_get_pw_limit_ranges(struct pse_controller_dev *pcdev, int id,
+ struct pse_pw_limit_ranges *out)
+{
+ struct ethtool_c33_pse_pw_limit_range *range;
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+
+ range = kzalloc_obj(*range, GFP_KERNEL);
+ if (!range)
+ return -ENOMEM;
+
+ range[0].min = 0;
+ range[0].max = pse->chip->max_mW_per_port;
+
+ out->c33_pw_limit_ranges = range;
+ return 1;
+}
+
+static int rtpse_port_get_prio(struct pse_controller_dev *pcdev, int id)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ struct rtpse_port_ext_config config;
+ int ret;
+
+ ret = rtpse_port_get_ext_config(pse, id, &config);
+ if (ret)
+ return ret;
+
+ return config.priority;
+}
+
+static int rtpse_port_set_prio(struct pse_controller_dev *pcdev, int id, unsigned int prio)
+{
+ struct rtpse_ctrl *pse = to_rtpse_ctrl(pcdev);
+ const struct rtpse_opcode *opc;
+
+ if (prio > RTPSE_PORT_MAX_PRIORITY)
+ return -ERANGE;
+
+ opc = &pse->dialect->opcode[RTPSE_CMD_PORT_SET_PRIORITY];
+ if (!opc->valid)
+ return -EOPNOTSUPP;
+
+ return rtpse_port_cmd(pse, id, opc->op, prio);
+}
+
+static const struct pse_controller_ops rtpse_ops = {
+ .pi_get_admin_state = rtpse_port_get_admin_state,
+ .pi_get_pw_status = rtpse_port_get_pw_status,
+ .pi_get_pw_class = rtpse_port_get_pw_class,
+ .pi_get_actual_pw = rtpse_port_get_actual_pw,
+ .pi_enable = rtpse_port_enable,
+ .pi_disable = rtpse_port_disable,
+ .pi_get_voltage = rtpse_port_get_voltage,
+ .pi_get_pw_limit = rtpse_port_get_pw_limit,
+ .pi_set_pw_limit = rtpse_port_set_pw_limit,
+ .pi_get_pw_limit_ranges = rtpse_port_get_pw_limit_ranges,
+ .pi_get_prio = rtpse_port_get_prio,
+ .pi_set_prio = rtpse_port_set_prio,
+};
+
+static int rtpse_discover(struct rtpse_ctrl *pse, struct rtpse_mcu_info *info)
+{
+ struct rtpse_mcu_ext_config ext_config;
+ unsigned long deadline;
+ int ret;
+
+ /*
+ * The MCU may not answer on the bus yet right after power-up or
+ * enable-gpios assertion: depending on the transport it either stays
+ * silent (-ETIMEDOUT) or does not ACK its address at all (-ENXIO /
+ * -EREMOTEIO). Retry within a bounded wall-time window so a slow boot
+ * still probes, while a genuinely unresponsive MCU fails with its real
+ * error instead of deferring forever and masking it.
+ */
+ deadline = jiffies + msecs_to_jiffies(RTPSE_MCU_BOOT_TIMEOUT_MS);
+ do {
+ ret = rtpse_mcu_get_info(pse, info);
+ if (ret != -ETIMEDOUT && ret != -ENXIO &&
+ ret != -EREMOTEIO && ret != -EAGAIN)
+ break;
+ msleep(RTPSE_MCU_BOOT_RETRY_MS);
+ } while (time_before(jiffies, deadline));
+ if (ret)
+ return dev_err_probe(pse->dev, ret, "failed to read MCU info\n");
+
+ switch (info->device_id) {
+ case RTPSE_DEVICE_ID_RTL8238B:
+ pse->chip = &rtl8238b_info;
+ break;
+ case RTPSE_DEVICE_ID_RTL8239:
+ pse->chip = &rtl8239_info;
+ break;
+ case RTPSE_DEVICE_ID_RTL8239C:
+ pse->chip = &rtl8239c_info;
+ break;
+ case RTPSE_DEVICE_ID_BCM59111:
+ pse->chip = &bcm59111_info;
+ break;
+ case RTPSE_DEVICE_ID_BCM59121:
+ pse->chip = &bcm59121_info;
+ break;
+ default:
+ return dev_err_probe(pse->dev, -EINVAL, "unknown PSE id 0x%x\n",
+ info->device_id);
+ }
+
+ if (!info->max_ports || info->max_ports > RTPSE_MAX_PORTS)
+ return dev_err_probe(pse->dev, -EINVAL,
+ "MCU reports invalid port count %u\n", info->max_ports);
+
+ ret = rtpse_mcu_get_ext_config(pse, &ext_config);
+ if (ret)
+ return dev_err_probe(pse->dev, ret, "failed to read MCU ext config\n");
+
+ dev_info(pse->dev, "%s MCU, %s (id 0x%04x), %u ports across %u PSE chip(s)\n",
+ pse->dialect->mcu_type_str(info->mcu_type), pse->chip->name,
+ info->device_id, info->max_ports, ext_config.num_of_pses);
+ return 0;
+}
+
+static void rtpse_regulator_disable(void *data)
+{
+ regulator_disable(data);
+}
+
+int rtpse_register(struct rtpse_ctrl *pse)
+{
+ const struct rtpse_match_data *match;
+ struct gpio_desc *enable_gpio;
+ struct rtpse_mcu_info info;
+ int ret;
+
+ BUILD_BUG_ON(sizeof(struct rtpse_mcu_msg) != RTPSE_MCU_MSG_SIZE);
+
+ ret = devm_mutex_init(pse->dev, &pse->mutex);
+ if (ret)
+ return ret;
+
+ match = device_get_match_data(pse->dev);
+ if (!match)
+ return dev_err_probe(pse->dev, -ENODEV, "missing match data\n");
+ pse->dialect = match->dialect;
+
+ /*
+ * Catch a dialect that forgot to set one of the required hooks at
+ * probe time, rather than NULL-deref'ing later from a fast path.
+ */
+ if (!pse->dialect ||
+ !pse->dialect->parse_system_info ||
+ !pse->dialect->parse_port_class ||
+ !pse->dialect->mcu_type_str)
+ return dev_err_probe(pse->dev, -EINVAL,
+ "dialect for chip is incomplete\n");
+
+ pse->poe_supply = devm_regulator_get(pse->dev, "power");
+ if (IS_ERR(pse->poe_supply))
+ return dev_err_probe(pse->dev, PTR_ERR(pse->poe_supply),
+ "failed to get PoE supply\n");
+
+ enable_gpio = devm_gpiod_get_optional(pse->dev, "enable", GPIOD_OUT_HIGH);
+ if (IS_ERR(enable_gpio))
+ return dev_err_probe(pse->dev, PTR_ERR(enable_gpio),
+ "failed to get enable gpio\n");
+
+ ret = rtpse_discover(pse, &info);
+ if (ret)
+ return ret;
+
+ if (!info.system_enable) {
+ ret = rtpse_set_global_state(pse, true);
+ /* Dialects without a global-state concept (e.g. BCM) return
+ * -EOPNOTSUPP; treat that as "no separate enable required".
+ */
+ if (ret && ret != -EOPNOTSUPP)
+ return dev_err_probe(pse->dev, ret,
+ "failed to enable PSE system\n");
+ }
+
+ ret = regulator_enable(pse->poe_supply);
+ if (ret)
+ return dev_err_probe(pse->dev, ret, "failed to enable PoE supply\n");
+
+ ret = devm_add_action_or_reset(pse->dev, rtpse_regulator_disable, pse->poe_supply);
+ if (ret)
+ return ret;
+
+ /*
+ * Depending on the MCU firmware configuration (which might be different
+ * for every board), it isn't known whether the PoE subsystem is active or
+ * inactive by default. At this stage, the PSE chips might already deliver
+ * power to PDs without any explicit enable.
+ */
+
+ pse->pcdev.owner = THIS_MODULE;
+ pse->pcdev.ops = &rtpse_ops;
+ pse->pcdev.dev = pse->dev;
+ pse->pcdev.types = ETHTOOL_PSE_C33;
+ pse->pcdev.nr_lines = info.max_ports;
+ pse->pcdev.pis_prio_max = RTPSE_PORT_MAX_PRIORITY;
+ pse->pcdev.supp_budget_eval_strategies = PSE_BUDGET_EVAL_STRAT_DYNAMIC;
+
+ return devm_pse_controller_register(pse->dev, &pse->pcdev);
+}
+EXPORT_SYMBOL_GPL(rtpse_register);
+
+static int rtpse_rtl_parse_system_info(const u8 *payload, struct rtpse_mcu_info *info)
+{
+ info->max_ports = payload[1];
+ info->system_enable = (payload[2] == 0x1);
+ info->device_id = get_unaligned_be16(&payload[3]);
+ info->sw_ver = payload[5];
+ info->mcu_type = payload[6];
+ info->config_status = payload[7];
+ info->ext_ver = payload[8];
+ return 0;
+}
+
+static int rtpse_rtl_parse_port_class(const struct rtpse_port_status *status)
+{
+ /* Class lives in the upper nibble of sts2. */
+ return FIELD_GET(GENMASK(7, 4), status->sts2);
+}
+
+static const char *rtpse_rtl_mcu_type_str(unsigned int mcu_type)
+{
+ switch (mcu_type) {
+ case 0x00: return "GigaDevice GD32F310";
+ case 0x01: return "GigaDevice GD32F230";
+ case 0x02: return "GigaDevice GD32F303";
+ case 0x03: return "GigaDevice GD32F103";
+ case 0x04: return "GigaDevice GD32E103";
+ case 0x10: return "Nuvoton M0516";
+ case 0x11: return "Nuvoton M0564";
+ case 0x12: return "Nuvoton NUC029";
+ default: return "unknown";
+ }
+}
+
+static int rtpse_bcm_parse_system_info(const u8 *payload, struct rtpse_mcu_info *info)
+{
+ info->max_ports = payload[1];
+ /* BCM has no explicit system_enable byte; the closest analog is the
+ * "remote enable" bit in the system-status flags at payload[7].
+ */
+ info->system_enable = !!(payload[7] & BIT(2));
+ info->device_id = get_unaligned_be16(&payload[3]);
+ info->sw_ver = payload[5];
+ info->mcu_type = payload[6];
+ info->config_status = payload[7];
+ info->ext_ver = payload[8];
+ return 0;
+}
+
+static int rtpse_bcm_parse_port_class(const struct rtpse_port_status *status)
+{
+ /* BCM puts the detected class in payload[3] (== sts3) directly.
+ * Mask to the low nibble; class is 0..8 and any high bits would be
+ * noise.
+ */
+ return status->sts3 & 0x0f;
+}
+
+static const char *rtpse_bcm_mcu_type_str(unsigned int mcu_type)
+{
+ switch (mcu_type) {
+ case 0x00: return "ST Micro ST32F100";
+ case 0x01: return "Nuvoton M05xx LAN";
+ case 0x02: return "ST Micro STF030C8";
+ case 0x03: return "Nuvoton M058SAN";
+ case 0x04: return "Nuvoton NUC122";
+ default: return "unknown";
+ }
+}
+
+/* Map each logical command the core issues to its per-dialect opcode. */
+static const struct rtpse_mcu_dialect rtpse_dialect_rtk = {
+ .parse_system_info = rtpse_rtl_parse_system_info,
+ .parse_port_class = rtpse_rtl_parse_port_class,
+ .mcu_type_str = rtpse_rtl_mcu_type_str,
+ .opcode = {
+ [RTPSE_CMD_MCU_SET_GLOBAL_STATE] = RTPSE_OP(0x00),
+ [RTPSE_CMD_MCU_GET_SYSTEM_INFO] = RTPSE_OP(0x40),
+ [RTPSE_CMD_MCU_GET_EXT_CONFIG] = RTPSE_OP(0x4a),
+
+ [RTPSE_CMD_PORT_ENABLE] = RTPSE_OP(0x01),
+ [RTPSE_CMD_PORT_SET_POWER_LIMIT_TYPE] = RTPSE_OP(0x12),
+ [RTPSE_CMD_PORT_SET_POWER_LIMIT] = RTPSE_OP(0x13),
+ [RTPSE_CMD_PORT_SET_POWER_LIMIT_EXT] = RTPSE_OP(0x14),
+ [RTPSE_CMD_PORT_SET_PRIORITY] = RTPSE_OP(0x15),
+ [RTPSE_CMD_PORT_GET_STATUS] = RTPSE_OP(0x42),
+ [RTPSE_CMD_PORT_GET_POWER_STATS] = RTPSE_OP(0x44),
+ [RTPSE_CMD_PORT_GET_CONFIG] = RTPSE_OP(0x48),
+ [RTPSE_CMD_PORT_GET_EXT_CONFIG] = RTPSE_OP(0x49),
+ },
+};
+
+static const struct rtpse_mcu_dialect rtpse_dialect_bcm = {
+ .parse_system_info = rtpse_bcm_parse_system_info,
+ .parse_port_class = rtpse_bcm_parse_port_class,
+ .mcu_type_str = rtpse_bcm_mcu_type_str,
+ .opcode = {
+ [RTPSE_CMD_MCU_GET_SYSTEM_INFO] = RTPSE_OP(0x20),
+ [RTPSE_CMD_MCU_GET_EXT_CONFIG] = RTPSE_OP(0x2b),
+
+ [RTPSE_CMD_PORT_ENABLE] = RTPSE_OP(0x00),
+ [RTPSE_CMD_PORT_SET_POWER_LIMIT_TYPE] = RTPSE_OP(0x15),
+ [RTPSE_CMD_PORT_SET_POWER_LIMIT] = RTPSE_OP(0x16),
+ [RTPSE_CMD_PORT_SET_PRIORITY] = RTPSE_OP(0x1a),
+ [RTPSE_CMD_PORT_GET_STATUS] = RTPSE_OP(0x21),
+ [RTPSE_CMD_PORT_GET_POWER_STATS] = RTPSE_OP(0x30),
+ [RTPSE_CMD_PORT_GET_CONFIG] = RTPSE_OP(0x25),
+ [RTPSE_CMD_PORT_GET_EXT_CONFIG] = RTPSE_OP(0x26),
+ },
+};
+
+const struct rtpse_match_data rtpse_rtk_data = {
+ .dialect = &rtpse_dialect_rtk,
+ .i2c_proto_dt_required = true,
+};
+EXPORT_SYMBOL_GPL(rtpse_rtk_data);
+
+const struct rtpse_match_data rtpse_bcm_data = {
+ .dialect = &rtpse_dialect_bcm,
+};
+EXPORT_SYMBOL_GPL(rtpse_bcm_data);
+
+MODULE_DESCRIPTION("Realtek/Broadcom PSE MCU driver (core)");
+MODULE_AUTHOR("Jonas Jelonek <jelonek.jonas@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/pse-pd/realtek-pse-i2c.c b/drivers/net/pse-pd/realtek-pse-i2c.c
new file mode 100644
index 000000000000..d89bd93da7e0
--- /dev/null
+++ b/drivers/net/pse-pd/realtek-pse-i2c.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pse-pd/pse.h>
+
+#include "realtek-pse.h"
+
+/*
+ * The core has already waited RTPSE_MCU_RESPONSE_MS before calling us, so
+ * the response is normally ready on the very first read. For commands the
+ * MCU produces more slowly, keep polling at the typical response cadence
+ * up to the worst-case ceiling.
+ */
+#define RTPSE_I2C_RETRY_MS RTPSE_MCU_RESPONSE_MS
+#define RTPSE_I2C_MAX_TRIES (RTPSE_MCU_RESPONSE_MAX_MS / RTPSE_I2C_RETRY_MS)
+
+static int rtpse_i2c_smbus_send(struct rtpse_ctrl *pse, const struct rtpse_mcu_msg *req)
+{
+ struct i2c_client *client = to_i2c_client(pse->dev);
+
+ /* Send opcode as SMBus command byte; remaining 11 bytes as block data */
+ return i2c_smbus_write_i2c_block_data(client, req->opcode, RTPSE_MCU_MSG_SIZE - 1,
+ (u8 *)req + 1);
+}
+
+static int rtpse_i2c_smbus_recv(struct rtpse_ctrl *pse,
+ const struct rtpse_mcu_msg *req,
+ struct rtpse_mcu_msg *resp)
+{
+ struct i2c_client *client = to_i2c_client(pse->dev);
+ int tries, ret;
+
+ for (tries = 0; tries < RTPSE_I2C_MAX_TRIES; tries++) {
+ if (tries > 0)
+ msleep(RTPSE_I2C_RETRY_MS);
+
+ /* MCU needs 0x00 as command byte for read */
+ ret = i2c_smbus_read_i2c_block_data(client, 0x00,
+ RTPSE_MCU_MSG_SIZE,
+ (u8 *)resp);
+ if (ret < 0)
+ return ret;
+ if (ret == RTPSE_MCU_MSG_SIZE && rtpse_resp_is_final(req, resp))
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static const struct rtpse_transport_ops rtpse_i2c_smbus_ops = {
+ .send = rtpse_i2c_smbus_send,
+ .recv = rtpse_i2c_smbus_recv,
+};
+
+static int rtpse_i2c_native_send(struct rtpse_ctrl *pse, const struct rtpse_mcu_msg *req)
+{
+ struct i2c_client *client = to_i2c_client(pse->dev);
+ int ret;
+
+ ret = i2c_master_send(client, (const u8 *)req, RTPSE_MCU_MSG_SIZE);
+ if (ret < 0)
+ return ret;
+ return ret == RTPSE_MCU_MSG_SIZE ? 0 : -EIO;
+}
+
+static int rtpse_i2c_native_recv(struct rtpse_ctrl *pse,
+ const struct rtpse_mcu_msg *req,
+ struct rtpse_mcu_msg *resp)
+{
+ struct i2c_client *client = to_i2c_client(pse->dev);
+ int tries, ret;
+
+ for (tries = 0; tries < RTPSE_I2C_MAX_TRIES; tries++) {
+ if (tries > 0)
+ msleep(RTPSE_I2C_RETRY_MS);
+
+ ret = i2c_master_recv(client, (u8 *)resp, RTPSE_MCU_MSG_SIZE);
+ if (ret < 0)
+ return ret;
+ if (ret == RTPSE_MCU_MSG_SIZE && rtpse_resp_is_final(req, resp))
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static const struct rtpse_transport_ops rtpse_i2c_native_ops = {
+ .send = rtpse_i2c_native_send,
+ .recv = rtpse_i2c_native_recv,
+};
+
+static int rtpse_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ const struct rtpse_match_data *match;
+ struct rtpse_ctrl *pse;
+ bool use_native = false;
+ int ret;
+
+ match = device_get_match_data(dev);
+ if (!match)
+ return dev_err_probe(dev, -ENODEV, "missing match data\n");
+
+ if (rtpse_needs_i2c_proto(match)) {
+ const char *proto;
+
+ ret = device_property_read_string(dev, "realtek,i2c-protocol", &proto);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "missing required \"realtek,i2c-protocol\" property\n");
+
+ if (!strcmp(proto, "i2c"))
+ use_native = true;
+ else if (!strcmp(proto, "smbus"))
+ use_native = false;
+ else
+ return dev_err_probe(dev, -EINVAL,
+ "unknown realtek,i2c-protocol \"%s\"\n", proto);
+ }
+
+ if (use_native) {
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+ return dev_err_probe(dev, -EOPNOTSUPP,
+ "plain-I2C MCU protocol requires I2C-capable adapter\n");
+ } else {
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK))
+ return dev_err_probe(dev, -EOPNOTSUPP,
+ "SMBus MCU protocol requires SMBus I2C-block support\n");
+ }
+
+ pse = devm_kzalloc(dev, sizeof(*pse), GFP_KERNEL);
+ if (!pse)
+ return -ENOMEM;
+
+ pse->dev = dev;
+ pse->transport = use_native ? &rtpse_i2c_native_ops : &rtpse_i2c_smbus_ops;
+
+ return rtpse_register(pse);
+}
+
+static const struct of_device_id rtpse_i2c_of_match[] = {
+ { .compatible = "realtek,pse-mcu-rtk", .data = &rtpse_rtk_data },
+ { .compatible = "realtek,pse-mcu-bcm", .data = &rtpse_bcm_data },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rtpse_i2c_of_match);
+
+static struct i2c_driver rtpse_i2c_driver = {
+ .driver = {
+ .name = "realtek-pse-i2c",
+ .of_match_table = rtpse_i2c_of_match,
+ },
+ .probe = rtpse_i2c_probe,
+};
+module_i2c_driver(rtpse_i2c_driver);
+
+MODULE_AUTHOR("Jonas Jelonek <jelonek.jonas@gmail.com>");
+MODULE_DESCRIPTION("Realtek/Broadcom PSE MCU driver (I2C transport)");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/pse-pd/realtek-pse-uart.c b/drivers/net/pse-pd/realtek-pse-uart.c
new file mode 100644
index 000000000000..785407b379a4
--- /dev/null
+++ b/drivers/net/pse-pd/realtek-pse-uart.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pse-pd/pse.h>
+#include <linux/serdev.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+
+#include "realtek-pse.h"
+
+#define RTPSE_UART_BAUD_DEFAULT 19200
+#define RTPSE_UART_TX_TIMEOUT msecs_to_jiffies(100)
+#define RTPSE_UART_RX_TIMEOUT msecs_to_jiffies(RTPSE_MCU_RESPONSE_MAX_MS)
+
+struct rtpse_uart {
+ struct rtpse_ctrl pse;
+ struct serdev_device *serdev;
+ struct completion rx_done;
+ spinlock_t rx_lock; /* protects rx_buf and rx_len */
+ size_t rx_len;
+ u8 rx_buf[RTPSE_MCU_MSG_SIZE];
+};
+
+#define to_rtpse_uart(p) container_of(p, struct rtpse_uart, pse)
+
+/*
+ * No framing is done here: a glitched frame costs one transaction, then
+ * the next _send re-frames from rx_len 0. Resync works by returning count
+ * (not take), dropping any overflow so serdev keeps no leftover to bleed
+ * into the next frame.
+ */
+static size_t rtpse_uart_receive(struct serdev_device *serdev,
+ const u8 *buf, size_t count)
+{
+ struct rtpse_uart *ctx = serdev_device_get_drvdata(serdev);
+ bool done = false;
+ size_t take;
+
+ scoped_guard(spinlock_irqsave, &ctx->rx_lock) {
+ take = min(count, sizeof(ctx->rx_buf) - ctx->rx_len);
+ if (take) {
+ memcpy(ctx->rx_buf + ctx->rx_len, buf, take);
+ ctx->rx_len += take;
+ done = (ctx->rx_len == sizeof(ctx->rx_buf));
+ }
+ }
+ if (done)
+ complete(&ctx->rx_done);
+
+ /* consume all to avoid desync/misalignment */
+ return count;
+}
+
+static const struct serdev_device_ops rtpse_uart_serdev_ops = {
+ .receive_buf = rtpse_uart_receive,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static int rtpse_uart_send(struct rtpse_ctrl *pse, const struct rtpse_mcu_msg *req)
+{
+ struct rtpse_uart *ctx = to_rtpse_uart(pse);
+ int written;
+
+ /* clear any leftover rx state before transmitting */
+ reinit_completion(&ctx->rx_done);
+ scoped_guard(spinlock_irqsave, &ctx->rx_lock)
+ ctx->rx_len = 0;
+
+ written = serdev_device_write(ctx->serdev, (const u8 *)req, sizeof(*req),
+ RTPSE_UART_TX_TIMEOUT);
+ if (written < 0)
+ return written;
+ if (written != sizeof(*req))
+ return -EIO;
+
+ return 0;
+}
+
+static int rtpse_uart_recv(struct rtpse_ctrl *pse,
+ const struct rtpse_mcu_msg *req,
+ struct rtpse_mcu_msg *resp)
+{
+ struct rtpse_uart *ctx = to_rtpse_uart(pse);
+
+ if (!wait_for_completion_timeout(&ctx->rx_done, RTPSE_UART_RX_TIMEOUT))
+ return -ETIMEDOUT;
+
+ scoped_guard(spinlock_irqsave, &ctx->rx_lock) {
+ if (ctx->rx_len != sizeof(*resp))
+ return -EIO;
+
+ memcpy(resp, ctx->rx_buf, sizeof(*resp));
+ }
+ return 0;
+}
+
+static const struct rtpse_transport_ops rtpse_uart_transport_ops = {
+ .send = rtpse_uart_send,
+ .recv = rtpse_uart_recv,
+};
+
+static int rtpse_uart_probe(struct serdev_device *serdev)
+{
+ u32 speed = RTPSE_UART_BAUD_DEFAULT;
+ struct device *dev = &serdev->dev;
+ struct rtpse_uart *ctx;
+ int ret;
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->serdev = serdev;
+ ctx->pse.dev = dev;
+ ctx->pse.transport = &rtpse_uart_transport_ops;
+ init_completion(&ctx->rx_done);
+ spin_lock_init(&ctx->rx_lock);
+
+ serdev_device_set_drvdata(serdev, ctx);
+ serdev_device_set_client_ops(serdev, &rtpse_uart_serdev_ops);
+
+ ret = devm_serdev_device_open(dev, serdev);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to open serdev\n");
+
+ fwnode_property_read_u32(dev_fwnode(dev), "current-speed", &speed);
+ serdev_device_set_baudrate(serdev, speed);
+ serdev_device_set_flow_control(serdev, false);
+ serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+
+ return rtpse_register(&ctx->pse);
+}
+
+static const struct of_device_id rtpse_uart_of_match[] = {
+ { .compatible = "realtek,pse-mcu-rtk", .data = &rtpse_rtk_data },
+ { .compatible = "realtek,pse-mcu-bcm", .data = &rtpse_bcm_data },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rtpse_uart_of_match);
+
+static struct serdev_device_driver rtpse_uart_driver = {
+ .driver = {
+ .name = "realtek-pse-uart",
+ .of_match_table = rtpse_uart_of_match,
+ },
+ .probe = rtpse_uart_probe,
+};
+module_serdev_device_driver(rtpse_uart_driver);
+
+MODULE_AUTHOR("Jonas Jelonek <jelonek.jonas@gmail.com>");
+MODULE_DESCRIPTION("Realtek/Broadcom PSE MCU driver (UART transport)");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/pse-pd/realtek-pse.h b/drivers/net/pse-pd/realtek-pse.h
new file mode 100644
index 000000000000..1de986586f11
--- /dev/null
+++ b/drivers/net/pse-pd/realtek-pse.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _REALTEK_PSE_H
+#define _REALTEK_PSE_H
+
+#include <linux/mutex.h>
+#include <linux/pse-pd/pse.h>
+#include <linux/types.h>
+
+/*
+ * Time the MCU itself needs between accepting a request and having a
+ * response ready. These are properties of the MCU firmware, not of the
+ * underlying transport: the core paces transactions by RTPSE_MCU_RESPONSE_MS
+ * and both transports size their per-transaction recv ceiling from
+ * RTPSE_MCU_RESPONSE_MAX_MS, since some commands are documented as
+ * needing up to ~1s to produce a reply.
+ */
+#define RTPSE_MCU_RESPONSE_MS 25
+#define RTPSE_MCU_RESPONSE_MAX_MS 1000
+
+/*
+ * Total time to keep retrying the first MCU read at probe, and the pause
+ * between attempts. Right after enable-gpios is asserted the MCU may not
+ * answer on the bus yet; give it a bounded window to come up before
+ * declaring the probe failed.
+ */
+#define RTPSE_MCU_BOOT_TIMEOUT_MS 3000
+#define RTPSE_MCU_BOOT_RETRY_MS 100
+
+#define RTPSE_MCU_MSG_SIZE 12
+
+struct rtpse_mcu_msg {
+ u8 opcode;
+ u8 seq_num;
+ u8 payload[9];
+ u8 checksum;
+} __packed;
+
+/*
+ * MCU status opcodes (seen on the BCM dialect; RTL never emits them).
+ * INCOMPLETE/BAD_CSUM are terminal; NOT_READY is transient.
+ */
+#define RTPSE_OPCODE_INCOMPLETE 0xfd /* -EBADE */
+#define RTPSE_OPCODE_BAD_CSUM 0xfe /* -EBADMSG */
+#define RTPSE_OPCODE_NOT_READY 0xff /* -EAGAIN */
+
+/* A polling transport can stop here: the matching reply, or a terminal error. */
+static inline bool rtpse_resp_is_final(const struct rtpse_mcu_msg *req,
+ const struct rtpse_mcu_msg *resp)
+{
+ return resp->opcode == req->opcode ||
+ resp->opcode == RTPSE_OPCODE_INCOMPLETE ||
+ resp->opcode == RTPSE_OPCODE_BAD_CSUM;
+}
+
+/* Opaque to transports; defined in realtek-pse-core.c. */
+struct rtpse_mcu_dialect;
+struct rtpse_match_data;
+struct rtpse_chip_info;
+struct rtpse_ctrl;
+
+struct rtpse_transport_ops {
+ int (*send)(struct rtpse_ctrl *pse, const struct rtpse_mcu_msg *req);
+ int (*recv)(struct rtpse_ctrl *pse, const struct rtpse_mcu_msg *req,
+ struct rtpse_mcu_msg *resp);
+};
+
+struct rtpse_ctrl {
+ struct device *dev;
+ struct pse_controller_dev pcdev;
+ struct mutex mutex; /* serializes MCU request/response transactions */
+ const struct rtpse_mcu_dialect *dialect;
+ const struct rtpse_chip_info *chip;
+ const struct rtpse_transport_ops *transport;
+
+ struct regulator *poe_supply;
+};
+
+int rtpse_register(struct rtpse_ctrl *pse);
+
+/* Whether the I2C transport must read "realtek,i2c-protocol" from DT. */
+bool rtpse_needs_i2c_proto(const struct rtpse_match_data *match);
+
+extern const struct rtpse_match_data rtpse_rtk_data;
+extern const struct rtpse_match_data rtpse_bcm_data;
+
+#endif
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: spi: snps,dw-apb-ssi: Add support for snps,dwc-ssi-2.00a
From: Mark Brown @ 2026-06-12 13:31 UTC (permalink / raw)
To: Changhuang Liang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-spi,
linux-kernel, devicetree
In-Reply-To: <20260612125856.8530-2-changhuang.liang@starfivetech.com>
[-- Attachment #1: Type: text/plain, Size: 509 bytes --]
On Fri, Jun 12, 2026 at 05:58:55AM -0700, Changhuang Liang wrote:
> Add a new compatible string "snps,dwc-ssi-2.00a" for the Synopsys
> DesignWare SSI controller version 2.00a.
Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 4/8] media: uapi: Add NXP NEOISP user interface header file
From: sashiko-bot @ 2026-06-12 13:34 UTC (permalink / raw)
To: Antoine Bouyer; +Cc: devicetree, robh, conor+dt, Frank.Li, imx
In-Reply-To: <20260612132039.2089051-5-antoine.bouyer@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] UAPI Compilation Failure due to undefined struct `v4l2_isp_block_header`
- [Medium] Implicit compiler padding (holes) in UAPI structures
--
commit 07ff92ed00f28aef4b4e762772b07f402cd3490d
Author: Antoine Bouyer <antoine.bouyer@nxp.com>
media: uapi: Add NXP NEOISP user interface header file
Add user space api header file for meta data structures definitions.
This header describes parameters buffer for the ISP blocks control by
userspace, and statistics buffer for userspace and IPA handling.
> diff --git a/include/uapi/linux/media/nxp/nxp_neoisp.h b/include/uapi/linux/media/nxp/nxp_neoisp.h
> new file mode 100644
> index 0000000000000..2dafa19df3242
> --- /dev/null
> +++ b/include/uapi/linux/media/nxp/nxp_neoisp.h
[ ... ]
> +struct neoisp_obwb_cfg_s {
> + __u8 ctrl_obpp;
> + __u16 r_ctrl_gain;
> + __u16 r_ctrl_offset;
[Severity: Medium]
Is there an implicit 1-byte compiler padding hole after ctrl_obpp to align
the __u16 r_ctrl_gain field?
UAPI guidelines generally require explicit reserved fields to fill all
holes. This guarantees a stable ABI and prevents uninitialized kernel memory
from potentially leaking to userspace.
[ ... ]
> +struct neoisp_ir_compress_cfg_s {
> + __u8 ctrl_obpp;
> + __u32 knee_point1_kneepoint;
[Severity: Medium]
Does this introduce a 3-byte implicit padding hole after ctrl_obpp to align
the __u32 knee_point1_kneepoint field?
[ ... ]
> +struct neoisp_bnr_cfg_s {
> + __u8 ctrl_debug;
> + __u8 ctrl_obpp;
> + __u8 ctrl_nhood;
> + __u8 ypeak_peak_outsel;
> + __u8 ypeak_peak_sel;
> + __u16 ypeak_peak_low;
[Severity: Medium]
Are there implicit padding holes in this structure, such as a 1-byte hole
after ypeak_peak_sel?
[ ... ]
> +struct neoisp_rgb2yuv_cfg_s {
> + __u16 gain_ctrl_rgain;
> + __u16 gain_ctrl_bgain;
> + __s16 mat_rxcy[NEO_RGB2YUV_MATRIX_SIZE][NEO_RGB2YUV_MATRIX_SIZE];
> + __s32 csc_offsets[NEO_RGB2YUV_MATRIX_SIZE];
> +};
[Severity: Medium]
Is there a 2-byte implicit padding hole after the 18-byte mat_rxcy array to
align the __s32 csc_offsets field?
[ ... ]
> +struct neoisp_ee_cfg_s {
> + __u8 ctrl_debug;
> + __u8 maskgain_gain;
> + __u32 coring_coring;
[Severity: Medium]
Does this leave a 2-byte implicit padding hole after maskgain_gain?
[ ... ]
> +struct neoisp_pipe_conf_cfg_es {
> + struct v4l2_isp_block_header header;
> + struct neoisp_pipe_conf_cfg_s cfg;
> +} __attribute__((aligned(8)));
[Severity: High]
Will embedding struct v4l2_isp_block_header by value cause an incomplete
type compilation error?
The struct v4l2_isp_block_header does not appear to be defined anywhere,
as the generic V4L2 ISP framework defines v4l2_isp_params_block_header
instead. This seems to break compilation for both the driver and any
userspace program including this header.
[ ... ]
> +struct neoisp_ctemp_reg_stats_es {
> + struct v4l2_isp_block_header header;
> + struct neoisp_ctemp_reg_stats_s stat;
> +} __attribute__((aligned(8)));
[Severity: Medium]
Does forcing 8-byte alignment here add 4 bytes of implicit trailing padding
to this statistics wrapper structure?
Adding explicit padding fields might be needed to prevent uninitialized
kernel memory from leaking to userspace via the statistics buffer.
[ ... ]
> +struct neoisp_nr_reg_stats_es {
> + struct v4l2_isp_block_header header;
> + struct neoisp_nr_reg_stats_s stat;
> +} __attribute__((aligned(8)));
[Severity: Medium]
Does this also contain 4 bytes of implicit trailing padding due to the
explicit 8-byte alignment requirement?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132039.2089051-1-antoine.bouyer@nxp.com?part=4
^ permalink raw reply
* Re: [PATCH v3 7/8] media: platform: neoisp: Add debugfs support
From: sashiko-bot @ 2026-06-12 13:34 UTC (permalink / raw)
To: Antoine Bouyer; +Cc: robh, devicetree, imx, conor+dt, Frank.Li
In-Reply-To: <20260612132039.2089051-8-antoine.bouyer@nxp.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Critical] Missing cleanup of media devices and debugfs files in the `neoisp_probe` error path leads to Resource Leaks and Use-After-Free vulnerabilities.
- [High] The device's PM usage counter is leaked if `CONFIG_DEBUG_FS` is disabled or if debugfs allocation fails.
- [High] Invalid direct pointer dereference of I/O memory (`__iomem`) in `neoisp_dump_memory()`.
- [Low] The `enable_debugfs` module parameter is defined with `0644` (read-write) permissions, but it is only evaluated once during module probe.
Pre-existing issues:
- [High] Race condition in Runtime PM state due to asynchronous `pm_runtime_put()` before `pm_runtime_disable()`.
- [High] Double unregister of the v4l2_device if neoisp_init_group() fails when called from neoisp_core_media_register().
--
commit 15c2086c65406ec4a2f05196f174786dbe315997
Author: Antoine Bouyer <antoine.bouyer@nxp.com>
Subject: media: platform: neoisp: Add debugfs support
Add debugfs entries to dump ISP registers, and some internal memory
regions used to store Vignetting, DRC global and DRC local coefficients.
Debug mode is activated with the enable_debugfs module parameter.
> diff --git a/drivers/media/platform/nxp/neoisp/neoisp_debugfs.c b/drivers/media/platform/nxp/neoisp/neoisp_debugfs.c
> new file mode 100644
> index 0000000000000..e730569184b3d
> --- /dev/null
> +++ b/drivers/media/platform/nxp/neoisp/neoisp_debugfs.c
[ ... ]
> +static inline int
> +neoisp_dump_memory(struct seq_file *m, enum isp_block_map_e map, int wsize)
> +{
> + struct neoisp_dev_s *neoispd = m->private;
> + union udata_t data;
> + u32 addr;
> + u32 *src = (u32 *)neoispd->local_mem;
> + u32 offset = ISP_GET_OFF(map) / sizeof(u32);
> + u32 size = ISP_GET_SZ(map) / sizeof(u32);
> + int i, j;
> +
> + for (i = 0; i < size; i++) {
> + addr = offset * sizeof(u32);
> + data.word = src[offset++];
[Severity: High]
Can this directly dereference I/O memory?
neoispd->local_mem is mapped using devm_ioremap_resource_wc(), which returns
an iomem pointer. Here it is cast to a regular u32 pointer and accessed
directly:
data.word = src[offset++];
Accessing I/O memory via direct pointer dereferencing bypasses required
compiler memory barriers and architecture-specific I/O memory instructions.
It invites unsafe compiler optimizations and can cause CPU faults.
Should this code use readl() or ioread32() to safely access the MMIO region
and ensure correct CPU endianness conversion?
[ ... ]
> diff --git a/drivers/media/platform/nxp/neoisp/neoisp_main.c b/drivers/media/platform/nxp/neoisp/neoisp_main.c
> index b08995403c597..ca61af8cf66e4 100644
> --- a/drivers/media/platform/nxp/neoisp/neoisp_main.c
> +++ b/drivers/media/platform/nxp/neoisp/neoisp_main.c
[ ... ]
> @@ -41,6 +42,10 @@ static int standalone_mdev;
> module_param_named(standalone_mdev, standalone_mdev, uint, 0644);
> MODULE_PARM_DESC(standalone_mdev, " Create standalone neoisp media device, default is 0 (off)");
>
> +static int enable_debugfs;
> +module_param_named(enable_debugfs, enable_debugfs, uint, 0644);
[Severity: Low]
Since the driver cannot respond to runtime changes of this parameter, could
the permissions be set to 0444 (read-only) instead of 0644?
Users may attempt to toggle debugfs support dynamically via sysfs after the
module is loaded, but changing this value at runtime will not dynamically
create or remove the debugfs entries, nor will it alter the device PM state.
> +MODULE_PARM_DESC(enable_debugfs, " Turn on/off debugfs, default is 0 (off)");
> +
> static inline bool node_desc_is_output(const struct neoisp_node_desc_s *desc)
[ ... ]
> @@ -1766,9 +1771,18 @@ static int neoisp_probe(struct platform_device *pdev)
> neoisp_init_hw(neoispd);
> neoisp_set_default_context(neoispd);
>
> + if (enable_debugfs) {
> + neoisp_debugfs_init(neoispd);
> + /* Increase pm_runtime counter to prevent suspend */
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + goto err_pm_runtime_suspend;
[Severity: Critical]
Does this error path leave dangling media device and debugfs registrations?
If pm_runtime_resume_and_get() fails, the execution jumps to
err_pm_runtime_suspend. This error path only cleans up the initial runtime
PM state.
It bypasses calling neoisp_destroy_devices() to unregister the previously
initialized V4L2 and media devices, and it fails to call
neoisp_debugfs_exit().
Since returning an error from probe causes devres to free the neoispd
structure, the V4L2 device nodes and debugfs files will remain alive and
point to freed memory, leading to use-after-free vulnerabilities if accessed
from user-space.
> + }
> +
> pm_runtime_mark_last_busy(dev);
> pm_runtime_put_autosuspend(dev);
[ ... ]
> @@ -1785,6 +1799,11 @@ static void neoisp_remove(struct platform_device *pdev)
> {
> struct neoisp_dev_s *neoispd = platform_get_drvdata(pdev);
>
> + if (neoispd->regset) {
> + neoisp_debugfs_exit(neoispd);
> + pm_runtime_put(neoispd->dev);
> + }
[Severity: High]
Does this unconditionally leak the PM usage counter if debugfs allocation
fails or is disabled?
In neoisp_probe(), if the enable_debugfs parameter is true, the driver calls
neoisp_debugfs_init() and unconditionally increments the PM counter via
pm_runtime_resume_and_get() to keep the device awake.
However, in neoisp_debugfs_init(), if devm_kzalloc() fails or if
CONFIG_DEBUG_FS is disabled, neoispd->regset remains NULL.
Later, neoisp_remove() incorrectly checks if (neoispd->regset) before calling
pm_runtime_put(). Since regset is NULL, the put is entirely skipped. This
permanently leaks the PM usage counter and prevents the device from
suspending.
[Severity: High]
This is a pre-existing issue, but does the use of pm_runtime_put() here
create a race condition with device removal?
pm_runtime_put() queues an asynchronous idle notification. However, at the
end of neoisp_remove(), pm_runtime_disable() is called:
pm_runtime_dont_use_autosuspend(neoispd->dev);
pm_runtime_disable(neoispd->dev);
The pending async suspend work can be canceled before it executes by
pm_runtime_disable(), leaving the hardware powered on in an incorrect
RPM_ACTIVE state.
Should this use pm_runtime_put_sync() instead to ensure the device powers
down correctly? The same pre-existing pattern appears in the
err_pm_runtime_suspend error path in neoisp_probe().
> +
> neoisp_destroy_devices(neoispd);
[Severity: High]
This is a pre-existing issue, but does neoisp_destroy_devices() cause a
double unregister of the v4l2_device?
If another driver calls neoisp_core_media_register() and invokes
neoisp_init_group(), an error in neoisp_init_group() will trigger its
error path to call v4l2_device_unregister().
However, the core neoisp device remains actively probed. When the driver is
later unbound, neoisp_remove() calls neoisp_destroy_devices(), which
unconditionally calls v4l2_device_unregister() a second time on the same
v4l2_dev, causing a use-after-free.
>
> if (standalone_mdev)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132039.2089051-1-antoine.bouyer@nxp.com?part=7
^ permalink raw reply
* Re: [PATCH v6 4/5] iio: adc: versal-sysmon: add threshold event support
From: Erim, Salih @ 2026-06-12 13:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260612135222.0cec353b@jic23-huawei>
Hi Jonathan,
Thanks for reviews, replies are inline.
On 12/06/2026 13:52, Jonathan Cameron wrote:
> On Thu, 11 Jun 2026 23:27:37 +0100
> Salih Erim <salih.erim@amd.com> wrote:
>
>> Add threshold event support for temperature and supply voltage
>> channels.
>>
>> Temperature events:
>> - Rising threshold with configurable value
>> - Over-temperature (OT) alarm with separate threshold
>
> Ah. I ask about this below. If this applies to the same channel
> as the main threshold we generally don't support that in IIO and
> definitely not by introducing a 'magic' extra channel.
> See below.
>
>> - Per-channel hysteresis as a millicelsius value
>> - Event direction is IIO_EV_DIR_RISING (hysteresis mode)
>>
>> Supply voltage events:
>> - Rising/falling threshold per supply channel
>> - Per-channel alarm enable via alarm configuration registers
>>
>> The hardware supports both window and hysteresis alarm modes for
>> temperature. This driver uses hysteresis mode, where the upper
>> threshold triggers the alarm and the lower threshold clears it
>> (re-arm point). The hardware has a single ISR bit per temperature
>> channel with no indication of which threshold was crossed, so
>> hysteresis mode is the natural fit. The lower threshold register
>> is computed internally as (upper - hysteresis).
>>
>> Hysteresis is stored in the driver as a millicelsius value,
>> initialized from the hardware registers at probe. Writing the
>> rising threshold or hysteresis recomputes the lower register.
>> ALARM_CONFIG is hard-coded to hysteresis mode during init.
>>
>> The interrupt handler masks active threshold interrupts (which are
>> level-sensitive) and schedules a delayed worker to poll for condition
>> clear before unmasking. When no hardware IRQ is available, event
>> channels are not created and interrupt init is skipped, since the
>> I2C regmap backend cannot be called from atomic context.
>>
>> When disabling a supply channel alarm, the group interrupt remains
>> active if any other channel in the same alarm group still has an
>> alarm enabled.
>>
>> Signed-off-by: Salih Erim <salih.erim@amd.com>
>
> Some follow on questions on the temperature channels and one thing
> Sashiko noticed that looks real.
>
>> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
>> index c875d156dbe..20fd3a87d44 100644
>> --- a/drivers/iio/adc/versal-sysmon-core.c
>> +++ b/drivers/iio/adc/versal-sysmon-core.c
>> @@ -11,7 +11,9 @@
>> #include <linux/bitops.h>
>> #include <linux/cleanup.h>
>> #include <linux/device.h>
>> +#include <linux/devm-helpers.h>
>> #include <linux/err.h>
>> +#include <linux/interrupt.h>
>> #include <linux/module.h>
>> #include <linux/property.h>
>> #include <linux/regmap.h>
>> @@ -19,10 +21,19 @@
>> #include <linux/sysfs.h>
>> #include <linux/units.h>
>>
>> +#include <linux/iio/events.h>
>> #include <linux/iio/iio.h>
>>
>> #include "versal-sysmon.h"
>>
>> +/* OT and TEMP hysteresis mode bits in SYSMON_TEMP_EV_CFG */
>> +#define SYSMON_OT_HYST_MASK BIT(0)
>> +#define SYSMON_TEMP_HYST_MASK BIT(1)
>> +
>> +/* Compute alarm register offset from a channel address */
>> +#define SYSMON_ALARM_OFFSET(addr) \
>> + (SYSMON_ALARM_REG + ((addr) / SYSMON_ALARM_BITS_PER_REG) * SYSMON_REG_STRIDE)
>> +
>> #define SYSMON_CHAN_TEMP(_chan, _address, _name) \
>> { \
>> .type = IIO_TEMP, \
>> @@ -34,14 +45,87 @@
>> .datasheet_name = _name, \
>> }
>>
>> +#define SYSMON_CHAN_TEMP_EVENT(_chan, _address, _name, _events) \
>> +{ \
>> + .type = IIO_TEMP, \
>> + .indexed = 1, \
>> + .address = _address, \
>> + .channel = _chan, \
>> + .event_spec = _events, \
>> + .num_event_specs = ARRAY_SIZE(_events), \
>> + .datasheet_name = _name, \
>> +}
>> +
>> +enum sysmon_alarm_bit {
>> + SYSMON_BIT_ALARM0 = 0,
>> + SYSMON_BIT_ALARM1 = 1,
>> + SYSMON_BIT_ALARM2 = 2,
>> + SYSMON_BIT_ALARM3 = 3,
>> + SYSMON_BIT_ALARM4 = 4,
>> + SYSMON_BIT_OT = 8,
>> + SYSMON_BIT_TEMP = 9,
>> +};
>
>> /* Static temperature channels (always present) */
>> -static const struct iio_chan_spec temp_channels[] = {
>> +static const struct iio_chan_spec temp_channels_no_events[] = {
>> SYSMON_CHAN_TEMP(0, SYSMON_TEMP_MAX, "temp"),
>> SYSMON_CHAN_TEMP(1, SYSMON_TEMP_MIN, "min"),
>> SYSMON_CHAN_TEMP(2, SYSMON_TEMP_MAX_MAX, "max_max"),
>> SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
>> };
>>
>> +/* Static temperature channels with event support (when IRQ available) */
>> +static const struct iio_chan_spec temp_channels_with_events[] = {
>> + SYSMON_CHAN_TEMP(0, SYSMON_TEMP_MAX, "temp"),
>> + SYSMON_CHAN_TEMP(1, SYSMON_TEMP_MIN, "min"),
>> + SYSMON_CHAN_TEMP(2, SYSMON_TEMP_MAX_MAX, "max_max"),
>> + SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
>> + SYSMON_CHAN_TEMP_EVENT(4, SYSMON_ADDR_TEMP_EVENT, "temp",
>> + sysmon_temp_events),
> Is this not an event on channel 0? Why does it need a separate one?
The hardware has two independent threshold register pairs on the
same DEVICE_TEMP_MAX measurement: a TEMP threshold (ISR bit 9)
and an OT threshold (ISR bit 8), each with its own hysteresis.
We modelled them as separate event-only channels because of the
independent HW registers.
However, you're right that they don't necessarily need separate
channels. Both monitor the same value that channel 0 reads, so
the TEMP event spec belongs directly on channel 0.
>> + SYSMON_CHAN_TEMP_EVENT(5, SYSMON_ADDR_OT_EVENT, "ot",
>
> Why two separate channels for events? Are we dealing with two separate
> events on the same signal? Generally we don't support that for IIO because
> it's largely meaningless except in hwmon usecases - what is the point in two
> thresholds if they are reported through the same path? Just use one and update
> it if you want to add another level of detection.
Yes, both are thresholds on the same physical measurement. OT is
a higher-severity threshold that can trigger the platform
management controller to initiate a hardware shutdown sequence.
If you agree, I'd propose for v7:
- Move TEMP threshold event spec onto channel 0 directly
- Drop OT as a separate IIO channel, since it's a hardware
safety mechanism better suited for the thermal framework
as a critical trip point (planned for the follow-up
thermal series)
Happy to take a different direction if you prefer.
>
>> + sysmon_temp_events),
>> +};
>
>> +
>> +static int sysmon_read_event_config(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir)
>> +{
>> + u32 alarm_event_mask = sysmon_get_event_mask(chan->address);
>> + struct sysmon *sysmon = iio_priv(indio_dev);
>> + unsigned int imr;
>> + int config_value;
>> + int ret;
>> +
>> + ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
>> + if (ret)
>> + return ret;
>> +
>> + /* IMR bits are 1=masked, invert to get 1=enabled */
>> + imr = ~imr;
>> +
>> + switch (chan->type) {
>> + case IIO_VOLTAGE:
>> + config_value = sysmon_read_alarm_config(sysmon, chan->address);
>> + if (config_value < 0)
>> + return config_value;
>> + return config_value && (imr & alarm_event_mask);
>> +
>> + case IIO_TEMP:
>> + return !!(imr & alarm_event_mask);
>
> Sashiko made a perhaps insightful observation here. When the interrupt
> is masked between sending an event and the worker reenabling it does
> this give an unexpected value to userspace? I think that condition
> we'd kind of expect this to return 0.
> https://sashiko.dev/#/patchset/20260611222738.2035062-1-salih.erim%40amd.com
Agreed. read_event_config currently reads the hardware IMR which
shows transient masking state during the 500ms polling window.
Will fix to return the administrative state from temp_mask instead.
>
> I think the rest of the feedback is probably false positives or debatable
> stuff but this one rang true. Please do take a look at the other stuff
> as I may have missed something (maybe the comment about needing to disable
> event interrupt generation is true?)
Good point. Will investigate whether a devm_add_action to
write SYSMON_IDR is needed on unbind and add it if so.
Reviewed the remaining Sashiko findings:
- Integer overflow in threshold conversions: for temperature,
the Q8.7 register range is -256C to +255C, so any
reasonable millicelsius input fits after the shift. For
supply, val * scale can overflow int32 above ~32V, but
supply rails on Versal are well under 4V. Extreme sysfs
inputs are outside the hardware range.
- I2C + IRQ panic on misconfigured DT: if an I2C node
incorrectly specifies an interrupts property, the driver
would register a hardirq handler on a sleeping regmap.
The binding does not list interrupts for I2C, so this
would be a DT authoring error.
Thanks,
Salih
>
>> +
>> + default:
>> + return -EINVAL;
>> + }
>
>
>
>
>> +
>
^ permalink raw reply
* Re: [PATCH v3 6/8] media: platform: Add NXP Neoisp Image Signal Processor
From: sashiko-bot @ 2026-06-12 13:41 UTC (permalink / raw)
To: Antoine Bouyer; +Cc: devicetree, Frank.Li, imx, robh, conor+dt
In-Reply-To: <20260612132039.2089051-7-antoine.bouyer@nxp.com>
Thank you for your contribution! Sashiko AI review found 14 potential issue(s) to consider:
- [Critical] The driver validates ISP parameters directly in the user-mapped shared memory buffer without a kernel copy, exposing a critical TOCTOU vulnerability.
- [High] Using asynchronous `pm_runtime_put()` before `pm_runtime_disable()` leaves the hardware permanently powered on.
- [High] Using standard `memcpy` on MMIO mapped memory causes architectural faults.
- [High] Missing NULL pointer checks for virtual addresses returned by videobuf2.
- [High] Invalid V4L2 teardown order in `neoisp_destroy_devices()` causes Use-After-Free of `v4l2_dev`.
- [High] Missing `media_entity_cleanup()` for video nodes causes memory leaks of media pads/links.
- [High] Unbounded wait for hardware completion in `neoisp_node_stop_streaming()` deadlocks and leaks buffers.
- [High] The IRQ handler can fire after `neoisp_destroy_devices()` has freed DMA buffers, causing a Use-After-Free.
- [High] The driver exposes video devices to userspace before initializing `hw_lock` and the hardware.
- [High] `start_streaming` prepares only a single job, stranding any other initially queued buffers and stalling the pipeline.
- [High] `start_streaming` modifies the shared `neoispd->context` without locking, causing a data race with the active IRQ handler.
- [High] The IRQ handler accesses hardware registers without checking if the device is active, violating runtime PM safety.
- [High] `VIDIOC_S_SELECTION` modifies `node->crop` locklessly while the IRQ handler reads it, causing address corruption.
- [High] `neoisp_core_media_register` lacks synchronization, allowing concurrent calls to corrupt media graph initialization.
--
commit 0f5211be403174984a3657c54a249e9f22caa901
Author: Antoine Bouyer <antoine.bouyer@nxp.com>
Date: Fri Jun 12 13:20:37 2026 +0000
media: platform: Add NXP Neoisp Image Signal Processor
This commit adds the first version of the NXP neoisp driver for processing
RAW images. It supports single context and multiple M2M operations,
including generic v4l2-isp parameters and statistics.
> diff --git a/drivers/media/platform/nxp/neoisp/neoisp_ctx.c b/drivers/media/platform/nxp/neoisp/neoisp_ctx.c
> new file mode 100644
> index 0000000000000..9d13eb7b732d2
> --- /dev/null
> +++ b/drivers/media/platform/nxp/neoisp/neoisp_ctx.c
[ ... ]
> +static inline void
> +local_mem_blk_write(struct neoisp_dev_s *neoispd, enum isp_block_map_e map, void *src)
> +{
> + u32 offset, count;
> +
> + get_offsize(map, &offset, &count);
> + memcpy(neoispd->local_mem + offset, src, count);
> +}
> +
> +static inline void
> +local_mem_blk_read(struct neoisp_dev_s *neoispd, enum isp_block_map_e map, void *dst)
> +{
> + u32 offset, count;
> +
> + get_offsize(map, &offset, &count);
> + memcpy(dst, neoispd->local_mem + offset, count);
> +}
[Severity: High]
Does this usage of memcpy() on an MMIO region trigger architectural faults?
The neoispd->local_mem region is mapped via devm_ioremap_resource_wc(),
returning an I/O memory pointer. Using standard string routines like
memcpy() on MMIO violates kernel API rules and can cause unaligned access
faults on certain architectures.
[ ... ]
> +void neoisp_ctx_update_buf_addr(struct neoisp_dev_s *neoispd)
> +{
> + struct neoisp_job_s *job = &neoispd->queued_job;
> + struct neoisp_pipe_conf_s *cfg = &neoispd->context->hw.pipe_conf;
> + struct neoisp_buffer_s *buf_inp0 = job->buf[NEOISP_INPUT0_NODE];
> + struct neoisp_buffer_s *buf_inp1 = job->buf[NEOISP_INPUT1_NODE];
> + struct neoisp_buffer_s *buf_out = job->buf[NEOISP_FRAME_NODE];
> + struct neoisp_buffer_s *buf_ir = job->buf[NEOISP_IR_NODE];
> + struct neoisp_node_s *nd;
> + u32 ibpp, inp0_stride, inp1_stride;
> + dma_addr_t inp0_addr, inp1_addr;
> +
> + /* Input0 specific */
> + nd = &neoispd->node[NEOISP_INPUT0_NODE];
> + ibpp = (nd->neoisp_format->bit_depth + 7) / 8;
> + inp0_stride = nd->format.fmt.pix_mp.plane_fmt[0].bytesperline;
> +
> + /* Input0 - Take crop into account if any */
> + inp0_addr = get_addr(buf_inp0, 0) + (nd->crop.left * ibpp) + (nd->crop.top * inp0_stride);
[Severity: High]
Is there a data race on nd->crop here?
In neoisp_s_selection(), the node->crop values are updated without checking
vb2_is_busy() while only holding the queue_lock. The hardirq handler runs
this code to calculate the active DMA address without synchronization,
potentially resulting in an invalid address if crop coordinates change
during active streaming.
[ ... ]
> +void neoisp_ctx_update_w_user_params(struct neoisp_dev_s *neoispd)
> +{
> + struct neoisp_buffer_s *buf = neoispd->queued_job.buf[NEOISP_PARAMS_NODE];
> + struct v4l2_isp_buffer *params;
> + size_t block_offset = 0, max_offset;
> +
> + if (IS_ERR_OR_NULL(buf))
> + return;
> +
> + params = (struct v4l2_isp_buffer *)get_vaddr(buf);
> +
> + if (!params || params->data_size == 0)
> + /* No relevant parameters in this buffer */
> + return;
> +
> + max_offset = params->data_size;
> +
> + /*
> + * Walk the list of parameter blocks and process them. No
> + * validation is done here, as the content of the parameters
> + * buffer is already checked when the buffer is queued.
> + */
> + while (block_offset < max_offset) {
> + const struct neoisp_block_handler_s *block_handler;
> + const union neoisp_params_block_u *block;
> +
> + block = (const union neoisp_params_block_u *)
> + ¶ms->data[block_offset];
> + block_offset += block->header.size;
> +
> + block_handler = &neoisp_block_handlers[block->header.type];
> + block_handler->handler(neoispd->context, block);
> + }
> +}
[Severity: Critical]
Is this vulnerable to a TOCTOU (Time-Of-Check to Time-Of-Use) race?
The parameters are validated during QBUF but never copied into private
kernel memory. Here, in the hardware interrupt context, the driver
dereferences block->header.type directly from the user-mapped buffer to
index the neoisp_block_handlers array.
An attacker modifying the mapped buffer after validation could cause an
out-of-bounds access on the function pointer array and obtain arbitrary
code execution.
[ ... ]
> +void neoisp_ctx_upload_context(struct neoisp_dev_s *neoispd)
> +{
> + struct neoisp_context_s *ctx = neoispd->context;
> + u32 *src = (u32 *)&ctx->hw.pipe_conf.img_conf;
> +
> + reg_blk_write(neoispd, NEO_PIPE_CONF_IMG_CONF_CAM0, src,
> + NEO_AUTOFOCUS_ROI0_SUM0_CAM0 - NEO_PIPE_CONF_IMG_CONF_CAM0);
[Severity: High]
Could this copy a partially updated context to the hardware?
This is executed in the hardirq handler, while neoisp_node_start_streaming()
modifies neoispd->context concurrently without holding hw_lock. If another
node is streaming, this could write torn configuration data to MMIO.
[ ... ]
> +void neoisp_ctx_get_stats(struct neoisp_dev_s *neoispd, struct neoisp_buffer_s *buf)
> +{
> + struct neoisp_node_s *node = &neoispd->node[NEOISP_STATS_NODE];
> + struct v4l2_isp_buffer *stats;
> + u32 *blk_list, count;
> +
> + /* Check if stats node link is enabled */
> + if (!neoisp_node_link_is_enabled(node))
> + return;
> +
> + if (IS_ERR_OR_NULL(buf)) {
> + dev_err(neoispd->dev, "Error: stats pointer\\n");
> + return;
> + }
> +
> + stats = (struct v4l2_isp_buffer *)get_vaddr(buf);
> + v4l2_isp_stats_init_buffer(stats, V4L2_ISP_VERSION_V1);
[Severity: High]
Can get_vaddr() return NULL here and cause an oops?
vb2_plane_vaddr() can return NULL if the memory isn't mapped, which gets
passed blindly to v4l2_isp_stats_init_buffer() resulting in a dereference
crash.
> diff --git a/drivers/media/platform/nxp/neoisp/neoisp_main.c b/drivers/media/platform/nxp/neoisp/neoisp_main.c
> new file mode 100644
> index 0000000000000..b08995403c597
> --- /dev/null
> +++ b/drivers/media/platform/nxp/neoisp/neoisp_main.c
[ ... ]
> +static int neoisp_params_node_buf_prepare(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct neoisp_node_s *node = vb2_get_drv_priv(vb->vb2_queue);
> + struct neoisp_dev_s *neoispd = node->neoisp;
> + struct v4l2_isp_params_buffer *params = vb2_plane_vaddr(&vbuf->vb2_buf, 0);
> + int ret;
> +
> + ret = v4l2_isp_params_validate_buffer_size(neoispd->dev, vb,
> + node->format.fmt.meta.buffersize);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_isp_params_validate_buffer(neoispd->dev, vb,
> + params, neoisp_params_block_types_info,
> + ARRAY_SIZE(neoisp_params_block_types_info));
[Severity: High]
Should we check if params is NULL before passing it to the validation?
vb2_plane_vaddr() can return NULL, and v4l2_isp_params_validate_buffer()
will crash if it tries to dereference a NULL pointer.
[ ... ]
> +static int neoisp_node_start_streaming(struct vb2_queue *q, u32 count)
> +{
> + struct neoisp_node_s *node = vb2_get_drv_priv(q);
> + struct neoisp_dev_s *neoispd = node->neoisp;
> + struct neoisp_buffer_s *buf, *tmp;
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(neoispd->dev);
> + if (ret < 0)
> + goto error;
> +
> + ret = neoisp_prepare_node_streaming(node);
> + if (ret < 0)
> + goto error_streaming;
> +
> + scoped_guard(spinlock_irq, &neoispd->hw_lock) {
> + neoispd->streaming_map |= BIT(node->id);
> + neoispd->frame_sequence = 0;
> + }
> +
> + dev_dbg(neoispd->dev, "%s: for node %s (count %u)\\n",
> + __func__, NODE_NAME(node), count);
> + dev_dbg(neoispd->dev, "Nodes streaming now 0x%x\\n",
> + neoispd->streaming_map);
> +
> + /* Update queued job context with current driver configuration */
> + neoisp_ctx_update_packetizer(neoispd);
> + neoisp_ctx_update_pipe_conf(neoispd);
[Severity: High]
Could neoispd->context be corrupted here?
Updating the context configuration is performed here without locking, which
can race with the hardware interrupt if the device is already streaming on
another node.
> +
> + /* Maybe we're ready to run. */
> + if (!neoisp_prepare_job(neoispd))
> + neoisp_schedule(neoispd, false);
[Severity: High]
Is a single neoisp_prepare_job() call sufficient here?
If userspace has queued multiple buffers before starting the stream,
preparing only one job leaves the rest stranded in the ready_queue, which
stalls the processing pipeline.
[ ... ]
> +static void neoisp_node_stop_streaming(struct vb2_queue *q)
> +{
> + struct neoisp_node_s *node = vb2_get_drv_priv(q);
> + struct neoisp_dev_s *neoispd = node->neoisp;
> + struct neoisp_job_desc_s *job, *temp;
> + struct neoisp_buffer_s *buf;
> + LIST_HEAD(tmp_list);
> +
> + /*
> + * Now this is a bit awkward. In a simple M2M device we could just wait
> + * for all queued jobs to complete, but here there's a risk that a
> + * partial set of buffers was queued and cannot be run. For now, just
> + * cancel all buffers stuck in the "ready queue", then wait for any
> + * running job.
> + *
> + * This may return buffers out of order.
> + */
> + dev_dbg(neoispd->dev, "%s: for node %s\\n", __func__, NODE_NAME(node));
> + do {
> + buf = list_first_entry_or_null(&node->ready_queue,
> + struct neoisp_buffer_s,
> + ready_list);
> + if (buf) {
> + list_del(&buf->ready_list);
> + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> + }
> + } while (buf);
> +
> + vb2_wait_for_all_buffers(&node->queue);
[Severity: High]
Can this lead to a permanent deadlock?
If the hardware has stalled or an interrupt is missed,
vb2_wait_for_all_buffers() will block indefinitely because the hardware is
never forcibly stopped.
> +
> + if (node->id == NEOISP_INPUT0_NODE)
> + neoisp_set_default_context(neoispd);
> +
> + spin_lock_irq(&neoispd->hw_lock);
> + neoispd->streaming_map &= ~BIT(node->id);
> +
> + if (neoispd->streaming_map == 0) {
> + /*
> + * If all nodes have stopped streaming release all jobs
> + * without holding the lock.
> + */
> + list_splice_init(&neoispd->job_queue, &tmp_list);
> + }
> + spin_unlock_irq(&neoispd->hw_lock);
> +
> + list_for_each_entry_safe(job, temp, &tmp_list, queue) {
> + list_del(&job->queue);
> + kfree(job);
> + }
[Severity: High]
Do these freed jobs leak their associated buffers?
Any buffers tracked inside the discarded job->buffers arrays are not
returned to vb2, resulting in a permanent buffer leak.
[ ... ]
> +static irqreturn_t neoisp_irq_handler(int irq, void *dev_id)
> +{
> + struct neoisp_dev_s *neoispd = (struct neoisp_dev_s *)dev_id;
> + struct neoisp_buffer_s **buf = neoispd->queued_job.buf;
> + u64 ts = ktime_get_ns();
> + u32 irq_status = 0;
> + u32 irq_clear = 0;
> + bool done = false;
> + int i;
> +
> + irq_status = neoisp_rd(neoispd, NEO_PIPE_CONF_INT_STAT0);
[Severity: High]
Does this access the hardware without ensuring it is powered?
The device supports runtime PM, but the IRQ handler does not call
pm_runtime_get_if_active() before accessing NEO_PIPE_CONF_INT_STAT0. A
spurious or shared interrupt while the device is suspended could trigger a
fatal bus error.
[ ... ]
> +static void neoisp_destroy_devices(struct neoisp_dev_s *neoispd)
> +{
> + int i;
> +
> + if (neoispd->context) {
> + dma_free_coherent(neoispd->dev,
> + sizeof(struct neoisp_context_s),
> + neoispd->context,
> + neoispd->params_dma_addr);
> + }
> +
> + v4l2_device_unregister(&neoispd->v4l2_dev);
> +
> + if (standalone_mdev)
> + media_device_unregister(&neoispd->mdev);
> + else if (!neoispd->media_registered)
> + return;
> +
> + dev_dbg(neoispd->dev, "Unregister from media controller\\n");
> +
> + v4l2_ctrl_handler_free(&neoispd->hdl);
> + media_entity_cleanup(&neoispd->sd.entity);
> +
> + for (i = NEOISP_NODES_COUNT - 1; i >= 0; i--) {
> + struct neoisp_node_s *node = &neoispd->node[i];
> +
> + video_unregister_device(&node->vfd);
> + vb2_queue_release(&node->queue);
> + }
[Severity: High]
Is the v4l2 unregistration sequence safe?
v4l2_device_unregister() is called before the video devices are unregistered
with video_unregister_device(). The V4L2 core requires video nodes to be
unregistered first to prevent Use-After-Free bugs if userspace is still
interacting with an open node.
[Severity: High]
Are we leaking the media pads allocated for the video nodes?
The video nodes had their pads dynamically initialized via
media_entity_pads_init() during setup, but media_entity_cleanup() is never
called for node->vfd.entity before releasing them.
[ ... ]
> +int neoisp_core_media_register(struct device *dev, struct v4l2_subdev *sd)
> +{
> + struct neoisp_dev_s *neoispd = dev_get_drvdata(dev);
> + struct media_device *mdev = sd->v4l2_dev->mdev;
> + int ret;
> +
> + if (!neoispd)
> + return -EINVAL;
> +
> + if (neoispd->media_registered || standalone_mdev)
> + return 0;
> +
> + ret = neoisp_init_group(neoispd, mdev);
> + if (ret)
> + return ret;
> +
> + neoispd->media_registered++;
> + return 0;
> +}
[Severity: High]
Is neoispd->media_registered protected from concurrent accesses?
This exported function can be called asynchronously. Without locking, two
callers can bypass the check concurrently, causing double-initialization of
the video nodes and a corrupted media device graph.
[ ... ]
> +static int neoisp_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct neoisp_dev_s *neoispd;
> + struct resource *r;
> + int ret, irq;
[ ... ]
> + ret = neoisp_init_devices(neoispd);
> + if (ret)
> + goto err_pm_runtime_suspend;
> +
> + spin_lock_init(&neoispd->hw_lock);
> + neoisp_init_hw(neoispd);
> + neoisp_set_default_context(neoispd);
[Severity: High]
Does this order expose uninitialized locks to userspace?
neoisp_init_devices() registers the /dev/videoX nodes. If userspace
immediately opens a node and starts streaming, it could hit an uninitialized
hw_lock or interact with unconfigured hardware before neoisp_init_hw()
runs.
[ ... ]
> +err_pm_runtime_suspend:
> + pm_runtime_put(dev);
> +err_pm_runtime_disable:
> + pm_runtime_dont_use_autosuspend(dev);
> + pm_runtime_disable(dev);
> +err_irq:
> + dev_err(dev, "probe: error %d\\n", ret);
> + return ret;
> +}
[Severity: High]
Does this sequence properly handle the asynchronous pm_runtime_put()?
Calling pm_runtime_disable() immediately after the asynchronous
pm_runtime_put() will cancel the pending suspend, permanently stranding the
hardware in a powered-on state. A synchronous put should be used before
disabling.
> +static void neoisp_remove(struct platform_device *pdev)
> +{
> + struct neoisp_dev_s *neoispd = platform_get_drvdata(pdev);
> +
> + neoisp_destroy_devices(neoispd);
> +
> + if (standalone_mdev)
> + media_device_cleanup(&neoispd->mdev);
> +
> + pm_runtime_dont_use_autosuspend(neoispd->dev);
> + pm_runtime_disable(neoispd->dev);
> +}
[Severity: High]
Could the hardware IRQ fire after structures are freed?
The IRQ handler is managed by devm_request_irq() and will remain active
until the devm teardown phase. However, neoisp_destroy_devices() frees
neoispd->context. A late or spurious interrupt could trigger a
use-after-free in the handler. Should the hardware interrupts be explicitly
disabled here?
[Severity: High]
Should we use a synchronous PM put here?
Similar to the probe error path, failing to transition the device to suspend
before calling pm_runtime_disable() can leave the hardware permanently
powered on.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132039.2089051-1-antoine.bouyer@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v6 4/5] iio: adc: versal-sysmon: add threshold event support
From: Jonathan Cameron @ 2026-06-12 13:43 UTC (permalink / raw)
To: Erim, Salih
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel
In-Reply-To: <bd6299ae-cd02-4914-ab1b-f6bb9d3d1b9a@amd.com>
On Fri, 12 Jun 2026 14:39:12 +0100
"Erim, Salih" <salih.erim@amd.com> wrote:
> Hi Jonathan,
>
> Thanks for reviews, replies are inline.
>
> On 12/06/2026 13:52, Jonathan Cameron wrote:
> > On Thu, 11 Jun 2026 23:27:37 +0100
> > Salih Erim <salih.erim@amd.com> wrote:
> >
> >> Add threshold event support for temperature and supply voltage
> >> channels.
> >>
> >> Temperature events:
> >> - Rising threshold with configurable value
> >> - Over-temperature (OT) alarm with separate threshold
> >
> > Ah. I ask about this below. If this applies to the same channel
> > as the main threshold we generally don't support that in IIO and
> > definitely not by introducing a 'magic' extra channel.
> > See below.
> >
> >> - Per-channel hysteresis as a millicelsius value
> >> - Event direction is IIO_EV_DIR_RISING (hysteresis mode)
> >>
> >> Supply voltage events:
> >> - Rising/falling threshold per supply channel
> >> - Per-channel alarm enable via alarm configuration registers
> >>
> >> The hardware supports both window and hysteresis alarm modes for
> >> temperature. This driver uses hysteresis mode, where the upper
> >> threshold triggers the alarm and the lower threshold clears it
> >> (re-arm point). The hardware has a single ISR bit per temperature
> >> channel with no indication of which threshold was crossed, so
> >> hysteresis mode is the natural fit. The lower threshold register
> >> is computed internally as (upper - hysteresis).
> >>
> >> Hysteresis is stored in the driver as a millicelsius value,
> >> initialized from the hardware registers at probe. Writing the
> >> rising threshold or hysteresis recomputes the lower register.
> >> ALARM_CONFIG is hard-coded to hysteresis mode during init.
> >>
> >> The interrupt handler masks active threshold interrupts (which are
> >> level-sensitive) and schedules a delayed worker to poll for condition
> >> clear before unmasking. When no hardware IRQ is available, event
> >> channels are not created and interrupt init is skipped, since the
> >> I2C regmap backend cannot be called from atomic context.
> >>
> >> When disabling a supply channel alarm, the group interrupt remains
> >> active if any other channel in the same alarm group still has an
> >> alarm enabled.
> >>
> >> Signed-off-by: Salih Erim <salih.erim@amd.com>
> >
> > Some follow on questions on the temperature channels and one thing
> > Sashiko noticed that looks real.
> >
> >> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> >> index c875d156dbe..20fd3a87d44 100644
> >> --- a/drivers/iio/adc/versal-sysmon-core.c
> >> +++ b/drivers/iio/adc/versal-sysmon-core.c
> >> @@ -11,7 +11,9 @@
> >> #include <linux/bitops.h>
> >> #include <linux/cleanup.h>
> >> #include <linux/device.h>
> >> +#include <linux/devm-helpers.h>
> >> #include <linux/err.h>
> >> +#include <linux/interrupt.h>
> >> #include <linux/module.h>
> >> #include <linux/property.h>
> >> #include <linux/regmap.h>
> >> @@ -19,10 +21,19 @@
> >> #include <linux/sysfs.h>
> >> #include <linux/units.h>
> >>
> >> +#include <linux/iio/events.h>
> >> #include <linux/iio/iio.h>
> >>
> >> #include "versal-sysmon.h"
> >>
> >> +/* OT and TEMP hysteresis mode bits in SYSMON_TEMP_EV_CFG */
> >> +#define SYSMON_OT_HYST_MASK BIT(0)
> >> +#define SYSMON_TEMP_HYST_MASK BIT(1)
> >> +
> >> +/* Compute alarm register offset from a channel address */
> >> +#define SYSMON_ALARM_OFFSET(addr) \
> >> + (SYSMON_ALARM_REG + ((addr) / SYSMON_ALARM_BITS_PER_REG) * SYSMON_REG_STRIDE)
> >> +
> >> #define SYSMON_CHAN_TEMP(_chan, _address, _name) \
> >> { \
> >> .type = IIO_TEMP, \
> >> @@ -34,14 +45,87 @@
> >> .datasheet_name = _name, \
> >> }
> >>
> >> +#define SYSMON_CHAN_TEMP_EVENT(_chan, _address, _name, _events) \
> >> +{ \
> >> + .type = IIO_TEMP, \
> >> + .indexed = 1, \
> >> + .address = _address, \
> >> + .channel = _chan, \
> >> + .event_spec = _events, \
> >> + .num_event_specs = ARRAY_SIZE(_events), \
> >> + .datasheet_name = _name, \
> >> +}
> >> +
> >> +enum sysmon_alarm_bit {
> >> + SYSMON_BIT_ALARM0 = 0,
> >> + SYSMON_BIT_ALARM1 = 1,
> >> + SYSMON_BIT_ALARM2 = 2,
> >> + SYSMON_BIT_ALARM3 = 3,
> >> + SYSMON_BIT_ALARM4 = 4,
> >> + SYSMON_BIT_OT = 8,
> >> + SYSMON_BIT_TEMP = 9,
> >> +};
> >
> >> /* Static temperature channels (always present) */
> >> -static const struct iio_chan_spec temp_channels[] = {
> >> +static const struct iio_chan_spec temp_channels_no_events[] = {
> >> SYSMON_CHAN_TEMP(0, SYSMON_TEMP_MAX, "temp"),
> >> SYSMON_CHAN_TEMP(1, SYSMON_TEMP_MIN, "min"),
> >> SYSMON_CHAN_TEMP(2, SYSMON_TEMP_MAX_MAX, "max_max"),
> >> SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
> >> };
> >>
> >> +/* Static temperature channels with event support (when IRQ available) */
> >> +static const struct iio_chan_spec temp_channels_with_events[] = {
> >> + SYSMON_CHAN_TEMP(0, SYSMON_TEMP_MAX, "temp"),
> >> + SYSMON_CHAN_TEMP(1, SYSMON_TEMP_MIN, "min"),
> >> + SYSMON_CHAN_TEMP(2, SYSMON_TEMP_MAX_MAX, "max_max"),
> >> + SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
> >> + SYSMON_CHAN_TEMP_EVENT(4, SYSMON_ADDR_TEMP_EVENT, "temp",
> >> + sysmon_temp_events),
> > Is this not an event on channel 0? Why does it need a separate one?
>
> The hardware has two independent threshold register pairs on the
> same DEVICE_TEMP_MAX measurement: a TEMP threshold (ISR bit 9)
> and an OT threshold (ISR bit 8), each with its own hysteresis.
> We modelled them as separate event-only channels because of the
> independent HW registers.
>
> However, you're right that they don't necessarily need separate
> channels. Both monitor the same value that channel 0 reads, so
> the TEMP event spec belongs directly on channel 0.
>
> >> + SYSMON_CHAN_TEMP_EVENT(5, SYSMON_ADDR_OT_EVENT, "ot",
> >
> > Why two separate channels for events? Are we dealing with two separate
> > events on the same signal? Generally we don't support that for IIO because
> > it's largely meaningless except in hwmon usecases - what is the point in two
> > thresholds if they are reported through the same path? Just use one and update
> > it if you want to add another level of detection.
>
> Yes, both are thresholds on the same physical measurement. OT is
> a higher-severity threshold that can trigger the platform
> management controller to initiate a hardware shutdown sequence.
>
> If you agree, I'd propose for v7:
> - Move TEMP threshold event spec onto channel 0 directly
> - Drop OT as a separate IIO channel, since it's a hardware
> safety mechanism better suited for the thermal framework
> as a critical trip point (planned for the follow-up
> thermal series)
>
> Happy to take a different direction if you prefer.
That sounds good.
Thanks,
Jonathan
>
> >
> >> + sysmon_temp_events),
> >> +};
> >
> >> +
> >> +static int sysmon_read_event_config(struct iio_dev *indio_dev,
> >> + const struct iio_chan_spec *chan,
> >> + enum iio_event_type type,
> >> + enum iio_event_direction dir)
> >> +{
> >> + u32 alarm_event_mask = sysmon_get_event_mask(chan->address);
> >> + struct sysmon *sysmon = iio_priv(indio_dev);
> >> + unsigned int imr;
> >> + int config_value;
> >> + int ret;
> >> +
> >> + ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* IMR bits are 1=masked, invert to get 1=enabled */
> >> + imr = ~imr;
> >> +
> >> + switch (chan->type) {
> >> + case IIO_VOLTAGE:
> >> + config_value = sysmon_read_alarm_config(sysmon, chan->address);
> >> + if (config_value < 0)
> >> + return config_value;
> >> + return config_value && (imr & alarm_event_mask);
> >> +
> >> + case IIO_TEMP:
> >> + return !!(imr & alarm_event_mask);
> >
> > Sashiko made a perhaps insightful observation here. When the interrupt
> > is masked between sending an event and the worker reenabling it does
> > this give an unexpected value to userspace? I think that condition
> > we'd kind of expect this to return 0.
> > https://sashiko.dev/#/patchset/20260611222738.2035062-1-salih.erim%40amd.com
>
> Agreed. read_event_config currently reads the hardware IMR which
> shows transient masking state during the 500ms polling window.
> Will fix to return the administrative state from temp_mask instead.
>
> >
> > I think the rest of the feedback is probably false positives or debatable
> > stuff but this one rang true. Please do take a look at the other stuff
> > as I may have missed something (maybe the comment about needing to disable
> > event interrupt generation is true?)
>
> Good point. Will investigate whether a devm_add_action to
> write SYSMON_IDR is needed on unbind and add it if so.
>
> Reviewed the remaining Sashiko findings:
>
> - Integer overflow in threshold conversions: for temperature,
> the Q8.7 register range is -256C to +255C, so any
> reasonable millicelsius input fits after the shift. For
> supply, val * scale can overflow int32 above ~32V, but
> supply rails on Versal are well under 4V. Extreme sysfs
> inputs are outside the hardware range.
>
> - I2C + IRQ panic on misconfigured DT: if an I2C node
> incorrectly specifies an interrupts property, the driver
> would register a hardirq handler on a sleeping regmap.
> The binding does not list interrupts for I2C, so this
> would be a DT authoring error.
>
> Thanks,
> Salih
>
> >
> >> +
> >> + default:
> >> + return -EINVAL;
> >> + }
> >
> >
> >
> >
> >> +
> >
>
^ permalink raw reply
* Re: [PATCH v6 5/5] iio: adc: versal-sysmon: add oversampling support
From: Erim, Salih @ 2026-06-12 13:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260612135836.526cc07f@jic23-huawei>
Hi Jonathan,
On 12/06/2026 13:58, Jonathan Cameron wrote:
> On Thu, 11 Jun 2026 23:27:38 +0100
> Salih Erim <salih.erim@amd.com> wrote:
>
>> Add support for reading and writing the oversampling ratio through
>> the IIO oversampling_ratio attribute. The hardware supports averaging
>> 2, 4, 8, or 16 samples, plus a ratio of 1 (no averaging).
>>
>> Temperature and supply channels share oversampling configuration at
>> the type level (all temperature channels share one ratio, all supply
>> channels share another), exposed through info_mask_shared_by_type.
>>
>> The hardware encoding uses sample_count / 2 in a 4-bit field within
>> the CONFIG register. Per-channel averaging enable registers must also
>> be updated to activate or deactivate averaging.
>>
>> Signed-off-by: Salih Erim <salih.erim@amd.com>
>
> One minor comment inline.
>
>> drivers/iio/adc/versal-sysmon-core.c | 147 ++++++++++++++++++++++++++-
>> drivers/iio/adc/versal-sysmon.h | 17 ++++
>> 2 files changed, 163 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
>> index 20fd3a87d44..fa8f0dc868a 100644
>> --- a/drivers/iio/adc/versal-sysmon-core.c
>> +++ b/drivers/iio/adc/versal-sysmon-core.c
>
>> +static int sysmon_osr_write(struct sysmon *sysmon, int channel_type, int val)
>
> There is almost nothing shared in here between the two channel_types.
> Might make sense to just split it into two helpers, particularly as
> there is a channel type if / else at the caller.
Agreed. The only shared code outside the switch is the hw_val
computation. Will split into two helpers in v7.
Thanks,
Salih
>
>> +{
>> + /*
>> + * HW register encoding is sample_count / 2:
>> + * 0=none, 1=2x, 2=4x, 4=8x, 8=16x (not log2-based).
>> + */
>> + int hw_val = val >> 1;
>> + unsigned int readback;
>> + int ret;
>> +
>> + switch (channel_type) {
>> + case IIO_TEMP:
>> + ret = regmap_update_bits(sysmon->regmap, SYSMON_CONFIG,
>> + SYSMON_CONFIG_TEMP_SAT_OSR,
>> + FIELD_PREP(SYSMON_CONFIG_TEMP_SAT_OSR, hw_val));
>> + if (ret)
>> + return ret;
>> +
>> + /*
>> + * Readback fence: the SysMon CONFIG register resides in the
>> + * PMC domain behind the NoC. A posted write may not reach the
>> + * hardware before the next MMIO access. Reading the register
>> + * back forces the interconnect to complete the write, preventing
>> + * a bus hang on the subsequent access.
>> + */
>> + regmap_read(sysmon->regmap, SYSMON_CONFIG, &readback);
>> +
>> + return sysmon_set_avg_enable(sysmon, SYSMON_TEMP_EN_AVG_BASE,
>> + SYSMON_TEMP_EN_AVG_COUNT,
>> + hw_val ? ~0U : 0);
>> + case IIO_VOLTAGE:
>> + ret = regmap_update_bits(sysmon->regmap, SYSMON_CONFIG,
>> + SYSMON_CONFIG_SUPPLY_OSR,
>> + FIELD_PREP(SYSMON_CONFIG_SUPPLY_OSR, hw_val));
>> + if (ret)
>> + return ret;
>> +
>> + /* Readback fence -- see above */
>> + regmap_read(sysmon->regmap, SYSMON_CONFIG, &readback);
>> +
>> + return sysmon_set_avg_enable(sysmon, SYSMON_SUPPLY_EN_AVG_BASE,
>> + SYSMON_SUPPLY_EN_AVG_COUNT,
>> + hw_val ? ~0U : 0);
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int sysmon_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int val, int val2, long mask)
>> +{
>> + struct sysmon *sysmon = iio_priv(indio_dev);
>> + unsigned int i;
>> + int ret;
>> +
>> + if (mask != IIO_CHAN_INFO_OVERSAMPLING_RATIO)
>> + return -EINVAL;
>> +
>> + for (i = 0; i < ARRAY_SIZE(sysmon_oversampling_avail); i++) {
>> + if (val == sysmon_oversampling_avail[i])
>> + break;
>> + }
>> + if (i == ARRAY_SIZE(sysmon_oversampling_avail))
>> + return -EINVAL;
>> +
>> + guard(mutex)(&sysmon->lock);
>> +
>> + ret = sysmon_osr_write(sysmon, chan->type, val);
>> + if (ret)
>> + return ret;
>> +
>> + if (chan->type == IIO_TEMP)
>> + sysmon->temp_oversampling = val;
>> + else
>> + sysmon->supply_oversampling = val;
>> +
>> + return 0;
>> +}
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor
From: Jonathan Cameron @ 2026-06-12 13:45 UTC (permalink / raw)
To: Joshua Crofts
Cc: Siratul Islam, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260612151324.0000704d@gmail.com>
On Fri, 12 Jun 2026 15:13:24 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> On Fri, 12 Jun 2026 18:45:26 +0600
> Siratul Islam <email@sirat.me> wrote:
>
> > +QST QMC5883L 3-Axis Magnetic Sensor
> > +M: Siratul Islam <email@sirat.me>
> > +L: linux-iio@vger.kernel.org
>
> There's no point in having the IIO list in your MAINTAINERS
> entry, get_maintainer.pl would return it automatically based
> on the driver file's path.
True, but I'm not sure there is a standard convention for whether
lists should be added in this case or not.
One of those things where we should decide on an answer perhaps
and stick to it. Andy, Nuno, David, Dt folk what do you think?
>
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: kodiak: Sort pinctrl subnodes by pins
From: Luca Weiss @ 2026-06-12 13:46 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, devicetree,
linux-kernel, Luca Weiss
In-Reply-To: <f05ad4ae-140a-40a7-a6ef-9ac2ddb0a939@linaro.org>
On Fri Jun 12, 2026 at 2:59 PM CEST, Vladimir Zapolskiy wrote:
> As documented in the "Devicetree Sources (DTS) Coding Style" document,
> pinctrl subnodes should be sorted by the pins property. Do this once for
> kodiak.dtsi so that future additions can be added at the right places.
>
> No functional change intended, verified with dtx_diff.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> arch/arm64/boot/dts/qcom/kodiak.dtsi | 1382 +++++++++++++++++-----------------
> 1 file changed, 691 insertions(+), 691 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> index fa540d8c2615..62daef726d32 100644
> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
<snip>
> + qup_uart12_cts: qup-uart12-cts-state {
> + pins = "gpio48";
> + function = "qup14";
> + };
> +
> + qup_uart12_rts: qup-uart12-rts-state {
> + pins = "gpio49";
> + function = "qup14";
> + };
> +
> + qup_uart12_tx: qup-uart12-tx-state {
> + pins = "gpio50";
> + function = "qup14";
> + };
>
> I understand and support the intention to keep this change non-functional,
> but this pad "gpio50" is for qup16 also, right?
According to my QCM6490 data sheet, GPIO_50 has these functions:
* UART for qup14 (OK)
* SPI for qup14 (OK)
* SPI for qup16 (no pinctrl)
>
> Similarly pads "gpio54"/"gpio55" for qup14 function, "gpio62"/"gpio63"
> for qup16 function, I find all of these are missing on the original list.
GPIO_54:
* UART qup15 (OK)
* SPI qup15 (OK)
* SPI qup14 (no pinctrl)
GPIO_55:
* UART qup15 (OK)
* SPI qup15 (OK)
* SPI qup14 (no pinctrl)
GPIO_62:
* UART qup17 (OK)
* SPI qup17 (OK)
* SPI qup16 (no pinctrl)
GPIO_63:
* UART qup16 (?)
* SPI qup16 (lane 3) (?)
* SPI qup16 (lane 5) (?)
But the GPIO_63 looks weird, is the data sheet wrong?! Where would
UART_RX of QUP1 SE7 go? Maybe it should be UART qup17 and SPI qup17 and
then SPI qup16 ??
Can somebody at Qualcomm please check 80-20659-1 Rev. AM and maybe make
the apppriate people there aware?
So yes Vladimir, you're correct. Some pinctrl definitions for those SPI
QUPs are not defined. And the datasheet seems wrong as well.
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
Thanks for checking!
Regards
Luca
^ permalink raw reply
* Re: [PATCH v4 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Andrea della Porta @ 2026-06-12 13:51 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andrea della Porta, sashiko-reviews, conor+dt, devicetree, robh,
Kevin Hilman
In-Reply-To: <aiM0VlVXAWs54v-G@monoceros>
Hi Uwe,
On 22:51 Fri 05 Jun , Uwe Kleine-König wrote:
> Hello Andrea,
>
> On Fri, Jun 05, 2026 at 06:49:17PM +0200, Andrea della Porta wrote:
> > On 23:28 Thu 04 Jun , Uwe Kleine-König wrote:
> > > Thinking again, the tohw() callback could be a bit more clever and also
> > > use period_ticks = 0xffffffff, as this fine if duty_ticks is less than
> > > this value and if duty_ticks = period_ticks = 0xffffffff you can still
> > > configure the hardware using period_ticks = 0xfffffffe to achieve the
> > > 100% relative dutycycle. (But keeping the current behaviour is fine for
> > > me, too.)
> >
> > I think this is what it's currently doing in this patch iteration. I stand
> > corrected here when I said that period_ticks should be at max U32_MAX-1: logically
> > it can be U32_MAX but it's getting 'translated' into U32_MAX-1 as the value which
> > is fed to the register. So the period is still U32_MAX, accounting for the extra
> > tick at the end. So I guess we're on the same page.
>
> Not sure you got what I wrote. You can use the maximal value possible in
> hardware, because for all but 100% relative duty cycle it works as
> expected. You only need to map 100% relative duty cycle with that
> maximal period to a different 100% relative duty cycle setting. I think
> PWM_DEBUG even doesn't yell at you for that. (And if it does, that needs
> fixing.)
I see. Since you said that teh current behaviour is also fine, I'd prefer
to stick with it to avoid complicating the code.
>
> > > > I'm not sure whether an inverted polarity pin shoudl stay low when disabled.
> > > > After all, the inactive state for a reversed pin is high.
> > >
> > > Sashiko's concern is correctly stated, if you go from
> > >
> > > polarity = inversed, enabled
> > >
> > > to
> > >
> > > polarity = normal, disabled
> > >
> > > the output stays high, which is active for polarity = normal.
> >
> > Ack. I'll set the polarity first so there will be no uncovered corner
> > case.
>
> You can, but as I wrote below, being lazy is also fine.
Already done :)
>
> > > However the behaviour of a disabled PWM isn't specified, so any
> > > behaviour is fine, the only objective is to save power. And if the
> > > consumer relies on a constant inactive output, it's supposed to not
> > > disable it.
> > >
> > > For me both behaviours are fine. Making the hardware emit the inactive
> > > level might prevent a surprise if the consumer isn't aware of the
> > > missing guarantee, but being lazy and so surprise the consumer is also
> > > fine as this might uncover that wrong assumption and allow the consumer
> > > to be fixed.
> > >
> > > (And not all PWM implementations allow to configure the output level, so
> > > a guarantee cannot be given. Some go to 0 irrespective of the configured
> > > polarity, some go to High-Z.)
> >
> > I was just curious about sashiko saying it's violating the pwm framework
> > expectations, while according to your words it seems there's no constraints.
>
> In doubt trust me :-)
Sure!
>
> > BTW, I've tried to install sashiko and use it on my patches but it's obvious
> > that some custom settings are in order, since all I can get is some error
> > aborting the review after 3 attempts. Any chance you can share your Settings.toml
> > or any customization so I can test it in advance before submitting the new patchset
> > or do you recommend just throwing the new V5 at your script?
>
> Note that sashiko is not "my script", it was setup by Google engineers
> and I have nothing to do with it (apart from benefitting from its review
> feedback). Kevin (added to Cc) tried to setup a local instance with his
> Claude plan, but (IIUC) it quickly ate his day's amount of tokens before
> completing review of a series of only 4 patches.
>
> So without knowing the size of Kevin's or your AI plan, probably it's
> easier to just rely on the public instance. (And IMHO that's nothing to
> be afraid of, just handle it like a human reviewer. For these you also
> don't know what they will reply for your next revision.)
Ack.
Regards,
Andrea
>
> Best regards
> Uwe
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
From: Joshua Crofts @ 2026-06-12 13:49 UTC (permalink / raw)
To: Siratul Islam
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260612124557.13750-4-email@sirat.me>
On Fri, 12 Jun 2026 18:45:27 +0600
Siratul Islam <email@sirat.me> wrote:
> Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
> connected via i2c.
>
> Signed-off-by: Siratul Islam <email@sirat.me>
> ---
Hi Siratul,
various comments inline. I've probably missed a few things
as I only took a quick look so feel free to call me out on that!
Josh
> --- /dev/null
> +++ b/drivers/iio/magnetometer/qmc5883l.c
> @@ -0,0 +1,512 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +/*
> + * Support for QST QMC5883L 3-Axis Magnetic Sensor on i2c bus.
Nitpick, but I'd write it I2C with capital letters.
> + *
> + * Copyright (C) 2026 Siratul Islam <email@sirat.me>
> + *
> + * Datasheet available at
> + * <https://www.qstcorp.com/upload/pdf/202512/13-52-04%20QMC5883L%20Datasheet%20Rev.%20B.pdf>
> + *
> + * Default 7-bit i2c slave address 0x0D.
You also have this as a macro, consider removing this comment.
> + *
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
Move IIO specific headers below generic linux headers, i. e.
#include <linux/x.h>
#include <linux/y.h>
#include <linux/iio/z.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
No, refrain from using kernel.h, include the actual headers
instead of relying on this (on second thought, I think you've
actually included all the required headers, so you definitely
don't need this).
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/time.h>
> +
> +#include <asm/byteorder.h>
> +
> +#define QMC5883L_REG_X_LSB 0x00
> +#define QMC5883L_REG_STATUS1 0x06
> +#define QMC5883L_REG_CTRL1 0x09
> +#define QMC5883L_REG_CTRL2 0x0A
> +#define QMC5883L_REG_SET_RESET 0x0B
> +#define QMC5883L_REG_ID 0x0D
> +
> +#define QMC5883L_CHIP_ID 0xFF
> +
> +#define QMC5883L_MODE_MASK GENMASK(1, 0)
> +#define QMC5883L_ODR_MASK GENMASK(3, 2)
> +#define QMC5883L_RNG_MASK GENMASK(5, 4)
> +#define QMC5883L_OSR_MASK GENMASK(7, 6)
> +
> +#define QMC5883L_MODE_STANDBY FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x00)
> +#define QMC5883L_MODE_CONT FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x01)
> +
> +#define QMC5883L_ODR_10HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x00)
> +#define QMC5883L_ODR_50HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x01)
> +#define QMC5883L_ODR_100HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x02)
> +#define QMC5883L_ODR_200HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x03)
> +
> +#define QMC5883L_RNG_2G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x00)
> +#define QMC5883L_RNG_8G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x01)
> +
> +#define QMC5883L_OSR_512 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x00)
> +#define QMC5883L_OSR_256 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x01)
> +#define QMC5883L_OSR_128 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x02)
> +#define QMC5883L_OSR_64 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x03)
> +
> +#define QMC5883L_STATUS_DRDY BIT(0)
> +#define QMC5883L_STATUS_OVL BIT(1)
> +
> +#define QMC5883L_SET_RESET_VAL BIT(0)
> +#define QMC5883L_INT_DISABLE BIT(0)
> +#define QMC5883L_SOFT_RESET BIT(7)
> +
> +#define QMC5883L_SCALE_2G 83333
> +#define QMC5883L_SCALE_8G 333333
> +
> +/* POR completion time max per datasheet */
> +#define QMC5883L_PORT_US 350
If it's POR completion, why does the macro contain PORT instead?
> +
> +struct qmc5883l_data {
> + struct regmap *regmap;
> + struct mutex mutex; /* update and read regmap data */
> + u8 range;
> + u8 odr;
> + u8 osr;
> +};
> +
> +enum qmc5883l_chan {
> + QMC5883L_AXIS_X,
> + QMC5883L_AXIS_Y,
> + QMC5883L_AXIS_Z,
> +};
> +
> +static const int qmc5883l_odr_avail[] = { 10, 50, 100, 200 };
> +
> +static const int qmc5883l_osr_avail[] = { 512, 256, 128, 64 };
> +
> +static const int qmc5883l_rng_avail[] = {
> + 0, QMC5883L_SCALE_2G, /* 2G */
These comments are redundant IMO, you're already mentioning
the value in the macro name.
> + 0, QMC5883L_SCALE_8G, /* 8G */
> +};
> +
> +static int qmc5883l_take_measurement(struct iio_dev *indio_dev, int index,
> + int *val)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + unsigned int status;
> + __le16 buf[3];
> + int ret;
> +
> + scoped_guard(mutex, &data->mutex)
Why scoped_guard? I don't see any other possible code paths in this
function etc., just regular guard() should suffice.
> + {
> + /* 50ms headroom over the slowest ODR (10Hz) */
> + ret = regmap_read_poll_timeout(data->regmap,
> + QMC5883L_REG_STATUS1, status,
> + (status & QMC5883L_STATUS_DRDY),
> + 2 * USEC_PER_MSEC,
> + 150 * USEC_PER_MSEC);
> + if (ret)
> + return ret;
> +
> + if (status & QMC5883L_STATUS_OVL)
> + return -ERANGE;
Sashiko has a remark:
If we return -ERANGE here when the overflow flag (OVL) is set, does the
sensor get permanently stuck in an overflow state?
In typical I2C magnetometers, the Data Ready (DRDY) and Overflow (OVL)
status bits are only cleared by reading the data registers. By returning
early without reading the data registers via regmap_bulk_read(), the DRDY
and OVL flags might remain set indefinitely.
On subsequent measurement attempts, regmap_read_poll_timeout() will return
immediately and this check will instantly fail again, potentially locking up
the sensor until a reset.
> +
> + ret = regmap_bulk_read(data->regmap, QMC5883L_REG_X_LSB, buf,
> + sizeof(buf));
> + if (ret)
> + return ret;
> +
> + *val = (s16)le16_to_cpu(buf[index]);
> + }
> +
> + return 0;
> +}
> +
> +static int qmc5883l_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val,
I'd put val on the same line as val2 and mask, more logical separation.
> + int *val2, long mask)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = qmc5883l_take_measurement(indio_dev, chan->address, val);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
Again, you can probably just add guard() before the switch.
> + scoped_guard(mutex, &data->mutex)
> + {
> + *val = 0;
> + *val2 = data->range == QMC5883L_RNG_2G ?
> + QMC5883L_SCALE_2G :
> + QMC5883L_SCALE_8G;
> + }
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + scoped_guard(mutex, &data->mutex)
> + {
> + switch (data->odr) {
> + case QMC5883L_ODR_200HZ:
> + *val = 200;
> + break;
> + case QMC5883L_ODR_100HZ:
> + *val = 100;
> + break;
> + case QMC5883L_ODR_50HZ:
> + *val = 50;
> + break;
> + case QMC5883L_ODR_10HZ:
> + *val = 10;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + scoped_guard(mutex, &data->mutex)
> + {
> + switch (data->osr) {
> + case QMC5883L_OSR_64:
> + *val = 64;
> + break;
> + case QMC5883L_OSR_128:
> + *val = 128;
> + break;
> + case QMC5883L_OSR_256:
> + *val = 256;
> + break;
> + case QMC5883L_OSR_512:
> + *val = 512;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int qmc5883l_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int val,
> + int val2, long mask)
Same comment with val as previous function.
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + u8 rng;
> + u8 osr;
> + u8 odr;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val != 0)
> + return -EINVAL;
> +
> + switch (val2) {
> + case QMC5883L_SCALE_2G:
> + rng = QMC5883L_RNG_2G;
> + break;
> + case QMC5883L_SCALE_8G:
> + rng = QMC5883L_RNG_8G;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
Again, guard() is probably good enough.
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_RNG_MASK, rng);
> + if (ret)
> + return ret;
> + data->range = rng;
> + }
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + switch (val) {
> + case 200:
> + odr = QMC5883L_ODR_200HZ;
> + break;
> + case 100:
> + odr = QMC5883L_ODR_100HZ;
> + break;
> + case 50:
> + odr = QMC5883L_ODR_50HZ;
> + break;
> + case 10:
> + odr = QMC5883L_ODR_10HZ;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_ODR_MASK, odr);
> + if (ret)
> + return ret;
> + data->odr = odr;
> + }
> + break;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + switch (val) {
> + case 64:
> + osr = QMC5883L_OSR_64;
> + break;
> + case 128:
> + osr = QMC5883L_OSR_128;
> + break;
> + case 256:
> + osr = QMC5883L_OSR_256;
> + break;
> + case 512:
> + osr = QMC5883L_OSR_512;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_OSR_MASK, osr);
> + if (ret)
> + return ret;
> + data->osr = osr;
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int qmc5883l_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *vals = qmc5883l_odr_avail;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(qmc5883l_odr_avail);
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *vals = qmc5883l_osr_avail;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(qmc5883l_osr_avail);
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_SCALE:
> + *vals = qmc5883l_rng_avail;
> + *type = IIO_VAL_INT_PLUS_NANO;
> + *length = ARRAY_SIZE(qmc5883l_rng_avail);
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int qmc5883l_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return IIO_VAL_INT;
> + }
> +}
> +
> +static const struct iio_info qmc5883l_info = {
> + .read_raw = qmc5883l_read_raw,
> + .write_raw = qmc5883l_write_raw,
> + .read_avail = qmc5883l_read_avail,
> + .write_raw_get_fmt = qmc5883l_write_raw_get_fmt,
> +};
> +
> +static int qmc5883l_init(struct qmc5883l_data *data)
> +{
> + unsigned int reg;
> + int ret;
struct regmap *regmap = data->regmap;
Makes your lines shorter.
> + ret = regmap_read(data->regmap, QMC5883L_REG_ID, ®);
> + if (ret)
> + return ret;
> +
> + /* Not failing because rev 1.0 had this register reserved */
> + if (reg != QMC5883L_CHIP_ID)
> + dev_warn(regmap_get_device(data->regmap),
> + "unknown chip id: 0x%02x, continuing\n", reg);
I recall a conversation about using dev_warn() or dev_info() for checking
the chip ID, up to personal preference. Nevertheless, your error messages
should follow the same format, so capital letter at the beginning (nitpick).
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
> + QMC5883L_SOFT_RESET);
> + if (ret)
> + return ret;
> +
> + fsleep(QMC5883L_PORT_US);
> +
> + /* DRDY pin no used in this version of the driver */
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
> + QMC5883L_INT_DISABLE);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_SET_RESET,
> + QMC5883L_SET_RESET_VAL);
> + if (ret)
> + return ret;
> +
> + data->odr = QMC5883L_ODR_50HZ;
> + data->range = QMC5883L_RNG_2G;
> + data->osr = QMC5883L_OSR_64;
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL1,
> + (QMC5883L_MODE_CONT | data->odr | data->range |
> + data->osr));
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static bool qmc5883l_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + return reg <= QMC5883L_REG_STATUS1;
> +}
> +
> +static bool qmc5883l_writable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case QMC5883L_REG_CTRL1:
> + case QMC5883L_REG_CTRL2:
> + case QMC5883L_REG_SET_RESET:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config qmc5883l_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = QMC5883L_REG_ID,
> + .cache_type = REGCACHE_MAPLE,
> + .volatile_reg = qmc5883l_volatile_reg,
> + .writeable_reg = qmc5883l_writable_reg
> +};
> +
> +#define QMC5883L_CHANNEL(_axis) \
> + { \
> + .type = IIO_MAGN, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_##_axis, \
> + .address = QMC5883L_AXIS_##_axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> + .info_mask_shared_by_type_available = \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> + }
> +
> +static const struct iio_chan_spec qmc5883l_channels[] = {
> + QMC5883L_CHANNEL(X),
> + QMC5883L_CHANNEL(Y),
> + QMC5883L_CHANNEL(Z),
> +};
> +
> +static int qmc5883l_probe(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev;
> + struct regmap *regmap;
> + struct qmc5883l_data *data;
> + struct device *dev = &client->dev;
Reverse Christmas tree order.
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init_i2c(client, &qmc5883l_regmap_config);
> + if (IS_ERR(regmap)) {
No point in adding brackets if this is a single line if statement
(checkpatch.pl should warn about this IMO).
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "regmap initialization failed\n");
> + }
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable VDD regulator\n");
> +
> + ret = devm_regulator_get_enable(dev, "vddio");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable VDDIO regulator\n");
> +
> + fsleep(QMC5883L_PORT_US);
> +
> + data = iio_priv(indio_dev);
> + data->regmap = regmap;
Blank line here.
> + ret = devm_mutex_init(dev, &data->mutex);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = "qmc5883l";
> + indio_dev->info = &qmc5883l_info;
> + indio_dev->channels = qmc5883l_channels;
> + indio_dev->num_channels = ARRAY_SIZE(qmc5883l_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = qmc5883l_init(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "qmc5883l init failed\n");
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
You're using devm_* functions, however I don't see any callback on driver
unbinding or a potential error during probing. Consider adding a power off
callback.
--
Kind regards
CJD
^ permalink raw reply
* Re: [PATCH v4 05/16] riscv: Add Zicclsm to cpufeature and hwprobe
From: Jesse Taube @ 2026-06-12 13:51 UTC (permalink / raw)
To: Guodong Xu
Cc: Jonathan Corbet, Shuah Khan, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Zong Li, Deepak Gupta, Anup Patel,
Atish Patra, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Yixun Lan, Chen Wang, Inochi Amaoto, linux-doc, linux-riscv,
linux-kernel, kvm, kvm-riscv, Paul Walmsley, Conor Dooley,
devicetree, spacemit, sophgo, linux-kselftest, Palmer Dabbelt,
Jesse Taube, Conor Dooley, Charlie Jenkins, Andrew Jones,
Andy Chiu
In-Reply-To: <20260611-rva23u64-hwprobe-v2-v4-5-3f01a2449488@gmail.com>
On Thu, Jun 11, 2026 at 4:14 PM Guodong Xu <docular.xu@gmail.com> wrote:
>
> From: Jesse Taube <jesse@rivosinc.com>
>
> Zicclsm requires misaligned support for all regular load and store
> instructions, both scalar and vector, but not AMOs or other
> specialized forms of memory access, to main memory regions with both
> the cacheability and coherence PMAs, as defined in the profiles spec.
> Even though mandated, misaligned loads and stores might execute
> extremely slowly. Standard software distributions should assume their
> existence only for correctness, not for performance.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andy Chiu <andy.chiu@sifive.com>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> Tested-by: Charlie Jenkins <charlie@rivosinc.com>
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
Thanks for the update! Just an fyi email has changed to
jtaubepe@redhat.com though.
No need to change the signoff though.
Thanks,
Jesse Taube
> [Rebased, rewrote doc text, minor commit message revisions]
> Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
> Signed-off-by: Guodong Xu <docular.xu@gmail.com>
>
> ---
> v4: No change.
> v3:
> - Move the hwprobe.rst entry to the IMA_EXT_1 section so its
> documentation matches the IMA_EXT_1 bit it was allocated in v2
> (Sashiko, agreed by Andrew).
> v2:
> - Rebased onto v7.1-rc2; moved ZICCLSM to IMA_EXT_1 and
> allocated a new bit for it
> ---
> Documentation/arch/riscv/hwprobe.rst | 4 ++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/uapi/asm/hwprobe.h | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> arch/riscv/kernel/sys_hwprobe.c | 1 +
> 5 files changed, 8 insertions(+)
>
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> index d9928641deb99..49d9fb68632d0 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -401,3 +401,7 @@ The following keys are defined:
> as defined in version 1.0 of the RISC-V Control-flow Integrity (CFI)
> extensions specification, ratified in commit 302a2d45c243
> ("Update build-pdf.yml") of riscv-cfi.
> +
> + * :c:macro:`RISCV_HWPROBE_EXT_ZICCLSM`: The Zicclsm extension is supported,
> + as defined in the RISC-V Profiles specification starting from commit
> + b1d80660 ("Updated to ratified state.")
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 44bf8c7d8acc5..e8f4a7dd96a93 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -112,6 +112,7 @@
> #define RISCV_ISA_EXT_ZCLSD 103
> #define RISCV_ISA_EXT_ZICFILP 104
> #define RISCV_ISA_EXT_ZICFISS 105
> +#define RISCV_ISA_EXT_ZICCLSM 106
>
> #define RISCV_ISA_EXT_XLINUXENVCFG 127
>
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 9139edba0aecb..6819df159c51e 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -116,6 +116,7 @@ struct riscv_hwprobe {
> #define RISCV_HWPROBE_KEY_ZICBOP_BLOCK_SIZE 15
> #define RISCV_HWPROBE_KEY_IMA_EXT_1 16
> #define RISCV_HWPROBE_EXT_ZICFISS (1ULL << 0)
> +#define RISCV_HWPROBE_EXT_ZICCLSM (1ULL << 1)
>
> /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 686dde3ce3b98..1fb595581adcf 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -502,6 +502,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicbom, RISCV_ISA_EXT_ZICBOM, riscv_xlinuxenvcfg_exts, riscv_ext_zicbom_validate),
> __RISCV_ISA_EXT_DATA_VALIDATE(zicbop, RISCV_ISA_EXT_ZICBOP, riscv_ext_zicbop_validate),
> __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicboz, RISCV_ISA_EXT_ZICBOZ, riscv_xlinuxenvcfg_exts, riscv_ext_zicboz_validate),
> + __RISCV_ISA_EXT_DATA(zicclsm, RISCV_ISA_EXT_ZICCLSM),
> __RISCV_ISA_EXT_DATA(ziccrse, RISCV_ISA_EXT_ZICCRSE),
> __RISCV_ISA_EXT_SUPERSET_VALIDATE(zicfilp, RISCV_ISA_EXT_ZICFILP, riscv_xlinuxenvcfg_exts,
> riscv_cfilp_validate),
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index f8f68ba781b45..9cf62266f1890 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -205,6 +205,7 @@ static void hwprobe_isa_ext1(struct riscv_hwprobe *pair,
> * in the hart_isa bitmap, are made.
> */
> EXT_KEY(isainfo->isa, ZICFISS, pair->value, missing);
> + EXT_KEY(isainfo->isa, ZICCLSM, pair->value, missing);
> }
>
> /* Now turn off reporting features if any CPU is missing it. */
>
> --
> 2.43.0
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
^ permalink raw reply
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