* Re: [PATCH v2 2/2] software node: Fix software_node_get_reference_args() with index -1
From: sashiko-bot @ 2026-06-18 15:31 UTC (permalink / raw)
To: Alban Bedel; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260618152035.1600436-2-alban.bedel@lht.dlh.de>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-after-free vulnerability in software node reference resolution.
- [High] Data race and linked list corruption in software node children management.
--
commit 3b564802f7102505002d84b52abe9e6a26646a5d
Author: Alban Bedel <alban.bedel@lht.dlh.de>
software node: Fix software_node_get_reference_args() with index -1
This patch corrects an out-of-bounds access check in
software_node_get_reference_args() by ensuring the bounds check correctly
handles UINT_MAX as an index.
Closes: https://lore.kernel.org/linux-devicetree/20260611103904.7CB131F00893@smtp.kernel.org/
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index a19f8f722bc8e..e76f7c36f19e6 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -529,7 +529,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
> if (prop->is_inline)
> return -EINVAL;
>
> - if ((index + 1) * sizeof(*ref) > prop->length)
> + if (index >= prop->length / sizeof(*ref))
> return -ENOENT;
>
> ref_array = prop->pointer;
[Severity: High]
This isn't a bug introduced by this patch, but it looks like there's a
pre-existing use-after-free risk later in this function.
When we resolve the reference here:
if (ref->swnode)
refnode = software_node_fwnode(ref->swnode);
...
args->fwnode = fwnode_handle_get(refnode);
software_node_fwnode() calls software_node_to_swnode(), which drops the
swnode_kset->list_lock and returns the swnode pointer without elevating its
reference count via kobject_get():
drivers/base/swnode.c:software_node_to_swnode() {
...
list_for_each_entry(k, &swnode_kset->list, entry) {
swnode = kobj_to_swnode(k);
if (swnode->node == node)
break;
swnode = NULL;
}
spin_unlock(&swnode_kset->list_lock);
return swnode;
}
Since the pointer is unreferenced, could it be concurrently unregistered
and freed (e.g., by software_node_unregister()) before we call
fwnode_handle_get() on it?
[Severity: High]
This is also a pre-existing issue, but I noticed a potential data race and
linked list corruption in the software node children management in
drivers/base/swnode.c.
In swnode_register(), child nodes are added to the parent's list without
a lock:
drivers/base/swnode.c:swnode_register() {
...
if (parent)
list_add_tail(&swnode->entry, &parent->children);
...
}
And in software_node_release(), they are removed without synchronization:
drivers/base/swnode.c:software_node_release() {
...
if (swnode->parent) {
ida_free(&swnode->parent->child_ids, swnode->id);
list_del(&swnode->entry);
}
...
}
If multiple child nodes are concurrently registered or unregistered under the
same parent (e.g., dynamically by drivers calling
fwnode_create_software_node()), could this corrupt the doubly linked list and
crash the kernel?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618152035.1600436-1-alban.bedel@lht.dlh.de?part=2
^ permalink raw reply
* [PATCH v4 0/3] perf: marvell: LLC-TAD PMU MPAM filtering support
From: Geetha sowjanya @ 2026-06-18 15:36 UTC (permalink / raw)
To: linux-perf-users, linux-kernel, linux-arm-kernel, devicetree
Cc: mark.rutland, will, krzk+dt, gakula
This series extends the Marvell LLC-TAD performance driver used on CN10K
and CN20K systems.
Patch 1 adds optional MPAM partition-id filtering for the subset of TAD
events that support it, exposes partid / partid_en in the PMU format string,
and keeps the reduced Odyssey event surface without advertising partid where
it does not apply. It also fixes probe resource handling (no in-place
mutation of platform_get_resource() bounds, validate MMIO window vs
tad-cnt), registers CPU hotplug before perf_pmu_register with unwind, and
aligns the filter-enable bit in config1 with the sysfs format (bit 9).
Patch 2 introduces CN20K LLC-TAD support: non-standard PFC/PRF offsets,
additional programmable events with visibility checks so CN10K does not
advertise V3-only events, CN20K-specific MPAM encoding for the V3 profile,
local64_set(prev_count) on counter start, and device discovery via OF and
ACPI.
Patch 3 extends the DeviceTree binding for marvell,cn20k-tad-pmu.
Changes since v3
----------------
- Add perf_ready: tad_pmu_offline_cpu skips perf_pmu_migrate_context until after
successful perf_pmu_register, so a CPU offline between hotplug add and perf
register does not touch perf core state for an unregistered PMU.
Changes since v2
----------------
- Validate the eventId using an appropriate mask to ensure it is restricted to 8 bits.
Changes since v1
----------------
- config1: use bit 9 for MPAM filter enable consistently with partid_en in
the PMU format; allow only bits 0..9 in event_init on CN10K/CN20K paths.
- Reject reserved bits in attr.config and use the same 8-bit event index in
start_counter as in event_init so MPAM validation cannot be bypassed.
- Register CPU hotplug before perf_pmu_register in probe (mainline order); add
perf_ready so offline migration is skipped until after perf registration
(reconciles v1 vs v2 ordering feedback).
- Hide V3-only sysfs events on V1.
- Reset prev_count when starting counters after clearing hardware.
- DT binding: explain non-fallback compatibles for CN10K vs CN20K.
Tanmay Jagdale (1):
perf: marvell: Add MPAM partid filtering to CN10K TAD PMU
Geetha sowjanya (2):
perf: marvell: Add CN20K LLC-TAD PMU support
dt-bindings: perf: marvell: Extend CN10K TAD PMU binding for CN20K
Signed-off-by: Geetha sowjanya <gakula@marvell.com>
--
2.25.1
^ permalink raw reply
* [PATCH v4 3/3] dt-bindings: perf: marvell: add CN20K TAD PMU support
From: Geetha sowjanya @ 2026-06-18 15:36 UTC (permalink / raw)
To: linux-perf-users, linux-kernel, linux-arm-kernel, devicetree
Cc: mark.rutland, will, krzk+dt, gakula
In-Reply-To: <20260618153610.13649-1-gakula@marvell.com>
Marvell CN20K SoCs integrate a Performance Monitoring Unit (PMU)
associated with the LLC Tag-and-Data (TAD) blocks. The PMU provides
hardware counters to monitor cache traffic and performance events
via a dedicated MMIO region.
The CN20K LLC-TAD PMU is largely similar to CN10K, but differs in the
layout of PFC/PRF register offsets relative to each TAD base. These
offsets are derived from the compatible string in the driver and are
not described through Devicetree properties.
Because of this, using "marvell,cn10k-tad-pmu" as a fallback for CN20K
would result in incorrect register programming. Therefore, add a
separate compatible string:
"marvell,cn20k-tad-pmu"
Update the binding to document CN20K alongside CN10K.
Signed-off-by: Geetha sowjanya <gakula@marvell.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../bindings/perf/marvell-cn10k-tad.yaml | 25 +++++++++++++------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/perf/marvell-cn10k-tad.yaml b/Documentation/devicetree/bindings/perf/marvell-cn10k-tad.yaml
index 362142252667..d11121a1e2c9 100644
--- a/Documentation/devicetree/bindings/perf/marvell-cn10k-tad.yaml
+++ b/Documentation/devicetree/bindings/perf/marvell-cn10k-tad.yaml
@@ -4,23 +4,32 @@
$id: http://devicetree.org/schemas/perf/marvell-cn10k-tad.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Marvell CN10K LLC-TAD performance monitor
+title: Marvell CN10K / CN20K LLC-TAD performance monitor
maintainers:
- Bhaskara Budiredla <bbudiredla@marvell.com>
+ - Geetha sowjanya <gakula@marvell.com>
description: |
- The Tag-and-Data units (TADs) maintain coherence and contain CN10K
- shared on-chip last level cache (LLC). The tad pmu measures the
- performance of last-level cache. Each tad pmu supports up to eight
- counters.
+ The Tag-and-Data units (TADs) maintain coherence and contain the
+ shared on-chip last level cache (LLC) on Marvell CN10K and CN20K SoCs.
+ The TAD PMU measures last-level cache performance. Each TAD PMU
+ supports up to eight counters.
- The DT setup comprises of number of tad blocks, the sizes of pmu
- regions, tad blocks and overall base address of the HW.
+ The DT setup describes the number of TAD blocks, the sizes of PMU
+ regions and TAD pages, and the overall MMIO base of the hardware.
+
+ marvell,cn20k-tad-pmu is not a compatible fallback for
+ marvell,cn10k-tad-pmu (and vice versa): the driver selects different
+ PFC/PRF MMIO offsets from the compatible string, and those offsets are
+ not described by separate DT properties today.
properties:
compatible:
- const: marvell,cn10k-tad-pmu
+ items:
+ - enum:
+ - marvell,cn10k-tad-pmu
+ - marvell,cn20k-tad-pmu
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [PATCH v4 1/3] perf: marvell: Add MPAM partid filtering to CN10K TAD PMU
From: Geetha sowjanya @ 2026-06-18 15:36 UTC (permalink / raw)
To: linux-perf-users, linux-kernel, linux-arm-kernel, devicetree
Cc: mark.rutland, will, krzk+dt, gakula
In-Reply-To: <20260618153610.13649-1-gakula@marvell.com>
From: Tanmay Jagdale <tanmay@marvell.com>
The TAD PMU exposes counters that can be filtered by MPAM partition id
for a subset of allocation and hit events.
Add a 9-bit partid format attribute (config1) and route counter programming
through variant-specific ops so CN10K keeps MPAM-capable programming while
Odyssey keeps the reduced event set without advertising partid in sysfs.
Probe no longer mutates the platform_device MMIO resource (walk a local
map_start), rejects tad-cnt / page sizes of zero, validates the memory
window against tad-cnt, and registers the perf PMU before hotplug with
correct unwind.
Example:
perf stat -e tad/tad_alloc_any,partid=0x12,partid_en=1/ -- <program>
Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
Signed-off-by: Geetha sowjanya <gakula@marvell.com>
---
Changelog (since v3)
--------------------
- Restore cpuhp_state_add_instance_nocalls before perf_pmu_register in probe
so users cannot attach events before the hotplug instance exists; unwind
removes the hotplug instance if perf registration fails.
- Add perf_ready: tad_pmu_offline_cpu skips perf_pmu_migrate_context until after
successful perf_pmu_register, so a CPU offline between hotplug add and perf
register does not touch perf core state for an unregistered PMU.
Changelog (since v2)
--------------------
- Validate the eventId using an appropriate mask to ensure
it is restricted to 8 bits.
Changelog (since v1)
--------------------
- Fix config1 filter enable to use bit 9 consistently with the PMU format
string (partid_en) and reject reserved bits with GENMASK(9, 0).
- Register perf_pmu_register before cpuhp_state_add_instance_nocalls and
unregister on hotplug failure.
drivers/perf/marvell_cn10k_tad_pmu.c | 220 +++++++++++++++++++++------
1 file changed, 171 insertions(+), 49 deletions(-)
diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c b/drivers/perf/marvell_cn10k_tad_pmu.c
index 51ccb0befa05..340be3776fe7 100644
--- a/drivers/perf/marvell_cn10k_tad_pmu.c
+++ b/drivers/perf/marvell_cn10k_tad_pmu.c
@@ -7,6 +7,8 @@
#define pr_fmt(fmt) "tad_pmu: " fmt
#include <linux/io.h>
+#include <linux/bits.h>
+#include <linux/compiler.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/cpuhotplug.h>
@@ -14,12 +16,20 @@
#include <linux/platform_device.h>
#include <linux/acpi.h>
-#define TAD_PFC_OFFSET 0x800
-#define TAD_PFC(counter) (TAD_PFC_OFFSET | (counter << 3))
#define TAD_PRF_OFFSET 0x900
-#define TAD_PRF(counter) (TAD_PRF_OFFSET | (counter << 3))
+#define TAD_PFC_OFFSET 0x800
+#define TAD_PFC(base, counter) ((base) | ((u64)(counter) << 3))
+#define TAD_PRF(base, counter) ((base) | ((u64)(counter) << 3))
#define TAD_PRF_CNTSEL_MASK 0xFF
+#define TAD_PRF_MATCH_PARTID BIT(8)
+#define TAD_PRF_PARTID_NS BIT(10)
+/*
+ * config1: bits 0..8 MPAM partition id (including 0); bit 9 requests
+ * filtering for MPAM-capable events. All-zero config1 means no filter.
+ */
+#define TAD_PARTID_FILTER_EN BIT(9)
#define TAD_MAX_COUNTERS 8
+#define TAD_EVENT_SEL_MASK GENMASK(7, 0)
#define to_tad_pmu(p) (container_of(p, struct tad_pmu, pmu))
@@ -27,30 +37,94 @@ struct tad_region {
void __iomem *base;
};
+enum mrvl_tad_pmu_version {
+ TAD_PMU_V1 = 1,
+ TAD_PMU_V2,
+};
+
+struct tad_pmu_data {
+ int id;
+ u64 tad_prf_offset;
+ u64 tad_pfc_offset;
+};
+
struct tad_pmu {
struct pmu pmu;
struct tad_region *regions;
u32 region_cnt;
unsigned int cpu;
+ /* Set after successful perf_pmu_register(); gates offline migration. */
+ bool perf_ready;
+ const struct tad_pmu_ops *ops;
+ const struct tad_pmu_data *pdata;
struct hlist_node node;
struct perf_event *events[TAD_MAX_COUNTERS];
DECLARE_BITMAP(counters_map, TAD_MAX_COUNTERS);
};
-enum mrvl_tad_pmu_version {
- TAD_PMU_V1 = 1,
- TAD_PMU_V2,
-};
-
-struct tad_pmu_data {
- int id;
+struct tad_pmu_ops {
+ void (*start_counter)(struct tad_pmu *pmu, struct perf_event *event);
};
static int tad_pmu_cpuhp_state;
+static void tad_pmu_start_counter(struct tad_pmu *pmu,
+ struct perf_event *event)
+{
+ const struct tad_pmu_data *pdata = pmu->pdata;
+ struct hw_perf_event *hwc = &event->hw;
+ u32 event_idx = (u32)(event->attr.config & TAD_EVENT_SEL_MASK);
+ u32 counter_idx = hwc->idx;
+ u64 partid_filter = 0;
+ u64 reg_val;
+ u64 cfg1 = event->attr.config1;
+ bool use_mpam = cfg1 & TAD_PARTID_FILTER_EN;
+ u32 partid = (u32)(cfg1 & GENMASK(8, 0));
+ int i;
+
+ for (i = 0; i < pmu->region_cnt; i++)
+ writeq_relaxed(0, pmu->regions[i].base +
+ TAD_PFC(pdata->tad_pfc_offset, counter_idx));
+
+ if (use_mpam && event_idx > 0x19 && event_idx < 0x21) {
+ partid_filter = TAD_PRF_MATCH_PARTID | TAD_PRF_PARTID_NS |
+ ((u64)partid << 11);
+ }
+
+
+ for (i = 0; i < pmu->region_cnt; i++) {
+ reg_val = event_idx & 0xFF;
+ reg_val |= partid_filter;
+ writeq_relaxed(reg_val, pmu->regions[i].base +
+ TAD_PRF(pdata->tad_prf_offset, counter_idx));
+ }
+}
+
+static void tad_pmu_v2_start_counter(struct tad_pmu *pmu,
+ struct perf_event *event)
+{
+ const struct tad_pmu_data *pdata = pmu->pdata;
+ struct hw_perf_event *hwc = &event->hw;
+ u32 event_idx = (u32)(event->attr.config & TAD_EVENT_SEL_MASK);
+ u32 counter_idx = hwc->idx;
+ u64 reg_val;
+ int i;
+
+ for (i = 0; i < pmu->region_cnt; i++)
+ writeq_relaxed(0, pmu->regions[i].base +
+ TAD_PFC(pdata->tad_pfc_offset, counter_idx));
+
+ for (i = 0; i < pmu->region_cnt; i++) {
+ reg_val = event_idx & 0xFF;
+ writeq_relaxed(reg_val, pmu->regions[i].base +
+ TAD_PRF(pdata->tad_prf_offset, counter_idx));
+ }
+}
+
static void tad_pmu_event_counter_read(struct perf_event *event)
{
struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
+ const struct tad_pmu_data *pdata = tad_pmu->pdata;
struct hw_perf_event *hwc = &event->hw;
u32 counter_idx = hwc->idx;
u64 prev, new;
@@ -60,7 +134,7 @@ static void tad_pmu_event_counter_read(struct perf_event *event)
prev = local64_read(&hwc->prev_count);
for (i = 0, new = 0; i < tad_pmu->region_cnt; i++)
new += readq(tad_pmu->regions[i].base +
- TAD_PFC(counter_idx));
+ TAD_PFC(pdata->tad_pfc_offset, counter_idx));
} while (local64_cmpxchg(&hwc->prev_count, prev, new) != prev);
local64_add(new - prev, &event->count);
@@ -69,16 +143,14 @@ static void tad_pmu_event_counter_read(struct perf_event *event)
static void tad_pmu_event_counter_stop(struct perf_event *event, int flags)
{
struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
+ const struct tad_pmu_data *pdata = tad_pmu->pdata;
struct hw_perf_event *hwc = &event->hw;
u32 counter_idx = hwc->idx;
int i;
- /* TAD()_PFC() stop counting on the write
- * which sets TAD()_PRF()[CNTSEL] == 0
- */
for (i = 0; i < tad_pmu->region_cnt; i++) {
writeq_relaxed(0, tad_pmu->regions[i].base +
- TAD_PRF(counter_idx));
+ TAD_PRF(pdata->tad_prf_offset, counter_idx));
}
tad_pmu_event_counter_read(event);
@@ -89,26 +161,10 @@ static void tad_pmu_event_counter_start(struct perf_event *event, int flags)
{
struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
struct hw_perf_event *hwc = &event->hw;
- u32 event_idx = event->attr.config;
- u32 counter_idx = hwc->idx;
- u64 reg_val;
- int i;
hwc->state = 0;
- /* Typically TAD_PFC() are zeroed to start counting */
- for (i = 0; i < tad_pmu->region_cnt; i++)
- writeq_relaxed(0, tad_pmu->regions[i].base +
- TAD_PFC(counter_idx));
-
- /* TAD()_PFC() start counting on the write
- * which sets TAD()_PRF()[CNTSEL] != 0
- */
- for (i = 0; i < tad_pmu->region_cnt; i++) {
- reg_val = event_idx & 0xFF;
- writeq_relaxed(reg_val, tad_pmu->regions[i].base +
- TAD_PRF(counter_idx));
- }
+ tad_pmu->ops->start_counter(tad_pmu, event);
}
static void tad_pmu_event_counter_del(struct perf_event *event, int flags)
@@ -128,7 +184,6 @@ static int tad_pmu_event_counter_add(struct perf_event *event, int flags)
struct hw_perf_event *hwc = &event->hw;
int idx;
- /* Get a free counter for this event */
idx = find_first_zero_bit(tad_pmu->counters_map, TAD_MAX_COUNTERS);
if (idx == TAD_MAX_COUNTERS)
return -EAGAIN;
@@ -148,6 +203,9 @@ static int tad_pmu_event_counter_add(struct perf_event *event, int flags)
static int tad_pmu_event_init(struct perf_event *event)
{
struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
+ const struct tad_pmu_data *pdata = tad_pmu->pdata;
+ u32 event_idx = (u32)(event->attr.config & TAD_EVENT_SEL_MASK);
+ u64 cfg1 = event->attr.config1;
if (event->attr.type != event->pmu->type)
return -ENOENT;
@@ -158,6 +216,23 @@ static int tad_pmu_event_init(struct perf_event *event)
if (event->state != PERF_EVENT_STATE_OFF)
return -EINVAL;
+ if (event->attr.config & ~TAD_EVENT_SEL_MASK)
+ return -EINVAL;
+
+ if (pdata->id == TAD_PMU_V2) {
+ if (cfg1)
+ return -EINVAL;
+ } else {
+ if ((cfg1 & GENMASK(8, 0)) && !(cfg1 & TAD_PARTID_FILTER_EN))
+ return -EINVAL;
+ if (cfg1 & TAD_PARTID_FILTER_EN) {
+ if (event_idx <= 0x19 || event_idx >= 0x21)
+ return -EINVAL;
+ }
+ if (cfg1 & ~GENMASK(9, 0))
+ return -EINVAL;
+ }
+
event->cpu = tad_pmu->cpu;
event->hw.idx = -1;
event->hw.config_base = event->attr.config;
@@ -232,7 +307,7 @@ static struct attribute *ody_tad_pmu_event_attrs[] = {
TAD_PMU_EVENT_ATTR(tad_hit_ltg, 0x1e),
TAD_PMU_EVENT_ATTR(tad_hit_any, 0x1f),
TAD_PMU_EVENT_ATTR(tad_tag_rd, 0x20),
- TAD_PMU_EVENT_ATTR(tad_tot_cycle, 0xFF),
+ TAD_PMU_EVENT_ATTR(tad_tot_cycle, 0xff),
NULL
};
@@ -242,9 +317,13 @@ static const struct attribute_group ody_tad_pmu_events_attr_group = {
};
PMU_FORMAT_ATTR(event, "config:0-7");
+PMU_FORMAT_ATTR(partid, "config1:0-8");
+PMU_FORMAT_ATTR(partid_en, "config1:9-9");
static struct attribute *tad_pmu_format_attrs[] = {
&format_attr_event.attr,
+ &format_attr_partid.attr,
+ &format_attr_partid_en.attr,
NULL
};
@@ -253,6 +332,16 @@ static struct attribute_group tad_pmu_format_attr_group = {
.attrs = tad_pmu_format_attrs,
};
+static struct attribute *ody_tad_pmu_format_attrs[] = {
+ &format_attr_event.attr,
+ NULL
+};
+
+static struct attribute_group ody_tad_pmu_format_attr_group = {
+ .name = "format",
+ .attrs = ody_tad_pmu_format_attrs,
+};
+
static ssize_t tad_pmu_cpumask_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -281,16 +370,25 @@ static const struct attribute_group *tad_pmu_attr_groups[] = {
static const struct attribute_group *ody_tad_pmu_attr_groups[] = {
&ody_tad_pmu_events_attr_group,
- &tad_pmu_format_attr_group,
+ &ody_tad_pmu_format_attr_group,
&tad_pmu_cpumask_attr_group,
NULL
};
+static const struct tad_pmu_ops tad_pmu_ops = {
+ .start_counter = tad_pmu_start_counter,
+};
+
+static const struct tad_pmu_ops tad_pmu_v2_ops = {
+ .start_counter = tad_pmu_v2_start_counter,
+};
+
static int tad_pmu_probe(struct platform_device *pdev)
{
const struct tad_pmu_data *dev_data;
struct device *dev = &pdev->dev;
struct tad_region *regions;
+ resource_size_t map_start;
struct tad_pmu *tad_pmu;
struct resource *res;
u32 tad_pmu_page_size;
@@ -298,7 +396,6 @@ static int tad_pmu_probe(struct platform_device *pdev)
u32 tad_cnt;
int version;
int i, ret;
- char *name;
tad_pmu = devm_kzalloc(&pdev->dev, sizeof(*tad_pmu), GFP_KERNEL);
if (!tad_pmu)
@@ -312,6 +409,7 @@ static int tad_pmu_probe(struct platform_device *pdev)
return -ENODEV;
}
version = dev_data->id;
+ tad_pmu->pdata = dev_data;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -338,22 +436,31 @@ static int tad_pmu_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "Can't find tad-cnt property\n");
return ret;
}
+ if (!tad_cnt || !tad_page_size || !tad_pmu_page_size) {
+ dev_err(&pdev->dev, "Invalid tad-cnt or page size\n");
+ return -EINVAL;
+ }
regions = devm_kcalloc(&pdev->dev, tad_cnt,
sizeof(*regions), GFP_KERNEL);
if (!regions)
return -ENOMEM;
- /* ioremap the distributed TAD pmu regions */
- for (i = 0; i < tad_cnt && res->start < res->end; i++) {
- regions[i].base = devm_ioremap(&pdev->dev,
- res->start,
+ map_start = res->start;
+ for (i = 0; i < tad_cnt; i++) {
+ if (map_start > res->end ||
+ tad_pmu_page_size > (resource_size_t)(res->end - map_start + 1)) {
+ dev_err(&pdev->dev, "TAD PMU mem window too small for tad-cnt=%u\n",
+ tad_cnt);
+ return -EINVAL;
+ }
+ regions[i].base = devm_ioremap(&pdev->dev, map_start,
tad_pmu_page_size);
if (!regions[i].base) {
dev_err(&pdev->dev, "TAD%d ioremap fail\n", i);
return -ENOMEM;
}
- res->start += tad_page_size;
+ map_start += tad_page_size;
}
tad_pmu->regions = regions;
@@ -374,14 +481,16 @@ static int tad_pmu_probe(struct platform_device *pdev)
.read = tad_pmu_event_counter_read,
};
- if (version == TAD_PMU_V1)
+ if (version == TAD_PMU_V1) {
tad_pmu->pmu.attr_groups = tad_pmu_attr_groups;
- else
+ tad_pmu->ops = &tad_pmu_ops;
+ } else {
tad_pmu->pmu.attr_groups = ody_tad_pmu_attr_groups;
+ tad_pmu->ops = &tad_pmu_v2_ops;
+ }
tad_pmu->cpu = raw_smp_processor_id();
- /* Register pmu instance for cpu hotplug */
ret = cpuhp_state_add_instance_nocalls(tad_pmu_cpuhp_state,
&tad_pmu->node);
if (ret) {
@@ -389,19 +498,24 @@ static int tad_pmu_probe(struct platform_device *pdev)
return ret;
}
- name = "tad";
- ret = perf_pmu_register(&tad_pmu->pmu, name, -1);
- if (ret)
+ ret = perf_pmu_register(&tad_pmu->pmu, "tad", -1);
+ if (ret) {
+ dev_err(&pdev->dev, "Error %d registering perf PMU\n", ret);
cpuhp_state_remove_instance_nocalls(tad_pmu_cpuhp_state,
&tad_pmu->node);
+ return ret;
+ }
- return ret;
+ WRITE_ONCE(tad_pmu->perf_ready, true);
+
+ return 0;
}
static void tad_pmu_remove(struct platform_device *pdev)
{
struct tad_pmu *pmu = platform_get_drvdata(pdev);
+ WRITE_ONCE(pmu->perf_ready, false);
cpuhp_state_remove_instance_nocalls(tad_pmu_cpuhp_state,
&pmu->node);
perf_pmu_unregister(&pmu->pmu);
@@ -410,12 +524,17 @@ static void tad_pmu_remove(struct platform_device *pdev)
#if defined(CONFIG_OF) || defined(CONFIG_ACPI)
static const struct tad_pmu_data tad_pmu_data = {
.id = TAD_PMU_V1,
+ .tad_prf_offset = TAD_PRF_OFFSET,
+ .tad_pfc_offset = TAD_PFC_OFFSET,
};
+
#endif
#ifdef CONFIG_ACPI
static const struct tad_pmu_data tad_pmu_v2_data = {
.id = TAD_PMU_V2,
+ .tad_prf_offset = TAD_PRF_OFFSET,
+ .tad_pfc_offset = TAD_PFC_OFFSET,
};
#endif
@@ -451,6 +570,9 @@ static int tad_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
struct tad_pmu *pmu = hlist_entry_safe(node, struct tad_pmu, node);
unsigned int target;
+ if (!READ_ONCE(pmu->perf_ready))
+ return 0;
+
if (cpu != pmu->cpu)
return 0;
@@ -491,6 +613,6 @@ static void __exit tad_pmu_exit(void)
module_init(tad_pmu_init);
module_exit(tad_pmu_exit);
-MODULE_DESCRIPTION("Marvell CN10K LLC-TAD Perf driver");
+MODULE_DESCRIPTION("Marvell CN10K LLC-TAD perf driver");
MODULE_AUTHOR("Bhaskara Budiredla <bbudiredla@marvell.com>");
MODULE_LICENSE("GPL v2");
--
2.25.1
^ permalink raw reply related
* [PATCH v4 2/3] perf: marvell: Add CN20K LLC-TAD PMU support
From: Geetha sowjanya @ 2026-06-18 15:36 UTC (permalink / raw)
To: linux-perf-users, linux-kernel, linux-arm-kernel, devicetree
Cc: mark.rutland, will, krzk+dt, gakula
In-Reply-To: <20260618153610.13649-1-gakula@marvell.com>
Add support for the LLC Tag-and-Data (TAD) PMU present in
Marvell CN20K SoCs.
The CN20K TAD PMU is based on the CN10K design but differs in the
layout of PFC/PRF register offsets relative to each TAD base, and
introduces additional events. These offsets are selected by the driver
based on the compatible string and are not described via DT properties.
Because of this, "marvell,cn10k-tad-pmu" cannot be used as a fallback
for CN20K, as it would result in incorrect register programming.
Add support for "marvell,cn20k-tad-pmu" by:
- Introducing a TAD_PMU_V3 profile with CN20K-specific register bases
- Extending the event map for new CN20K events
- Matching the PMU via OF and ACPI (MRVL000F)
Signed-off-by: Geetha sowjanya <gakula@marvell.com>
---
Changelog (since v2)
--------------------
- Validate the eventId using an appropriate mask to ensure
it is restricted to 8 bits.
Changelog (since v1)
--------------------
- Hide V3-only events on CN10K via sysfs is_visible and reject them in
event_init.
- Use CN20K-specific MPAM PRF bits (MATCH_MPAMNS, partid << 10) for V3;
software partid is limited to nine bits so this does not collide with
the fixed bit at 25.
- Reset hwc->prev_count when starting counters so reads match cleared HW.
drivers/perf/marvell_cn10k_tad_pmu.c | 54 ++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c b/drivers/perf/marvell_cn10k_tad_pmu.c
index 340be3776fe7..b73ee2f58fd4 100644
--- a/drivers/perf/marvell_cn10k_tad_pmu.c
+++ b/drivers/perf/marvell_cn10k_tad_pmu.c
@@ -18,11 +18,14 @@
#define TAD_PRF_OFFSET 0x900
#define TAD_PFC_OFFSET 0x800
+#define TAD_PRF_NS_OFFSET 0x30900
+#define TAD_PFC_NS_OFFSET 0x30800
#define TAD_PFC(base, counter) ((base) | ((u64)(counter) << 3))
#define TAD_PRF(base, counter) ((base) | ((u64)(counter) << 3))
#define TAD_PRF_CNTSEL_MASK 0xFF
#define TAD_PRF_MATCH_PARTID BIT(8)
#define TAD_PRF_PARTID_NS BIT(10)
+#define TAD_PRF_MATCH_MPAMNS BIT(25)
/*
* config1: bits 0..8 MPAM partition id (including 0); bit 9 requests
* filtering for MPAM-capable events. All-zero config1 means no filter.
@@ -40,6 +43,7 @@ struct tad_region {
enum mrvl_tad_pmu_version {
TAD_PMU_V1 = 1,
TAD_PMU_V2,
+ TAD_PMU_V3,
};
struct tad_pmu_data {
@@ -89,8 +93,15 @@ static void tad_pmu_start_counter(struct tad_pmu *pmu,
if (use_mpam && event_idx > 0x19 && event_idx < 0x21) {
partid_filter = TAD_PRF_MATCH_PARTID | TAD_PRF_PARTID_NS |
((u64)partid << 11);
+
+ if (pdata->id == TAD_PMU_V3)
+ partid_filter = TAD_PRF_MATCH_PARTID | TAD_PRF_MATCH_MPAMNS |
+ ((u64)partid << 10);
}
+ /* CN10K support events 0:24*/
+ if (pdata->id == TAD_PMU_V1 && event_idx >= 0x25)
+ return;
for (i = 0; i < pmu->region_cnt; i++) {
reg_val = event_idx & 0xFF;
@@ -163,6 +174,7 @@ static void tad_pmu_event_counter_start(struct perf_event *event, int flags)
struct hw_perf_event *hwc = &event->hw;
hwc->state = 0;
+ local64_set(&hwc->prev_count, 0);
tad_pmu->ops->start_counter(tad_pmu, event);
}
@@ -223,6 +235,8 @@ static int tad_pmu_event_init(struct perf_event *event)
if (cfg1)
return -EINVAL;
} else {
+ if (pdata->id == TAD_PMU_V1 && event_idx >= 0x25)
+ return -EINVAL;
if ((cfg1 & GENMASK(8, 0)) && !(cfg1 & TAD_PARTID_FILTER_EN))
return -EINVAL;
if (cfg1 & TAD_PARTID_FILTER_EN) {
@@ -249,6 +263,22 @@ static ssize_t tad_pmu_event_show(struct device *dev,
return sysfs_emit(page, "event=0x%02llx\n", pmu_attr->id);
}
+static umode_t tad_pmu_event_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int unused)
+{
+ struct pmu *pmu = dev_get_drvdata(kobj_to_dev(kobj));
+ struct tad_pmu *t = to_tad_pmu(pmu);
+ struct device_attribute *da = container_of(attr, struct device_attribute,
+ attr);
+ struct perf_pmu_events_attr *e = container_of(da, struct perf_pmu_events_attr,
+ attr);
+ u64 id = e->id;
+
+ if (t->pdata->id != TAD_PMU_V3 && id >= 0x25)
+ return 0;
+ return attr->mode;
+}
+
#define TAD_PMU_EVENT_ATTR(name, config) \
PMU_EVENT_ATTR_ID(name, tad_pmu_event_show, config)
@@ -290,12 +320,25 @@ static struct attribute *tad_pmu_event_attrs[] = {
TAD_PMU_EVENT_ATTR(tad_dat_rd_byp, 0x22),
TAD_PMU_EVENT_ATTR(tad_ifb_occ, 0x23),
TAD_PMU_EVENT_ATTR(tad_req_occ, 0x24),
+ TAD_PMU_EVENT_ATTR(tad_req_msh_out_dtg_evict, 0x25),
+ TAD_PMU_EVENT_ATTR(tad_req_msh_out_ltg_evict, 0x26),
+ TAD_PMU_EVENT_ATTR(tad_rsp_msh_out_mpam, 0x28),
+ TAD_PMU_EVENT_ATTR(tad_replays, 0x29),
+ TAD_PMU_EVENT_ATTR(tad_req_byp0, 0x2a),
+ TAD_PMU_EVENT_ATTR(tad_req_byp1, 0x2b),
+ TAD_PMU_EVENT_ATTR(tad_txreq_byp, 0x2c),
+ TAD_PMU_EVENT_ATTR(tad_time_in_dslp, 0x2d),
+ TAD_PMU_EVENT_ATTR(tad_time_elapsed, 0x2e),
+ TAD_PMU_EVENT_ATTR(tad_req_msh_out_dss_rd_128mrg, 0x2f),
+ TAD_PMU_EVENT_ATTR(tad_req_msh_out_dss_wr_128mrg, 0x30),
+ TAD_PMU_EVENT_ATTR(tad_tot_cycle, 0xff),
NULL
};
static const struct attribute_group tad_pmu_events_attr_group = {
.name = "events",
.attrs = tad_pmu_event_attrs,
+ .is_visible = tad_pmu_event_attr_is_visible,
};
static struct attribute *ody_tad_pmu_event_attrs[] = {
@@ -481,7 +524,7 @@ static int tad_pmu_probe(struct platform_device *pdev)
.read = tad_pmu_event_counter_read,
};
- if (version == TAD_PMU_V1) {
+ if (version == TAD_PMU_V1 || version == TAD_PMU_V3) {
tad_pmu->pmu.attr_groups = tad_pmu_attr_groups;
tad_pmu->ops = &tad_pmu_ops;
} else {
@@ -528,6 +571,11 @@ static const struct tad_pmu_data tad_pmu_data = {
.tad_pfc_offset = TAD_PFC_OFFSET,
};
+static const struct tad_pmu_data tad_pmu_cn20k_data = {
+ .id = TAD_PMU_V3,
+ .tad_prf_offset = TAD_PRF_NS_OFFSET,
+ .tad_pfc_offset = TAD_PFC_NS_OFFSET,
+};
#endif
#ifdef CONFIG_ACPI
@@ -541,6 +589,7 @@ static const struct tad_pmu_data tad_pmu_v2_data = {
#ifdef CONFIG_OF
static const struct of_device_id tad_pmu_of_match[] = {
{ .compatible = "marvell,cn10k-tad-pmu", .data = &tad_pmu_data },
+ { .compatible = "marvell,cn20k-tad-pmu", .data = &tad_pmu_cn20k_data },
{},
};
#endif
@@ -549,6 +598,7 @@ static const struct of_device_id tad_pmu_of_match[] = {
static const struct acpi_device_id tad_pmu_acpi_match[] = {
{"MRVL000B", (kernel_ulong_t)&tad_pmu_data},
{"MRVL000D", (kernel_ulong_t)&tad_pmu_v2_data},
+ {"MRVL000F", (kernel_ulong_t)&tad_pmu_cn20k_data},
{},
};
MODULE_DEVICE_TABLE(acpi, tad_pmu_acpi_match);
@@ -613,6 +663,6 @@ static void __exit tad_pmu_exit(void)
module_init(tad_pmu_init);
module_exit(tad_pmu_exit);
-MODULE_DESCRIPTION("Marvell CN10K LLC-TAD perf driver");
+MODULE_DESCRIPTION("Marvell CN10K/CN20K LLC-TAD perf driver");
MODULE_AUTHOR("Bhaskara Budiredla <bbudiredla@marvell.com>");
MODULE_LICENSE("GPL v2");
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 3/6] drm/tiny: Add DRM driver for Solomon SSD16xx e-paper display controllers
From: Devarsh Thakkar @ 2026-06-18 15:41 UTC (permalink / raw)
To: Thomas Zimmermann, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Bjorn Andersson, dri-devel, devicetree,
linux-kernel
Cc: praneeth, vigneshr, s-jain1, r-donadkar, r-sharma3, afd, Sen Wang,
LiangCheng Wang, Aldea, Andrei, Judith Mendez, D, Yashas
In-Reply-To: <182a7cd1-09b0-4ed2-bcb7-0924adb30375@ti.com>
Hi,
On 17/06/26 17:17, Devarsh Thakkar wrote:
> Hi Thomas,
>
> On 08/05/26 21:42, Devarsh Thakkar wrote:
>> Hi Thomas,
>>
>> Thanks for the quick review.
>>
>> On 05/05/26 12:35, Thomas Zimmermann wrote:
>>> Hi,
>>>
>>> thanks for the driver. See below for a review.
>>>
>>> There is support for Solomon SSD13xx in solomon/. You should check if
>>> the new driver belongs there.
>>>
>
>
> Just wanted to follow up on my response [0] to to your v1 review where I
> had addressed your comments/questions inline in the thread and had a
> couple of questions as I am preparing a V2 patch for the series which I
> am planning to send soon.
>
> Would really appreciate your thoughts when you get a chance.
>
>
>
> [0]: https://lore.kernel.org/all/eccf407a-c469-4744-a56f-
> aa7366c58be3@ti.com/
>
<snip>
>>>> --- a/drivers/gpu/drm/tiny/Kconfig
>>>> +++ b/drivers/gpu/drm/tiny/Kconfig
>>>> @@ -215,3 +215,16 @@ config TINYDRM_SHARP_MEMORY
>>>> * 4.40" Sharp Memory LCD (LS044Q7DH01)
>>>> If M is selected the module will be called sharp_memory.
>>>> +
>>>> +config DRM_PANEL_SSD16XX
>>>
>>> Just call it DRM_SSD16XX without the panel. In DRM, things named
>>> 'panel' are usually built around struct drm_panel, which doesn't seem
>>> the case here.
>>>
>>
I see drivers/gpu/drm/tiny/panel-mipi-dbi.c [0] using
CONFIG_DRM_PANEL_MIPI_DBI [1] in the same directory, and that driver
does not use struct drm_panel either - the only drm_panel reference
there is a call to of_get_drm_panel_display_mode(), which is a DT
display-mode helper unrelated to the panel framework.
Given this existing precedent in drm/tiny/ itself and also for the
reasons explained below as this driver houses both controller specific
and panel specific logic, I would prefer to keep DRM_PANEL_SSD16XX.
>> Oh ok, I preferred DRM_PANEL_SSD16XX since it also enumerates and uses
>> panel specific data/compatible such as this driver supporting
>> gooddisplay,gdey042t81 and more can be added too (just like panel-
>> ilitek* for e.g.) unlike controller only drivers which need to be
>> linked to separate panel drivers.
>>
>> Do you prefer to change it to DRM_SSD16XX_PANEL to not conflict with
>> DRM_PANEL* drivers and for better context or still prefer to keep it
>> as DRM_SSD16XX ?
>>
>>>> +
<snip>
>>>> diff --git a/drivers/gpu/drm/tiny/panel-ssd16xx.c b/drivers/gpu/drm/
>>>> tiny/panel-ssd16xx.c
>>>> new file mode 100644
>>>> inde`x 000000000000..b232837c54ff
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/tiny/panel-ssd16xx.c
>>>
>>> Again, remove 'panel'.
>>
Also for the naming too, I'd prefer to keep panel-ssd16xx.c similar to
panel-mipi-dbi.c [0] for the reasons mentioned below.
>> Yes I can remove the panel, but I am just concerned if it won't
>> mislead folks to understand ssd16xx as a controller only driver,
>> requiring a separate panel driver to interface with ?
>>
>> Basically panel-ssd16xx naming was chosen since this driver houses
>> both the ssd16xx controller context and also the panel being used
>> along with that (similar to panel-ilitek-ili9881c.c) and i did not
>> want to confuse it with a controller only driver (similar to
>> tc358775.c), if it is overalapping a known pattern reserved for
>> drm_panel drivers do you think we should rename it to ssd16xx-panel.c
>> instead or you prefer ssd16xx.c as more appropriate one ?
>>
Kindly let me know if it sounds okay.
[0] :
https://gitlab.com/linux-kernel/linux-next/-/blob/next-20260618/drivers/gpu/drm/tiny/panel-mipi-dbi.c?ref_type=tags
[1] :
https://gitlab.com/linux-kernel/linux-next/-/blob/next-20260618/drivers/gpu/drm/tiny/Makefile?ref_type=tags#L8
Regards
Devarsh
>>>
>>>> @@ -0,0 +1,2548 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * DRM driver for e-paper display panels using Solomon SSD16xx
>>>> family controllers
>>>> + *
>>>> + * Copyright (C) 2026 Texas Instruments Incorporated - https://
>>>> www.ti.com/
>>>> + *
>>>> + * Author: Devarsh Thakkar <devarsht@ti.com>
>>>> + *
>>>> + * References: https://github.com/Lesords/epaper
>>>> + */
>>>> +
>>>> +#include <linux/delay.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/property.h>
>>>> +#include <linux/spi/spi.h>
>>>> +
>>>> +#include <drm/clients/drm_client_setup.h>
>>>> +#include <drm/drm_atomic.h>
>>>> +#include <drm/drm_atomic_helper.h>
>>>> +#include <drm/drm_damage_helper.h>
>>>> +#include <drm/drm_drv.h>
>>>> +#include <drm/drm_fb_helper.h>
>>>> +#include <drm/drm_fbdev_dma.h>
>>>> +#include <drm/drm_fb_dma_helper.h>
>>>> +#include <drm/drm_framebuffer.h>
>>>> +#include <drm/drm_gem_dma_helper.h>
>>>> +#include <drm/drm_gem_framebuffer_helper.h>
>>>> +#include <drm/drm_probe_helper.h>
>>>
>>>> +#include <drm/drm_simple_kms_helper.h>
>>>
>>> Obsolete. Anything you use from this header should be open-coded in
>>> the driver.
>>>
>>
>> Agreed, will remove it in V2.
>>
>>>> +#include <drm/drm_print.h>
>>>
>>>
>>> Please remove all of the parameters below. They might be nice for
>>> your debugging, but they do not belong in the upstream driver.
>>>
>>
>> As mentioned previously, had kept these params mainly for legacy non-
>> drm fbdev based applications.
>>
>>>> +
>>>> +static int rotation = -1;
>>>> +module_param(rotation, int, 0644);
>>>> +MODULE_PARM_DESC(rotation,
>>>> + "Display rotation (-1=use DT, 0/180=landscape,
>>>> 90/270=portrait)");
>>>
>>> Please remove this. There is a rotation property in struct
>>> drm_connector, which stores the rotation. IIRC it can be overridden
>>> on the kernel command line.
>>>
>>
>> As I understand you are referring to below fields from drm_connector
>> struct, please correct me if I am wrong here but I think the rotation/
>> orientation functionality supported by ssd16xx controller does not
>> match much with below model but instead matches what is done in
>> drivers/gpu/ drm/drm_mipi_dbi.c (although that does not support
>> runtime rotation) as explained below :
>>
>> drm_connector (rotation specific members):
>>
>>
>> 1. panel_orientation (display_info.panel_orientation):
>> Readable from DT via of_drm_get_panel_orientation(), overridable from
>> cmdline. However it is not writable by userspace at runtime (which we
>> require). More importantly, when Weston reads panel_orientation it
>> applies an output transform and then attempts to offload rotation to
>> the plane via plane.rotation. This model assumes the plane can
>> geometrically map a 300x400 source framebuffer to a 400x300 CRTC i.e.
>> hardware scan- out rotation. Our driver has no such hardware as
>> explained in detail below.
>>
>>
>> 2. rotation_reflection (cmdline_mode.rotation_reflection):
>> Cmdline-only (video=...:rotate=N), no DT path (we require both DT-path
>> and runtime suport). Also I think this is strictly for in-kernel
>> drm_clients and also It currently returns false for 90/270 unless
>> the plane has a hardware rotation property.
>>
>>
>> Both paths therefore ultimately require hardware plane rotation that
>> this driver does not have and both seem to be supported just
>> statically i.e. cmdline or dt property.
>>
>> Our use-case needs to support runtime rotation configuration ours is
>> not a mounted display but a portable hand-held device (https://
>> www.beagleboard.org/boards/beaglebadge) and we have an accelerometer
>> in our device which can detect panel orientation and based on
>> accelerometer reading the drm app can runtime set the custom drm
>> rotation property to switch to new orientation dynamically.
>>
>> Also our driver is fundamentally different from a GPU display pipeline
>> or controllers supporting transpose function. The SSD16xx display
>> controller has no transpose or rotation function but instead supports
>> different scan-modes, so there is no hardware path that can take a
>> 400x300 plane and transpose it to a 300x400 display output. The
>> controller is a simple RAM writer: the CPU writes a byte stream over
>> SPI, and the controller's internal cursor
>> advances sequentially according to the data entry mode register (command
>> 0x11), which selects between X++/Y++ and X--/Y-- scan directions with a
>> configurable start position.
>>
>>
>> For portrait orientation we therefore change the DRM mode itself to
>> 300x400 from the original 400x300, so the application is asked to
>> provide a 300x400 framebuffer.
>> The driver then writes this buffer column-by-column over SPI to the
>> display controller's RAM. Since the controller supports different scan
>> start positions (cursor at origin vs cursor at maximum address) combined
>> with the appropriate X/Y scan direction, we are able to correctly render
>> the 300x400 buffer onto the panel when it is held in portrait
>> orientation (90*, 270*).
>>
>>
>> This means the CRTC mode must reflect the logical dimensions directly,
>> exactly as drm_mipi_dbi_dev_init() does via mipi_dbi_rotate_mode() for
>> MIPI DBI drivers. Accepting a 300x400 framebuffer onto a 400x300 CRTC
>> (as the panel_orientation + plane.rotation model requires) is not
>> possible: drm_atomic_helper_check_plane_state(DRM_PLANE_NO_SCALING)
>> enforces src_w == crtc_w and src_h == crtc_h, and there is no hardware
>> to perform the geometric remapping between the two sizes.
>>
>>
>> For runtime rotation changes (which are required as the panel is not
>> physically fixed), we therefor wanted to use a custom drm connector
>> property. We can look to use the standard DRM_MODE_ROTATE_* bitmask
>> (not a custom enum, that was used in v1), we can also look to check if
>> driver can triggers a full modeset through the normal DRM path,
>> connector_get_modes returns the correctly dimensioned mode for the new
>> orientation, and userspace receives a mode-changed event with the new
>> dimensions.
>>
>>
>> This is semantically what MIPI DBI tiny drivers do at boot (fixed from
>> DT), made runtime-changeable via the custom drm connector property in
>> this driver.
>>
>> Maybe, I can try to use standard bitmask instead of custom enum to re-
>> use standard macros :
>>
>> drm_property_create_bitmask(drm, 0, "rotation",
>> rotation_props,
>> ARRAY_SIZE(rotation_props),
>> DRM_MODE_ROTATE_0 DRM_MODE_ROTATE_90 |
>> DRM_MODE_ROTATE_180 |DRM_MODE_ROTATE_270);
>>
>> but keep it as connector property?
>>
>>>> +
>>>> +static int refresh_mode = -1;
>>>> +module_param(refresh_mode, int, 0644);
>>>> +MODULE_PARM_DESC(refresh_mode,
>>>> + "Refresh mode (-1=panel default, 0=partial ~300-500ms,
>>>> 1=full ~1.5-2s, 2=fast ~1.0-1.5s)");
>>>> +
>>>> +static int border_waveform_init_lut = -1;
>>>> +module_param(border_waveform_init_lut, int, 0644);
>>>> +MODULE_PARM_DESC(border_waveform_init_lut,
>>>> + "Border waveform index during clear/init (-1=panel
>>>> default, 0-9=enum index)");
>>>> +
>>>> +static int border_waveform_lut = -1;
>>>> +module_param(border_waveform_lut, int, 0644);
>>>> +MODULE_PARM_DESC(border_waveform_lut,
>>>> + "Border waveform index during display updates (-1=panel
>>>> default, 0-9=enum index)");
>>>> +
>>>
>>> Please remove it. Only the panel default. If you have panels where
>>> the default is known to be incorrect, you can add specific
>>> workarounds in the driver.
>>>
>>
>> I think the most of these params are kept to sane defaults but they
>> may change w.r.t use-cases and each panel can be used in context of
>> multiple use-cases.
>>
>>>
>>>> +static bool border_refresh_on_every_update;
>>>> +module_param(border_refresh_on_every_update, bool, 0644);
>>>> +MODULE_PARM_DESC(border_refresh_on_every_update,
>>>> + "Re-send border waveform command before each display
>>>> update (default: false)");
>>>
>>> Pick a sane default.
>>>
>>
>> Yes driver is picking a sane default already for this (refresh border
>> on init once with white border and keep it as floating in later
>> updates), but just a back-door for the application in case it wants to
>> avoid ghosting totally altogether or has specific needs w.r.t border
>> handling.
>>
>>>> +
>>>> +static int clear_on_init = -1;
>>>> +module_param(clear_on_init, int, 0644);
>>>> +MODULE_PARM_DESC(clear_on_init,
>>>> + "Clear display on first app launch (-1=disabled,
>>>> 0=partial, 1=full, 2=fast)");
>>>> +
>>>> +static int clear_on_close = -1;
>>>> +module_param(clear_on_close, int, 0644);
>>>> +MODULE_PARM_DESC(clear_on_close,
>>>> + "Clear display on app close/CRTC disable (-1=disabled,
>>>> 0=partial, 1=full, 2=fast)");
>>>> +
>>>> +static int clear_on_disable = -1;
>>>> +module_param(clear_on_disable, int, 0644);
>>>> +MODULE_PARM_DESC(clear_on_disable,
>>>> + "Clear display on CRTC disable/DPMS off (-1=disabled,
>>>> 0=partial, 1=full, 2=fast)");
>>>> +
>>>> +static int refresh_mode_init = -1;
>>>> +module_param(refresh_mode_init, int, 0644);
>>>> +MODULE_PARM_DESC(refresh_mode_init,
>>>> + "Skip baseline establishment on first enable (-1=disabled,
>>>> 0=partial, 1=full, 2=fast)");
>>>
>>> Use 'disabled' for all of them.
>>>
>>>> +
>>>> +static int color_mode = -1;
>>>> +module_param(color_mode, int, 0644);
>>>> +MODULE_PARM_DESC(color_mode,
>>>> + "Color mode (-1=panel default, 0=black-white, 1=3-color;
>>>> 3- color only valid for panels with red plane support)");
>>>
>>> 'Panel default.' Colors should be controlled by DRM clients via the
>>> framebuffer.
>>>
>>
>> As mentioned previously, say user-space is only supporting and giving
>> XR24 or XR32 format, from that we can't infer whether user-space want
>> to drive display in B/W mode or color-mode.
>>
>>>> +
>>>> +/*
>>>> -----------------------------------------------------------------------
>>>> + * SSD16xx family common: commands, data values, and bit definitions.
>>>> + * These apply equally to SSD1673, SSD1680, and SSD1683.
>>>> + *
>>>> -----------------------------------------------------------------------
>>>> + */
>>>> +
>>>> +/* SPI command codes (common) */
>>>> +#define SSD16XX_CMD_DRIVER_OUTPUT_CONTROL 0x01
>>>> +#define SSD16XX_CMD_DATA_ENTRY_MODE 0x11
>>>> +#define SSD16XX_CMD_SW_RESET 0x12
>>>> +#define SSD16XX_CMD_MASTER_ACTIVATION 0x20
>>>> +#define SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1 0x21
>>>> +#define SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2 0x22
>>>> +#define SSD16XX_CMD_WRITE_RAM_BW 0x24
>>>> +#define SSD16XX_CMD_BORDER_WAVEFORM_CONTROL 0x3C
>>>> +#define SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END 0x44
>>>> +#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END 0x45
>>>> +#define SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER 0x4E
>>>> +#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER 0x4F
>>>> +
>>>> +/*
>>>> + * Data Entry Mode (command 0x11) AM/IDY/IDX bit encoding (common).
>>>> + *
>>>> + * Bit 2 (AM): Address update direction: 0 = X direction, 1 = Y
>>>> direction
>>>> + * ID[1:0] when AM=0 (X-direction modes, address counter advances
>>>> in X):
>>>> + * 00 = X decrement, Y decrement 01 = X increment, Y decrement
>>>> + * 10 = X decrement, Y increment 11 = X increment, Y increment
>>>> (default)
>>>> + *
>>>> + * Rotation to data entry mode mapping (actual implementation uses
>>>> two modes,
>>>> + * with scan direction controlled via RAM cursor positioning and
>>>> manual tweaking):
>>>> + * 0°/270° → 0x03 (X++, Y++) Landscape/Portrait-CW: cursor at
>>>> (0, 0)
>>>> + * 90°/180° → 0x00 (X--, Y--) Portrait-CCW/Upside-down: cursor
>>>> at (max, max)
>>>> + *
>>>> + * The pixel packing in convert_fb_to_1bpp is grouped by physical
>>>> layout:
>>>> + * - Portrait (90°/270°): column-major packing, rightmost column
>>>> first
>>>> + * - Landscape (0°/180°): row-major packing, top to bottom, left
>>>> to right
>>>> + * Hardware cursor position and scan mode handle the final
>>>> orientation.
>>>> + */
>>>> +#define SSD16XX_DATA_ENTRY_XDEC_YDEC 0x00 /* X--, Y-- (X-
>>>> mode) */
>>>> +#define SSD16XX_DATA_ENTRY_XINC_YINC 0x03 /* X++, Y++ (X-
>>>> mode, default) */
>>>> +
>>>> +/* POR reset value: GD=0 (G0 first), SM=0 (interlaced), TB=0 (G0-
>>>> >G299) */
>>>> +#define SSD16XX_DRIVER_OUTPUT_CTRL_DEFAULT 0x00
>>>> +
>>>> +/* Display Update Control 1 (0x21) byte 2 default (common) */
>>>> +#define SSD16XX_CTRL1_BYTE2_DEFAULT 0x00
>>>> +
>>>> +/*
>>>> + * Display Update Control 2 (0x22) individual bit definitions
>>>> (common).
>>>> + * NOTE: BIT(3) is NOT common — see SSD1683_CTRL2_MODE2 in the SSD1683
>>>> + * section below; it has a completely different meaning in SSD1673.
>>>> + */
>>>> +#define SSD16XX_CTRL2_ENABLE_CLK BIT(7)
>>>> +#define SSD16XX_CTRL2_ENABLE_ANALOG BIT(6)
>>>> +#define SSD16XX_CTRL2_LOAD_TEMPERATURE BIT(5)
>>>> +#define SSD16XX_CTRL2_LOAD_LUT BIT(4)
>>>> +#define SSD16XX_CTRL2_DISPLAY BIT(2)
>>>> +#define SSD16XX_CTRL2_DISABLE_ANALOG BIT(1)
>>>> +#define SSD16XX_CTRL2_DISABLE_CLK BIT(0)
>>>> +
>>>> +#define SSD16XX_SPI_BITS_PER_WORD 8
>>>> +#define SSD16XX_SPI_SPEED_DEFAULT 1000000
>>>> +
>>>> +/* Maximum time to wait for the BUSY pin to deassert after a
>>>> display update */
>>>> +#define SSD16XX_BUSY_WAIT_TIMEOUT_MS 6000
>>>> +
>>>> +/*
>>>> -----------------------------------------------------------------------
>>>> + * SSD1683 / SSD1680 specific: commands, data values, and bit
>>>> definitions.
>>>> + *
>>>> -----------------------------------------------------------------------
>>>> + */
>>>> +
>>>> +/*
>>>> + * Deep Sleep Mode values (command 0x10).
>>>> + */
>>>> +#define SSD1683_DEEP_SLEEP_MODE_1 0x01 /* RAM retained */
>>>> +#define SSD1683_DEEP_SLEEP_MODE_2 0x03 /* RAM lost (max
>>>> power) */
>>>> +
>>>> +/*
>>>> + * Temperature Sensor Selection (command 0x18).
>>>> + */
>>>> +#define SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL 0x18
>>>> +#define SSD1683_TEMP_SENSOR_INTERNAL 0x80 /* Bit 7: use
>>>> internal sensor */
>>>> +
>>>> +/*
>>>> + * Write RED RAM (command 0x26).
>>>> + */
>>>> +#define SSD1683_CMD_WRITE_RAM_RED 0x26
>>>> +
>>>> +/*
>>>> + * Border Waveform Control (command 0x3C) byte values.
>>>> + */
>>>> +#define SSD1683_BORDER_WAVEFORM_LUT0 0x00 /* GS Transition
>>>> LUT0 (black) */
>>>> +#define SSD1683_BORDER_WAVEFORM_LUT1 0x01 /* GS Transition
>>>> LUT1 (white) */
>>>> +#define SSD1683_BORDER_WAVEFORM_LUT2 0x02 /* GS Transition
>>>> LUT2 (black) */
>>>> +#define SSD1683_BORDER_WAVEFORM_LUT3 0x03 /* GS Transition
>>>> LUT3 (gray) */
>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSS 0x40 /* Fix Level
>>>> VSS (0V, black) */
>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSH1 0x50 /* Fix Level
>>>> VSH1 (+15V, black) */
>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSL 0x60 /* Fix Level
>>>> VSL (-15V, white) */
>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSH2 0x70 /* Fix Level
>>>> VSH2 (+15V alt, black) */
>>>> +#define SSD1683_BORDER_WAVEFORM_VCOM 0x80 /* Follow VCOM
>>>> (-2V~-3V, preserve) */
>>>> +#define SSD1683_BORDER_WAVEFORM_HIZ 0xC0 /* HiZ (floating,
>>>> default) */
>>>> +
>>>> +/*
>>>> + * Display Update Control 1 (0x21) byte 1 — RED RAM control.
>>>> + */
>>>> +#define SSD1683_CTRL1_NORMAL 0x00 /* Both BW and RED
>>>> RAMs enabled */
>>>> +#define SSD1683_CTRL1_BYPASS_RED_RAM 0x40 /* Bypass RED RAM
>>>> (force RED=0) */
>>>> +
>>>> +/*
>>>> + * Display Update Control 2 (0x22) BIT(3) — "Display Mode
>>>> 2" (partial/BW).
>>>> + */
>>>> +#define SSD1683_CTRL2_MODE2 BIT(3)
>>>> +
>>>> +/* Composite CTRL2 sequences for each refresh mode */
>>>> +#define SSD1683_CTRL2_FULL_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
>>>> + SSD16XX_CTRL2_ENABLE_ANALOG | \
>>>> + SSD16XX_CTRL2_LOAD_TEMPERATURE | \
>>>> + SSD16XX_CTRL2_LOAD_LUT | \
>>>> + SSD16XX_CTRL2_DISPLAY | \
>>>> + SSD16XX_CTRL2_DISABLE_ANALOG | \
>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xF7, ~1.5-2s */
>>>> +
>>>> +#define SSD1683_CTRL2_FAST_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
>>>> + SSD16XX_CTRL2_ENABLE_ANALOG | \
>>>> + SSD16XX_CTRL2_DISPLAY | \
>>>> + SSD16XX_CTRL2_DISABLE_ANALOG | \
>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xC7, ~1.0-1.5s */
>>>> +
>>>> +#define SSD1683_CTRL2_PARTIAL_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
>>>> + SSD16XX_CTRL2_ENABLE_ANALOG | \
>>>> + SSD16XX_CTRL2_LOAD_TEMPERATURE | \
>>>> + SSD16XX_CTRL2_LOAD_LUT | \
>>>> + SSD1683_CTRL2_MODE2 | \
>>>> + SSD16XX_CTRL2_DISPLAY | \
>>>> + SSD16XX_CTRL2_DISABLE_ANALOG | \
>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xFF,
>>>> ~300-500ms */
>>>> +
>>>> +/*
>>>> + * Standalone LUT pre-load sequence (0x91 = ENABLE_CLK | LOAD_LUT |
>>>> LOAD_TEMPERATURE |
>>>> + * DISABLE_CLK).
>>>> + * Pre-loads the OTP LUT without triggering a display update.
>>>> Required for
>>>> + * FAST refresh mode (0xC7) which omits LOAD_LUT from each update
>>>> cycle.
>>>> + */
>>>> +#define SSD1683_CTRL2_LOAD_TEMP_LUT
>>>> (SSD16XX_CTRL2_ENABLE_CLK | \
>>>> + SSD16XX_CTRL2_LOAD_LUT | \
>>>> + SSD16XX_CTRL2_LOAD_TEMPERATURE | \
>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xB1 */
>>>> +
>>>> +MODULE_IMPORT_NS("DMA_BUF");
>>>> +
>>>> +enum ssd16xx_controller {
>>>> + SSD1683 = 1,
>>>> +};
>>>> +
>>>> +enum ssd16xx_model {
>>>> + GDEY042T81 = 1,
>>>> +};
>>>> +
>>>> +enum ssd16xx_refresh_mode {
>>>> + SSD16XX_REFRESH_PARTIAL = 0, /* Partial refresh (~300-500ms) */
>>>> + SSD16XX_REFRESH_FULL, /* Full refresh (~1.5-2s) */
>>>> + SSD16XX_REFRESH_FAST, /* Fast refresh, skip temp load
>>>> (~1.0-1.5s) */
>>>> +};
>>>> +
>>>> +enum ssd16xx_color_mode {
>>>> + SSD16XX_COLOR_MODE_BW = 0, /* Black/white only; RED RAM
>>>> always bypassed */
>>>> + SSD16XX_COLOR_MODE_3COLOR = 1, /* 3-colour BWR; RED RAM used
>>>> for red pixels */
>>>> +};
>>>> +
>>>> +/* Border waveform enum indices (0-9); mapped to HW bytes via
>>>> + * controller_cfg->border_waveform_table[]
>>>> + */
>>>> +enum ssd16xx_border_waveform {
>>>> + SSD16XX_BORDER_LUT0 = 0, /* GS Transition LUT0 (black) */
>>>> + SSD16XX_BORDER_LUT1, /* GS Transition LUT1 (white) */
>>>> + SSD16XX_BORDER_LUT2, /* GS Transition LUT2 (black) */
>>>> + SSD16XX_BORDER_LUT3, /* GS Transition LUT3 (gray) */
>>>> + SSD16XX_BORDER_VSS, /* Fix Level VSS (black) */
>>>> + SSD16XX_BORDER_VSH1, /* Fix Level VSH1 (black) */
>>>> + SSD16XX_BORDER_VSL, /* Fix Level VSL (white) */
>>>> + SSD16XX_BORDER_VSH2, /* Fix Level VSH2 (black) */
>>>> + SSD16XX_BORDER_VCOM, /* Follow VCOM (preserve) */
>>>> + SSD16XX_BORDER_HIZ, /* HiZ (floating, default) */
>>>> +};
>>>> +
>>>> +/* SSD1683/SSD1680 border waveform byte encoding for command 0x3C */
>>>> +static const u8 ssd1683_border_waveform_table[] = {
>>>> + [SSD16XX_BORDER_LUT0] = SSD1683_BORDER_WAVEFORM_LUT0,
>>>> + [SSD16XX_BORDER_LUT1] = SSD1683_BORDER_WAVEFORM_LUT1,
>>>> + [SSD16XX_BORDER_LUT2] = SSD1683_BORDER_WAVEFORM_LUT2,
>>>> + [SSD16XX_BORDER_LUT3] = SSD1683_BORDER_WAVEFORM_LUT3,
>>>> + [SSD16XX_BORDER_VSS] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSS,
>>>> + [SSD16XX_BORDER_VSH1] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSH1,
>>>> + [SSD16XX_BORDER_VSL] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSL,
>>>> + [SSD16XX_BORDER_VSH2] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSH2,
>>>> + [SSD16XX_BORDER_VCOM] = SSD1683_BORDER_WAVEFORM_VCOM,
>>>> + [SSD16XX_BORDER_HIZ] = SSD1683_BORDER_WAVEFORM_HIZ,
>>>> +};
>>>> +
>>>> +struct ssd16xx_controller_config {
>>>> + u16 max_width;
>>>> + u16 max_height;
>>>> + u8 ram_x_address_bits;
>>>> + u8 ram_y_address_bits;
>>>> +
>>>> + /*
>>>> + * has_temp_sensor_ctrl: controller supports command 0x18
>>>> (Temperature
>>>> + * Sensor Selection). Present in SSD1683/SSD1680; absent in
>>>> SSD1673
>>>> + * which uses command 0x1A (direct temperature write) instead.
>>>> + */
>>>> + bool has_temp_sensor_ctrl;
>>>> +
>>>> + /*
>>>> + * Deep sleep mode byte values for command 0x10.
>>>> + * deep_sleep_mode_level1: lower-power sleep, RAM content
>>>> retained
>>>> + * (MODE_1 on SSD1683/SSD1680; used for runtime idle / app-
>>>> close).
>>>> + * deep_sleep_mode_level2: maximum power savings, RAM may be
>>>> lost
>>>> + * (MODE_2 on SSD1683/SSD1680; used for system suspend).
>>>> + * Chips with a single sleep mode set both fields to the same
>>>> value.
>>>> + */
>>>> + u8 deep_sleep_mode_level1;
>>>> + u8 deep_sleep_mode_level2;
>>>> +
>>>> + /*
>>>> + * border_waveform_table: chip-specific byte values for the 10
>>>> logical
>>>> + * border waveform modes (indexed by enum
>>>> ssd16xx_border_waveform).
>>>> + * The encoding of command 0x3C differs between SSD1683/SSD1680
>>>> and
>>>> + * SSD1673, so each controller provides its own translation table.
>>>> + */
>>>> + const u8 *border_waveform_table;
>>>> +
>>>> + /*
>>>> + * Display Update Control 1 (cmd 0x21) byte 1 values.
>>>> + * ctrl1_normal: both BW and RED RAMs participate in
>>>> the waveform.
>>>> + * ctrl1_bypass_red_ram: RED RAM bypassed; waveform driven from
>>>> BW RAM only.
>>>> + * SSD1673 has no RED RAM so both fields carry the same value.
>>>> + */
>>>> + u8 ctrl1_normal;
>>>> + u8 ctrl1_bypass_red_ram;
>>>> +
>>>> + /*
>>>> + * Display Update Control 2 (cmd 0x22) composite sequences for
>>>> each
>>>> + * refresh mode (indexed by enum ssd16xx_refresh_mode) and the
>>>> + * standalone LUT pre-load sequence used before fast refresh.
>>>> + * Values differ between SSD1683/SSD1680 and SSD1673 (MODE2
>>>> bit, etc.).
>>>> + */
>>>> + u8 ctrl2_refresh[3]; /* indexed by SSD16XX_REFRESH_PARTIAL/
>>>> FULL/FAST */
>>>> + u8 ctrl2_load_temp_lut; /* standalone LUT pre-load (no display
>>>> update) */
>>>> +};
>>>> +
>>>> +struct ssd16xx_panel_config {
>>>> + /* Data Entry Mode - controls X/Y increment direction for
>>>> landscape (0°) */
>>>> + u8 data_entry_mode;
>>>> +
>>>> + /* Driver Output Control - third byte (scan direction) */
>>>> + u8 driver_output_ctrl_byte3;
>>>> +
>>>> + /* Default refresh mode for this panel */
>>>> + enum ssd16xx_refresh_mode default_refresh_mode;
>>>> +
>>>> + /* Default border waveform during clear/init (enum index 0-9) */
>>>> + enum ssd16xx_border_waveform default_border_waveform_init;
>>>> +
>>>> + /* Default border waveform during display updates (enum index
>>>> 0-9) */
>>>> + enum ssd16xx_border_waveform default_border_waveform_update;
>>>> +
>>>> + /* Whether to re-send border waveform command before each
>>>> display update */
>>>> + bool default_border_refresh_on_every_update;
>>>> +
>>>> + /*
>>>> + * Default clear-on-init behaviour.
>>>> + * -1=disabled, 0=partial, 1=full, 2=fast (matches enum
>>>> ssd16xx_refresh_mode)
>>>> + */
>>>> + int default_clear_on_init;
>>>> +
>>>> + /* Default clear-on-close behaviour (-1=disabled, 0=partial,
>>>> 1=full, 2=fast) */
>>>> + int default_clear_on_close;
>>>> +
>>>> + /* Default clear-on-disable behaviour (-1=disabled, 0=partial,
>>>> 1=full, 2=fast) */
>>>> + int default_clear_on_disable;
>>>> +
>>>> + /*
>>>> + * Default refresh-mode-init: -1=disabled, else skip baseline
>>>> establishment
>>>> + * and start directly in this refresh mode.
>>>> + */
>>>> + int default_refresh_mode_init;
>>>> +
>>>> + /*
>>>> + * Whether this panel has a physical red colour plane (3-colour
>>>> BWR).
>>>> + * false: 2-colour black/white only; the RED RAM is always
>>>> bypassed.
>>>> + * true: 3-colour panel; full-refresh writes to the RED RAM so
>>>> that
>>>> + * red pixels are driven through the red waveform.
>>>> + */
>>>> + bool red_supported;
>>>> +
>>>> + /* Panel-specific display mode (resolution and physical
>>>> dimensions) */
>>>> + const struct drm_display_mode *mode;
>>>> +};
>>>> +
>>>> +struct ssd16xx_panel {
>>>
>>> Better call this 'struct ssd16xx_device' and the rsp variables
>>> 'ssd16xx'. As mentioned, the name 'panel' already has a specific
>>> meaning in DRM.
>>>
>>
>> Alright I can do that, I thought folks won't confuse it since this is
>> not importing drm_panel struct.
>>
>>>
>>>> + struct drm_device drm;
>>>> +
>>>> + struct drm_plane primary_plane;
>>>> + struct drm_crtc crtc;
>>>> + struct drm_encoder encoder;
>>>> + struct drm_connector connector;
>>>> +
>>>> + struct spi_device *spi;
>>>> + struct gpio_desc *reset;
>>>> + struct gpio_desc *busy;
>>>> + struct gpio_desc *dc;
>>>> +
>>>> + enum ssd16xx_model model;
>>>> + enum ssd16xx_controller controller;
>>>> + const struct ssd16xx_controller_config *controller_cfg;
>>>> + const struct ssd16xx_panel_config *panel_cfg;
>>>> + struct drm_display_mode *mode;
>>>> + u32 width;
>>>> + u32 height;
>>>> +
>>>> + bool initialized;
>>>> + bool reinit_pending; /* HW re-init required after
>>>> orientation change */
>>>> + bool init_refresh_pending; /* First frame after
>>>> refresh_mode_init enable */
>>>> + bool first_clear_done; /* clear_on_init has already fired once */
>>>> + bool display_cleared_on_deinit; /* Avoid redundant clear in
>>>> atomic_disable/master_drop */
>>>> +
>>>> + int orientation; /* Display orientation in degrees:
>>>> 0/90/180/270 */
>>>> + enum ssd16xx_refresh_mode refresh_mode; /* Active refresh mode */
>>>> + enum ssd16xx_color_mode color_mode; /* Active color mode
>>>> (BW or 3-color) */
>>>> + bool fast_lut_pending; /* LUT pre-load needed before next fast
>>>> refresh */
>>>> +
>>>> + /* Border waveform (as enum indices) */
>>>> + int border_waveform_init_idx; /* Border waveform during
>>>> clear/ init */
>>>> + int border_waveform_update_idx; /* Border waveform during
>>>> display updates */
>>>> + bool border_refresh_on_every_update; /* Re-send border cmd each
>>>> display update */
>>>> + bool border_waveform_pending; /* One-shot: send border cmd on
>>>> next update */
>>>> +
>>>> + /* Display control */
>>>> + int clear_on_init; /* -1=disabled, 0=partial, 1=full, 2=fast */
>>>> + int clear_on_close; /* -1=disabled, 0=partial, 1=full, 2=fast */
>>>> + int clear_on_disable; /* -1=disabled, 0=partial, 1=full, 2=fast */
>>>> + int refresh_mode_init; /* -1=disabled, else use this mode for
>>>> the first frame */
>>>> +
>>>> + u8 *tx_buf; /* 1bpp frame buffer (mono + white) */
>>>> + u8 *tx_red_buf; /* 1bpp red-channel buffer (3-color panels
>>>> only) */
>>>> + u16 *tx_buf9; /* 9-bit SPI expansion buffer (3-wire mode
>>>> only) */
>>>> +
>>>> + struct drm_framebuffer *last_fb; /* Last drawn FB for
>>>> reinit redraws */
>>>> + struct drm_property *rotation_property;
>>>> + struct drm_property *refresh_mode_property;
>>>> + struct drm_property *border_waveform_init_property;
>>>> + struct drm_property *border_waveform_update_property;
>>>> + struct drm_property *border_refresh_on_every_update_property;
>>>> + struct drm_property *clear_on_init_property;
>>>> + struct drm_property *clear_on_close_property;
>>>> + struct drm_property *clear_on_disable_property;
>>>> + struct drm_property *refresh_mode_init_property;
>>>> + struct drm_property *color_mode_property;
>>>> +};
>>>> +
>>>> +static inline struct ssd16xx_panel *to_ssd16xx_panel(struct
>>>> drm_device *drm)
>>>> +{
>>>> + return container_of(drm, struct ssd16xx_panel, drm);
>>>> +}
>>>> +
>>>> +static inline struct ssd16xx_panel *crtc_to_ssd16xx_panel(struct
>>>> drm_crtc *crtc)
>>>> +{
>>>> + return container_of(crtc, struct ssd16xx_panel, crtc);
>>>> +}
>>>> +
>>>> +static inline struct ssd16xx_panel *plane_to_ssd16xx_panel(struct
>>>> drm_plane *plane)
>>>> +{
>>>> + return container_of(plane, struct ssd16xx_panel, primary_plane);
>>>> +}
>>>> +
>>>> +static const struct ssd16xx_controller_config
>>>> ssd16xx_controller_configs[] = {
>>>> + [SSD1683] = {
>>>> + .max_width = 400,
>>>> + .max_height = 300,
>>>> + .ram_x_address_bits = 8,
>>>> + .ram_y_address_bits = 16,
>>>> + .has_temp_sensor_ctrl = true,
>>>> + .deep_sleep_mode_level1 = SSD1683_DEEP_SLEEP_MODE_1,
>>>> + .deep_sleep_mode_level2 = SSD1683_DEEP_SLEEP_MODE_2,
>>>> + .border_waveform_table = ssd1683_border_waveform_table,
>>>> + .ctrl1_normal = SSD1683_CTRL1_NORMAL,
>>>> + .ctrl1_bypass_red_ram = SSD1683_CTRL1_BYPASS_RED_RAM,
>>>> + .ctrl2_refresh = {
>>>> + [SSD16XX_REFRESH_PARTIAL] = SSD1683_CTRL2_PARTIAL_REFRESH,
>>>> + [SSD16XX_REFRESH_FULL] = SSD1683_CTRL2_FULL_REFRESH,
>>>> + [SSD16XX_REFRESH_FAST] = SSD1683_CTRL2_FAST_REFRESH,
>>>> + },
>>>> + .ctrl2_load_temp_lut = SSD1683_CTRL2_LOAD_TEMP_LUT,
>>>> + },
>>>> +};
>>>> +
>>>> +/* GDEY042T81: 4.2" 400x300 panel, 84.8x63.6mm active area */
>>>> +static const struct drm_display_mode gdey042t81_mode = {
>>>> + DRM_SIMPLE_MODE(400, 300, 85, 64),
>>>> +};
>>>> +
>>>> +static const struct ssd16xx_panel_config ssd16xx_panel_configs[] = {
>>>> + [GDEY042T81] = {
>>>> + .data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC,
>>>> + .driver_output_ctrl_byte3 =
>>>> SSD16XX_DRIVER_OUTPUT_CTRL_DEFAULT,
>>>> + .default_refresh_mode = SSD16XX_REFRESH_PARTIAL,
>>>> + .default_border_waveform_init = SSD16XX_BORDER_LUT1, /*
>>>> white, clean clear */
>>>> + .default_border_waveform_update = SSD16XX_BORDER_HIZ, /*
>>>> floating, preserve */
>>>> + .default_border_refresh_on_every_update = false,
>>>> + .default_clear_on_init = -1,
>>>> + .default_clear_on_close = -1,
>>>> + .default_clear_on_disable = -1,
>>>> + .default_refresh_mode_init = SSD16XX_REFRESH_FULL,
>>>> + .red_supported = false, /* 2-colour black/white panel */
>>>> + .mode = &gdey042t81_mode,
>>>> + },
>>>> +};
>>>> +
>>>> +static void ssd16xx_wait_for_panel(struct ssd16xx_panel *panel,
>>>> + int *err)
>>>> +{
>>>> + unsigned long timeout_jiffies = jiffies +
>>>> + msecs_to_jiffies(SSD16XX_BUSY_WAIT_TIMEOUT_MS);
>>>> + unsigned long start_ms = jiffies_to_msecs(jiffies);
>>>> + int busy_val;
>>>> +
>>>> + if (*err)
>>>> + return;
>>>
>>> This is good. It'll simplify error handling in other places.
>>>
>>>> +
>>>> + busy_val = gpiod_get_value_cansleep(panel->busy);
>>>> + drm_dbg(&panel->drm, "BUSY initial value: %d\n", busy_val);
>>>> +
>>>> + while (gpiod_get_value_cansleep(panel->busy) == 1) {
>>>> + if (time_after(jiffies, timeout_jiffies)) {
>>>> + drm_err(&panel->drm, "Busy wait timed out after %lums\n",
>>>> + jiffies_to_msecs(jiffies) - start_ms);
>>>> + *err = -ETIMEDOUT;
>>>> + return;
>>>> + }
>>>> + usleep_range(100, 200);
>>>> + }
>>>> +
>>>> + drm_dbg(&panel->drm, "BUSY became ready after %lums\n",
>>>> + jiffies_to_msecs(jiffies) - start_ms);
>>>> +}
>>>> +
>>>> +static void ssd16xx_spi_sync(struct spi_device *spi, struct
>>>> spi_message *msg,
>>>> + int *err)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + ret = spi_sync(spi, msg);
>>>> + if (ret < 0)
>>>> + *err = ret;
>>>> +}
>>>> +
>>>> +static void ssd16xx_send_cmd(struct ssd16xx_panel *panel, u8 cmd,
>>>> + int *err)
>>>> +{
>>>> + u16 word;
>>>> + struct spi_transfer xfer = {};
>>>> + struct spi_message msg;
>>>> +
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + spi_message_init(&msg);
>>>> + spi_message_add_tail(&xfer, &msg);
>>>> +
>>>> + if (panel->dc) {
>>>> + /* 4-wire SPI: D/C# GPIO low selects command mode */
>>>> + xfer.tx_buf = &cmd;
>>>> + xfer.len = 1;
>>>> + gpiod_set_value_cansleep(panel->dc, 0);
>>>> + } else {
>>>> + /*
>>>> + * 3-wire SPI (9-bit): bit 8 is the D/C# bit.
>>>> + * D/C# = 0 means the following 8 bits are a command.
>>>> + */
>>>> + word = cmd; /* bit 8 = 0 for command */
>>>> + xfer.tx_buf = &word;
>>>> + xfer.len = sizeof(u16);
>>>> + xfer.bits_per_word = 9;
>>>> + }
>>>> +
>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>> +}
>>>> +
>>>> +static void ssd16xx_send_data(struct ssd16xx_panel *panel, u8 data,
>>>> + int *err)
>>>> +{
>>>> + u16 word;
>>>> + struct spi_transfer xfer = {};
>>>> + struct spi_message msg;
>>>> +
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + spi_message_init(&msg);
>>>> + spi_message_add_tail(&xfer, &msg);
>>>> +
>>>> + if (panel->dc) {
>>>> + /* 4-wire SPI: D/C# GPIO high selects data mode */
>>>> + xfer.tx_buf = &data;
>>>> + xfer.len = 1;
>>>> + gpiod_set_value_cansleep(panel->dc, 1);
>>>> + } else {
>>>> + /*
>>>> + * 3-wire SPI (9-bit): bit 8 is the D/C# bit.
>>>> + * D/C# = 1 means the following 8 bits are data.
>>>> + */
>>>> + word = 0x100 | data;
>>>> + xfer.tx_buf = &word;
>>>> + xfer.len = sizeof(u16);
>>>> + xfer.bits_per_word = 9;
>>>> + }
>>>> +
>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>> +}
>>>> +
>>>> +static void ssd16xx_send_x_param(struct ssd16xx_panel *panel, u16 x,
>>>> + int *err)
>>>> +{
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + if (panel->controller_cfg->ram_x_address_bits == 8) {
>>>> + ssd16xx_send_data(panel, (u8)x, err);
>>>> + } else {
>>>> + ssd16xx_send_data(panel, x & 0xFF, err);
>>>> + ssd16xx_send_data(panel, (x >> 8) & 0xFF, err);
>>>> + }
>>>> +}
>>>> +
>>>> +static void ssd16xx_send_y_param(struct ssd16xx_panel *panel, u16 y,
>>>> + int *err)
>>>> +{
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + if (panel->controller_cfg->ram_y_address_bits == 8) {
>>>> + ssd16xx_send_data(panel, (u8)y, err);
>>>> + } else {
>>>> + ssd16xx_send_data(panel, y & 0xFF, err);
>>>> + ssd16xx_send_data(panel, (y >> 8) & 0xFF, err);
>>>> + }
>>>> +}
>>>> +
>>>> +static void ssd16xx_send_data_bulk(struct ssd16xx_panel *panel,
>>>> + const u8 *data, size_t len,
>>>> + int *err)
>>>> +{
>>>> + struct spi_transfer xfer = {};
>>>> + struct spi_message msg;
>>>> +
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + if (!data || !len)
>>>> + return;
>>>> +
>>>> + spi_message_init(&msg);
>>>> + spi_message_add_tail(&xfer, &msg);
>>>> +
>>>> + if (panel->dc) {
>>>> + /* 4-wire SPI: D/C# GPIO high selects data mode */
>>>> + xfer.tx_buf = data;
>>>> + xfer.len = len;
>>>> + gpiod_set_value_cansleep(panel->dc, 1);
>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>> + } else {
>>>> + /* 3-wire (9-bit): expand u8 → u16 with D/C#=1 in bit 8. */
>>>> + size_t i;
>>>> + u16 *buf = panel->tx_buf9;
>>>> +
>>>> + for (i = 0; i < len; i++)
>>>> + buf[i] = 0x100 | data[i];
>>>> +
>>>> + xfer.tx_buf = buf;
>>>> + xfer.len = len * sizeof(u16);
>>>> + xfer.bits_per_word = 9;
>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>> + }
>>>> +}
>>>> +
>>>> +static void ssd16xx_display_update(struct ssd16xx_panel *panel,
>>>> + u8 ctrl1_byte1, u8 ctrl1_byte2, u8 ctrl2_mode,
>>>> + int *err)
>>>> +{
>>>> + if (*err)
>>>> + return;
>>>> +
>>>> + drm_dbg(&panel->drm,
>>>> + "display_update: Setting ctrl1=0x%02x,0x%02x mode=0x%02x\n",
>>>> + ctrl1_byte1, ctrl1_byte2, ctrl2_mode);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, err);
>>>> + ssd16xx_send_data(panel, ctrl1_byte1, err);
>>>> + ssd16xx_send_data(panel, ctrl1_byte2, err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, err);
>>>> + ssd16xx_send_data(panel, ctrl2_mode, err);
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_MASTER_ACTIVATION, err);
>>>> +
>>>> + drm_dbg(&panel->drm,
>>>> + "display_update: Master activation sent, waiting...\n");
>>>> +
>>>> + ssd16xx_wait_for_panel(panel, err);
>>>> +}
>>>> +
>>>> +static void ssd16xx_hw_reset(struct ssd16xx_panel *panel)
>>>> +{
>>>> + gpiod_set_value_cansleep(panel->reset, 1);
>>>> + usleep_range(10000, 11000);
>>>> + gpiod_set_value_cansleep(panel->reset, 0);
>>>> + usleep_range(10000, 11000);
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_preload_fast_lut() - pre-load the OTP LUT for fast
>>>> refresh mode.
>>>> + *
>>>> + * Fast refresh (CTRL2 = 0xC7) omits the LOAD_LUT step on every
>>>> update to save
>>>> + * time. It relies on the LUT being loaded upfront via this
>>>> standalone sequence
>>>> + * (CTRL2 = 0xB1: ENABLE_CLK | LOAD_LUT |
>>>> SSD16XX_CTRL2_LOAD_TEMPERATURE | DISABLE_CLK,
>>>> + * no display update).
>>>> + *
>>>> + * Must be called when:
>>>> + * a) hw_init runs with refresh_mode == FAST, and
>>>> + * b) switching to fast refresh from a mode that did not leave a
>>>> valid Mode1
>>>> + * LUT in the controller (i.e. previous mode was not FULL
>>>> refresh, which
>>>> + * carries LOAD_LUT in its own CTRL2 sequence).
>>>> + */
>>>> +static int ssd16xx_preload_fast_lut(struct ssd16xx_panel *panel)
>>>> +{
>>>> + int err = 0;
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1,
>>>> &err);
>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>> >ctrl1_bypass_red_ram, &err);
>>>> + ssd16xx_send_data(panel, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2,
>>>> &err);
>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>> >ctrl2_load_temp_lut, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_MASTER_ACTIVATION, &err);
>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>> +
>>>> + return err;
>>>> +}
>>>> +
>>>> +static int ssd16xx_hw_init(struct ssd16xx_panel *panel)
>>>> +{
>>>> + int err = 0;
>>>> + u16 ram_height = panel->controller_cfg->max_height;
>>>> + u8 data_entry_mode;
>>>> +
>>>> + ssd16xx_hw_reset(panel);
>>>> +
>>>> + /* Software reset */
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_SW_RESET, &err);
>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>> +
>>>> + /* Driver output control (0x01): MUX ratio and scan direction. */
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DRIVER_OUTPUT_CONTROL, &err);
>>>> + ssd16xx_send_y_param(panel, ram_height - 1, &err);
>>>> + ssd16xx_send_data(panel, panel->panel_cfg-
>>>> >driver_output_ctrl_byte3, &err);
>>>> +
>>>> + /* Internal temperature sensor (SSD1683/SSD1680 only; not
>>>> present in SSD1673) */
>>>> + if (panel->controller_cfg->has_temp_sensor_ctrl) {
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL, &err);
>>>> + ssd16xx_send_data(panel, SSD1683_TEMP_SENSOR_INTERNAL, &err);
>>>> + }
>>>> +
>>>> + /*
>>>> + * For FAST refresh mode, pre-load the LUT once here during
>>>> initialization.
>>>> + * FAST mode ctrl2 (0xC7) omits LOAD_LUT on every update for
>>>> speed, so the
>>>> + * LUT must be loaded upfront. FULL (0xF7) and PARTIAL (0xFF)
>>>> load LUT on
>>>> + * every update, so no preload is needed for those modes.
>>>> + */
>>>> + if (panel->refresh_mode == SSD16XX_REFRESH_FAST) {
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, &err);
>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>> >ctrl1_bypass_red_ram, &err);
>>>> + ssd16xx_send_data(panel, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, &err);
>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>> >ctrl2_load_temp_lut, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_MASTER_ACTIVATION, &err);
>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>> + }
>>>> +
>>>> + /*
>>>> + * Set Data Entry Mode (0x11) based on orientation. This controls
>>>> + * how the RAM address counter auto-advances after each byte
>>>> write.
>>>> + *
>>>> + * Implementation uses two data entry modes:
>>>> + * - 90°/180° use XDEC_YDEC (0x00): X--, Y-- with cursor at
>>>> (max, max)
>>>> + * - 0°/270° use XINC_YINC (0x03): X++, Y++ with cursor at
>>>> (0, 0)
>>>> + *
>>>> + * The convert_fb_to_1bpp packing is grouped by physical layout:
>>>> + * - Portrait orientations (90°/270°): column-major packing
>>>> + * - Landscape orientations (0°/180°): row-major packing
>>>> + *
>>>> + * Final scan direction and image orientation are controlled by
>>>> the
>>>> + * combination of data entry mode and RAM cursor position set
>>>> in fb_dirty.
>>>> + *
>>>> + * The RAM address window and cursor are NOT set here; fb_dirty
>>>> + * always programmes them (with the correct end-before-start order
>>>> + * for decrement modes) immediately before writing frame data.
>>>> + */
>>>> + switch (panel->orientation) {
>>>
>>> As mentioned, use the connector property instead.
>>>
>>>> + case 90:
>>>> + case 180:
>>>> + data_entry_mode = SSD16XX_DATA_ENTRY_XDEC_YDEC;
>>>> + break;
>>>> + default: /* 0°/270° */
>>>> + data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC;
>>>> + break;
>>>> + }
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DATA_ENTRY_MODE, &err);
>>>> + ssd16xx_send_data(panel, data_entry_mode, &err);
>>>> + drm_dbg(&panel->drm, "hw_init: orientation=%u°
>>>> data_entry=0x%02x\n",
>>>> + panel->orientation, data_entry_mode);
>>>> +
>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>> +
>>>> + if (err)
>>>> + drm_err(&panel->drm, "Hardware initialization failed:
>>>> %d\n", err);
>>>> +
>>>> + return err;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Clear display by writing all-white to both BW and RED RAM.
>>>> + * The ctrl2 argument selects the waveform (full/partial/fast
>>>> refresh).
>>>> + * Border waveform is set to init value before clearing, then restored
>>>> + * to the update value to preserve the border during subsequent
>>>> updates.
>>>> + */
>>>> +static int ssd16xx_clear_display(struct ssd16xx_panel *panel, u8
>>>> ctrl2)
>>>> +{
>>>> + const u8 *bw_tbl = panel->controller_cfg->border_waveform_table;
>>>> + int err = 0;
>>>> + unsigned int data_size = (panel->width * panel->height) / 8;
>>>> + u8 *white_buffer = panel->tx_buf;
>>>> +
>>>> + memset(white_buffer, 0xFF, data_size);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER,
>>>> &err);
>>>> + ssd16xx_send_x_param(panel, 0x00, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER,
>>>> &err);
>>>> + ssd16xx_send_y_param(panel, 0x00, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_WRITE_RAM_BW, &err);
>>>> + ssd16xx_send_data_bulk(panel, white_buffer, data_size, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>> + ssd16xx_send_data_bulk(panel, white_buffer, data_size, &err);
>>>> +
>>>> + /* Set border waveform for the clear operation */
>>>> + drm_dbg(&panel->drm, "clear_display: Set border init waveform:
>>>> 0x%02x\n",
>>>> + bw_tbl[panel->border_waveform_init_idx]);
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL,
>>>> &err);
>>>> + ssd16xx_send_data(panel,
>>>> + bw_tbl[panel->border_waveform_init_idx],
>>>> + &err);
>>>> +
>>>> + /* 3-colour mode: CTRL1_NORMAL (read both RAMs); BW mode:
>>>> bypass RED. */
>>>> + ssd16xx_display_update(panel,
>>>> + panel->color_mode == SSD16XX_COLOR_MODE_3COLOR
>>>> + ? panel->controller_cfg->ctrl1_normal
>>>> + : panel->controller_cfg->ctrl1_bypass_red_ram,
>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT, ctrl2, &err);
>>>> +
>>>> + /* Restore border waveform to update/preservation value */
>>>> + drm_dbg(&panel->drm, "clear_display: Restored border update
>>>> waveform: 0x%02x\n",
>>>> + bw_tbl[panel->border_waveform_update_idx]);
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL,
>>>> &err);
>>>> + ssd16xx_send_data(panel,
>>>> + bw_tbl[panel->border_waveform_update_idx],
>>>> + &err);
>>>> +
>>>> + return err;
>>>> +}
>>>> +
>>>> +static u8 ssd16xx_refresh_mode_to_ctrl2(struct ssd16xx_panel *panel,
>>>> + enum ssd16xx_refresh_mode mode)
>>>> +{
>>>> + if (mode < ARRAY_SIZE(panel->controller_cfg->ctrl2_refresh))
>>>> + return panel->controller_cfg->ctrl2_refresh[mode];
>>>> + return panel->controller_cfg->ctrl2_refresh[SSD16XX_REFRESH_FULL];
>>>> +}
>>>> +
>>>> +/*
>>>> + * Clear display on new DRM master open (if clear_on_init >= 0).
>>>> + * Guarded by panel->first_clear_done; master_drop resets it
>>>> unconditionally
>>>> + * so each new client session gets a fresh clear.
>>>> + */
>>>> +static int ssd16xx_clear_display_on_init(struct ssd16xx_panel *panel)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + if (panel->clear_on_init < 0 || panel->first_clear_done)
>>>> + return 0;
>>>> +
>>>> + drm_dbg(&panel->drm, "clear_on_init: running, mode=%d\n",
>>>> + panel->clear_on_init);
>>>> + ret = ssd16xx_clear_display(panel,
>>>> + ssd16xx_refresh_mode_to_ctrl2(panel, panel-
>>>> >clear_on_init));
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + panel->first_clear_done = true;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Clear display when the displaying client exits (if
>>>> clear_on_close >= 0).
>>>> + * Called from ssd16xx_drm_master_drop().
>>>> + */
>>>> +static int ssd16xx_clear_display_on_exit(struct ssd16xx_panel *panel)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + if (panel->clear_on_close < 0)
>>>> + return 0;
>>>> +
>>>> + drm_dbg(&panel->drm, "clear_on_close: running, mode=%d\n",
>>>> + panel->clear_on_close);
>>>> + ret = ssd16xx_clear_display(panel,
>>>> + ssd16xx_refresh_mode_to_ctrl2(panel, panel-
>>>> >clear_on_close));
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_pixel_luma() - return ITU-R BT.601 luminance (0-255) for
>>>> one pixel.
>>>> + *
>>>> + * For colour formats the result is (299*R + 587*G + 114*B) / 1000;
>>>> + * for luma-only formats the luma byte is returned directly.
>>>> + *
>>>> + * R1 is never passed here — it is already 1bpp and is handled
>>>> directly by
>>>> + * the callers.
>>>> + */
>>>> +static u8 ssd16xx_pixel_luma(struct iosys_map *src,
>>>> + struct drm_framebuffer *fb,
>>>> + unsigned int x, unsigned int y)
>>>> +{
>>>> + switch (fb->format->format) {
>>>> + case DRM_FORMAT_XRGB8888: {
>>>> + u32 *line = (u32 *)(src->vaddr + y * fb->pitches[0]);
>>>> + u32 px = line[x];
>>>> + u8 r = (px >> 16) & 0xFF, g = (px >> 8) & 0xFF, b = px & 0xFF;
>>>> +
>>>> + return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
>>>> + }
>>>> + case DRM_FORMAT_RGB888: {
>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>> + u8 r = line[x * 3], g = line[x * 3 + 1], b = line[x * 3 + 2];
>>>> +
>>>> + return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
>>>> + }
>>>> + case DRM_FORMAT_RGB565: {
>>>> + u16 *line = (u16 *)(src->vaddr + y * fb->pitches[0]);
>>>> + u16 px = line[x];
>>>> + u8 r = ((px >> 11) & 0x1F) << 3;
>>>> + u8 g = ((px >> 5) & 0x3F) << 2;
>>>> + u8 b = (px & 0x1F) << 3;
>>>> +
>>>> + return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
>>>> + }
>>>> + case DRM_FORMAT_R8: {
>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>> +
>>>> + return line[x];
>>>> + }
>>>> + case DRM_FORMAT_NV12:
>>>> + case DRM_FORMAT_NV16:
>>>> + return ((u8 *)(src->vaddr))[y * fb->pitches[0] + x];
>>>> + case DRM_FORMAT_YUYV: {
>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>> +
>>>> + return line[x * 2];
>>>> + }
>>>> + case DRM_FORMAT_UYVY: {
>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>> +
>>>> + return line[x * 2 + 1];
>>>> + }
>>>> + default:
>>>> + return 0;
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_pixel_is_white() - test whether a pixel maps to white in
>>>> 1bpp output.
>>>> + *
>>>> + * Uses fixed threshold of 127. Pixels with luma strictly greater
>>>> than 127
>>>> + * are rendered white.
>>>> + */
>>>> +static bool ssd16xx_pixel_is_white(struct iosys_map *src,
>>>> + struct drm_framebuffer *fb,
>>>> + unsigned int x, unsigned int y)
>>>> +{
>>>> + /* R1 is already binarised; avoid the luma computation
>>>> entirely. */
>>>> + if (fb->format->format == DRM_FORMAT_R1) {
>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>> +
>>>> + return !!(line[x / 8] & (1 << (7 - (x % 8))));
>>>> + }
>>>> + return ssd16xx_pixel_luma(src, fb, x, y) > 127;
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_pixel_is_red() - test whether a pixel is dominated by
>>>> the red channel.
>>>> + *
>>>> + * Only meaningful for formats that carry RGB information
>>>> (XRGB8888, RGB888,
>>>> + * RGB565). For luma-only and monochrome formats there is no red
>>>> channel, so
>>>> + * the function always returns false; callers should use
>>>> ssd16xx_pixel_is_white()
>>>> + * to obtain the BW value for those formats.
>>>> + *
>>>> + * Returns true when the red component exceeds 50% intensity AND is
>>>> strictly
>>>> + * greater than both green and blue (dominant red hue).
>>>> + */
>>>> +static bool ssd16xx_pixel_is_red(struct iosys_map *src,
>>>> + struct drm_framebuffer *fb,
>>>> + unsigned int x, unsigned int y)
>>>> +{
>>>> + u32 format = fb->format->format;
>>>> +
>>>> + switch (format) {
>>>> + case DRM_FORMAT_XRGB8888: {
>>>> + u32 *line = (u32 *)(src->vaddr + y * fb->pitches[0]);
>>>> + u32 px = line[x];
>>>> + u8 r = (px >> 16) & 0xFF;
>>>> + u8 g = (px >> 8) & 0xFF;
>>>> + u8 b = px & 0xFF;
>>>> +
>>>> + return r > 127 && r > g && r > b;
>>>> + }
>>>> + case DRM_FORMAT_RGB888: {
>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>> + u8 r = line[x * 3];
>>>> + u8 g = line[x * 3 + 1];
>>>> + u8 b = line[x * 3 + 2];
>>>> +
>>>> + return r > 127 && r > g && r > b;
>>>> + }
>>>> + case DRM_FORMAT_RGB565: {
>>>> + u16 *line = (u16 *)(src->vaddr + y * fb->pitches[0]);
>>>> + u16 px = line[x];
>>>> + u8 r = ((px >> 11) & 0x1F) << 3;
>>>> + u8 g = ((px >> 5) & 0x3F) << 2;
>>>> + u8 b = (px & 0x1F) << 3;
>>>> +
>>>> + return r > 127 && r > g && r > b;
>>>> + }
>>>> + default:
>>>> + return false; /* No colour channel information */
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_convert_fb_to_3color() - split a framebuffer into BW and
>>>> RED planes.
>>>> + * @bw_dst: output buffer for the black/white RAM plane (1=white,
>>>> 0=black)
>>>> + * @red_dst: output buffer for the red RAM plane (1=red,
>>>> 0=not red)
>>>> + * @src: mapped framebuffer memory
>>>> + * @fb: DRM framebuffer descriptor
>>>> + * @rect: region to convert (must be aligned to 8-pixel boundaries)
>>>> + *
>>>> + * Each output buffer must be at least rect_width/8 * rect_height
>>>> bytes.
>>>> + * Pixels are classified as:
>>>> + * - red: written to red_dst as 1, bw_dst as 0 (black)
>>>> + * - white: written to bw_dst as 1, red_dst as 0
>>>> + * - black: written to both as 0
>>>> + *
>>>> + * For monochrome formats (R1) where no colour information is
>>>> available the
>>>> + * source data is copied verbatim to bw_dst and red_dst is cleared
>>>> to 0xFF
>>>> + * (all-white = no red pixels).
>>>> + */
>>>> +static void ssd16xx_convert_fb_to_3color(u8 *bw_dst, u8 *red_dst,
>>>> + struct iosys_map *src,
>>>> + struct drm_framebuffer *fb,
>>>> + struct drm_rect *rect)
>>>> +{
>>>> + unsigned int x, y;
>>>> + u8 bw_byte = 0, red_byte = 0;
>>>> + unsigned int bit_pos = 0;
>>>> + unsigned int dst_idx = 0;
>>>> +
>>>> + drm_dbg(fb->dev,
>>>> + "convert_3color: fmt=%p4cc rect=(%d,%d)-(%d,%d) path=%s\n",
>>>> + &fb->format->format,
>>>> + rect->x1, rect->y1, rect->x2, rect->y2,
>>>> + fb->format->format == DRM_FORMAT_R1 ? "R1-direct" : "color-
>>>> pixel");
>>>> +
>>>> + /*
>>>> + * R1 is already monochrome — no colour channel exists.
>>>> + * Copy BW data directly and leave the red plane all-white
>>>> (transparent).
>>>> + */
>>>> + if (fb->format->format == DRM_FORMAT_R1) {
>>>> + unsigned int src_pitch = fb->pitches[0];
>>>> + unsigned int width_bytes = drm_rect_width(rect) / 8;
>>>> + unsigned int data_size = width_bytes * drm_rect_height(rect);
>>>> +
>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>> + u8 *line = src->vaddr + y * src_pitch + (rect->x1 / 8);
>>>> +
>>>> + memcpy(bw_dst + dst_idx, line, width_bytes);
>>>> + dst_idx += width_bytes;
>>>> + }
>>>> + memset(red_dst, 0xFF, data_size); /* 0xFF = all white: no
>>>> red pixels */
>>>> + return;
>>>> + }
>>>> +
>>>> + /* Use fixed threshold of 127 for grayscale to monochrome
>>>> conversion. */
>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>> + for (x = rect->x1; x < rect->x2; x++) {
>>>> + bool is_red = ssd16xx_pixel_is_red(src, fb, x, y);
>>>> +
>>>> + if (is_red)
>>>> + red_byte |= (1 << (7 - bit_pos));
>>>> + else if (ssd16xx_pixel_is_white(src, fb, x, y))
>>>> + bw_byte |= (1 << (7 - bit_pos));
>>>> + /* else: black pixel — both bits remain 0 */
>>>> + if (++bit_pos == 8) {
>>>> + bw_dst[dst_idx] = bw_byte;
>>>> + red_dst[dst_idx] = red_byte;
>>>> + dst_idx++;
>>>> + bw_byte = 0;
>>>> + red_byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> +
>>>> + /* Flush any partial byte at the end of each row */
>>>> + if (bit_pos > 0) {
>>>> + bw_dst[dst_idx] = bw_byte;
>>>> + red_dst[dst_idx] = red_byte;
>>>> + dst_idx++;
>>>> + bw_byte = 0;
>>>> + red_byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_convert_r8_to_red_only() - map an R8 framebuffer to the
>>>> RED RAM plane.
>>>> + *
>>>> + * Used when the panel has a physical red colour plane
>>>> (red_supported == true)
>>>> + * and the framebuffer format is DRM_FORMAT_R8. Pixels with value
>>>> >= 128 are
>>>> + * treated as red ink; the BW RAM is set to all-white so that only
>>>> red ink
>>>> + * appears on the white background.
>>>> + *
>>>> + * Hardware orientation is handled by the caller via RAM counter
>>>> positioning;
>>>> + * data is written in normal row-major order here (same as
>>>> convert_fb_to_3color).
>>>> + */
>>>> +static void ssd16xx_convert_r8_to_red_only(u8 *bw_dst, u8 *red_dst,
>>>> + struct iosys_map *src,
>>>> + struct drm_framebuffer *fb,
>>>> + struct drm_rect *rect)
>>>> +{
>>>> + unsigned int src_pitch = fb->pitches[0];
>>>> + unsigned int width = drm_rect_width(rect);
>>>> + unsigned int height = drm_rect_height(rect);
>>>> + unsigned int data_size = DIV_ROUND_UP(width, 8) * height;
>>>> + unsigned int dst_idx = 0;
>>>> + unsigned int x, y;
>>>> + u8 red_byte = 0;
>>>> + unsigned int bit_pos = 0;
>>>> +
>>>> + /* BW RAM: all-white background - no black ink, only red ink
>>>> shows */
>>>> + memset(bw_dst, 0xFF, data_size);
>>>> +
>>>> + /* RED RAM: R8 >= 128 -> red ink (1-bit set) */
>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>> + u8 *line = src->vaddr + y * src_pitch;
>>>> +
>>>> + for (x = rect->x1; x < rect->x2; x++) {
>>>> + if (line[x] >= 128)
>>>> + red_byte |= (1 << (7 - bit_pos));
>>>> + if (++bit_pos == 8) {
>>>> + red_dst[dst_idx++] = red_byte;
>>>> + red_byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> + if (bit_pos > 0) {
>>>> + red_dst[dst_idx++] = red_byte;
>>>> + red_byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * Convert framebuffer to 1-bit monochrome for e-paper display.
>>>> + *
>>>> + * Supported formats: XRGB8888, RGB888, RGB565, R8, NV12, NV16,
>>>> YUYV, UYVY, R1.
>>>> + * For colour and luma formats, Otsu's global binarisation method
>>>> computes an
>>>> + * optimal per-image threshold from the luminance histogram.
>>>> + * R1 is the controller's native format and bypasses conversion
>>>> entirely.
>>>> + *
>>>> + * Output layout:
>>>> + * 0°/180° landscape: row-major, left-to-right, top-to-bottom
>>>> + * 90°/270° CW portrait: column-major, rightmost column first
>>>> + */
>>>> +static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
>>>> + struct drm_framebuffer *fb,
>>>> + struct drm_rect *rect,
>>>> + unsigned int orientation)
>>>> +{
>>>> + u32 format = fb->format->format;
>>>> + int x, y;
>>>> + u8 byte = 0;
>>>> + unsigned int bit_pos = 0;
>>>> + unsigned int dst_idx = 0;
>>>> +
>>>> + /* Use fixed threshold of 127 for grayscale to monochrome
>>>> conversion. */
>>>> + drm_dbg(fb->dev,
>>>> + "convert_1bpp: fmt=%p4cc rect=(%d,%d)-(%d,%d) orient=%u°
>>>> path=%s\n",
>>>> + &fb->format->format,
>>>> + rect->x1, rect->y1, rect->x2, rect->y2,
>>>> + orientation,
>>>> + (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 %
>>>> 8 == 0) ? "R1-fast" :
>>>> + (orientation == 90 || orientation == 270) ? "portrait" :
>>>> "landscape");
>>>> +
>>>> + /*
>>>> + * R1 fast path: 0° landscape with byte-aligned rect.
>>>> + * R1 is already 1bpp so landscape rows map directly to output
>>>> bytes via
>>>> + * memcpy — no per-pixel computation needed. rect->x1 must be a
>>>> + * multiple of 8 so that (rect->x1 / 8) gives the correct byte
>>>> offset;
>>>> + * if not, the generic pixel-by-pixel loop below handles non-
>>>> aligned
>>>> + * rects safely.
>>>> + */
>>>> + if (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 % 8
>>>> == 0) {
>>>> + unsigned int src_pitch = fb->pitches[0];
>>>> + unsigned int width_bytes = drm_rect_width(rect) / 8;
>>>> +
>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>> + u8 *src_line = src->vaddr + y * src_pitch + (rect->x1 /
>>>> 8);
>>>> +
>>>> + memcpy(dst + dst_idx, src_line, width_bytes);
>>>> + dst_idx += width_bytes;
>>>> + }
>>>> + return;
>>>> + }
>>>> +
>>>> + switch (orientation) {
>>>> + case 90:
>>>> + case 270:
>>>> + /*
>>>> + * Portrait (90° or 270°): column-major packing.
>>>> + * Each portrait source column becomes one physical RAM row.
>>>> + * The data entry mode and cursor position control scan
>>>> direction.
>>>> + */
>>>> + for (x = rect->x2 - 1; x >= (int)rect->x1; x--) {
>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>> + if (ssd16xx_pixel_is_white(src, fb, x, y))
>>>> + byte |= (1 << (7 - bit_pos));
>>>> + if (++bit_pos == 8) {
>>>> + dst[dst_idx++] = byte;
>>>> + byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> + if (bit_pos > 0) {
>>>> + dst[dst_idx++] = byte;
>>>> + byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> + break;
>>>> +
>>>> + case 0:
>>>> + case 180:
>>>> + default:
>>>> + /*
>>>> + * Landscape (0° or 180°): row-major packing.
>>>> + * Each landscape source row becomes one physical RAM row.
>>>> + * The data entry mode and cursor position control scan
>>>> direction.
>>>> + */
>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>> + for (x = rect->x1; x < rect->x2; x++) {
>>>> + if (ssd16xx_pixel_is_white(src, fb, x, y))
>>>> + byte |= (1 << (7 - bit_pos));
>>>> + if (++bit_pos == 8) {
>>>> + dst[dst_idx++] = byte;
>>>> + byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> + if (bit_pos > 0) {
>>>> + dst[dst_idx++] = byte;
>>>> + byte = 0;
>>>> + bit_pos = 0;
>>>> + }
>>>> + }
>>>> + break;
>>>> + }
>>>> +}
>>>> +
>>>> +static int ssd16xx_fb_dirty(struct drm_framebuffer *fb, struct
>>>> drm_rect *rect,
>>>> + struct ssd16xx_panel *panel)
>>>> +{
>>>> + const u8 *ctrl2_tbl = panel->controller_cfg->ctrl2_refresh;
>>>> + struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb,
>>>> 0);
>>>> + struct iosys_map map;
>>>> + int err = 0;
>>>> + unsigned int data_size = (panel->width * panel->height) / 8;
>>>> + u8 *mono_buffer = NULL;
>>>> + u8 *red_buffer = NULL;
>>>> + u16 ram_x_start, ram_x_end, ram_y_start, ram_y_end;
>>>> +
>>>> + /* Process full display area; convert handles orientation
>>>> traversal. */
>>>> + rect->x1 = 0;
>>>> + rect->y1 = 0;
>>>> + rect->x2 = panel->width;
>>>> + rect->y2 = panel->height;
>>>> +
>>>> + drm_dbg(&panel->drm,
>>>> + "fb_dirty: fb=%dx%d, refresh_mode=%d, orientation=%d\n",
>>>> + fb->width, fb->height, panel->refresh_mode, panel-
>>>> >orientation);
>>>> +
>>>> + mono_buffer = panel->tx_buf;
>>>> + memset(mono_buffer, 0, data_size);
>>>> +
>>>> + /* 3-colour FULL/FAST: populate red channel. */
>>>> + if (panel->color_mode == SSD16XX_COLOR_MODE_3COLOR &&
>>>> + (panel->refresh_mode == SSD16XX_REFRESH_FULL ||
>>>> + panel->refresh_mode == SSD16XX_REFRESH_FAST)) {
>>>> + red_buffer = panel->tx_red_buf;
>>>> + memset(red_buffer, 0, data_size);
>>>> + }
>>>> +
>>>> + iosys_map_set_vaddr(&map, dma_obj->vaddr);
>>>> +
>>>> + if (red_buffer && fb->format->format == DRM_FORMAT_R8)
>>>> + ssd16xx_convert_r8_to_red_only(mono_buffer, red_buffer,
>>>> &map, fb, rect);
>>>> + else if (red_buffer)
>>>> + ssd16xx_convert_fb_to_3color(mono_buffer, red_buffer, &map,
>>>> fb, rect);
>>>> + else
>>>> + ssd16xx_convert_fb_to_1bpp(mono_buffer, &map, fb, rect,
>>>> panel->orientation);
>>>> +
>>>> + drm_dbg(&panel->drm,
>>>> + "fb_dirty: mono[0..3]=0x%02x 0x%02x 0x%02x 0x%02x
>>>> (data_size=%u)\n",
>>>> + mono_buffer[0], mono_buffer[1], mono_buffer[2],
>>>> mono_buffer[3],
>>>> + data_size);
>>>> +
>>>> + /* Set RAM window and cursor for current orientation. */
>>>> + ram_x_start = 0;
>>>> + ram_x_end = (panel->controller_cfg->max_width / 8) - 1;
>>>> + ram_y_start = 0;
>>>> + ram_y_end = panel->controller_cfg->max_height - 1;
>>>> +
>>>> + switch (panel->orientation) {
>>>> + case 90:
>>>> + case 180:
>>>> + /* 90°/180°: XDEC_YDEC mode, send end-before-start; cursor
>>>> at (max, max). */
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
>>>> + ssd16xx_send_x_param(panel, ram_x_end, &err);
>>>> + ssd16xx_send_x_param(panel, ram_x_start, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
>>>> + ssd16xx_send_y_param(panel, ram_y_end, &err);
>>>> + ssd16xx_send_y_param(panel, ram_y_start, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
>>>> + ssd16xx_send_x_param(panel, ram_x_end, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
>>>> + ssd16xx_send_y_param(panel, ram_y_end, &err);
>>>> + break;
>>>> +
>>>> + default: /* 0°/270° */
>>>> + /* 0°/270°: XINC_YINC mode, cursor at (0, 0). */
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
>>>> + ssd16xx_send_x_param(panel, ram_x_start, &err);
>>>> + ssd16xx_send_x_param(panel, ram_x_end, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
>>>> + ssd16xx_send_y_param(panel, ram_y_start, &err);
>>>> + ssd16xx_send_y_param(panel, ram_y_end, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
>>>> + ssd16xx_send_x_param(panel, ram_x_start, &err);
>>>> +
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
>>>> + ssd16xx_send_y_param(panel, ram_y_start, &err);
>>>> + break;
>>>> + }
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_WRITE_RAM_BW, &err);
>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size, &err);
>>>> +
>>>> + /* Re-send border waveform when: every-update mode, init frame, or
>>>> + * the border_waveform_update property just changed (one-shot).
>>>> + */
>>>> + drm_dbg(&panel->drm,
>>>> + "fb_dirty: border check: every_update=%d init_pending=%d
>>>> border_pending=%d idx=%d hw=0x%02x\n",
>>>> + panel->border_refresh_on_every_update, panel-
>>>> >init_refresh_pending,
>>>> + panel->border_waveform_pending, panel-
>>>> >border_waveform_update_idx,
>>>> + panel->controller_cfg->border_waveform_table[panel-
>>>> >border_waveform_update_idx]);
>>>> + if (panel->border_refresh_on_every_update || panel-
>>>> >init_refresh_pending ||
>>>> + panel->border_waveform_pending) {
>>>> + u8 idx = panel->border_waveform_update_idx;
>>>> + u8 border = panel->controller_cfg->border_waveform_table[idx];
>>>> +
>>>> + drm_dbg(&panel->drm, "fb_dirty: Sending border waveform:
>>>> 0x%02x\n",
>>>> + border);
>>>> + ssd16xx_send_cmd(panel,
>>>> SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
>>>> + ssd16xx_send_data(panel, border, &err);
>>>> + panel->border_waveform_pending = false;
>>>> + }
>>>> +
>>>> + switch (panel->refresh_mode) {
>>>> + case SSD16XX_REFRESH_FULL:
>>>> + /*
>>>> + * BW full refresh: write RED RAM BEFORE display_update
>>>> + * to avoid a post-BUSY write timing issue on some
>>>> + * controller revisions that silently corrupts RED RAM.
>>>> + * RED RAM is then bypassed (CTRL1_BYPASS_RED_RAM) so
>>>> + * stale RED RAM content does not affect the output.
>>>> + */
>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>> + if (red_buffer) {
>>>> + /* 3-colour: write red channel before activating */
>>>> + ssd16xx_send_data_bulk(panel, red_buffer, data_size,
>>>> &err);
>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>> >ctrl1_normal,
>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>> + ctrl2_tbl[SSD16XX_REFRESH_FULL], &err);
>>>> + } else {
>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size,
>>>> &err);
>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>> >ctrl1_bypass_red_ram,
>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>> + ctrl2_tbl[SSD16XX_REFRESH_FULL], &err);
>>>> + }
>>>> + break;
>>>> + case SSD16XX_REFRESH_FAST:
>>>> + /*
>>>> + * Fast refresh: LUT pre-loaded during hw_init; BYPASS_RED_RAM
>>>> + * so RED RAM does not affect the current output.
>>>> + * Write RED RAM BEFORE display_update (same reasoning as
>>>> FULL)
>>>> + * so it holds the just-displayed frame as a valid
>>>> reference for
>>>> + * any subsequent PARTIAL refresh.
>>>> + */
>>>> +
>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>> + if (red_buffer) {
>>>> + /* 3-colour: write red channel before activating */
>>>> + ssd16xx_send_data_bulk(panel, red_buffer, data_size,
>>>> &err);
>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>> >ctrl1_normal,
>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>> + ctrl2_tbl[SSD16XX_REFRESH_FAST], &err);
>>>> + } else {
>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size,
>>>> &err);
>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>> >ctrl1_bypass_red_ram,
>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>> + ctrl2_tbl[SSD16XX_REFRESH_FAST], &err);
>>>> + }
>>>> + break;
>>>> + case SSD16XX_REFRESH_PARTIAL:
>>>> + default:
>>>> + /*
>>>> + * Partial refresh: both RAMs used for transition waveforms.
>>>> + * RED RAM must hold the PREVIOUS frame (= current display
>>>> + * content) so the controller can compute pixel transitions.
>>>> + * Write RED RAM AFTER display_update so it captures the
>>>> + * just-displayed frame as the reference for the next partial.
>>>> + */
>>>> + drm_dbg(&panel->drm,
>>>> + "fb_dirty: partial pre-update: mono[0]=0x%02x (BW=new,
>>>> RED=prev)\n",
>>>> + mono_buffer[0]);
>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>> >ctrl1_normal,
>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>> + ctrl2_tbl[SSD16XX_REFRESH_PARTIAL], &err);
>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size, &err);
>>>> + drm_dbg(&panel->drm,
>>>> + "fb_dirty: partial post-update: wrote RED baseline
>>>> mono[0]=0x%02x\n",
>>>> + mono_buffer[0]);
>>>> + break;
>>>> + }
>>>> +
>>>> + return err;
>>>> +}
>>>> +
>>>> +/*
>>>> -----------------------------------------------------------------------------
>>>> + * Plane Functions
>>>> + */
>>>> +
>>>> +static void ssd16xx_plane_destroy(struct drm_plane *plane)
>>>> +{
>>>> + drm_plane_cleanup(plane);
>>>> +}
>>>> +
>>>> +static void ssd16xx_plane_reset(struct drm_plane *plane)
>>>> +{
>>>> + drm_atomic_helper_plane_reset(plane);
>>>> +}
>>>
>>> Please avoid these wrappers.
>>>
>>
>> Understood, will update in V2.
>>
>>>> +
>>>> +static const struct drm_plane_funcs ssd16xx_plane_funcs = {
>>>> + .update_plane = drm_atomic_helper_update_plane,
>>>> + .disable_plane = drm_atomic_helper_disable_plane,
>>>> + .destroy = ssd16xx_plane_destroy,
>>>> + .reset = ssd16xx_plane_reset,
>>>> + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>>>> + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>>>> +};
>>>> +
>>>> +static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct drm_plane_state *new_plane_state =
>>>> + drm_atomic_get_new_plane_state(state, plane);
>>>> + struct drm_crtc_state *crtc_state;
>>>> +
>>>> + if (!new_plane_state->crtc)
>>>> + return 0;
>>>> +
>>>> + crtc_state = drm_atomic_get_new_crtc_state(state,
>>>> new_plane_state->crtc);
>>>> +
>>>> + return drm_atomic_helper_check_plane_state(new_plane_state,
>>>> crtc_state,
>>>> + DRM_PLANE_NO_SCALING,
>>>> + DRM_PLANE_NO_SCALING,
>>>> + false, false);
>>>> +}
>>>> +
>>>> +static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct drm_plane_state *old_state =
>>>> drm_atomic_get_old_plane_state(state, plane);
>>>> + struct drm_plane_state *new_state =
>>>> drm_atomic_get_new_plane_state(state, plane);
>>>> + struct ssd16xx_panel *panel = plane_to_ssd16xx_panel(plane);
>>>> + enum ssd16xx_refresh_mode saved_mode;
>>>> + u8 saved_border_waveform_idx;
>>>> + struct drm_framebuffer *fb = new_state->fb;
>>>> + struct drm_rect rect;
>>>> + int ret;
>>>> +
>>>> + drm_dbg(&panel->drm, "plane_atomic_update: fb=%p,
>>>> initialized=%d\n",
>>>> + fb, panel->initialized);
>>>> +
>>>> + if (!fb || !panel->initialized)
>>>> + return;
>>>> +
>>>> + /*
>>>> + * If a rotation change is pending, skip the update here —
>>>> crtc_atomic_flush
>>>> + * will re-init the hardware for the new orientation and redraw.
>>>> + */
>>>> + if (panel->reinit_pending) {
>>>> + drm_dbg(&panel->drm, "plane_atomic_update: skipping (reinit
>>>> pending)\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + if (!drm_atomic_helper_damage_merged(old_state, new_state,
>>>> &rect)) {
>>>> + rect.x1 = 0;
>>>> + rect.y1 = 0;
>>>> + rect.x2 = fb->width;
>>>> + rect.y2 = fb->height;
>>>> + drm_dbg(&panel->drm, "plane_atomic_update: no damage, using
>>>> full screen\n");
>>>> + }
>>>> +
>>>> + drm_dbg(&panel->drm, "plane_atomic_update: calling fb_dirty
>>>> rect=(%d,%d)-(%d,%d)\n",
>>>> + rect.x1, rect.y1, rect.x2, rect.y2);
>>>> + /*
>>>> + * When refresh_mode_init was set, use the specified mode for
>>>> this first
>>>> + * frame only, then restore the user-configured refresh_mode so
>>>> + * subsequent updates continue with the configured mode.
>>>> + */
>>>> + saved_mode = panel->refresh_mode;
>>>> + saved_border_waveform_idx = panel->border_waveform_update_idx;
>>>> + if (panel->init_refresh_pending) {
>>>> + panel->refresh_mode = panel->refresh_mode_init;
>>>> + panel->border_waveform_update_idx = panel-
>>>> >border_waveform_init_idx;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Fast refresh (0xC7) omits LOAD_LUT on every update cycle and
>>>> relies
>>>> + * on the LUT being pre-loaded upfront. The property setter arms
>>>> + * fast_lut_pending whenever the user switches into fast mode.
>>>> Consume
>>>> + * the flag here (once) before the first fast-refresh frame so the
>>>> + * controller's LUT is in the correct state.
>>>> + */
>>>> + if (panel->fast_lut_pending) {
>>>> + ret = ssd16xx_preload_fast_lut(panel);
>>>> + if (ret) {
>>>> + drm_err(&panel->drm,
>>>> + "plane_atomic_update: fast LUT preload failed:
>>>> %d\n", ret);
>>>> + }
>>>> +
>>>> + panel->fast_lut_pending = false;
>>>> + }
>>>> +
>>>> + ret = ssd16xx_fb_dirty(fb, &rect, panel);
>>>> + if (ret)
>>>> + drm_err(&panel->drm, "plane_atomic_update: display update
>>>> failed: %d\n", ret);
>>>> + else
>>>> + panel->last_fb = fb;
>>>> +
>>>> + panel->refresh_mode = saved_mode;
>>>> + panel->border_waveform_update_idx = saved_border_waveform_idx;
>>>> +
>>>> + /*
>>>> + * If this was the init frame (which used border_waveform_init_idx
>>>> + * inside fb_dirty), arm border_waveform_pending so the normal
>>>> + * (non-init) border value is sent at the start of the next
>>>> update.
>>>> + */
>>>> + if (panel->init_refresh_pending) {
>>>> + panel->init_refresh_pending = false;
>>>> + panel->border_waveform_pending = true;
>>>> + }
>>>> +}
>>>> +
>>>> +static const struct drm_plane_helper_funcs
>>>> ssd16xx_plane_helper_funcs = {
>>>> + .atomic_check = ssd16xx_plane_atomic_check,
>>>> + .atomic_update = ssd16xx_plane_atomic_update,
>>>> +};
>>>> +
>>>> +/*
>>>> -----------------------------------------------------------------------------
>>>> + * CRTC Functions
>>>> + */
>>>> +
>>>> +static void ssd16xx_crtc_destroy(struct drm_crtc *crtc)
>>>> +{
>>>> + drm_crtc_cleanup(crtc);
>>>> +}
>>>> +
>>>> +static const struct drm_crtc_funcs ssd16xx_crtc_funcs = {
>>>> + .reset = drm_atomic_helper_crtc_reset,
>>>> + .destroy = ssd16xx_crtc_destroy,
>>>> + .set_config = drm_atomic_helper_set_config,
>>>> + .page_flip = drm_atomic_helper_page_flip,
>>>> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>>>> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>>> +};
>>>> +
>>>> +static enum drm_mode_status ssd16xx_crtc_mode_valid(struct drm_crtc
>>>> *crtc,
>>>> + const struct drm_display_mode *mode)
>>>> +{
>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>> +
>>>> + /* Accept only our panel's native mode (landscape or portrait) */
>>>> + if ((mode->hdisplay == panel->mode->hdisplay &&
>>>> + mode->vdisplay == panel->mode->vdisplay) ||
>>>> + (mode->hdisplay == panel->mode->vdisplay &&
>>>> + mode->vdisplay == panel->mode->hdisplay))
>>>> + return MODE_OK;
>>>> +
>>>> + return MODE_BAD;
>>>> +}
>>>> +
>>>> +static int ssd16xx_crtc_atomic_check(struct drm_crtc *crtc,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>> + int ret, idx;
>>>> +
>>>> + if (!drm_dev_enter(&panel->drm, &idx))
>>>> + return;
>>>> +
>>>> + if (panel->clear_on_disable < 0 || panel-
>>>> >display_cleared_on_deinit)
>>>> + goto out;
>>>> +
>>>> + drm_dbg(&panel->drm, "clear_on_disable: running, mode=%d\n",
>>>> + panel->clear_on_disable);
>>>> + ret = ssd16xx_clear_display(panel,
>>>> + ssd16xx_refresh_mode_to_ctrl2(panel,
>>>> + panel->clear_on_disable));
>>>> + if (ret) {
>>>> + drm_err(&panel->drm, "atomic_disable: clear failed: %d\n",
>>>> ret);
>>>> + goto out;
>>>> + }
>>>> +
>>>> + panel->display_cleared_on_deinit = true;
>>>> +out:
>>>> + drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>> + struct drm_crtc_state *crtc_state =
>>>> drm_atomic_get_new_crtc_state(state, crtc);
>>>> + int ret, idx;
>>>> +
>>>> + if (!drm_dev_enter(&panel->drm, &idx))
>>>> + return;
>>>> +
>>>> + panel->display_cleared_on_deinit = false;
>>>> +
>>>> + drm_dbg(&panel->drm, "atomic_enable: %dx%d\n",
>>>> + crtc_state->mode.hdisplay, crtc_state->mode.vdisplay);
>>>> +
>>>> + panel->width = crtc_state->mode.hdisplay;
>>>> + panel->height = crtc_state->mode.vdisplay;
>>>> +
>>>> + ret = ssd16xx_hw_init(panel);
>>>> + if (ret) {
>>>> + drm_err(&panel->drm, "crtc_atomic_enable: HW init failed:
>>>> %d\n", ret);
>>>> + goto out;
>>>> + }
>>>> + panel->initialized = true;
>>>> +
>>>> + /* Clear display on first app launch if configured */
>>>> + ret = ssd16xx_clear_display_on_init(panel);
>>>> + if (ret)
>>>> + drm_err(&panel->drm, "crtc_atomic_enable: clear on init
>>>> failed: %d\n", ret);
>>>> +
>>>> + /*
>>>> + * If refresh_mode_init is set, arm init_refresh_pending so
>>>> + * plane_atomic_update uses the specified mode for the first frame
>>>> + * then restores the user-configured or panel default
>>>> refresh_mode.
>>>> + */
>>>> + if (panel->refresh_mode_init >= 0) {
>>>> + drm_dbg(&panel->drm,
>>>> + "atomic_enable: refresh_mode_init=%d, using for first
>>>> frame\n",
>>>> + panel->refresh_mode_init);
>>>> + panel->init_refresh_pending = true;
>>>> + }
>>>> +
>>>> +out:
>>>> + drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +/*
>>>> + * Re-initialize hardware and redraw the current framebuffer when the
>>>> + * display orientation changes at runtime via the rotation
>>>> connector property.
>>>> + * Called by the DRM atomic helper after atomic_enable/disable have
>>>> run.
>>>> + */
>>>> +static void ssd16xx_crtc_atomic_flush(struct drm_crtc *crtc,
>>>> + struct drm_atomic_state *state)
>>>> +{
>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>> + struct drm_framebuffer *fb;
>>>> + struct drm_rect full;
>>>> + int ret, idx;
>>>> +
>>>> + if (!panel->reinit_pending || !panel->initialized)
>>>> + return;
>>>> +
>>>> + if (!drm_dev_enter(&panel->drm, &idx))
>>>> + return;
>>>> +
>>>> + panel->reinit_pending = false;
>>>> +
>>>> + drm_dbg(&panel->drm, "atomic_flush: reinit, orientation=%u°\n",
>>>> + panel->orientation);
>>>> +
>>>> + ret = ssd16xx_hw_init(panel);
>>>> + if (ret) {
>>>> + drm_err(&panel->drm, "Orientation re-init failed: %d\n", ret);
>>>> + goto out;
>>>> + }
>>>> +
>>>> + fb = panel->primary_plane.state ? panel->primary_plane.state->fb
>>>> + : panel->last_fb;
>>>> + if (fb) {
>>>> + full.x1 = 0;
>>>> + full.y1 = 0;
>>>> + full.x2 = fb->width;
>>>> + full.y2 = fb->height;
>>>> + ret = ssd16xx_fb_dirty(fb, &full, panel);
>>>> + if (ret)
>>>> + drm_err(&panel->drm, "atomic_flush: display update
>>>> failed: %d\n", ret);
>>>> + else
>>>> + panel->last_fb = fb;
>>>> + }
>>>> +
>>>> +out:
>>>> + drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +static const struct drm_crtc_helper_funcs ssd16xx_crtc_helper_funcs
>>>> = {
>>>> + .mode_valid = ssd16xx_crtc_mode_valid,
>>>> + .atomic_check = ssd16xx_crtc_atomic_check,
>>>> + .atomic_disable = ssd16xx_crtc_atomic_disable,
>>>> + .atomic_enable = ssd16xx_crtc_atomic_enable,
>>>> + .atomic_flush = ssd16xx_crtc_atomic_flush,
>>>> +};
>>>> +
>>>> +/*
>>>> -----------------------------------------------------------------------------
>>>> + * Connector Functions
>>>> + */
>>>> +
>>>> +static int ssd16xx_connector_get_modes(struct drm_connector
>>>> *connector)
>>>> +{
>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(connector->dev);
>>>> + bool mode_is_portrait = (panel->mode->hdisplay < panel->mode-
>>>> >vdisplay);
>>>> + bool orient_is_portrait = (panel->orientation == 90 || panel-
>>>> >orientation == 270);
>>>> +
>>>> + drm_dbg(&panel->drm, "connector_get_modes: orientation=%u°\n",
>>>> + panel->orientation);
>>>> +
>>>> + /* For portrait, swap dimensions so clients see logical size. */
>>>> + if (mode_is_portrait != orient_is_portrait) {
>>>> + struct drm_display_mode *mode;
>>>> +
>>>> + mode = drm_mode_duplicate(&panel->drm, panel->mode);
>>>> + if (!mode)
>>>> + return 0;
>>>> + swap(mode->hdisplay, mode->vdisplay);
>>>> + swap(mode->hsync_start, mode->vsync_start);
>>>> + swap(mode->hsync_end, mode->vsync_end);
>>>> + swap(mode->htotal, mode->vtotal);
>>>> + swap(mode->width_mm, mode->height_mm);
>>>> + mode->type |= DRM_MODE_TYPE_PREFERRED;
>>>> + drm_mode_set_name(mode);
>>>> + drm_mode_probed_add(connector, mode);
>>>> + return 1;
>>>> + }
>>>> +
>>>> + return drm_connector_helper_get_modes_fixed(connector, panel-
>>>> >mode);
>>>> +}
>>>> +
>>>> +static const struct drm_connector_helper_funcs
>>>> ssd16xx_connector_helper_funcs = {
>>>> + .get_modes = ssd16xx_connector_get_modes,
>>>> +};
>>>> +
>>>> +/* Enum values for the rotation connector property (degrees
>>>> clockwise) */
>>>> +static const struct drm_prop_enum_list ssd16xx_rotation_enum[] = {
>>>> + { 0, "0" },
>>>> + { 90, "90" },
>>>> + { 180, "180" },
>>>> + { 270, "270" },
>>>> +};
>>>> +
>>>> +/* Enum values for the refresh_mode connector property */
>>>> +static const struct drm_prop_enum_list ssd16xx_refresh_mode_enum[] = {
>>>> + { SSD16XX_REFRESH_PARTIAL, "partial" },
>>>> + { SSD16XX_REFRESH_FULL, "full" },
>>>> + { SSD16XX_REFRESH_FAST, "fast" },
>>>> +};
>>>> +
>>>> +/*
>>>> + * Enum for clear_on_init, clear_on_close, refresh_mode_init
>>>> properties.
>>>> + * Value 0 = disabled; values 1-3 = partial/full/fast (refresh mode
>>>> + 1).
>>>> + * The +1 offset allows a single enum to represent both "disabled"
>>>> and the
>>>> + * three refresh modes without sign-extending the DRM property value.
>>>> + */
>>>> +static const struct drm_prop_enum_list ssd16xx_init_refresh_enum[] = {
>>>> + { 0, "disabled" },
>>>> + { 1, "partial" },
>>>> + { 2, "full" },
>>>> + { 3, "fast" },
>>>> +};
>>>> +
>>>> +/* Enum values for the color_mode connector property */
>>>> +static const struct drm_prop_enum_list ssd16xx_color_mode_enum[] = {
>>>> + { SSD16XX_COLOR_MODE_BW, "black-white" },
>>>> + { SSD16XX_COLOR_MODE_3COLOR, "3-color" },
>>>> +};
>>>> +
>>>> +/* Enum values for border_waveform connector properties (one per HW
>>>> mode) */
>>>> +static const struct drm_prop_enum_list
>>>> ssd16xx_border_waveform_enum[] = {
>>>> + { SSD16XX_BORDER_LUT0, "lut0_black" },
>>>> + { SSD16XX_BORDER_LUT1, "lut1_white" },
>>>> + { SSD16XX_BORDER_LUT2, "lut2_black" },
>>>> + { SSD16XX_BORDER_LUT3, "lut3_gray" },
>>>> + { SSD16XX_BORDER_VSS, "vss_black" },
>>>> + { SSD16XX_BORDER_VSH1, "vsh1_black" },
>>>> + { SSD16XX_BORDER_VSL, "vsl_white" },
>>>> + { SSD16XX_BORDER_VSH2, "vsh2_black" },
>>>> + { SSD16XX_BORDER_VCOM, "vcom_preserve" },
>>>> + { SSD16XX_BORDER_HIZ, "hiz_float" },
>>>> +};
>>>> +
>>>> +static int ssd16xx_connector_create_properties(struct ssd16xx_panel
>>>> *panel)
>>>> +{
>>>> + struct drm_device *drm = &panel->drm;
>>>> + struct drm_connector *connector = &panel->connector;
>>>> +
>>>> + panel->rotation_property =
>>>> + drm_property_create_enum(drm, 0, "rotation",
>>>> + ssd16xx_rotation_enum,
>>>> + ARRAY_SIZE(ssd16xx_rotation_enum));
>>>> + if (!panel->rotation_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->rotation_property, panel->orientation);
>>>> +
>>>> + panel->refresh_mode_property =
>>>> + drm_property_create_enum(drm, 0, "refresh_mode",
>>>> + ssd16xx_refresh_mode_enum,
>>>> + ARRAY_SIZE(ssd16xx_refresh_mode_enum));
>>>> + if (!panel->refresh_mode_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->refresh_mode_property, panel->refresh_mode);
>>>> +
>>>> + panel->border_waveform_init_property =
>>>> + drm_property_create_enum(drm, 0, "border_waveform_init",
>>>> + ssd16xx_border_waveform_enum,
>>>> + ARRAY_SIZE(ssd16xx_border_waveform_enum));
>>>> + if (!panel->border_waveform_init_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->border_waveform_init_property,
>>>> + panel->border_waveform_init_idx);
>>>> +
>>>> + panel->border_waveform_update_property =
>>>> + drm_property_create_enum(drm, 0, "border_waveform_update",
>>>> + ssd16xx_border_waveform_enum,
>>>> + ARRAY_SIZE(ssd16xx_border_waveform_enum));
>>>> + if (!panel->border_waveform_update_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->border_waveform_update_property,
>>>> + panel->border_waveform_update_idx);
>>>> +
>>>> + panel->border_refresh_on_every_update_property =
>>>> + drm_property_create_bool(drm, 0,
>>>> "border_refresh_on_every_update");
>>>> + if (!panel->border_refresh_on_every_update_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->border_refresh_on_every_update_property,
>>>> + panel->border_refresh_on_every_update);
>>>> +
>>>> + panel->clear_on_init_property =
>>>> + drm_property_create_enum(drm, 0, "clear_on_init",
>>>> + ssd16xx_init_refresh_enum,
>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>> + if (!panel->clear_on_init_property)
>>>> + return -ENOMEM;
>>>> + /* Property value 0=disabled, 1-3=mode; field is -1/0/1/2 → val
>>>> = field+1 */
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->clear_on_init_property,
>>>> + panel->clear_on_init + 1);
>>>> +
>>>> + panel->clear_on_close_property =
>>>> + drm_property_create_enum(drm, 0, "clear_on_close",
>>>> + ssd16xx_init_refresh_enum,
>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>> + if (!panel->clear_on_close_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->clear_on_close_property,
>>>> + panel->clear_on_close + 1);
>>>> +
>>>> + panel->clear_on_disable_property =
>>>> + drm_property_create_enum(drm, 0, "clear_on_disable",
>>>> + ssd16xx_init_refresh_enum,
>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>> + if (!panel->clear_on_disable_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->clear_on_disable_property,
>>>> + panel->clear_on_disable + 1);
>>>> +
>>>> + panel->refresh_mode_init_property =
>>>> + drm_property_create_enum(drm, 0, "refresh_mode_init",
>>>> + ssd16xx_init_refresh_enum,
>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>> + if (!panel->refresh_mode_init_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->refresh_mode_init_property,
>>>> + panel->refresh_mode_init + 1);
>>>> +
>>>> + /*
>>>> + * color_mode: only expose 3-color option on panels that
>>>> physically have
>>>> + * a red plane; on BW-only panels the property still exists for
>>>> + * consistency but userspace can only set "black-white".
>>>> + */
>>>> + panel->color_mode_property =
>>>> + drm_property_create_enum(drm, 0, "color_mode",
>>>> + ssd16xx_color_mode_enum,
>>>> + panel->panel_cfg->red_supported
>>>> + ? ARRAY_SIZE(ssd16xx_color_mode_enum)
>>>> + : 1);
>>>> + if (!panel->color_mode_property)
>>>> + return -ENOMEM;
>>>> + drm_object_attach_property(&connector->base,
>>>> + panel->color_mode_property,
>>>> + panel->color_mode);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int ssd16xx_connector_atomic_get_property(struct
>>>> drm_connector *connector,
>>>> + const struct drm_connector_state *state,
>>>> + struct drm_property *property,
>>>> + uint64_t *val)
>>>> +{
>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(connector->dev);
>>>> +
>>>> + drm_dbg(&panel->drm, "get_property: %s\n", property->name);
>>>> +
>>>> + if (property == panel->rotation_property) {
>>>> + *val = panel->orientation;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->refresh_mode_property) {
>>>> + *val = panel->refresh_mode;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->border_waveform_init_property) {
>>>> + *val = panel->border_waveform_init_idx;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->border_waveform_update_property) {
>>>> + *val = panel->border_waveform_update_idx;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->border_refresh_on_every_update_property) {
>>>> + *val = panel->border_refresh_on_every_update;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->clear_on_init_property) {
>>>> + *val = panel->clear_on_init + 1; /* field -1/0/1/2 → val
>>>> 0/1/2/3 */
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->clear_on_close_property) {
>>>> + *val = panel->clear_on_close + 1;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->clear_on_disable_property) {
>>>> + *val = panel->clear_on_disable + 1;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->refresh_mode_init_property) {
>>>> + *val = panel->refresh_mode_init + 1;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->color_mode_property) {
>>>> + *val = panel->color_mode;
>>>> + return 0;
>>>> + }
>>>> +
>>>> + return -EINVAL;
>>>> +}
>>>> +
>>>> +static int ssd16xx_connector_atomic_set_property(struct
>>>> drm_connector *connector,
>>>> + struct drm_connector_state *state,
>>>> + struct drm_property *property,
>>>> + uint64_t val)
>>>> +{
>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(connector->dev);
>>>> +
>>>> + drm_dbg(&panel->drm, "set_property: %s = %llu\n", property-
>>>> >name, val);
>>>> +
>>>> + if (property == panel->rotation_property) {
>>>> + if (val != 0 && val != 90 && val != 180 && val != 270)
>>>> + return -EINVAL;
>>>> + panel->orientation = val;
>>>> + /*
>>>> + * Flag hardware re-init needed. crtc_atomic_flush will call
>>>> + * ssd16xx_hw_init() with the new orientation and redraw.
>>>> + */
>>>> + panel->reinit_pending = true;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->refresh_mode_property) {
>>>> + if (val > SSD16XX_REFRESH_FAST)
>>>> + return -EINVAL;
>>>> + /*
>>>> + * Fast refresh (0xC7) omits LOAD_LUT on every update and
>>>> relies
>>>> + * on the LUT being pre-loaded upfront. Arm the one-shot flag
>>>> + * when switching into fast mode so the next
>>>> plane_atomic_update
>>>> + * loads the LUT before the first fast-refresh cycle.
>>>> Clear it
>>>> + * when switching away so a fresh pre-load happens if the user
>>>> + * returns to fast mode later.
>>>> + */
>>>> + if (val == SSD16XX_REFRESH_FAST &&
>>>> + panel->refresh_mode != SSD16XX_REFRESH_FULL)
>>>> + panel->fast_lut_pending = true;
>>>> + else
>>>> + panel->fast_lut_pending = false;
>>>> + panel->refresh_mode = val;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->border_waveform_init_property) {
>>>> + if (val >= ARRAY_SIZE(ssd1683_border_waveform_table))
>>>> + return -EINVAL;
>>>> + panel->border_waveform_init_idx = val;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->border_waveform_update_property) {
>>>> + const u8 *bw_tbl = panel->controller_cfg-
>>>> >border_waveform_table;
>>>> + bool changed = (int)val != panel->border_waveform_update_idx;
>>>> +
>>>> + if (val >= ARRAY_SIZE(ssd1683_border_waveform_table))
>>>> + return -EINVAL;
>>>> + drm_dbg(&panel->drm,
>>>> + "set_property: border_waveform_update old=%d new=%llu
>>>> hw=0x%02x -> 0x%02x %s\n",
>>>> + panel->border_waveform_update_idx, val,
>>>> + bw_tbl[panel->border_waveform_update_idx],
>>>> + bw_tbl[val],
>>>> + changed ? "(arming pending)" : "(no change)");
>>>> + /* Arm one-shot flag so the new border value is sent on the
>>>> very
>>>> + * next display update, even if
>>>> border_refresh_on_every_update is
>>>> + * not set. Cleared in fb_dirty after the command is sent.
>>>> + */
>>>> + if ((int)val != panel->border_waveform_update_idx)
>>>> + panel->border_waveform_pending = true;
>>>> + panel->border_waveform_update_idx = val;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->border_refresh_on_every_update_property) {
>>>> + panel->border_refresh_on_every_update = !!val;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->clear_on_init_property) {
>>>> + if (val > 3)
>>>> + return -EINVAL;
>>>> + panel->clear_on_init = (int)val - 1; /* val 0/1/2/3 →
>>>> field -1/0/1/2 */
>>>> + panel->first_clear_done = false; /* allow re-fire on next
>>>> enable */
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->clear_on_close_property) {
>>>> + if (val > 3)
>>>> + return -EINVAL;
>>>> + panel->clear_on_close = (int)val - 1;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->clear_on_disable_property) {
>>>> + if (val > 3)
>>>> + return -EINVAL;
>>>> + panel->clear_on_disable = (int)val - 1;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->refresh_mode_init_property) {
>>>> + if (val > 3)
>>>> + return -EINVAL;
>>>> + panel->refresh_mode_init = (int)val - 1;
>>>> + return 0;
>>>> + }
>>>> + if (property == panel->color_mode_property) {
>>>> + if (val > SSD16XX_COLOR_MODE_3COLOR)
>>>> + return -EINVAL;
>>>> + if (val == SSD16XX_COLOR_MODE_3COLOR && !panel->panel_cfg-
>>>> >red_supported) {
>>>> + drm_dbg(&panel->drm,
>>>> + "set_property: 3-color mode not supported by this
>>>> panel\n");
>>>> + return -EINVAL;
>>>> + }
>>>> + panel->color_mode = val;
>>>> + return 0;
>>>> + }
>>>> +
>>>> + return -EINVAL;
>>>> +}
>>>> +
>>>> +static const struct drm_connector_funcs ssd16xx_connector_funcs = {
>>>> + .reset = drm_atomic_helper_connector_reset,
>>>> + .fill_modes = drm_helper_probe_single_connector_modes,
>>>> + .destroy = drm_connector_cleanup,
>>>> + .atomic_duplicate_state =
>>>> drm_atomic_helper_connector_duplicate_state,
>>>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>>> + .atomic_get_property = ssd16xx_connector_atomic_get_property,
>>>> + .atomic_set_property = ssd16xx_connector_atomic_set_property,
>>>> +};
>>>> +
>>>> +static const u32 ssd16xx_formats[] = {
>>>> + DRM_FORMAT_XRGB8888, /* 32-bit RGB with padding (preferred) */
>>>> + DRM_FORMAT_RGB888, /* 24-bit packed RGB */
>>>> + DRM_FORMAT_RGB565, /* 16-bit RGB (5:6:5) */
>>>> + DRM_FORMAT_R8, /* 8-bit grayscale */
>>>> + DRM_FORMAT_NV12, /* YUV 4:2:0 planar */
>>>> + DRM_FORMAT_NV16, /* YUV 4:2:2 planar */
>>>> + DRM_FORMAT_YUYV, /* Packed YUV 4:2:2 (Y0 U0 Y1 V0) */
>>>> + DRM_FORMAT_UYVY, /* Packed YUV 4:2:2 (U0 Y0 V0 Y1) */
>>>> + DRM_FORMAT_R1, /* 1-bit monochrome (native, 8 pixels/
>>>> byte) */
>>>> +};
>>>
>>> Why do you have all these formats?
>>>
>>> Only export the modes your panel can do natively; plus maybe XRGB8888
>>> for compatibility.
>>>
>>
>> I wanted to keep YUV formats too since some apps such as camera apps
>> (in case we want to click a picture and display over on the e-paper
>> badge directly) support only YUV formats but yeah if it's too much I
>> can remove them from driver and instead have the conversion in the app
>> itself.
>>
>>>> +
>>>> +DEFINE_DRM_GEM_FOPS(ssd16xx_fops);
>>>> +
>>>> +/*
>>>> + * ssd16xx_drm_master_set - arm init refresh when a new master
>>>> takes control.
>>>> + */
>>>> +static void ssd16xx_drm_master_set(struct drm_device *drm,
>>>> + struct drm_file *file, bool from_open)
>>>> +{
>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(drm);
>>>> +
>>>> + panel->display_cleared_on_deinit = false;
>>>> + panel->first_clear_done = false;
>>>> +
>>>> + if (panel->refresh_mode_init >= 0)
>>>> + panel->init_refresh_pending = true;
>>>> +}
>>>> +
>>>> +/*
>>>> + * ssd16xx_drm_master_drop - clear display and disarm init refresh
>>>> when the
>>>> + * master client exits.
>>>> + */
>>>> +static void ssd16xx_drm_master_drop(struct drm_device *drm,
>>>> + struct drm_file *file)
>>>> +{
>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(drm);
>>>> + int ret;
>>>> +
>>>> + panel->init_refresh_pending = false;
>>>> + panel->first_clear_done = false;
>>>> +
>>>> + if (panel->clear_on_close < 0 || panel->display_cleared_on_deinit)
>>>> + return;
>>>> +
>>>> + ret = ssd16xx_clear_display_on_exit(panel);
>>>> + if (ret)
>>>> + drm_err(drm, "master_drop: clear on close failed: %d\n", ret);
>>>> +
>>>> + panel->display_cleared_on_deinit = true;
>>>> +}
>>>
>>> No, don't overload these. Just remove all this. Clearing should be
>>> left to the DRM client.
>>>
>>
>> Yes, the choice to clear or not to clear is left to drm client
>> depending on drm property setting done by drm client, the driver
>> clears the display. It would be difficult to update all different apps
>> to pass a blank white buffer to clear the screen and what if the app
>> gets closed abruptly (as master drop callback will get triggered),
>> then in that case the current driver logic ensures that screen gets
>> cleared. In normal LCD displays if app gets closed abruptly, the
>> display would have gone-off automatically as signals would stop
>> getting transmitted but in e-paper panel the last display context
>> would remain and I think it is driver responsibility to clear that if
>> that was the policy communicated by application to the driver.
>>
>>>> +
>>>> +static struct drm_driver ssd16xx_drm_driver = {
>>>> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>>> + .fops = &ssd16xx_fops,
>>>> + .name = "ssd16xx",
>>>> + .desc = "DRM driver for SSD16xx e-paper controller family",
>>>> + .major = 1,
>>>> + .minor = 0,
>>>> + .master_set = ssd16xx_drm_master_set,
>>>> + .master_drop = ssd16xx_drm_master_drop,
>>>> + DRM_GEM_DMA_DRIVER_OPS,
>>>> + DRM_FBDEV_DMA_DRIVER_OPS,
>>>> +};
>>>> +
>>>> +static const struct drm_mode_config_funcs ssd16xx_mode_config_funcs
>>>> = {
>>>> + .fb_create = drm_gem_fb_create_with_dirty,
>>>> + .atomic_check = drm_atomic_helper_check,
>>>> + .atomic_commit = drm_atomic_helper_commit,
>>>> +};
>>>> +
>>>> +/*
>>>> + * Use the RPM commit-tail variant so that
>>>> drm_atomic_helper_commit_modeset_enables
>>>> + * (which calls crtc_atomic_enable) runs before
>>>> drm_atomic_helper_commit_planes.
>>>> + * Without this, the standard commit_tail calls commit_planes before
>>>> + * modeset_enables, so plane_atomic_update would see initialized ==
>>>> false on the
>>>> + * first commit and silently drop the frame.
>>>> + */
>>>> +static const struct drm_mode_config_helper_funcs
>>>> ssd16xx_mode_config_helper_funcs = {
>>>> + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>>>> +};
>>>> +
>>>> +static int ssd16xx_alloc_tx_bufs(struct ssd16xx_panel *panel)
>>>> +{
>>>> + struct device *dev = &panel->spi->dev;
>>>> + size_t frame_size = (panel->controller_cfg->max_width *
>>>> + panel->controller_cfg->max_height) / 8;
>>>> +
>>>> + panel->tx_buf = devm_kmalloc(dev, frame_size, GFP_KERNEL);
>>>
>>> drmm_kmalloc() here and for the other buffers.
>>>
>>
>> Understood, thanks for pointing will fix it in V2.
>>
>> Best Regards
>> Devarsh
>>
>>> Best regards
>>> Thomas
>>
>>>
>>>> + if (!panel->tx_buf)
>>>> + return -ENOMEM;
>>>> +
>>>> + if (panel->panel_cfg->red_supported) {
>>>> + panel->tx_red_buf = devm_kmalloc(dev, frame_size, GFP_KERNEL);
>>>> + if (!panel->tx_red_buf)
>>>> + return -ENOMEM;
>>>> + }
>>>> +
>>>> + if (!panel->dc) {
>>>> + panel->tx_buf9 = devm_kmalloc_array(dev, frame_size,
>>>> + sizeof(u16), GFP_KERNEL);
>>>> + if (!panel->tx_buf9)
>>>> + return -ENOMEM;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int ssd16xx_probe(struct spi_device *spi)
>>>> +{
>>>> + struct device *dev = &spi->dev;
>>>> + struct ssd16xx_panel *panel;
>>>> + struct drm_device *drm;
>>>> + const struct spi_device_id *spi_id;
>>>> + struct drm_display_mode *mode;
>>>> + const void *match;
>>>> + enum ssd16xx_model model;
>>>> + u32 dt_rotation = 0;
>>>> + int ret;
>>>> +
>>>> + match = device_get_match_data(dev);
>>>> + if (match) {
>>>> + model = (enum ssd16xx_model)(uintptr_t)match;
>>>> + } else {
>>>> + spi_id = spi_get_device_id(spi);
>>>> + model = (enum ssd16xx_model)spi_id->driver_data;
>>>> + }
>>>> +
>>>> + if (!dev->coherent_dma_mask) {
>>>> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>>>> + if (ret) {
>>>> + dev_warn(dev, "Failed to set DMA mask: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +
>>>> + panel = devm_drm_dev_alloc(dev, &ssd16xx_drm_driver,
>>>> + struct ssd16xx_panel, drm);
>>>> + if (IS_ERR(panel))
>>>> + return PTR_ERR(panel);
>>>> +
>>>> + drm = &panel->drm;
>>>> + panel->spi = spi;
>>>> + panel->model = model;
>>>> + spi_set_drvdata(spi, panel);
>>>> +
>>>> + spi->mode = SPI_MODE_0;
>>>> + spi->bits_per_word = SSD16XX_SPI_BITS_PER_WORD;
>>>> +
>>>> + if (!spi->max_speed_hz) {
>>>> + drm_warn(drm, "spi-max-frequency not specified, using %u
>>>> Hz\n",
>>>> + SSD16XX_SPI_SPEED_DEFAULT);
>>>> + spi->max_speed_hz = SSD16XX_SPI_SPEED_DEFAULT;
>>>> + }
>>>> +
>>>> + ret = spi_setup(spi);
>>>> + if (ret < 0) {
>>>> + drm_err(drm, "SPI setup failed: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + switch (model) {
>>>> + case GDEY042T81:
>>>> + panel->controller = SSD1683;
>>>> + break;
>>>> + default:
>>>> + drm_err(drm, "Unknown panel model: %d\n", model);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + if (panel->controller >= ARRAY_SIZE(ssd16xx_controller_configs) ||
>>>> + !ssd16xx_controller_configs[panel->controller].max_width)
>>>> + return -EINVAL;
>>>> + panel->controller_cfg = &ssd16xx_controller_configs[panel-
>>>> >controller];
>>>> +
>>>> + if (model >= ARRAY_SIZE(ssd16xx_panel_configs))
>>>> + return -EINVAL;
>>>> + panel->panel_cfg = &ssd16xx_panel_configs[model];
>>>> +
>>>> + mode = devm_kmemdup(dev, panel->panel_cfg->mode,
>>>> + sizeof(*panel->panel_cfg->mode), GFP_KERNEL);
>>>> + if (!mode)
>>>> + return -ENOMEM;
>>>> +
>>>> + panel->refresh_mode = panel->panel_cfg->default_refresh_mode;
>>>> + /* Default color mode: 3-color for panels with red plane, BW
>>>> otherwise */
>>>> + panel->color_mode = panel->panel_cfg->red_supported
>>>> + ? SSD16XX_COLOR_MODE_3COLOR
>>>> + : SSD16XX_COLOR_MODE_BW;
>>>> + panel->border_waveform_init_idx = panel->panel_cfg-
>>>> >default_border_waveform_init;
>>>> + panel->border_waveform_update_idx = panel->panel_cfg-
>>>> >default_border_waveform_update;
>>>> + panel->border_refresh_on_every_update =
>>>> + panel->panel_cfg->default_border_refresh_on_every_update;
>>>> + panel->clear_on_init = panel->panel_cfg->default_clear_on_init;
>>>> + panel->clear_on_close = panel->panel_cfg-
>>>> >default_clear_on_close;
>>>> + panel->clear_on_disable = panel->panel_cfg-
>>>> >default_clear_on_disable;
>>>> + panel->refresh_mode_init = panel->panel_cfg-
>>>> >default_refresh_mode_init;
>>>> +
>>>> + /* Module parameter overrides for border/display control */
>>>> + if (border_waveform_init_lut >= 0 &&
>>>> + border_waveform_init_lut <
>>>> (int)ARRAY_SIZE(ssd1683_border_waveform_table))
>>>> + panel->border_waveform_init_idx = border_waveform_init_lut;
>>>> + if (border_waveform_lut >= 0 &&
>>>> + border_waveform_lut <
>>>> (int)ARRAY_SIZE(ssd1683_border_waveform_table))
>>>> + panel->border_waveform_update_idx = border_waveform_lut;
>>>> + if (border_refresh_on_every_update)
>>>> + panel->border_refresh_on_every_update = true;
>>>> + if (clear_on_init >= 0 && clear_on_init <= 2)
>>>> + panel->clear_on_init = clear_on_init;
>>>> + if (clear_on_close >= 0 && clear_on_close <= 2)
>>>> + panel->clear_on_close = clear_on_close;
>>>> + if (clear_on_disable >= 0 && clear_on_disable <= 2)
>>>> + panel->clear_on_disable = clear_on_disable;
>>>> + if (refresh_mode_init >= 0 && refresh_mode_init <= 2)
>>>> + panel->refresh_mode_init = refresh_mode_init;
>>>> +
>>>> + /* Module parameter overrides panel default refresh mode when
>>>> set */
>>>> + if (refresh_mode >= 0) {
>>>> + if (refresh_mode > SSD16XX_REFRESH_FAST)
>>>> + drm_warn(drm, "Invalid refresh_mode module param %d,
>>>> ignored\n",
>>>> + refresh_mode);
>>>> + else
>>>> + panel->refresh_mode = refresh_mode;
>>>> + }
>>>> +
>>>> + /* Module parameter overrides panel default color mode when set */
>>>> + if (color_mode >= 0) {
>>>> + if (color_mode > SSD16XX_COLOR_MODE_3COLOR)
>>>> + drm_warn(drm, "Invalid color_mode module param %d,
>>>> ignored\n",
>>>> + color_mode);
>>>> + else if (color_mode == SSD16XX_COLOR_MODE_3COLOR &&
>>>> + !panel->panel_cfg->red_supported)
>>>> + drm_warn(drm,
>>>> + "color_mode=3-color requested but panel has no red
>>>> plane, ignored\n");
>>>> + else
>>>> + panel->color_mode = color_mode;
>>>> + }
>>>> +
>>>> + /* Parse "rotation" DT property; swap mode dimensions for
>>>> portrait. */
>>>> + device_property_read_u32(dev, "rotation", &dt_rotation);
>>>> + if (dt_rotation != 0 && dt_rotation != 90 && dt_rotation != 180
>>>> && dt_rotation != 270) {
>>>> + drm_warn(drm, "Invalid DT rotation %u, defaulting to 0°\n",
>>>> dt_rotation);
>>>> + dt_rotation = 0;
>>>> + }
>>>> + panel->orientation = dt_rotation;
>>>> +
>>>> + /* Module parameter overrides DT rotation when set */
>>>> + if (rotation >= 0) {
>>>> + if (rotation != 0 && rotation != 90 && rotation != 180 &&
>>>> rotation != 270)
>>>> + drm_warn(drm, "Invalid rotation module param %d,
>>>> ignored\n",
>>>> + rotation);
>>>> + else
>>>> + panel->orientation = rotation;
>>>> + }
>>>> +
>>>> + drm_dbg(drm, "Using %s orientation (%u°, %ux%u logical)\n",
>>>> + (panel->orientation == 90 || panel->orientation == 270) ?
>>>> "portrait" : "landscape",
>>>> + panel->orientation, mode->hdisplay, mode->vdisplay);
>>>> +
>>>> + /* Swap mode dimensions for portrait so clients see logical
>>>> size. */
>>>> + if (panel->orientation == 90 || panel->orientation == 270) {
>>>> + swap(mode->hdisplay, mode->vdisplay);
>>>> + swap(mode->hsync_start, mode->vsync_start);
>>>> + swap(mode->hsync_end, mode->vsync_end);
>>>> + swap(mode->htotal, mode->vtotal);
>>>> + swap(mode->width_mm, mode->height_mm);
>>>> + drm_dbg(drm, "Mode dimensions swapped for portrait: %ux%u\n",
>>>> + mode->hdisplay, mode->vdisplay);
>>>> + } else {
>>>> + drm_dbg(drm, "Mode dimensions unchanged: %ux%u\n",
>>>> + mode->hdisplay, mode->vdisplay);
>>>> + }
>>>> + panel->mode = mode;
>>>> + panel->width = mode->hdisplay;
>>>> + panel->height = mode->vdisplay;
>>>> +
>>>> + /* Acquire GPIOs. */
>>>> + panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>>>> + if (IS_ERR(panel->reset))
>>>> + return dev_err_probe(dev, PTR_ERR(panel->reset), "Failed to
>>>> get RESET GPIO\n");
>>>> +
>>>> + panel->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
>>>> + if (IS_ERR(panel->busy))
>>>> + return dev_err_probe(dev, PTR_ERR(panel->busy), "Failed to
>>>> get BUSY GPIO\n");
>>>> +
>>>> + panel->dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
>>>> + if (IS_ERR(panel->dc))
>>>> + return dev_err_probe(dev, PTR_ERR(panel->dc), "Failed to
>>>> get DC GPIO\n");
>>>> + if (!panel->dc) {
>>>> + if (!spi_is_bpw_supported(spi, 9))
>>>> + return dev_err_probe(dev, -EINVAL,
>>>> + "3-wire SPI mode requires 9-bit word
>>>> support\n");
>>>> + drm_dbg(drm, "dc-gpios not specified, using 3-wire (9-bit)
>>>> SPI mode\n");
>>>> + }
>>>> +
>>>> + ret = ssd16xx_alloc_tx_bufs(panel);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ssd16xx_hw_reset(panel);
>>>> +
>>>> + ret = drmm_mode_config_init(drm);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + drm->mode_config.funcs = &ssd16xx_mode_config_funcs;
>>>> + drm->mode_config.helper_private =
>>>> &ssd16xx_mode_config_helper_funcs;
>>>> + drm->mode_config.min_width = min(panel->width, panel->height);
>>>> + drm->mode_config.max_width = max(panel->width, panel->height);
>>>> + drm->mode_config.min_height = min(panel->width, panel->height);
>>>> + drm->mode_config.max_height = max(panel->width, panel->height);
>>>> +
>>>> + drm_connector_helper_add(&panel->connector,
>>>> &ssd16xx_connector_helper_funcs);
>>>> + ret = drm_connector_init(drm, &panel->connector,
>>>> &ssd16xx_connector_funcs,
>>>> + DRM_MODE_CONNECTOR_SPI);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = drm_universal_plane_init(drm, &panel->primary_plane, 0,
>>>> + &ssd16xx_plane_funcs,
>>>> + ssd16xx_formats, ARRAY_SIZE(ssd16xx_formats),
>>>> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
>>>> + if (ret)
>>>> + return ret;
>>>> + drm_plane_helper_add(&panel->primary_plane,
>>>> &ssd16xx_plane_helper_funcs);
>>>> + drm_plane_enable_fb_damage_clips(&panel->primary_plane);
>>>> +
>>>> + ret = drm_crtc_init_with_planes(drm, &panel->crtc, &panel-
>>>> >primary_plane,
>>>> + NULL, &ssd16xx_crtc_funcs, NULL);
>>>> + if (ret)
>>>> + return ret;
>>>> + drm_crtc_helper_add(&panel->crtc, &ssd16xx_crtc_helper_funcs);
>>>> +
>>>> + ret = drm_simple_encoder_init(drm, &panel->encoder,
>>>> DRM_MODE_ENCODER_NONE);
>>>> + if (ret)
>>>> + return ret;
>>>> + panel->encoder.possible_crtcs = drm_crtc_mask(&panel->crtc);
>>>> +
>>>> + ret = drm_connector_attach_encoder(&panel->connector, &panel-
>>>> >encoder);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = ssd16xx_connector_create_properties(panel);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + drm_mode_config_reset(drm);
>>>> +
>>>> + ret = drm_dev_register(drm, 0);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + drm_dbg(drm, "SSD16xx e-paper display initialized (%dx%d, %d°
>>>> rotation)\n",
>>>> + panel->width, panel->height, panel->orientation);
>>>> +
>>>> + drm_client_setup(drm, NULL);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void ssd16xx_remove(struct spi_device *spi)
>>>> +{
>>>> + struct ssd16xx_panel *panel = spi_get_drvdata(spi);
>>>> +
>>>> + drm_dev_unplug(&panel->drm);
>>>> + drm_atomic_helper_shutdown(&panel->drm);
>>>> +}
>>>> +
>>>> +static void ssd16xx_shutdown(struct spi_device *spi)
>>>> +{
>>>> + struct ssd16xx_panel *panel = spi_get_drvdata(spi);
>>>> +
>>>> + drm_atomic_helper_shutdown(&panel->drm);
>>>> +}
>>>> +
>>>> +static const struct of_device_id ssd16xx_of_match[] = {
>>>> + { .compatible = "gooddisplay,gdey042t81", .data = (void
>>>> *)GDEY042T81 },
>>>> + { }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, ssd16xx_of_match);
>>>> +
>>>> +static const struct spi_device_id ssd16xx_id[] = {
>>>> + { "gdey042t81", GDEY042T81 },
>>>> + { }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(spi, ssd16xx_id);
>>>> +
>>>> +static struct spi_driver ssd16xx_spi_driver = {
>>>> + .driver = {
>>>> + .name = "ssd16xx",
>>>> + .of_match_table = ssd16xx_of_match,
>>>> + },
>>>> + .probe = ssd16xx_probe,
>>>> + .remove = ssd16xx_remove,
>>>> + .shutdown = ssd16xx_shutdown,
>>>> + .id_table = ssd16xx_id,
>>>> +};
>>>> +module_spi_driver(ssd16xx_spi_driver);
>>>> +
>>>> +MODULE_AUTHOR("Devarsh Thakkar <devarsht@ti.com>");
>>>> +MODULE_DESCRIPTION("DRM driver for Solomon SSD16xx e-paper display
>>>> controller family");
>>>> +MODULE_LICENSE("GPL");
>>>
>>
>
^ permalink raw reply
* Re: [PATCH v4 1/3] perf: marvell: Add MPAM partid filtering to CN10K TAD PMU
From: sashiko-bot @ 2026-06-18 15:50 UTC (permalink / raw)
To: Geetha sowjanya; +Cc: linux-perf-users, robh, devicetree, conor+dt
In-Reply-To: <20260618153610.13649-2-gakula@marvell.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `perf_ready` early return in `tad_pmu_offline_cpu()` completely skips updating `pmu->cpu`, potentially pinning the PMU to an offline CPU permanently.
- [Medium] The commit message contradicts the implementation regarding the registration order of the PMU and CPU hotplug instance.
--
commit c5919d39926621f8d8420dda8d78271eeeb122e2
Author: Tanmay Jagdale <tanmay@marvell.com>
Subject: perf: marvell: Add MPAM partid filtering to CN10K TAD PMU
This commit adds MPAM partition ID filtering to the CN10K TAD PMU. It updates
the counter programming to use MPAM-capable filtering for specific events. It
also modifies the probe function's MMIO mapping and attempts to address
hotplug registration races.
> Probe no longer mutates the platform_device MMIO resource (walk a local
> map_start), rejects tad-cnt / page sizes of zero, validates the memory
> window against tad-cnt, and registers the perf PMU before hotplug with
> correct unwind.
[Severity: Medium]
The commit message states that it "registers the perf PMU before hotplug
with correct unwind", but looking at tad_pmu_probe(), the hotplug instance
is still registered before the perf PMU:
ret = cpuhp_state_add_instance_nocalls(tad_pmu_cpuhp_state,
&tad_pmu->node);
if (ret) {
dev_err(&pdev->dev, "Error %d registering hotplug\n", ret);
return ret;
}
ret = perf_pmu_register(&tad_pmu->pmu, "tad", -1);
Could the commit message be updated, or was the code reordering missed?
> diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c b/drivers/perf/marvell_cn10k_tad_pmu.c
> index 51ccb0befa05d..340be3776fe75 100644
> --- a/drivers/perf/marvell_cn10k_tad_pmu.c
> +++ b/drivers/perf/marvell_cn10k_tad_pmu.c
[ ... ]
> @@ -451,6 +570,9 @@ static int tad_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> struct tad_pmu *pmu = hlist_entry_safe(node, struct tad_pmu, node);
> unsigned int target;
>
> + if (!READ_ONCE(pmu->perf_ready))
> + return 0;
> +
[Severity: High]
If the originally assigned CPU goes offline during the race window between
cpuhp_state_add_instance_nocalls() and the setting of perf_ready = true in
tad_pmu_probe(), does this early return skip reassigning pmu->cpu?
Since this driver does not define an online CPU hotplug callback, if
tad_pmu_offline_cpu() returns early here, could pmu->cpu permanently remain
assigned to the offline CPU and break the PMU once registration finishes?
> if (cpu != pmu->cpu)
> return 0;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618153610.13649-1-gakula@marvell.com?part=1
^ permalink raw reply
* Re: [PATCH 3/3] dt-bindings: spi: nxp,imx94-xspi: add DMA properties
From: Mark Brown @ 2026-06-18 15:54 UTC (permalink / raw)
To: han.xu
Cc: Han Xu, Haibo Chen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, linux-spi, imx, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260617215520.3327836-3-han.xu@oss.nxp.com>
[-- Attachment #1: Type: text/plain, Size: 539 bytes --]
On Wed, Jun 17, 2026 at 04:55:19PM -0500, han.xu@oss.nxp.com wrote:
> From: Han Xu <han.xu@nxp.com>
>
> Add dmas and dma-names to describe TX and RX DMA channels for the i.MX94
> XSPI controller.
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
* [PATCH v2 0/2] iio: magnetometer: add support for Melexis MLX90393
From: Nikhil Gautam @ 2026-06-18 16:01 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
devicetree, linux-kernel, Nikhil Gautam
Hi,
This series adds initial Industrial I/O subsystem support for the
Melexis MLX90393 3-axis magnetometer and temperature sensor.
The MLX90393 supports both I2C and SPI interfaces. This series
implements support for the I2C interface while keeping the driver
structure transport-independent to simplify future SPI support.
Currently supported features:
* Raw magnetic field measurements for X/Y/Z axes
* Raw temperature measurements
* Configurable gain/scale selection
* Configurable oversampling ratio
* Direct mode operation through the IIO subsystem
* I2C interface support
The driver has been tested on Raspberry Pi 5 hardware using an
MLX90393 sensor connected over I2C. Magnetic field and temperature
measurements were verified through the IIO sysfs interface.
Previous Submission:
Link: https://lore.kernel.org/linux-iio/20260510191010.155380-1-nikhilgtr@gmail.com/
Changes in v2:
[DT]
- Extended the DT binding to document power supply regulators and
optional interrupt and trigger GPIOs.
[IIO]
- Removed the RFC tag based on reviewer feedback.
- Added a MAINTAINERS entry as part of the initial submission and
expanded it in the driver patch.
- Reworked the scale availability implementation to simplify the
data layout and eliminate the need for constructing a temporary
table, avoiding potential race conditions.
- Replaced usleep_range() with fsleep() where appropriate and
documented initialization delays.
- Simplified helper functions and improved error handling by
returning directly where appropriate.
- Reduced unnecessary local variables and line wrapping to improve
readability and align with kernel coding style.
- Added comments for lock protection and command definitions to
improve code clarity.
- Switched to devm_mutex_init() and cleaned up include usage in
accordance with the "include what you use" principle.
- Improved consistency across the driver, including conditional
handling, switch statements, formatting, and general code style.
- Addressed all review comments from Jonathan Cameron.
I would like to thank Jonathan Cameron for the prompt and thorough
review of the previous revision. The detailed feedback on both the
submission process and the implementation has significantly improved
the quality and maintainability of this series.
Further review and comments are greatly appreciated.
Thanks,
Nikhil Gautam
Nikhil Gautam (2):
dt-bindings: iio: magnetometer: add Melexis MLX90393
iio: magnetometer: add support for Melexis MLX90393
.../iio/magnetometer/melexis,mlx90393.yaml | 55 ++
MAINTAINERS | 7 +
drivers/iio/magnetometer/Kconfig | 10 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/mlx90393.h | 74 ++
drivers/iio/magnetometer/mlx90393_core.c | 681 ++++++++++++++++++
drivers/iio/magnetometer/mlx90393_i2c.c | 72 ++
7 files changed, 901 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
create mode 100644 drivers/iio/magnetometer/mlx90393.h
create mode 100644 drivers/iio/magnetometer/mlx90393_core.c
create mode 100644 drivers/iio/magnetometer/mlx90393_i2c.c
--
2.39.5
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393
From: Nikhil Gautam @ 2026-06-18 16:01 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
devicetree, linux-kernel, Nikhil Gautam
In-Reply-To: <20260618160141.11409-1-nikhilgtr@gmail.com>
Add devicetree bindings for the Melexis MLX90393
3-axis magnetometer and temperature sensor.
The device supports magnetic field and temperature
measurements over I2C and SPI interfaces.
This initial binding documents the I2C interface.
Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
.../iio/magnetometer/melexis,mlx90393.yaml | 55 +++++++++++++++++++
MAINTAINERS | 6 ++
2 files changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
diff --git a/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
new file mode 100644
index 000000000000..79e7e4a124b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/melexis,mlx90393.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Melexis MLX90393 magnetometer sensor
+
+maintainers:
+ - Nikhil Gautam <nikhilgtr@gmail.com>
+
+description:
+ Melexis MLX90393 3-axis magnetometer and temperature sensor.
+
+properties:
+ compatible:
+ const: melexis,mlx90393
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+ vddio-supply: true
+
+ interrupts:
+ maxItems: 1
+
+ trigger-gpios:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ magnetometer@c {
+ compatible = "melexis,mlx90393";
+ reg = <0x0c>;
+
+ interrupt-parent = <&gpio>;
+ interrupts = <17 IRQ_TYPE_EDGE_RISING>;
+
+ trigger-gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa16..e9ddcd12feb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24926,6 +24926,12 @@ S: Maintained
F: Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
F: drivers/iio/magnetometer/tmag5273.c
+MELEXIS MLX90393 MAGNETOMETER DRIVER
+M: Nikhil Gautam <nikhilgtr@gmail.com>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
+
TI TRF7970A NFC DRIVER
M: Mark Greer <mgreer@animalcreek.com>
L: linux-wireless@vger.kernel.org
--
2.39.5
^ permalink raw reply related
* [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393
From: Nikhil Gautam @ 2026-06-18 16:01 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
devicetree, linux-kernel, Nikhil Gautam
In-Reply-To: <20260618160141.11409-1-nikhilgtr@gmail.com>
Add Industrial I/O subsystem support for the Melexis
MLX90393 3-axis magnetometer and temperature sensor.
The driver currently supports:
raw magnetic field measurements
raw temperature measurements
configurable gain/scale selection
configurable oversampling ratio
direct mode operation
The MLX90393 supports both I2C and SPI interfaces. This
initial implementation adds support for the I2C interface.
The driver is structured around a shared sensor core with
a small transport abstraction layer to simplify future SPI
support without duplicating sensor logic.
Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
MAINTAINERS | 1 +
drivers/iio/magnetometer/Kconfig | 10 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/mlx90393.h | 74 +++
drivers/iio/magnetometer/mlx90393_core.c | 681 +++++++++++++++++++++++
drivers/iio/magnetometer/mlx90393_i2c.c | 72 +++
6 files changed, 840 insertions(+)
create mode 100644 drivers/iio/magnetometer/mlx90393.h
create mode 100644 drivers/iio/magnetometer/mlx90393_core.c
create mode 100644 drivers/iio/magnetometer/mlx90393_i2c.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e9ddcd12feb5..ef7eb6fec0c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24931,6 +24931,7 @@ M: Nikhil Gautam <nikhilgtr@gmail.com>
L: linux-iio@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
+F: drivers/iio/magnetometer/mlx90393*
TI TRF7970A NFC DRIVER
M: Mark Greer <mgreer@animalcreek.com>
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 3debf1320ad1..e6b74e7e3317 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -128,6 +128,16 @@ config HID_SENSOR_MAGNETOMETER_3D
Say yes here to build support for the HID SENSOR
Magnetometer 3D.
+config MLX90393
+ tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
+ depends on I2C
+ help
+ Say yes here to build support for the MELEXIS MLX90393 3-axis
+ magnetometer.
+
+ To compile this driver as a module, choose M here: the module
+ will be called mlx90393.
+
config MMC35240
tristate "MEMSIC MMC35240 3-axis magnetic sensor"
select REGMAP_I2C
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 9297723a97d8..542c89d38a59 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -14,6 +14,8 @@ obj-$(CONFIG_BMC150_MAGN_SPI) += bmc150_magn_spi.o
obj-$(CONFIG_MAG3110) += mag3110.o
obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
+obj-$(CONFIG_MLX90393) += mlx90393_core.o
+obj-$(CONFIG_MLX90393) += mlx90393_i2c.o
obj-$(CONFIG_MMC35240) += mmc35240.o
obj-$(CONFIG_IIO_ST_MAGN_3AXIS) += st_magn.o
diff --git a/drivers/iio/magnetometer/mlx90393.h b/drivers/iio/magnetometer/mlx90393.h
new file mode 100644
index 000000000000..b3356f9521f8
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * MLX90393 magnetometer & temperature sensor driver
+ *
+ * Copyright (c) 2026 Nikhil Gautam <nikhilgtr@gmail.com>
+ */
+
+#ifndef MLX90393_H
+#define MLX90393_H
+
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/types.h>
+
+#define MLX90393_AXIS_MAX 2
+#define MLX90393_GAIN_MAX 8
+#define MLX90393_RES_MAX 4
+#define MLX90393_OSR2_MAX 4
+#define MLX90393_OSR_MAX 4
+
+#define MLX90393_CMD_MASK GENMASK(7, 4)
+
+/* Commands (datasheet, Table 11 - Command List) */
+#define MLX90393_CMD_SB 0x10 /* Start Burst Mode */
+#define MLX90393_CMD_SW 0x20 /* Start Wake-up on Change Mode */
+#define MLX90393_CMD_SM 0x30 /* Start Single Measurement Mode */
+#define MLX90393_CMD_RM 0x40 /* Read Measurement */
+#define MLX90393_CMD_RR 0x50 /* Read Register */
+#define MLX90393_CMD_WR 0x60 /* Write Register */
+#define MLX90393_CMD_EX 0x80 /* Exit Mode */
+#define MLX90393_CMD_HR 0xD0 /* Memory Recall */
+#define MLX90393_CMD_HS 0xE0 /* Memory Store */
+#define MLX90393_CMD_RT 0xF0 /* Reset Device */
+
+#define MLX90393_MEASURE_Z BIT(0)
+#define MLX90393_MEASURE_Y BIT(1)
+#define MLX90393_MEASURE_X BIT(2)
+#define MLX90393_MEASURE_TEMP BIT(3)
+
+#define MLX90393_MEASURE_ALL \
+ (MLX90393_MEASURE_TEMP | MLX90393_MEASURE_X | \
+ MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)
+
+#define MLX90393_NUM_CHANNELS 4
+
+#define MLX90393_STATUS_RESP GENMASK(1, 0)
+#define MLX90393_STATUS_RT BIT(2)
+#define MLX90393_STATUS_ERROR BIT(4)
+
+#define MLX90393_REG_CONF1 0x00
+#define MLX90393_REG_CONF2 0x01
+#define MLX90393_REG_CONF3 0x02
+#define MLX90393_REG_CONF4 0x03
+
+#define MLX90393_CONF1_GAIN_SEL GENMASK(6, 4)
+#define MLX90393_CONF1_HALLCONF GENMASK(3, 0)
+
+#define MLX90393_CONF3_OSR GENMASK(1, 0)
+#define MLX90393_CONF3_DIG_FILT GENMASK(4, 2)
+#define MLX90393_CONF3_RES_X GENMASK(6, 5)
+#define MLX90393_CONF3_RES_Y GENMASK(8, 7)
+#define MLX90393_CONF3_RES_Z GENMASK(10, 9)
+#define MLX90393_CONF3_OSR2 GENMASK(12, 11)
+
+struct mlx90393_transfer_ops {
+ int (*xfer)(void *context, const u8 *tx, int tx_len,
+ u8 *rx, int rx_len);
+};
+
+int mlx90393_core_probe(struct device *dev,
+ const struct mlx90393_transfer_ops *ops,
+ void *context);
+
+#endif
diff --git a/drivers/iio/magnetometer/mlx90393_core.c b/drivers/iio/magnetometer/mlx90393_core.c
new file mode 100644
index 000000000000..0ad4a30c0be9
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393_core.c
@@ -0,0 +1,681 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MLX90393 magnetometer & temperature sensor driver
+ *
+ * Copyright (c) 2026 Nikhil Gautam <nikhilgtr@gmail.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+#include "mlx90393.h"
+
+struct mlx90393_data {
+ /* Protects sensor configuration and measurement operations */
+ struct mutex lock;
+ struct device *dev;
+ void *bus_context;
+ const struct mlx90393_transfer_ops *ops;
+ u8 gain_sel;
+ u8 hallconf;
+
+ u8 res_xy;
+ u8 res_z;
+
+ u8 dig_filt;
+ u8 osr;
+ u8 osr2;
+};
+
+enum mlx90393_channels {
+ MLX90393_CHAN_X,
+ MLX90393_CHAN_Y,
+ MLX90393_CHAN_Z,
+ MLX90393_CHAN_TEMP,
+};
+
+enum mlx90393_axis_type {
+ MLX90393_AXIS_TYPE_XY,
+ MLX90393_AXIS_TYPE_Z,
+};
+
+/* Datasheet: Table no.17 */
+static const int mlx90393_scale_table[MLX90393_AXIS_MAX]
+ [MLX90393_GAIN_MAX]
+ [MLX90393_RES_MAX] = {
+ /* XY axis */
+ {
+ { 751, 1502, 3004, 6009},
+ { 601, 1202, 2403, 4840},
+ { 451, 901, 1803, 3605},
+ { 376, 751, 1502, 3004},
+ { 300, 601, 1202, 2403},
+ { 250, 501, 1001, 2003},
+ { 200, 401, 801, 1602},
+ { 150, 300, 601, 1202},
+ },
+ /* Z axis */
+ {
+ { 1210, 2420, 4840, 9680},
+ { 968, 1936, 3872, 7744},
+ { 726, 1452, 2904, 5808},
+ { 605, 1210, 2420, 4840},
+ { 484, 968, 1936, 3872},
+ { 403, 807, 1613, 3227},
+ { 323, 645, 1291, 2581},
+ { 242, 484, 968, 1936},
+ }
+};
+
+static const int mlx90393_osr2_avail[MLX90393_OSR2_MAX] = {
+ 0, 1, 2, 3,
+};
+
+static const int mlx90393_osr_avail[MLX90393_OSR_MAX] = {
+ 1, 2, 4, 8,
+};
+
+#define MLX90393_CHAN(idx, axis, addr) { \
+ .type = IIO_MAGN, \
+ .modified = 1, \
+ .channel = idx, \
+ .address = addr, \
+ .channel2 = IIO_MOD_##axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\
+ .info_mask_separate_available = \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_type_available = \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+}
+
+static const struct iio_chan_spec mlx90393_channels[] = {
+ MLX90393_CHAN(0, X, MLX90393_CHAN_X),
+ MLX90393_CHAN(1, Y, MLX90393_CHAN_Y),
+ MLX90393_CHAN(2, Z, MLX90393_CHAN_Z),
+ {
+ .type = IIO_TEMP,
+ .address = MLX90393_CHAN_TEMP,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .info_mask_separate_available =
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+/*
+ * Calculate total conversion time in microseconds.
+ *
+ * Formula derived from datasheet timing equations.
+ */
+
+static int mlx90393_get_tconv_us(struct mlx90393_data *data)
+{
+ const int osr = data->osr;
+ const int osr2 = data->osr2;
+ const int df = data->dig_filt;
+
+ int tconvm;
+ int tconvt;
+
+ int m = 3; /* X,Y,Z */
+
+ /*
+ * Datasheet:
+ * TCONVM = 67 + 64 * 2^OSR * (2 + 2^DIG_FILT)
+ */
+ tconvm = 67 + (64 * BIT(osr) * (2 + BIT(df)));
+
+ /*
+ * Datasheet:
+ * TCONVT = 67 + 192 * 2^OSR2
+ */
+ tconvt = 67 + (192 * BIT(osr2));
+ /*
+ * Total conversion time:
+ * TSTBY + TACTIVE + m * TCONVM + TCONVT + TCONV_END
+ */
+ return 220 + 360 + (m * tconvm) + tconvt + 1100;
+}
+
+static int mlx90393_xfer(struct mlx90393_data *data,
+ const u8 *tx, int tx_len,
+ u8 *rx, int rx_len)
+{
+ return data->ops->xfer(data->bus_context,
+ tx, tx_len,
+ rx, rx_len);
+}
+
+static int mlx90393_check_status(u8 cmd, u8 status)
+{
+ /* Always validate error bit */
+ if (status & MLX90393_STATUS_ERROR)
+ return -EIO;
+
+ switch (cmd & MLX90393_CMD_MASK) {
+ case MLX90393_CMD_RM:
+ /*
+ * D1:D0 indicates response availability
+ * 00 means invalid/no measurement
+ */
+ if ((status & MLX90393_STATUS_RESP) == 0)
+ return -EIO;
+ return 0;
+ case MLX90393_CMD_RT:
+ /* Reset acknowledge */
+ if (!(status & MLX90393_STATUS_RT))
+ return -EIO;
+ return 0;
+ default:
+ return 0;
+ }
+}
+
+static int mlx90393_write_cmd(struct mlx90393_data *data, u8 cmd)
+{
+ u8 status;
+ int ret;
+
+ ret = mlx90393_xfer(data, &cmd, 1, &status, 1);
+ if (ret)
+ return ret;
+
+ return mlx90393_check_status(cmd, status);
+}
+
+static int mlx90393_read_cmd(struct mlx90393_data *data, u8 cmd, u8 *rx,
+ int rx_len)
+{
+ int ret;
+
+ ret = mlx90393_xfer(data, &cmd, 1, rx, rx_len);
+ if (ret)
+ return ret;
+
+ return mlx90393_check_status(cmd, rx[0]);
+}
+
+static int mlx90393_read_reg(struct mlx90393_data *data, u8 reg, u16 *val)
+{
+ u8 tx[2];
+ u8 rx[3];
+ int ret;
+
+ tx[0] = MLX90393_CMD_RR;
+ /* Register address is encoded in bits [7:2] */
+ tx[1] = reg << 2;
+
+ ret = mlx90393_xfer(data, tx, sizeof(tx), rx, sizeof(rx));
+ if (ret)
+ return ret;
+
+ ret = mlx90393_check_status(tx[0], rx[0]);
+ if (ret)
+ return ret;
+
+ *val = get_unaligned_be16(&rx[1]);
+
+ return 0;
+}
+
+static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)
+{
+ u8 tx[4];
+ u8 status;
+ int ret;
+
+ tx[0] = MLX90393_CMD_WR;
+ put_unaligned_be16(val, &tx[1]);
+ /* Register address is encoded in bits [7:2] */
+ tx[3] = reg << 2;
+
+ ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);
+ if (ret)
+ return ret;
+
+ return mlx90393_check_status(tx[0], status);
+}
+
+static int mlx90393_update_bits(struct mlx90393_data *data, u8 reg_addr,
+ u16 mask, u16 val)
+{
+ u16 reg;
+ int ret;
+
+ ret = mlx90393_read_reg(data, reg_addr, ®);
+ if (ret)
+ return ret;
+
+ reg &= ~mask;
+ reg |= (val << __ffs(mask)) & mask;
+
+ return mlx90393_write_reg(data, reg_addr, reg);
+}
+
+static int mlx90393_read_measurement(struct mlx90393_data *data,
+ enum mlx90393_channels chan, int *val)
+{
+ u8 rx[9];
+ int ret;
+
+ /* Start measurement */
+ ret = mlx90393_write_cmd(data, MLX90393_CMD_SM | MLX90393_MEASURE_ALL);
+ if (ret)
+ return ret;
+
+ /* Wait conversion */
+ fsleep(mlx90393_get_tconv_us(data));
+
+ /* Read measurement */
+ ret = mlx90393_read_cmd(data, MLX90393_CMD_RM | MLX90393_MEASURE_ALL,
+ rx, sizeof(rx));
+ if (ret)
+ return ret;
+ /*
+ * Measurement response layout:
+ * [status][temp][x][y][z]
+ */
+
+ switch (chan) {
+ case MLX90393_CHAN_TEMP:
+ *val = get_unaligned_be16(&rx[1]);
+ return 0;
+
+ case MLX90393_CHAN_X:
+ *val = sign_extend32(get_unaligned_be16(&rx[3]), 15);
+ return 0;
+
+ case MLX90393_CHAN_Y:
+ *val = sign_extend32(get_unaligned_be16(&rx[5]), 15);
+ return 0;
+
+ case MLX90393_CHAN_Z:
+ *val = sign_extend32(get_unaligned_be16(&rx[7]), 15);
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mlx90393_get_scale(struct mlx90393_data *data,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2)
+{
+ enum mlx90393_axis_type axis;
+ u8 res;
+
+ if (chan->channel2 == IIO_MOD_Z) {
+ axis = MLX90393_AXIS_TYPE_Z;
+ res = data->res_z;
+ } else {
+ axis = MLX90393_AXIS_TYPE_XY;
+ res = data->res_xy;
+ }
+
+ /*
+ * Convert:
+ * µT × 1000 → nT
+ */
+ *val = 0;
+ *val2 = mlx90393_scale_table[axis][data->gain_sel][res];
+
+ return IIO_VAL_INT_PLUS_NANO;
+}
+
+static int mlx90393_find_scale(struct mlx90393_data *data, bool z_axis,
+ int val, int val2,
+ int *gain)
+{
+ u8 res;
+ enum mlx90393_axis_type axis;
+
+ if (z_axis) {
+ axis = MLX90393_AXIS_TYPE_Z;
+ res = data->res_z;
+ } else {
+ axis = MLX90393_AXIS_TYPE_XY;
+ res = data->res_xy;
+ }
+
+ if (val != 0)
+ return -EINVAL;
+
+ for (unsigned int i = 0; i < ARRAY_SIZE(mlx90393_scale_table[0]); i++)
+ if (mlx90393_scale_table[axis][i][res] == val2) {
+ *gain = i;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int mlx90393_set_scale(struct mlx90393_data *data,
+ const struct iio_chan_spec *chan,
+ int val, int val2)
+{
+ bool z_axis;
+ int gain;
+ int ret;
+
+ z_axis = chan->channel2 == IIO_MOD_Z;
+
+ ret = mlx90393_find_scale(data, z_axis, val, val2, &gain);
+ if (ret)
+ return ret;
+
+ ret = mlx90393_update_bits(data, MLX90393_REG_CONF1, MLX90393_CONF1_GAIN_SEL,
+ gain);
+ if (ret)
+ return ret;
+
+ data->gain_sel = gain;
+
+ return 0;
+}
+
+static int mlx90393_get_osr(struct mlx90393_data *data, int *val)
+{
+ *val = mlx90393_osr_avail[data->osr];
+
+ return IIO_VAL_INT;
+}
+
+static int mlx90393_find_osr(int val, int *osr)
+{
+ for (unsigned int i = 0; i < MLX90393_OSR_MAX; i++)
+ if (mlx90393_osr_avail[i] == val) {
+ *osr = i;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int mlx90393_get_temp_osr2(struct mlx90393_data *data, int *val)
+{
+ *val = mlx90393_osr2_avail[data->osr2];
+ return IIO_VAL_INT;
+}
+
+static int mlx90393_set_osr(struct mlx90393_data *data, int val)
+{
+ int osr;
+ int ret;
+
+ ret = mlx90393_find_osr(val, &osr);
+ if (ret)
+ return ret;
+
+ if (osr == data->osr)
+ return 0;
+
+ ret = mlx90393_update_bits(data, MLX90393_REG_CONF3, MLX90393_CONF3_OSR,
+ osr);
+ if (ret)
+ return ret;
+
+ data->osr = osr;
+ return 0;
+}
+
+static int mlx90393_set_temp_osr2(struct mlx90393_data *data, int val)
+{
+ int ret;
+
+ if (val < 0 || val >= MLX90393_OSR2_MAX)
+ return -EINVAL;
+
+ if (val == data->osr2)
+ return 0;
+
+ ret = mlx90393_update_bits(data, MLX90393_REG_CONF3, MLX90393_CONF3_OSR2,
+ val);
+ if (ret)
+ return ret;
+
+ data->osr2 = val;
+
+ return 0;
+}
+
+static int mlx90393_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;
+
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mlx90393_write_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int val, int val2,
+ long mask)
+{
+ struct mlx90393_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE: {
+ guard(mutex)(&data->lock);
+ ret = mlx90393_set_scale(data, chan, val, val2);
+ return ret;
+ }
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO: {
+ guard(mutex)(&data->lock);
+ switch (chan->type) {
+ case IIO_TEMP:
+ return mlx90393_set_temp_osr2(data, val);
+
+ case IIO_MAGN:
+ return mlx90393_set_osr(data, val);
+
+ default:
+ return -EINVAL;
+ }
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mlx90393_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long mask)
+{
+ struct mlx90393_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW: {
+ guard(mutex)(&data->lock);
+ ret = mlx90393_read_measurement(data, chan->address, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_MAGN:
+ return mlx90393_get_scale(data, chan, val, val2);
+
+ case IIO_TEMP:
+ /* Datasheet: 22124 millidegC/LSB */
+ *val = 0;
+ *val2 = 22124;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ default:
+ return -EINVAL;
+ }
+
+ case IIO_CHAN_INFO_OFFSET:
+ if (chan->type != IIO_TEMP)
+ return -EINVAL;
+
+ /* Datasheet: temperature offset */
+ *val = -45114;
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ switch (chan->type) {
+ case IIO_TEMP:
+ return mlx90393_get_temp_osr2(data, val);
+ case IIO_MAGN:
+ return mlx90393_get_osr(data, val);
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mlx90393_read_avail(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ const int **vals,
+ int *type,
+ int *length,
+ long mask)
+{
+ struct mlx90393_data *data = iio_priv(indio_dev);
+ static int scale_avail[MLX90393_GAIN_MAX][MLX90393_AXIS_MAX];
+ enum mlx90393_axis_type axis;
+ u8 res;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE: {
+ guard(mutex)(&data->lock);
+ axis = chan->channel2 == IIO_MOD_Z;
+ res = axis ? data->res_z : data->res_xy;
+
+ for (unsigned int i = 0; i < MLX90393_GAIN_MAX; i++) {
+ scale_avail[i][0] = 0;
+ scale_avail[i][1] = mlx90393_scale_table[axis][i][res];
+ }
+
+ *vals = &scale_avail[0][0];
+ *type = IIO_VAL_INT_PLUS_NANO;
+ *length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
+ return IIO_AVAIL_LIST;
+ }
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ if (chan->type == IIO_TEMP) {
+ *vals = mlx90393_osr2_avail;
+ *type = IIO_VAL_INT;
+ *length = MLX90393_OSR2_MAX;
+ } else {
+ *vals = mlx90393_osr_avail;
+ *type = IIO_VAL_INT;
+ *length = MLX90393_OSR_MAX;
+ }
+ return IIO_AVAIL_LIST;
+
+ default:
+ return -EINVAL;
+ }
+ return -EINVAL;
+}
+
+static const struct iio_info mlx90393_info = {
+ .read_raw = mlx90393_read_raw,
+ .write_raw = mlx90393_write_raw,
+ .read_avail = mlx90393_read_avail,
+ .write_raw_get_fmt = mlx90393_write_raw_get_fmt,
+};
+
+static int mlx90393_init(struct mlx90393_data *data)
+{
+ int ret;
+ u16 reg;
+
+ /* Exit mode */
+ ret = mlx90393_write_cmd(data, MLX90393_CMD_EX);
+ if (ret)
+ return ret;
+
+ /* Wait for device comes out of reset */
+ fsleep(1000);
+
+ /* Reset device */
+ ret = mlx90393_write_cmd(data, MLX90393_CMD_RT);
+ if (ret)
+ return ret;
+
+ /* Wait for device to reset */
+ fsleep(6000);
+
+ ret = mlx90393_read_reg(data, MLX90393_REG_CONF1, ®);
+ if (ret)
+ return ret;
+
+ data->gain_sel = FIELD_GET(MLX90393_CONF1_GAIN_SEL, reg);
+ data->hallconf = FIELD_GET(MLX90393_CONF1_HALLCONF, reg);
+
+ ret = mlx90393_read_reg(data, MLX90393_REG_CONF3, ®);
+ if (ret)
+ return ret;
+
+ data->res_xy = FIELD_GET(MLX90393_CONF3_RES_X, reg);
+ data->res_z = FIELD_GET(MLX90393_CONF3_RES_Z, reg);
+ data->dig_filt = FIELD_GET(MLX90393_CONF3_DIG_FILT, reg);
+ data->osr = FIELD_GET(MLX90393_CONF3_OSR, reg);
+ data->osr2 = FIELD_GET(MLX90393_CONF3_OSR2, reg);
+
+ return 0;
+}
+
+int mlx90393_core_probe(struct device *dev,
+ const struct mlx90393_transfer_ops *ops,
+ void *context)
+{
+ struct iio_dev *indio_dev;
+ struct mlx90393_data *data;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ devm_mutex_init(dev, &data->lock);
+
+ data->dev = dev;
+ data->ops = ops;
+ data->bus_context = context;
+
+ indio_dev->name = "mlx90393";
+ indio_dev->info = &mlx90393_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = mlx90393_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mlx90393_channels);
+
+ ret = mlx90393_init(data);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to initialize device\n");
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+EXPORT_SYMBOL_GPL(mlx90393_core_probe);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Nikhil Gautam <nikhilgtr@gmail.com>");
+MODULE_DESCRIPTION("MLX90393 magnetometer sensor driver");
diff --git a/drivers/iio/magnetometer/mlx90393_i2c.c b/drivers/iio/magnetometer/mlx90393_i2c.c
new file mode 100644
index 000000000000..52233b6295c2
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393_i2c.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+
+#include "mlx90393.h"
+
+/*
+ * MLX90393 commands use repeated-start transfers where
+ * every command is followed by a status/data response.
+ */
+static int mlx90393_i2c_xfer(void *context,
+ const u8 *tx, int tx_len,
+ u8 *rx, int rx_len)
+{
+ struct i2c_client *client = context;
+ int ret;
+ struct i2c_msg msgs[2] = {
+ [0] = {
+ .addr = client->addr,
+ .len = tx_len,
+ .buf = (u8 *)tx,
+ },
+ [1] = {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = rx_len,
+ .buf = rx,
+ },
+ };
+
+ ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (ret != ARRAY_SIZE(msgs))
+ return ret < 0 ? ret : -EIO;
+
+ return 0;
+}
+
+static const struct mlx90393_transfer_ops mlx90393_i2c_ops = {
+ .xfer = mlx90393_i2c_xfer,
+};
+
+static int mlx90393_i2c_probe(struct i2c_client *client)
+{
+ return mlx90393_core_probe(&client->dev, &mlx90393_i2c_ops, client);
+}
+
+static const struct i2c_device_id mlx90393_id[] = {
+ { "mlx90393" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mlx90393_id);
+
+static const struct of_device_id mlx90393_of_match[] = {
+ { .compatible = "melexis,mlx90393" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, mlx90393_of_match);
+
+static struct i2c_driver mlx90393_i2c_driver = {
+ .driver = {
+ .name = "mlx90393",
+ .of_match_table = mlx90393_of_match,
+ },
+ .probe = mlx90393_i2c_probe,
+};
+
+module_i2c_driver(mlx90393_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Nikhil Gautam <nikhilgtr@gmail.com>");
+MODULE_DESCRIPTION("MLX90393 magnetometer sensor driver");
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v2 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393
From: sashiko-bot @ 2026-06-18 16:10 UTC (permalink / raw)
To: Nikhil Gautam; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260618160141.11409-2-nikhilgtr@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The new entry for 'MELEXIS MLX90393 MAGNETOMETER DRIVER' in the MAINTAINERS file is inserted out of alphabetical order.
--
commit 756049ed43f6fd027532ae2aea78e2889e87ad8e
Author: Nikhil Gautam <nikhilgtr@gmail.com>
dt-bindings: iio: magnetometer: add Melexis MLX90393
This commit adds devicetree bindings for the Melexis MLX90393 3-axis
magnetometer and temperature sensor. The initial binding documents the
I2C interface.
[ ... ]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa163..e9ddcd12feb5e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24926,6 +24926,12 @@ S: Maintained
> F: Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
> F: drivers/iio/magnetometer/tmag5273.c
>
> +MELEXIS MLX90393 MAGNETOMETER DRIVER
> +M: Nikhil Gautam <nikhilgtr@gmail.com>
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
> +
> TI TRF7970A NFC DRIVER
[Severity: Low]
This isn't a bug, but should this new entry be placed earlier in the file?
The MAINTAINERS list is generally sorted alphabetically, and the "MELEXIS"
driver block is currently being inserted in the "T" section, between
"TI TMAG5273" and "TI TRF7970A".
> M: Mark Greer <mgreer@animalcreek.com>
> L: linux-wireless@vger.kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618160141.11409-1-nikhilgtr@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse
From: Rodrigo Alencar @ 2026-06-18 16:14 UTC (permalink / raw)
To: Nuno Sá, rodrigo.alencar
Cc: linux-iio, devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
Kees Cook, Gustavo A. R. Silva
In-Reply-To: <ajQGTQ1_qcOwfzne@nsa>
On 18/06/26 16:06, Nuno Sá wrote:
> On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> >
> > Move logic to create a channel prefix for naming attribute files into a
> > separate __iio_chan_prefix_emit() function for reuse.
...
> > +static int __iio_chan_prefix_emit(const struct iio_chan_spec *chan,
> > + enum iio_shared_by shared_by,
> > + char *buf, size_t len)
> > +{
> > + const char *dir = iio_direction[chan->output];
> > + const char *type = iio_chan_type_name_spec[chan->type];
> > + int n = 0;
> > +
> > + switch (shared_by) {
> > + case IIO_SHARED_BY_ALL:
> > + buf[0] = '\0'; /* empty channel prefix */
> > + break;
> > + case IIO_SHARED_BY_DIR:
> > + n = scnprintf(buf, len, "%s", dir);
> > + break;
> > + case IIO_SHARED_BY_TYPE:
> > + n = scnprintf(buf, len, "%s_%s", dir, type);
> > + if (chan->differential)
> > + n += scnprintf(buf + n, len - n, "-%s", type);
> > + break;
> > + case IIO_SEPARATE:
> > + if (chan->indexed) {
> > + n = scnprintf(buf, len, "%s_%s%d", dir, type,
> > + chan->channel);
> > + if (chan->differential)
> > + n += scnprintf(buf + n, len - n, "-%s%d", type,
> > + chan->channel2);
> > + } else {
> > + if (chan->differential) {
> > + WARN(1, "Differential channels must be indexed\n");
> > + return -EINVAL;
> > + }
> > + n = scnprintf(buf, len, "%s_%s", dir, type);
> > + }
> > +
> > + if (chan->modified) {
> > + if (chan->differential) {
> > + WARN(1, "Differential channels can not have modifier\n");
> > + return -EINVAL;
>
> WARN() looks too much to me. dev_error() as we're treating it as such. I
> guess you don't want to pass struct device but not really an issue IMHO.
__iio_device_attr_init() also used WARN(), probably because it didnt have
access to a dev pointer. It would not be a problem to add an extra param.
>
> > + }
> > + n += scnprintf(buf + n, len - n, "_%s",
> > + iio_modifier_names[chan->channel2]);
> > + }
> > +
> > + if (chan->extend_name)
> > + n += scnprintf(buf + n, len - n, "_%s", chan->extend_name);
> > + break;
> > + }
> > +
> > + if (n > 0 && n < len - 1) { /* prefix termination if not empty */
> > + buf[n++] = '_';
> > + buf[n] = '\0';
> > + }
> > +
>
> Can't we handle the above in the caller on kasprintf()? Then we could
> simplify and return in place.
I felt like doing this here would get a cleaner logic in the caller, which
would have to add the '_' conditionally.
>
> > + return n;
> > +}
> > +
> > /**
> > * iio_device_id() - query the unique ID for the device
> > * @indio_dev: Device structure whose ID is being queried
> > @@ -1100,106 +1159,19 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> > size_t len),
> > enum iio_shared_by shared_by)
> > {
> > - int ret = 0;
> > - char *name = NULL;
> > - char *full_postfix;
> > + char prefix[NAME_MAX + 1];
> > + int ret;
> >
> > sysfs_attr_init(&dev_attr->attr);
> >
> > - /* Build up postfix of <extend_name>_<modifier>_postfix */
> > - if (chan->modified && (shared_by == IIO_SEPARATE)) {
> > - if (chan->extend_name)
> > - full_postfix = kasprintf(GFP_KERNEL, "%s_%s_%s",
> > - iio_modifier_names[chan->channel2],
> > - chan->extend_name,
> > - postfix);
> > - else
> > - full_postfix = kasprintf(GFP_KERNEL, "%s_%s",
> > - iio_modifier_names[chan->channel2],
> > - postfix);
> > - } else {
> > - if (chan->extend_name == NULL || shared_by != IIO_SEPARATE)
> > - full_postfix = kstrdup(postfix, GFP_KERNEL);
> > - else
> > - full_postfix = kasprintf(GFP_KERNEL,
> > - "%s_%s",
> > - chan->extend_name,
> > - postfix);
> > - }
> > - if (full_postfix == NULL)
> > + ret = __iio_chan_prefix_emit(chan, shared_by, prefix, sizeof(prefix));
> > + if (ret < 0)
> > + return ret;
> > +
> > + dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, postfix);
> > + if (!dev_attr->attr.name)
> > return -ENOMEM;
>
> I don't oppose the change. Looks like a nice cleanup. But bear in mind
> this very sensible as any subtle mistake means ABI breakage.
Yes! I tried to be careful... this is dangerous stuff!
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393
From: sashiko-bot @ 2026-06-18 16:15 UTC (permalink / raw)
To: Nikhil Gautam; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260618160141.11409-3-nikhilgtr@gmail.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Data race on statically allocated `scale_avail` array.
- [High] DMA safety violation due to stack-allocated transfer buffers.
- [High] Writing to the temperature scale attribute corrupts the magnetometer gain configuration.
- [Medium] IIO scale values violate ABI standard (Tesla and Celsius instead of Gauss and milli-Celsius).
- [Medium] Missing `id_table` assignment in the I2C driver structure.
- [Medium] Temperature channel exposes raw register exponent values instead of true oversampling ratios.
- [Medium] Lockless state reads in `mlx90393_read_raw` lead to data races.
--
commit bdec68e57f45f6e35e5c0c76d37b8ee5efa9691b
Author: Nikhil Gautam <nikhilgtr@gmail.com>
iio: magnetometer: add support for Melexis MLX90393
Add Industrial I/O subsystem support for the Melexis
MLX90393 3-axis magnetometer and temperature sensor.
> diff --git a/drivers/iio/magnetometer/mlx90393_core.c b/drivers/iio/magnetometer/mlx90393_core.c
> new file mode 100644
> index 0000000000000..0ad4a30c0be96
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_core.c
[ ... ]
> +static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)
> +{
> + u8 tx[4];
> + u8 status;
> + int ret;
> +
> + tx[0] = MLX90393_CMD_WR;
> + put_unaligned_be16(val, &tx[1]);
> + /* Register address is encoded in bits [7:2] */
> + tx[3] = reg << 2;
> +
> + ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);
[Severity: High]
Is it safe to allocate the transfer buffers on the stack here and in
mlx90393_read_measurement()?
If the I2C host adapter uses DMA and CONFIG_VMAP_STACK is enabled, mapping
stack memory for DMA operations will fail. Should these buffers be
dynamically allocated to ensure DMA safety?
[ ... ]
> +static int mlx90393_get_scale(struct mlx90393_data *data,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2)
> +{
> + enum mlx90393_axis_type axis;
> + u8 res;
> +
> + if (chan->channel2 == IIO_MOD_Z) {
> + axis = MLX90393_AXIS_TYPE_Z;
> + res = data->res_z;
> + } else {
> + axis = MLX90393_AXIS_TYPE_XY;
> + res = data->res_xy;
> + }
> +
> + /*
> + * Convert:
> + * µT × 1000 → nT
> + */
> + *val = 0;
> + *val2 = mlx90393_scale_table[axis][data->gain_sel][res];
> +
> + return IIO_VAL_INT_PLUS_NANO;
> +}
[Severity: Medium]
Does this return the scale in Gauss as required by the IIO ABI for
in_magn_scale? Using IIO_VAL_INT_PLUS_NANO with the current scale table yields
Tesla, which is off by a factor of 10,000 compared to Gauss.
[ ... ]
> +static int mlx90393_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int val, int val2,
> + long mask)
> +{
> + struct mlx90393_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE: {
> + guard(mutex)(&data->lock);
> + ret = mlx90393_set_scale(data, chan, val, val2);
> + return ret;
> + }
[Severity: High]
Can writing to the temperature scale attribute corrupt the magnetometer gain
configuration?
The IIO_TEMP channel exposes a writable scale attribute, but this case does
not check chan->type before calling mlx90393_set_scale(). Because the
temperature channel's channel2 field is 0, it evaluates as an XY magnetometer
channel and will reconfigure the global gain_sel register.
[ ... ]
> +static int mlx90393_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct mlx90393_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW: {
> + guard(mutex)(&data->lock);
> + ret = mlx90393_read_measurement(data, chan->address, val);
> + if (ret)
> + return ret;
> +
> + return IIO_VAL_INT;
> + }
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_MAGN:
> + return mlx90393_get_scale(data, chan, val, val2);
[Severity: Medium]
Is a lock needed here?
Configuration variables like gain_sel, osr, and osr2 are updated while
holding data->lock in mlx90393_write_raw(), but mlx90393_read_raw() reads
these properties without acquiring the lock.
Can concurrent updates result in stale or inconsistently-read configurations?
> +
> + case IIO_TEMP:
> + /* Datasheet: 22124 millidegC/LSB */
> + *val = 0;
> + *val2 = 22124;
> + return IIO_VAL_INT_PLUS_MICRO;
[Severity: Medium]
Does this return the temperature scale in milli-Celsius as required by the IIO
ABI? Returning 22124 with IIO_VAL_INT_PLUS_MICRO yields Celsius, which is off
by a factor of 1,000.
[ ... ]
> +static int mlx90393_read_avail(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + const int **vals,
> + int *type,
> + int *length,
> + long mask)
> +{
> + struct mlx90393_data *data = iio_priv(indio_dev);
> + static int scale_avail[MLX90393_GAIN_MAX][MLX90393_AXIS_MAX];
[Severity: High]
Is it safe to use a statically allocated local array for scale_avail?
The lock is released before the IIO core iterates over the returned pointer.
Can concurrent sysfs reads across different channels or multiple device
instances clobber the shared array and return corrupted lists to userspace?
> + enum mlx90393_axis_type axis;
> + u8 res;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE: {
> + guard(mutex)(&data->lock);
> + axis = chan->channel2 == IIO_MOD_Z;
> + res = axis ? data->res_z : data->res_xy;
> +
> + for (unsigned int i = 0; i < MLX90393_GAIN_MAX; i++) {
> + scale_avail[i][0] = 0;
> + scale_avail[i][1] = mlx90393_scale_table[axis][i][res];
> + }
> +
> + *vals = &scale_avail[0][0];
> + *type = IIO_VAL_INT_PLUS_NANO;
> + *length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
> + return IIO_AVAIL_LIST;
> + }
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + if (chan->type == IIO_TEMP) {
> + *vals = mlx90393_osr2_avail;
> + *type = IIO_VAL_INT;
> + *length = MLX90393_OSR2_MAX;
[Severity: Medium]
Does this expose raw register exponent values instead of true oversampling
ratios for the temperature channel?
Returning the register exponents (0, 1, 2, 3) directly violates the IIO ABI
which dictates reflecting the actual number of hardware samples applied.
[ ... ]
> diff --git a/drivers/iio/magnetometer/mlx90393_i2c.c b/drivers/iio/magnetometer/mlx90393_i2c.c
> new file mode 100644
> index 0000000000000..52233b6295c2b
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_i2c.c
[ ... ]
> +static struct i2c_driver mlx90393_i2c_driver = {
> + .driver = {
> + .name = "mlx90393",
> + .of_match_table = mlx90393_of_match,
> + },
> + .probe = mlx90393_i2c_probe,
> +};
[Severity: Medium]
Is the id_table assignment missing here?
The driver defines the mlx90393_id table and exports it, but it is not
assigned to the id_table field of this structure. Could this prevent the
driver from binding on systems relying on legacy board-file setups or sysfs
new_device instantiation?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618160141.11409-1-nikhilgtr@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v4 01/16] spi: dt-bindings: add spi-max-post-config-frequency property
From: Conor Dooley @ 2026-06-18 16:36 UTC (permalink / raw)
To: Santhosh Kumar K
Cc: broonie, robh, krzk+dt, conor+dt, miquel.raynal, richard,
vigneshr, pratyush, mwalle, takahiro.kuwano, linux-spi,
devicetree, linux-kernel, linux-mtd, praneeth, u-kumar1, a-dutta
In-Reply-To: <20260618073725.84733-2-s-k6@ti.com>
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
On Thu, Jun 18, 2026 at 01:07:10PM +0530, Santhosh Kumar K wrote:
> Add spi-max-post-config-frequency, a generic uint32 property for SPI
> peripherals that support two distinct clock rates: a conservative rate
> always reachable without controller configuration, and a higher rate
> reachable only after controller-side configuration such as PHY tuning.
>
> When both properties are present, spi-max-frequency gives the
> conservative pre-configuration rate and spi-max-post-config-frequency
> gives the higher post-configuration target.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/3] dt-bindings: riscv: describe Svadu as disabled at boot
From: Conor Dooley @ 2026-06-18 16:37 UTC (permalink / raw)
To: Yunhui Cui
Cc: akpm, alex, andrew+kernel, aou, apatel, apopple, atishp,
baolin.wang, cleger, conor+dt, debug, devicetree, guodong,
hui.wang, krzk+dt, linux-kernel, linux-riscv, liu.xuemei1, namcao,
nick.hu, palmer, pincheng.plct, pjw, qingwei.hu, ritesh.list,
rmclure, robh, wangruikang, zhangchunyan, zong.li
In-Reply-To: <20260618064406.14508-2-cuiyunhui@bytedance.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v6 00/10] ACPI: APEI: share GHES CPER helpers and add DT FFH provider
From: Borislav Petkov @ 2026-06-18 16:48 UTC (permalink / raw)
To: Ahmed Tiba
Cc: Rafael J. Wysocki, Tony Luck, Hanjun Guo, Mauro Carvalho Chehab,
Shuai Xue, Len Brown, Saket Dumbre, Davidlohr Bueso,
Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma,
Ira Weiny, Dan Williams, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan, linux-kernel,
linux-acpi, acpica-devel, linux-cxl, devicetree, linux-edac,
linux-doc, Dmitry.Lamerov
In-Reply-To: <20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com>
On Wed, Jun 17, 2026 at 02:54:38PM +0100, Ahmed Tiba wrote:
> This is v6 of the GHES refactor series. Compared to v5, it addresses
> the latest review comments and tightens the DT CPER provider and
> related helper wiring.
Sashiko has comments:
https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0%40arm.com
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 01/11] dt-bindings: power: Convert Ux500 PM domains to schema
From: Rob Herring (Arm) @ 2026-06-18 16:51 UTC (permalink / raw)
To: Linus Walleij
Cc: Lee Jones, dri-devel, linux-pm, Maarten Lankhorst, David Airlie,
linux-arm-kernel, Ulf Hansson, Frank Li, devicetree,
Maxime Ripard, dmaengine, Thomas Zimmermann, Mark Brown,
Simona Vetter, Vinod Koul, Conor Dooley, Krzysztof Kozlowski
In-Reply-To: <20260618-ux500-power-domains-v7-1-v1-1-eb5e50b1a588@kernel.org>
On Thu, 18 Jun 2026 07:00:47 +0200, Linus Walleij wrote:
> Convert the legacy Ux500 power domain text binding to YAML.
>
> Move it under bindings/power.
>
> Reference the generic power-domain schema.
>
> Update MAINTAINERS for the new path.
>
> Assisted-by: Codex:gpt-5-5
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
> .../devicetree/bindings/arm/ux500/power_domain.txt | 35 ----------------
> .../power/stericsson,ux500-pm-domains.yaml | 46 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 3 files changed, 47 insertions(+), 35 deletions(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/power/stericsson,ux500-pm-domains.example.dts:25.28-28.11: Warning (unit_address_vs_reg): /example-0/sdi0_per1@80126000: node has a unit name, but no reg or ranges property
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/stericsson,ux500-pm-domains.example.dtb: sdi0_per1@80126000 (arm,pl18x): $nodename:0: 'sdi0_per1@80126000' does not match '^mmc(@.*)?$'
from schema $id: http://devicetree.org/schemas/mmc/arm,pl18x.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/stericsson,ux500-pm-domains.example.dtb: sdi0_per1@80126000 (arm,pl18x): 'oneOf' conditional failed, one must be fixed:
'interrupts' is a required property
'interrupts-extended' is a required property
from schema $id: http://devicetree.org/schemas/mmc/arm,pl18x.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/stericsson,ux500-pm-domains.example.dtb: sdi0_per1@80126000 (arm,pl18x): 'reg' is a required property
from schema $id: http://devicetree.org/schemas/mmc/arm,pl18x.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260618-ux500-power-domains-v7-1-v1-1-eb5e50b1a588@kernel.org
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH v7 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels
From: Doug Anderson @ 2026-06-18 17:00 UTC (permalink / raw)
To: Neil Armstrong
Cc: Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, dri-devel, devicetree, linux-kernel,
linux-renesas-soc, Dmitry Baryshkov, KancyJoe
In-Reply-To: <20260605-topic-sm8650-ayaneo-pocket-s2-r63419-v7-2-b84b6da84293@linaro.org>
Hi,
On Fri, Jun 5, 2026 at 7:51 AM Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> From: KancyJoe <kancy2333@outlook.com>
>
> Implement support for the Renesas 63419 based dual-DSI video mode
> Display Panels found in the Ayaneo gaming handled devices.
>
> Signed-off-by: KancyJoe <kancy2333@outlook.com>
I notice "Kancy Joe" has a space in the source files, but not in the
signoff. I guess Signed-off-by isn't necessarily required to be real
names these days, but still seems odd...
> +/*
> + * Helper to switch between DSI links, so we share a single dsi_ctx
> + * for both links, so in case of an error all writes & sleep for
> + * both links are ignored.
> + */
> +static inline void dsi_link_switch(struct renesas_r63419_panel *ctx,
> + struct mipi_dsi_multi_context *dsi_ctx,
> + unsigned int link)
> +{
> + dsi_ctx->dsi = ctx->dsi[link];
> +}
> +
> +static int renesas_r63419_on(struct renesas_r63419_panel *ctx)
> +{
> + struct mipi_dsi_multi_context dsi_ctx = { 0 };
> +
> + /* Panel registers are loaded from DDIC Non Volatile Memory */
> +
> + dsi_link_switch(ctx, &dsi_ctx, 0);
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> + dsi_link_switch(ctx, &dsi_ctx, 1);
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
Instead of dsi_link_switch(), can't you use the mipi_dsi_dual()
function? I think it would be:
mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi, dsi_ctx,
ctx->dsi[0], ctx->dsi[1]);
> +static int renesas_r63419_disable(struct drm_panel *panel)
> +{
> + struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> + struct mipi_dsi_multi_context dsi_ctx = { 0 };
> +
> + dsi_link_switch(ctx, &dsi_ctx, 0);
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> + dsi_link_switch(ctx, &dsi_ctx, 1);
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 50);
> +
> + dsi_link_switch(ctx, &dsi_ctx, 0);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + dsi_link_switch(ctx, &dsi_ctx, 1);
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> + mipi_dsi_msleep(&dsi_ctx, 120);
> +
> + return dsi_ctx.accum_err;
I'm not sure we've been terribly consistent, but should the above be
"return 0"? I'm not actually sure there's any benefit to a panel's
disable() function returning an error to begin with.
drm_panel_disable() doesn't return an error, so all this does is skip
setting "panel->enabled" to false and make it harder for the system to
recover.
> +static int renesas_r63419_prepare(struct drm_panel *panel)
> +{
> + struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
> + ctx->vdd_supplies);
> + if (ret < 0)
> + return ret;
> +
> + usleep_range(1000, 2000);
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
> + ctx->vcc_supplies);
> + if (ret < 0) {
> + regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
> + ctx->vdd_supplies);
> + return ret;
> + }
> +
> + usleep_range(1000, 2000);
> +
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +
> + usleep_range(3000, 4000);
> +
> + ret = renesas_r63419_on(ctx);
> + if (ret < 0) {
> + dev_err(panel->dev, "Failed to initialize panel: %d\n", ret);
> +
> + /* Power off sequence from the r63419 datasheet */
> + regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies),
> + ctx->vcc_supplies);
> + regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies),
> + ctx->vdd_supplies);
> +
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
To make de-init opposite to init, shouldn't the reset come before you
turn the regulators off? Depending on the design of the panel, I'd
imagine this could prevent back-powering some logic?
I'd also expect vdd supplies to be turned off first?
> +static int renesas_r63419_unprepare(struct drm_panel *panel)
> +{
> + struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> +
> + /* Power off sequence from the r63419 datasheet */
> + regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vcc_supplies), ctx->vcc_supplies);
> + regulator_bulk_disable(ARRAY_SIZE(renesas_r63419_vdd_supplies), ctx->vdd_supplies);
> +
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
Similar: shouldn't the reset come before the regulators to make
power-off the opposite of init.
> +static int renesas_r63419_get_modes(struct drm_panel *panel,
> + struct drm_connector *connector)
> +{
> + struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel);
> + const struct drm_display_mode *mode = ctx->desc->mode;
> +
> + drm_connector_set_panel_orientation(connector, ctx->orientation);
IIRC, the above was a workaround that caused a warning splat. Is your
panel used on a system that actually needs it? Could your DRM driver
be fixed rather than persisting this hack? For context, see commit
47bef230225b ("drm/panel: panel-edp: Implement .get_orientation
callback")
> +static int renesas_r63419_probe(struct mipi_dsi_device *dsi)
> +{
> + struct mipi_dsi_device_info info = { };
> + struct device *dev = &dsi->dev;
> + struct renesas_r63419_panel *ctx;
> + struct device_node *dsi1_node;
> + struct mipi_dsi_host *dsi1_host;
> + int ret, i;
> +
> + ctx = devm_drm_panel_alloc(dev, struct renesas_r63419_panel, panel,
> + &renesas_r63419_panel_funcs, DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(ctx))
> + return PTR_ERR(ctx);
> +
> + ctx->desc = of_device_get_match_data(dev);
> + if (!ctx->desc)
> + return dev_err_probe(dev, -ENODEV,
> + "Failed to get panel description\n");
> +
> + ret = devm_regulator_bulk_get_const(&dsi->dev,
> + ARRAY_SIZE(renesas_r63419_vdd_supplies),
> + renesas_r63419_vdd_supplies, &ctx->vdd_supplies);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_regulator_bulk_get_const(&dsi->dev,
> + ARRAY_SIZE(renesas_r63419_vcc_supplies),
> + renesas_r63419_vcc_supplies, &ctx->vcc_supplies);
> + if (ret < 0)
> + return ret;
It seems like both sets of supplies are always enabled / disabled
together with no delay between them. Do you truly need two lists, or
can this be combined to one list of regulators. That would simplify a
bunch of logic.
> + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(ctx->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> + "Failed to get reset gpio\n");
> +
> + /* Get second DSI host */
> + dsi1_node = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
> + if (!dsi1_node)
> + return dev_err_probe(dev, -ENODEV,
> + "Failed to get remote node for second DSI\n");
> +
> + dsi1_host = of_find_mipi_dsi_host_by_node(dsi1_node);
> + of_node_put(dsi1_node);
> + if (!dsi1_host)
> + return dev_err_probe(dev, -EPROBE_DEFER,
> + "Failed to find second DSI host\n");
> +
> + /* Copy current DSI info, do not provide OF node since no driver needs to be attached */
> + strscpy(info.type, dsi->name, sizeof(info.type));
Can't you use the two-argument form of strscpy()?
FWIW, I also notice that the Sashiko AI bot had some comments. Did you
already look all of those over and decide they don't need fixing? I
have a vague recollection that there's no need to worry about someone
calling disable() and then enable() without going through the
unprepare() / prepare(). If my memory is correct, I guess that would
be nice to document... I didn't analyze some of the other claims that
the AI bot had.
-Doug
^ permalink raw reply
* [PATCH 00/10] rust: driver: use pointers instead of indices for ID info
From: Gary Guo @ 2026-06-18 17:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
FUJITA Tomonori, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Abdiel Janulgue, Robin Murphy,
Dave Ertman, Ira Weiny, Leon Romanovsky, Len Brown, Igor Korotin,
Rob Herring, Saravana Kannan, Viresh Kumar, Michal Wilczynski,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König
Cc: driver-core, rust-for-linux, linux-kernel, netdev, nova-gpu,
dri-devel, linux-pci, linux-acpi, devicetree, linux-pm,
linux-riscv, linux-pwm, Gary Guo
Most C drivers use a pointer (and cast to kernel_ulong_t) for driver_data
fields in device_id. Rust code currently does not do this, but rather use
indices. These indices then needs to be translated to `&IdInfo` separately
and this is by a side table.
This leads to open-coded ACPI/OF handling in driver.rs, which is not
desirable. Convert the code to use pointers (or rather, static references)
instead.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
Gary Guo (10):
rust: driver: remove `IdTable::id`
rust: driver: simplify `IdArray::new_without_index`
rust: pci: use `Option<&IdInfo>` for device ID info
rust: net/phy: remove expansion from doc
rust: driver: centralize device ID handling
rust: driver: remove `$module_table_name` from `module_device_table`
rust: driver: store pointers in `DeviceId`
rust: driver: remove open-coded matching logic
rust: driver: remove duplicate ID table
RFC: rust: driver: support map-like syntax for ID table
drivers/cpufreq/rcpufreq_dt.rs | 1 -
drivers/gpu/drm/nova/driver.rs | 1 -
drivers/gpu/drm/tyr/driver.rs | 1 -
drivers/gpu/nova-core/driver.rs | 3 +-
drivers/pwm/pwm_th1520.rs | 1 -
rust/kernel/acpi.rs | 14 +--
rust/kernel/auxiliary.rs | 18 +--
rust/kernel/device_id.rs | 205 +++++++++++++++++++---------------
rust/kernel/driver.rs | 114 ++-----------------
rust/kernel/i2c.rs | 26 ++---
rust/kernel/net/phy.rs | 66 +----------
rust/kernel/of.rs | 14 +--
rust/kernel/pci.rs | 24 ++--
rust/kernel/platform.rs | 5 +-
rust/kernel/usb.rs | 18 +--
samples/rust/rust_debugfs.rs | 1 -
samples/rust/rust_dma.rs | 3 +-
samples/rust/rust_driver_auxiliary.rs | 4 +-
samples/rust/rust_driver_i2c.rs | 3 -
samples/rust/rust_driver_pci.rs | 11 +-
samples/rust/rust_driver_platform.rs | 2 -
samples/rust/rust_driver_usb.rs | 1 -
samples/rust/rust_i2c_client.rs | 2 -
samples/rust/rust_soc.rs | 2 -
24 files changed, 166 insertions(+), 374 deletions(-)
---
base-commit: 4fa3f5fabb30bf00d7475d5a33459ea83d639bf9
change-id: 20260612-id_info-23eca472ccd8
Best regards,
--
Gary Guo <gary@garyguo.net>
^ permalink raw reply
* [PATCH 02/10] rust: driver: simplify `IdArray::new_without_index`
From: Gary Guo @ 2026-06-18 17:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
FUJITA Tomonori, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Abdiel Janulgue, Robin Murphy,
Dave Ertman, Ira Weiny, Leon Romanovsky, Len Brown, Igor Korotin,
Rob Herring, Saravana Kannan, Viresh Kumar, Michal Wilczynski,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König
Cc: driver-core, rust-for-linux, linux-kernel, netdev, nova-gpu,
dri-devel, linux-pci, linux-acpi, devicetree, linux-pm,
linux-riscv, linux-pwm, Gary Guo
In-Reply-To: <20260618-id_info-v1-0-96af1e559ef9@garyguo.net>
This method can very easily construct the `IdArray` on its own without
delegating to `Self::build`. Doing so also simplifies the phy device table
macro because it does not need to construct tuples anymore.
This also allows simplification of `new` and `build` which removes the
`unsafe`.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/device_id.rs | 64 ++++++++++++++++++++----------------------------
rust/kernel/net/phy.rs | 2 +-
2 files changed, 28 insertions(+), 38 deletions(-)
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index fbf6d8e6afb9..eeef3f5e7b63 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -73,19 +73,11 @@ pub struct IdArray<T: RawDeviceId, U, const N: usize> {
id_infos: [U; N],
}
-impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
+impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
/// Creates a new instance of the array.
///
/// The contents are derived from the given identifiers and context information.
- ///
- /// # Safety
- ///
- /// `data_offset` as `None` is always safe.
- /// If `data_offset` is `Some(data_offset)`, then:
- /// - `data_offset` must be the correct offset (in bytes) to the context/data field
- /// (e.g., the `driver_data` field) within the raw device ID structure.
- /// - The field at `data_offset` must be correctly sized to hold a `usize`.
- const unsafe fn build(ids: [(T, U); N], data_offset: Option<usize>) -> Self {
+ pub const fn new(ids: [(T, U); N]) -> Self {
let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
let mut infos = [const { MaybeUninit::uninit() }; N];
@@ -94,16 +86,14 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
// SAFETY: by the safety requirement of `RawDeviceId`, we're guaranteed that `T` is
// layout-wise compatible with `RawType`.
raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
- if let Some(data_offset) = data_offset {
- // SAFETY: by the safety requirement of this function, this would be effectively
- // `raw_ids[i].driver_data = i;`.
- unsafe {
- raw_ids[i]
- .as_mut_ptr()
- .byte_add(data_offset)
- .cast::<usize>()
- .write(i);
- }
+ // SAFETY: by the safety requirement of `RawDeviceIdIndex`, this would be effectively
+ // `raw_ids[i].driver_data = i;`.
+ unsafe {
+ raw_ids[i]
+ .as_mut_ptr()
+ .byte_add(T::DRIVER_DATA_OFFSET)
+ .cast::<usize>()
+ .write(i);
}
// SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
@@ -127,32 +117,32 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
id_infos: unsafe { core::mem::transmute_copy(&infos) },
}
}
+}
- /// Creates a new instance of the array without writing index values.
- ///
- /// The contents are derived from the given identifiers and context information.
- /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
- pub const fn new_without_index(ids: [(T, U); N]) -> Self {
- // SAFETY: Calling `Self::build` with `offset = None` is always safe,
- // because no raw memory writes are performed in this case.
- unsafe { Self::build(ids, None) }
- }
-
+impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
/// Reference to the contained [`RawIdArray`].
pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
&self.raw_ids
}
}
-impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
- /// Creates a new instance of the array.
+impl<T: RawDeviceId, const N: usize> IdArray<T, (), N> {
+ /// Creates a new instance of the array without writing index values.
///
/// The contents are derived from the given identifiers and context information.
- pub const fn new(ids: [(T, U); N]) -> Self {
- // SAFETY: by the safety requirement of `RawDeviceIdIndex`,
- // `T::DRIVER_DATA_OFFSET` is guaranteed to be the correct offset (in bytes) to
- // a field within `T::RawType`.
- unsafe { Self::build(ids, Some(T::DRIVER_DATA_OFFSET)) }
+ /// If the device implements [`RawDeviceIdIndex`], consider using [`IdArray::new`] instead.
+ pub const fn new_without_index(ids: [T; N]) -> Self {
+ // SAFETY: `T` is layout-wise compatible with `T::RawType`, so is the array of them.
+ let raw_ids: [T::RawType; N] = unsafe { core::mem::transmute_copy(&ids) };
+ core::mem::forget(ids);
+
+ Self {
+ raw_ids: RawIdArray {
+ ids: raw_ids,
+ sentinel: MaybeUninit::zeroed(),
+ },
+ id_infos: [(); N],
+ }
}
}
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 3ca99db5cccf..2868e3a9e02c 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -868,7 +868,7 @@ macro_rules! module_phy_driver {
const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
- $crate::device_id::IdArray::new_without_index([ $(($dev,())),+, ]);
+ $crate::device_id::IdArray::new_without_index([ $($dev),+, ]);
$crate::module_device_table!("mdio", phydev, TABLE);
};
--
2.54.0
^ permalink raw reply related
* [PATCH 01/10] rust: driver: remove `IdTable::id`
From: Gary Guo @ 2026-06-18 17:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
FUJITA Tomonori, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Abdiel Janulgue, Robin Murphy,
Dave Ertman, Ira Weiny, Leon Romanovsky, Len Brown, Igor Korotin,
Rob Herring, Saravana Kannan, Viresh Kumar, Michal Wilczynski,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König
Cc: driver-core, rust-for-linux, linux-kernel, netdev, nova-gpu,
dri-devel, linux-pci, linux-acpi, devicetree, linux-pm,
linux-riscv, linux-pwm, Gary Guo
In-Reply-To: <20260618-id_info-v1-0-96af1e559ef9@garyguo.net>
This is unused.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/device_id.rs | 7 -------
1 file changed, 7 deletions(-)
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 8e9721446014..fbf6d8e6afb9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -166,9 +166,6 @@ pub trait IdTable<T: RawDeviceId, U> {
/// Obtain the pointer to the ID table.
fn as_ptr(&self) -> *const T::RawType;
- /// Obtain the pointer to the bus specific device ID from an index.
- fn id(&self, index: usize) -> &T::RawType;
-
/// Obtain the pointer to the driver-specific information from an index.
fn info(&self, index: usize) -> &U;
}
@@ -180,10 +177,6 @@ fn as_ptr(&self) -> *const T::RawType {
core::ptr::from_ref(self).cast()
}
- fn id(&self, index: usize) -> &T::RawType {
- &self.raw_ids.ids[index]
- }
-
fn info(&self, index: usize) -> &U {
&self.id_infos[index]
}
--
2.54.0
^ permalink raw reply related
* [PATCH 03/10] rust: pci: use `Option<&IdInfo>` for device ID info
From: Gary Guo @ 2026-06-18 17:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
FUJITA Tomonori, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Abdiel Janulgue, Robin Murphy,
Dave Ertman, Ira Weiny, Leon Romanovsky, Len Brown, Igor Korotin,
Rob Herring, Saravana Kannan, Viresh Kumar, Michal Wilczynski,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König
Cc: driver-core, rust-for-linux, linux-kernel, netdev, nova-gpu,
dri-devel, linux-pci, linux-acpi, devicetree, linux-pm,
linux-riscv, linux-pwm, Gary Guo
In-Reply-To: <20260618-id_info-v1-0-96af1e559ef9@garyguo.net>
It is possible that `pci_device_id_any` will be passed to the driver, e.g.
`driver_override` is used on the device. Therefore, the driver must be able
to handle the case where `driver_data` is 0. Thus, update the `probe`
functions to get `Option`.
The current code cannot tell if the info does not exist or is the first
entry; however this will be achievable once the code is updated to use a
`&'static IdInfo` pointer instead of indices.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
drivers/gpu/nova-core/driver.rs | 2 +-
rust/kernel/pci.rs | 6 +++---
samples/rust/rust_dma.rs | 2 +-
samples/rust/rust_driver_auxiliary.rs | 2 +-
samples/rust/rust_driver_pci.rs | 3 ++-
5 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5738d4ac521b..5a5f0b63e0f3 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -70,7 +70,7 @@ impl pci::Driver for NovaCoreDriver {
fn probe<'bound>(
pdev: &'bound pci::Device<Core<'_>>,
- _info: &'bound Self::IdInfo,
+ _info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
pin_init::pin_init_scope(move || {
dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5071cae6543f..0e055e4df99e 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -113,7 +113,7 @@ extern "C" fn probe_callback(
let info = T::ID_TABLE.info(id.index());
from_result(|| {
- let data = T::probe(pdev, info);
+ let data = T::probe(pdev, Some(info));
pdev.as_ref().set_drvdata(data)?;
Ok(0)
@@ -284,7 +284,7 @@ macro_rules! pci_device_table {
///
/// fn probe<'bound>(
/// _pdev: &'bound pci::Device<Core<'_>>,
-/// _id_info: &'bound Self::IdInfo,
+/// _id_info: Option<&'bound Self::IdInfo>,
/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
/// Err(ENODEV)
/// }
@@ -313,7 +313,7 @@ pub trait Driver {
/// attempt to initialize the device here.
fn probe<'bound>(
dev: &'bound Device<device::Core<'_>>,
- id_info: &'bound Self::IdInfo,
+ id_info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;
/// PCI driver unbind.
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 5046b4628d0e..9beb37275e0d 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -63,7 +63,7 @@ impl pci::Driver for DmaSampleDriver {
fn probe<'bound>(
pdev: &'bound pci::Device<Core<'_>>,
- _info: &'bound Self::IdInfo,
+ _info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self, Error> + 'bound {
pin_init::pin_init_scope(move || {
dev_info!(pdev, "Probe DMA test driver.\n");
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 2c1351040e45..73c63afc046a 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -79,7 +79,7 @@ impl pci::Driver for ParentDriver {
fn probe<'bound>(
pdev: &'bound pci::Device<Core<'_>>,
- _info: &'bound Self::IdInfo,
+ _info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
Ok(ParentData {
// SAFETY: `ParentData` is the driver's private data, which is dropped when the
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1aa8197d8698..5547dd704a1b 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -144,7 +144,7 @@ impl pci::Driver for SampleDriver {
fn probe<'bound>(
pdev: &'bound pci::Device<Core<'_>>,
- info: &'bound Self::IdInfo,
+ info: Option<&'bound Self::IdInfo>,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
let vendor = pdev.vendor_id();
dev_dbg!(
@@ -153,6 +153,7 @@ fn probe<'bound>(
vendor,
pdev.device_id()
);
+ let info = info.ok_or(ENODEV)?;
pdev.enable_device_mem()?;
pdev.set_master();
--
2.54.0
^ permalink raw reply related
* [PATCH 04/10] rust: net/phy: remove expansion from doc
From: Gary Guo @ 2026-06-18 17:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
FUJITA Tomonori, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Abdiel Janulgue, Robin Murphy,
Dave Ertman, Ira Weiny, Leon Romanovsky, Len Brown, Igor Korotin,
Rob Herring, Saravana Kannan, Viresh Kumar, Michal Wilczynski,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König
Cc: driver-core, rust-for-linux, linux-kernel, netdev, nova-gpu,
dri-devel, linux-pci, linux-acpi, devicetree, linux-pm,
linux-riscv, linux-pwm, Gary Guo
In-Reply-To: <20260618-id_info-v1-0-96af1e559ef9@garyguo.net>
The expansion serves little purpose and it can easily diverge.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/net/phy.rs | 56 --------------------------------------------------
1 file changed, 56 deletions(-)
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 2868e3a9e02c..965ecca7d55f 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -800,62 +800,6 @@ const fn as_int(&self) -> u32 {
/// }
/// # }
/// ```
-///
-/// This expands to the following code:
-///
-/// ```ignore
-/// use kernel::net::phy::{self, DeviceId};
-/// use kernel::prelude::*;
-///
-/// struct Module {
-/// _reg: ::kernel::net::phy::Registration,
-/// }
-///
-/// module! {
-/// type: Module,
-/// name: "rust_sample_phy",
-/// authors: ["Rust for Linux Contributors"],
-/// description: "Rust sample PHYs driver",
-/// license: "GPL",
-/// }
-///
-/// struct PhySample;
-///
-/// #[vtable]
-/// impl phy::Driver for PhySample {
-/// const NAME: &'static CStr = c"PhySample";
-/// const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x00000001);
-/// }
-///
-/// const _: () = {
-/// static mut DRIVERS: [::kernel::net::phy::DriverVTable; 1] =
-/// [::kernel::net::phy::create_phy_driver::<PhySample>()];
-///
-/// impl ::kernel::Module for Module {
-/// fn init(module: &'static ::kernel::ThisModule) -> Result<Self> {
-/// let drivers = unsafe { &mut DRIVERS };
-/// let mut reg = ::kernel::net::phy::Registration::register(
-/// module,
-/// ::core::pin::Pin::static_mut(drivers),
-/// )?;
-/// Ok(Module { _reg: reg })
-/// }
-/// }
-/// };
-///
-/// const N: usize = 1;
-///
-/// const TABLE: ::kernel::device_id::IdArray<::kernel::net::phy::DeviceId, (), N> =
-/// ::kernel::device_id::IdArray::new_without_index([
-/// ::kernel::net::phy::DeviceId(
-/// ::kernel::bindings::mdio_device_id {
-/// phy_id: 0x00000001,
-/// phy_id_mask: 0xffffffff,
-/// }),
-/// ]);
-///
-/// ::kernel::module_device_table!("mdio", phydev, TABLE);
-/// ```
#[macro_export]
macro_rules! module_phy_driver {
(@replace_expr $_t:tt $sub:expr) => {$sub};
--
2.54.0
^ permalink raw reply related
* [PATCH 05/10] rust: driver: centralize device ID handling
From: Gary Guo @ 2026-06-18 17:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
FUJITA Tomonori, David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Abdiel Janulgue, Robin Murphy,
Dave Ertman, Ira Weiny, Leon Romanovsky, Len Brown, Igor Korotin,
Rob Herring, Saravana Kannan, Viresh Kumar, Michal Wilczynski,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König
Cc: driver-core, rust-for-linux, linux-kernel, netdev, nova-gpu,
dri-devel, linux-pci, linux-acpi, devicetree, linux-pm,
linux-riscv, linux-pwm, Gary Guo
In-Reply-To: <20260618-id_info-v1-0-96af1e559ef9@garyguo.net>
Move the `IdArray` creation from individual buses to be handled by shared
code in `device_id.rs`.
Signed-off-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/acpi.rs | 10 ++--------
rust/kernel/auxiliary.rs | 10 ++--------
rust/kernel/device_id.rs | 31 ++++++++++++++++++++++++++++++-
rust/kernel/i2c.rs | 10 ++--------
rust/kernel/net/phy.rs | 10 ++++------
rust/kernel/of.rs | 10 ++--------
rust/kernel/pci.rs | 10 ++--------
rust/kernel/usb.rs | 10 ++--------
8 files changed, 46 insertions(+), 55 deletions(-)
diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 9b8efa623130..315f2f2af446 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -53,13 +53,7 @@ pub const fn new(id: &'static CStr) -> Self {
/// Create an ACPI `IdTable` with an "alias" for modpost.
#[macro_export]
macro_rules! acpi_device_table {
- ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
- const $table_name: $crate::device_id::IdArray<
- $crate::acpi::DeviceId,
- $id_info_type,
- { $table_data.len() },
- > = $crate::device_id::IdArray::new($table_data);
-
- $crate::module_device_table!("acpi", $module_table_name, $table_name);
+ ($($tt:tt)*) => {
+ $crate::module_device_table!("acpi", $crate::acpi::DeviceId, $($tt)*);
};
}
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index c42928d5a239..59787c9bff26 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -181,14 +181,8 @@ fn index(&self) -> usize {
/// Create a auxiliary `IdTable` with its alias for modpost.
#[macro_export]
macro_rules! auxiliary_device_table {
- ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
- const $table_name: $crate::device_id::IdArray<
- $crate::auxiliary::DeviceId,
- $id_info_type,
- { $table_data.len() },
- > = $crate::device_id::IdArray::new($table_data);
-
- $crate::module_device_table!("auxiliary", $module_table_name, $table_name);
+ ($($tt:tt)*) => {
+ $crate::module_device_table!("auxiliary", $crate::auxiliary::DeviceId, $($tt)*);
};
}
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index eeef3f5e7b63..0239f89d5f69 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -175,7 +175,36 @@ fn info(&self, index: usize) -> &U {
/// Create device table alias for modpost.
#[macro_export]
macro_rules! module_device_table {
- ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
+ (
+ $table_type: literal, $device_id_ty: ty,
+ $table_name: ident, $module_table_name: ident, $id_info_type: ty,
+ [$(($id: expr, $info:expr $(,)?)),* $(,)?]
+ ) => {
+ const $table_name: $crate::device_id::IdArray<
+ $device_id_ty,
+ $id_info_type,
+ { <[$device_id_ty]>::len(&[$($id,)*]) },
+ > = $crate::device_id::IdArray::new([$(($id, $info),)*]);
+
+ $crate::module_device_table!($table_type, $module_table_name, $table_name);
+ };
+
+ // Case for no ID info.
+ (
+ $table_type: literal, $device_id_ty: ty,
+ $table_name: ident, $module_table_name: ident, @none,
+ [$($id: expr),* $(,)?]
+ ) => {
+ const $table_name: $crate::device_id::IdArray<
+ $device_id_ty,
+ (),
+ { <[$device_id_ty]>::len(&[$($id,)*]) },
+ > = $crate::device_id::IdArray::new_without_index([$($id),*]);
+
+ $crate::module_device_table!($table_type, $module_table_name, $table_name);
+ };
+
+ ($table_type: literal, $module_table_name: ident, $table_name:ident) => {
#[rustfmt::skip]
#[export_name =
concat!("__mod_device_table__", line!(),
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 7655d61daac8..a7d9b88ae616 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -77,14 +77,8 @@ fn index(&self) -> usize {
/// Create a I2C `IdTable` with its alias for modpost.
#[macro_export]
macro_rules! i2c_device_table {
- ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
- const $table_name: $crate::device_id::IdArray<
- $crate::i2c::DeviceId,
- $id_info_type,
- { $table_data.len() },
- > = $crate::device_id::IdArray::new($table_data);
-
- $crate::module_device_table!("i2c", $module_table_name, $table_name);
+ ($($tt:tt)*) => {
+ $crate::module_device_table!("i2c", $crate::i2c::DeviceId, $($tt)*);
};
}
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 965ecca7d55f..166572861e61 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -809,12 +809,10 @@ macro_rules! module_phy_driver {
};
(@device_table [$($dev:expr),+]) => {
- const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
-
- const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
- $crate::device_id::IdArray::new_without_index([ $($dev),+, ]);
-
- $crate::module_device_table!("mdio", phydev, TABLE);
+ $crate::module_device_table!(
+ "mdio", $crate::net::phy::DeviceId,
+ phydev, TABLE, @none, [$($dev),+]
+ );
};
(drivers: [$($driver:ident),+ $(,)?], device_table: [$($dev:expr),+ $(,)?], $($f:tt)*) => {
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 58b20c367f99..35aa6d36d309 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -53,13 +53,7 @@ pub const fn new(compatible: &'static CStr) -> Self {
/// Create an OF `IdTable` with an "alias" for modpost.
#[macro_export]
macro_rules! of_device_table {
- ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
- const $table_name: $crate::device_id::IdArray<
- $crate::of::DeviceId,
- $id_info_type,
- { $table_data.len() },
- > = $crate::device_id::IdArray::new($table_data);
-
- $crate::module_device_table!("of", $module_table_name, $table_name);
+ ($($tt:tt)*) => {
+ $crate::module_device_table!("of", $crate::of::DeviceId, $($tt)*);
};
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 0e055e4df99e..34e07a53244d 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -245,14 +245,8 @@ fn index(&self) -> usize {
/// Create a PCI `IdTable` with its alias for modpost.
#[macro_export]
macro_rules! pci_device_table {
- ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
- const $table_name: $crate::device_id::IdArray<
- $crate::pci::DeviceId,
- $id_info_type,
- { $table_data.len() },
- > = $crate::device_id::IdArray::new($table_data);
-
- $crate::module_device_table!("pci", $module_table_name, $table_name);
+ ($($tt:tt)*) => {
+ $crate::module_device_table!("pci", $crate::pci::DeviceId, $($tt)*);
};
}
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 7aff0c82d0af..154919ee1e19 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -254,14 +254,8 @@ fn index(&self) -> usize {
/// Create a USB `IdTable` with its alias for modpost.
#[macro_export]
macro_rules! usb_device_table {
- ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
- const $table_name: $crate::device_id::IdArray<
- $crate::usb::DeviceId,
- $id_info_type,
- { $table_data.len() },
- > = $crate::device_id::IdArray::new($table_data);
-
- $crate::module_device_table!("usb", $module_table_name, $table_name);
+ ($($tt:tt)*) => {
+ $crate::module_device_table!("usb", $crate::usb::DeviceId, $($tt)*);
};
}
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox